Include project diagnostic suppressors in host analyzer execution (#7…

…5684) Fixes #75399
dotnet · Nov 22, 2024 · dfa7fc6 · dfa7fc6
2 parents 913fb4e + aa3df16
commit dfa7fc6
Show file tree

Hide file tree

Showing 7 changed files with 310 additions and 121 deletions.
diff --git a/...Features/CSharpTest/Diagnostics/DiagnosticAnalyzerDriver/DiagnosticAnalyzerDriverTests.cs b/...Features/CSharpTest/Diagnostics/DiagnosticAnalyzerDriver/DiagnosticAnalyzerDriverTests.cs
@@ -656,28 +656,21 @@ await TestNuGetAndVsixAnalyzerCoreAsync(
         //   1) No duplicate diagnostics
         //   2) Both NuGet and Vsix analyzers execute
         //   3) Appropriate diagnostic filtering is done - Nuget suppressor suppresses VSIX analyzer.
-        //
-        // 🐛 After splitting fallback options into separate CompilationWithAnalyzers for project and host analyzers,
-        // NuGet-installed suppressors no longer act on VSIX-installed analyzer diagnostics. Fixing this requires us to
-        // add NuGet-installed analyzer references to the host CompilationWithAnalyzers, with an additional flag
-        // indicating that only suppressors should run for these references.
-        // https://github.com/dotnet/roslyn/issues/75399
-        const bool FalseButShouldBeTrue = false;
         await TestNuGetAndVsixAnalyzerCoreAsync(
             nugetAnalyzers: ImmutableArray.Create(firstNugetAnalyzer),
             expectedNugetAnalyzersExecuted: true,
             vsixAnalyzers: ImmutableArray.Create(vsixAnalyzer),
             expectedVsixAnalyzersExecuted: true,
             nugetSuppressors: ImmutableArray.Create(nugetSuppressor),
-            expectedNugetSuppressorsExecuted: FalseButShouldBeTrue,
+            expectedNugetSuppressorsExecuted: true,
             vsixSuppressors: ImmutableArray<VsixSuppressor>.Empty,
             expectedVsixSuppressorsExecuted: false,
             new[]
             {
                 (Diagnostic("A", "Class").WithLocation(1, 7), nameof(NuGetAnalyzer)),
-                (Diagnostic("X", "Class", isSuppressed: FalseButShouldBeTrue).WithLocation(1, 7), nameof(VsixAnalyzer)),
-                (Diagnostic("Y", "Class", isSuppressed: FalseButShouldBeTrue).WithLocation(1, 7), nameof(VsixAnalyzer)),
-                (Diagnostic("Z", "Class", isSuppressed: FalseButShouldBeTrue).WithLocation(1, 7), nameof(VsixAnalyzer))
+                (Diagnostic("X", "Class", isSuppressed: true).WithLocation(1, 7), nameof(VsixAnalyzer)),
+                (Diagnostic("Y", "Class", isSuppressed: true).WithLocation(1, 7), nameof(VsixAnalyzer)),
+                (Diagnostic("Z", "Class", isSuppressed: true).WithLocation(1, 7), nameof(VsixAnalyzer))
             });
 
         // Suppressors with duplicate support for VsixAnalyzer, but not 100% overlap. Verify the following:
@@ -691,15 +684,15 @@ await TestNuGetAndVsixAnalyzerCoreAsync(
             vsixAnalyzers: ImmutableArray.Create(vsixAnalyzer),
             expectedVsixAnalyzersExecuted: true,
             nugetSuppressors: ImmutableArray.Create(partialNugetSuppressor),
-            expectedNugetSuppressorsExecuted: FalseButShouldBeTrue,
+            expectedNugetSuppressorsExecuted: true,
             vsixSuppressors: ImmutableArray.Create(vsixSuppressor),
             expectedVsixSuppressorsExecuted: false,
             new[]
             {
                 (Diagnostic("A", "Class").WithLocation(1, 7), nameof(NuGetAnalyzer)),
                 (Diagnostic("X", "Class", isSuppressed: false).WithLocation(1, 7), nameof(VsixAnalyzer)),
-                (Diagnostic("Y", "Class", isSuppressed: FalseButShouldBeTrue).WithLocation(1, 7), nameof(VsixAnalyzer)),
-                (Diagnostic("Z", "Class", isSuppressed: FalseButShouldBeTrue).WithLocation(1, 7), nameof(VsixAnalyzer))
+                (Diagnostic("Y", "Class", isSuppressed: true).WithLocation(1, 7), nameof(VsixAnalyzer)),
+                (Diagnostic("Z", "Class", isSuppressed: true).WithLocation(1, 7), nameof(VsixAnalyzer))
             });
 
         // Suppressors with duplicate support for VsixAnalyzer, with 100% overlap. Verify the following:
@@ -713,15 +706,15 @@ await TestNuGetAndVsixAnalyzerCoreAsync(
             vsixAnalyzers: ImmutableArray.Create(vsixAnalyzer),
             expectedVsixAnalyzersExecuted: true,
             nugetSuppressors: ImmutableArray.Create(nugetSuppressor),
-            expectedNugetSuppressorsExecuted: FalseButShouldBeTrue,
+            expectedNugetSuppressorsExecuted: true,
             vsixSuppressors: ImmutableArray.Create(vsixSuppressor),
             expectedVsixSuppressorsExecuted: false,
             new[]
             {
                 (Diagnostic("A", "Class").WithLocation(1, 7), nameof(NuGetAnalyzer)),
-                (Diagnostic("X", "Class", isSuppressed: FalseButShouldBeTrue).WithLocation(1, 7), nameof(VsixAnalyzer)),
-                (Diagnostic("Y", "Class", isSuppressed: FalseButShouldBeTrue).WithLocation(1, 7), nameof(VsixAnalyzer)),
-                (Diagnostic("Z", "Class", isSuppressed: FalseButShouldBeTrue).WithLocation(1, 7), nameof(VsixAnalyzer))
+                (Diagnostic("X", "Class", isSuppressed: true).WithLocation(1, 7), nameof(VsixAnalyzer)),
+                (Diagnostic("Y", "Class", isSuppressed: true).WithLocation(1, 7), nameof(VsixAnalyzer)),
+                (Diagnostic("Z", "Class", isSuppressed: true).WithLocation(1, 7), nameof(VsixAnalyzer))
             });
     }
 

