Skip to content

Commit

Permalink
Merge pull request #75317 from sharwell/host-options
Browse files Browse the repository at this point in the history
Only provide fallback options for host analyzers
  • Loading branch information
sharwell authored Oct 10, 2024
2 parents bde21ee + c860af4 commit cf4dbdf
Show file tree
Hide file tree
Showing 60 changed files with 819 additions and 303 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ private class AddNewKeywordAction(Document document, SyntaxNode node) : CodeActi
protected override async Task<Document> GetChangedDocumentAsync(CancellationToken cancellationToken)
{
var root = await _document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
var configOptions = await _document.GetAnalyzerConfigOptionsAsync(cancellationToken).ConfigureAwait(false);
var configOptions = await _document.GetHostAnalyzerConfigOptionsAsync(cancellationToken).ConfigureAwait(false);

var newNode = GetNewNode(_node, configOptions.GetOption(CSharpCodeStyleOptions.PreferredModifierOrder).Value);
var newRoot = root.ReplaceNode(_node, newNode);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
var syntaxRoot = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
var compilationUnit = (CompilationUnitSyntax)syntaxRoot;

var configOptions = await document.GetAnalyzerConfigOptionsAsync(cancellationToken).ConfigureAwait(false);
var configOptions = await document.GetHostAnalyzerConfigOptionsAsync(cancellationToken).ConfigureAwait(false);
var simplifierOptions = new CSharpSimplifierOptions(configOptions);

// Read the preferred placement option and verify if it can be applied to this code file. There are cases
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ public static ImplementTypeOptions GetImplementTypeOptions(this IOptionsReader r

public static async ValueTask<ImplementTypeOptions> GetImplementTypeOptionsAsync(this Document document, CancellationToken cancellationToken)
{
var configOptions = await document.GetAnalyzerConfigOptionsAsync(cancellationToken).ConfigureAwait(false);
var configOptions = await document.GetHostAnalyzerConfigOptionsAsync(cancellationToken).ConfigureAwait(false);
return configOptions.GetImplementTypeOptions(document.Project.Language);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -656,21 +656,28 @@ 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: 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 +691,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 +713,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;
}
2 changes: 1 addition & 1 deletion src/Features/Core/Portable/AddImport/AddImportOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public static AddImportOptions GetAddImportOptions(this IOptionsReader options,

public static async ValueTask<AddImportOptions> GetAddImportOptionsAsync(this Document document, SymbolSearchOptions searchOptions, CancellationToken cancellationToken)
{
var configOptions = await document.GetAnalyzerConfigOptionsAsync(cancellationToken).ConfigureAwait(false);
var configOptions = await document.GetHostAnalyzerConfigOptionsAsync(cancellationToken).ConfigureAwait(false);
return configOptions.GetAddImportOptions(document.Project.Services, searchOptions, document.AllowImportsInHiddenRegions());
}
}
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
Loading

0 comments on commit cf4dbdf

Please sign in to comment.