Merge pull request #40488 from mavasani/AnalyzerSuppression

Remove CompilationWithAnalyzers.IsDiagnosticAnalyzerSuppressed checks…

src/EditorFeatures/Test/Diagnostics/DiagnosticAnalyzerServiceTests.cs

@@ -9,14 +9,11 @@
 using Microsoft.CodeAnalysis.Diagnostics;
 using Microsoft.CodeAnalysis.Diagnostics.CSharp;
 using Microsoft.CodeAnalysis.Diagnostics.EngineV2;
-using Microsoft.CodeAnalysis.Host.Mef;
-using Microsoft.CodeAnalysis.Shared.Options;
 using Microsoft.CodeAnalysis.Shared.TestHooks;
 using Microsoft.CodeAnalysis.SolutionCrawler;
 using Microsoft.CodeAnalysis.Test.Utilities;
 using Microsoft.CodeAnalysis.Text;
 using Microsoft.CodeAnalysis.UnitTests;
-using Microsoft.VisualStudio.Composition;
 using Microsoft.VisualStudio.LanguageServices;
 using Roslyn.Test.Utilities;
 using Roslyn.Utilities;
@@ -96,6 +93,81 @@ public async Task TestHasSuccessfullyLoadedBeingFalseWithCompilerAnalyzerFSAOn()
             await TestAnalyzerAsync(workspace, document, new CSharpCompilerDiagnosticAnalyzer(), CompilerAnalyzerResultSetter, expectedSyntax: true, expectedSemantic: false);
         }
 
+        [Theory]
+        [InlineData(true)]
+        [InlineData(false)]
+        public async Task TestDisabledByDefaultAnalyzerEnabledWithEditorConfig(bool enabledWithEditorconfig)
+        {
+            using var workspace = new AdhocWorkspace();
+            workspace.Options = workspace.Options.WithChangedOption(SolutionCrawlerOptions.BackgroundAnalysisScopeOption, LanguageNames.CSharp, BackgroundAnalysisScope.FullSolution);
+
+            var project = workspace.AddProject(
+                ProjectInfo.Create(
+                    ProjectId.CreateNewId(),
+                    VersionStamp.Create(),
+                    "CSharpProject",
+                    "CSharpProject",
+                    LanguageNames.CSharp,
+                    filePath: "z:\\CSharpProject.csproj"));
+
+            if (enabledWithEditorconfig)
+            {
+                var editorconfigText = @$"
+[*.cs]
+dotnet_diagnostic.{DisabledByDefaultAnalyzer.s_syntaxRule.Id}.severity = warning
+dotnet_diagnostic.{DisabledByDefaultAnalyzer.s_semanticRule.Id}.severity = warning
+dotnet_diagnostic.{DisabledByDefaultAnalyzer.s_compilationRule.Id}.severity = warning";
+
+                project = project.AddAnalyzerConfigDocument(".editorconfig", filePath: "z:\\.editorconfig", text: SourceText.From(editorconfigText)).Project;
+            }
+
+            var document = project.AddDocument("test.cs", SourceText.From("class A {}"), filePath: "z:\\test.cs");
+            var applied = workspace.TryApplyChanges(document.Project.Solution);
+            Assert.True(applied);
+
+            // create listener/service/analyzer
+            var listener = new AsynchronousOperationListener();
+            var service = new MyDiagnosticAnalyzerService(new DisabledByDefaultAnalyzer(), listener);
+            var analyzer = service.CreateIncrementalAnalyzer(workspace);
+
+            // listen to events
+            var syntaxDiagnostic = false;
+            var semanticDiagnostic = false;
+            var compilationDiagnostic = false;
+            service.DiagnosticsUpdated += (s, a) =>
+            {
+                var diagnostic = Assert.Single(a.Diagnostics);
+                Assert.Equal(DiagnosticSeverity.Warning, diagnostic.Severity);
+
+                if (diagnostic.Id == DisabledByDefaultAnalyzer.s_syntaxRule.Id)
+                {
+                    syntaxDiagnostic = true;
+                }
+                else if (diagnostic.Id == DisabledByDefaultAnalyzer.s_semanticRule.Id)
+                {
+                    semanticDiagnostic = true;
+                }
+                else if (diagnostic.Id == DisabledByDefaultAnalyzer.s_compilationRule.Id)
+                {
+                    compilationDiagnostic = true;
+                }
+            };
+
+            // open document
+            workspace.OpenDocument(document.Id);
+            await analyzer.DocumentOpenAsync(document, CancellationToken.None).ConfigureAwait(false);
+
+            // run analysis
+            await RunAllAnalysisAsync(analyzer, document).ConfigureAwait(false);
+
+            // wait for all events to raised
+            await listener.CreateExpeditedWaitTask().ConfigureAwait(false);
+
+            Assert.Equal(enabledWithEditorconfig, syntaxDiagnostic);
+            Assert.Equal(enabledWithEditorconfig, semanticDiagnostic);
+            Assert.Equal(enabledWithEditorconfig, compilationDiagnostic);
+        }
+
         private static async Task TestAnalyzerAsync(
             AdhocWorkspace workspace,
             Document document,
@@ -417,6 +489,22 @@ public override void Initialize(AnalysisContext context)
             }
         }
 