diff --git a/src/LanguageServer/Protocol/Features/Diagnostics/DocumentAnalysisExecutor_Helpers.cs b/src/LanguageServer/Protocol/Features/Diagnostics/DocumentAnalysisExecutor_Helpers.cs
@@ -146,6 +146,8 @@ static string GetLanguageSpecificId(string? language, string noLanguageId, strin
             // Create driver that holds onto compilation and associated analyzers
             var filteredProjectAnalyzers = projectAnalyzers.WhereAsArray(static a => !a.IsWorkspaceDiagnosticAnalyzer());
             var filteredHostAnalyzers = hostAnalyzers.WhereAsArray(static a => !a.IsWorkspaceDiagnosticAnalyzer());
+            var filteredProjectSuppressors = filteredProjectAnalyzers.WhereAsArray(static a => a is DiagnosticSuppressor);
+            filteredHostAnalyzers = filteredHostAnalyzers.AddRange(filteredProjectSuppressors);
 
             // PERF: there is no analyzers for this compilation.
             //       compilationWithAnalyzer will throw if it is created with no analyzers which is perf optimization.

diff --git a/...es/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.InProcOrRemoteHostAnalyzerRunner.cs b/...es/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.InProcOrRemoteHostAnalyzerRunner.cs
@@ -122,14 +122,14 @@ private async Task<DiagnosticAnalysisResultMap<DiagnosticAnalyzer, DiagnosticAna
         {
             var version = await DiagnosticIncrementalAnalyzer.GetDiagnosticVersionAsync(project, cancellationToken).ConfigureAwait(false);
 
-            var (projectAnalysisResult, hostAnalysisResult, additionalPragmaSuppressionDiagnostics) = await compilationWithAnalyzers.GetAnalysisResultAsync(
+            var (analysisResult, additionalPragmaSuppressionDiagnostics) = await compilationWithAnalyzers.GetAnalysisResultAsync(
                 documentAnalysisScope, project, AnalyzerInfoCache, cancellationToken).ConfigureAwait(false);
 
             if (logPerformanceInfo)
             {
                 // if remote host is there, report performance data
                 var asyncToken = _asyncOperationListener.BeginAsyncOperation(nameof(AnalyzeInProcAsync));
-                var _ = FireAndForgetReportAnalyzerPerformanceAsync(documentAnalysisScope, project, client, projectAnalysisResult, hostAnalysisResult, cancellationToken).CompletesAsyncOperation(asyncToken);
+                var _ = FireAndForgetReportAnalyzerPerformanceAsync(documentAnalysisScope, project, client, analysisResult, cancellationToken).CompletesAsyncOperation(asyncToken);
             }
 
             var projectAnalyzers = documentAnalysisScope?.ProjectAnalyzers ?? compilationWithAnalyzers.ProjectAnalyzers;
@@ -138,37 +138,20 @@ private async Task<DiagnosticAnalysisResultMap<DiagnosticAnalyzer, DiagnosticAna
 
             // get compiler result builder map
             var builderMap = ImmutableDictionary<DiagnosticAnalyzer, DiagnosticAnalysisResultBuilder>.Empty;
-            if (projectAnalysisResult is not null)
+            if (analysisResult is not null)
             {
-                var map = await projectAnalysisResult.ToResultBuilderMapAsync(
+                var map = await analysisResult.ToResultBuilderMapAsync(
                     additionalPragmaSuppressionDiagnostics, documentAnalysisScope, project, version,
-                    compilationWithAnalyzers.ProjectCompilation!, projectAnalyzers, skippedAnalyzersInfo,
-                    compilationWithAnalyzers.ReportSuppressedDiagnostics, cancellationToken).ConfigureAwait(false);
-                builderMap = builderMap.AddRange(map);
-            }
-
-            if (hostAnalysisResult is not null)
-            {
-                var map = await hostAnalysisResult.ToResultBuilderMapAsync(
-                    additionalPragmaSuppressionDiagnostics, documentAnalysisScope, project, version,
-                    compilationWithAnalyzers.HostCompilation!, hostAnalyzers, skippedAnalyzersInfo,
+                    projectAnalyzers, hostAnalyzers, skippedAnalyzersInfo,
                     compilationWithAnalyzers.ReportSuppressedDiagnostics, cancellationToken).ConfigureAwait(false);
                 builderMap = builderMap.AddRange(map);
             }
 
             var result = builderMap.ToImmutableDictionary(kv => kv.Key, kv => DiagnosticAnalysisResult.CreateFromBuilder(kv.Value));
             var telemetry = ImmutableDictionary<DiagnosticAnalyzer, AnalyzerTelemetryInfo>.Empty;
-            if (getTelemetryInfo)
+            if (getTelemetryInfo && analysisResult is not null)
             {
-                if (projectAnalysisResult is not null)
-                {
-                    telemetry = telemetry.AddRange(projectAnalysisResult.AnalyzerTelemetryInfo);
-                }
-
-                if (hostAnalysisResult is not null)
-                {
-                    telemetry = telemetry.AddRange(hostAnalysisResult.AnalyzerTelemetryInfo);
-                }
+                telemetry = analysisResult.MergedAnalyzerTelemetryInfo;
             }
 
             return DiagnosticAnalysisResultMap.Create(result, telemetry);
@@ -178,8 +161,7 @@ private async Task FireAndForgetReportAnalyzerPerformanceAsync(
             DocumentAnalysisScope? documentAnalysisScope,
             Project project,
             RemoteHostClient? client,
-            AnalysisResult? projectAnalysisResult,
-            AnalysisResult? hostAnalysisResult,
+            AnalysisResultPair? analysisResult,
             CancellationToken cancellationToken)
         {
             if (client == null)
@@ -194,14 +176,9 @@ private async Task FireAndForgetReportAnalyzerPerformanceAsync(
                 var forSpanAnalysis = documentAnalysisScope?.Span.HasValue ?? false;
 
                 ImmutableArray<AnalyzerPerformanceInfo> performanceInfo = [];
-                if (projectAnalysisResult is not null)
-                {
-                    performanceInfo = performanceInfo.AddRange(projectAnalysisResult.AnalyzerTelemetryInfo.ToAnalyzerPerformanceInfo(AnalyzerInfoCache));
-                }
-
-                if (hostAnalysisResult is not null)
+                if (analysisResult is not null)
                 {
-                    performanceInfo = performanceInfo.AddRange(hostAnalysisResult.AnalyzerTelemetryInfo.ToAnalyzerPerformanceInfo(AnalyzerInfoCache));
+                    performanceInfo = performanceInfo.AddRange(analysisResult.MergedAnalyzerTelemetryInfo.ToAnalyzerPerformanceInfo(AnalyzerInfoCache));
                 }
 
                 _ = await client.TryInvokeAsync<IRemoteDiagnosticAnalyzerService>(