Skip to content

Push diagnostic suppression filtering flag to feature levels #76849

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 12 commits into from
Jan 22, 2025
Merged

Push diagnostic suppression filtering flag to feature levels #76849

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
bf43020
Push diagnostic suppression filtering flag to feature levels
CyrusNajmabadi Jan 22, 2025
728a8cc
Remove
CyrusNajmabadi Jan 22, 2025
5a1c0b2
Remove
CyrusNajmabadi Jan 22, 2025
e5b1721
Fix
CyrusNajmabadi Jan 22, 2025
25e37fb
Fix
CyrusNajmabadi Jan 22, 2025
9a4da49
Merge branch 'deleteExtension' into suppressedDiagnosticsFlag
CyrusNajmabadi Jan 22, 2025
e296e09
Merge remote-tracking branch 'upstream/main' into suppressedDiagnosti…
CyrusNajmabadi Jan 22, 2025
5984584
Remove unused usings
CyrusNajmabadi Jan 22, 2025
30a0670
Simplify
CyrusNajmabadi Jan 22, 2025
e9fa4a6
Update doc
CyrusNajmabadi Jan 22, 2025
68810d9
Merge remote-tracking branch 'upstream/main' into suppressedDiagnosti…
CyrusNajmabadi Jan 22, 2025
3482093
move into loop
CyrusNajmabadi Jan 22, 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
1 change: 0 additions & 1 deletion ...neDiagnostics/AbstractDiagnosticsTaggerProvider.SingleDiagnosticKindPullTaggerProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,6 @@ private async Task ProduceTagsAsync(
document,
requestedSpan.Span.ToTextSpan(),
diagnosticKind: _diagnosticKind,
includeSuppressedDiagnostics: true,
Copy link
Member Author

@CyrusNajmabadi CyrusNajmabadi Jan 22, 2025

Choose a reason for hiding this comment

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

Note: if a featuree passes includeSuppressedDiagnostics: true, it needs no change. We always return all diagnostics by default, including suppressed ones.

cancellationToken: cancellationToken).ConfigureAwait(false);

// Copilot code analysis is a special analyzer that reports semantic correctness
Expand Down
6 changes: 4 additions & 2 deletions src/EditorFeatures/Test/CodeFixes/CodeFixServiceTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1078,7 +1078,7 @@ void M()

await diagnosticIncrementalAnalyzer.GetDiagnosticsForIdsAsync(
sourceDocument.Project.Solution, sourceDocument.Project.Id, sourceDocument.Id, diagnosticIds: null, shouldIncludeAnalyzer: null, getDocuments: null,
includeSuppressedDiagnostics: true, includeLocalDocumentDiagnostics: true, includeNonLocalDocumentDiagnostics: true, CancellationToken.None);
includeLocalDocumentDiagnostics: true, includeNonLocalDocumentDiagnostics: true, CancellationToken.None);
await diagnosticIncrementalAnalyzer.GetTestAccessor().TextDocumentOpenAsync(sourceDocument);

