Skip to content

Remove 'filters' from IDiagService and make it a pure data api. #80177

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
merged 7 commits into from
Sep 9, 2025
Merged

Remove 'filters' from IDiagService and make it a pure data api. #80177

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9e30303
Do not expose in proc filters in diag service
CyrusNajmabadi Sep 8, 2025
890672d
In progress
CyrusNajmabadi Sep 8, 2025
561adde
In progress
CyrusNajmabadi Sep 8, 2025
391e760
Downstream
CyrusNajmabadi Sep 8, 2025
d460c2f
Tests
CyrusNajmabadi Sep 8, 2025
08c722b
Merge remote-tracking branch 'upstream/main' into diagnosticFilters
CyrusNajmabadi Sep 8, 2025
dcf28e9
Rename
CyrusNajmabadi Sep 8, 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
2 changes: 1 addition & 1 deletion src/EditorFeatures/Test/CodeFixes/CodeFixServiceTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1060,7 +1060,7 @@ await VerifyCachedDiagnosticsAsync(
: root.DescendantNodes().OfType<CodeAnalysis.CSharp.Syntax.InvocationExpressionSyntax>().First().Span;

await analyzerService.GetDiagnosticsForIdsAsync(
sourceDocument.Project, [sourceDocument.Id], diagnosticIds: null, shouldIncludeAnalyzer: null,
sourceDocument.Project, [sourceDocument.Id], diagnosticIds: null, AnalyzerFilter.All,
Copy link
Member Author

Choose a reason for hiding this comment

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

callback replaced with pure-data enum. 'null' means 'include everything'.

Copy link
Member

Choose a reason for hiding this comment

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

filter appears to be required and never nullable, or did I miss something

includeLocalDocumentDiagnostics: true, CancellationToken.None);
// await diagnosticIncrementalAnalyzer.GetTestAccessor().TextDocumentOpenAsync(sourceDocument);

Expand Down
16 changes: 8 additions & 8 deletions src/EditorFeatures/Test/Diagnostics/DiagnosticAnalyzerServiceTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public async Task TestHasSuccessfullyLoadedBeingFalse()
var service = workspace.Services.GetRequiredService<IDiagnosticAnalyzerService>();

var diagnostics = await service.GetDiagnosticsForIdsAsync(
workspace.CurrentSolution.Projects.Single(), documentIds: default, diagnosticIds: null, shouldIncludeAnalyzer: null,
workspace.CurrentSolution.Projects.Single(), documentIds: default, diagnosticIds: null, AnalyzerFilter.All,
includeLocalDocumentDiagnostics: true, CancellationToken.None);
Assert.NotEmpty(diagnostics);
}
Expand Down Expand Up @@ -634,7 +634,7 @@ internal async Task TestOnlyRequiredAnalyzerExecutedDuringDiagnosticComputation(

var service = project.Solution.Services.GetRequiredService<IDiagnosticAnalyzerService>();
var diagnosticsMapResults = await service.GetDiagnosticsForIdsAsync(
project, documentIds: default, [analyzer1.Descriptor.Id], shouldIncludeAnalyzer: null, includeLocalDocumentDiagnostics: true, CancellationToken.None);
project, documentIds: default, [analyzer1.Descriptor.Id], AnalyzerFilter.All, includeLocalDocumentDiagnostics: true, CancellationToken.None);
Assert.False(analyzer2.ReceivedSymbolCallback);

Assert.Equal(1, diagnosticsMapResults.Length);
Expand All @@ -659,7 +659,7 @@ internal async Task TestAnalysisWithAnalyzerInBothProjectAndHost_SameAnalyzerIns
var document = documentAnalysis ? project.Documents.Single() : null;
var service = project.Solution.Services.GetRequiredService<IDiagnosticAnalyzerService>();
var diagnosticsMapResults = await service.GetDiagnosticsForIdsAsync(
project, documentIds: default, [analyzer.Descriptor.Id], shouldIncludeAnalyzer: null, includeLocalDocumentDiagnostics: true, CancellationToken.None);
project, documentIds: default, [analyzer.Descriptor.Id], AnalyzerFilter.All, includeLocalDocumentDiagnostics: true, CancellationToken.None);

