diff --git a/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionItem.cs b/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionItem.cs index aceb370b5a..1a659e4617 100644 --- a/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionItem.cs +++ b/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionItem.cs @@ -61,10 +61,17 @@ public class CompletionItem public string? InsertText { get; set; } /// - /// The format of . + /// The format of . This applies to both and + /// .. /// public InsertTextFormat? InsertTextFormat { get; set; } + /// + /// An edit which is applied to a document when selecting this completion. When an edit is provided the value of + /// is ignored. + /// + public LinePositionSpanTextChange? TextEdit { get; set; } + /// /// An optional set of characters that when pressed while this completion is active will accept it first and /// then type that character. diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionService.cs index 187383eb8d..4f8f6eaf2d 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionService.cs @@ -162,6 +162,9 @@ public async Task Handle(CompletionRequest request) bool expectingImportedItems = expandedItemsAvailable && _workspace.Options.GetOption(CompletionItemExtensions.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp) == true; var syntax = await document.GetSyntaxTreeAsync(); + var replacingSpanStartPosition = sourceText.Lines.GetLinePosition(typedSpan.Start); + var replacingSpanEndPosition = sourceText.Lines.GetLinePosition(typedSpan.End); + for (int i = 0; i < completions.Items.Length; i++) { var completion = completions.Items[i]; @@ -233,14 +236,30 @@ public async Task Handle(CompletionRequest request) var change = await completionService.GetChangeAsync(document, completion); - // If the span we're using to key the completion off is the same as the replacement - // span, then we don't need to do anything special, just snippitize the text and - // exit if (typedSpan == change.TextChange.Span) { + // If the span we're using to key the completion off is the same as the replacement + // span, then we don't need to do anything special, just snippitize the text and + // exit (insertText, insertTextFormat) = getAdjustedInsertTextWithPosition(change, position, newOffset: 0); break; } + if (change.TextChange.Span.Start > typedSpan.Start) + { + // If the span we're using to key the replacement span within the original typed span + // span, we want to prepend the missing text from the original typed text to here. The + // reason is that some lsp clients, such as vscode, use the range from the text edit as + // the selector for what filter text to use. This can lead to odd scenarios where invoking + // completion and typing `EQ` will bring up the Equals override, but then dismissing and + // reinvoking completion will have a range that just replaces the Q. Vscode will then consider + // that capital Q to be the start of the filter text, and filter out the Equals overload + // leaving the user with no completion and no explanation + Debug.Assert(change.TextChange.Span.End == typedSpan.End); + var prefix = typedText.Substring(0, change.TextChange.Span.Start - typedSpan.Start); + + (insertText, insertTextFormat) = getAdjustedInsertTextWithPosition(change, position, newOffset: 0, prefix); + break; + } int additionalEditEndOffset; (additionalTextEdits, additionalEditEndOffset) = await GetAdditionalTextEdits(change, sourceText, (CSharpParseOptions)syntax!.Options, typedSpan, completion.DisplayText, providerName); @@ -271,7 +290,14 @@ public async Task Handle(CompletionRequest request) completionsBuilder.Add(new CompletionItem { Label = completion.DisplayTextPrefix + completion.DisplayText + completion.DisplayTextSuffix, - InsertText = insertText, + TextEdit = new LinePositionSpanTextChange + { + NewText = insertText, + StartLine = replacingSpanStartPosition.Line, + StartColumn = replacingSpanStartPosition.Character, + EndLine = replacingSpanEndPosition.Line, + EndColumn = replacingSpanEndPosition.Character + }, InsertTextFormat = insertTextFormat, AdditionalTextEdits = additionalTextEdits, // Ensure that unimported items are sorted after things already imported. @@ -359,7 +385,8 @@ static ImmutableArray buildCommitCharacters(CSharpCompletionList completio static (string, InsertTextFormat) getAdjustedInsertTextWithPosition( CompletionChange change, int originalPosition, - int newOffset) + int newOffset, + string? prependText = null) { // We often have to trim part of the given change off the front, but we // still want to turn the resulting change into a snippet and control @@ -375,7 +402,7 @@ static ImmutableArray buildCommitCharacters(CSharpCompletionList completio if (!(change.NewPosition is int newPosition) || newPosition >= (change.TextChange.Span.Start + newText.Length)) { - return (newText.Substring(newOffset), InsertTextFormat.PlainText); + return (prependText + newText.Substring(newOffset), InsertTextFormat.PlainText); } if (newPosition < (originalPosition + newOffset)) @@ -389,7 +416,7 @@ static ImmutableArray buildCommitCharacters(CSharpCompletionList completio // requested start to the new position, and from the new position to the end of the // string. int midpoint = newPosition - change.TextChange.Span.Start; - var beforeText = LspSnippetHelpers.Escape(newText.Substring(newOffset, midpoint - newOffset)); + var beforeText = LspSnippetHelpers.Escape(prependText + newText.Substring(newOffset, midpoint - newOffset)); var afterText = LspSnippetHelpers.Escape(newText.Substring(midpoint)); return (beforeText + "$0" + afterText, InsertTextFormat.Snippet); diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/CompletionFacts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/CompletionFacts.cs index 917fb0dec2..10ce20edcf 100644 --- a/tests/OmniSharp.Roslyn.CSharp.Tests/CompletionFacts.cs +++ b/tests/OmniSharp.Roslyn.CSharp.Tests/CompletionFacts.cs @@ -44,7 +44,7 @@ public Class1() var completions = await FindCompletionsAsync(filename, input, SharedOmniSharpTestHost); Assert.Contains("Foo", completions.Items.Select(c => c.Label)); - Assert.Contains("Foo", completions.Items.Select(c => c.InsertText)); + Assert.Contains("Foo", completions.Items.Select(c => c.TextEdit.NewText)); } [Theory] @@ -63,7 +63,7 @@ public Class1() var completions = await FindCompletionsAsync(filename, input, SharedOmniSharpTestHost); Assert.Contains("foo", completions.Items.Select(c => c.Label)); - Assert.Contains("foo", completions.Items.Select(c => c.InsertText)); + Assert.Contains("foo", completions.Items.Select(c => c.TextEdit.NewText)); } [Theory] @@ -105,7 +105,7 @@ public Class1() }"; var completions = await FindCompletionsAsync(filename, input, SharedOmniSharpTestHost); - Assert.Contains("TryParse", completions.Items.Select(c => c.InsertText)); + Assert.Contains("TryParse", completions.Items.Select(c => c.TextEdit.NewText)); } [Theory] @@ -123,7 +123,7 @@ public Class1() var completions = await FindCompletionsAsync(filename, input, SharedOmniSharpTestHost); Assert.False(completions.IsIncomplete); - Assert.DoesNotContain("Guid", completions.Items.Select(c => c.InsertText)); + Assert.DoesNotContain("Guid", completions.Items.Select(c => c.TextEdit.NewText)); } [Theory] @@ -144,7 +144,7 @@ public Class1() // First completion request should kick off the task to update the completion cache. var completions = await FindCompletionsAsync(filename, input, host); Assert.True(completions.IsIncomplete); - Assert.DoesNotContain("Guid", completions.Items.Select(c => c.InsertText)); + Assert.DoesNotContain("Guid", completions.Items.Select(c => c.TextEdit.NewText)); // Populating the completion cache should take no more than a few ms, don't let it take too // long @@ -159,7 +159,7 @@ await Task.Run(async () => }, cts.Token); Assert.False(completions.IsIncomplete); - Assert.Contains("Guid", completions.Items.Select(c => c.InsertText)); + Assert.Contains("Guid", completions.Items.Select(c => c.TextEdit.NewText)); } [Theory] @@ -179,8 +179,8 @@ public Class1() using var host = GetImportCompletionHost(); var completions = await FindCompletionsWithImportedAsync(filename, input, host); - CompletionItem localCompletion = completions.Items.First(c => c.InsertText == "guid"); - CompletionItem typeCompletion = completions.Items.First(c => c.InsertText == "Guid"); + CompletionItem localCompletion = completions.Items.First(c => c.TextEdit.NewText == "guid"); + CompletionItem typeCompletion = completions.Items.First(c => c.TextEdit.NewText == "Guid"); Assert.True(localCompletion.Data < typeCompletion.Data); Assert.StartsWith("0", localCompletion.SortText); Assert.StartsWith("1", typeCompletion.SortText); @@ -215,7 +215,7 @@ public static void Test(this object o) using var host = GetImportCompletionHost(); var completions = await FindCompletionsWithImportedAsync(filename, input, host); - Assert.Contains("Test", completions.Items.Select(c => c.InsertText)); + Assert.Contains("Test", completions.Items.Select(c => c.TextEdit.NewText)); VerifySortOrders(completions.Items); } @@ -247,7 +247,7 @@ public static void Test(this object o) using var host = GetImportCompletionHost(); var completions = await FindCompletionsWithImportedAsync(filename, input, host); - var resolved = await ResolveCompletionAsync(completions.Items.First(c => c.InsertText == "Test"), host); + var resolved = await ResolveCompletionAsync(completions.Items.First(c => c.TextEdit.NewText == "Test"), host); Assert.Single(resolved.Item.AdditionalTextEdits); var additionalEdit = resolved.Item.AdditionalTextEdits[0]; @@ -288,7 +288,7 @@ public static void Test(this object o) using var host = GetImportCompletionHost(); var completions = await FindCompletionsWithImportedAsync(filename, input, host); - var resolved = await ResolveCompletionAsync(completions.Items.First(c => c.InsertText == "Guid"), host); + var resolved = await ResolveCompletionAsync(completions.Items.First(c => c.TextEdit.NewText == "Guid"), host); Assert.Single(resolved.Item.AdditionalTextEdits); var additionalEdit = resolved.Item.AdditionalTextEdits[0]; @@ -335,7 +335,7 @@ public class C3 using var host = GetImportCompletionHost(); var completions = await FindCompletionsWithImportedAsync(filename, input, host); - var resolved = await ResolveCompletionAsync(completions.Items.First(c => c.InsertText == "C2"), host); + var resolved = await ResolveCompletionAsync(completions.Items.First(c => c.TextEdit.NewText == "C2"), host); Assert.Single(resolved.Item.AdditionalTextEdits); var additionalEdit = resolved.Item.AdditionalTextEdits[0]; @@ -500,7 +500,7 @@ public class Foo {}"; var completions = await FindCompletionsAsync(filename, source, SharedOmniSharpTestHost); Assert.Contains(completions.Items, c => c.Label == "Bar"); - Assert.Contains(completions.Items, c => c.InsertText == "Bar"); + Assert.Contains(completions.Items, c => c.TextEdit.NewText == "Bar"); Assert.All(completions.Items, c => { switch (c.Label) @@ -538,7 +538,7 @@ public MyClass2() var completions = await FindCompletionsAsync(filename, source, SharedOmniSharpTestHost); Assert.Single(completions.Items); Assert.Equal("Foo", completions.Items[0].Label); - Assert.Equal("Foo", completions.Items[0].InsertText); + Assert.Equal("Foo", completions.Items[0].TextEdit.NewText); } [Theory] @@ -564,7 +564,7 @@ public MyClass2() var completions = await FindCompletionsAsync(filename, source, SharedOmniSharpTestHost); var item = completions.Items.First(c => c.Label == "text:"); Assert.NotNull(item); - Assert.Equal("text", item.InsertText); + Assert.Equal("text", item.TextEdit.NewText); Assert.All(completions.Items, c => { switch (c.Label) @@ -624,7 +624,7 @@ class FooChild : Foo "Test(string text, string moreText)\n {\n base.Test(text, moreText);$0\n \\}", "ToString()\n {\n return base.ToString();$0\n \\}" }, - completions.Items.Select(c => c.InsertText)); + completions.Items.Select(c => c.TextEdit.NewText)); Assert.Equal(new[] { "public override bool", "public override int", @@ -678,7 +678,7 @@ class CN3 : IN2 "GetN1()\n {\n throw new System.NotImplementedException();$0\n \\}", "ToString()\n {\n return base.ToString();$0\n \\}" }, - completions.Items.Select(c => c.InsertText)); + completions.Items.Select(c => c.TextEdit.NewText)); Assert.Equal(new[] { "public override bool", "public override int", @@ -717,7 +717,7 @@ public override $$ "int GetHashCode()\n {\n return base.GetHashCode();$0\n \\}", "string ToString()\n {\n return base.ToString();$0\n \\}" }, - completions.Items.Select(c => c.InsertText)); + completions.Items.Select(c => c.TextEdit.NewText)); Assert.All(completions.Items.Select(c => c.AdditionalTextEdits), a => Assert.Null(a)); Assert.All(completions.Items, c => Assert.Equal(InsertTextFormat.Snippet, c.InsertTextFormat)); @@ -739,7 +739,7 @@ public override bool $$ completions.Items.Select(c => c.Label)); Assert.Equal(new[] { "Equals(object obj)\n {\n return base.Equals(obj);$0\n \\}" }, - completions.Items.Select(c => c.InsertText)); + completions.Items.Select(c => c.TextEdit.NewText)); Assert.All(completions.Items.Select(c => c.AdditionalTextEdits), a => Assert.Null(a)); Assert.All(completions.Items, c => Assert.Equal(InsertTextFormat.Snippet, c.InsertTextFormat)); @@ -770,7 +770,7 @@ class Derived : Base "Test()\n {\n throw new System.NotImplementedException();$0\n \\}", "ToString()\n {\n return base.ToString();$0\n \\}" }, - completions.Items.Select(c => c.InsertText)); + completions.Items.Select(c => c.TextEdit.NewText)); Assert.Equal(new[] { "public override bool", "public override int", @@ -817,7 +817,7 @@ public class Derived : Base Assert.Equal(0, item.AdditionalTextEdits[0].StartColumn); Assert.Equal(3, item.AdditionalTextEdits[0].EndLine); Assert.Equal(12, item.AdditionalTextEdits[0].EndColumn); - Assert.Equal("GetAction(Action a)\n {\n return base.GetAction(a);$0\n \\}", item.InsertText); + Assert.Equal("GetAction(Action a)\n {\n return base.GetAction(a);$0\n \\}", item.TextEdit.NewText); } [Fact] @@ -845,7 +845,7 @@ public class Derived : Base Assert.Equal(4, item.AdditionalTextEdits[0].StartColumn); Assert.Equal(9, item.AdditionalTextEdits[0].EndLine); Assert.Equal(12, item.AdditionalTextEdits[0].EndColumn); - Assert.Equal("M1(object param)\n {\n return base.M1(param);$0\n \\}", item.InsertText); + Assert.Equal("M1(object param)\n {\n return base.M1(param);$0\n \\}", item.TextEdit.NewText); } [Theory] @@ -869,7 +869,7 @@ partial class C completions.Items.Select(c => c.Label)); Assert.Equal(new[] { "void M1(string param)\n {\n throw new System.NotImplementedException();$0\n \\}" }, - completions.Items.Select(c => c.InsertText)); + completions.Items.Select(c => c.TextEdit.NewText)); Assert.All(completions.Items.Select(c => c.AdditionalTextEdits), a => Assert.Null(a)); Assert.All(completions.Items, c => Assert.Equal(InsertTextFormat.Snippet, c.InsertTextFormat)); @@ -901,7 +901,7 @@ public partial class C Assert.Equal(0, item.AdditionalTextEdits[0].StartColumn); Assert.Equal(3, item.AdditionalTextEdits[0].EndLine); Assert.Equal(11, item.AdditionalTextEdits[0].EndColumn); - Assert.Equal("M(Action a)\n {\n throw new NotImplementedException();$0\n \\}", item.InsertText); + Assert.Equal("M(Action a)\n {\n throw new NotImplementedException();$0\n \\}", item.TextEdit.NewText); } [Fact] @@ -923,7 +923,7 @@ public partial class C var item = completions.Items.Single(c => c.Label.StartsWith("M1")); Assert.Equal("M1(object param)", item.Label); Assert.Null(item.AdditionalTextEdits); - Assert.Equal("void M1(object param)\n {\n throw new System.NotImplementedException();$0\n \\}", item.InsertText); + Assert.Equal("void M1(object param)\n {\n throw new System.NotImplementedException();$0\n \\}", item.TextEdit.NewText); } [Theory] @@ -945,7 +945,7 @@ override Ge$$ "GetHashCode()\n {\n return base.GetHashCode();$0\n \\}", "ToString()\n {\n return base.ToString();$0\n \\}" }, - completions.Items.Select(c => c.InsertText)); + completions.Items.Select(c => c.TextEdit.NewText)); Assert.Equal(new[] { "public override bool", "public override int", @@ -1030,8 +1030,8 @@ public class MyClass1 { "see cref=\"$0\"/>", "seealso cref=\"$0\"/>" }, - completions.Items.Select(c => c.InsertText)); - Assert.All(completions.Items, c => Assert.Equal(c.InsertText.Contains("$0"), c.InsertTextFormat == InsertTextFormat.Snippet)); + completions.Items.Select(c => c.TextEdit.NewText)); + Assert.All(completions.Items, c => Assert.Equal(c.TextEdit.NewText.Contains("$0"), c.InsertTextFormat == InsertTextFormat.Snippet)); } [Fact] @@ -1307,7 +1307,7 @@ public async Task InternalsVisibleToCompletion() var completions = await FindCompletionsAsync("dummy.cs", input, SharedOmniSharpTestHost); Assert.Single(completions.Items); Assert.Equal("AssemblyNameVal", completions.Items[0].Label); - Assert.Equal("AssemblyNameVal", completions.Items[0].InsertText); + Assert.Equal("AssemblyNameVal", completions.Items[0].TextEdit.NewText); } [Fact] @@ -1333,7 +1333,73 @@ public async Task InternalsVisibleToCompletionSkipsMiscProject() var completions = await FindCompletionsAsync("dummy.cs", input, SharedOmniSharpTestHost); Assert.Single(completions.Items); Assert.Equal("AssemblyNameVal", completions.Items[0].Label); - Assert.Equal("AssemblyNameVal", completions.Items[0].InsertText); + Assert.Equal("AssemblyNameVal", completions.Items[0].TextEdit.NewText); + } + + [Theory] + [InlineData("dummy.cs")] + [InlineData("dummy.csx")] + public async Task PrefixHeaderIsFullyCorrect(string filename) + { + const string input = +@"public class Base +{ + protected virtual void OnEnable() {} +} +public class Derived : Base +{ + protected override void On$$ +}"; + + var completions = await FindCompletionsAsync(filename, input, SharedOmniSharpTestHost); + var onEnable = completions.Items.Single(c => c.TextEdit.NewText.Contains("OnEnable")); + Assert.Equal(onEnable.TextEdit.StartLine, onEnable.TextEdit.EndLine); + Assert.Equal(2, onEnable.TextEdit.EndColumn - onEnable.TextEdit.StartColumn); + Assert.Equal("OnEnable()\n {\n base.OnEnable();$0\n \\}", onEnable.TextEdit.NewText); + } + + [Theory] + [InlineData("dummy.cs")] + [InlineData("dummy.csx")] + public async Task PrefixHeaderIsPartiallyCorrect_1(string filename) + { + const string input = +@"public class Base +{ + protected virtual void OnEnable() {} +} +public class Derived : Base +{ + protected override void ON$$ +}"; + + var completions = await FindCompletionsAsync(filename, input, SharedOmniSharpTestHost); + var onEnable = completions.Items.Single(c => c.TextEdit.NewText.Contains("OnEnable")); + Assert.Equal(onEnable.TextEdit.StartLine, onEnable.TextEdit.EndLine); + Assert.Equal(2, onEnable.TextEdit.EndColumn - onEnable.TextEdit.StartColumn); + Assert.Equal("OnEnable()\n {\n base.OnEnable();$0\n \\}", onEnable.TextEdit.NewText); + } + + [Theory] + [InlineData("dummy.cs")] + [InlineData("dummy.csx")] + public async Task PrefixHeaderIsPartiallyCorrect_2(string filename) + { + const string input = +@"public class Base +{ + protected virtual void OnEnable() {} +} +public class Derived : Base +{ + protected override void on$$ +}"; + + var completions = await FindCompletionsAsync(filename, input, SharedOmniSharpTestHost); + var onEnable = completions.Items.Single(c => c.TextEdit.NewText.Contains("OnEnable")); + Assert.Equal(onEnable.TextEdit.StartLine, onEnable.TextEdit.EndLine); + Assert.Equal(2, onEnable.TextEdit.EndColumn - onEnable.TextEdit.StartColumn); + Assert.Equal("OnEnable()\n {\n base.OnEnable();$0\n \\}", onEnable.TextEdit.NewText); } private CompletionService GetCompletionService(OmniSharpTestHost host)