var lowPriorityAnalyzerData = new SuggestedActionPriorityProvider.LowPriorityAnalyzersAndDiagnosticIds();
Expand Down Expand Up @@ -1142,7 +1142,9 @@ static bool GetExpectDeprioritization(
static async Task VerifyCachedDiagnosticsAsync(Document sourceDocument, bool expectedCachedDiagnostic, TextSpan testSpan, DiagnosticIncrementalAnalyzer diagnosticIncrementalAnalyzer)
{
var cachedDiagnostics = await diagnosticIncrementalAnalyzer.GetCachedDiagnosticsAsync(sourceDocument.Project.Solution, sourceDocument.Project.Id, sourceDocument.Id,
includeSuppressedDiagnostics: false, includeLocalDocumentDiagnostics: true, includeNonLocalDocumentDiagnostics: true, CancellationToken.None);
includeLocalDocumentDiagnostics: true, includeNonLocalDocumentDiagnostics: true, CancellationToken.None);
cachedDiagnostics = cachedDiagnostics.WhereAsArray(d => !d.IsSuppressed);

if (!expectedCachedDiagnostic)
{
Assert.Empty(cachedDiagnostics);
Expand Down
10 changes: 5 additions & 5 deletions src/EditorFeatures/Test/Diagnostics/DiagnosticAnalyzerServiceTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public async Task TestHasSuccessfullyLoadedBeingFalse()

var diagnostics = await analyzer.GetDiagnosticsForIdsAsync(
workspace.CurrentSolution, projectId: null, documentId: null, diagnosticIds: null, shouldIncludeAnalyzer: null, getDocuments: null,
includeSuppressedDiagnostics: false, includeLocalDocumentDiagnostics: true, includeNonLocalDocumentDiagnostics: false, CancellationToken.None);
Copy link
Member Author

@CyrusNajmabadi CyrusNajmabadi Jan 22, 2025

Choose a reason for hiding this comment

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

note: code that passes includeSuppressedDiagnostics: false needs to filter the results. I do that for features. HOwever, for tests, it doesn't seem to have any impact on results. so for simplicity i'm not doing it.

includeLocalDocumentDiagnostics: true, includeNonLocalDocumentDiagnostics: false, CancellationToken.None);
Assert.NotEmpty(diagnostics);
}

Expand Down Expand Up @@ -705,7 +705,7 @@ internal async Task TestOnlyRequiredAnalyzerExecutedDuringDiagnosticComputation(
var diagnosticsMapResults = await DiagnosticComputer.GetDiagnosticsAsync(
document, project, Checksum.Null, span: null, projectAnalyzerIds: [], analyzerIdsToRequestDiagnostics,
AnalysisKind.Semantic, new DiagnosticAnalyzerInfoCache(), workspace.Services,
isExplicit: false, reportSuppressedDiagnostics: false, logPerformanceInfo: false, getTelemetryInfo: false,
isExplicit: false, logPerformanceInfo: false, getTelemetryInfo: false,
cancellationToken: CancellationToken.None);
Assert.False(analyzer2.ReceivedSymbolCallback);

Expand Down Expand Up @@ -773,7 +773,7 @@ async Task VerifyCallbackSpanAsync(TextSpan? filterSpan)
_ = await DiagnosticComputer.GetDiagnosticsAsync(
documentToAnalyze, project, Checksum.Null, filterSpan, analyzerIdsToRequestDiagnostics, hostAnalyzerIds: [],
analysisKind, new DiagnosticAnalyzerInfoCache(), workspace.Services,
isExplicit: false, reportSuppressedDiagnostics: false, logPerformanceInfo: false, getTelemetryInfo: false,
isExplicit: false, logPerformanceInfo: false, getTelemetryInfo: false,
CancellationToken.None);
Assert.Equal(filterSpan, analyzer.CallbackFilterSpan);
if (kind == FilterSpanTestAnalyzer.AnalysisKind.AdditionalFile)
Expand Down Expand Up @@ -827,7 +827,7 @@ void M()
try
{
_ = await DiagnosticComputer.GetDiagnosticsAsync(document, project, Checksum.Null, span: null,
projectAnalyzerIds: [], analyzerIds, kind, diagnosticAnalyzerInfoCache, workspace.Services, isExplicit: false, reportSuppressedDiagnostics: false,
projectAnalyzerIds: [], analyzerIds, kind, diagnosticAnalyzerInfoCache, workspace.Services, isExplicit: false,
logPerformanceInfo: false, getTelemetryInfo: false, cancellationToken: analyzer.CancellationToken);

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

// Then invoke analysis without cancellation token, and verify non-cancelled diagnostic.
var diagnosticsMap = await DiagnosticComputer.GetDiagnosticsAsync(document, project, Checksum.Null, span: null,
projectAnalyzerIds: [], analyzerIds, kind, diagnosticAnalyzerInfoCache, workspace.Services, isExplicit: false, reportSuppressedDiagnostics: false,
projectAnalyzerIds: [], analyzerIds, kind, diagnosticAnalyzerInfoCache, workspace.Services, isExplicit: false,
logPerformanceInfo: false, getTelemetryInfo: false, cancellationToken: CancellationToken.None);
var builder = diagnosticsMap.Diagnostics.Single().diagnosticMap;
var diagnostic = kind == AnalysisKind.Syntax ? builder.Syntax.Single().Item2.Single() : builder.Semantic.Single().Item2.Single();
Expand Down
4 changes: 2 additions & 2 deletions src/EditorFeatures/Test2/CodeFixes/CodeFixServiceTests.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ Namespace Microsoft.CodeAnalysis.Editor.Implementation.CodeFixes.UnitTests
' Verify available diagnostics
Dim document = project.Documents.Single()
Dim diagnostics = Await diagnosticService.GetDiagnosticsForSpanAsync(document,
range:=(Await document.GetSyntaxRootAsync()).FullSpan, DiagnosticKind.All, includeSuppressedDiagnostics:=False, CancellationToken.None)
range:=(Await document.GetSyntaxRootAsync()).FullSpan, DiagnosticKind.All, CancellationToken.None)

Assert.Equal(1, diagnostics.Length)

Expand Down Expand Up @@ -140,7 +140,7 @@ Namespace Microsoft.CodeAnalysis.Editor.Implementation.CodeFixes.UnitTests
' Verify available diagnostics
Dim document = project.Documents.Single()
Dim diagnostics = Await diagnosticService.GetDiagnosticsForSpanAsync(document,
range:=(Await document.GetSyntaxRootAsync()).FullSpan, DiagnosticKind.All, includeSuppressedDiagnostics:=False, CancellationToken.None)
range:=(Await document.GetSyntaxRootAsync()).FullSpan, DiagnosticKind.All, CancellationToken.None)

