Skip to content

Commit 1f81109

Browse files
CyrusNajmabadiCyrusNajmabadi
authored
Cleanup how we compute and report file-content-load issues. (#77880)
2 parents f050f58 + e208042 commit 1f81109

File tree

17 files changed

+148
-269
lines changed

17 files changed

+148
-269
lines changed

‎src/EditorFeatures/Test/Diagnostics/DiagnosticAnalyzerServiceTests.cs‎

Lines changed: 0 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -232,53 +232,6 @@ private static async Task TestAnalyzerAsync(
232232
Assert.Equal(expectedSemantic, semantic);
233233
}
234234

235-
[Fact]
236-
public async Task TestHostAnalyzerOrderingAsync()
237-
{
238-
using var workspace = CreateWorkspace();
239-
var exportProvider = workspace.Services.SolutionServices.ExportProvider;
240-
241-
var analyzerReference = new AnalyzerImageReference(
242-
[
243-
new Priority20Analyzer(),
244-
new Priority15Analyzer(),
245-
new Priority10Analyzer(),
246-
new Priority1Analyzer(),
247-
new Priority0Analyzer(),
248-
new CSharpCompilerDiagnosticAnalyzer(),
249-
new Analyzer()
250-
,
251-
]);
252-
253-
workspace.TryApplyChanges(workspace.CurrentSolution.WithAnalyzerReferences([analyzerReference]));
254-
255-
var project = workspace.AddProject(
256-
ProjectInfo.Create(
257-
ProjectId.CreateNewId(),
258-
VersionStamp.Create(),
259-
"Dummy",
260-
"Dummy",
261-
LanguageNames.CSharp));
262-
263-
var service = Assert.IsType<DiagnosticAnalyzerService>(exportProvider.GetExportedValue<IDiagnosticAnalyzerService>());
264-
265-
var analyzers = await service.GetTestAccessor().GetAnalyzersAsync(project, CancellationToken.None).ConfigureAwait(false);
266-
var analyzersArray = analyzers.ToArray();
267-
268-
AssertEx.Equal(
269-
[
270-
typeof(FileContentLoadAnalyzer),
271-
typeof(GeneratorDiagnosticsPlaceholderAnalyzer),
272-
typeof(CSharpCompilerDiagnosticAnalyzer),
273-
typeof(Analyzer),
274-
typeof(Priority0Analyzer),
275-
typeof(Priority1Analyzer),
276-
typeof(Priority10Analyzer),
277-
typeof(Priority15Analyzer),
278-
typeof(Priority20Analyzer)
279-
], analyzersArray.Select(a => a.GetType()));
280-
}
281-
282235
[Fact]
283236
public async Task TestHostAnalyzerErrorNotLeaking()
284237
{
@@ -931,44 +884,6 @@ public override void Initialize(AnalysisContext context)
931884
}
932885
}
933886

