Skip to content

Move the StateManager type up to the DiagnosticService from the DiagnosticIncrementalANalyzer #79984

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

Merged
CyrusNajmabadi merged 2 commits into from
Aug 20, 2025
Merged

Move the StateManager type up to the DiagnosticService from the DiagnosticIncrementalANalyzer #79984

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e19ead3
Immediately remote diagnostics call to OOP
CyrusNajmabadi Aug 20, 2025
8fc7874
Move the StateManager type up to the DiagnosticService from the Diagn…
CyrusNajmabadi Aug 20, 2025
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
4 changes: 1 addition & 3 deletions src/Features/Core/Portable/Diagnostics/IDiagnosticAnalyzerService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,7 @@ internal interface IDiagnosticAnalyzerService : IWorkspaceService
/// </remarks>
void RequestDiagnosticRefresh();

/// <summary>
/// Force analyzes the given project by running all applicable analyzers on the project.
/// </summary>
/// <inheritdoc cref="IRemoteDiagnosticAnalyzerService.ForceAnalyzeProjectAsync"/>
Task<ImmutableArray<DiagnosticData>> ForceAnalyzeProjectAsync(Project project, CancellationToken cancellationToken);

/// <summary>
Expand Down
20 changes: 18 additions & 2 deletions src/Features/Core/Portable/Diagnostics/Service/DiagnosticAnalyzerService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ internal sealed partial class DiagnosticAnalyzerService : IDiagnosticAnalyzerSer
private readonly IDiagnosticsRefresher _diagnosticsRefresher;
Copy link
Member Author

@CyrusNajmabadi CyrusNajmabadi Aug 20, 2025

Choose a reason for hiding this comment

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

View with whitespace off.

private readonly DiagnosticIncrementalAnalyzer _incrementalAnalyzer;
private readonly DiagnosticAnalyzerInfoCache _analyzerInfoCache;
private readonly StateManager _stateManager;

