diff --git a/src/LanguageServer/Protocol/Features/Diagnostics/DiagnosticAnalyzerService.cs b/src/LanguageServer/Protocol/Features/Diagnostics/DiagnosticAnalyzerService.cs index 373a7f7be6d87..79595a195b782 100644 --- a/src/LanguageServer/Protocol/Features/Diagnostics/DiagnosticAnalyzerService.cs +++ b/src/LanguageServer/Protocol/Features/Diagnostics/DiagnosticAnalyzerService.cs @@ -3,7 +3,6 @@ // See the LICENSE file in the project root for more information. using System; -using System.Collections.Generic; using System.Collections.Immutable; using System.Composition; using System.Runtime.CompilerServices; @@ -13,7 +12,6 @@ using Microsoft.CodeAnalysis.CodeStyle; using Microsoft.CodeAnalysis.Host.Mef; using Microsoft.CodeAnalysis.Options; -using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Shared.TestHooks; using Microsoft.CodeAnalysis.SolutionCrawler; using Microsoft.CodeAnalysis.Text; diff --git a/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_GetDiagnostics.cs b/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_GetDiagnostics.cs index 4dd83e658d0ce..5d6c9799b1038 100644 --- a/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_GetDiagnostics.cs +++ b/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_GetDiagnostics.cs @@ -18,57 +18,82 @@ internal partial class DiagnosticAnalyzerService { private partial class DiagnosticIncrementalAnalyzer { - public Task> GetDiagnosticsForIdsAsync(Project project, DocumentId? documentId, ImmutableHashSet? diagnosticIds, Func? shouldIncludeAnalyzer, bool includeLocalDocumentDiagnostics, bool includeNonLocalDocumentDiagnostics, CancellationToken cancellationToken) - => new DiagnosticGetter(this, project, documentId, diagnosticIds, shouldIncludeAnalyzer, includeLocalDocumentDiagnostics, includeNonLocalDocumentDiagnostics).GetDiagnosticsAsync(cancellationToken); + public async Task> GetDiagnosticsForIdsAsync(Project project, DocumentId? documentId, ImmutableHashSet? diagnosticIds, Func? shouldIncludeAnalyzer, bool includeLocalDocumentDiagnostics, bool includeNonLocalDocumentDiagnostics, CancellationToken cancellationToken) + { + return await ProduceProjectDiagnosticsAsync( + project, diagnosticIds, shouldIncludeAnalyzer, + // Ensure we compute and return diagnostics for both the normal docs and the additional docs in this + // project if no specific document id was requested. + documentId != null ? [documentId] : [.. project.DocumentIds, .. project.AdditionalDocumentIds], + includeLocalDocumentDiagnostics, + includeNonLocalDocumentDiagnostics, + // return diagnostics specific to one project or document + includeProjectNonLocalResult: documentId == null, + cancellationToken).ConfigureAwait(false); + } - public Task> GetProjectDiagnosticsForIdsAsync(Project project, ImmutableHashSet? diagnosticIds, Func? shouldIncludeAnalyzer, bool includeNonLocalDocumentDiagnostics, CancellationToken cancellationToken) - => new DiagnosticGetter(this, project, documentId: null, diagnosticIds, shouldIncludeAnalyzer, includeLocalDocumentDiagnostics: false, includeNonLocalDocumentDiagnostics).GetProjectDiagnosticsAsync(cancellationToken); + public async Task> GetProjectDiagnosticsForIdsAsync( + Project project, + ImmutableHashSet? diagnosticIds, + Func? shouldIncludeAnalyzer, + bool includeNonLocalDocumentDiagnostics, + CancellationToken cancellationToken) + { + return await ProduceProjectDiagnosticsAsync( + project, diagnosticIds, shouldIncludeAnalyzer, + documentIds: [], + includeLocalDocumentDiagnostics: false, + includeNonLocalDocumentDiagnostics: includeNonLocalDocumentDiagnostics, + includeProjectNonLocalResult: true, + cancellationToken).ConfigureAwait(false); + } - private sealed class DiagnosticGetter( - DiagnosticIncrementalAnalyzer owner, + private async Task> ProduceProjectDiagnosticsAsync( Project project, - DocumentId? documentId, ImmutableHashSet? diagnosticIds, Func? shouldIncludeAnalyzer, + IReadOnlyList documentIds, bool includeLocalDocumentDiagnostics, - bool includeNonLocalDocumentDiagnostics) + bool includeNonLocalDocumentDiagnostics, + bool includeProjectNonLocalResult, + CancellationToken cancellationToken) { - private readonly DiagnosticIncrementalAnalyzer Owner = owner; - - private readonly Project Project = project; - private readonly DocumentId? DocumentId = documentId; - private readonly ImmutableHashSet? _diagnosticIds = diagnosticIds; - private readonly Func? _shouldIncludeAnalyzer = shouldIncludeAnalyzer; - private readonly bool IncludeLocalDocumentDiagnostics = includeLocalDocumentDiagnostics; - private readonly bool IncludeNonLocalDocumentDiagnostics = includeNonLocalDocumentDiagnostics; + using var _ = ArrayBuilder.GetInstance(out var builder); - private StateManager StateManager => Owner._stateManager; + var analyzersForProject = await _stateManager.GetOrCreateAnalyzersAsync(project, cancellationToken).ConfigureAwait(false); + var hostAnalyzerInfo = await _stateManager.GetOrCreateHostAnalyzerInfoAsync(project, cancellationToken).ConfigureAwait(false); + var analyzers = analyzersForProject.WhereAsArray(a => ShouldIncludeAnalyzer(project, a)); - private bool ShouldIncludeDiagnostic(DiagnosticData diagnostic) - => _diagnosticIds == null || _diagnosticIds.Contains(diagnostic.Id); + var result = await GetOrComputeDiagnosticAnalysisResultsAsync(analyzers).ConfigureAwait(false); - public async Task> GetDiagnosticsAsync(CancellationToken cancellationToken) + foreach (var analyzer in analyzers) { - // return diagnostics specific to one project or document - var includeProjectNonLocalResult = DocumentId == null; - return await ProduceProjectDiagnosticsAsync( - // Ensure we compute and return diagnostics for both the normal docs and the additional docs in this - // project if no specific document id was requested. - this.DocumentId != null ? [this.DocumentId] : [.. this.Project.DocumentIds, .. this.Project.AdditionalDocumentIds], - includeProjectNonLocalResult, cancellationToken).ConfigureAwait(false); - } + if (!result.TryGetValue(analyzer, out var analysisResult)) + continue; - private async Task> ProduceProjectDiagnosticsAsync( - IReadOnlyList documentIds, - bool includeProjectNonLocalResult, CancellationToken cancellationToken) - { - using var _ = ArrayBuilder.GetInstance(out var builder); - await this.ProduceDiagnosticsAsync( - documentIds, includeProjectNonLocalResult, builder, cancellationToken).ConfigureAwait(false); - return builder.ToImmutableAndClear(); + foreach (var documentId in documentIds) + { + if (includeLocalDocumentDiagnostics) + { + AddIncludedDiagnostics(builder, analysisResult.GetDocumentDiagnostics(documentId, AnalysisKind.Syntax)); + AddIncludedDiagnostics(builder, analysisResult.GetDocumentDiagnostics(documentId, AnalysisKind.Semantic)); + } + + if (includeNonLocalDocumentDiagnostics) + AddIncludedDiagnostics(builder, analysisResult.GetDocumentDiagnostics(documentId, AnalysisKind.NonLocal)); + } + + // include project diagnostics if there is no target document + if (includeProjectNonLocalResult) + AddIncludedDiagnostics(builder, analysisResult.GetOtherDiagnostics()); } - private void AddIncludedDiagnostics(ArrayBuilder builder, ImmutableArray diagnostics) + return builder.ToImmutableAndClear(); + + bool ShouldIncludeDiagnostic(DiagnosticData diagnostic) + => diagnosticIds == null || diagnosticIds.Contains(diagnostic.Id); + + void AddIncludedDiagnostics(ArrayBuilder builder, ImmutableArray diagnostics) { foreach (var diagnostic in diagnostics) { @@ -77,93 +102,43 @@ private void AddIncludedDiagnostics(ArrayBuilder builder, Immuta } } - public async Task> GetProjectDiagnosticsAsync(CancellationToken cancellationToken) - { - return await ProduceProjectDiagnosticsAsync( - documentIds: [], includeProjectNonLocalResult: true, cancellationToken).ConfigureAwait(false); - } - - private async Task ProduceDiagnosticsAsync( - IReadOnlyList documentIds, - bool includeProjectNonLocalResult, - ArrayBuilder builder, - CancellationToken cancellationToken) + async Task> GetOrComputeDiagnosticAnalysisResultsAsync( + ImmutableArray analyzers) { - var project = this.Project; - var analyzersForProject = await StateManager.GetOrCreateAnalyzersAsync(project, cancellationToken).ConfigureAwait(false); - var hostAnalyzerInfo = await StateManager.GetOrCreateHostAnalyzerInfoAsync(project, cancellationToken).ConfigureAwait(false); - var analyzers = analyzersForProject.WhereAsArray(a => ShouldIncludeAnalyzer(project, a)); - - var result = await GetOrComputeDiagnosticAnalysisResultsAsync(analyzers).ConfigureAwait(false); - - foreach (var analyzer in analyzers) + // If there was a 'ForceAnalyzeProjectAsync' run for this project, we can piggy back off of the + // prior computed/cached results as they will be a superset of the results we want. + // + // Note: the caller will loop over *its* analzyers, grabbing from the full set of data we've cached + // for this project, and filtering down further. So it's ok to return this potentially larger set. + // + // Note: While ForceAnalyzeProjectAsync should always run with a larger set of analyzers than us + // (since it runs all analyzers), we still run a paranoia check that the analyzers we care about are + // a subset of that call so that we don't accidentally reuse results that would not correspond to + // what we are computing ourselves. + if (_projectToForceAnalysisData.TryGetValue(project, out var box) && + analyzers.IsSubsetOf(box.Value.analyzers)) { - if (!result.TryGetValue(analyzer, out var analysisResult)) - continue; - - foreach (var documentId in documentIds) - { - if (IncludeLocalDocumentDiagnostics) - { - AddIncludedDiagnostics(builder, analysisResult.GetDocumentDiagnostics(documentId, AnalysisKind.Syntax)); - AddIncludedDiagnostics(builder, analysisResult.GetDocumentDiagnostics(documentId, AnalysisKind.Semantic)); - } - - if (IncludeNonLocalDocumentDiagnostics) - AddIncludedDiagnostics(builder, analysisResult.GetDocumentDiagnostics(documentId, AnalysisKind.NonLocal)); - } - - if (includeProjectNonLocalResult) - { - // include project diagnostics if there is no target document - AddIncludedDiagnostics(builder, analysisResult.GetOtherDiagnostics()); - } + return box.Value.diagnosticAnalysisResults; } - async Task> GetOrComputeDiagnosticAnalysisResultsAsync( - ImmutableArray analyzers) - { - // If there was a 'ForceAnalyzeProjectAsync' run for this project, we can piggy back off of the - // prior computed/cached results as they will be a superset of the results we want. - // - // Note: the caller will loop over *its* analzyers, grabbing from the full set of data we've cached - // for this project, and filtering down further. So it's ok to return this potentially larger set. - // - // Note: While ForceAnalyzeProjectAsync should always run with a larger set of analyzers than us - // (since it runs all analyzers), we still run a paranoia check that the analyzers we care about are - // a subset of that call so that we don't accidentally reuse results that would not correspond to - // what we are computing ourselves. - if (this.Owner._projectToForceAnalysisData.TryGetValue(project, out var box) && - analyzers.IsSubsetOf(box.Value.analyzers)) - { - return box.Value.diagnosticAnalysisResults; - } - - // Otherwise, just compute for the analyzers we care about. - var compilation = await GetOrCreateCompilationWithAnalyzersAsync( - project, analyzers, hostAnalyzerInfo, Owner.AnalyzerService.CrashOnAnalyzerException, cancellationToken).ConfigureAwait(false); + // Otherwise, just compute for the analyzers we care about. + var compilation = await GetOrCreateCompilationWithAnalyzersAsync( + project, analyzers, hostAnalyzerInfo, AnalyzerService.CrashOnAnalyzerException, cancellationToken).ConfigureAwait(false); - var result = await Owner.ComputeDiagnosticAnalysisResultsAsync(compilation, project, analyzers, cancellationToken).ConfigureAwait(false); - return result; - } + var result = await ComputeDiagnosticAnalysisResultsAsync(compilation, project, analyzers, cancellationToken).ConfigureAwait(false); + return result; } - private bool ShouldIncludeAnalyzer(Project project, DiagnosticAnalyzer analyzer) + bool ShouldIncludeAnalyzer(Project project, DiagnosticAnalyzer analyzer) { - if (!DocumentAnalysisExecutor.IsAnalyzerEnabledForProject(analyzer, project, Owner.GlobalOptions)) - { + if (!DocumentAnalysisExecutor.IsAnalyzerEnabledForProject(analyzer, project, this.GlobalOptions)) return false; - } - if (_shouldIncludeAnalyzer != null && !_shouldIncludeAnalyzer(analyzer)) - { + if (shouldIncludeAnalyzer != null && !shouldIncludeAnalyzer(analyzer)) return false; - } - if (_diagnosticIds != null && Owner.DiagnosticAnalyzerInfoCache.GetDiagnosticDescriptors(analyzer).All(d => !_diagnosticIds.Contains(d.Id))) - { + if (diagnosticIds != null && this.DiagnosticAnalyzerInfoCache.GetDiagnosticDescriptors(analyzer).All(d => !diagnosticIds.Contains(d.Id))) return false; - } return true; } diff --git a/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_GetDiagnosticsForSpan.cs b/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_GetDiagnosticsForSpan.cs index 14fdadf20f0a6..f53626179aa28 100644 --- a/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_GetDiagnosticsForSpan.cs +++ b/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_GetDiagnosticsForSpan.cs @@ -3,11 +3,9 @@ // See the LICENSE file in the project root for more information. using System; -using System.Collections.Generic; using System.Collections.Immutable; using System.Diagnostics; using System.Linq; -using System.Runtime.CompilerServices; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.CodeActions; @@ -25,24 +23,6 @@ internal partial class DiagnosticAnalyzerService { private partial class DiagnosticIncrementalAnalyzer { - public async Task> GetDiagnosticsForSpanAsync( - TextDocument document, - TextSpan? range, - Func? shouldIncludeDiagnostic, - ICodeActionRequestPriorityProvider priorityProvider, - DiagnosticKind diagnosticKinds, - bool isExplicit, - CancellationToken cancellationToken) - { - using var _ = ArrayBuilder.GetInstance(out var list); - - var getter = await LatestDiagnosticsForSpanGetter.CreateAsync( - this, document, range, priorityProvider, shouldIncludeDiagnostic, diagnosticKinds, isExplicit, cancellationToken).ConfigureAwait(false); - await getter.GetAsync(list, cancellationToken).ConfigureAwait(false); - - return list.ToImmutableAndClear(); - } - private static async Task>> ComputeDocumentDiagnosticsCoreAsync( DocumentAnalysisExecutor executor, CancellationToken cancellationToken) @@ -68,94 +48,45 @@ private static async Task> ComputeDocumentDiagnos return diagnostics?.ToImmutableArrayOrEmpty() ?? []; } - /// - /// Get diagnostics for given span either by using cache or calculating it on the spot. - /// - private sealed class LatestDiagnosticsForSpanGetter + public async Task> GetDiagnosticsForSpanAsync( + TextDocument document, + TextSpan? range, + Func? shouldIncludeDiagnostic, + ICodeActionRequestPriorityProvider priorityProvider, + DiagnosticKind diagnosticKind, + bool isExplicit, + CancellationToken cancellationToken) { - private readonly DiagnosticIncrementalAnalyzer _owner; - private readonly TextDocument _document; - private readonly SourceText _text; - - private readonly ImmutableArray _analyzers; - private readonly CompilationWithAnalyzersPair? _compilationWithAnalyzers; - - private readonly TextSpan? _range; - private readonly ICodeActionRequestPriorityProvider _priorityProvider; - private readonly Func? _shouldIncludeDiagnostic; - private readonly bool _isExplicit; - private readonly bool _logPerformanceInfo; - private readonly bool _incrementalAnalysis; - private readonly DiagnosticKind _diagnosticKind; - - public static async Task CreateAsync( - DiagnosticIncrementalAnalyzer owner, - TextDocument document, - TextSpan? range, - ICodeActionRequestPriorityProvider priorityProvider, - Func? shouldIncludeDiagnostic, - DiagnosticKind diagnosticKinds, - bool isExplicit, - CancellationToken cancellationToken) - { - var text = await document.GetValueTextAsync(cancellationToken).ConfigureAwait(false); - - var unfilteredAnalyzers = await owner._stateManager - .GetOrCreateAnalyzersAsync(document.Project, cancellationToken) - .ConfigureAwait(false); - var analyzers = unfilteredAnalyzers - .WhereAsArray(a => DocumentAnalysisExecutor.IsAnalyzerEnabledForProject(a, document.Project, owner.GlobalOptions)); - var hostAnalyzerInfo = await owner._stateManager.GetOrCreateHostAnalyzerInfoAsync(document.Project, cancellationToken).ConfigureAwait(false); - - // Note that some callers, such as diagnostic tagger, might pass in a range equal to the entire document span. - // We clear out range for such cases as we are computing full document diagnostics. - if (range == new TextSpan(0, text.Length)) - range = null; - - // We log performance info when we are computing diagnostics for a span - var logPerformanceInfo = range.HasValue; - var compilationWithAnalyzers = await GetOrCreateCompilationWithAnalyzersAsync( - document.Project, analyzers, hostAnalyzerInfo, owner.AnalyzerService.CrashOnAnalyzerException, cancellationToken).ConfigureAwait(false); - - // If we are computing full document diagnostics, we will attempt to perform incremental - // member edit analysis. This analysis is currently only enabled with LSP pull diagnostics. - var incrementalAnalysis = !range.HasValue - && document is Document { SupportsSyntaxTree: true }; - - return new LatestDiagnosticsForSpanGetter( - owner, compilationWithAnalyzers, document, text, analyzers, shouldIncludeDiagnostic, - range, priorityProvider, isExplicit, logPerformanceInfo, incrementalAnalysis, diagnosticKinds); - } + var text = await document.GetValueTextAsync(cancellationToken).ConfigureAwait(false); + + var unfilteredAnalyzers = await _stateManager + .GetOrCreateAnalyzersAsync(document.Project, cancellationToken) + .ConfigureAwait(false); + var analyzers = unfilteredAnalyzers + .WhereAsArray(a => DocumentAnalysisExecutor.IsAnalyzerEnabledForProject(a, document.Project, GlobalOptions)); + var hostAnalyzerInfo = await _stateManager.GetOrCreateHostAnalyzerInfoAsync(document.Project, cancellationToken).ConfigureAwait(false); + + // Note that some callers, such as diagnostic tagger, might pass in a range equal to the entire document span. + // We clear out range for such cases as we are computing full document diagnostics. + if (range == new TextSpan(0, text.Length)) + range = null; + + // We log performance info when we are computing diagnostics for a span + var logPerformanceInfo = range.HasValue; + var compilationWithAnalyzers = await GetOrCreateCompilationWithAnalyzersAsync( + document.Project, analyzers, hostAnalyzerInfo, AnalyzerService.CrashOnAnalyzerException, cancellationToken).ConfigureAwait(false); + + // If we are computing full document diagnostics, we will attempt to perform incremental + // member edit analysis. This analysis is currently only enabled with LSP pull diagnostics. + var incrementalAnalysis = !range.HasValue + && document is Document { SupportsSyntaxTree: true }; - private LatestDiagnosticsForSpanGetter( - DiagnosticIncrementalAnalyzer owner, - CompilationWithAnalyzersPair? compilationWithAnalyzers, - TextDocument document, - SourceText text, - ImmutableArray analyzers, - Func? shouldIncludeDiagnostic, - TextSpan? range, - ICodeActionRequestPriorityProvider priorityProvider, - bool isExplicit, - bool logPerformanceInfo, - bool incrementalAnalysis, - DiagnosticKind diagnosticKind) - { - _owner = owner; - _compilationWithAnalyzers = compilationWithAnalyzers; - _document = document; - _text = text; - _analyzers = analyzers; - _shouldIncludeDiagnostic = shouldIncludeDiagnostic; - _range = range; - _priorityProvider = priorityProvider; - _isExplicit = isExplicit; - _logPerformanceInfo = logPerformanceInfo; - _incrementalAnalysis = incrementalAnalysis; - _diagnosticKind = diagnosticKind; - } + using var _ = ArrayBuilder.GetInstance(out var list); + await GetAsync(list).ConfigureAwait(false); - public async Task GetAsync(ArrayBuilder list, CancellationToken cancellationToken) + return list.ToImmutableAndClear(); + + async Task GetAsync(ArrayBuilder list) { try { @@ -169,27 +100,27 @@ public async Task GetAsync(ArrayBuilder list, CancellationToken using var _2 = ArrayBuilder.GetInstance(out var semanticSpanBasedAnalyzers); using var _3 = ArrayBuilder.GetInstance(out var semanticDocumentBasedAnalyzers); - using var _4 = TelemetryLogging.LogBlockTimeAggregatedHistogram(FunctionId.RequestDiagnostics_Summary, $"Pri{_priorityProvider.Priority.GetPriorityInt()}"); + using var _4 = TelemetryLogging.LogBlockTimeAggregatedHistogram(FunctionId.RequestDiagnostics_Summary, $"Pri{priorityProvider.Priority.GetPriorityInt()}"); - foreach (var analyzer in _analyzers) + foreach (var analyzer in analyzers) { - if (!ShouldIncludeAnalyzer(analyzer, _shouldIncludeDiagnostic, _priorityProvider, _owner)) + if (!ShouldIncludeAnalyzer(analyzer, shouldIncludeDiagnostic, priorityProvider, this)) continue; bool includeSyntax = true, includeSemantic = true; - if (_diagnosticKind != DiagnosticKind.All) + if (diagnosticKind != DiagnosticKind.All) { var isCompilerAnalyzer = analyzer.IsCompilerAnalyzer(); includeSyntax = isCompilerAnalyzer - ? _diagnosticKind == DiagnosticKind.CompilerSyntax - : _diagnosticKind == DiagnosticKind.AnalyzerSyntax; + ? diagnosticKind == DiagnosticKind.CompilerSyntax + : diagnosticKind == DiagnosticKind.AnalyzerSyntax; includeSemantic = isCompilerAnalyzer - ? _diagnosticKind == DiagnosticKind.CompilerSemantic - : _diagnosticKind == DiagnosticKind.AnalyzerSemantic; + ? diagnosticKind == DiagnosticKind.CompilerSemantic + : diagnosticKind == DiagnosticKind.AnalyzerSemantic; } includeSyntax = includeSyntax && analyzer.SupportAnalysisKind(AnalysisKind.Syntax); - includeSemantic = includeSemantic && analyzer.SupportAnalysisKind(AnalysisKind.Semantic) && _document is Document; + includeSemantic = includeSemantic && analyzer.SupportAnalysisKind(AnalysisKind.Semantic) && document is Document; if (includeSyntax || includeSemantic) { @@ -200,8 +131,7 @@ public async Task GetAsync(ArrayBuilder list, CancellationToken if (includeSemantic) { - - if (!_incrementalAnalysis) + if (!incrementalAnalysis) { // For non-incremental analysis, we always attempt to compute all // analyzer diagnostics for the requested span. @@ -221,8 +151,8 @@ public async Task GetAsync(ArrayBuilder list, CancellationToken } } - await ComputeDocumentDiagnosticsAsync(syntaxAnalyzers.ToImmutable(), AnalysisKind.Syntax, _range, list, incrementalAnalysis: false, cancellationToken).ConfigureAwait(false); - await ComputeDocumentDiagnosticsAsync(semanticSpanBasedAnalyzers.ToImmutable(), AnalysisKind.Semantic, _range, list, _incrementalAnalysis, cancellationToken).ConfigureAwait(false); + await ComputeDocumentDiagnosticsAsync(syntaxAnalyzers.ToImmutable(), AnalysisKind.Syntax, range, list, incrementalAnalysis: false, cancellationToken).ConfigureAwait(false); + await ComputeDocumentDiagnosticsAsync(semanticSpanBasedAnalyzers.ToImmutable(), AnalysisKind.Semantic, range, list, incrementalAnalysis, cancellationToken).ConfigureAwait(false); await ComputeDocumentDiagnosticsAsync(semanticDocumentBasedAnalyzers.ToImmutable(), AnalysisKind.Semantic, span: null, list, incrementalAnalysis: false, cancellationToken).ConfigureAwait(false); return; @@ -231,43 +161,43 @@ public async Task GetAsync(ArrayBuilder list, CancellationToken { throw ExceptionUtilities.Unreachable(); } + } - // Local functions - static bool ShouldIncludeAnalyzer( - DiagnosticAnalyzer analyzer, - Func? shouldIncludeDiagnostic, - ICodeActionRequestPriorityProvider priorityProvider, - DiagnosticIncrementalAnalyzer owner) - { - // Skip executing analyzer if its priority does not match the request priority. - if (!priorityProvider.MatchesPriority(analyzer)) - return false; - - // Special case DocumentDiagnosticAnalyzer to never skip these document analyzers - // based on 'shouldIncludeDiagnostic' predicate. More specifically, TS has special document - // analyzer which report 0 supported diagnostics, but we always want to execute it. - if (analyzer is DocumentDiagnosticAnalyzer) - return true; - - // Special case GeneratorDiagnosticsPlaceholderAnalyzer to never skip it based on - // 'shouldIncludeDiagnostic' predicate. More specifically, this is a placeholder analyzer - // for threading through all source generator reported diagnostics, but this special analyzer - // reports 0 supported diagnostics, and we always want to execute it. - if (analyzer is GeneratorDiagnosticsPlaceholderAnalyzer) - return true; - - // Skip analyzer if none of its reported diagnostics should be included. - if (shouldIncludeDiagnostic != null && - !owner.DiagnosticAnalyzerInfoCache.GetDiagnosticDescriptors(analyzer).Any(static (a, shouldIncludeDiagnostic) => shouldIncludeDiagnostic(a.Id), shouldIncludeDiagnostic)) - { - return false; - } + // Local functions + static bool ShouldIncludeAnalyzer( + DiagnosticAnalyzer analyzer, + Func? shouldIncludeDiagnostic, + ICodeActionRequestPriorityProvider priorityProvider, + DiagnosticIncrementalAnalyzer owner) + { + // Skip executing analyzer if its priority does not match the request priority. + if (!priorityProvider.MatchesPriority(analyzer)) + return false; + + // Special case DocumentDiagnosticAnalyzer to never skip these document analyzers + // based on 'shouldIncludeDiagnostic' predicate. More specifically, TS has special document + // analyzer which report 0 supported diagnostics, but we always want to execute it. + if (analyzer is DocumentDiagnosticAnalyzer) + return true; + // Special case GeneratorDiagnosticsPlaceholderAnalyzer to never skip it based on + // 'shouldIncludeDiagnostic' predicate. More specifically, this is a placeholder analyzer + // for threading through all source generator reported diagnostics, but this special analyzer + // reports 0 supported diagnostics, and we always want to execute it. + if (analyzer is GeneratorDiagnosticsPlaceholderAnalyzer) return true; + + // Skip analyzer if none of its reported diagnostics should be included. + if (shouldIncludeDiagnostic != null && + !owner.DiagnosticAnalyzerInfoCache.GetDiagnosticDescriptors(analyzer).Any(static (a, shouldIncludeDiagnostic) => shouldIncludeDiagnostic(a.Id), shouldIncludeDiagnostic)) + { + return false; } + + return true; } - private async Task ComputeDocumentDiagnosticsAsync( + async Task ComputeDocumentDiagnosticsAsync( ImmutableArray analyzers, AnalysisKind kind, TextSpan? span, @@ -281,12 +211,12 @@ private async Task ComputeDocumentDiagnosticsAsync( using var _ = ArrayBuilder.GetInstance(analyzers.Length, out var filteredAnalyzers); foreach (var analyzer in analyzers) { - Debug.Assert(_priorityProvider.MatchesPriority(analyzer)); + Debug.Assert(priorityProvider.MatchesPriority(analyzer)); // Check if this is an expensive analyzer that needs to be de-prioritized to a lower priority bucket. // If so, we skip this analyzer from execution in the current priority bucket. // We will subsequently execute this analyzer in the lower priority bucket. - if (await TryDeprioritizeAnalyzerAsync(analyzer).ConfigureAwait(false)) + if (await TryDeprioritizeAnalyzerAsync(analyzer, kind, span).ConfigureAwait(false)) { continue; } @@ -299,20 +229,20 @@ private async Task ComputeDocumentDiagnosticsAsync( analyzers = filteredAnalyzers.ToImmutable(); - var hostAnalyzerInfo = await _owner._stateManager.GetOrCreateHostAnalyzerInfoAsync(_document.Project, cancellationToken).ConfigureAwait(false); + var hostAnalyzerInfo = await _stateManager.GetOrCreateHostAnalyzerInfoAsync(document.Project, cancellationToken).ConfigureAwait(false); var projectAnalyzers = analyzers.WhereAsArray(static (a, info) => !info.IsHostAnalyzer(a), hostAnalyzerInfo); var hostAnalyzers = analyzers.WhereAsArray(static (a, info) => info.IsHostAnalyzer(a), hostAnalyzerInfo); - var analysisScope = new DocumentAnalysisScope(_document, span, projectAnalyzers, hostAnalyzers, kind); - var executor = new DocumentAnalysisExecutor(analysisScope, _compilationWithAnalyzers, _owner._diagnosticAnalyzerRunner, _isExplicit, _logPerformanceInfo); - var version = await GetDiagnosticVersionAsync(_document.Project, cancellationToken).ConfigureAwait(false); + var analysisScope = new DocumentAnalysisScope(document, span, projectAnalyzers, hostAnalyzers, kind); + var executor = new DocumentAnalysisExecutor(analysisScope, compilationWithAnalyzers, _diagnosticAnalyzerRunner, isExplicit, logPerformanceInfo); + var version = await GetDiagnosticVersionAsync(document.Project, cancellationToken).ConfigureAwait(false); ImmutableDictionary> diagnosticsMap; if (incrementalAnalysis) { - using var _2 = TelemetryLogging.LogBlockTimeAggregatedHistogram(FunctionId.RequestDiagnostics_Summary, $"Pri{_priorityProvider.Priority.GetPriorityInt()}.Incremental"); + using var _2 = TelemetryLogging.LogBlockTimeAggregatedHistogram(FunctionId.RequestDiagnostics_Summary, $"Pri{priorityProvider.Priority.GetPriorityInt()}.Incremental"); - diagnosticsMap = await _owner._incrementalMemberEditAnalyzer.ComputeDiagnosticsAsync( + diagnosticsMap = await _incrementalMemberEditAnalyzer.ComputeDiagnosticsAsync( executor, analyzers, version, @@ -320,7 +250,7 @@ private async Task ComputeDocumentDiagnosticsAsync( } else { - using var _2 = TelemetryLogging.LogBlockTimeAggregatedHistogram(FunctionId.RequestDiagnostics_Summary, $"Pri{_priorityProvider.Priority.GetPriorityInt()}.Document"); + using var _2 = TelemetryLogging.LogBlockTimeAggregatedHistogram(FunctionId.RequestDiagnostics_Summary, $"Pri{priorityProvider.Priority.GetPriorityInt()}.Document"); diagnosticsMap = await ComputeDocumentDiagnosticsCoreAsync(executor, cancellationToken).ConfigureAwait(false); } @@ -332,79 +262,80 @@ private async Task ComputeDocumentDiagnosticsAsync( } if (incrementalAnalysis) - _owner._incrementalMemberEditAnalyzer.UpdateDocumentWithCachedDiagnostics((Document)_document); + _incrementalMemberEditAnalyzer.UpdateDocumentWithCachedDiagnostics((Document)document); + } - async Task TryDeprioritizeAnalyzerAsync(DiagnosticAnalyzer analyzer) + async Task TryDeprioritizeAnalyzerAsync( + DiagnosticAnalyzer analyzer, AnalysisKind kind, TextSpan? span) + { + // PERF: In order to improve lightbulb performance, we perform de-prioritization optimization for certain analyzers + // that moves the analyzer to a lower priority bucket. However, to ensure that de-prioritization happens for very rare cases, + // we only perform this optimizations when following conditions are met: + // 1. We are performing semantic span-based analysis. + // 2. We are processing 'CodeActionRequestPriority.Normal' priority request. + // 3. Analyzer registers certain actions that are known to lead to high performance impact due to its broad analysis scope, + // such as SymbolStart/End actions and SemanticModel actions. + // 4. Analyzer did not report a diagnostic on the same line in prior document snapshot. + + // Conditions 1. and 2. + if (kind != AnalysisKind.Semantic || + !span.HasValue || + priorityProvider.Priority != CodeActionRequestPriority.Default) { - // PERF: In order to improve lightbulb performance, we perform de-prioritization optimization for certain analyzers - // that moves the analyzer to a lower priority bucket. However, to ensure that de-prioritization happens for very rare cases, - // we only perform this optimizations when following conditions are met: - // 1. We are performing semantic span-based analysis. - // 2. We are processing 'CodeActionRequestPriority.Normal' priority request. - // 3. Analyzer registers certain actions that are known to lead to high performance impact due to its broad analysis scope, - // such as SymbolStart/End actions and SemanticModel actions. - // 4. Analyzer did not report a diagnostic on the same line in prior document snapshot. - - // Conditions 1. and 2. - if (kind != AnalysisKind.Semantic || - !span.HasValue || - _priorityProvider.Priority != CodeActionRequestPriority.Default) - { - return false; - } + return false; + } - Debug.Assert(span.Value.Length < _text.Length); + Debug.Assert(span.Value.Length < text.Length); - // Condition 3. - // Check if this is a candidate analyzer that can be de-prioritized into a lower priority bucket based on registered actions. - if (!await IsCandidateForDeprioritizationBasedOnRegisteredActionsAsync(analyzer).ConfigureAwait(false)) - { - return false; - } + // Condition 3. + // Check if this is a candidate analyzer that can be de-prioritized into a lower priority bucket based on registered actions. + if (!await IsCandidateForDeprioritizationBasedOnRegisteredActionsAsync(analyzer).ConfigureAwait(false)) + { + return false; + } - // 'LightbulbSkipExecutingDeprioritizedAnalyzers' option determines if we want to execute this analyzer - // in low priority bucket or skip it completely. If the option is not set, track the de-prioritized - // analyzer to be executed in low priority bucket. - // Note that 'AddDeprioritizedAnalyzerWithLowPriority' call below mutates the state in the provider to - // track this analyzer. This ensures that when the owner of this provider calls us back to execute - // the low priority bucket, we can still get back to this analyzer and execute it that time. - if (!_owner.GlobalOptions.GetOption(DiagnosticOptionsStorage.LightbulbSkipExecutingDeprioritizedAnalyzers)) - _priorityProvider.AddDeprioritizedAnalyzerWithLowPriority(analyzer); + // 'LightbulbSkipExecutingDeprioritizedAnalyzers' option determines if we want to execute this analyzer + // in low priority bucket or skip it completely. If the option is not set, track the de-prioritized + // analyzer to be executed in low priority bucket. + // Note that 'AddDeprioritizedAnalyzerWithLowPriority' call below mutates the state in the provider to + // track this analyzer. This ensures that when the owner of this provider calls us back to execute + // the low priority bucket, we can still get back to this analyzer and execute it that time. + if (!this.GlobalOptions.GetOption(DiagnosticOptionsStorage.LightbulbSkipExecutingDeprioritizedAnalyzers)) + priorityProvider.AddDeprioritizedAnalyzerWithLowPriority(analyzer); - return true; - } + return true; + } - // Returns true if this is an analyzer that is a candidate to be de-prioritized to - // 'CodeActionRequestPriority.Low' priority for improvement in analyzer - // execution performance for priority buckets above 'Low' priority. - // Based on performance measurements, currently only analyzers which register SymbolStart/End actions - // or SemanticModel actions are considered candidates to be de-prioritized. However, these semantics - // could be changed in future based on performance measurements. - async Task IsCandidateForDeprioritizationBasedOnRegisteredActionsAsync(DiagnosticAnalyzer analyzer) + // Returns true if this is an analyzer that is a candidate to be de-prioritized to + // 'CodeActionRequestPriority.Low' priority for improvement in analyzer + // execution performance for priority buckets above 'Low' priority. + // Based on performance measurements, currently only analyzers which register SymbolStart/End actions + // or SemanticModel actions are considered candidates to be de-prioritized. However, these semantics + // could be changed in future based on performance measurements. + async Task IsCandidateForDeprioritizationBasedOnRegisteredActionsAsync(DiagnosticAnalyzer analyzer) + { + // We deprioritize SymbolStart/End and SemanticModel analyzers from 'Normal' to 'Low' priority bucket, + // as these are computationally more expensive. + // Note that we never de-prioritize compiler analyzer, even though it registers a SemanticModel action. + if (compilationWithAnalyzers == null || + analyzer.IsWorkspaceDiagnosticAnalyzer() || + analyzer.IsCompilerAnalyzer()) { - // We deprioritize SymbolStart/End and SemanticModel analyzers from 'Normal' to 'Low' priority bucket, - // as these are computationally more expensive. - // Note that we never de-prioritize compiler analyzer, even though it registers a SemanticModel action. - if (_compilationWithAnalyzers == null || - analyzer.IsWorkspaceDiagnosticAnalyzer() || - analyzer.IsCompilerAnalyzer()) - { - return false; - } + return false; + } - var telemetryInfo = await _compilationWithAnalyzers.GetAnalyzerTelemetryInfoAsync(analyzer, cancellationToken).ConfigureAwait(false); - if (telemetryInfo == null) - return false; + var telemetryInfo = await compilationWithAnalyzers.GetAnalyzerTelemetryInfoAsync(analyzer, cancellationToken).ConfigureAwait(false); + if (telemetryInfo == null) + return false; - return telemetryInfo.SymbolStartActionsCount > 0 || telemetryInfo.SemanticModelActionsCount > 0; - } + return telemetryInfo.SymbolStartActionsCount > 0 || telemetryInfo.SemanticModelActionsCount > 0; } - private bool ShouldInclude(DiagnosticData diagnostic) + bool ShouldInclude(DiagnosticData diagnostic) { - return diagnostic.DocumentId == _document.Id && - (_range == null || _range.Value.IntersectsWith(diagnostic.DataLocation.UnmappedFileSpan.GetClampedTextSpan(_text))) - && (_shouldIncludeDiagnostic == null || _shouldIncludeDiagnostic(diagnostic.Id)); + return diagnostic.DocumentId == document.Id && + (range == null || range.Value.IntersectsWith(diagnostic.DataLocation.UnmappedFileSpan.GetClampedTextSpan(text))) + && (shouldIncludeDiagnostic == null || shouldIncludeDiagnostic(diagnostic.Id)); } } } diff --git a/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_IncrementalAnalyzer.cs b/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_IncrementalAnalyzer.cs index e879db37ddc0e..99147ea7972d1 100644 --- a/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_IncrementalAnalyzer.cs +++ b/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_IncrementalAnalyzer.cs @@ -22,7 +22,7 @@ private partial class DiagnosticIncrementalAnalyzer /// /// Cached data from a real instance to the cached diagnostic data produced by /// all the analyzers for the project. This data can then be used by to speed up subsequent calls through the normal to speed up subsequent calls through the normal entry points as long as the project hasn't changed at all. /// private readonly ConditionalWeakTable analyzers, ImmutableDictionary diagnosticAnalysisResults)>> _projectToForceAnalysisData = new(); @@ -67,7 +67,8 @@ public async Task> ForceAnalyzeProjectAsync(Proje var hostAnalyzerInfo = await _stateManager.GetOrCreateHostAnalyzerInfoAsync(project, cancellationToken).ConfigureAwait(false); var fullSolutionAnalysisAnalyzers = allAnalyzers.WhereAsArray( - static (analyzer, arg) => arg.self.IsCandidateForFullSolutionAnalysis(analyzer, arg.hostAnalyzerInfo.IsHostAnalyzer(analyzer), arg.project), + static (analyzer, arg) => IsCandidateForFullSolutionAnalysis( + arg.self.DiagnosticAnalyzerInfoCache, analyzer, arg.hostAnalyzerInfo.IsHostAnalyzer(analyzer), arg.project), (self: this, project, hostAnalyzerInfo)); var compilationWithAnalyzers = await GetOrCreateCompilationWithAnalyzersAsync( @@ -76,49 +77,50 @@ public async Task> ForceAnalyzeProjectAsync(Proje var projectAnalysisData = await ComputeDiagnosticAnalysisResultsAsync(compilationWithAnalyzers, project, fullSolutionAnalysisAnalyzers, cancellationToken).ConfigureAwait(false); return (fullSolutionAnalysisAnalyzers, projectAnalysisData); } - } - private bool IsCandidateForFullSolutionAnalysis(DiagnosticAnalyzer analyzer, bool isHostAnalyzer, Project project) - { - // PERF: Don't query descriptors for compiler analyzer or workspace load analyzer, always execute them. - if (analyzer == FileContentLoadAnalyzer.Instance || - analyzer == GeneratorDiagnosticsPlaceholderAnalyzer.Instance || - analyzer.IsCompilerAnalyzer()) + static bool IsCandidateForFullSolutionAnalysis( + DiagnosticAnalyzerInfoCache infoCache, DiagnosticAnalyzer analyzer, bool isHostAnalyzer, Project project) { - return true; - } + // PERF: Don't query descriptors for compiler analyzer or workspace load analyzer, always execute them. + if (analyzer == FileContentLoadAnalyzer.Instance || + analyzer == GeneratorDiagnosticsPlaceholderAnalyzer.Instance || + analyzer.IsCompilerAnalyzer()) + { + return true; + } - if (analyzer.IsBuiltInAnalyzer()) - { - // always return true for builtin analyzer. we can't use - // descriptor check since many builtin analyzer always return - // hidden descriptor regardless what descriptor it actually - // return on runtime. they do this so that they can control - // severity through option page rather than rule set editor. - // this is special behavior only ide analyzer can do. we hope - // once we support editorconfig fully, third party can use this - // ability as well and we can remove this kind special treatment on builtin - // analyzer. - return true; - } + if (analyzer.IsBuiltInAnalyzer()) + { + // always return true for builtin analyzer. we can't use + // descriptor check since many builtin analyzer always return + // hidden descriptor regardless what descriptor it actually + // return on runtime. they do this so that they can control + // severity through option page rather than rule set editor. + // this is special behavior only ide analyzer can do. we hope + // once we support editorconfig fully, third party can use this + // ability as well and we can remove this kind special treatment on builtin + // analyzer. + return true; + } - if (analyzer is DiagnosticSuppressor) - { - // Always execute diagnostic suppressors. - return true; - } + if (analyzer is DiagnosticSuppressor) + { + // Always execute diagnostic suppressors. + return true; + } - if (project.CompilationOptions is null) - { - // Skip compilation options based checks for non-C#/VB projects. - return true; - } + if (project.CompilationOptions is null) + { + // Skip compilation options based checks for non-C#/VB projects. + return true; + } - // For most of analyzers, the number of diagnostic descriptors is small, so this should be cheap. - var descriptors = DiagnosticAnalyzerInfoCache.GetDiagnosticDescriptors(analyzer); - var analyzerConfigOptions = project.GetAnalyzerConfigOptions(); + // For most of analyzers, the number of diagnostic descriptors is small, so this should be cheap. + var descriptors = infoCache.GetDiagnosticDescriptors(analyzer); + var analyzerConfigOptions = project.GetAnalyzerConfigOptions(); - return descriptors.Any(static (d, arg) => d.GetEffectiveSeverity(arg.CompilationOptions, arg.isHostAnalyzer ? arg.analyzerConfigOptions?.ConfigOptionsWithFallback : arg.analyzerConfigOptions?.ConfigOptionsWithoutFallback, arg.analyzerConfigOptions?.TreeOptions) != ReportDiagnostic.Hidden, (project.CompilationOptions, isHostAnalyzer, analyzerConfigOptions)); + return descriptors.Any(static (d, arg) => d.GetEffectiveSeverity(arg.CompilationOptions, arg.isHostAnalyzer ? arg.analyzerConfigOptions?.ConfigOptionsWithFallback : arg.analyzerConfigOptions?.ConfigOptionsWithoutFallback, arg.analyzerConfigOptions?.TreeOptions) != ReportDiagnostic.Hidden, (project.CompilationOptions, isHostAnalyzer, analyzerConfigOptions)); + } } } }