Assert.Equal(1, diagnostics.Length)

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 @@ -263,7 +263,7 @@ Namespace Microsoft.CodeAnalysis.Editor.Implementation.Diagnostics.UnitTests
Dim diagnosticProvider = GetDiagnosticProvider(workspace)
Dim actualDiagnostics = diagnosticProvider.GetDiagnosticsForIdsAsync(
workspace.CurrentSolution, projectId:=Nothing, documentId:=Nothing, diagnosticIds:=Nothing, shouldIncludeAnalyzer:=Nothing,
includeSuppressedDiagnostics:=False, includeLocalDocumentDiagnostics:=True, includeNonLocalDocumentDiagnostics:=True, CancellationToken.None).Result
includeLocalDocumentDiagnostics:=True, includeNonLocalDocumentDiagnostics:=True, CancellationToken.None).Result

If diagnostics Is Nothing Then
Assert.Empty(actualDiagnostics)
Expand Down
36 changes: 16 additions & 20 deletions src/EditorFeatures/Test2/Diagnostics/DiagnosticServiceTests.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,12 @@ Imports System.IO
Imports System.Reflection
Imports System.Threading
Imports Microsoft.CodeAnalysis
Imports Microsoft.CodeAnalysis.CodeActions
Imports Microsoft.CodeAnalysis.CommonDiagnosticAnalyzers
Imports Microsoft.CodeAnalysis.CSharp
Imports Microsoft.CodeAnalysis.Diagnostics
Imports Microsoft.CodeAnalysis.Diagnostics.CSharp
Imports Microsoft.CodeAnalysis.Editor.UnitTests
Imports Microsoft.CodeAnalysis.Editor.UnitTests.Diagnostics
Imports Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces
Imports Microsoft.CodeAnalysis.Host.Mef
Imports Microsoft.CodeAnalysis.Options
Imports Microsoft.CodeAnalysis.Simplification
Imports Microsoft.CodeAnalysis.SolutionCrawler
Imports Microsoft.CodeAnalysis.Text
Imports Microsoft.CodeAnalysis.UnitTests.Diagnostics
Expand Down Expand Up @@ -65,7 +60,7 @@ Namespace Microsoft.CodeAnalysis.Editor.Implementation.Diagnostics.UnitTests
End Function