// In this case, since the analyzer identity is identical, we ran it once
Assert.Single(diagnosticsMapResults);
Expand Down Expand Up @@ -692,7 +692,7 @@ internal async Task TestAnalysisWithAnalyzerInBothProjectAndHost_DifferentAnalyz
var service = project.Solution.Services.GetRequiredService<IDiagnosticAnalyzerService>();
var diagnosticsMapResults = await service.GetDiagnosticsForIdsAsync(
project, documentIds: default, [analyzerProject.Descriptor.Id, analyzerHost.Descriptor.Id],
shouldIncludeAnalyzer: null, includeLocalDocumentDiagnostics: true, CancellationToken.None);
AnalyzerFilter.All, includeLocalDocumentDiagnostics: true, CancellationToken.None);

// In this case, since the analyzer reference identity is identical, we ran it once
var analyzerResults = diagnosticsMapResults.Single();
Expand Down Expand Up @@ -723,7 +723,7 @@ internal async Task TestAnalysisWithAnalyzerInBothProjectAndHost_DifferentAnalyz
var service = project.Solution.Services.GetRequiredService<IDiagnosticAnalyzerService>();
var diagnosticsMapResults = await service.GetDiagnosticsForIdsAsync(
project, documentIds: default, [analyzerProject.Descriptor.Id, analyzerHost.Descriptor.Id],
shouldIncludeAnalyzer: null, includeLocalDocumentDiagnostics: true, CancellationToken.None);
AnalyzerFilter.All, includeLocalDocumentDiagnostics: true, CancellationToken.None);

// We should only get one diagnostic as the two analyzers have the same ID and will be deduped.
Assert.Equal(1, diagnosticsMapResults.Length);
Expand Down Expand Up @@ -774,7 +774,7 @@ internal async Task TestAnalysisWithAnalyzerInBothProjectAndHost_SameAnalyzerIns
var service = project.Solution.Services.GetRequiredService<IDiagnosticAnalyzerService>();
var diagnosticsMapResults = await service.GetDiagnosticsForIdsAsync(
project, documentIds: default, [analyzer.Descriptor.Id],
shouldIncludeAnalyzer: null, includeLocalDocumentDiagnostics: true, CancellationToken.None);
AnalyzerFilter.All, includeLocalDocumentDiagnostics: true, CancellationToken.None);

Assert.Single(diagnosticsMapResults);

Expand Down Expand Up @@ -890,7 +890,7 @@ void M()
try
{
_ = await service.GetDiagnosticsForIdsAsync(
project, [document.Id], diagnosticIds: null, shouldIncludeAnalyzer: null, includeLocalDocumentDiagnostics: true,
project, [document.Id], diagnosticIds: null, AnalyzerFilter.All, includeLocalDocumentDiagnostics: true,
analyzer.CancellationToken);

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

// Then invoke analysis without cancellation token, and verify non-cancelled diagnostic.
var diagnosticsMap = await service.GetDiagnosticsForIdsAsync(
project, [document.Id], diagnosticIds: null, shouldIncludeAnalyzer: null, includeLocalDocumentDiagnostics: true, CancellationToken.None);
project, [document.Id], diagnosticIds: null, AnalyzerFilter.All, includeLocalDocumentDiagnostics: true, CancellationToken.None);
Assert.Equal(CancellationTestAnalyzer.DiagnosticId, diagnosticsMap.Single().Id);
}

Expand Down
2 changes: 1 addition & 1 deletion src/EditorFeatures/Test2/Diagnostics/DiagnosticProviderTests.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ Namespace Microsoft.CodeAnalysis.Editor.Implementation.Diagnostics.UnitTests

For Each project In workspace.CurrentSolution.Projects
actualDiagnostics.AddRange(diagnosticProvider.GetDiagnosticsForIdsAsync(
project, documentIds:=Nothing, diagnosticIds:=Nothing, shouldIncludeAnalyzer:=Nothing,
project, documentIds:=Nothing, diagnosticIds:=Nothing, AnalyzerFilter.All,
includeLocalDocumentDiagnostics:=True, CancellationToken.None).Result)
Next

Expand Down
14 changes: 7 additions & 7 deletions src/EditorFeatures/Test2/Diagnostics/DiagnosticServiceTests.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,7 @@ Namespace Microsoft.CodeAnalysis.Editor.Implementation.Diagnostics.UnitTests
Assert.Empty(diagnostics)

