Skip to content

Ban DiagnosticDescriptor ctor in CodeStyle layer #50031

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Jan 6, 2021
Merged

Ban DiagnosticDescriptor ctor in CodeStyle layer #50031

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
cf2cd1b
Ban DiagnosticAnalyzer ctor in CodeStyle layer
Youssef1313 Dec 17, 2020
1e2df05
Ban DiagnosticDescriptor instead
Youssef1313 Dec 17, 2020
bc3ada2
Update src/CodeStyle/BannedSymbols.txt
mavasani Dec 17, 2020
3ab3aef
Suppressions
Youssef1313 Jan 5, 2021
4956d33
Merge branch 'master' into patch-5
Youssef1313 Jan 5, 2021
e0bb19e
Help uri
Youssef1313 Jan 5, 2021
c0e231b
Fix tests
Youssef1313 Jan 5, 2021
d33bc90
Update ErrorSquiggleProducerTests.cs
Youssef1313 Jan 6, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/Analyzers/Core/Analyzers/AbstractBuiltInCodeStyleDiagnosticAnalyzer_Core.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ protected static DiagnosticDescriptor CreateDescriptorWithId(
bool isUnnecessary = false,
bool isConfigurable = true,
LocalizableString? description = null)
#pragma warning disable RS0030 // Do not used banned APIs
=> new(
id, title, messageFormat,
DiagnosticCategory.Style,
Expand All @@ -62,6 +63,7 @@ protected static DiagnosticDescriptor CreateDescriptorWithId(
description: description,
helpLinkUri: DiagnosticHelper.GetHelpLinkForDiagnosticId(id),
customTags: DiagnosticCustomTags.Create(isUnnecessary, isConfigurable, enforceOnBuild));
#pragma warning restore RS0030 // Do not used banned APIs

public sealed override void Initialize(AnalysisContext context)
{
Expand Down
2 changes: 2 additions & 0 deletions src/Analyzers/Core/Analyzers/AbstractCodeQualityDiagnosticAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ protected static DiagnosticDescriptor CreateDescriptor(
bool isEnabledByDefault = true,
bool isConfigurable = true,
LocalizableString description = null)
#pragma warning disable RS0030 // Do not used banned APIs
=> new(
id, title, messageFormat,
DiagnosticCategory.CodeQuality,
Expand All @@ -62,5 +63,6 @@ protected static DiagnosticDescriptor CreateDescriptor(
description,
helpLinkUri: DiagnosticHelper.GetHelpLinkForDiagnosticId(id),
customTags: DiagnosticCustomTags.Create(isUnnecessary, isConfigurable, enforceOnBuild));
#pragma warning disable RS0030 // Do not used banned APIs
}
}
9 changes: 9 additions & 0 deletions .../Analyzers/RemoveUnnecessaryImports/AbstractRemoveUnnecessaryImportsDiagnosticAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,15 @@ internal abstract class AbstractRemoveUnnecessaryImportsDiagnosticAnalyzer
// The NotConfigurable custom tag ensures that user can't turn this diagnostic into a warning / error via
// ruleset editor or solution explorer. Setting messageFormat to empty string ensures that we won't display
// this diagnostic in the preview pane header.
#pragma warning disable RS0030 // Do not used banned APIs - We cannot use AbstractBuiltInCodeStyleDiagnosticAnalyzer nor AbstractCodeQualityDiagnosticAnalyzer.
// This analyzer is run against generated code while the abstract base classes mentioned doesn't.
private static readonly DiagnosticDescriptor s_fixableIdDescriptor =
new(DiagnosticFixableId,
title: "", messageFormat: "", category: "",
defaultSeverity: DiagnosticSeverity.Hidden,
isEnabledByDefault: true,
customTags: DiagnosticCustomTags.NotConfigurable);
#pragma warning restore RS0030 // Do not used banned APIs

protected abstract LocalizableString GetTitleAndMessageFormatForClassificationIdDescriptor();
protected abstract ImmutableArray<SyntaxNode> MergeImports(ImmutableArray<SyntaxNode> unnecessaryImports);
Expand All @@ -56,13 +59,15 @@ private void EnsureClassificationIdDescriptors()
{
var titleAndMessageFormat = GetTitleAndMessageFormatForClassificationIdDescriptor();

#pragma warning disable RS0030 // Do not used banned APIs
_unnecessaryClassificationIdDescriptor =
new DiagnosticDescriptor(IDEDiagnosticIds.RemoveUnnecessaryImportsDiagnosticId,
titleAndMessageFormat,
titleAndMessageFormat,
DiagnosticCategory.Style,
DiagnosticSeverity.Hidden,
isEnabledByDefault: true,
helpLinkUri: DiagnosticHelper.GetHelpLinkForDiagnosticId(IDEDiagnosticIds.RemoveUnnecessaryImportsDiagnosticId),
customTags: DiagnosticCustomTags.Unnecessary.Concat(EnforceOnBuildValues.RemoveUnnecessaryImports.ToCustomTag()).ToArray());

_classificationIdDescriptor =
Expand All @@ -72,6 +77,7 @@ private void EnsureClassificationIdDescriptors()
DiagnosticCategory.Style,
DiagnosticSeverity.Hidden,
isEnabledByDefault: true,
helpLinkUri: DiagnosticHelper.GetHelpLinkForDiagnosticId(IDEDiagnosticIds.RemoveUnnecessaryImportsDiagnosticId),
customTags: EnforceOnBuildValues.RemoveUnnecessaryImports.ToCustomTag());

_unnecessaryGeneratedCodeClassificationIdDescriptor =
Expand All @@ -81,6 +87,7 @@ private void EnsureClassificationIdDescriptors()
DiagnosticCategory.Style,
DiagnosticSeverity.Hidden,
isEnabledByDefault: true,
helpLinkUri: DiagnosticHelper.GetHelpLinkForDiagnosticId(IDEDiagnosticIds.RemoveUnnecessaryImportsDiagnosticId),
customTags: DiagnosticCustomTags.UnnecessaryAndNotConfigurable);

