diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionListBuilder_Sync.cs b/src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionListBuilder_Sync.cs index a036e9d11e..b1a0ea746b 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionListBuilder_Sync.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionListBuilder_Sync.cs @@ -17,6 +17,7 @@ using CSharpCompletionItem = Microsoft.CodeAnalysis.Completion.CompletionItem; using CSharpCompletionList = Microsoft.CodeAnalysis.Completion.CompletionList; using CSharpCompletionService = Microsoft.CodeAnalysis.Completion.CompletionService; +using OmniSharp.Extensions; namespace OmniSharp.Roslyn.CSharp.Services.Completion { @@ -148,6 +149,9 @@ private static void GetCompletionInfo( // span. var adjustedNewPosition = change!.NewPosition; + var cursorPoint = sourceText.GetPointFromPosition(position); + var lineStartPosition = sourceText.GetPositionFromLineAndOffset(cursorPoint.Line, offset: 0); + // There must be at least one change that affects the current location, or something is seriously wrong Debug.Assert(change.TextChanges.Any(change => change.Span.IntersectsWith(position))); @@ -155,31 +159,57 @@ private static void GetCompletionInfo( { if (!textChange.Span.IntersectsWith(position)) { - additionalTextEdits ??= new(); - additionalTextEdits.Add(GetChangeForTextAndSpan(textChange.NewText!, textChange.Span, sourceText)); - - if (adjustedNewPosition is int newPosition) - { - // Find the diff between the original text length and the new text length. - var diff = (textChange.NewText?.Length ?? 0) - textChange.Span.Length; - - // If the new text is longer than the replaced text, we want to subtract that - // length from the current new position to find the adjusted position in the old - // document. If the new text was shorter, diff will be negative, and subtracting - // will result in increasing the adjusted position as expected - adjustedNewPosition = newPosition - diff; - } + handleNonInsertsectingEdit(sourceText, ref additionalTextEdits, ref adjustedNewPosition, textChange); } else { // Either there should be no new position, or it should be within the text that is being added // by this change. + int changeSpanStart = textChange.Span.Start; Debug.Assert(adjustedNewPosition is null || - (adjustedNewPosition.Value <= textChange.Span.Start + textChange.NewText!.Length) && - (adjustedNewPosition.Value >= textChange.Span.Start)); + (adjustedNewPosition.Value <= changeSpanStart + textChange.NewText!.Length) && + (adjustedNewPosition.Value >= changeSpanStart)); + + // Filtering needs a range that is a _single_ line. Consider a case like this (whitespace documented with escapes): + // + // 1: class C + // 2: {\t\r\n + // 3: override $$ + // + // Roslyn will see the trailing \t on line 2 and remove it when creating the _main_ text change. However, that will + // break filtering because filtering expects a single line as part of the range. So what we want to do is break the + // the text change up into two: one to cover the previous line, as an additional edit, and then one to cover the + // rest of the change. + + var updatedChange = textChange; + + if (changeSpanStart < lineStartPosition) + { + // We know we're in the special case. In order to correctly determine the amount of leading newlines to trim, we want + // to calculate the number of lines before the cursor we're editing + var editStartPoint = sourceText.GetPointFromPosition(changeSpanStart); + var numLinesEdited = cursorPoint.Line - editStartPoint.Line; - changeSpan = textChange.Span; - (insertText, insertTextFormat) = getPossiblySnippetizedInsertText(textChange, adjustedNewPosition); + Debug.Assert(textChange.NewText != null); + + // Now count that many newlines forward in the edited text + int cutoffPosition = 0; + for (int numNewlinesFound = 0; numNewlinesFound < numLinesEdited; cutoffPosition++) + { + if (textChange.NewText![cutoffPosition] == '\n') + { + numNewlinesFound++; + } + } + + // Now that we've found the cuttoff, we can build our two subchanges + var prefixChange = new TextChange(new TextSpan(changeSpanStart, length: lineStartPosition - changeSpanStart), textChange.NewText!.Substring(0, cutoffPosition)); + handleNonInsertsectingEdit(sourceText, ref additionalTextEdits, ref adjustedNewPosition, prefixChange); + updatedChange = new TextChange(new TextSpan(lineStartPosition, length: textChange.Span.End - lineStartPosition), textChange.NewText.Substring(cutoffPosition)); + } + + changeSpan = updatedChange.Span; + (insertText, insertTextFormat) = getPossiblySnippetizedInsertText(updatedChange, adjustedNewPosition); // If we're expecting there to be unimported types, put in an explicit sort text to put things already in scope first. // Otherwise, omit the sort text if it's the same as the label to save on space. @@ -236,6 +266,24 @@ private static void GetCompletionInfo( return ($"{beforeText}$0{afterText}", InsertTextFormat.Snippet); } + + static void handleNonInsertsectingEdit(SourceText sourceText, ref List? additionalTextEdits, ref int? adjustedNewPosition, TextChange textChange) + { + additionalTextEdits ??= new(); + additionalTextEdits.Add(GetChangeForTextAndSpan(textChange.NewText!, textChange.Span, sourceText)); + + if (adjustedNewPosition is int newPosition) + { + // Find the diff between the original text length and the new text length. + var diff = (textChange.NewText?.Length ?? 0) - textChange.Span.Length; + + // If the new text is longer than the replaced text, we want to subtract that + // length from the current new position to find the adjusted position in the old + // document. If the new text was shorter, diff will be negative, and subtracting + // will result in increasing the adjusted position as expected + adjustedNewPosition = newPosition - diff; + } + } } private static string? GetSortText(CSharpCompletionItem completion, string labelText, bool expectingImportedItems) diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/CompletionFacts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/CompletionFacts.cs index 93aa3bc6d4..b54d71d03a 100644 --- a/tests/OmniSharp.Roslyn.CSharp.Tests/CompletionFacts.cs +++ b/tests/OmniSharp.Roslyn.CSharp.Tests/CompletionFacts.cs @@ -2220,6 +2220,36 @@ public static void Main() } } + [Theory] + [InlineData("dummy.cs")] + [InlineData("dummy.csx")] + public async Task TestOverrideWithTrailingWhitespacePrior(string filename) + { + const string input = @" +namespace N +{ + internal class C + { +// The trailing tabs on the previous line and the next line are integral to this bug + + override $$ + public C() + { + } + } +} +"; + + var completions = await FindCompletionsAsync(filename, input, SharedOmniSharpTestHost); + + foreach (var item in completions.Items) + { + Assert.Single(item.AdditionalTextEdits); + Assert.Equal("\n // The trailing tabs on the previous line and the next line are integral to this bug\n\n", NormalizeNewlines(item.AdditionalTextEdits[0].NewText)); + Assert.StartsWith(" public override ", item.TextEdit.NewText); + } + } + private CompletionService GetCompletionService(OmniSharpTestHost host) => host.GetRequestHandler(EndpointName);