Skip to content

Commit

Permalink
Add support for reporting non-local LSP pull document diagnostics for…
Browse files Browse the repository at this point in the history 
… open documents.

Fixes dotnet#5634

Implemented as per dotnet/vscode-csharp#5634 (comment)

Currently, we only report local document diagnostics for open documents in the DocumentPullDiagnosticHandler. We report local and non-local diagnostics for closed documents, as well as no-location projects in the WorkspacePullDiagnosticHandler.

This change adds a new dynamically registered document diagnostic source in the public DocumentPullDiagnosticHandler with a new identifier to report non-local document diagnostics for open documents. I verified that after this change compilation end diagnostics are reported for open documents when FSA is enabled.
  • Loading branch information
@mavasani
mavasani committed Aug 16, 2023
1 parent 1fd4ff9 commit c15e853
Show file tree
Hide file tree
Showing 21 changed files with 281 additions and 53 deletions.
2 changes: 1 addition & 1 deletion src/EditorFeatures/Test2/Diagnostics/DiagnosticServiceTests.vb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -855,7 +855,7 @@ class AnonymousFunctions
' 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,
includeNonLocalDocumentDiagnostics:=True, CancellationToken.None)
includeLocalDocumentDiagnostics:=True, includeNonLocalDocumentDiagnostics:=True, CancellationToken.None)
Assert.Equal(2, projectDiagnostics.Count())