_generatedCodeClassificationIdDescriptor =
Expand All @@ -90,7 +97,9 @@ private void EnsureClassificationIdDescriptors()
DiagnosticCategory.Style,
DiagnosticSeverity.Hidden,
isEnabledByDefault: true,
helpLinkUri: DiagnosticHelper.GetHelpLinkForDiagnosticId(IDEDiagnosticIds.RemoveUnnecessaryImportsDiagnosticId),
customTags: DiagnosticCustomTags.NotConfigurable);
#pragma warning restore RS0030 // Do not used banned APIs
}
}

Expand Down
4 changes: 4 additions & 0 deletions ...ers/Core/Analyzers/ValidateFormatString/AbstractValidateFormatStringDiagnosticAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ internal abstract class AbstractValidateFormatStringDiagnosticAnalyzer<TSyntaxKi
AnalyzersResources.ResourceManager,
typeof(AnalyzersResources));

#pragma warning disable RS0030 // Do not used banned APIs - We cannot use AbstractBuiltInCodeStyleDiagnosticAnalyzer nor AbstractCodeQualityDiagnosticAnalyzer.
// This analyzer is run against generated code while the abstract base classes mentioned doesn't. The rule is also not documented.
// There is even a current work to remove the rule completely in favor of CA2241.
private static readonly DiagnosticDescriptor Rule = new(
DiagnosticID,
Title,
Expand All @@ -42,6 +45,7 @@ internal abstract class AbstractValidateFormatStringDiagnosticAnalyzer<TSyntaxKi
isEnabledByDefault: true,
description: Description,
customTags: EnforceOnBuildValues.ValidateFormatString.ToCustomTag());
#pragma warning restore RS0030 // Do not used banned APIs

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics
=> ImmutableArray.Create(Rule);
Expand Down
4 changes: 3 additions & 1 deletion src/CodeStyle/BannedSymbols.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,6 @@ T:Microsoft.CodeAnalysis.Options.OptionSet; 'OptionSet' is not available in Code
T:Microsoft.CodeAnalysis.CodeStyle.CodeStyleOption`1; Use 'Microsoft.CodeAnalysis.CodeStyle.CodeStyleOption2' instead
T:Microsoft.CodeAnalysis.CodeStyle.CodeStyleOptions; Use 'Microsoft.CodeAnalysis.CodeStyle.CodeStyleOptions2' instead
T:Microsoft.CodeAnalysis.Options.OptionKey; Use 'Microsoft.CodeAnalysis.Options.OptionKey2' instead
T:Microsoft.CodeAnalysis.CodeStyle.NotificationOption; Use 'Microsoft.CodeAnalysis.CodeStyle.NotificationOption2' instead
T:Microsoft.CodeAnalysis.CodeStyle.NotificationOption; Use 'Microsoft.CodeAnalysis.CodeStyle.NotificationOption2' instead
M:Microsoft.CodeAnalysis.DiagnosticDescriptor.#ctor(System.String,System.String,System.String,System.String,Microsoft.CodeAnalysis.DiagnosticSeverity,System.Boolean,System.String,System.String,System.String[]); Analyzers should extend 'AbstractBuiltInCodeStyleDiagnosticAnalyzer' or 'AbstractCodeQualityDiagnosticAnalyzer' instead
M:Microsoft.CodeAnalysis.DiagnosticDescriptor.#ctor(System.String,Microsoft.CodeAnalysis.LocalizableString,Microsoft.CodeAnalysis.LocalizableString,System.String,Microsoft.CodeAnalysis.DiagnosticSeverity,System.Boolean,Microsoft.CodeAnalysis.LocalizableString,System.String,System.String[]); Analyzers should extend 'AbstractBuiltInCodeStyleDiagnosticAnalyzer' or 'AbstractCodeQualityDiagnosticAnalyzer' instead
Copy link
Member Author

@Youssef1313 Youssef1313 Jan 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to ban all overloads of the constructor?

Copy link
Contributor

@mavasani mavasani Jan 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe so.

Copy link
Contributor

@mavasani mavasani Jan 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Youssef1313 That would be a feature request for the Banned API analyzer. Feel free to file one on the roslyn-analyzers repo.

4 changes: 2 additions & 2 deletions src/EditorFeatures/CSharpTest/Squiggles/ErrorSquiggleProducerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ void Test()
var expectedToolTip = new ContainerElement(
ContainerElementStyle.Wrapped,
new ClassifiedTextElement(
new ClassifiedTextRun(ClassificationTypeNames.Text, "IDE0005"),
new ClassifiedTextRun(ClassificationTypeNames.Text, "IDE0005", QuickInfoHyperLink.TestAccessor.CreateNavigationAction(new Uri("https://docs.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0005", UriKind.Absolute)), "https://docs.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0005"),
new ClassifiedTextRun(ClassificationTypeNames.Punctuation, ":"),
new ClassifiedTextRun(ClassificationTypeNames.WhiteSpace, " "),
new ClassifiedTextRun(ClassificationTypeNames.Text, CSharpAnalyzersResources.Using_directive_is_unnecessary)));
Expand All @@ -155,7 +155,7 @@ void Test()
expectedToolTip = new ContainerElement(
ContainerElementStyle.Wrapped,
new ClassifiedTextElement(
new ClassifiedTextRun(ClassificationTypeNames.Text, "IDE0005"),
new ClassifiedTextRun(ClassificationTypeNames.Text, "IDE0005", QuickInfoHyperLink.TestAccessor.CreateNavigationAction(new Uri("https://docs.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0005", UriKind.Absolute)), "https://docs.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0005"),
new ClassifiedTextRun(ClassificationTypeNames.Punctuation, ":"),
new ClassifiedTextRun(ClassificationTypeNames.WhiteSpace, " "),
new ClassifiedTextRun(ClassificationTypeNames.Text, CSharpAnalyzersResources.Using_directive_is_unnecessary)));
Expand Down
2 changes: 1 addition & 1 deletion src/EditorFeatures/VisualBasicTest/Squiggles/ErrorSquiggleProducerTests.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ End Class"
Dim expectedToolTip = New ContainerElement(
ContainerElementStyle.Wrapped,
New ClassifiedTextElement(
New ClassifiedTextRun(ClassificationTypeNames.Text, "IDE0005"),
New ClassifiedTextRun(ClassificationTypeNames.Text, "IDE0005", QuickInfoHyperLink.TestAccessor.CreateNavigationAction(new Uri("https://docs.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0005", UriKind.Absolute)), "https://docs.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0005"),
New ClassifiedTextRun(ClassificationTypeNames.Punctuation, ":"),
New ClassifiedTextRun(ClassificationTypeNames.WhiteSpace, " "),
New ClassifiedTextRun(ClassificationTypeNames.Text, VisualBasicAnalyzersResources.Imports_statement_is_unnecessary)))
Expand Down