Skip to content

Commit

Permalink
Include project diagnostic suppressors in host analyzer execution
Browse files Browse the repository at this point in the history
Fixes #75399
  • Loading branch information
sharwell committed Oct 31, 2024
1 parent 16f58fa commit c8d2f1e
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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:
Expand All @@ -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))
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,9 @@ private async Task<DiagnosticAnalysisResultMap<DiagnosticAnalyzer, DiagnosticAna

if (hostAnalysisResult is not null)
{
telemetry = telemetry.AddRange(hostAnalysisResult.AnalyzerTelemetryInfo);
// Use SetItems instead of AddRange so exceptions will not occur if diagnostic suppressors are
// counted in both project and host analysis results.
telemetry = telemetry.SetItems(hostAnalysisResult.AnalyzerTelemetryInfo);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,15 @@ private static (ImmutableArray<DiagnosticAnalyzer> projectAnalyzers, ImmutableAr
}
}

return (projectBuilder.ToImmutableAndClear(), hostBuilder.ToImmutableAndClear());
var projectAnalyzers = projectBuilder.ToImmutableAndClear();

if (hostAnalyzerIds.Any())
{
// If any host analyzers are active, make sure to also include any project diagnostic suppressors
hostBuilder.AddRange(projectAnalyzers.WhereAsArray(static a => a is DiagnosticSuppressor));
}

return (projectAnalyzers, hostBuilder.ToImmutableAndClear());
}

private async Task<(CompilationWithAnalyzersPair? compilationWithAnalyzers, BidirectionalMap<string, DiagnosticAnalyzer> analyzerToIdMap)> GetOrCreateCompilationWithAnalyzersAsync(CancellationToken cancellationToken)
Expand Down Expand Up @@ -609,6 +617,7 @@ private async Task<CompilationWithAnalyzersCacheEntry> CreateCompilationWithAnal

var analyzers = reference.GetAnalyzers(_project.Language);
projectAnalyzerBuilder.AddRange(analyzers);
hostAnalyzerBuilder.AddRange(analyzers.WhereAsArray(static a => a is DiagnosticSuppressor));
analyzerMapBuilder.AppendAnalyzerMap(analyzers);
}

Expand Down

0 comments on commit c8d2f1e

Please sign in to comment.