public DiagnosticAnalyzerService(
IGlobalOptionService globalOptions,
Expand All @@ -71,6 +72,7 @@ public DiagnosticAnalyzerService(
GlobalOptions = globalOptions;
_diagnosticsRefresher = diagnosticsRefresher;
_incrementalAnalyzer = new DiagnosticIncrementalAnalyzer(this, _analyzerInfoCache, this.GlobalOptions);
_stateManager = new StateManager(_analyzerInfoCache);

globalOptions.AddOptionChangedHandler(this, (_, _, e) =>
{
Expand Down Expand Up @@ -122,8 +124,22 @@ public async Task<ImmutableArray<DiagnosticData>> GetDiagnosticsForSpanAsync(
document, range, shouldIncludeDiagnostic, priorityProvider, diagnosticKinds, cancellationToken).ConfigureAwait(false);
}

public Task<ImmutableArray<DiagnosticData>> ForceAnalyzeProjectAsync(Project project, CancellationToken cancellationToken)
=> _incrementalAnalyzer.ForceAnalyzeProjectAsync(project, cancellationToken);
public async Task<ImmutableArray<DiagnosticData>> ForceAnalyzeProjectAsync(Project project, CancellationToken cancellationToken)
Copy link
Contributor

@AbhitejJohn AbhitejJohn Aug 20, 2025

Choose a reason for hiding this comment

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

Might already know but this one includes the changes from here: #79983. So there's probably some merge fun to be had with this when the other is merged.

Copy link
Member Author

@CyrusNajmabadi CyrusNajmabadi Aug 20, 2025

Choose a reason for hiding this comment

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

Yup. That's why the PR says: > Followup to #79983.

Note: there will be no merge issues. this is a branch off of a branch. So git already knows how the code flows from one into the other.

This is a common pattern for me when i'm going a large refactoring, and i'm trying to keep it from being a ginormous final PR. It also allows me to get smaller bites of code merged in in a less risky fashion.

Copy link
Member

@jasonmalinowski jasonmalinowski Aug 20, 2025

Choose a reason for hiding this comment

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

@CyrusNajmabadi is building off the commits from the other PR, so Git won't care as long he doesn't squash the first PR.

Copy link
Contributor

@AbhitejJohn AbhitejJohn Aug 20, 2025

Choose a reason for hiding this comment

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

ah ok, so you just review the new commit(s) then. That makes sense. Hopefully comments in commits stick (unlike Azure DevOps).

{
var client = await RemoteHostClient.TryGetClientAsync(project, cancellationToken).ConfigureAwait(false);
if (client is not null)
{
var result = await client.TryInvokeAsync<IRemoteDiagnosticAnalyzerService, ImmutableArray<DiagnosticData>>(
project,
(service, solution, cancellationToken) => service.ForceAnalyzeProjectAsync(solution, project.Id, cancellationToken),
cancellationToken).ConfigureAwait(false);

return result.HasValue ? result.Value : [];
}

// No OOP connection. Compute in proc.
return await _incrementalAnalyzer.ForceAnalyzeProjectAsync(project, cancellationToken).ConfigureAwait(false);
}

public Task<ImmutableArray<DiagnosticData>> GetDiagnosticsForIdsAsync(
Project project, DocumentId? documentId, ImmutableHashSet<string>? diagnosticIds, Func<DiagnosticAnalyzer, bool>? shouldIncludeAnalyzer, bool includeLocalDocumentDiagnostics, bool includeNonLocalDocumentDiagnostics, CancellationToken cancellationToken)
Expand Down
115 changes: 55 additions & 60 deletions ...e/Portable/Diagnostics/Service/EngineV2/DiagnosticIncrementalAnalyzer.HostAnalyzerInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,95 +2,90 @@
// 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;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using Microsoft.CodeAnalysis.PooledObjects;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.Diagnostics;

internal sealed partial class DiagnosticAnalyzerService
{
private sealed partial class DiagnosticIncrementalAnalyzer
private sealed partial class StateManager
{
private sealed partial class StateManager
private HostAnalyzerInfo GetOrCreateHostAnalyzerInfo(
SolutionState solution, ProjectState project, ProjectAnalyzerInfo projectAnalyzerInfo)
{
private HostAnalyzerInfo GetOrCreateHostAnalyzerInfo(
SolutionState solution, ProjectState project, ProjectAnalyzerInfo projectAnalyzerInfo)
var key = new HostAnalyzerInfoKey(project.Language, project.HasSdkCodeStyleAnalyzers, solution.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(solution, project);
var hostAnalyzerInfo = ImmutableInterlocked.GetOrAdd(ref _hostAnalyzerStateMap, key, CreateLanguageSpecificAnalyzerMap, (solution.Analyzers, referenceIdsToRedirect));
return hostAnalyzerInfo.WithExcludedAnalyzers(projectAnalyzerInfo.SkippedAnalyzersInfo.SkippedAnalyzers);

static HostAnalyzerInfo CreateLanguageSpecificAnalyzerMap(HostAnalyzerInfoKey arg, (HostDiagnosticAnalyzers HostAnalyzers, ImmutableHashSet<object> ReferenceIdsToRedirect) state)
{
var key = new HostAnalyzerInfoKey(project.Language, project.HasSdkCodeStyleAnalyzers, solution.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(solution, project);
var hostAnalyzerInfo = ImmutableInterlocked.GetOrAdd(ref _hostAnalyzerStateMap, key, CreateLanguageSpecificAnalyzerMap, (solution.Analyzers, referenceIdsToRedirect));
return hostAnalyzerInfo.WithExcludedAnalyzers(projectAnalyzerInfo.SkippedAnalyzersInfo.SkippedAnalyzers);

static HostAnalyzerInfo CreateLanguageSpecificAnalyzerMap(HostAnalyzerInfoKey arg, (HostDiagnosticAnalyzers HostAnalyzers, ImmutableHashSet<object> ReferenceIdsToRedirect) state)
{
var language = arg.Language;
var analyzersPerReference = state.HostAnalyzers.GetOrCreateHostDiagnosticAnalyzersPerReference(language);
var language = arg.Language;
var analyzersPerReference = state.HostAnalyzers.GetOrCreateHostDiagnosticAnalyzersPerReference(language);

var (hostAnalyzerCollection, projectAnalyzerCollection) = GetAnalyzerCollections(analyzersPerReference, state.ReferenceIdsToRedirect);
var (hostAnalyzers, allAnalyzers) = PartitionAnalyzers(projectAnalyzerCollection, hostAnalyzerCollection, includeWorkspacePlaceholderAnalyzers: true);

var (hostAnalyzerCollection, projectAnalyzerCollection) = GetAnalyzerCollections(analyzersPerReference, state.ReferenceIdsToRedirect);
var (hostAnalyzers, allAnalyzers) = PartitionAnalyzers(projectAnalyzerCollection, hostAnalyzerCollection, includeWorkspacePlaceholderAnalyzers: true);
return new HostAnalyzerInfo(hostAnalyzers, allAnalyzers);
}

return new HostAnalyzerInfo(hostAnalyzers, allAnalyzers);
static (IEnumerable<ImmutableArray<DiagnosticAnalyzer>> HostAnalyzerCollection, IEnumerable<ImmutableArray<DiagnosticAnalyzer>> ProjectAnalyzerCollection) GetAnalyzerCollections(
ImmutableDictionary<object, ImmutableArray<DiagnosticAnalyzer>> analyzersPerReference,
ImmutableHashSet<object> referenceIdsToRedirectAsProjectAnalyzers)
{
if (referenceIdsToRedirectAsProjectAnalyzers.IsEmpty)
{
return (analyzersPerReference.Values, []);
}

static (IEnumerable<ImmutableArray<DiagnosticAnalyzer>> HostAnalyzerCollection, IEnumerable<ImmutableArray<DiagnosticAnalyzer>> ProjectAnalyzerCollection) GetAnalyzerCollections(
ImmutableDictionary<object, ImmutableArray<DiagnosticAnalyzer>> analyzersPerReference,
ImmutableHashSet<object> referenceIdsToRedirectAsProjectAnalyzers)
var hostAnalyzerCollection = ArrayBuilder<ImmutableArray<DiagnosticAnalyzer>>.GetInstance();
var projectAnalyzerCollection = ArrayBuilder<ImmutableArray<DiagnosticAnalyzer>>.GetInstance();

foreach (var (referenceId, analyzers) in analyzersPerReference)
{
if (referenceIdsToRedirectAsProjectAnalyzers.IsEmpty)
if (referenceIdsToRedirectAsProjectAnalyzers.Contains(referenceId))
{
return (analyzersPerReference.Values, []);
projectAnalyzerCollection.Add(analyzers);
}

var hostAnalyzerCollection = ArrayBuilder<ImmutableArray<DiagnosticAnalyzer>>.GetInstance();
var projectAnalyzerCollection = ArrayBuilder<ImmutableArray<DiagnosticAnalyzer>>.GetInstance();

foreach (var (referenceId, analyzers) in analyzersPerReference)
else
{
if (referenceIdsToRedirectAsProjectAnalyzers.Contains(referenceId))
{
projectAnalyzerCollection.Add(analyzers);
}
else
{
hostAnalyzerCollection.Add(analyzers);
}
hostAnalyzerCollection.Add(analyzers);
}

return (hostAnalyzerCollection.ToImmutableAndFree(), projectAnalyzerCollection.ToImmutableAndFree());
}

return (hostAnalyzerCollection.ToImmutableAndFree(), projectAnalyzerCollection.ToImmutableAndFree());
}
}

private static ImmutableHashSet<object> GetReferenceIdsToRedirectAsProjectAnalyzers(
SolutionState solution, ProjectState project)
private static ImmutableHashSet<object> GetReferenceIdsToRedirectAsProjectAnalyzers(
SolutionState solution, ProjectState project)
{
if (project.HasSdkCodeStyleAnalyzers)
{
if (project.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(solution.Analyzers);
}

return [];
// 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(solution.Analyzers);
}

static ImmutableHashSet<object> GetFeaturesAnalyzerReferenceIds(HostDiagnosticAnalyzers hostAnalyzers)
{
var builder = ImmutableHashSet.CreateBuilder<object>();
return [];

foreach (var analyzerReference in hostAnalyzers.HostAnalyzerReferences)
{
if (analyzerReference.IsFeaturesAnalyzer())
builder.Add(analyzerReference.Id);
}
static ImmutableHashSet<object> GetFeaturesAnalyzerReferenceIds(HostDiagnosticAnalyzers hostAnalyzers)
{
var builder = ImmutableHashSet.CreateBuilder<object>();

return builder.ToImmutable();
foreach (var analyzerReference in hostAnalyzers.HostAnalyzerReferences)
{
if (analyzerReference.IsFeaturesAnalyzer())
builder.Add(analyzerReference.Id);
}

return builder.ToImmutable();
}
}
}
Copy link
Member Author

@CyrusNajmabadi CyrusNajmabadi Aug 20, 2025

Choose a reason for hiding this comment

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

i didn't move this type into its own file, to keep the diff simple.

Expand Down
Loading
Loading