Ban DiagnosticDescriptor ctor in CodeStyle layer

[Youssef1313]

Fixes #50011

[Youssef1313]

@mavasani I can't figure out why the build is passing here. Can you assist please?

[mavasani]

Sure, let me investigate.

[mavasani]

Hopefully, you should see build failure with RS0030 diagnostics now.

[Youssef1313]

@mavasani The analyzers that cause RS0030 are run against generated code, while AbstractBuiltInCodeStyleDiagnosticAnalyzer doesn't. e.g:

roslyn/src/Analyzers/Core/Analyzers/ValidateFormatString/AbstractValidateFormatStringDiagnosticAnalyzer.cs

Line 71 in d57cddb

context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics);

[mavasani]

@Youssef1313 I wouldn't worry about violation in AbstractValidateFormatStringDiagnosticAnalyzer.cs - just add a source suppression as @Evangelink's PR dotnet/roslyn-analyzers#4226 will soon help us remove that analyzer completely.

I think we do want to run AbstractRemoveUnnecessaryImportsDiagnosticAnalyzer.cs on generated code, so a source suppression with a good comment should be good for it as well.

[Youssef1313]

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

[mavasani]

I don't believe so.

[mavasani]

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

[mavasani]

LGTM

If you can also update the help links for IDE0005 descriptors, that will be nice. Thanks!

[mavasani]

@Youssef1313 You also have some merge conflicts.

[Youssef1313]

roslyn/src/Features/Core/Portable/CodeStyle/AbstractCodeStyleProvider.cs

Lines 77 to 85 in d57cddb

	protected DiagnosticDescriptor CreateDescriptorWithId(
	LocalizableString title, LocalizableString message)
	{
	return new DiagnosticDescriptor(
	_descriptorId, title, message,
	DiagnosticCategory.Style,
	DiagnosticSeverity.Hidden,
	isEnabledByDefault: true);
	}

The above method is used by IDE0053

We should check whether it currently has the correct link. If it doesn't and the current build didn't fail, we need think of an alternative/enhancement. Probably add a new test to [IDEDiagnosticIDConfigurationTests] (https://github.com/dotnet/roslyn/blob/5eae4001901cb03a71c668f2aaa42783ea50570b/src/EditorFeatures/Test/Diagnostics/IDEDiagnosticIDConfigurationTests.cs) for validating the help links.

[mavasani]

We should check whether it currently has the correct link. If it doesn't and the current build didn't fail, we need think of an alternative/enhancement

@Youssef1313 I think we are fine here - we don't expect many analyzers to be IDE-only analyzers (not present in CodeStyle layer). The guidance for future is absolutely to add analyzers to the shared layer, which implicitly adds them to CodeStyle layer. If absolutely required, we can also add the banned symbols added in this PR to https://github.com/dotnet/roslyn/blob/master/src/Features/BannedSymbols.txt

[mavasani]

@Youssef1313 Tests are still failing.

[ghost]

Auto-approval