From 0a838b961c1988eb7abbcd2a3c49373239a269f9 Mon Sep 17 00:00:00 2001 From: Joey Robichaud Date: Wed, 16 Oct 2024 00:29:41 -0700 Subject: [PATCH 1/7] Add a HasSdkCodeStyleAnalyzers property to ProjectAttributes Also plumbs through updating the attribute throughout the various Solution and *State classes. --- .../Workspace/Solution/ProjectInfo.cs | 33 ++++++++++++++++--- .../Workspace/Solution/ProjectState.cs | 12 +++++-- .../Portable/Workspace/Solution/Solution.cs | 11 +++++++ .../Solution/SolutionCompilationState.cs | 13 +++++++- .../Workspace/Solution/SolutionState.cs | 18 ++++++++++ 5 files changed, 78 insertions(+), 9 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/ProjectInfo.cs b/src/Workspaces/Core/Portable/Workspace/Solution/ProjectInfo.cs index 391486737ce7f..dadb0b596045f 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/ProjectInfo.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/ProjectInfo.cs @@ -103,6 +103,11 @@ public sealed class ProjectInfo /// internal bool RunAnalyzers => Attributes.RunAnalyzers; + /// + /// True if the project contains references to the SDK CodeStyle analyzers. + /// + internal bool HasSdkCodeStyleAnalyzers => Attributes.HasSdkCodeStyleAnalyzers; + /// /// The initial compilation options for the project, or null if the default options should be used. /// @@ -391,6 +396,11 @@ internal ProjectInfo WithTelemetryId(Guid telemetryId) return With(attributes: Attributes.With(telemetryId: telemetryId)); } + internal ProjectInfo WithHasSdkCodeStyleAnalyzers(bool hasSdkCodeStyleAnalyzers) + { + return With(attributes: Attributes.With(hasSdkCodeStyleAnalyzers: hasSdkCodeStyleAnalyzers)); + } + internal string GetDebuggerDisplay() => nameof(ProjectInfo) + " " + Name + (!string.IsNullOrWhiteSpace(FilePath) ? " " + FilePath : ""); @@ -413,7 +423,8 @@ internal sealed class ProjectAttributes( Guid telemetryId = default, bool isSubmission = false, bool hasAllInformation = true, - bool runAnalyzers = true) + bool runAnalyzers = true, + bool hasSdkCodeStyleAnalyzers = false) { /// /// Matches names like: Microsoft.CodeAnalysis.Features (netcoreapp3.1) @@ -497,6 +508,11 @@ internal sealed class ProjectAttributes( /// public Guid TelemetryId { get; } = telemetryId; + /// + /// True if the project contains references to the SDK CodeStyle analyzers. + /// + public bool HasSdkCodeStyleAnalyzers { get; } = hasSdkCodeStyleAnalyzers; + private SingleInitNullable<(string? name, string? flavor)> _lazyNameAndFlavor; private SingleInitNullable _lazyChecksum; @@ -527,7 +543,8 @@ public ProjectAttributes With( Optional isSubmission = default, Optional hasAllInformation = default, Optional runAnalyzers = default, - Optional telemetryId = default) + Optional telemetryId = default, + Optional hasSdkCodeStyleAnalyzers = default) { var newId = id ?? Id; var newVersion = version ?? Version; @@ -543,6 +560,7 @@ public ProjectAttributes With( var newHasAllInformation = hasAllInformation.HasValue ? hasAllInformation.Value : HasAllInformation; var newRunAnalyzers = runAnalyzers.HasValue ? runAnalyzers.Value : RunAnalyzers; var newTelemetryId = telemetryId.HasValue ? telemetryId.Value : TelemetryId; + var newHasSdkCodeStyleAnalyzers = hasSdkCodeStyleAnalyzers.HasValue ? hasSdkCodeStyleAnalyzers.Value : HasSdkCodeStyleAnalyzers; if (newId == Id && newVersion == Version && @@ -557,7 +575,8 @@ public ProjectAttributes With( newIsSubmission == IsSubmission && newHasAllInformation == HasAllInformation && newRunAnalyzers == RunAnalyzers && - newTelemetryId == TelemetryId) + newTelemetryId == TelemetryId && + newHasSdkCodeStyleAnalyzers == HasSdkCodeStyleAnalyzers) { return this; } @@ -577,7 +596,8 @@ public ProjectAttributes With( newTelemetryId, newIsSubmission, newHasAllInformation, - newRunAnalyzers); + newRunAnalyzers, + newHasSdkCodeStyleAnalyzers); } public void WriteTo(ObjectWriter writer) @@ -600,6 +620,7 @@ public void WriteTo(ObjectWriter writer) writer.WriteBoolean(HasAllInformation); writer.WriteBoolean(RunAnalyzers); writer.WriteGuid(TelemetryId); + writer.WriteBoolean(HasSdkCodeStyleAnalyzers); // TODO: once CompilationOptions, ParseOptions, ProjectReference, MetadataReference, AnalyzerReference supports // serialization, we should include those here as well. @@ -623,6 +644,7 @@ public static ProjectAttributes ReadFrom(ObjectReader reader) var hasAllInformation = reader.ReadBoolean(); var runAnalyzers = reader.ReadBoolean(); var telemetryId = reader.ReadGuid(); + var hasSdkCodeStyleAnalyzers = reader.ReadBoolean(); return new ProjectAttributes( projectId, @@ -639,7 +661,8 @@ public static ProjectAttributes ReadFrom(ObjectReader reader) telemetryId, isSubmission: isSubmission, hasAllInformation: hasAllInformation, - runAnalyzers: runAnalyzers); + runAnalyzers: runAnalyzers, + hasSdkCodeStyleAnalyzers: hasSdkCodeStyleAnalyzers); } public Checksum Checksum diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/ProjectState.cs b/src/Workspaces/Core/Portable/Workspace/Solution/ProjectState.cs index dfa05d720a367..9fd4c4cd2810c 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/ProjectState.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/ProjectState.cs @@ -597,9 +597,9 @@ public async Task GetSemanticVersionAsync(CancellationToken cancel { var docVersion = await _lazyLatestDocumentTopLevelChangeVersion.GetValueAsync(cancellationToken).ConfigureAwait(false); - // This is unfortunate, however the impact of this is that *any* change to our project-state version will + // This is unfortunate, however the impact of this is that *any* change to our project-state version will // cause us to think the semantic version of the project has changed. Thus, any change to a project property - // that does *not* flow into the compiler still makes us think the semantic version has changed. This is + // that does *not* flow into the compiler still makes us think the semantic version has changed. This is // likely to not be too much of an issue as these changes should be rare, and it's better to be conservative // and assume there was a change than to wrongly presume there was not. return docVersion.GetNewerVersion(this.Version); @@ -675,6 +675,9 @@ public async Task GetSemanticVersionAsync(CancellationToken cancel [DebuggerBrowsable(DebuggerBrowsableState.Collapsed)] public bool RunAnalyzers => this.ProjectInfo.RunAnalyzers; + [DebuggerBrowsable(DebuggerBrowsableState.Collapsed)] + internal bool HasSdkCodeStyleAnalyzers => this.ProjectInfo.HasSdkCodeStyleAnalyzers; + private ProjectState With( ProjectInfo? projectInfo = null, TextDocumentStates? documentStates = null, @@ -736,6 +739,9 @@ public ProjectState WithHasAllInformation(bool hasAllInformation) public ProjectState WithRunAnalyzers(bool runAnalyzers) => (runAnalyzers == RunAnalyzers) ? this : WithNewerAttributes(Attributes.With(runAnalyzers: runAnalyzers, version: Version.GetNewerVersion())); + internal ProjectState WithHasSdkCodeStyleAnalyzers(bool hasSdkCodeStyleAnalyzers) + => (hasSdkCodeStyleAnalyzers == HasSdkCodeStyleAnalyzers) ? this : WithNewerAttributes(Attributes.With(hasSdkCodeStyleAnalyzers: hasSdkCodeStyleAnalyzers, version: Version.GetNewerVersion())); + public ProjectState WithChecksumAlgorithm(SourceHashAlgorithm checksumAlgorithm) { if (checksumAlgorithm == ChecksumAlgorithm) @@ -962,7 +968,7 @@ public ProjectState UpdateDocuments(ImmutableArray oldDocuments, var newDocumentStates = DocumentStates.SetStates(newDocuments); - // When computing the latest dependent version, we just need to know how + // When computing the latest dependent version, we just need to know how GetLatestDependentVersions( newDocumentStates, AdditionalDocumentStates, diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/Solution.cs b/src/Workspaces/Core/Portable/Workspace/Solution/Solution.cs index 796fa8e99cf79..ba736e0fd13ef 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/Solution.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/Solution.cs @@ -532,6 +532,17 @@ internal Solution WithRunAnalyzers(ProjectId projectId, bool runAnalyzers) return WithCompilationState(CompilationState.WithRunAnalyzers(projectId, runAnalyzers)); } + /// + /// Create a new solution instance with the project specified updated to have + /// the specified hasSdkCodeStyleAnalyzers. + /// + internal Solution WithHasSdkCodeStyleAnalyzers(ProjectId projectId, bool hasSdkCodeStyleAnalyzers) + { + CheckContainsProject(projectId); + + return WithCompilationState(CompilationState.WithHasSdkCodeStyleAnalyzers(projectId, hasSdkCodeStyleAnalyzers)); + } + /// /// Creates a new solution instance with the project documents in the order by the specified document ids. /// The specified document ids must be the same as what is already in the project; no adding or removing is allowed. diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionCompilationState.cs b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionCompilationState.cs index 2ec5b6f14a04c..1345ed8d2d0b8 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionCompilationState.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionCompilationState.cs @@ -538,6 +538,16 @@ public SolutionCompilationState WithRunAnalyzers( forkTracker: true); } + /// + internal SolutionCompilationState WithHasSdkCodeStyleAnalyzers( + ProjectId projectId, bool hasSdkCodeStyleAnalyzers) + { + return ForkProject( + this.SolutionState.WithHasSdkCodeStyleAnalyzers(projectId, hasSdkCodeStyleAnalyzers), + translate: null, + forkTracker: true); + } + /// public SolutionCompilationState WithProjectDocumentsOrder( ProjectId projectId, ImmutableList documentIds) @@ -574,7 +584,8 @@ public SolutionCompilationState WithProjectAttributes(ProjectInfo.ProjectAttribu .WithProjectDefaultNamespace(projectId, attributes.DefaultNamespace) .WithHasAllInformation(projectId, attributes.HasAllInformation) .WithRunAnalyzers(projectId, attributes.RunAnalyzers) - .WithProjectChecksumAlgorithm(projectId, attributes.ChecksumAlgorithm); + .WithProjectChecksumAlgorithm(projectId, attributes.ChecksumAlgorithm) + .WithHasSdkCodeStyleAnalyzers(projectId, attributes.HasSdkCodeStyleAnalyzers); } public SolutionCompilationState WithProjectInfo(ProjectInfo info) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.cs b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.cs index 653edb237c4e6..b11825e62330a 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.cs @@ -701,6 +701,24 @@ public StateChange WithRunAnalyzers(ProjectId projectId, bool runAnalyzers) return ForkProject(oldProject, newProject); } + /// + /// Create a new solution instance with the project specified updated to have + /// the specified hasSdkCodeStyleAnalyzers. + /// + internal StateChange WithHasSdkCodeStyleAnalyzers(ProjectId projectId, bool hasSdkCodeStyleAnalyzers) + { + var oldProject = GetRequiredProjectState(projectId); + var newProject = oldProject.WithHasSdkCodeStyleAnalyzers(hasSdkCodeStyleAnalyzers); + + if (oldProject == newProject) + { + return new(this, oldProject, newProject); + } + + // fork without any change on compilation. + return ForkProject(oldProject, newProject); + } + /// /// Create a new solution instance with the project specified updated to include /// the specified project references. From 180455c6047af61c2fcbc947c998534748583346 Mon Sep 17 00:00:00 2001 From: Joey Robichaud Date: Wed, 16 Oct 2024 00:31:54 -0700 Subject: [PATCH 2/7] Track unmapped AnalyzerReferences added by the project system. Use the unmapped references to update the HasSdkCodeStyleAnalyzer ProjectAttribute. --- .../ProjectSystem/ProjectSystemProject.cs | 52 ++++++++++++++++--- .../ProjectSystemProjectFactory.cs | 5 +- 2 files changed, 49 insertions(+), 8 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/ProjectSystem/ProjectSystemProject.cs b/src/Workspaces/Core/Portable/Workspace/ProjectSystem/ProjectSystemProject.cs index cfcfaa42c8103..106cf420cb4d0 100644 --- a/src/Workspaces/Core/Portable/Workspace/ProjectSystem/ProjectSystemProject.cs +++ b/src/Workspaces/Core/Portable/Workspace/ProjectSystem/ProjectSystemProject.cs @@ -58,6 +58,11 @@ internal sealed partial class ProjectSystemProject /// private readonly HashSet _projectAnalyzerPaths = []; + /// + /// The set of unmapped analyzer reference paths that the project knows about. + /// + private readonly HashSet _unmappedProjectAnalyzerPaths = []; + /// /// Paths to analyzers we want to add when the current batch completes. /// @@ -81,6 +86,7 @@ internal sealed partial class ProjectSystemProject private string? _outputFilePath; private string? _outputRefFilePath; private string? _defaultNamespace; + private bool _hasSdkCodeStyleAnalyzers; /// /// If this project is the 'primary' project the project system cares about for a group of Roslyn projects that @@ -446,14 +452,20 @@ private void UpdateRunAnalyzers() ChangeProjectProperty(ref _runAnalyzers, runAnalyzers, s => s.WithRunAnalyzers(Id, runAnalyzers)); } + internal bool HasSdkCodeStyleAnalyzers + { + get => _hasSdkCodeStyleAnalyzers; + set => ChangeProjectProperty(ref _hasSdkCodeStyleAnalyzers, value, s => s.WithHasSdkCodeStyleAnalyzers(Id, value)); + } + /// /// The default namespace of the project. /// /// - /// In C#, this is defined as the value of "rootnamespace" msbuild property. Right now VB doesn't + /// In C#, this is defined as the value of "rootnamespace" msbuild property. Right now VB doesn't /// have the concept of "default namespace", but we conjure one in workspace by assigning the value /// of the project's root namespace to it. So various features can choose to use it for their own purpose. - /// + /// /// In the future, we might consider officially exposing "default namespace" for VB project /// (e.g.through a "defaultnamespace" msbuild property) /// @@ -464,8 +476,8 @@ internal string? DefaultNamespace } /// - /// The max language version supported for this project, if applicable. Useful to help indicate what - /// language version features should be suggested to a user, as well as if they can be upgraded. + /// The max language version supported for this project, if applicable. Useful to help indicate what + /// language version features should be suggested to a user, as well as if they can be upgraded. /// internal string? MaxLangVersion { @@ -857,7 +869,7 @@ public void AddDynamicSourceFile(string dynamicFilePath, ImmutableArray // Don't get confused by _filePath and filePath. // VisualStudioProject._filePath points to csproj/vbproj of the project // and the parameter filePath points to dynamic file such as ASP.NET .g.cs files. - // + // // Also, provider is free-threaded. so fine to call Wait rather than JTF. fileInfo = provider.Value.GetDynamicFileInfoAsync( projectId: Id, projectFilePath: _filePath, filePath: dynamicFilePath, CancellationToken.None).WaitAndGetResult_CanCallOnBackground(CancellationToken.None); @@ -948,7 +960,7 @@ private void OnDynamicFileInfoUpdated(object? sender, string dynamicFilePath) { if (!_dynamicFilePathMaps.TryGetValue(dynamicFilePath, out fileInfoPath)) { - // given file doesn't belong to this project. + // given file doesn't belong to this project. // this happen since the event this is handling is shared between all projects return; } @@ -970,10 +982,18 @@ public void AddAnalyzerReference(string fullPath) var mappedPaths = GetMappedAnalyzerPaths(fullPath); + bool containsSdkCodeStyleAnalyzers; + using var _ = CreateBatchScope(); using (_gate.DisposableWait()) { + // Track the unmapped analyzer paths + _unmappedProjectAnalyzerPaths.Add(fullPath); + + // Determine if we started using SDK CodeStyle analyzers while access to _unmappedProjectAnalyzerPaths is gated. + containsSdkCodeStyleAnalyzers = _unmappedProjectAnalyzerPaths.Any(IsSdkCodeStyleAnalyzer); + // check all mapped paths first, so that all analyzers are either added or not foreach (var mappedFullPath in mappedPaths) { @@ -998,6 +1018,8 @@ public void AddAnalyzerReference(string fullPath) } } } + + HasSdkCodeStyleAnalyzers = containsSdkCodeStyleAnalyzers; } public void RemoveAnalyzerReference(string fullPath) @@ -1007,10 +1029,18 @@ public void RemoveAnalyzerReference(string fullPath) var mappedPaths = GetMappedAnalyzerPaths(fullPath); + bool containsSdkCodeStyleAnalyzers; + using var _ = CreateBatchScope(); using (_gate.DisposableWait()) { + // Track the unmapped analyzer paths + _unmappedProjectAnalyzerPaths.Remove(fullPath); + + // Determine if we are still using SDK CodeStyle analyzers while access to _unmappedProjectAnalyzerPaths is gated. + containsSdkCodeStyleAnalyzers = _unmappedProjectAnalyzerPaths.Any(IsSdkCodeStyleAnalyzer); + // check all mapped paths first, so that all analyzers are either removed or not foreach (var mappedFullPath in mappedPaths) { @@ -1028,6 +1058,8 @@ public void RemoveAnalyzerReference(string fullPath) _analyzersRemovedInBatch.Add(mappedFullPath); } } + + HasSdkCodeStyleAnalyzers = containsSdkCodeStyleAnalyzers; } private OneOrMany GetMappedAnalyzerPaths(string fullPath) @@ -1061,6 +1093,14 @@ private OneOrMany GetMappedAnalyzerPaths(string fullPath) _ => false, }; + private void UpdateHasSdkCodeStyleAnalyzers() + { + var containsSdkCodeStyleAnalyzers = _unmappedProjectAnalyzerPaths.Any(IsSdkCodeStyleAnalyzer); + if (containsSdkCodeStyleAnalyzers == HasSdkCodeStyleAnalyzers) + return; + HasSdkCodeStyleAnalyzers = containsSdkCodeStyleAnalyzers; + } + internal const string RazorVsixExtensionId = "Microsoft.VisualStudio.RazorExtension"; private static readonly string s_razorSourceGeneratorSdkDirectory = CreateDirectoryPathFragment("Sdks", "Microsoft.NET.Sdk.Razor", "source-generators"); private static readonly ImmutableArray s_razorSourceGeneratorAssemblyNames = diff --git a/src/Workspaces/Core/Portable/Workspace/ProjectSystem/ProjectSystemProjectFactory.cs b/src/Workspaces/Core/Portable/Workspace/ProjectSystem/ProjectSystemProjectFactory.cs index 0981fffb29976..fb41649055de1 100644 --- a/src/Workspaces/Core/Portable/Workspace/ProjectSystem/ProjectSystemProjectFactory.cs +++ b/src/Workspaces/Core/Portable/Workspace/ProjectSystem/ProjectSystemProjectFactory.cs @@ -128,7 +128,8 @@ public async Task CreateAndAddToWorkspaceAsync(string proj compilationOutputInfo: new(creationInfo.CompilationOutputAssemblyFilePath), SourceHashAlgorithms.Default, // will be updated when command line is set filePath: creationInfo.FilePath, - telemetryId: creationInfo.TelemetryId), + telemetryId: creationInfo.TelemetryId, + hasSdkCodeStyleAnalyzers: project.HasSdkCodeStyleAnalyzers), compilationOptions: creationInfo.CompilationOptions, parseOptions: creationInfo.ParseOptions); @@ -260,7 +261,7 @@ public void ApplyChangeToWorkspaceWithProjectUpdateState(Func /// Applies a solution transformation to the workspace and triggers workspace changed event for specified . /// The transformation shall only update the project of the solution with the specified . - /// + /// /// The function must be safe to be attempted multiple times (and not update local state). /// public void ApplyChangeToWorkspace(ProjectId projectId, Func solutionTransformation) From 03eccdd7a5be4d29e758154c7745c564fd97b477 Mon Sep 17 00:00:00 2001 From: Joey Robichaud Date: Wed, 16 Oct 2024 13:06:32 -0700 Subject: [PATCH 3/7] Selectively treat some Host analyzers as Project analyzers. When Features analyzers are being used to replace SDK CodeStyle analyzer we need to treat them as project analyzers and not provide them with the Host's fallback options. --- ...ementalAnalyzer.StateManager.HostStates.cs | 75 ++++++++++++++++++- 1 file changed, 71 insertions(+), 4 deletions(-) diff --git a/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.StateManager.HostStates.cs b/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.StateManager.HostStates.cs index f1df195677501..4e637155211b7 100644 --- a/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.StateManager.HostStates.cs +++ b/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.StateManager.HostStates.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.Collections.Immutable; +using System.IO; using System.Linq; using Roslyn.Utilities; @@ -32,18 +33,84 @@ public IEnumerable GetAllHostStateSets() private HostAnalyzerStateSets GetOrCreateHostStateSets(Project project, ProjectAnalyzerStateSets projectStateSets) { var key = new HostAnalyzerStateSetKey(project.Language, project.Solution.SolutionState.Analyzers.HostAnalyzerReferences); - var hostStateSets = ImmutableInterlocked.GetOrAdd(ref _hostAnalyzerStateMap, key, CreateLanguageSpecificAnalyzerMap, project.Solution.SolutionState.Analyzers); + // Some Host Analyzers may need to be treated as Project Analyzers so that they do not have access to the + // Host fallback options. These ids will be used when building up the Host and Project analyzer collections. + var referenceIdsToRedirect = GetReferenceIdsToRedirectAsProjectAnalyzers(project); + var hostStateSets = ImmutableInterlocked.GetOrAdd(ref _hostAnalyzerStateMap, key, CreateLanguageSpecificAnalyzerMap, (project.Solution.SolutionState.Analyzers, referenceIdsToRedirect)); return hostStateSets.WithExcludedAnalyzers(projectStateSets.SkippedAnalyzersInfo.SkippedAnalyzers); - static HostAnalyzerStateSets CreateLanguageSpecificAnalyzerMap(HostAnalyzerStateSetKey arg, HostDiagnosticAnalyzers hostAnalyzers) + static HostAnalyzerStateSets CreateLanguageSpecificAnalyzerMap(HostAnalyzerStateSetKey arg, (HostDiagnosticAnalyzers HostAnalyzers, ImmutableHashSet ReferenceIdsToRedirect) state) { var language = arg.Language; - var analyzersPerReference = hostAnalyzers.GetOrCreateHostDiagnosticAnalyzersPerReference(language); + var analyzersPerReference = state.HostAnalyzers.GetOrCreateHostDiagnosticAnalyzersPerReference(language); - var analyzerMap = CreateStateSetMap(language, [], analyzersPerReference.Values, includeWorkspacePlaceholderAnalyzers: true); + var (hostAnalyzerCollection, projectAnalyzerCollection) = GetAnalyzerCollections(analyzersPerReference, state.ReferenceIdsToRedirect); + var analyzerMap = CreateStateSetMap(language, projectAnalyzerCollection, hostAnalyzerCollection, includeWorkspacePlaceholderAnalyzers: true); return new HostAnalyzerStateSets(analyzerMap); } + + static (IEnumerable> HostAnalyzerCollection, IEnumerable> ProjectAnalyzerCollection) GetAnalyzerCollections( + ImmutableDictionary> analyzersPerReference, + ImmutableHashSet referenceIdsToRedirectAsProjectAnalyzers) + { + if (referenceIdsToRedirectAsProjectAnalyzers.IsEmpty) + { + return (analyzersPerReference.Values, []); + } + + var hostAnalyzerCollection = new List>(); + var projectAnalyzerCollection = new List>(); + + foreach (var kvp in analyzersPerReference) + { + if (referenceIdsToRedirectAsProjectAnalyzers.Contains(kvp.Key)) + { + projectAnalyzerCollection.Add(kvp.Value); + } + else + { + hostAnalyzerCollection.Add(kvp.Value); + } + } + + return (hostAnalyzerCollection, projectAnalyzerCollection); + } + } + + private static readonly ImmutableHashSet FeaturesAnalyzerFileNames = [ + "Microsoft.CodeAnalysis.Features.dll", + "Microsoft.CodeAnalysis.CSharp.Features.dll", + "Microsoft.CodeAnalysis.VisualBasic.Features.dll", + ]; + + private static ImmutableHashSet GetReferenceIdsToRedirectAsProjectAnalyzers(Project project) + { + if (project.State.HasSdkCodeStyleAnalyzers) + { + // When a project uses CodeStyle analyzers added by the SDK, we remove them in favor of the + // Features analyzers. We need to then treat the Features analyzers as Project analyzers so + // they do not get access to the Host fallback options. + return GetFeaturesAnalyzerReferenceIds(project.Solution.SolutionState.Analyzers); + } + + return []; + + static ImmutableHashSet GetFeaturesAnalyzerReferenceIds(HostDiagnosticAnalyzers hostAnalyzers) + { + var builder = ImmutableHashSet.CreateBuilder(); + + foreach (var analyzerReference in hostAnalyzers.HostAnalyzerReferences) + { + var fileName = Path.GetFileName(analyzerReference.FullPath)!; + if (FeaturesAnalyzerFileNames.Contains(fileName)) + { + builder.Add(analyzerReference.Id); + } + } + + return builder.ToImmutable(); + } } private sealed class HostAnalyzerStateSets From e9f7c049d89361b148616b722ba4365493f48c2a Mon Sep 17 00:00:00 2001 From: Joey Robichaud Date: Mon, 21 Oct 2024 23:48:42 -0700 Subject: [PATCH 4/7] Allow the DiagnosticComputer to treat some Host analyzers as Project analyzers. In particular we will treat Features analyzers as project analyzers when a project is using SDK CodeStyle analyzers. This mirrors the change that redirects Features analyzers in the HostStateSets. This is necessary to keep the CompilationsWithAnalyzers in sync with the statesets. --- ...ementalAnalyzer.StateManager.HostStates.cs | 12 +------- .../AnalyzerReferenceTests.vb | 29 +++++++++++++++++++ .../Core/Portable/Diagnostics/Extensions.cs | 22 +++++++++++++- .../DiagnosticAnalyzer/DiagnosticComputer.cs | 26 ++++++++++++++--- 4 files changed, 73 insertions(+), 16 deletions(-) diff --git a/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.StateManager.HostStates.cs b/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.StateManager.HostStates.cs index 4e637155211b7..23493b4cd0ae2 100644 --- a/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.StateManager.HostStates.cs +++ b/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.StateManager.HostStates.cs @@ -5,7 +5,6 @@ using System; using System.Collections.Generic; using System.Collections.Immutable; -using System.IO; using System.Linq; using Roslyn.Utilities; @@ -78,12 +77,6 @@ static HostAnalyzerStateSets CreateLanguageSpecificAnalyzerMap(HostAnalyzerState } } - private static readonly ImmutableHashSet FeaturesAnalyzerFileNames = [ - "Microsoft.CodeAnalysis.Features.dll", - "Microsoft.CodeAnalysis.CSharp.Features.dll", - "Microsoft.CodeAnalysis.VisualBasic.Features.dll", - ]; - private static ImmutableHashSet GetReferenceIdsToRedirectAsProjectAnalyzers(Project project) { if (project.State.HasSdkCodeStyleAnalyzers) @@ -102,11 +95,8 @@ static ImmutableHashSet GetFeaturesAnalyzerReferenceIds(HostDiagnosticAn foreach (var analyzerReference in hostAnalyzers.HostAnalyzerReferences) { - var fileName = Path.GetFileName(analyzerReference.FullPath)!; - if (FeaturesAnalyzerFileNames.Contains(fileName)) - { + if (analyzerReference.IsFeaturesAnalyzer()) builder.Add(analyzerReference.Id); - } } return builder.ToImmutable(); diff --git a/src/VisualStudio/Core/Test/ProjectSystemShim/VisualStudioProjectTests/AnalyzerReferenceTests.vb b/src/VisualStudio/Core/Test/ProjectSystemShim/VisualStudioProjectTests/AnalyzerReferenceTests.vb index a8354283cebcf..6d9f10df1256e 100644 --- a/src/VisualStudio/Core/Test/ProjectSystemShim/VisualStudioProjectTests/AnalyzerReferenceTests.vb +++ b/src/VisualStudio/Core/Test/ProjectSystemShim/VisualStudioProjectTests/AnalyzerReferenceTests.vb @@ -211,12 +211,18 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim Dim project = Await environment.ProjectFactory.CreateAndAddToWorkspaceAsync( "Project", LanguageNames.CSharp, CancellationToken.None) + ' Ensure HasSdkCodeStyleAnalyzers is proper. + Assert.False(project.HasSdkCodeStyleAnalyzers) + ' These are the in-box C# codestyle analyzers that ship with the SDK project.AddAnalyzerReference(Path.Combine(TempRoot.Root, "Sdks", "Microsoft.NET.Sdk", "codestyle", "cs", "Microsoft.CodeAnalysis.CodeStyle.dll")) project.AddAnalyzerReference(Path.Combine(TempRoot.Root, "Sdks", "Microsoft.NET.Sdk", "codestyle", "cs", "Microsoft.CodeAnalysis.CodeStyle.Fixes.dll")) project.AddAnalyzerReference(Path.Combine(TempRoot.Root, "Sdks", "Microsoft.NET.Sdk", "codestyle", "cs", "Microsoft.CodeAnalysis.CSharp.CodeStyle.dll")) project.AddAnalyzerReference(Path.Combine(TempRoot.Root, "Sdks", "Microsoft.NET.Sdk", "codestyle", "cs", "Microsoft.CodeAnalysis.CSharp.CodeStyle.Fixes.dll")) + ' Ensure HasSdkCodeStyleAnalyzers is being properly updated. + Assert.True(project.HasSdkCodeStyleAnalyzers) + ' Ensure they are not returned when getting AnalyzerReferences Assert.Empty(environment.Workspace.CurrentSolution.Projects.Single().AnalyzerReferences) @@ -228,6 +234,15 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim { Path.Combine(TempRoot.Root, "Dir", "File.dll") }, environment.Workspace.CurrentSolution.Projects.Single().AnalyzerReferences.Select(Function(r) r.FullPath)) + + ' Remove codestyle analyzers + project.RemoveAnalyzerReference(Path.Combine(TempRoot.Root, "Sdks", "Microsoft.NET.Sdk", "codestyle", "cs", "Microsoft.CodeAnalysis.CodeStyle.dll")) + project.RemoveAnalyzerReference(Path.Combine(TempRoot.Root, "Sdks", "Microsoft.NET.Sdk", "codestyle", "cs", "Microsoft.CodeAnalysis.CodeStyle.Fixes.dll")) + project.RemoveAnalyzerReference(Path.Combine(TempRoot.Root, "Sdks", "Microsoft.NET.Sdk", "codestyle", "cs", "Microsoft.CodeAnalysis.CSharp.CodeStyle.dll")) + project.RemoveAnalyzerReference(Path.Combine(TempRoot.Root, "Sdks", "Microsoft.NET.Sdk", "codestyle", "cs", "Microsoft.CodeAnalysis.CSharp.CodeStyle.Fixes.dll")) + + ' Ensure HasSdkCodeStyleAnalyzers is being properly updated. + Assert.False(project.HasSdkCodeStyleAnalyzers) End Using End Function @@ -237,12 +252,18 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim Dim project = Await environment.ProjectFactory.CreateAndAddToWorkspaceAsync( "Project", LanguageNames.VisualBasic, CancellationToken.None) + ' Ensure HasSdkCodeStyleAnalyzers is proper. + Assert.False(project.HasSdkCodeStyleAnalyzers) + ' These are the in-box VB codestyle analyzers that ship with the SDK project.AddAnalyzerReference(Path.Combine(TempRoot.Root, "Sdks", "Microsoft.NET.Sdk", "codestyle", "vb", "Microsoft.CodeAnalysis.CodeStyle.dll")) project.AddAnalyzerReference(Path.Combine(TempRoot.Root, "Sdks", "Microsoft.NET.Sdk", "codestyle", "vb", "Microsoft.CodeAnalysis.CodeStyle.Fixes.dll")) project.AddAnalyzerReference(Path.Combine(TempRoot.Root, "Sdks", "Microsoft.NET.Sdk", "codestyle", "vb", "Microsoft.CodeAnalysis.VisualBasic.CodeStyle.dll")) project.AddAnalyzerReference(Path.Combine(TempRoot.Root, "Sdks", "Microsoft.NET.Sdk", "codestyle", "vb", "Microsoft.CodeAnalysis.VisualBasic.CodeStyle.Fixes.dll")) + ' Ensure HasSdkCodeStyleAnalyzers is being properly updated. + Assert.True(project.HasSdkCodeStyleAnalyzers) + ' Ensure they are not returned when getting AnalyzerReferences Assert.Empty(environment.Workspace.CurrentSolution.Projects.Single().AnalyzerReferences) @@ -254,6 +275,14 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim { Path.Combine(TempRoot.Root, "Dir", "File.dll") }, environment.Workspace.CurrentSolution.Projects.Single().AnalyzerReferences.Select(Function(r) r.FullPath)) + + project.RemoveAnalyzerReference(Path.Combine(TempRoot.Root, "Sdks", "Microsoft.NET.Sdk", "codestyle", "vb", "Microsoft.CodeAnalysis.CodeStyle.dll")) + project.RemoveAnalyzerReference(Path.Combine(TempRoot.Root, "Sdks", "Microsoft.NET.Sdk", "codestyle", "vb", "Microsoft.CodeAnalysis.CodeStyle.Fixes.dll")) + project.RemoveAnalyzerReference(Path.Combine(TempRoot.Root, "Sdks", "Microsoft.NET.Sdk", "codestyle", "vb", "Microsoft.CodeAnalysis.VisualBasic.CodeStyle.dll")) + project.RemoveAnalyzerReference(Path.Combine(TempRoot.Root, "Sdks", "Microsoft.NET.Sdk", "codestyle", "vb", "Microsoft.CodeAnalysis.VisualBasic.CodeStyle.Fixes.dll")) + + ' Ensure HasSdkCodeStyleAnalyzers is being properly updated. + Assert.False(project.HasSdkCodeStyleAnalyzers) End Using End Function End Class diff --git a/src/Workspaces/Core/Portable/Diagnostics/Extensions.cs b/src/Workspaces/Core/Portable/Diagnostics/Extensions.cs index b88f6abf6da55..9640e104f1c48 100644 --- a/src/Workspaces/Core/Portable/Diagnostics/Extensions.cs +++ b/src/Workspaces/Core/Portable/Diagnostics/Extensions.cs @@ -6,6 +6,7 @@ using System.Collections.Generic; using System.Collections.Immutable; using System.Diagnostics; +using System.IO; using System.Linq; using System.Threading; using System.Threading.Tasks; @@ -81,7 +82,7 @@ public static string GetAnalyzerId(this DiagnosticAnalyzer analyzer) private static string GetAssemblyQualifiedName(Type type) { - // AnalyzerFileReference now includes things like versions, public key as part of its identity. + // AnalyzerFileReference now includes things like versions, public key as part of its identity. // so we need to consider them. return RoslynImmutableInterlocked.GetOrAdd( ref s_typeToAssemblyQualifiedName, @@ -89,6 +90,25 @@ private static string GetAssemblyQualifiedName(Type type) static type => type.AssemblyQualifiedName ?? throw ExceptionUtilities.UnexpectedValue(type)); } + private static readonly ImmutableHashSet s_featuresAnalyzerFileNames = new[] { + "Microsoft.CodeAnalysis.Features.dll", + "Microsoft.CodeAnalysis.CSharp.Features.dll", + "Microsoft.CodeAnalysis.VisualBasic.Features.dll", + }.ToImmutableHashSet(StringComparer.OrdinalIgnoreCase); + + private static ImmutableSegmentedDictionary s_analyzerFullPathToIsFeatureAnalyzer = ImmutableSegmentedDictionary.Empty; + + public static bool IsFeaturesAnalyzer(this AnalyzerReference reference) + { + if (reference.FullPath is null) + return false; + + return RoslynImmutableInterlocked.GetOrAdd( + ref s_analyzerFullPathToIsFeatureAnalyzer, + reference.FullPath, + static fullPath => s_featuresAnalyzerFileNames.Contains(Path.GetFileName(fullPath))); + } + public static async Task> ToResultBuilderMapAsync( this AnalysisResult analysisResult, ImmutableArray additionalPragmaSuppressionDiagnostics, diff --git a/src/Workspaces/Remote/ServiceHub/Services/DiagnosticAnalyzer/DiagnosticComputer.cs b/src/Workspaces/Remote/ServiceHub/Services/DiagnosticAnalyzer/DiagnosticComputer.cs index dcfca32e3e4d1..825e8bd300781 100644 --- a/src/Workspaces/Remote/ServiceHub/Services/DiagnosticAnalyzer/DiagnosticComputer.cs +++ b/src/Workspaces/Remote/ServiceHub/Services/DiagnosticAnalyzer/DiagnosticComputer.cs @@ -31,7 +31,7 @@ internal class DiagnosticComputer /// The instance is shared between all the following document analyses modes for the project: /// 1. Span-based analysis for active document (lightbulb) /// 2. Background analysis for active and open documents. - /// + /// /// NOTE: We do not re-use this cache for project analysis as it leads to significant memory increase in the OOP process. /// Additionally, we only store the cache entry for the last project to be analyzed instead of maintaining a CWT keyed off /// each project in the solution, as the CWT does not seem to drop entries until ForceGC happens, leading to significant memory @@ -171,7 +171,7 @@ private async Task GetHighPriorityDiagnos // the executing high priority tasks before starting its execution. // - Note that it is critical to do this step prior to Step 3 below to ensure that // any canceled normal priority tasks in Step 3 do not resume execution prior to - // completion of this high priority computeTask. + // completion of this high priority computeTask. lock (s_gate) { Debug.Assert(!s_highPriorityComputeTasks.Contains(computeTask)); @@ -583,7 +583,18 @@ private async Task CreateCompilationWithAnal } var analyzers = reference.GetAnalyzers(_project.Language); - hostAnalyzerBuilder.AddRange(analyzers); + + // At times some Host analyzers should be treated as project analyzers and + // not be given access to the Host fallback options. In particular when we + // replace SDK CodeStyle analyzers with the Features analyzers. + if (ShouldRedirectAnalyzers(_project, reference)) + { + projectAnalyzerBuilder.AddRange(analyzers); + } + else + { + hostAnalyzerBuilder.AddRange(analyzers); + } analyzerMapBuilder.AppendAnalyzerMap(analyzers); } @@ -607,6 +618,13 @@ private async Task CreateCompilationWithAnal var analyzerToIdMap = new BidirectionalMap(analyzerMapBuilder); return new CompilationWithAnalyzersCacheEntry(_solutionChecksum, _project, compilationWithAnalyzers, analyzerToIdMap); + + static bool ShouldRedirectAnalyzers(Project project, AnalyzerReference reference) + { + // When replacing SDK CodeStyle analyzers we should redirect Features analyzers + // so they are treated as project analyzers. + return project.State.HasSdkCodeStyleAnalyzers && reference.IsFeaturesAnalyzer(); + } } private async Task CreateCompilationWithAnalyzerAsync(ImmutableArray projectAnalyzers, ImmutableArray hostAnalyzers, CancellationToken cancellationToken) @@ -626,7 +644,7 @@ private async Task CreateCompilationWithAnalyzerAs // Run analyzers concurrently, with performance logging and reporting suppressed diagnostics. // This allows all client requests with or without performance data and/or suppressed diagnostics to be satisfied. - // TODO: can we support analyzerExceptionFilter in remote host? + // TODO: can we support analyzerExceptionFilter in remote host? // right now, host doesn't support watson, we might try to use new NonFatal watson API? var projectAnalyzerOptions = new CompilationWithAnalyzersOptions( options: _project.AnalyzerOptions, From 1198405c0a5871876286cfd60f91858a4335089e Mon Sep 17 00:00:00 2001 From: Joey Robichaud Date: Wed, 23 Oct 2024 02:46:40 -0700 Subject: [PATCH 5/7] Added integration tests --- .../CSharp/CSharpRedirectFeaturesAnalyzers.cs | 167 ++++++++++++++++++ 1 file changed, 167 insertions(+) create mode 100644 src/VisualStudio/IntegrationTest/New.IntegrationTests/CSharp/CSharpRedirectFeaturesAnalyzers.cs diff --git a/src/VisualStudio/IntegrationTest/New.IntegrationTests/CSharp/CSharpRedirectFeaturesAnalyzers.cs b/src/VisualStudio/IntegrationTest/New.IntegrationTests/CSharp/CSharpRedirectFeaturesAnalyzers.cs new file mode 100644 index 0000000000000..9be3c3b789c61 --- /dev/null +++ b/src/VisualStudio/IntegrationTest/New.IntegrationTests/CSharp/CSharpRedirectFeaturesAnalyzers.cs @@ -0,0 +1,167 @@ +// Licensed to the .NET Foundation under one or more agreements. +// 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.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Formatting; +using Microsoft.CodeAnalysis.Options; +using Microsoft.CodeAnalysis.Shared.TestHooks; +using Roslyn.Test.Utilities; +using Roslyn.VisualStudio.IntegrationTests; +using Roslyn.VisualStudio.NewIntegrationTests.InProcess; +using Xunit; + +namespace Roslyn.VisualStudio.NewIntegrationTests.CSharp; + +public class CSharpRedirectFeaturesAnalyzers : AbstractEditorTest +{ + protected override string LanguageName => LanguageNames.CSharp; + + private const int GlobalIndentationSize = 6; + private const int DefaultIndentationSize = 4; + + [IdeFact] + public async Task DoesNotUseHostOptions_WhenEnforceCodeStyleInBuildIsTrue() + { + await SetupSolutionAsync( + enforceCodeStyleInBuild: true, + GlobalIndentationSize, + HangMitigatingCancellationToken); + + var code = GenerateTestCode(GlobalIndentationSize); + + await TestServices.SolutionExplorer.AddFileAsync(ProjectName, "C.cs", code, open: true, cancellationToken: HangMitigatingCancellationToken); + + var errors = await GetErrorsFromErrorListAsync(HangMitigatingCancellationToken); + AssertEx.Equal( + [ + "C.cs(3, 5): error IDE0055: Fix formatting", + "C.cs(4, 5): error IDE0055: Fix formatting", + "C.cs(6, 5): error IDE0055: Fix formatting", + ], + errors); + } + + [IdeFact] + public async Task UsesHostOptions_WhenEnforceCodeStyleInBuildIsFalse() + { + await SetupSolutionAsync( + enforceCodeStyleInBuild: false, + GlobalIndentationSize, + HangMitigatingCancellationToken); + + var code = GenerateTestCode(DefaultIndentationSize); + + await TestServices.SolutionExplorer.AddFileAsync(ProjectName, "C.cs", code, open: true, cancellationToken: HangMitigatingCancellationToken); + + var errors = await GetErrorsFromErrorListAsync(HangMitigatingCancellationToken); + AssertEx.Equal( + [ + "C.cs(3, 5): error IDE0055: Fix formatting", + "C.cs(4, 5): error IDE0055: Fix formatting", + "C.cs(6, 5): error IDE0055: Fix formatting", + ], + errors); + } + + private async Task SetupSolutionAsync(bool enforceCodeStyleInBuild, int globalIndentationSize, CancellationToken cancellationToken) + { + await TestServices.SolutionExplorer.CreateSolutionAsync(SolutionName, cancellationToken); + + await TestServices.SolutionExplorer.AddCustomProjectAsync( + ProjectName, + ".csproj", + $""" + + + + net8.0 + enable + {enforceCodeStyleInBuild} + + + + """, + cancellationToken); + await TestServices.SolutionExplorer.RestoreNuGetPackagesAsync(ProjectName, cancellationToken); + + // Configure the global indentation size which would be part of the Host fallback options. + var globalOptions = await TestServices.Shell.GetComponentModelServiceAsync(cancellationToken); + globalOptions.SetGlobalOption(FormattingOptions2.UseTabs, LanguageNames.CSharp, false); + globalOptions.SetGlobalOption(FormattingOptions2.IndentationSize, LanguageNames.CSharp, globalIndentationSize); + + // Add .editorconfig to configure so that formatting diagnostics are errors + await TestServices.SolutionExplorer.AddFileAsync(ProjectName, + ".editorconfig", + """ + root = true + + [*.cs] + dotnet_diagnostic.IDE0055.severity = error + """, + open: false, + cancellationToken); + + await TestServices.Workspace.WaitForAllAsyncOperationsAsync( + [ + FeatureAttribute.Workspace, + FeatureAttribute.SolutionCrawlerLegacy, + FeatureAttribute.DiagnosticService, + FeatureAttribute.ErrorSquiggles + ], + cancellationToken); + } + + private static string GenerateTestCode(int indentationSize) + { + var indentation = new string(' ', indentationSize); + return $$""" + class C + { + {{indentation}}void M() + {{indentation}}{ + + {{indentation}}} + } + """; + } + + private async Task> GetErrorsFromErrorListAsync(CancellationToken cancellationToken) + { + await WaitForCodeActionListToPopulateAsync(cancellationToken); + + await TestServices.ErrorList.ShowErrorListAsync(cancellationToken); + await TestServices.Workspace.WaitForAllAsyncOperationsAsync( + [ + FeatureAttribute.Workspace, + FeatureAttribute.SolutionCrawlerLegacy, + FeatureAttribute.DiagnosticService, + FeatureAttribute.ErrorSquiggles, + FeatureAttribute.ErrorList + ], + cancellationToken); + return await TestServices.ErrorList.GetErrorsAsync(cancellationToken); + } + + private async Task WaitForCodeActionListToPopulateAsync(CancellationToken cancellationToken) + { + await TestServices.Editor.ActivateAsync(cancellationToken); + await TestServices.Editor.PlaceCaretAsync("void M()", charsOffset: -1, cancellationToken); + + await TestServices.Editor.InvokeCodeActionListAsync(cancellationToken); + + await TestServices.Workspace.WaitForAllAsyncOperationsAsync( + [ + FeatureAttribute.Workspace, + FeatureAttribute.SolutionCrawlerLegacy, + FeatureAttribute.DiagnosticService, + FeatureAttribute.ErrorSquiggles + ], + cancellationToken); + + await TestServices.Editor.DismissLightBulbSessionAsync(cancellationToken); + } +} From 7344695cc9cfe5a03feef448490701a5a60dfb49 Mon Sep 17 00:00:00 2001 From: Joey Robichaud Date: Wed, 23 Oct 2024 17:55:07 -0700 Subject: [PATCH 6/7] PR feedback --- ...ementalAnalyzer.StateManager.HostStates.cs | 17 +++++----- ...gnosticIncrementalAnalyzer.StateManager.cs | 11 ++++-- .../Core/Portable/Diagnostics/Extensions.cs | 21 +++--------- .../ProjectSystem/ProjectSystemProject.cs | 34 +++++++++---------- 4 files changed, 38 insertions(+), 45 deletions(-) diff --git a/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.StateManager.HostStates.cs b/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.StateManager.HostStates.cs index 23493b4cd0ae2..0cb12b892ce0c 100644 --- a/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.StateManager.HostStates.cs +++ b/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.StateManager.HostStates.cs @@ -6,6 +6,7 @@ using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; +using Microsoft.CodeAnalysis.PooledObjects; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.Diagnostics.EngineV2 @@ -31,7 +32,7 @@ public IEnumerable GetAllHostStateSets() private HostAnalyzerStateSets GetOrCreateHostStateSets(Project project, ProjectAnalyzerStateSets projectStateSets) { - var key = new HostAnalyzerStateSetKey(project.Language, project.Solution.SolutionState.Analyzers.HostAnalyzerReferences); + var key = new HostAnalyzerStateSetKey(project.Language, project.State.HasSdkCodeStyleAnalyzers, project.Solution.SolutionState.Analyzers.HostAnalyzerReferences); // Some Host Analyzers may need to be treated as Project Analyzers so that they do not have access to the // Host fallback options. These ids will be used when building up the Host and Project analyzer collections. var referenceIdsToRedirect = GetReferenceIdsToRedirectAsProjectAnalyzers(project); @@ -58,22 +59,22 @@ static HostAnalyzerStateSets CreateLanguageSpecificAnalyzerMap(HostAnalyzerState return (analyzersPerReference.Values, []); } - var hostAnalyzerCollection = new List>(); - var projectAnalyzerCollection = new List>(); + var hostAnalyzerCollection = ArrayBuilder>.GetInstance(); + var projectAnalyzerCollection = ArrayBuilder>.GetInstance(); - foreach (var kvp in analyzersPerReference) + foreach (var (referenceId, analyzers) in analyzersPerReference) { - if (referenceIdsToRedirectAsProjectAnalyzers.Contains(kvp.Key)) + if (referenceIdsToRedirectAsProjectAnalyzers.Contains(referenceId)) { - projectAnalyzerCollection.Add(kvp.Value); + projectAnalyzerCollection.Add(analyzers); } else { - hostAnalyzerCollection.Add(kvp.Value); + hostAnalyzerCollection.Add(analyzers); } } - return (hostAnalyzerCollection, projectAnalyzerCollection); + return (hostAnalyzerCollection.ToImmutableAndFree(), projectAnalyzerCollection.ToImmutableAndFree()); } } diff --git a/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.StateManager.cs b/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.StateManager.cs index fe69cf93d0371..78e1d431cdbe2 100644 --- a/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.StateManager.cs +++ b/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.StateManager.cs @@ -187,23 +187,28 @@ private static ImmutableDictionary CreateStateSetM private readonly struct HostAnalyzerStateSetKey : IEquatable { - public HostAnalyzerStateSetKey(string language, IReadOnlyList analyzerReferences) + public HostAnalyzerStateSetKey(string language, bool hasSdkCodeStyleAnalyzers, IReadOnlyList analyzerReferences) { Language = language; + HasSdkCodeStyleAnalyzers = hasSdkCodeStyleAnalyzers; AnalyzerReferences = analyzerReferences; } public string Language { get; } + public bool HasSdkCodeStyleAnalyzers { get; } public IReadOnlyList AnalyzerReferences { get; } public bool Equals(HostAnalyzerStateSetKey other) - => Language == other.Language && AnalyzerReferences == other.AnalyzerReferences; + => Language == other.Language && + HasSdkCodeStyleAnalyzers == other.HasSdkCodeStyleAnalyzers && + AnalyzerReferences == other.AnalyzerReferences; public override bool Equals(object? obj) => obj is HostAnalyzerStateSetKey key && Equals(key); public override int GetHashCode() - => Hash.Combine(Language.GetHashCode(), AnalyzerReferences.GetHashCode()); + => Hash.Combine(Language.GetHashCode(), + Hash.Combine(HasSdkCodeStyleAnalyzers.GetHashCode(), AnalyzerReferences.GetHashCode())); } } } diff --git a/src/Workspaces/Core/Portable/Diagnostics/Extensions.cs b/src/Workspaces/Core/Portable/Diagnostics/Extensions.cs index 9640e104f1c48..0226ee3e59f92 100644 --- a/src/Workspaces/Core/Portable/Diagnostics/Extensions.cs +++ b/src/Workspaces/Core/Portable/Diagnostics/Extensions.cs @@ -6,7 +6,6 @@ using System.Collections.Generic; using System.Collections.Immutable; using System.Diagnostics; -using System.IO; using System.Linq; using System.Threading; using System.Threading.Tasks; @@ -90,23 +89,13 @@ private static string GetAssemblyQualifiedName(Type type) static type => type.AssemblyQualifiedName ?? throw ExceptionUtilities.UnexpectedValue(type)); } - private static readonly ImmutableHashSet s_featuresAnalyzerFileNames = new[] { - "Microsoft.CodeAnalysis.Features.dll", - "Microsoft.CodeAnalysis.CSharp.Features.dll", - "Microsoft.CodeAnalysis.VisualBasic.Features.dll", - }.ToImmutableHashSet(StringComparer.OrdinalIgnoreCase); - - private static ImmutableSegmentedDictionary s_analyzerFullPathToIsFeatureAnalyzer = ImmutableSegmentedDictionary.Empty; - public static bool IsFeaturesAnalyzer(this AnalyzerReference reference) { - if (reference.FullPath is null) - return false; - - return RoslynImmutableInterlocked.GetOrAdd( - ref s_analyzerFullPathToIsFeatureAnalyzer, - reference.FullPath, - static fullPath => s_featuresAnalyzerFileNames.Contains(Path.GetFileName(fullPath))); + var fileNameSpan = reference.FullPath.AsSpan(FileNameUtilities.IndexOfFileName(reference.FullPath)); + return + fileNameSpan.Equals("Microsoft.CodeAnalysis.Features.dll".AsSpan(), StringComparison.OrdinalIgnoreCase) || + fileNameSpan.Equals("Microsoft.CodeAnalysis.CSharp.Features.dll".AsSpan(), StringComparison.OrdinalIgnoreCase) || + fileNameSpan.Equals("Microsoft.CodeAnalysis.VisualBasic.Features.dll".AsSpan(), StringComparison.OrdinalIgnoreCase); } public static async Task> ToResultBuilderMapAsync( diff --git a/src/Workspaces/Core/Portable/Workspace/ProjectSystem/ProjectSystemProject.cs b/src/Workspaces/Core/Portable/Workspace/ProjectSystem/ProjectSystemProject.cs index 106cf420cb4d0..ba5ee75b1c622 100644 --- a/src/Workspaces/Core/Portable/Workspace/ProjectSystem/ProjectSystemProject.cs +++ b/src/Workspaces/Core/Portable/Workspace/ProjectSystem/ProjectSystemProject.cs @@ -59,9 +59,9 @@ internal sealed partial class ProjectSystemProject private readonly HashSet _projectAnalyzerPaths = []; /// - /// The set of unmapped analyzer reference paths that the project knows about. + /// The set of SDK code style analyzer reference paths that the project knows about. /// - private readonly HashSet _unmappedProjectAnalyzerPaths = []; + private readonly HashSet _sdkCodeStyleAnalyzerPaths = []; /// /// Paths to analyzers we want to add when the current batch completes. @@ -988,11 +988,14 @@ public void AddAnalyzerReference(string fullPath) using (_gate.DisposableWait()) { - // Track the unmapped analyzer paths - _unmappedProjectAnalyzerPaths.Add(fullPath); + if (IsSdkCodeStyleAnalyzer(fullPath)) + { + // Track the sdk code style analyzer paths + _sdkCodeStyleAnalyzerPaths.Add(fullPath); + } - // Determine if we started using SDK CodeStyle analyzers while access to _unmappedProjectAnalyzerPaths is gated. - containsSdkCodeStyleAnalyzers = _unmappedProjectAnalyzerPaths.Any(IsSdkCodeStyleAnalyzer); + // Determine if we are still using SDK CodeStyle analyzers while access to _sdkCodeStyleAnalyzerPaths is gated. + containsSdkCodeStyleAnalyzers = _sdkCodeStyleAnalyzerPaths.Count > 0; // check all mapped paths first, so that all analyzers are either added or not foreach (var mappedFullPath in mappedPaths) @@ -1035,11 +1038,14 @@ public void RemoveAnalyzerReference(string fullPath) using (_gate.DisposableWait()) { - // Track the unmapped analyzer paths - _unmappedProjectAnalyzerPaths.Remove(fullPath); + if (IsSdkCodeStyleAnalyzer(fullPath)) + { + // Track the sdk code style analyzer paths + _sdkCodeStyleAnalyzerPaths.Remove(fullPath); + } - // Determine if we are still using SDK CodeStyle analyzers while access to _unmappedProjectAnalyzerPaths is gated. - containsSdkCodeStyleAnalyzers = _unmappedProjectAnalyzerPaths.Any(IsSdkCodeStyleAnalyzer); + // Determine if we are still using SDK CodeStyle analyzers while access to _sdkCodeStyleAnalyzerPaths is gated. + containsSdkCodeStyleAnalyzers = _sdkCodeStyleAnalyzerPaths.Count > 0; // check all mapped paths first, so that all analyzers are either removed or not foreach (var mappedFullPath in mappedPaths) @@ -1093,14 +1099,6 @@ private OneOrMany GetMappedAnalyzerPaths(string fullPath) _ => false, }; - private void UpdateHasSdkCodeStyleAnalyzers() - { - var containsSdkCodeStyleAnalyzers = _unmappedProjectAnalyzerPaths.Any(IsSdkCodeStyleAnalyzer); - if (containsSdkCodeStyleAnalyzers == HasSdkCodeStyleAnalyzers) - return; - HasSdkCodeStyleAnalyzers = containsSdkCodeStyleAnalyzers; - } - internal const string RazorVsixExtensionId = "Microsoft.VisualStudio.RazorExtension"; private static readonly string s_razorSourceGeneratorSdkDirectory = CreateDirectoryPathFragment("Sdks", "Microsoft.NET.Sdk.Razor", "source-generators"); private static readonly ImmutableArray s_razorSourceGeneratorAssemblyNames = From dff04feeb6043b363ae099a24906ab2ca65d8b32 Mon Sep 17 00:00:00 2001 From: Joey Robichaud Date: Wed, 23 Oct 2024 21:52:01 -0700 Subject: [PATCH 7/7] Backport test skip from #75612 --- .../ProtocolUnitTests/Completion/CompletionFeaturesTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/LanguageServer/ProtocolUnitTests/Completion/CompletionFeaturesTests.cs b/src/LanguageServer/ProtocolUnitTests/Completion/CompletionFeaturesTests.cs index 19994181f2183..35b09a5472830 100644 --- a/src/LanguageServer/ProtocolUnitTests/Completion/CompletionFeaturesTests.cs +++ b/src/LanguageServer/ProtocolUnitTests/Completion/CompletionFeaturesTests.cs @@ -866,7 +866,7 @@ public ILanguageService CreateLanguageService(HostLanguageServices languageServi } } - [Theory, CombinatorialData] + [Theory(Skip = "https://github.com/dotnet/roslyn/issues/75611"), CombinatorialData] public async Task TestHandleExceptionFromGetCompletionChange(bool mutatingLspWorkspace) { var markup = "Item {|caret:|}";