Deprecate CompilationWithAnalyzers.IsDiagnosticAnalyzerSuppressed public API

[mavasani]

roslyn/src/Compilers/Core/Portable/DiagnosticAnalyzer/CompilationWithAnalyzers.cs

Lines 1224 to 1237 in 5d77712

	/// <summary>
	/// Returns true if all the diagnostics that can be produced by this analyzer are suppressed through options.
	/// </summary>
	/// <param name="analyzer">Analyzer to be checked for suppression.</param>
	/// <param name="options">Compilation options.</param>
	/// <param name="onAnalyzerException">
	/// Optional delegate which is invoked when an analyzer throws an exception.
	/// Delegate can do custom tasks such as report the given analyzer exception diagnostic, report a non-fatal watson for the exception, etc.
	/// </param>
	public static bool IsDiagnosticAnalyzerSuppressed(
	DiagnosticAnalyzer analyzer,
	CompilationOptions options,
	Action<Exception, DiagnosticAnalyzer, Diagnostic>? onAnalyzerException = null)
	{

• This static API on CompilationWithAnalyzers was introduced prior to the editorconfig support in the compiler layer and only accounts for severity configurations via rulesets and command line switches.
• With it's current input parameters, i.e. just the analyzer and CompilationOptions, this API can no longer return the correct value when severities are configured in editorconfig/globalconfig files.
• This API is no longer used anywhere in IDE code: https://sourceroslyn.io/#Microsoft.CodeAnalysis/DiagnosticAnalyzer/CompilationWithAnalyzers.cs,554e9af46aec830e,references
• Supporting this method requires us to thread in null value for compilation in lot of internal driver code, which makes the code unnecessarily complex and unintuitive.

[CyrusNajmabadi]

Yes. Let's def deprecate this :)

[mavasani]

Draft PR: #67671

[333fred]

API Review

• Could we pull the existing impl out so it doesn't have to be updated in the future?
  • This could be pretty complex, it calls into some gnarly code.
  • What's the timeline for deprecation?
• Put it on the BannedAPI list for Roslyn users?

We'll move forward with obsoletion, and investigate how difficult freezing the current implementation will be. If it's easy, then we can remove at the next major version. If it's not, then we'll need to discuss when we want to break the functionality of the API.

[mavasani]

We'll move forward with obsoletion, and investigate how difficult freezing the current implementation will be. If it's easy, then we can remove at the next major version. If it's not, then we'll need to discuss when we want to break the functionality of the API.

Thanks! I was able to refactor the code in #67671 to freeze the current implementation and also restore the unit tests for this public API, while also cleaning up the code in AnalyzerExecutor and AnalyzerManager for rest of the code paths.

[333fred]

@mavasani let's send an email to API review and let them know we plan to move forward with the deprecation/removal in the next major version.

[mavasani]

@mavasani let's send an email to API review and let them know we plan to move forward with the deprecation/removal in the next major version.

FYI: This proposal was accepted via offline email discussion.