diagnostics = Await diagnosticService.GetDiagnosticsForIdsAsync(
project, documentIds:=Nothing, diagnosticIds:=Nothing, shouldIncludeAnalyzer:=Nothing,
project, documentIds:=Nothing, diagnosticIds:=Nothing, AnalyzerFilter.All,
includeLocalDocumentDiagnostics:=True, CancellationToken.None)
Dim diagnostic = diagnostics.First()
Assert.True(diagnostic.Id = "AD0001")
Expand Down Expand Up @@ -606,7 +606,7 @@ Namespace Microsoft.CodeAnalysis.Editor.Implementation.Diagnostics.UnitTests

Dim document = project.Documents.Single()
Dim diagnostics = Await diagnosticService.GetDiagnosticsForIdsAsync(
project, documentIds:=Nothing, diagnosticIds:=Nothing, shouldIncludeAnalyzer:=Nothing,
project, documentIds:=Nothing, diagnosticIds:=Nothing, AnalyzerFilter.All,
includeLocalDocumentDiagnostics:=True, CancellationToken.None)
Assert.Equal(1, diagnostics.Length)
Dim diagnostic = diagnostics.First()
Expand Down Expand Up @@ -805,7 +805,7 @@ class AnonymousFunctions
' Verify compilation diagnostics are reported with correct location info when asked for project diagnostics.
Dim projectDiagnostics = Await diagnosticService.GetDiagnosticsForIdsAsync(
project, documentIds:=Nothing,
diagnosticIds:=Nothing, shouldIncludeAnalyzer:=Nothing,
diagnosticIds:=Nothing, AnalyzerFilter.All,
includeLocalDocumentDiagnostics:=True, CancellationToken.None)
Assert.Equal(2, projectDiagnostics.Length)

Expand Down Expand Up @@ -955,7 +955,7 @@ class AnonymousFunctions
' Verify no duplicate analysis/diagnostics.
Dim document = project.Documents.Single()
Dim diagnostics = (Await diagnosticService.GetDiagnosticsForIdsAsync(
project, documentIds:=Nothing, diagnosticIds:=Nothing, shouldIncludeAnalyzer:=Nothing,
project, documentIds:=Nothing, diagnosticIds:=Nothing, AnalyzerFilter.All,
includeLocalDocumentDiagnostics:=True, CancellationToken.None)).
Select(Function(d) d.Id = NamedTypeAnalyzer.DiagDescriptor.Id)

Expand Down Expand Up @@ -1050,7 +1050,7 @@ class AnonymousFunctions

' Verify project diagnostics contains diagnostics reported on both partial definitions.
Dim diagnostics = Await diagnosticService.GetDiagnosticsForIdsAsync(
project, documentIds:=Nothing, diagnosticIds:=Nothing, shouldIncludeAnalyzer:=Nothing,
project, documentIds:=Nothing, diagnosticIds:=Nothing, AnalyzerFilter.All,
includeLocalDocumentDiagnostics:=True, CancellationToken.None)
Assert.Equal(2, diagnostics.Length)
Dim file1HasDiag = False, file2HasDiag = False
Expand Down Expand Up @@ -2152,7 +2152,7 @@ class MyClass

' Get diagnostics explicitly
Dim hiddenDiagnostics = Await diagnosticService.GetDiagnosticsForIdsAsync(
project, documentIds:=Nothing, diagnosticIds:=Nothing, shouldIncludeAnalyzer:=Nothing,
project, documentIds:=Nothing, diagnosticIds:=Nothing, AnalyzerFilter.All,
includeLocalDocumentDiagnostics:=True, CancellationToken.None)
Assert.Equal(1, hiddenDiagnostics.Length)
Assert.Equal(analyzer.Descriptor.Id, hiddenDiagnostics.Single().Id)
Expand Down Expand Up @@ -2239,7 +2239,7 @@ class C
Assert.Equal(1, descriptorsMap.Count)