Dim noLocationDiagnostic = projectDiagnostics.First(Function(d) d.DataLocation.DocumentId Is Nothing)
Expand Down
2 changes: 1 addition & 1 deletion src/EditorFeatures/TestUtilities/Diagnostics/MockDiagnosticAnalyzerService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public Task<ImmutableArray<DiagnosticData>> GetCachedDiagnosticsAsync(Workspace
public Task<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(Solution solution, ProjectId? projectId, DocumentId? documentId, bool includeSuppressedDiagnostics, bool includeNonLocalDocumentDiagnostics, CancellationToken cancellationToken)
=> throw new NotImplementedException();

public Task<ImmutableArray<DiagnosticData>> GetDiagnosticsForIdsAsync(Solution solution, ProjectId? projectId, DocumentId? documentId, ImmutableHashSet<string>? diagnosticIds, Func<DiagnosticAnalyzer, bool>? shouldIncludeAnalyzer, bool includeSuppressedDiagnostics, bool includeNonLocalDocumentDiagnostics, CancellationToken cancellationToken)
public Task<ImmutableArray<DiagnosticData>> GetDiagnosticsForIdsAsync(Solution solution, ProjectId? projectId, DocumentId? documentId, ImmutableHashSet<string>? diagnosticIds, Func<DiagnosticAnalyzer, bool>? shouldIncludeAnalyzer, bool includeSuppressedDiagnostics, bool includeLocalDocumentDiagnostics, bool includeNonLocalDocumentDiagnostics, CancellationToken cancellationToken)
=> throw new NotImplementedException();

public Task<ImmutableArray<DiagnosticData>> GetDiagnosticsForSpanAsync(TextDocument document, TextSpan? range, Func<string, bool>? shouldIncludeDiagnostic, bool includeCompilerDiagnostics, bool includeSuppressedDiagnostics, ICodeActionRequestPriorityProvider priorityProvider, Func<string, IDisposable?>? addOperationScope, DiagnosticKind diagnosticKind, bool isExplicit, CancellationToken cancellationToken)
Expand Down
4 changes: 2 additions & 2 deletions src/Features/Core/Portable/Diagnostics/IDiagnosticAnalyzerService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,11 @@ internal interface IDiagnosticAnalyzerService
/// Note that for project case, this method returns diagnostics from all project documents as well. Use <see cref="GetProjectDiagnosticsForIdsAsync(Solution, ProjectId?, ImmutableHashSet{string}?, Func{DiagnosticAnalyzer, bool}?, bool, bool, CancellationToken)"/>
/// if you want to fetch only project diagnostics without source locations.
/// </summary>
Task<ImmutableArray<DiagnosticData>> GetDiagnosticsForIdsAsync(Solution solution, ProjectId? projectId, DocumentId? documentId, ImmutableHashSet<string>? diagnosticIds, Func<DiagnosticAnalyzer, bool>? shouldIncludeAnalyzer, bool includeSuppressedDiagnostics, bool includeNonLocalDocumentDiagnostics, CancellationToken cancellationToken);
Task<ImmutableArray<DiagnosticData>> GetDiagnosticsForIdsAsync(Solution solution, ProjectId? projectId, DocumentId? documentId, ImmutableHashSet<string>? diagnosticIds, Func<DiagnosticAnalyzer, bool>? shouldIncludeAnalyzer, bool includeSuppressedDiagnostics, bool includeLocalDocumentDiagnostics, bool includeNonLocalDocumentDiagnostics, CancellationToken cancellationToken);

/// <summary>
/// Get project diagnostics (diagnostics with no source location) of the given diagnostic ids and/or analyzers from the given solution. all diagnostics returned should be up-to-date with respect to the given solution.
/// Note that this method doesn't return any document diagnostics. Use <see cref="GetDiagnosticsForIdsAsync(Solution, ProjectId, DocumentId, ImmutableHashSet{string}, Func{DiagnosticAnalyzer, bool}?, bool, bool, CancellationToken)"/> to also fetch those.
/// Note that this method doesn't return any document diagnostics. Use <see cref="GetDiagnosticsForIdsAsync(Solution, ProjectId, DocumentId, ImmutableHashSet{string}, Func{DiagnosticAnalyzer, bool}?, bool, bool, bool, CancellationToken)"/> to also fetch those.
/// </summary>
Task<ImmutableArray<DiagnosticData>> GetProjectDiagnosticsForIdsAsync(Solution solution, ProjectId? projectId, ImmutableHashSet<string>? diagnosticIds, Func<DiagnosticAnalyzer, bool>? shouldIncludeAnalyzer, bool includeSuppressedDiagnostics, bool includeNonLocalDocumentDiagnostics, CancellationToken cancellationToken);

Expand Down
4 changes: 2 additions & 2 deletions ...res/LanguageServer/Protocol/Features/CodeFixes/CodeFixService.FixAllDiagnosticProvider.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public FixAllDiagnosticProvider(IDiagnosticAnalyzerService diagnosticService, Im
public override async Task<IEnumerable<Diagnostic>> GetDocumentDiagnosticsAsync(Document document, CancellationToken cancellationToken)
{
var solution = document.Project.Solution;
var diagnostics = await _diagnosticService.GetDiagnosticsForIdsAsync(solution, projectId: null, document.Id, _diagnosticIds, shouldIncludeAnalyzer: null, _includeSuppressedDiagnostics, includeNonLocalDocumentDiagnostics: false, cancellationToken).ConfigureAwait(false);
var diagnostics = await _diagnosticService.GetDiagnosticsForIdsAsync(solution, projectId: null, document.Id, _diagnosticIds, shouldIncludeAnalyzer: null, _includeSuppressedDiagnostics, includeLocalDocumentDiagnostics: true, includeNonLocalDocumentDiagnostics: false, cancellationToken).ConfigureAwait(false);
Contract.ThrowIfFalse(diagnostics.All(d => d.DocumentId != null));
return await diagnostics.ToDiagnosticsAsync(document.Project, cancellationToken).ConfigureAwait(false);
}
Expand All @@ -64,7 +64,7 @@ public override async Task<IEnumerable<Diagnostic>> GetDocumentSpanDiagnosticsAs
public override async Task<IEnumerable<Diagnostic>> GetAllDiagnosticsAsync(Project project, CancellationToken cancellationToken)
{
// Get all diagnostics for the entire project, including document diagnostics.
var diagnostics = await _diagnosticService.GetDiagnosticsForIdsAsync(project.Solution, project.Id, documentId: null, _diagnosticIds, shouldIncludeAnalyzer: null, _includeSuppressedDiagnostics, includeNonLocalDocumentDiagnostics: false, cancellationToken).ConfigureAwait(false);
var diagnostics = await _diagnosticService.GetDiagnosticsForIdsAsync(project.Solution, project.Id, documentId: null, _diagnosticIds, shouldIncludeAnalyzer: null, _includeSuppressedDiagnostics, includeLocalDocumentDiagnostics: true, includeNonLocalDocumentDiagnostics: false, cancellationToken).ConfigureAwait(false);
return await diagnostics.ToDiagnosticsAsync(project, cancellationToken).ConfigureAwait(false);
}

Expand Down
12 changes: 6 additions & 6 deletions src/Features/LanguageServer/Protocol/Features/Diagnostics/DiagnosticAnalyzerService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,10 +188,10 @@ public async Task ForceAnalyzeAsync(Solution solution, Action<Project> onProject
foreach (var project in solution.Projects)
{
tasks[index++] = Task.Run(async () =>
{
await analyzer.ForceAnalyzeProjectAsync(project, cancellationToken).ConfigureAwait(false);
onProjectAnalyzed(project);
}, cancellationToken);
{
await analyzer.ForceAnalyzeProjectAsync(project, cancellationToken).ConfigureAwait(false);
onProjectAnalyzed(project);
}, cancellationToken);
}

await Task.WhenAll(tasks).ConfigureAwait(false);
Expand All @@ -200,11 +200,11 @@ public async Task ForceAnalyzeAsync(Solution solution, Action<Project> onProject
}

public Task<ImmutableArray<DiagnosticData>> GetDiagnosticsForIdsAsync(
Solution solution, ProjectId? projectId, DocumentId? documentId, ImmutableHashSet<string>? diagnosticIds, Func<DiagnosticAnalyzer, bool>? shouldIncludeAnalyzer, bool includeSuppressedDiagnostics, bool includeNonLocalDocumentDiagnostics, CancellationToken cancellationToken)
Solution solution, ProjectId? projectId, DocumentId? documentId, ImmutableHashSet<string>? diagnosticIds, Func<DiagnosticAnalyzer, bool>? shouldIncludeAnalyzer, bool includeSuppressedDiagnostics, bool includeLocalDocumentDiagnostics, bool includeNonLocalDocumentDiagnostics, CancellationToken cancellationToken)
{
if (_map.TryGetValue(solution.Workspace, out var analyzer))
{
return analyzer.GetDiagnosticsForIdsAsync(solution, projectId, documentId, diagnosticIds, shouldIncludeAnalyzer, includeSuppressedDiagnostics, includeNonLocalDocumentDiagnostics, cancellationToken);
return analyzer.GetDiagnosticsForIdsAsync(solution, projectId, documentId, diagnosticIds, shouldIncludeAnalyzer, includeSuppressedDiagnostics, includeLocalDocumentDiagnostics, includeNonLocalDocumentDiagnostics, cancellationToken);
}

return SpecializedTasks.EmptyImmutableArray<DiagnosticData>();
Expand Down
39 changes: 25 additions & 14 deletions ...er/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_GetDiagnostics.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,20 @@ public Task<ImmutableArray<DiagnosticData>> GetSpecificCachedDiagnosticsAsync(So
}

var (documentId, projectId) = (argsId.ProjectOrDocumentId is DocumentId docId) ? (docId, docId.ProjectId) : (null, (ProjectId)argsId.ProjectOrDocumentId);
return new IdeCachedDiagnosticGetter(this, solution, projectId, documentId, includeSuppressedDiagnostics, includeNonLocalDocumentDiagnostics).GetSpecificDiagnosticsAsync(argsId.Analyzer, argsId.Kind, cancellationToken);
return new IdeCachedDiagnosticGetter(this, solution, projectId, documentId, includeSuppressedDiagnostics, includeLocalDocumentDiagnostics: true, includeNonLocalDocumentDiagnostics).GetSpecificDiagnosticsAsync(argsId.Analyzer, argsId.Kind, cancellationToken);
}

public Task<ImmutableArray<DiagnosticData>> GetCachedDiagnosticsAsync(Solution solution, ProjectId? projectId, DocumentId? documentId, bool includeSuppressedDiagnostics, bool includeNonLocalDocumentDiagnostics, CancellationToken cancellationToken)
=> new IdeCachedDiagnosticGetter(this, solution, projectId, documentId, includeSuppressedDiagnostics, includeNonLocalDocumentDiagnostics).GetDiagnosticsAsync(cancellationToken);
=> new IdeCachedDiagnosticGetter(this, solution, projectId, documentId, includeSuppressedDiagnostics, includeLocalDocumentDiagnostics: true, includeNonLocalDocumentDiagnostics).GetDiagnosticsAsync(cancellationToken);

public Task<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(Solution solution, ProjectId? projectId, DocumentId? documentId, bool includeSuppressedDiagnostics, bool includeNonLocalDocumentDiagnostics, CancellationToken cancellationToken)
=> new IdeLatestDiagnosticGetter(this, solution, projectId, documentId, diagnosticIds: null, shouldIncludeAnalyzer: null, includeSuppressedDiagnostics, includeNonLocalDocumentDiagnostics).GetDiagnosticsAsync(cancellationToken);
=> new IdeLatestDiagnosticGetter(this, solution, projectId, documentId, diagnosticIds: null, shouldIncludeAnalyzer: null, includeSuppressedDiagnostics, includeLocalDocumentDiagnostics: true, includeNonLocalDocumentDiagnostics).GetDiagnosticsAsync(cancellationToken);

public Task<ImmutableArray<DiagnosticData>> GetDiagnosticsForIdsAsync(Solution solution, ProjectId? projectId, DocumentId? documentId, ImmutableHashSet<string>? diagnosticIds, Func<DiagnosticAnalyzer, bool>? shouldIncludeAnalyzer, bool includeSuppressedDiagnostics, bool includeNonLocalDocumentDiagnostics, CancellationToken cancellationToken)
=> new IdeLatestDiagnosticGetter(this, solution, projectId, documentId, diagnosticIds, shouldIncludeAnalyzer, includeSuppressedDiagnostics, includeNonLocalDocumentDiagnostics).GetDiagnosticsAsync(cancellationToken);
public Task<ImmutableArray<DiagnosticData>> GetDiagnosticsForIdsAsync(Solution solution, ProjectId? projectId, DocumentId? documentId, ImmutableHashSet<string>? diagnosticIds, Func<DiagnosticAnalyzer, bool>? shouldIncludeAnalyzer, bool includeSuppressedDiagnostics, bool includeLocalDocumentDiagnostics, bool includeNonLocalDocumentDiagnostics, CancellationToken cancellationToken)
=> new IdeLatestDiagnosticGetter(this, solution, projectId, documentId, diagnosticIds, shouldIncludeAnalyzer, includeSuppressedDiagnostics, includeLocalDocumentDiagnostics, includeNonLocalDocumentDiagnostics).GetDiagnosticsAsync(cancellationToken);

public Task<ImmutableArray<DiagnosticData>> GetProjectDiagnosticsForIdsAsync(Solution solution, ProjectId? projectId, ImmutableHashSet<string>? diagnosticIds, Func<DiagnosticAnalyzer, bool>? shouldIncludeAnalyzer, bool includeSuppressedDiagnostics, bool includeNonLocalDocumentDiagnostics, CancellationToken cancellationToken)
=> new IdeLatestDiagnosticGetter(this, solution, projectId, documentId: null, diagnosticIds, shouldIncludeAnalyzer, includeSuppressedDiagnostics, includeNonLocalDocumentDiagnostics).GetProjectDiagnosticsAsync(cancellationToken);
=> new IdeLatestDiagnosticGetter(this, solution, projectId, documentId: null, diagnosticIds, shouldIncludeAnalyzer, includeSuppressedDiagnostics, includeLocalDocumentDiagnostics: false, includeNonLocalDocumentDiagnostics).GetProjectDiagnosticsAsync(cancellationToken);

private abstract class DiagnosticGetter
{
Expand All @@ -46,6 +46,7 @@ private abstract class DiagnosticGetter
protected readonly ProjectId? ProjectId;
protected readonly DocumentId? DocumentId;
protected readonly bool IncludeSuppressedDiagnostics;
protected readonly bool IncludeLocalDocumentDiagnostics;
protected readonly bool IncludeNonLocalDocumentDiagnostics;

private ImmutableArray<DiagnosticData>.Builder? _lazyDataBuilder;
Expand All @@ -56,6 +57,7 @@ public DiagnosticGetter(
ProjectId? projectId,
DocumentId? documentId,
bool includeSuppressedDiagnostics,
bool includeLocalDocumentDiagnostics,
bool includeNonLocalDocumentDiagnostics)
{
Owner = owner;
Expand All @@ -65,6 +67,7 @@ public DiagnosticGetter(
ProjectId = projectId ?? documentId?.ProjectId;

IncludeSuppressedDiagnostics = includeSuppressedDiagnostics;
IncludeLocalDocumentDiagnostics = includeLocalDocumentDiagnostics;
IncludeNonLocalDocumentDiagnostics = includeNonLocalDocumentDiagnostics;
}

Expand Down Expand Up @@ -139,8 +142,8 @@ private bool ShouldIncludeSuppressedDiagnostic(DiagnosticData diagnostic)

private sealed class IdeCachedDiagnosticGetter : DiagnosticGetter
{
public IdeCachedDiagnosticGetter(DiagnosticIncrementalAnalyzer owner, Solution solution, ProjectId? projectId, DocumentId? documentId, bool includeSuppressedDiagnostics, bool includeNonLocalDocumentDiagnostics)
: base(owner, solution, projectId, documentId, includeSuppressedDiagnostics, includeNonLocalDocumentDiagnostics)
public IdeCachedDiagnosticGetter(DiagnosticIncrementalAnalyzer owner, Solution solution, ProjectId? projectId, DocumentId? documentId, bool includeSuppressedDiagnostics, bool includeLocalDocumentDiagnostics, bool includeNonLocalDocumentDiagnostics)
: base(owner, solution, projectId, documentId, includeSuppressedDiagnostics, includeLocalDocumentDiagnostics, includeNonLocalDocumentDiagnostics)
{
}

Expand All @@ -150,8 +153,12 @@ protected override async Task AppendDiagnosticsAsync(Project project, IEnumerabl
{
foreach (var documentId in documentIds)
{
AppendDiagnostics(await GetDiagnosticsAsync(stateSet, project, documentId, AnalysisKind.Syntax, cancellationToken).ConfigureAwait(false));
AppendDiagnostics(await GetDiagnosticsAsync(stateSet, project, documentId, AnalysisKind.Semantic, cancellationToken).ConfigureAwait(false));
if (IncludeLocalDocumentDiagnostics)
{
AppendDiagnostics(await GetDiagnosticsAsync(stateSet, project, documentId, AnalysisKind.Syntax, cancellationToken).ConfigureAwait(false));
AppendDiagnostics(await GetDiagnosticsAsync(stateSet, project, documentId, AnalysisKind.Semantic, cancellationToken).ConfigureAwait(false));
}

if (IncludeNonLocalDocumentDiagnostics)
AppendDiagnostics(await GetDiagnosticsAsync(stateSet, project, documentId, AnalysisKind.NonLocal, cancellationToken).ConfigureAwait(false));
}
Expand Down Expand Up @@ -236,8 +243,8 @@ private sealed class IdeLatestDiagnosticGetter : DiagnosticGetter
public IdeLatestDiagnosticGetter(
DiagnosticIncrementalAnalyzer owner, Solution solution, ProjectId? projectId,
DocumentId? documentId, ImmutableHashSet<string>? diagnosticIds, Func<DiagnosticAnalyzer, bool>? shouldIncludeAnalyzer,
bool includeSuppressedDiagnostics, bool includeNonLocalDocumentDiagnostics)
: base(owner, solution, projectId, documentId, includeSuppressedDiagnostics, includeNonLocalDocumentDiagnostics)
bool includeSuppressedDiagnostics, bool includeLocalDocumentDiagnostics, bool includeNonLocalDocumentDiagnostics)
: base(owner, solution, projectId, documentId, includeSuppressedDiagnostics, includeLocalDocumentDiagnostics, includeNonLocalDocumentDiagnostics)
{
_diagnosticIds = diagnosticIds;
_shouldIncludeAnalyzer = shouldIncludeAnalyzer;
Expand Down Expand Up @@ -281,8 +288,12 @@ protected override async Task AppendDiagnosticsAsync(Project project, IEnumerabl

foreach (var documentId in documentIds)
{
AppendDiagnostics(analysisResult.GetDocumentDiagnostics(documentId, AnalysisKind.Syntax));
AppendDiagnostics(analysisResult.GetDocumentDiagnostics(documentId, AnalysisKind.Semantic));
if (IncludeLocalDocumentDiagnostics)
{
AppendDiagnostics(analysisResult.GetDocumentDiagnostics(documentId, AnalysisKind.Syntax));
AppendDiagnostics(analysisResult.GetDocumentDiagnostics(documentId, AnalysisKind.Semantic));
}

if (IncludeNonLocalDocumentDiagnostics)
AppendDiagnostics(analysisResult.GetDocumentDiagnostics(documentId, AnalysisKind.NonLocal));
}
Expand Down
Loading

0 comments on commit c15e853

Please sign in to comment.