Private Shared Async Function GetDiagnosticsForSpanAsync(diagnosticService As IDiagnosticAnalyzerService, document As Document, range As TextSpan, diagnosticKind As DiagnosticKind) As Task(Of ImmutableArray(Of DiagnosticData))
Return Await diagnosticService.GetDiagnosticsForSpanAsync(document, range, diagnosticKind, includeSuppressedDiagnostics:=False, CancellationToken.None)
Return Await diagnosticService.GetDiagnosticsForSpanAsync(document, range, diagnosticKind, CancellationToken.None)
End Function

<WpfFact, Trait(Traits.Feature, Traits.Features.Diagnostics)>
Expand Down Expand Up @@ -538,7 +533,7 @@ Namespace Microsoft.CodeAnalysis.Editor.Implementation.Diagnostics.UnitTests

diagnostics = Await diagnosticService.GetDiagnosticsForIdsAsync(
project.Solution, projectId:=Nothing, documentId:=Nothing, diagnosticIds:=Nothing, shouldIncludeAnalyzer:=Nothing,
includeSuppressedDiagnostics:=False, includeLocalDocumentDiagnostics:=True, includeNonLocalDocumentDiagnostics:=True, CancellationToken.None)
includeLocalDocumentDiagnostics:=True, includeNonLocalDocumentDiagnostics:=True, CancellationToken.None)
Dim diagnostic = diagnostics.First()
Assert.True(diagnostic.Id = "AD0001")
Assert.Contains("CodeBlockStartedAnalyzer", diagnostic.Message, StringComparison.Ordinal)
Expand Down Expand Up @@ -609,7 +604,7 @@ Namespace Microsoft.CodeAnalysis.Editor.Implementation.Diagnostics.UnitTests
Dim document = project.Documents.Single()
Dim diagnostics = Await diagnosticService.GetDiagnosticsForIdsAsync(
project.Solution, project.Id, documentId:=Nothing, diagnosticIds:=Nothing, shouldIncludeAnalyzer:=Nothing,
includeSuppressedDiagnostics:=False, includeLocalDocumentDiagnostics:=True, includeNonLocalDocumentDiagnostics:=True, CancellationToken.None)
includeLocalDocumentDiagnostics:=True, includeNonLocalDocumentDiagnostics:=True, CancellationToken.None)
Assert.Equal(1, diagnostics.Length)
Dim diagnostic = diagnostics.First()
Assert.Equal(OperationAnalyzer.Descriptor.Id, diagnostic.Id)
Expand Down Expand Up @@ -805,9 +800,10 @@ class AnonymousFunctions

' Test "GetDiagnosticsForIdsAsync" does force computation of compilation end diagnostics.
' Verify compilation diagnostics are reported with correct location info when asked for project diagnostics.
Dim projectDiagnostics = Await diagnosticService.GetDiagnosticsForIdsAsync(project.Solution, project.Id, documentId:=Nothing,
diagnosticIds:=Nothing, shouldIncludeAnalyzer:=Nothing, includeSuppressedDiagnostics:=False,
includeLocalDocumentDiagnostics:=True, includeNonLocalDocumentDiagnostics:=True, CancellationToken.None)
Dim projectDiagnostics = Await diagnosticService.GetDiagnosticsForIdsAsync(
project.Solution, project.Id, documentId:=Nothing,
diagnosticIds:=Nothing, shouldIncludeAnalyzer:=Nothing,
includeLocalDocumentDiagnostics:=True, includeNonLocalDocumentDiagnostics:=True, CancellationToken.None)
Assert.Equal(2, projectDiagnostics.Length)