Dim diagnostics = Await diagnosticService.GetDiagnosticsForIdsAsync(
project, documentIds:=Nothing, diagnosticIds:=Nothing, shouldIncludeAnalyzer:=Nothing,
project, documentIds:=Nothing, diagnosticIds:=Nothing, AnalyzerFilter.All,
includeLocalDocumentDiagnostics:=True, CancellationToken.None)
Assert.Empty(diagnostics)
End Using
Expand Down
6 changes: 3 additions & 3 deletions src/Features/Core/Portable/CodeFixes/Service/CodeFixService.FixAllDiagnosticProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public override async Task<IEnumerable<Diagnostic>> GetDocumentDiagnosticsAsync(
{
var service = document.Project.Solution.Services.GetRequiredService<IDiagnosticAnalyzerService>();
var diagnostics = Filter(await service.GetDiagnosticsForIdsAsync(
document.Project, [document.Id], _diagnosticIds, shouldIncludeAnalyzer: null, includeLocalDocumentDiagnostics: true, cancellationToken).ConfigureAwait(false));
document.Project, [document.Id], _diagnosticIds, AnalyzerFilter.All, includeLocalDocumentDiagnostics: true, cancellationToken).ConfigureAwait(false));
Contract.ThrowIfFalse(diagnostics.All(d => d.DocumentId != null));
return await diagnostics.ToDiagnosticsAsync(document.Project, cancellationToken).ConfigureAwait(false);
}
Expand All @@ -67,7 +67,7 @@ public override async Task<IEnumerable<Diagnostic>> GetAllDiagnosticsAsync(Proje
// Get all diagnostics for the entire project, including document diagnostics.
var service = project.Solution.Services.GetRequiredService<IDiagnosticAnalyzerService>();
var diagnostics = Filter(await service.GetDiagnosticsForIdsAsync(
project, documentIds: default, _diagnosticIds, shouldIncludeAnalyzer: null, includeLocalDocumentDiagnostics: true, cancellationToken).ConfigureAwait(false));
project, documentIds: default, _diagnosticIds, AnalyzerFilter.All, includeLocalDocumentDiagnostics: true, cancellationToken).ConfigureAwait(false));
return await diagnostics.ToDiagnosticsAsync(project, cancellationToken).ConfigureAwait(false);
}

Expand All @@ -76,7 +76,7 @@ public override async Task<IEnumerable<Diagnostic>> GetProjectDiagnosticsAsync(P
// Get all no-location diagnostics for the project, doesn't include document diagnostics.
var service = project.Solution.Services.GetRequiredService<IDiagnosticAnalyzerService>();
var diagnostics = Filter(await service.GetProjectDiagnosticsForIdsAsync(
project, _diagnosticIds, shouldIncludeAnalyzer: null, cancellationToken).ConfigureAwait(false));
project, _diagnosticIds, AnalyzerFilter.All, cancellationToken).ConfigureAwait(false));
Contract.ThrowIfFalse(diagnostics.All(d => d.DocumentId == null));
return await diagnostics.ToDiagnosticsAsync(project, cancellationToken).ConfigureAwait(false);
}
Expand Down
31 changes: 4 additions & 27 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 @@ -33,23 +33,6 @@ internal interface IDiagnosticAnalyzerService : IWorkspaceService
Task<ImmutableArray<DiagnosticData>> ForceRunCodeAnalysisDiagnosticsAsync(
Project project, CancellationToken cancellationToken);

/// <summary>
/// The default analyzer filter that will be used in functions like <see cref="GetDiagnosticsForIdsAsync"/> if
/// no filter is provided. The default filter has the following rules:
/// <list type="number">
/// <item>The standard compiler analyzer will not be run if the compiler diagnostic scope is <see cref="CompilerDiagnosticsScope.None"/>.</item>
/// <item>A regular analyzer will not be run if <see cref="ProjectState.RunAnalyzers"/> is false.</item>
/// <item>A regular analyzer will not be run if if the background analysis scope is <see cref="BackgroundAnalysisScope.None"/>.</item>
/// <item>If a set of diagnostic ids are provided, the analyzer will not be run unless it declares at least one
/// descriptor in that set.</item>
/// <item>Otherwise, the analyzer will be run</item>
/// </list>
/// </summary>
/// <param name="additionalFilter">An additional filter that can accept or reject analyzers that the default
/// rules have accepted.</param>
Func<DiagnosticAnalyzer, bool> GetDefaultAnalyzerFilter(
Project project, ImmutableHashSet<string>? diagnosticIds, Func<DiagnosticAnalyzer, bool>? additionalFilter = null);
Copy link
Member Author

