Skip to content

Provide a way for specific DiagnosticAnalyzers to get specific AnalyzerOptions when running analyzers #80143

@CyrusNajmabadi

Description

@CyrusNajmabadi

Background and Motivation

The IDE has a challenging scenario it supports today. Specifically, because some Analyzers ship both in VS and in teh SDK, it needs to customize behavior depending on which of those it is running. Specifically, if it sees the analyzer is coming from the sdk, it wants to ONLY use options declared in EditorCOnfig without falling back to options set in VS. This mimics the behavior the user will get when running the analyzer from tools like dotnet-format and the like.

Conversely, if not using the SDK, then the analyzers come from VS and then want to respect editorconfig options if present or fallback to reading VS options due to that being hte classic behavior for analyzers since the beginning, and still being how things work for the large set of users not referencing the sdk and not using editorconfig.

Note: when referencing the sdk we still always run VS's analyzers (due to the 'tearing' problem we previously encountered when trying to actually run the sdk analyzers on newer VS apis they did not support).

--

Our solution today is to actually make two separate CompilationWithAnalyzers. One that contains the options that are "only editorconfig options", and another which is the editorconfig options with the fallback to VS options. This is necessary as the current Compiler analyzer apis only allow a single AnalyzerOptions instance to be passed along that applies to all analyzers.

Unsurprisingly, this is enormously costly. In one test scenario, this accounts for 30% of the allocations (to the tune of 8GB of allocs):

Image

To address this, we propose allowing hosts to pass in a callback to provide specific options on a per analyzer basis.

Proposed API

namespace Microsoft.CodeAnalysis.Diagnostics;

public sealed class CompilationWithAnalyzersOptions
{
    // Existing constructor defers to this new one with one new argument:

        public CompilationWithAnalyzersOptions(
           AnalyzerOptions? options,
           Action<Exception, DiagnosticAnalyzer, Diagnostic>? onAnalyzerException,
           bool concurrentAnalysis,
           bool logAnalyzerExecutionTime,
           bool reportSuppressedDiagnostics,
           Func<Exception, bool>? analyzerExceptionFilter,
+           Func<DiagnosticAnalyzer, AnalyzerConfigOptionsProvider>? analyzerSpecificOptionsFactory);

// Also, we can optionally have the following.  It follows the pattern of this type, but is technically optional:

+        /// <summary>
+        /// Callback to allow individual analyzers to have their own <see cref="AnalyzerConfigOptionsProvider"/> distinct
+        /// from the shared instance provided in <see cref="Options"/>.  If <see langword="null"/> then <see cref="Options"/>
+        /// will be used for all analyzers.
+        /// </summary>
+        public Func<DiagnosticAnalyzer, AnalyzerConfigOptionsProvider>? AnalyzerSpecificOptionsFactory { get; }
}

Usage Examples

Within AnalyzerExecutor we use the above prop if present to populate:

+        /// <summary>
+        /// Cache of analyzer to analyzer specific options.  If <see langword="null"/> there are no specific
+        /// options, and the shared options should be used for all analyzers.  Note: this map is generated 
+        /// at construction time, and is unchanging after that point.  So it can be safely read from multiple
+        /// threads without need for locks.
+        /// </summary>
+       private readonly Dictionary<DiagnosticAnalyzer, AnalyzerOptions>? _analyzerToCachedOptions;

// Then, later:

            var options = GetAnalyzerSpecificOptions(syntaxNodeAction.Analyzer);
            var syntaxNodeContext = new SyntaxNodeAnalysisContext(
                node, executionData.DeclaredSymbol, executionData.SemanticModel, options, addDiagnostic,
                isSupportedDiagnostic, executionData.FilterSpan, executionData.IsGeneratedCode, cancellationToken);

Alternatives/Notes

  1. The callback we're providing here is from DiagAnalyzer to AnalyzerConfigOptionsProvider. It could be from DiagAnalyzer to AnalyzerOptions (the type exposed by callback 'context' objects). We chose not to do that because AnalyzerOption holds onto more info that would be weird for the DiagAnalyzer to change. Specifically AnalyzerOptions contains both AnalyzerConfigOptionsProvider and the list of 'AdditionalFiles' to analyze. It doesn't make sense for a DiagAnalyzer to override this. it actually is a bit off that this is an 'option' as opposed to just data passed into the CompilationWithAnalyzers at the top level.
  2. We could consider passing in an single-method interface instead of a delegate callback. CompilationWithAnalyzerOptions already uses delegates for callbacks. For example Action<Exception, DiagnosticAnalyzer, Diagnostic>? onAnalyzerException and Func<Exception, bool>? analyzerExceptionFilter

Metadata

Metadata

Labels

Area-AnalyzersConcept-APIThis issue involves adding, removing, clarification, or modification of an API.Feature Requestapi-approvedAPI was approved in API review, it can be implemented

Type

No type

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions