Skip to content

Commit

Permalink
Only provide fallback options for host analyzers
Browse files Browse the repository at this point in the history
  • Loading branch information
sharwell committed Oct 4, 2024
1 parent dda119a commit 36ec95a
Show file tree
Hide file tree
Showing 39 changed files with 784 additions and 280 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -656,21 +656,27 @@ 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.
const bool FalseButShouldBeTrue = false;
await TestNuGetAndVsixAnalyzerCoreAsync(
nugetAnalyzers: ImmutableArray.Create(firstNugetAnalyzer),
expectedNugetAnalyzersExecuted: true,
vsixAnalyzers: ImmutableArray.Create(vsixAnalyzer),
expectedVsixAnalyzersExecuted: true,
nugetSuppressors: ImmutableArray.Create(nugetSuppressor),
expectedNugetSuppressorsExecuted: true,
expectedNugetSuppressorsExecuted: FalseButShouldBeTrue,
vsixSuppressors: ImmutableArray<VsixSuppressor>.Empty,
expectedVsixSuppressorsExecuted: false,
new[]
{
(Diagnostic("A", "Class").WithLocation(1, 7), nameof(NuGetAnalyzer)),
(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))
(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))
});

// Suppressors with duplicate support for VsixAnalyzer, but not 100% overlap. Verify the following:
Expand All @@ -684,15 +690,15 @@ await TestNuGetAndVsixAnalyzerCoreAsync(
vsixAnalyzers: ImmutableArray.Create(vsixAnalyzer),
expectedVsixAnalyzersExecuted: true,
nugetSuppressors: ImmutableArray.Create(partialNugetSuppressor),
expectedNugetSuppressorsExecuted: true,
expectedNugetSuppressorsExecuted: FalseButShouldBeTrue,
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: true).WithLocation(1, 7), nameof(VsixAnalyzer)),
(Diagnostic("Z", "Class", isSuppressed: true).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))
});

