Merge pull request #75709 from dibarbet/inlay_hint_nfw

Recalculate if LSP inlay hint cache miss

src/LanguageServer/Protocol/Handler/CodeLens/CodeLensResolveHandler.cs

@@ -2,29 +2,31 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using System;
+using System.Composition;
+using System.Text.Json;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.CodeLens;
-using Roslyn.Utilities;
+using Microsoft.CodeAnalysis.Host.Mef;
 using Microsoft.CodeAnalysis.Shared.Extensions;
-using LSP = Roslyn.LanguageServer.Protocol;
-using System.Text.Json;
+using Roslyn.Utilities;
 using StreamJsonRpc;
+using LSP = Roslyn.LanguageServer.Protocol;
 
 namespace Microsoft.CodeAnalysis.LanguageServer.Handler.CodeLens;
 
+[ExportCSharpVisualBasicStatelessLspService(typeof(CodeLensResolveHandler)), Shared]
 [Method(LSP.Methods.CodeLensResolveName)]
-internal sealed class CodeLensResolveHandler : ILspServiceDocumentRequestHandler<LSP.CodeLens, LSP.CodeLens>
+[method: ImportingConstructor]
+[method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
+internal sealed class CodeLensResolveHandler() : ILspServiceDocumentRequestHandler<LSP.CodeLens, LSP.CodeLens>
 {
     /// <summary>
     /// Command name implemented by the client and invoked when the references code lens is selected.
     /// </summary>
     private const string ClientReferencesCommand = "roslyn.client.peekReferences";
 
-    public CodeLensResolveHandler()
-    {
-    }
-
     public bool MutatesSolutionState => false;
 
     public bool RequiresLSPSolution => true;

src/LanguageServer/Protocol/Handler/InlayHint/InlayHintCache.cs

@@ -17,5 +17,5 @@ public InlayHintCache() : base(maxCacheSize: 3)
     /// <summary>
     /// Cached data need to resolve a specific inlay hint item.
     /// </summary>
-    internal record InlayHintCacheEntry(ImmutableArray<InlineHint> InlayHintMembers, VersionStamp SyntaxVersion);
+    internal record InlayHintCacheEntry(ImmutableArray<InlineHint> InlayHintMembers);
 }

src/LanguageServer/Protocol/Handler/InlayHint/InlayHintHandler.cs

@@ -50,16 +50,12 @@ public TextDocumentIdentifier GetTextDocumentIdentifier(InlayHintParams request)
         internal static async Task<LSP.InlayHint[]?> GetInlayHintsAsync(Document document, TextDocumentIdentifier textDocumentIdentifier, LSP.Range range, InlineHintsOptions options, bool displayAllOverride, InlayHintCache inlayHintCache, CancellationToken cancellationToken)
         {
             var text = await document.GetValueTextAsync(cancellationToken).ConfigureAwait(false);
-            var textSpan = ProtocolConversions.RangeToTextSpan(range, text);
-
-            var inlineHintService = document.GetRequiredLanguageService<IInlineHintsService>();
-            var hints = await inlineHintService.GetInlineHintsAsync(document, textSpan, options, displayAllOverride, cancellationToken).ConfigureAwait(false);
-
+            var hints = await CalculateInlayHintsAsync(document, range, options, displayAllOverride, cancellationToken).ConfigureAwait(false);
             var syntaxVersion = await document.GetSyntaxVersionAsync(cancellationToken).ConfigureAwait(false);
 
             // Store the members in the resolve cache so that when we get a resolve request for a particular
             // member we can re-use the inline hint.
-            var resultId = inlayHintCache.UpdateCache(new InlayHintCache.InlayHintCacheEntry(hints, syntaxVersion));
+            var resultId = inlayHintCache.UpdateCache(new InlayHintCache.InlayHintCacheEntry(hints));
 
             var inlayHints = new LSP.InlayHint[hints.Length];
             for (var i = 0; i < hints.Length; i++)
@@ -89,7 +85,7 @@ public TextDocumentIdentifier GetTextDocumentIdentifier(InlayHintParams request)
                     ToolTip = null,
                     PaddingLeft = leftPadding,
                     PaddingRight = rightPadding,
-                    Data = new InlayHintResolveData(resultId, i, textDocumentIdentifier)
+                    Data = new InlayHintResolveData(resultId, i, textDocumentIdentifier, syntaxVersion.ToString(), range, displayAllOverride)
                 };
 
                 inlayHints[i] = inlayHint;
@@ -98,6 +94,16 @@ public TextDocumentIdentifier GetTextDocumentIdentifier(InlayHintParams request)
             return inlayHints;
         }
 