934-
private sealed class NoNameAnalyzer : DocumentDiagnosticAnalyzer
935-
{
936-
internal static readonly DiagnosticDescriptor s_syntaxRule = new DiagnosticDescriptor("syntax", "test", "test", "test", DiagnosticSeverity.Error, isEnabledByDefault: true);
937-
938-
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => [s_syntaxRule];
939-
940-
public override Task<ImmutableArray<Diagnostic>> AnalyzeSyntaxAsync(Document document, CancellationToken cancellationToken)
941-
=> Task.FromResult(ImmutableArray.Create(Diagnostic.Create(s_syntaxRule, Location.Create(document.FilePath, TextSpan.FromBounds(0, 0), new LinePositionSpan(new LinePosition(0, 0), new LinePosition(0, 0))))));
942-
943-
public override Task<ImmutableArray<Diagnostic>> AnalyzeSemanticsAsync(Document document, CancellationToken cancellationToken)
944-
=> SpecializedTasks.Default<ImmutableArray<Diagnostic>>();
945-
}
946-
947-
private sealed class Priority20Analyzer : PriorityTestDocumentDiagnosticAnalyzer
948-
{
949-
public Priority20Analyzer() : base(priority: 20) { }
950-
}
951-
952-
private sealed class Priority15Analyzer : PriorityTestProjectDiagnosticAnalyzer
953-
{
954-
public Priority15Analyzer() : base(priority: 15) { }
955-
}
956-
957-
private sealed class Priority10Analyzer : PriorityTestDocumentDiagnosticAnalyzer
958-
{
959-
public Priority10Analyzer() : base(priority: 10) { }
960-
}
961-
962-
private sealed class Priority1Analyzer : PriorityTestProjectDiagnosticAnalyzer
963-
{
964-
public Priority1Analyzer() : base(priority: 1) { }
965-
}
966-
967-
private sealed class Priority0Analyzer : PriorityTestDocumentDiagnosticAnalyzer
968-
{
969-
public Priority0Analyzer() : base(priority: -1) { }
970-
}
971-
972887
private class PriorityTestDocumentDiagnosticAnalyzer : DocumentDiagnosticAnalyzer
973888
{
974889
protected PriorityTestDocumentDiagnosticAnalyzer(int priority)
@@ -982,17 +897,6 @@ public override Task<ImmutableArray<Diagnostic>> AnalyzeSyntaxAsync(Document doc
982897
=> Task.FromResult(ImmutableArray<Diagnostic>.Empty);
983898
}
984899

985-
private class PriorityTestProjectDiagnosticAnalyzer : ProjectDiagnosticAnalyzer
986-
{
987-
protected PriorityTestProjectDiagnosticAnalyzer(int priority)
988-
=> Priority = priority;
989-
990-
public override int Priority { get; }
991-
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => [];
992-
public override Task<ImmutableArray<Diagnostic>> AnalyzeProjectAsync(Project project, CancellationToken cancellationToken)
993-
=> Task.FromResult(ImmutableArray<Diagnostic>.Empty);
994-
}
995-
996900
private sealed class LeakDocumentAnalyzer : DocumentDiagnosticAnalyzer
997901
{
998902
internal static readonly DiagnosticDescriptor s_syntaxRule = new DiagnosticDescriptor("leak", "test", "test", "test", DiagnosticSeverity.Error, isEnabledByDefault: true);

‎src/Features/Core/Portable/Diagnostics/DiagnosticAnalyzerExtensions.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ internal static class DiagnosticAnalyzerExtensions
1616
public static bool IsWorkspaceDiagnosticAnalyzer(this DiagnosticAnalyzer analyzer)
1717
=> analyzer is DocumentDiagnosticAnalyzer
1818
|| analyzer is ProjectDiagnosticAnalyzer
19-
|| analyzer == FileContentLoadAnalyzer.Instance
2019
|| analyzer == GeneratorDiagnosticsPlaceholderAnalyzer.Instance;
2120

2221
public static bool IsBuiltInAnalyzer(this DiagnosticAnalyzer analyzer)

‎src/LanguageServer/Protocol/Features/Diagnostics/DocumentAnalysisExecutor.cs‎

Lines changed: 7 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -77,19 +77,6 @@ public async Task<IEnumerable<DiagnosticData>> ComputeDiagnosticsAsync(Diagnosti
7777
var kind = AnalysisScope.Kind;
7878

7979
var document = textDocument as Document;
80-
RoslynDebug.Assert(document != null || kind == AnalysisKind.Syntax, "We only support syntactic analysis for non-source documents");
81-
82-
var loadDiagnostic = await textDocument.State.GetLoadDiagnosticAsync(cancellationToken).ConfigureAwait(false);
83-
84-
if (analyzer == FileContentLoadAnalyzer.Instance)
85-
{
86-
return loadDiagnostic != null
87-
? [DiagnosticData.Create(loadDiagnostic, textDocument)]
88-
: [];
89-
}
90-
91-
if (loadDiagnostic != null)
92-
return [];
9380

9481
if (analyzer == GeneratorDiagnosticsPlaceholderAnalyzer.Instance)
9582
{
@@ -107,14 +94,14 @@ public async Task<IEnumerable<DiagnosticData>> ComputeDiagnosticsAsync(Diagnosti
10794

10895
if (analyzer is DocumentDiagnosticAnalyzer documentAnalyzer)
10996
{
110-
if (document == null)
111-
return [];
112-
11397
// DocumentDiagnosticAnalyzer is a host-only analyzer
98+
var tree = document is null
99+
? null
100+
: await document.GetSyntaxTreeAsync(cancellationToken).ConfigureAwait(false);
114101
var documentDiagnostics = await ComputeDocumentDiagnosticAnalyzerDiagnosticsAsync(
115-
documentAnalyzer, document, kind, _compilationWithAnalyzers?.HostCompilation, cancellationToken).ConfigureAwait(false);
102+
documentAnalyzer, textDocument, kind, _compilationWithAnalyzers?.HostCompilation, tree, cancellationToken).ConfigureAwait(false);
116103

117-
return ConvertToLocalDiagnostics(documentDiagnostics, document, span);
104+
return ConvertToLocalDiagnostics(documentDiagnostics, textDocument, span);
118105
}
119106

120107
// quick optimization to reduce allocations.
@@ -155,9 +142,9 @@ public async Task<IEnumerable<DiagnosticData>> ComputeDiagnosticsAsync(Diagnosti
155142
// Remap diagnostic locations, if required.
156143
diagnostics = await RemapDiagnosticLocationsIfRequiredAsync(textDocument, diagnostics, cancellationToken).ConfigureAwait(false);
157144

158-
if (span.HasValue && document != null)
145+
if (span.HasValue)
159146
{
160-
var sourceText = await document.GetValueTextAsync(cancellationToken).ConfigureAwait(false);
147+
var sourceText = await textDocument.GetValueTextAsync(cancellationToken).ConfigureAwait(false);
161148

162149
// TODO: Unclear if using the unmapped span here is correct. It does feel somewhat appropriate as the
163150
// caller should be asking about diagnostics in an actual document, and not where they were remapped to.

‎src/LanguageServer/Protocol/Features/Diagnostics/DocumentAnalysisExecutor_Helpers.cs‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -157,24 +157,25 @@ public static bool IsAnalyzerEnabledForProject(DiagnosticAnalyzer analyzer, Proj
157157

158158
public static async Task<ImmutableArray<Diagnostic>> ComputeDocumentDiagnosticAnalyzerDiagnosticsAsync(
159159
DocumentDiagnosticAnalyzer analyzer,
160-
Document document,
160+
TextDocument document,
161161
AnalysisKind kind,
162162
Compilation? compilation,
163+
SyntaxTree? tree,
163164
CancellationToken cancellationToken)
164165
{
165166
cancellationToken.ThrowIfCancellationRequested();
166167

167168
ImmutableArray<Diagnostic> diagnostics;
168169
try
169170
{
170-
var analyzeAsync = kind switch
171+
diagnostics = kind switch
171172
{
172-
AnalysisKind.Syntax => analyzer.AnalyzeSyntaxAsync(document, cancellationToken),
173-
AnalysisKind.Semantic => analyzer.AnalyzeSemanticsAsync(document, cancellationToken),
173+
AnalysisKind.Syntax => await analyzer.AnalyzeSyntaxAsync(document, tree, cancellationToken).ConfigureAwait(false),
174+
AnalysisKind.Semantic => await analyzer.AnalyzeSemanticsAsync(document, tree, cancellationToken).ConfigureAwait(false),
174175
_ => throw ExceptionUtilities.UnexpectedValue(kind),
175176
};
176177

177-
diagnostics = (await analyzeAsync.ConfigureAwait(false)).NullToEmpty();
178+
diagnostics = diagnostics.NullToEmpty();
178179

179180
#if DEBUG
180181
// since all DocumentDiagnosticAnalyzers are from internal users, we only do debug check. also this can be expensive at runtime

‎src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.Executor.cs‎

Lines changed: 19 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
using Microsoft.CodeAnalysis.Diagnostics.Telemetry;
1212
using Microsoft.CodeAnalysis.ErrorReporting;
1313
using Microsoft.CodeAnalysis.Internal.Log;
14+
using Microsoft.CodeAnalysis.Shared.Extensions;
1415
using Microsoft.CodeAnalysis.Workspaces.Diagnostics;
1516
using Roslyn.Utilities;
1617

@@ -138,7 +139,7 @@ async Task<ImmutableDictionary<DiagnosticAnalyzer, DiagnosticAnalysisResult>> Me
138139
{
139140
var compilation = compilationWithAnalyzers?.HostCompilation;
140141

141-
(result, var failedDocuments) = await UpdateWithDocumentLoadAndGeneratorFailuresAsync(result).ConfigureAwait(false);
142+
result = await UpdateWithGeneratorFailuresAsync(result).ConfigureAwait(false);
142143

143144
foreach (var analyzer in ideAnalyzers)
144145
{
@@ -147,22 +148,23 @@ async Task<ImmutableDictionary<DiagnosticAnalyzer, DiagnosticAnalysisResult>> Me
147148
switch (analyzer)
148149
{
149150
case DocumentDiagnosticAnalyzer documentAnalyzer:
150-
foreach (var document in project.Documents)
151+
foreach (var textDocument in project.AdditionalDocuments.Concat(project.Documents))
151152
{
152-
// don't analyze documents whose content failed to load
153-
if (failedDocuments == null || !failedDocuments.Contains(document))
153+
var tree = textDocument is Document document
154+
? await document.GetSyntaxTreeAsync(cancellationToken).ConfigureAwait(false)
155+
: null;
156+
var syntaxDiagnostics = await DocumentAnalysisExecutor.ComputeDocumentDiagnosticAnalyzerDiagnosticsAsync(documentAnalyzer, textDocument, AnalysisKind.Syntax, compilation, tree, cancellationToken).ConfigureAwait(false);
157+
var semanticDiagnostics = await DocumentAnalysisExecutor.ComputeDocumentDiagnosticAnalyzerDiagnosticsAsync(documentAnalyzer, textDocument, AnalysisKind.Semantic, compilation, tree, cancellationToken).ConfigureAwait(false);
158+
159+
if (tree != null)
160+
{
161+
builder.AddSyntaxDiagnostics(tree, syntaxDiagnostics);
162+
builder.AddSemanticDiagnostics(tree, semanticDiagnostics);
163+
}
164+
else
154165
{
155-
var tree = await document.GetSyntaxTreeAsync(cancellationToken).ConfigureAwait(false);
156-
if (tree != null)
157-
{
158-
builder.AddSyntaxDiagnostics(tree, await DocumentAnalysisExecutor.ComputeDocumentDiagnosticAnalyzerDiagnosticsAsync(documentAnalyzer, document, AnalysisKind.Syntax, compilation, cancellationToken).ConfigureAwait(false));
159-
builder.AddSemanticDiagnostics(tree, await DocumentAnalysisExecutor.ComputeDocumentDiagnosticAnalyzerDiagnosticsAsync(documentAnalyzer, document, AnalysisKind.Semantic, compilation, cancellationToken).ConfigureAwait(false));
160-
}
161-
else
162-
{
163-
builder.AddExternalSyntaxDiagnostics(document.Id, await DocumentAnalysisExecutor.ComputeDocumentDiagnosticAnalyzerDiagnosticsAsync(documentAnalyzer, document, AnalysisKind.Syntax, compilation, cancellationToken).ConfigureAwait(false));
164-
builder.AddExternalSemanticDiagnostics(document.Id, await DocumentAnalysisExecutor.ComputeDocumentDiagnosticAnalyzerDiagnosticsAsync(documentAnalyzer, document, AnalysisKind.Semantic, compilation, cancellationToken).ConfigureAwait(false));
165-
}
166+
builder.AddExternalSyntaxDiagnostics(textDocument.Id, syntaxDiagnostics);
167+
builder.AddExternalSemanticDiagnostics(textDocument.Id, semanticDiagnostics);
166168
}
167169
}
168170

@@ -187,35 +189,9 @@ async Task<ImmutableDictionary<DiagnosticAnalyzer, DiagnosticAnalysisResult>> Me
187189
}
188190
}
189191

190-
async Task<(ImmutableDictionary<DiagnosticAnalyzer, DiagnosticAnalysisResult> results, ImmutableHashSet<Document>? failedDocuments)> UpdateWithDocumentLoadAndGeneratorFailuresAsync(
192+
async Task<ImmutableDictionary<DiagnosticAnalyzer, DiagnosticAnalysisResult>> UpdateWithGeneratorFailuresAsync(
191193
ImmutableDictionary<DiagnosticAnalyzer, DiagnosticAnalysisResult> results)
192194
{
193-
ImmutableHashSet<Document>.Builder? failedDocuments = null;
194-
ImmutableDictionary<DocumentId, ImmutableArray<DiagnosticData>>.Builder? lazyLoadDiagnostics = null;
195-
196-
foreach (var document in project.Documents)
197-
{
198-
var loadDiagnostic = await document.State.GetLoadDiagnosticAsync(cancellationToken).ConfigureAwait(false);
199-
if (loadDiagnostic != null)
200-
{
201-
lazyLoadDiagnostics ??= ImmutableDictionary.CreateBuilder<DocumentId, ImmutableArray<DiagnosticData>>();
202-
lazyLoadDiagnostics.Add(document.Id, [DiagnosticData.Create(loadDiagnostic, document)]);
203-
204-
failedDocuments ??= ImmutableHashSet.CreateBuilder<Document>();
205-
failedDocuments.Add(document);
206-
}
207-
}
208-
209-
results = results.SetItem(
210-
FileContentLoadAnalyzer.Instance,
211-
DiagnosticAnalysisResult.Create(
212-
project,
213-
syntaxLocalMap: lazyLoadDiagnostics?.ToImmutable() ?? ImmutableDictionary<DocumentId, ImmutableArray<DiagnosticData>>.Empty,
214-
semanticLocalMap: ImmutableDictionary<DocumentId, ImmutableArray<DiagnosticData>>.Empty,
215-
nonLocalMap: ImmutableDictionary<DocumentId, ImmutableArray<DiagnosticData>>.Empty,
216-
others: [],
217-
documentIds: null));
218-
219195
var generatorDiagnostics = await _diagnosticAnalyzerRunner.GetSourceGeneratorDiagnosticsAsync(project, cancellationToken).ConfigureAwait(false);
220196
var diagnosticResultBuilder = new DiagnosticAnalysisResultBuilder(project);
221197
foreach (var generatorDiagnostic in generatorDiagnostics)
@@ -229,7 +205,7 @@ async Task<ImmutableDictionary<DiagnosticAnalyzer, DiagnosticAnalysisResult>> Me
229205
GeneratorDiagnosticsPlaceholderAnalyzer.Instance,
230206
DiagnosticAnalysisResult.CreateFromBuilder(diagnosticResultBuilder));
231207

232-
return (results, failedDocuments?.ToImmutable());
208+
return results;
233209
}
234210

235211
void UpdateAnalyzerTelemetryData(ImmutableDictionary<DiagnosticAnalyzer, AnalyzerTelemetryInfo> telemetry)

‎src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.HostAnalyzerInfo.cs‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,6 @@ static ImmutableHashSet<object> GetFeaturesAnalyzerReferenceIds(HostDiagnosticAn
9797

9898
private sealed class HostAnalyzerInfo
9999
{
100-
private const int FileContentLoadAnalyzerPriority = -4;
101100
private const int GeneratorDiagnosticsPlaceholderAnalyzerPriority = -3;
102101
private const int BuiltInCompilerPriority = -2;
103102
private const int RegularDiagnosticAnalyzerPriority = -1;
@@ -145,9 +144,8 @@ private static int GetPriority(DiagnosticAnalyzer state)
145144

146145
return state switch
147146
{
148-
FileContentLoadAnalyzer _ => FileContentLoadAnalyzerPriority,
149147
GeneratorDiagnosticsPlaceholderAnalyzer _ => GeneratorDiagnosticsPlaceholderAnalyzerPriority,
150-
DocumentDiagnosticAnalyzer analyzer => Math.Max(0, analyzer.Priority),
148+
DocumentDiagnosticAnalyzer analyzer => analyzer.Priority,
151149
ProjectDiagnosticAnalyzer analyzer => Math.Max(0, analyzer.Priority),
152150
_ => RegularDiagnosticAnalyzerPriority,
153151
};

0 commit comments

Comments
 (0)