// Suppressors with duplicate support for VsixAnalyzer, with 100% overlap. Verify the following:
Expand All @@ -706,15 +712,15 @@ await TestNuGetAndVsixAnalyzerCoreAsync(
vsixAnalyzers: ImmutableArray.Create(vsixAnalyzer),
expectedVsixAnalyzersExecuted: true,
nugetSuppressors: ImmutableArray.Create(nugetSuppressor),
expectedNugetSuppressorsExecuted: true,
expectedNugetSuppressorsExecuted: FalseButShouldBeTrue,
vsixSuppressors: ImmutableArray.Create(vsixSuppressor),
expectedVsixSuppressorsExecuted: false,
new[]
{
(Diagnostic("A", "Class").WithLocation(1, 7), nameof(NuGetAnalyzer)),
(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))
(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))
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,18 +112,18 @@ private sealed class CombinedAnalyzerConfigOptions(AnalyzerConfigData fileDirect

public override NamingStylePreferences GetNamingStylePreferences()
{
var preferences = _fileDirectoryConfigData.ConfigOptions.GetNamingStylePreferences();
var preferences = _fileDirectoryConfigData.ConfigOptionsWithoutFallback.GetNamingStylePreferences();
if (preferences.IsEmpty && _projectDirectoryConfigData.HasValue)
{
preferences = _projectDirectoryConfigData.Value.ConfigOptions.GetNamingStylePreferences();
preferences = _projectDirectoryConfigData.Value.ConfigOptionsWithoutFallback.GetNamingStylePreferences();
}

return preferences;
}

public override bool TryGetValue(string key, [NotNullWhen(true)] out string? value)
{
if (_fileDirectoryConfigData.ConfigOptions.TryGetValue(key, out value))
if (_fileDirectoryConfigData.ConfigOptionsWithoutFallback.TryGetValue(key, out value))
{
return true;
}
Expand All @@ -134,7 +134,7 @@ public override bool TryGetValue(string key, [NotNullWhen(true)] out string? val
return false;
}

if (_projectDirectoryConfigData.Value.ConfigOptions.TryGetValue(key, out value))
if (_projectDirectoryConfigData.Value.ConfigOptionsWithoutFallback.TryGetValue(key, out value))
{
return true;
}
Expand All @@ -156,23 +156,23 @@ public override IEnumerable<string> Keys
{
get
{
foreach (var key in _fileDirectoryConfigData.ConfigOptions.Keys)
foreach (var key in _fileDirectoryConfigData.ConfigOptionsWithoutFallback.Keys)
yield return key;

if (!_projectDirectoryConfigData.HasValue)
yield break;

foreach (var key in _projectDirectoryConfigData.Value.ConfigOptions.Keys)
foreach (var key in _projectDirectoryConfigData.Value.ConfigOptionsWithoutFallback.Keys)
{
if (!_fileDirectoryConfigData.ConfigOptions.TryGetValue(key, out _))
if (!_fileDirectoryConfigData.ConfigOptionsWithoutFallback.TryGetValue(key, out _))
yield return key;
}

foreach (var (key, severity) in _projectDirectoryConfigData.Value.TreeOptions)
{
var diagnosticKey = "dotnet_diagnostic." + key + ".severity";
if (!_fileDirectoryConfigData.ConfigOptions.TryGetValue(diagnosticKey, out _) &&
!_projectDirectoryConfigData.Value.ConfigOptions.TryGetValue(diagnosticKey, out _))
if (!_fileDirectoryConfigData.ConfigOptionsWithoutFallback.TryGetValue(diagnosticKey, out _) &&
!_projectDirectoryConfigData.Value.ConfigOptionsWithoutFallback.TryGetValue(diagnosticKey, out _))
{
yield return diagnosticKey;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -697,13 +697,13 @@ internal async Task TestOnlyRequiredAnalyzerExecutedDuringDiagnosticComputation(
var analyzer1 = new NamedTypeAnalyzerWithConfigurableEnabledByDefault(isEnabledByDefault: true, DiagnosticSeverity.Warning, throwOnAllNamedTypes: false);
var analyzer1Id = analyzer1.GetAnalyzerId();
var analyzer2 = new NamedTypeAnalyzer();
var analyzerIdsToRequestDiagnostics = new[] { analyzer1Id };
var analyzerIdsToRequestDiagnostics = ImmutableArray.Create(analyzer1Id);
var analyzerReference = new AnalyzerImageReference(ImmutableArray.Create<DiagnosticAnalyzer>(analyzer1, analyzer2));
workspace.TryApplyChanges(workspace.CurrentSolution.WithAnalyzerReferences([analyzerReference]));
var project = workspace.CurrentSolution.Projects.Single();
var document = documentAnalysis ? project.Documents.Single() : null;
var diagnosticsMapResults = await DiagnosticComputer.GetDiagnosticsAsync(
document, project, Checksum.Null, span: null, analyzerIdsToRequestDiagnostics,
document, project, Checksum.Null, span: null, projectAnalyzerIds: [], analyzerIdsToRequestDiagnostics,
AnalysisKind.Semantic, new DiagnosticAnalyzerInfoCache(), workspace.Services,
isExplicit: false, reportSuppressedDiagnostics: false, logPerformanceInfo: false, getTelemetryInfo: false,
cancellationToken: CancellationToken.None);
Expand Down Expand Up @@ -742,7 +742,7 @@ void M()

var analyzer = new FilterSpanTestAnalyzer(kind);
var analyzerId = analyzer.GetAnalyzerId();
var analyzerIdsToRequestDiagnostics = new[] { analyzerId };
var analyzerIdsToRequestDiagnostics = ImmutableArray.Create(analyzerId);
var analyzerReference = new AnalyzerImageReference(ImmutableArray.Create<DiagnosticAnalyzer>(analyzer));
project = project.AddAnalyzerReference(analyzerReference);

Expand Down Expand Up @@ -771,7 +771,7 @@ async Task VerifyCallbackSpanAsync(TextSpan? filterSpan)
: AnalysisKind.Semantic;
var documentToAnalyze = kind == FilterSpanTestAnalyzer.AnalysisKind.AdditionalFile ? additionalDocument : document;
_ = await DiagnosticComputer.GetDiagnosticsAsync(
documentToAnalyze, project, Checksum.Null, filterSpan, analyzerIdsToRequestDiagnostics,
documentToAnalyze, project, Checksum.Null, filterSpan, analyzerIdsToRequestDiagnostics, hostAnalyzerIds: [],
analysisKind, new DiagnosticAnalyzerInfoCache(), workspace.Services,
isExplicit: false, reportSuppressedDiagnostics: false, logPerformanceInfo: false, getTelemetryInfo: false,
CancellationToken.None);
Expand Down Expand Up @@ -820,14 +820,14 @@ void M()
var diagnosticAnalyzerInfoCache = new DiagnosticAnalyzerInfoCache();

var kind = actionKind == AnalyzerRegisterActionKind.SyntaxTree ? AnalysisKind.Syntax : AnalysisKind.Semantic;
var analyzerIds = new[] { analyzer.GetAnalyzerId() };
var analyzerIds = ImmutableArray.Create(analyzer.GetAnalyzerId());

// First invoke analysis with cancellation token, and verify canceled compilation and no reported diagnostics.
Assert.Empty(analyzer.CanceledCompilations);
try
{
_ = await DiagnosticComputer.GetDiagnosticsAsync(document, project, Checksum.Null, span: null,
analyzerIds, kind, diagnosticAnalyzerInfoCache, workspace.Services, isExplicit: false, reportSuppressedDiagnostics: false,
projectAnalyzerIds: [], analyzerIds, kind, diagnosticAnalyzerInfoCache, workspace.Services, isExplicit: false, reportSuppressedDiagnostics: false,
logPerformanceInfo: false, getTelemetryInfo: false, cancellationToken: analyzer.CancellationToken);

throw ExceptionUtilities.Unreachable();
Expand All @@ -840,7 +840,7 @@ void M()

// Then invoke analysis without cancellation token, and verify non-cancelled diagnostic.
var diagnosticsMap = await DiagnosticComputer.GetDiagnosticsAsync(document, project, Checksum.Null, span: null,
analyzerIds, kind, diagnosticAnalyzerInfoCache, workspace.Services, isExplicit: false, reportSuppressedDiagnostics: false,
projectAnalyzerIds: [], analyzerIds, kind, diagnosticAnalyzerInfoCache, workspace.Services, isExplicit: false, reportSuppressedDiagnostics: false,
logPerformanceInfo: false, getTelemetryInfo: false, cancellationToken: CancellationToken.None);
var builder = diagnosticsMap.Diagnostics.Single().diagnosticMap;
var diagnostic = kind == AnalysisKind.Syntax ? builder.Syntax.Single().Item2.Single() : builder.Semantic.Single().Item2.Single();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,7 @@ internal sealed class CSharpSyncNamespacesService(
{
public override AbstractMatchFolderAndNamespaceDiagnosticAnalyzer<SyntaxKind, BaseNamespaceDeclarationSyntax> DiagnosticAnalyzer { get; } = diagnosticAnalyzer;

public override bool IsHostAnalyzer => false;

public override AbstractChangeNamespaceToMatchFolderCodeFixProvider CodeFixProvider { get; } = codeFixProvider;
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// See the LICENSE file in the project root for more information.

using System.Collections.Generic;
using System.Collections.Immutable;
using System.IO;
using System.Linq;
using System.Reflection;
Expand Down Expand Up @@ -51,7 +52,7 @@ private static VersionStamp GetAnalyzerVersion(string path)
public static string GetAnalyzerAssemblyName(this DiagnosticAnalyzer analyzer)
=> analyzer.GetType().Assembly.GetName().Name ?? throw ExceptionUtilities.Unreachable();

public static void AppendAnalyzerMap(this Dictionary<string, DiagnosticAnalyzer> analyzerMap, IEnumerable<DiagnosticAnalyzer> analyzers)
public static void AppendAnalyzerMap(this Dictionary<string, DiagnosticAnalyzer> analyzerMap, ImmutableArray<DiagnosticAnalyzer> analyzers)
{
foreach (var analyzer in analyzers)
{
Expand Down
17 changes: 13 additions & 4 deletions src/Features/Core/Portable/Diagnostics/DiagnosticArguments.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Collections.Immutable;
using System.Diagnostics;
using System.Runtime.Serialization;
using Microsoft.CodeAnalysis.Text;
Expand Down Expand Up @@ -66,7 +67,13 @@ internal class DiagnosticArguments
/// Array of analyzer IDs for analyzers that need to be executed for computing diagnostics.
/// </summary>
[DataMember(Order = 7)]
public string[] AnalyzerIds;
public ImmutableArray<string> ProjectAnalyzerIds;

/// <summary>
/// Array of analyzer IDs for analyzers that need to be executed for computing diagnostics.
/// </summary>
[DataMember(Order = 8)]
public ImmutableArray<string> HostAnalyzerIds;

/// <summary>
/// Indicates diagnostic computation for an explicit user-invoked request,
Expand All @@ -83,14 +90,15 @@ public DiagnosticArguments(
TextSpan? documentSpan,
AnalysisKind? documentAnalysisKind,
ProjectId projectId,
string[] analyzerIds,
ImmutableArray<string> projectAnalyzerIds,
ImmutableArray<string> hostAnalyzerIds,
bool isExplicit)
{
Debug.Assert(documentId != null || documentSpan == null);
Debug.Assert(documentId != null || documentAnalysisKind == null);
Debug.Assert(documentAnalysisKind is null or
(AnalysisKind?)AnalysisKind.Syntax or (AnalysisKind?)AnalysisKind.Semantic);
Debug.Assert(analyzerIds.Length > 0);
Debug.Assert(projectAnalyzerIds.Length > 0 || hostAnalyzerIds.Length > 0);

ReportSuppressedDiagnostics = reportSuppressedDiagnostics;
LogPerformanceInfo = logPerformanceInfo;
Expand All @@ -99,7 +107,8 @@ public DiagnosticArguments(
DocumentSpan = documentSpan;
DocumentAnalysisKind = documentAnalysisKind;
ProjectId = projectId;
AnalyzerIds = analyzerIds;
ProjectAnalyzerIds = projectAnalyzerIds;
HostAnalyzerIds = hostAnalyzerIds;
IsExplicit = isExplicit;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,7 @@ private async Task EnqueueProjectConfigurationChangeWorkItemAsync(ProjectChanges
!object.Equals(oldProject.AssemblyName, newProject.AssemblyName) ||
!object.Equals(oldProject.Name, newProject.Name) ||
!object.Equals(oldProject.AnalyzerOptions, newProject.AnalyzerOptions) ||
!object.Equals(oldProject.HostAnalyzerOptions, newProject.HostAnalyzerOptions) ||
!object.Equals(oldProject.DefaultNamespace, newProject.DefaultNamespace) ||
!object.Equals(oldProject.OutputFilePath, newProject.OutputFilePath) ||
!object.Equals(oldProject.OutputRefFilePath, newProject.OutputRefFilePath) ||
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ internal abstract class AbstractSyncNamespacesService<TSyntaxKind, TNamespaceSyn
where TNamespaceSyntax : SyntaxNode
{
public abstract AbstractMatchFolderAndNamespaceDiagnosticAnalyzer<TSyntaxKind, TNamespaceSyntax> DiagnosticAnalyzer { get; }
public abstract bool IsHostAnalyzer { get; }
public abstract AbstractChangeNamespaceToMatchFolderCodeFixProvider CodeFixProvider { get; }

/// <inheritdoc/>
Expand All @@ -38,7 +39,7 @@ public async Task<Solution> SyncNamespacesAsync(

var solution = projects[0].Solution;
var diagnosticAnalyzers = ImmutableArray.Create<DiagnosticAnalyzer>(DiagnosticAnalyzer);
var diagnosticsByProject = await GetDiagnosticsByProjectAsync(projects, diagnosticAnalyzers, cancellationToken).ConfigureAwait(false);
var diagnosticsByProject = await GetDiagnosticsByProjectAsync(projects, diagnosticAnalyzers, IsHostAnalyzer, cancellationToken).ConfigureAwait(false);

// If no diagnostics are reported, then there is nothing to fix.
if (diagnosticsByProject.Values.All(diagnostics => diagnostics.IsEmpty))
Expand All @@ -57,13 +58,14 @@ public async Task<Solution> SyncNamespacesAsync(
private static async Task<ImmutableDictionary<Project, ImmutableArray<Diagnostic>>> GetDiagnosticsByProjectAsync(
ImmutableArray<Project> projects,
ImmutableArray<DiagnosticAnalyzer> diagnosticAnalyzers,
bool isHostAnalyzer,
CancellationToken cancellationToken)
{
var builder = ImmutableDictionary.CreateBuilder<Project, ImmutableArray<Diagnostic>>();

foreach (var project in projects)
{
var diagnostics = await GetDiagnosticsAsync(project, diagnosticAnalyzers, cancellationToken).ConfigureAwait(false);
var diagnostics = await GetDiagnosticsAsync(project, diagnosticAnalyzers, isHostAnalyzer, cancellationToken).ConfigureAwait(false);
builder.Add(project, diagnostics);
}

Expand All @@ -73,13 +75,14 @@ private static async Task<ImmutableDictionary<Project, ImmutableArray<Diagnostic
private static async Task<ImmutableArray<Diagnostic>> GetDiagnosticsAsync(
Project project,
ImmutableArray<DiagnosticAnalyzer> diagnosticAnalyzers,
bool isHostAnalyzer,
CancellationToken cancellationToken)
{
var compilation = await project.GetCompilationAsync(cancellationToken).ConfigureAwait(false);
RoslynDebug.AssertNotNull(compilation);

var analyzerOptions = new CompilationWithAnalyzersOptions(
project.AnalyzerOptions,
isHostAnalyzer ? project.HostAnalyzerOptions : project.AnalyzerOptions,
onAnalyzerException: null,
concurrentAnalysis: true,
logAnalyzerExecutionTime: false,
Expand Down
Loading

0 comments on commit 36ec95a

Please sign in to comment.