Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

When replacing CodeStyle analyzers do not provide Features analyzers fallback options #75534

Merged
merged 7 commits into from
Oct 24, 2024
Merged

When replacing CodeStyle analyzers do not provide Features analyzers fallback options #75534

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0a838b9
Add a HasSdkCodeStyleAnalyzers property to ProjectAttributes
JoeRobich Oct 16, 2024
180455c
Track unmapped AnalyzerReferences added by the project system.
JoeRobich Oct 16, 2024
03eccdd
Selectively treat some Host analyzers as Project analyzers.
JoeRobich Oct 16, 2024
e9f7c04
Allow the DiagnosticComputer to treat some Host analyzers as Project …
JoeRobich Oct 22, 2024
1198405
Added integration tests
JoeRobich Oct 23, 2024
7344695
PR feedback
JoeRobich Oct 24, 2024
dff04fe
Backport test skip from #75612
JoeRobich Oct 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 71 additions & 4 deletions ...ol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.StateManager.HostStates.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.IO;
using System.Linq;
using Roslyn.Utilities;

Expand Down Expand Up @@ -32,18 +33,84 @@ public IEnumerable<StateSet> 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<object> 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<ImmutableArray<DiagnosticAnalyzer>> HostAnalyzerCollection, IEnumerable<ImmutableArray<DiagnosticAnalyzer>> ProjectAnalyzerCollection) GetAnalyzerCollections(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

def don't like returning these as lazy enumerbles. can you make both an ImmutableArray<ImmutableArray<...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could but then we have to do something about the common case where we aren't redirecting and are returning analyzersPerReference.Values which is an IEnumerable<ImmutableArray<DiagnosticAnalyzer>>. The use of these IEnumerable's is also piped into StateManager.CreateStateSetMap. This might be cleanup for a follow up PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which is an IEnumerable<ImmutableArray>

Ick :) Can that one change?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

at the very least, don't return mutable lists. return an immutable array here. that way we don't have to worry about it changing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to use ArrayBuilder instead of List.

ImmutableDictionary<object, ImmutableArray<DiagnosticAnalyzer>> analyzersPerReference,
ImmutableHashSet<object> referenceIdsToRedirectAsProjectAnalyzers)
{
if (referenceIdsToRedirectAsProjectAnalyzers.IsEmpty)
{
return (analyzersPerReference.Values, []);
}

var hostAnalyzerCollection = new List<ImmutableArray<DiagnosticAnalyzer>>();
var projectAnalyzerCollection = new List<ImmutableArray<DiagnosticAnalyzer>>();

foreach (var kvp in analyzersPerReference)
Copy link
Member

@tmat tmat Oct 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deconstruct: foreach (var (referenceId, analyzers) in analyzersPerReference)

{
if (referenceIdsToRedirectAsProjectAnalyzers.Contains(kvp.Key))
{
projectAnalyzerCollection.Add(kvp.Value);
}
else
{
hostAnalyzerCollection.Add(kvp.Value);
}
}

return (hostAnalyzerCollection, projectAnalyzerCollection);
}
}

private static readonly ImmutableHashSet<string> FeaturesAnalyzerFileNames = [
"Microsoft.CodeAnalysis.Features.dll",
"Microsoft.CodeAnalysis.CSharp.Features.dll",
"Microsoft.CodeAnalysis.VisualBasic.Features.dll",
];

private static ImmutableHashSet<object> 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<object> GetFeaturesAnalyzerReferenceIds(HostDiagnosticAnalyzers hostAnalyzers)
{
var builder = ImmutableHashSet.CreateBuilder<object>();

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
Expand Down
Loading