From 86171b02dc7dcd0aea04f2167cdf548d96a28e99 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 24 Feb 2025 01:08:06 -0800 Subject: [PATCH 1/3] Refresh diagnostics when fading options change --- .../Features/Diagnostics/DiagnosticAnalyzerService.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/LanguageServer/Protocol/Features/Diagnostics/DiagnosticAnalyzerService.cs b/src/LanguageServer/Protocol/Features/Diagnostics/DiagnosticAnalyzerService.cs index 79595a195b782..d70c50f09b744 100644 --- a/src/LanguageServer/Protocol/Features/Diagnostics/DiagnosticAnalyzerService.cs +++ b/src/LanguageServer/Protocol/Features/Diagnostics/DiagnosticAnalyzerService.cs @@ -68,7 +68,12 @@ public static bool IsGlobalOptionAffectingDiagnostics(IOption2 option) option == SolutionCrawlerOptionsStorage.BackgroundAnalysisScopeOption || option == SolutionCrawlerOptionsStorage.SolutionBackgroundAnalysisScopeOption || option == SolutionCrawlerOptionsStorage.CompilerDiagnosticsScopeOption || - option == s_crashOnAnalyzerException; + option == s_crashOnAnalyzerException || + // Fading is controlled by reporting diagnostics for the faded region. So if a fading option changes we + // want to recompute and rereport up to date diagnostics. + option == FadingOptions.FadeOutUnusedImports || + option == FadingOptions.FadeOutUnusedMembers || + option == FadingOptions.FadeOutUnreachableCode; public void RequestDiagnosticRefresh() => _diagnosticsRefresher.RequestWorkspaceRefresh(); From 0d312d2078bc43dcda47ddc3c2a22f4624ffb5f1 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 24 Feb 2025 01:40:52 -0800 Subject: [PATCH 2/3] Mix options into diagnostic data checksum --- .../AbstractPullDiagnosticHandler.cs | 3 +- .../Diagnostics/DiagnosticsPullCache.cs | 32 +++++++++++++++---- .../VersionedPullCache.CacheItem.cs | 3 +- .../PullHandlers/VersionedPullCache.cs | 15 +++++---- .../SourceGeneratedDocumentCache.cs | 2 +- .../Handler/SpellCheck/SpellCheckPullCache.cs | 2 +- 6 files changed, 41 insertions(+), 16 deletions(-) diff --git a/src/LanguageServer/Protocol/Handler/Diagnostics/AbstractPullDiagnosticHandler.cs b/src/LanguageServer/Protocol/Handler/Diagnostics/AbstractPullDiagnosticHandler.cs index 768adc82f6111..e3af0eef63d85 100644 --- a/src/LanguageServer/Protocol/Handler/Diagnostics/AbstractPullDiagnosticHandler.cs +++ b/src/LanguageServer/Protocol/Handler/Diagnostics/AbstractPullDiagnosticHandler.cs @@ -123,7 +123,8 @@ protected virtual Task WaitForChangesAsync(string? category, RequestContext cont var handlerName = $"{this.GetType().Name}(category: {category})"; context.TraceInformation($"{handlerName} started getting diagnostics"); - var versionedCache = _categoryToVersionedCache.GetOrAdd(handlerName, static handlerName => new(handlerName)); + var versionedCache = _categoryToVersionedCache.GetOrAdd( + handlerName, static (handlerName, globalOptions) => new(globalOptions, handlerName), GlobalOptions); // Get the set of results the request said were previously reported. We can use this to determine both // what to skip, and what files we have to tell the client have been removed. diff --git a/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticsPullCache.cs b/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticsPullCache.cs index 4a5bb70b16219..b4b702c4fa5de 100644 --- a/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticsPullCache.cs +++ b/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticsPullCache.cs @@ -5,7 +5,10 @@ using System.Collections.Immutable; using System.Threading; using System.Threading.Tasks; +using Microsoft.CodeAnalysis.CodeStyle; using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Options; +using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Text; using Roslyn.LanguageServer.Protocol; using Roslyn.Utilities; @@ -25,8 +28,11 @@ internal abstract partial class AbstractPullDiagnosticHandler - private sealed class DiagnosticsPullCache(string uniqueKey) : VersionedPullCache<(int globalStateVersion, VersionStamp? dependentVersion), (int globalStateVersion, Checksum dependentChecksum), DiagnosticsRequestState, ImmutableArray>(uniqueKey) + private sealed class DiagnosticsPullCache(IGlobalOptionService globalOptions, string uniqueKey) + : VersionedPullCache<(int globalStateVersion, VersionStamp? dependentVersion), (int globalStateVersion, Checksum dependentChecksum), DiagnosticsRequestState, ImmutableArray>(uniqueKey) { + private readonly IGlobalOptionService _globalOptions = globalOptions; + public override async Task<(int globalStateVersion, VersionStamp? dependentVersion)> ComputeCheapVersionAsync(DiagnosticsRequestState state, CancellationToken cancellationToken) { return (state.GlobalStateVersion, await state.Project.GetDependentVersionAsync(cancellationToken).ConfigureAwait(false)); @@ -45,14 +51,28 @@ public override async Task> ComputeDataAsync(Diag return diagnostics; } - public override Checksum ComputeChecksum(ImmutableArray data) + public override Checksum ComputeChecksum(ImmutableArray data, string language) { // Create checksums of diagnostic data and sort to ensure stable ordering for comparison. - var diagnosticDataChecksums = data - .SelectAsArray(d => Checksum.Create(d, SerializeDiagnosticData)) - .Sort(); + using var _ = ArrayBuilder.GetInstance(out var builder); + foreach (var datum in data) + builder.Add(Checksum.Create(datum, SerializeDiagnosticData)); + + // Ensure that if fading options change that we recompute the checksum as it will produce different data + // that we would report to the client. + var option1 = _globalOptions.GetOption(FadingOptions.FadeOutUnreachableCode, language); + var option2 = _globalOptions.GetOption(FadingOptions.FadeOutUnusedImports, language); + var option3 = _globalOptions.GetOption(FadingOptions.FadeOutUnusedMembers, language); + + var value = + (option1 ? (1 << 2) : 0) | + (option2 ? (1 << 1) : 0) | + (option3 ? (1 << 0) : 0); + + builder.Add(new Checksum(0, value)); + builder.Sort(); - return Checksum.Create(diagnosticDataChecksums); + return Checksum.Create(builder); } private static void SerializeDiagnosticData(DiagnosticData diagnosticData, ObjectWriter writer) diff --git a/src/LanguageServer/Protocol/Handler/PullHandlers/VersionedPullCache.CacheItem.cs b/src/LanguageServer/Protocol/Handler/PullHandlers/VersionedPullCache.CacheItem.cs index 6def0f52d7f81..4632d10f9b813 100644 --- a/src/LanguageServer/Protocol/Handler/PullHandlers/VersionedPullCache.CacheItem.cs +++ b/src/LanguageServer/Protocol/Handler/PullHandlers/VersionedPullCache.CacheItem.cs @@ -57,6 +57,7 @@ private sealed class CacheItem(string uniqueKey) PreviousPullResult? previousPullResult, bool isFullyLoaded, TState state, + string language, CancellationToken cancellationToken) { // Ensure that we only update the cache item one at a time. @@ -99,7 +100,7 @@ _lastResult is not null && // Compute the new result for the request. var data = await cache.ComputeDataAsync(state, cancellationToken).ConfigureAwait(false); - var dataChecksum = cache.ComputeChecksum(data); + var dataChecksum = cache.ComputeChecksum(data, language); string newResultId; if (_lastResult is not null && _lastResult?.resultId == previousPullResult?.PreviousResultId && _lastResult?.dataChecksum == dataChecksum) diff --git a/src/LanguageServer/Protocol/Handler/PullHandlers/VersionedPullCache.cs b/src/LanguageServer/Protocol/Handler/PullHandlers/VersionedPullCache.cs index aacaeca1cdc14..27b5263a0ca57 100644 --- a/src/LanguageServer/Protocol/Handler/PullHandlers/VersionedPullCache.cs +++ b/src/LanguageServer/Protocol/Handler/PullHandlers/VersionedPullCache.cs @@ -17,7 +17,8 @@ namespace Microsoft.CodeAnalysis.LanguageServer.Handler; /// that existing results can be reused, or if new results need to be computed. Multiple keys can be used, /// with different computation costs to determine if the previous cached data is still valid. /// -internal abstract partial class VersionedPullCache(string uniqueKey) +internal abstract partial class VersionedPullCache( + string uniqueKey) { /// /// Map of workspace and diagnostic source to the data used to make the last pull report. @@ -59,7 +60,7 @@ internal abstract partial class VersionedPullCache public abstract Task ComputeDataAsync(TState state, CancellationToken cancellationToken); - public abstract Checksum ComputeChecksum(TComputedData data); + public abstract Checksum ComputeChecksum(TComputedData data, string language); /// /// If results have changed since the last request this calculates and returns a new @@ -75,14 +76,16 @@ internal abstract partial class VersionedPullCache new CacheItem(uniqueKey)); - return await cacheEntry.UpdateCacheItemAsync(this, previousResult, isFullyLoaded, state, cancellationToken).ConfigureAwait(false); + var cacheEntry = _idToLastReportedResult.GetOrAdd( + (project.Solution.Workspace, projectOrDocumentId), + static (_, uniqueKey) => new CacheItem(uniqueKey), + uniqueKey); + return await cacheEntry.UpdateCacheItemAsync( + this, previousResult, isFullyLoaded, state, project.Language, cancellationToken).ConfigureAwait(false); } private long GetNextResultId() diff --git a/src/LanguageServer/Protocol/Handler/SourceGenerators/SourceGeneratedDocumentCache.cs b/src/LanguageServer/Protocol/Handler/SourceGenerators/SourceGeneratedDocumentCache.cs index 4bbdc4e175db9..31ebb7e377f02 100644 --- a/src/LanguageServer/Protocol/Handler/SourceGenerators/SourceGeneratedDocumentCache.cs +++ b/src/LanguageServer/Protocol/Handler/SourceGenerators/SourceGeneratedDocumentCache.cs @@ -30,7 +30,7 @@ internal sealed class SourceGeneratedDocumentCache(string uniqueKey) : Versioned return SpecializedTasks.Null(); } - public override Checksum ComputeChecksum(SourceText? data) + public override Checksum ComputeChecksum(SourceText? data, string language) { return data is null ? Checksum.Null : Checksum.From(data.GetChecksum()); } diff --git a/src/LanguageServer/Protocol/Handler/SpellCheck/SpellCheckPullCache.cs b/src/LanguageServer/Protocol/Handler/SpellCheck/SpellCheckPullCache.cs index d997635b6cb60..b334349d74e92 100644 --- a/src/LanguageServer/Protocol/Handler/SpellCheck/SpellCheckPullCache.cs +++ b/src/LanguageServer/Protocol/Handler/SpellCheck/SpellCheckPullCache.cs @@ -29,7 +29,7 @@ internal sealed class SpellCheckPullCache(string uniqueKey) : VersionedPullCache return (parseOptionsChecksum, textChecksum); } - public override Checksum ComputeChecksum(ImmutableArray data) + public override Checksum ComputeChecksum(ImmutableArray data, string language) { var checksums = data.SelectAsArray(s => Checksum.Create(s, SerializeSpellCheckSpan)).Sort(); return Checksum.Create(checksums); From 61394d15c893c005cff840a64177a7a078381232 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 24 Feb 2025 01:45:54 -0800 Subject: [PATCH 3/3] Apply suggestions from code review --- .../Protocol/Handler/PullHandlers/VersionedPullCache.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/LanguageServer/Protocol/Handler/PullHandlers/VersionedPullCache.cs b/src/LanguageServer/Protocol/Handler/PullHandlers/VersionedPullCache.cs index 27b5263a0ca57..f378b4b092ecf 100644 --- a/src/LanguageServer/Protocol/Handler/PullHandlers/VersionedPullCache.cs +++ b/src/LanguageServer/Protocol/Handler/PullHandlers/VersionedPullCache.cs @@ -17,8 +17,7 @@ namespace Microsoft.CodeAnalysis.LanguageServer.Handler; /// that existing results can be reused, or if new results need to be computed. Multiple keys can be used, /// with different computation costs to determine if the previous cached data is still valid. /// -internal abstract partial class VersionedPullCache( - string uniqueKey) +internal abstract partial class VersionedPullCache(string uniqueKey) { /// /// Map of workspace and diagnostic source to the data used to make the last pull report.