+        internal static async Task<ImmutableArray<InlineHint>> CalculateInlayHintsAsync(Document document, LSP.Range range, InlineHintsOptions options, bool displayAllOverride, CancellationToken cancellationToken)
+        {
+            var text = await document.GetValueTextAsync(cancellationToken).ConfigureAwait(false);
+            var textSpan = ProtocolConversions.RangeToTextSpan(range, text);
+
+            var inlineHintService = document.GetRequiredLanguageService<IInlineHintsService>();
+            var hints = await inlineHintService.GetInlineHintsAsync(document, textSpan, options, displayAllOverride, cancellationToken).ConfigureAwait(false);
+            return hints;
+        }
+
         /// <summary>
         /// Goes through the tagged text of the hint and trims off leading and trailing spaces. 
         /// If there is leading or trailing space, then we want to add padding to the left and right accordingly.

src/LanguageServer/Protocol/Handler/InlayHint/InlayHintResolveData.cs

@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using Microsoft.CodeAnalysis.Text;
 using Roslyn.LanguageServer.Protocol;
 
 namespace Microsoft.CodeAnalysis.LanguageServer.Handler.InlayHint;
@@ -12,4 +13,4 @@ namespace Microsoft.CodeAnalysis.LanguageServer.Handler.InlayHint;
 /// <param name="ResultId">the resultId associated with the inlay hint created on original request.</param>
 /// <param name="ListIndex">the index of the specific inlay hint item in the original list.</param>
 /// <param name="TextDocument">the text document associated with the inlay hint to resolve.</param>
-internal sealed record InlayHintResolveData(long ResultId, int ListIndex, TextDocumentIdentifier TextDocument) : DocumentResolveData(TextDocument);
+internal sealed record InlayHintResolveData(long ResultId, int ListIndex, TextDocumentIdentifier TextDocument, string SyntaxVersion, Range Range, bool DisplayAllOverride) : DocumentResolveData(TextDocument);

src/LanguageServer/Protocol/Handler/InlayHint/InlayHintResolveHandler.cs

@@ -2,27 +2,27 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using System;
+using System.Composition;
 using System.Text.Json;
 using System.Threading;
 using System.Threading.Tasks;
+using Microsoft.CodeAnalysis.Host.Mef;
 using Microsoft.CodeAnalysis.InlineHints;
