From 3000f47a3428f5278e0a8d96973972c19b963ab2 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 18 May 2021 13:03:32 -0700 Subject: [PATCH 1/2] Fix out of bound crash in lsp navto. --- .../Extensions/ProtocolConversions.cs | 34 +++++++++++++++---- .../AbstractGoToDefinitionHandler.cs | 3 +- .../Symbols/WorkspaceSymbolsHandler.cs | 3 +- 3 files changed, 31 insertions(+), 9 deletions(-) diff --git a/src/Features/LanguageServer/Protocol/Extensions/ProtocolConversions.cs b/src/Features/LanguageServer/Protocol/Extensions/ProtocolConversions.cs index 48da63029f19b..0ef4613a5621e 100644 --- a/src/Features/LanguageServer/Protocol/Extensions/ProtocolConversions.cs +++ b/src/Features/LanguageServer/Protocol/Extensions/ProtocolConversions.cs @@ -197,11 +197,13 @@ public static LSP.Range TextSpanToRange(TextSpan textSpan, SourceText text) } public static Task DocumentSpanToLocationAsync(DocumentSpan documentSpan, CancellationToken cancellationToken) - => TextSpanToLocationAsync(documentSpan.Document, documentSpan.SourceSpan, cancellationToken); + => TextSpanToLocationAsync(documentSpan.Document, documentSpan.SourceSpan, isStale: false, cancellationToken); - public static async Task DocumentSpanToLocationWithTextAsync(DocumentSpan documentSpan, ClassifiedTextElement text, CancellationToken cancellationToken) + public static async Task DocumentSpanToLocationWithTextAsync( + DocumentSpan documentSpan, ClassifiedTextElement text, CancellationToken cancellationToken) { - var location = await TextSpanToLocationAsync(documentSpan.Document, documentSpan.SourceSpan, cancellationToken).ConfigureAwait(false); + var location = await TextSpanToLocationAsync( + documentSpan.Document, documentSpan.SourceSpan, isStale: false, cancellationToken).ConfigureAwait(false); return location == null ? null : new LSP.LocationWithText { @@ -280,18 +282,22 @@ public static LSP.Range TextSpanToRange(TextSpan textSpan, SourceText text) return documentEdits; } - public static async Task TextSpanToLocationAsync(Document document, TextSpan textSpan, CancellationToken cancellationToken) + public static async Task TextSpanToLocationAsync( + Document document, + TextSpan textSpan, + bool isStale, + CancellationToken cancellationToken) { var result = await GetMappedSpanResultAsync(document, ImmutableArray.Create(textSpan), cancellationToken).ConfigureAwait(false); if (result == null) { - return await ConvertTextSpanToLocation(document, textSpan, cancellationToken).ConfigureAwait(false); + return await ConvertTextSpanToLocation(document, textSpan, isStale, cancellationToken).ConfigureAwait(false); } var mappedSpan = result.Value.Single(); if (mappedSpan.IsDefault) { - return await ConvertTextSpanToLocation(document, textSpan, cancellationToken).ConfigureAwait(false); + return await ConvertTextSpanToLocation(document, textSpan, isStale, cancellationToken).ConfigureAwait(false); } return new LSP.Location @@ -300,10 +306,24 @@ public static LSP.Range TextSpanToRange(TextSpan textSpan, SourceText text) Range = MappedSpanResultToRange(mappedSpan) }; - static async Task ConvertTextSpanToLocation(Document document, TextSpan span, CancellationToken cancellationToken) + static async Task ConvertTextSpanToLocation( + Document document, + TextSpan span, + bool isStale, + CancellationToken cancellationToken) { var text = await document.GetTextAsync(cancellationToken).ConfigureAwait(false); + if (isStale) + { + // in the case of a stale item, the span may be out of bounds of the document. Cap + // us to the end of the document as that's where we're going to navigate the user + // to. + var start = Math.Min(text.Length, span.Start); + var end = Math.Min(text.Length, span.End); + span = TextSpan.FromBounds(start, end); + } + return ConvertTextSpanWithTextToLocation(span, text, document.GetURI()); } diff --git a/src/Features/LanguageServer/Protocol/Handler/Definitions/AbstractGoToDefinitionHandler.cs b/src/Features/LanguageServer/Protocol/Handler/Definitions/AbstractGoToDefinitionHandler.cs index 0759e9f573eca..75b06a9728f85 100644 --- a/src/Features/LanguageServer/Protocol/Handler/Definitions/AbstractGoToDefinitionHandler.cs +++ b/src/Features/LanguageServer/Protocol/Handler/Definitions/AbstractGoToDefinitionHandler.cs @@ -53,7 +53,8 @@ public AbstractGoToDefinitionHandler(IMetadataAsSourceFileService metadataAsSour continue; } - var location = await ProtocolConversions.TextSpanToLocationAsync(definition.Document, definition.SourceSpan, cancellationToken).ConfigureAwait(false); + var location = await ProtocolConversions.TextSpanToLocationAsync( + definition.Document, definition.SourceSpan, definition.IsStale, cancellationToken).ConfigureAwait(false); locations.AddIfNotNull(location); } } diff --git a/src/Features/LanguageServer/Protocol/Handler/Symbols/WorkspaceSymbolsHandler.cs b/src/Features/LanguageServer/Protocol/Handler/Symbols/WorkspaceSymbolsHandler.cs index cdcde12cbc943..387c09bc40886 100644 --- a/src/Features/LanguageServer/Protocol/Handler/Symbols/WorkspaceSymbolsHandler.cs +++ b/src/Features/LanguageServer/Protocol/Handler/Symbols/WorkspaceSymbolsHandler.cs @@ -105,7 +105,8 @@ public void ReportProgress(int current, int maximum) private async Task ReportSymbolInformationAsync(INavigateToSearchResult result, CancellationToken cancellationToken) { - var location = await ProtocolConversions.TextSpanToLocationAsync(result.NavigableItem.Document, result.NavigableItem.SourceSpan, cancellationToken).ConfigureAwait(false); + var location = await ProtocolConversions.TextSpanToLocationAsync( + result.NavigableItem.Document, result.NavigableItem.SourceSpan, result.NavigableItem.IsStale, cancellationToken).ConfigureAwait(false); Contract.ThrowIfNull(location); _progress.Report(new VSSymbolInformation { From e90d26e26ab6cc5566f7d6c117fb4d62b15b6419 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 18 May 2021 13:05:01 -0700 Subject: [PATCH 2/2] SImplify --- .../Protocol/Extensions/ProtocolConversions.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Features/LanguageServer/Protocol/Extensions/ProtocolConversions.cs b/src/Features/LanguageServer/Protocol/Extensions/ProtocolConversions.cs index 0ef4613a5621e..5a405668713a4 100644 --- a/src/Features/LanguageServer/Protocol/Extensions/ProtocolConversions.cs +++ b/src/Features/LanguageServer/Protocol/Extensions/ProtocolConversions.cs @@ -319,9 +319,9 @@ public static LSP.Range TextSpanToRange(TextSpan textSpan, SourceText text) // in the case of a stale item, the span may be out of bounds of the document. Cap // us to the end of the document as that's where we're going to navigate the user // to. - var start = Math.Min(text.Length, span.Start); - var end = Math.Min(text.Length, span.End); - span = TextSpan.FromBounds(start, end); + span = TextSpan.FromBounds( + Math.Min(text.Length, span.Start), + Math.Min(text.Length, span.End)); } return ConvertTextSpanWithTextToLocation(span, text, document.GetURI());