Choose a reason for hiding this comment

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

filter callbacks removed from api, as ti requires processing by teh caller (which is on the VS side, not the OOP side).


/// <inheritdoc cref="IRemoteDiagnosticAnalyzerService.GetDeprioritizationCandidatesAsync"/>
Task<ImmutableArray<DiagnosticAnalyzer>> GetDeprioritizationCandidatesAsync(
Project project, ImmutableArray<DiagnosticAnalyzer> analyzers, CancellationToken cancellationToken);
Expand All @@ -65,10 +48,7 @@ Task<ImmutableArray<DiagnosticAnalyzer>> GetDeprioritizationCandidatesAsync(
/// <param name="documentIds">Optional documents to scope the returned diagnostics. If <see langword="default"/>,
/// then diagnostics will be returned for <see cref="Project.DocumentIds"/> and <see cref="Project.AdditionalDocumentIds"/>.</param>
/// <param name="diagnosticIds">Optional set of diagnostic IDs to scope the returned diagnostics.</param>
/// <param name="shouldIncludeAnalyzer">Optional callback to filter out analyzers to execute for computing diagnostics.
/// If not present, <see cref="GetDefaultAnalyzerFilter"/> will be used. If present, no default behavior
/// is used, and the callback is defered to entirely. To augment the existing default rules call
/// <see cref="GetDefaultAnalyzerFilter"/> explicitly, and pass the result of that into this method.</param>
/// <param name="analyzerFilter">Which analyzers to run.</param>
/// <param name="includeLocalDocumentDiagnostics">
/// Indicates if local document diagnostics must be returned.
/// Local diagnostics are the ones that are reported by analyzers on the same file for which the callback was received
Expand All @@ -80,7 +60,7 @@ Task<ImmutableArray<DiagnosticAnalyzer>> GetDeprioritizationCandidatesAsync(
/// project must be analyzed to get the complete set of non-local document diagnostics.
/// </remarks>
Task<ImmutableArray<DiagnosticData>> GetDiagnosticsForIdsAsync(
Project project, ImmutableArray<DocumentId> documentIds, ImmutableHashSet<string>? diagnosticIds, Func<DiagnosticAnalyzer, bool>? shouldIncludeAnalyzer, bool includeLocalDocumentDiagnostics, CancellationToken cancellationToken);
Project project, ImmutableArray<DocumentId> documentIds, ImmutableHashSet<string>? diagnosticIds, AnalyzerFilter analyzerFilter, bool includeLocalDocumentDiagnostics, CancellationToken cancellationToken);

/// <summary>
/// Get project diagnostics (diagnostics with no source location) of the given diagnostic ids and/or analyzers from
Expand All @@ -90,12 +70,9 @@ Task<ImmutableArray<DiagnosticData>> GetDiagnosticsForIdsAsync(
/// </summary>
/// <param name="project">Project to fetch the diagnostics for.</param>
/// <param name="diagnosticIds">Optional set of diagnostic IDs to scope the returned diagnostics.</param>
/// <param name="shouldIncludeAnalyzer">Optional callback to filter out analyzers to execute for computing diagnostics.
/// If not present, <see cref="GetDefaultAnalyzerFilter"/> will be used. If present, no default behavior
/// is used, and the callback is defered to entirely. To augment the existing default rules call
/// <see cref="GetDefaultAnalyzerFilter"/> explicitly, and pass the result of that into this method.</param>
/// <param name="analyzerFilter">Which analyzers to run.</param>
Task<ImmutableArray<DiagnosticData>> GetProjectDiagnosticsForIdsAsync(
Project project, ImmutableHashSet<string>? diagnosticIds, Func<DiagnosticAnalyzer, bool>? shouldIncludeAnalyzer, CancellationToken cancellationToken);
Project project, ImmutableHashSet<string>? diagnosticIds, AnalyzerFilter analyzerFilter, CancellationToken cancellationToken);

/// <summary>
/// Return up to date diagnostics for the given span for the document
Expand Down
Loading
Loading