+using Microsoft.CodeAnalysis.Options;
 using Roslyn.LanguageServer.Protocol;
 using Roslyn.Utilities;
 using StreamJsonRpc;
 using LSP = Roslyn.LanguageServer.Protocol;
 
 namespace Microsoft.CodeAnalysis.LanguageServer.Handler.InlayHint
 {
+    [ExportCSharpVisualBasicStatelessLspService(typeof(InlayHintResolveHandler)), Shared]
     [Method(Methods.InlayHintResolveName)]
-    internal sealed class InlayHintResolveHandler : ILspServiceDocumentRequestHandler<LSP.InlayHint, LSP.InlayHint>
+    [method: ImportingConstructor]
+    [method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
+    internal sealed class InlayHintResolveHandler(IGlobalOptionService globalOptions) : ILspServiceDocumentRequestHandler<LSP.InlayHint, LSP.InlayHint>
     {
-        private readonly InlayHintCache _inlayHintCache;
-
-        public InlayHintResolveHandler(InlayHintCache inlayHintCache)
-        {
-            _inlayHintCache = inlayHintCache;
-        }
-
         public bool MutatesSolutionState => false;
 
         public bool RequiresLSPSolution => true;
@@ -33,39 +33,53 @@ public TextDocumentIdentifier GetTextDocumentIdentifier(LSP.InlayHint request)
         public Task<LSP.InlayHint> HandleRequestAsync(LSP.InlayHint request, RequestContext context, CancellationToken cancellationToken)
         {
             var document = context.GetRequiredDocument();
-            return ResolveInlayHintAsync(document, request, _inlayHintCache, cancellationToken);
+            var options = globalOptions.GetInlineHintsOptions(document.Project.Language);
+            var inlayHintCache = context.GetRequiredService<InlayHintCache>();
+            var resolveData = GetInlayHintResolveData(request);
+            return ResolveInlayHintAsync(document, request, inlayHintCache, resolveData, options, cancellationToken);
         }
 
-        internal static async Task<LSP.InlayHint> ResolveInlayHintAsync(Document document, LSP.InlayHint request, InlayHintCache inlayHintCache, CancellationToken cancellationToken)
+        internal static async Task<LSP.InlayHint> ResolveInlayHintAsync(
+            Document document,
+            LSP.InlayHint request,
+            InlayHintCache inlayHintCache,
+            InlayHintResolveData resolveData,
+            InlineHintsOptions options,
+            CancellationToken cancellationToken)
         {
-            var resolveData = GetInlayHintResolveData(request);
-            var (cacheEntry, inlineHintToResolve) = GetCacheEntry(resolveData, inlayHintCache);
-
             var currentSyntaxVersion = await document.GetSyntaxVersionAsync(cancellationToken).ConfigureAwait(false);
-            var cachedSyntaxVersion = cacheEntry.SyntaxVersion;
+            var resolveSyntaxVersion = resolveData.SyntaxVersion;
 
-            if (currentSyntaxVersion != cachedSyntaxVersion)
+            if (currentSyntaxVersion.ToString() != resolveSyntaxVersion)
             {
-                throw new LocalRpcException($"Cached resolve version {cachedSyntaxVersion} does not match current version {currentSyntaxVersion}")
+                throw new LocalRpcException($"Request resolve version {resolveSyntaxVersion} does not match current version {currentSyntaxVersion}")
                 {
                     ErrorCode = LspErrorCodes.ContentModified
                 };
             }
 
-            var taggedText = await inlineHintToResolve.GetDescriptionAsync(document, cancellationToken).ConfigureAwait(false);
+            var inlineHintToResolve = GetCacheEntry(resolveData, inlayHintCache);
+            if (inlineHintToResolve is null)
+            {
+                // It is very possible that the cache no longer contains the hint being resolved (for example, multiple documents open side by side).
+                // Instead of throwing, we can recompute the hints since we've already verified above that the version has not changed.
+                var hints = await InlayHintHandler.CalculateInlayHintsAsync(document, resolveData.Range, options, resolveData.DisplayAllOverride, cancellationToken).ConfigureAwait(false);
+                inlineHintToResolve = hints[resolveData.ListIndex];
+            }
+
+            var taggedText = await inlineHintToResolve.Value.GetDescriptionAsync(document, cancellationToken).ConfigureAwait(false);
 
             request.ToolTip = ProtocolConversions.GetDocumentationMarkupContent(taggedText, document, true);
             return request;
         }
 
-        private static (InlayHintCache.InlayHintCacheEntry CacheEntry, InlineHint InlineHintToResolve) GetCacheEntry(InlayHintResolveData resolveData, InlayHintCache inlayHintCache)
+        private static InlineHint? GetCacheEntry(InlayHintResolveData resolveData, InlayHintCache inlayHintCache)
         {
             var cacheEntry = inlayHintCache.GetCachedEntry(resolveData.ResultId);
-            Contract.ThrowIfNull(cacheEntry, "Missing cache entry for inlay hint resolve request");
-            return (cacheEntry, cacheEntry.InlayHintMembers[resolveData.ListIndex]);
+            return cacheEntry?.InlayHintMembers[resolveData.ListIndex];
         }
 
-        private static InlayHintResolveData GetInlayHintResolveData(LSP.InlayHint inlayHint)
+        internal static InlayHintResolveData GetInlayHintResolveData(LSP.InlayHint inlayHint)
         {
             Contract.ThrowIfNull(inlayHint.Data);
             var resolveData = JsonSerializer.Deserialize<InlayHintResolveData>((JsonElement)inlayHint.Data, ProtocolConversions.LspJsonSerializerOptions);

src/LanguageServer/ProtocolUnitTests/InlayHint/CSharpInlayHintTests.cs

@@ -105,7 +105,7 @@ void X((int, bool) d)
         }
 
         [Theory, CombinatorialData]
-        public async Task TestDoesNotShutdownServerIfCacheEntryMissing(bool mutatingLspWorkspace)
+        public async Task TestReturnsInlayHintsEvenIfCacheMisses(bool mutatingLspWorkspace)
         {
             var markup =
 @"class A
@@ -148,12 +148,9 @@ void M()
             // Assert that the first result id is no longer in the cache.
             Assert.Null(cache.GetCachedEntry(firstResultId));
 
-            // Assert that the request throws because the item no longer exists in the cache.
-            await Assert.ThrowsAsync<RemoteInvocationException>(async () => await testLspServer.ExecuteRequestAsync<LSP.InlayHint, LSP.InlayHint>(LSP.Methods.InlayHintResolveName, firstInlayHint, CancellationToken.None));
-
-            // Assert that the server did not shutdown and that we can resolve the latest inlay hint request we made.
-            var lastInlayHint = await testLspServer.ExecuteRequestAsync<LSP.InlayHint, LSP.InlayHint>(LSP.Methods.InlayHintResolveName, lastInlayHints.First(), CancellationToken.None);
-            Assert.NotNull(lastInlayHint?.ToolTip);
+            // Assert that the resolve request returns the inlay hint even if not in the cache.
+            var firstResolvedHint = await testLspServer.ExecuteRequestAsync<LSP.InlayHint, LSP.InlayHint>(LSP.Methods.InlayHintResolveName, firstInlayHint, CancellationToken.None);
+            Assert.NotNull(firstResolvedHint?.ToolTip);
         }
 
         private async Task RunVerifyInlayHintAsync(string markup, bool mutatingLspWorkspace, bool hasTextEdits = true)

src/Tools/ExternalAccess/Razor/Cohost/Handlers/InlayHints.cs

@@ -26,6 +26,21 @@ internal static class InlayHints
             // always just result in the defaults, which for inline hints are to not show anything. However, the editor has a
             // setting for LSP inlay hints, so we can assume that if we get a request from the client, the user wants hints.
             // When overriding however, Roslyn does a nicer job if type hints are off.
+            var options = GetOptions(displayAllOverride);
+
+            return InlayHintHandler.GetInlayHintsAsync(document, textDocumentIdentifier, range, options, displayAllOverride, s_resolveCache, cancellationToken);
+        }
+
+        public static Task<InlayHint> ResolveInlayHintAsync(Document document, InlayHint request, CancellationToken cancellationToken)
+        {
+            Contract.ThrowIfNull(s_resolveCache, "Cache should never be null for resolve, since it should have been created by the original request");
+            var data = InlayHintResolveHandler.GetInlayHintResolveData(request);
+            var options = GetOptions(data.DisplayAllOverride);
+            return InlayHintResolveHandler.ResolveInlayHintAsync(document, request, s_resolveCache, data, options, cancellationToken);
+        }
+
+        private static InlineHintsOptions GetOptions(bool displayAllOverride)
+        {
             var options = InlineHintsOptions.Default;
             if (!displayAllOverride)
             {
@@ -36,14 +51,7 @@ internal static class InlayHints
                 };
             }
 
-            return InlayHintHandler.GetInlayHintsAsync(document, textDocumentIdentifier, range, options, displayAllOverride, s_resolveCache, cancellationToken);
-        }
-
-        public static Task<InlayHint> ResolveInlayHintAsync(Document document, InlayHint request, CancellationToken cancellationToken)
-        {
-            Contract.ThrowIfNull(s_resolveCache, "Cache should never be null for resolve, since it should have been created by the original request");
-
-            return InlayHintResolveHandler.ResolveInlayHintAsync(document, request, s_resolveCache, cancellationToken);
+            return options;
         }
     }
 }