+        private class DisabledByDefaultAnalyzer : DiagnosticAnalyzer
+        {
+            internal static readonly DiagnosticDescriptor s_syntaxRule = new DiagnosticDescriptor("syntax", "test", "test", "test", DiagnosticSeverity.Error, isEnabledByDefault: false);
+            internal static readonly DiagnosticDescriptor s_semanticRule = new DiagnosticDescriptor("semantic", "test", "test", "test", DiagnosticSeverity.Error, isEnabledByDefault: false);
+            internal static readonly DiagnosticDescriptor s_compilationRule = new DiagnosticDescriptor("compilation", "test", "test", "test", DiagnosticSeverity.Error, isEnabledByDefault: false);
+
+            public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(s_syntaxRule, s_semanticRule, s_compilationRule);
+
+            public override void Initialize(AnalysisContext context)
+            {
+                context.RegisterSyntaxTreeAction(c => c.ReportDiagnostic(Diagnostic.Create(s_syntaxRule, c.Tree.GetRoot().GetLocation())));
+                context.RegisterSemanticModelAction(c => c.ReportDiagnostic(Diagnostic.Create(s_semanticRule, c.SemanticModel.SyntaxTree.GetRoot().GetLocation())));
+                context.RegisterCompilationAction(c => c.ReportDiagnostic(Diagnostic.Create(s_compilationRule, c.Compilation.SyntaxTrees.First().GetRoot().GetLocation())));
+            }
+        }
+
         private class OpenFileOnlyAnalyzer : DiagnosticAnalyzer, IBuiltInAnalyzer
         {
             internal static readonly DiagnosticDescriptor s_syntaxRule = new DiagnosticDescriptor("syntax", "test", "test", "test", DiagnosticSeverity.Error, isEnabledByDefault: true);

src/Features/Core/Portable/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.Executor.cs

@@ -46,7 +46,6 @@ internal partial class DiagnosticIncrementalAnalyzer
                     }
 
                     // perf optimization. check whether analyzer is suppressed and avoid getting diagnostics if suppressed.