Dim noLocationDiagnostic = projectDiagnostics.First(Function(d) d.DataLocation.DocumentId Is Nothing)
Expand Down Expand Up @@ -951,7 +947,7 @@ class AnonymousFunctions
Dim document = project.Documents.Single()
Dim diagnostics = (Await diagnosticService.GetDiagnosticsForIdsAsync(
project.Solution, project.Id, documentId:=Nothing, diagnosticIds:=Nothing, shouldIncludeAnalyzer:=Nothing,
includeSuppressedDiagnostics:=False, includeLocalDocumentDiagnostics:=True, includeNonLocalDocumentDiagnostics:=True, CancellationToken.None)).
includeLocalDocumentDiagnostics:=True, includeNonLocalDocumentDiagnostics:=True, CancellationToken.None)).
Select(Function(d) d.Id = NamedTypeAnalyzer.DiagDescriptor.Id)

Assert.Equal(1, diagnostics.Count)
Expand Down Expand Up @@ -1046,7 +1042,7 @@ class AnonymousFunctions
Dim incrementalAnalyzer = diagnosticService.CreateIncrementalAnalyzer(workspace)
Dim diagnostics = Await diagnosticService.GetDiagnosticsForIdsAsync(
project.Solution, project.Id, documentId:=Nothing, diagnosticIds:=Nothing, shouldIncludeAnalyzer:=Nothing,
includeSuppressedDiagnostics:=False, includeLocalDocumentDiagnostics:=True, includeNonLocalDocumentDiagnostics:=True, CancellationToken.None)
includeLocalDocumentDiagnostics:=True, includeNonLocalDocumentDiagnostics:=True, CancellationToken.None)
Assert.Equal(2, diagnostics.Length)
Dim file1HasDiag = False, file2HasDiag = False
For Each diagnostic In diagnostics
Expand Down Expand Up @@ -2135,11 +2131,11 @@ class MyClass
Assert.Equal(analyzer.Descriptor.Id, descriptors.Single().Id)

' Get cached project diagnostics.
Dim diagnostics = Await diagnosticService.GetCachedDiagnosticsAsync(workspace, project.Id, documentId:=Nothing,
includeSuppressedDiagnostics:=False,
includeLocalDocumentDiagnostics:=True,
includeNonLocalDocumentDiagnostics:=True,
CancellationToken.None)
Dim diagnostics = Await diagnosticService.GetCachedDiagnosticsAsync(
workspace, project.Id, documentId:=Nothing,
includeLocalDocumentDiagnostics:=True,
includeNonLocalDocumentDiagnostics:=True,
CancellationToken.None)

' in v2, solution crawler never creates non-local hidden diagnostics.
' v2 still creates those for LB and explicit queries such as FixAll.
Expand All @@ -2149,7 +2145,7 @@ class MyClass
' Get diagnostics explicitly
Dim hiddenDiagnostics = Await diagnosticService.GetDiagnosticsForIdsAsync(
project.Solution, project.Id, documentId:=Nothing, diagnosticIds:=Nothing, shouldIncludeAnalyzer:=Nothing,
includeSuppressedDiagnostics:=False, includeLocalDocumentDiagnostics:=True, includeNonLocalDocumentDiagnostics:=True, CancellationToken.None)
includeLocalDocumentDiagnostics:=True, includeNonLocalDocumentDiagnostics:=True, CancellationToken.None)
Assert.Equal(1, hiddenDiagnostics.Length)
Assert.Equal(analyzer.Descriptor.Id, hiddenDiagnostics.Single().Id)
End Using
Expand Down Expand Up @@ -2236,7 +2232,7 @@ class C
Dim incrementalAnalyzer = diagnosticService.CreateIncrementalAnalyzer(workspace)
Dim diagnostics = Await diagnosticService.GetDiagnosticsForIdsAsync(
project.Solution, project.Id, documentId:=Nothing, diagnosticIds:=Nothing, shouldIncludeAnalyzer:=Nothing,
includeSuppressedDiagnostics:=False, includeLocalDocumentDiagnostics:=True, includeNonLocalDocumentDiagnostics:=True, CancellationToken.None)
includeLocalDocumentDiagnostics:=True, includeNonLocalDocumentDiagnostics:=True, CancellationToken.None)
Assert.Empty(diagnostics)
End Using
End Function
Expand Down
Loading
Loading