-                    // REVIEW: IsAnalyzerSuppressed call seems can be quite expensive in certain condition. is there any other way to do this?
                     if (AnalyzerService.IsAnalyzerSuppressed(stateSet.Analyzer, document.Project))
                     {
                         return new DocumentAnalysisData(version, existingData.Items, ImmutableArray<DiagnosticData>.Empty);

src/Features/Core/Portable/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_GetDiagnostics.cs

@@ -266,7 +266,6 @@ protected override bool ShouldIncludeDiagnostic(DiagnosticData diagnostic)
             protected override async Task AppendDiagnosticsAsync(Project project, IEnumerable<DocumentId> documentIds, bool includeProjectNonLocalResult, CancellationToken cancellationToken)
             {
                 // get analyzers that are not suppressed.
-                // REVIEW: IsAnalyzerSuppressed call seems can be quite expensive in certain condition. is there any other way to do this?
                 var stateSets = StateManager.GetOrCreateStateSets(project).Where(s => ShouldIncludeStateSet(project, s)).ToImmutableArrayOrEmpty();
 
                 // unlike the suppressed (disabled) analyzer, we will include hidden diagnostic only analyzers here.
@@ -295,7 +294,6 @@ protected override async Task AppendDiagnosticsAsync(Project project, IEnumerabl
 
             private bool ShouldIncludeStateSet(Project project, StateSet stateSet)
             {
-                // REVIEW: this can be expensive. any way to do this cheaper?
                 var diagnosticService = Owner.AnalyzerService;
                 if (diagnosticService.IsAnalyzerSuppressed(stateSet.Analyzer, project))
                 {

src/Features/Core/Portable/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_GetDiagnosticsForSpan.cs

@@ -67,9 +67,6 @@ private sealed class LatestDiagnosticsForSpanGetter
                  string? diagnosticId = null,
                  CancellationToken cancellationToken = default)
             {
-                // REVIEW: IsAnalyzerSuppressed can be quite expensive in some cases. try to find a way to make it cheaper
-                //         Here we don't filter out hidden diagnostic only analyzer since such analyzer can produce hidden diagnostic
-                //         on active file (non local diagnostic)
                 var stateSets = owner._stateManager
                                      .GetOrCreateStateSets(document.Project).Where(s => !owner.AnalyzerService.IsAnalyzerSuppressed(s.Analyzer, document.Project));
 

src/Features/Core/Portable/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_IncrementalAnalyzer.cs

@@ -98,7 +98,6 @@ private async Task AnalyzeProjectAsync(Project project, bool forceAnalyzerRun, C
 
                 // PERF: get analyzers that are not suppressed and marked as open file only
                 // this is perf optimization. we cache these result since we know the result. (no diagnostics)
-                // REVIEW: IsAnalyzerSuppressed call seems can be quite expensive in certain condition. is there any other way to do this?
                 var activeAnalyzers = stateSets
                                         .Select(s => s.Analyzer)
                                         .Where(a => !AnalyzerService.IsAnalyzerSuppressed(a, project) && !a.IsOpenFileOnly(options));

src/Features/Core/Portable/Diagnostics/HostAnalyzerManager.cs

@@ -154,6 +154,7 @@ private ImmutableArray<DiagnosticDescriptor> GetDiagnosticDescriptorsCore(Diagno
 
         /// <summary>
         /// Return true if the given <paramref name="analyzer"/> is suppressed for the given project.
+        /// NOTE: This API is intended to be used only for performance optimization.
         /// </summary>
         public bool IsAnalyzerSuppressed(DiagnosticAnalyzer analyzer, Project project)
         {
@@ -171,14 +172,12 @@ public bool IsAnalyzerSuppressed(DiagnosticAnalyzer analyzer, Project project)
                 return true;
             }
 
-            // don't capture project
-            var projectId = project.Id;
-
-            // Skip telemetry logging for supported diagnostics, as that can cause an infinite loop.
-            void onAnalyzerException(Exception ex, DiagnosticAnalyzer a, Diagnostic diagnostic) =>
-                    AnalyzerHelper.OnAnalyzerException_NoTelemetryLogging(a, diagnostic, _hostDiagnosticUpdateSource, projectId);
-
-            return CompilationWithAnalyzers.IsDiagnosticAnalyzerSuppressed(analyzer, options, onAnalyzerException);
+            // NOTE: Previously we used to return "CompilationWithAnalyzers.IsDiagnosticAnalyzerSuppressed(options)"
+            //       on this code path, which returns true if analyzer is suppressed through compilation options.
+            //       However, this check is no longer correct as analyzers can be enabled/disabled for individual
+            //       documents through .editorconfig files. So we pessimistically assume analyzer is not suppressed
+            //       and let the core analyzer driver in the compiler layer handle skipping redundant analysis callbacks.
+            return false;
         }
 
         /// <summary>