diff --git a/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionItem.cs b/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionItem.cs
index 141c3a2575..aceb370b5a 100644
--- a/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionItem.cs
+++ b/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionItem.cs
@@ -1,6 +1,6 @@
#nullable enable
-using System.Collections.Immutable;
+using System.Collections.Generic;
namespace OmniSharp.Models.v1.Completion
{
@@ -23,7 +23,7 @@ public class CompletionItem
///
/// Tags for this completion item
///
- public ImmutableArray? Tags { get; set; }
+ public IReadOnlyList? Tags { get; set; }
///
/// A human-readable string with additional information
@@ -69,7 +69,7 @@ public class CompletionItem
/// An optional set of characters that when pressed while this completion is active will accept it first and
/// then type that character.
///
- public ImmutableArray? CommitCharacters { get; set; }
+ public IReadOnlyList? CommitCharacters { get; set; }
///
/// An optional array of additional text edits that are applied when
@@ -80,7 +80,7 @@ public class CompletionItem
/// (for example adding an import statement at the top of the file if the completion item will
/// insert an unqualified type).
///
- public ImmutableArray? AdditionalTextEdits { get; set; }
+ public IReadOnlyList? AdditionalTextEdits { get; set; }
///
/// Index in the completions list that this completion occurred.
diff --git a/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionResponse.cs b/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionResponse.cs
index e624f14158..057bf44f50 100644
--- a/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionResponse.cs
+++ b/src/OmniSharp.Abstractions/Models/v1/Completion/CompletionResponse.cs
@@ -1,6 +1,6 @@
#nullable enable
-using System.Collections.Immutable;
+using System.Collections.Generic;
namespace OmniSharp.Models.v1.Completion
{
@@ -14,6 +14,6 @@ public class CompletionResponse
///
/// The completion items.
///
- public ImmutableArray Items { get; set; }
+ public IReadOnlyList Items { get; set; } = null!;
}
}
diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionService.cs
index 52554abfc7..e10f871fee 100644
--- a/src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionService.cs
+++ b/src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionService.cs
@@ -7,9 +7,10 @@
using System.Linq;
using System.Text;
using System.Threading.Tasks;
-using System.Xml.Resolvers;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Completion;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Tags;
using Microsoft.CodeAnalysis.Text;
using Microsoft.Extensions.Logging;
@@ -159,17 +160,19 @@ public async Task Handle(CompletionRequest request)
// the completion as done.
bool seenUnimportedCompletions = false;
bool expectingImportedItems = expandedItemsAvailable && _workspace.Options.GetOption(CompletionItemExtensions.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp) == true;
+ var syntax = await document.GetSyntaxTreeAsync();
for (int i = 0; i < completions.Items.Length; i++)
{
var completion = completions.Items[i];
var insertTextFormat = InsertTextFormat.PlainText;
- ImmutableArray? additionalTextEdits = null;
+ IReadOnlyList? additionalTextEdits = null;
char sortTextPrepend = '0';
if (!completion.TryGetInsertionText(out string insertText))
{
- switch (completion.GetProviderName())
+ string providerName = completion.GetProviderName();
+ switch (providerName)
{
case CompletionItemExtensions.InternalsVisibleToCompletionProvider:
// The IVT completer doesn't add extra things before the completion
@@ -240,7 +243,7 @@ public async Task Handle(CompletionRequest request)
}
int additionalEditEndOffset;
- (additionalTextEdits, additionalEditEndOffset) = GetAdditionalTextEdits(change, sourceText, typedSpan, completion.DisplayText, isImportCompletion: false);
+ (additionalTextEdits, additionalEditEndOffset) = await GetAdditionalTextEdits(change, sourceText, (CSharpParseOptions)syntax!.Options, typedSpan, completion.DisplayText, providerName);
// Now that we have the additional edit, adjust the rest of the new text
(insertText, insertTextFormat) = getAdjustedInsertTextWithPosition(change, position, additionalEditEndOffset);
@@ -435,14 +438,16 @@ public async Task Handle(CompletionResolveRequest req
request.Item.Documentation = textBuilder.ToString();
- switch (lastCompletionItem.GetProviderName())
+ string providerName = lastCompletionItem.GetProviderName();
+ switch (providerName)
{
case CompletionItemExtensions.ExtensionMethodImportCompletionProvider:
case CompletionItemExtensions.TypeImportCompletionProvider:
+ var syntax = await document.GetSyntaxTreeAsync();
var sourceText = await document.GetTextAsync();
var typedSpan = completionService.GetDefaultCompletionListSpan(sourceText, position);
var change = await completionService.GetChangeAsync(document, lastCompletionItem, typedSpan);
- (request.Item.AdditionalTextEdits, _) = GetAdditionalTextEdits(change, sourceText, typedSpan, lastCompletionItem.DisplayText, isImportCompletion: true);
+ (request.Item.AdditionalTextEdits, _) = await GetAdditionalTextEdits(change, sourceText, (CSharpParseOptions)syntax!.Options, typedSpan, lastCompletionItem.DisplayText, providerName);
break;
}
@@ -452,19 +457,20 @@ public async Task Handle(CompletionResolveRequest req
};
}
- private (ImmutableArray edits, int endOffset) GetAdditionalTextEdits(CompletionChange change, SourceText sourceText, TextSpan typedSpan, string completionDisplayText, bool isImportCompletion)
+ private async ValueTask<(IReadOnlyList edits, int endOffset)> GetAdditionalTextEdits(
+ CompletionChange change,
+ SourceText sourceText,
+ CSharpParseOptions parseOptions,
+ TextSpan typedSpan,
+ string completionDisplayText,
+ string providerName)
{
// We know the span starts before the text we're keying off of. So, break that
// out into a separate edit. We need to cut out the space before the current word,
// as the additional edit is not allowed to overlap with the insertion point.
var additionalEditStartPosition = sourceText.Lines.GetLinePosition(change.TextChange.Span.Start);
var additionalEditEndPosition = sourceText.Lines.GetLinePosition(typedSpan.Start - 1);
- int additionalEditEndOffset = isImportCompletion
- // Import completion will put the displaytext at the end of the line, override completion will
- // put it at the front.
- ? change.TextChange.NewText!.LastIndexOf(completionDisplayText)
- : change.TextChange.NewText!.IndexOf(completionDisplayText);
-
+ int additionalEditEndOffset = await getAdditionalTextEditEndOffset(change, sourceText, parseOptions, typedSpan, completionDisplayText, providerName);
if (additionalEditEndOffset < 1)
{
// The first index of this was either 0 and the edit span was wrong,
@@ -484,6 +490,44 @@ public async Task Handle(CompletionResolveRequest req
EndLine = additionalEditEndPosition.Line,
EndColumn = additionalEditEndPosition.Character,
}), additionalEditEndOffset);
+
+ static async ValueTask getAdditionalTextEditEndOffset(CompletionChange change, SourceText sourceText, CSharpParseOptions parseOptions, TextSpan typedSpan, string completionDisplayText, string providerName)
+ {
+ // For many simple cases, we can just find the first or last index of the completionDisplayText and that's good
+ // enough
+ int endOffset = (providerName == CompletionItemExtensions.ExtensionMethodImportCompletionProvider ||
+ providerName == CompletionItemExtensions.TypeImportCompletionProvider)
+ // Import completion will put the displaytext at the end of the line, override completion will
+ // put it at the front.
+ ? change.TextChange.NewText!.LastIndexOf(completionDisplayText)
+ : change.TextChange.NewText!.IndexOf(completionDisplayText);
+
+ if (endOffset > -1)
+ {
+ return endOffset;
+ }
+
+ // The DisplayText wasn't in the final string. This can happen in a few cases:
+ // * The override or partial method completion is involving types that need
+ // to have a using added in the final version, and won't be fully qualified
+ // as they were in the DisplayText
+ // * Nullable context differences, such as if the thing you're overriding is
+ // annotated but the final context being generated into does not have
+ // annotations enabled.
+ // For these cases, we currently should only be seeing override or partial
+ // completions, as import completions don't have nullable annotations or
+ // fully-qualified types in their DisplayTexts. If that ever changes, we'll have
+ // to adjust the API here.
+ //
+ // In order to find the correct location here, we parse the change. The location
+ // of the name of the last method is the correct location in the string.
+ Debug.Assert(providerName == CompletionItemExtensions.OverrideCompletionProvider ||
+ providerName == CompletionItemExtensions.PartialMethodCompletionProvider);
+
+ var parsedTree = CSharpSyntaxTree.ParseText(change.TextChange.NewText, parseOptions);
+ var lastMethodDecl = (await parsedTree.GetRootAsync()).DescendantNodes().OfType().Last();
+ return lastMethodDecl.Identifier.SpanStart;
+ }
}
}
}
diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/CompletionFacts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/CompletionFacts.cs
index c9c050e2b9..917fb0dec2 100644
--- a/tests/OmniSharp.Roslyn.CSharp.Tests/CompletionFacts.cs
+++ b/tests/OmniSharp.Roslyn.CSharp.Tests/CompletionFacts.cs
@@ -249,8 +249,8 @@ public static void Test(this object o)
var completions = await FindCompletionsWithImportedAsync(filename, input, host);
var resolved = await ResolveCompletionAsync(completions.Items.First(c => c.InsertText == "Test"), host);
- Assert.Single(resolved.Item.AdditionalTextEdits.Value);
- var additionalEdit = resolved.Item.AdditionalTextEdits.Value[0];
+ Assert.Single(resolved.Item.AdditionalTextEdits);
+ var additionalEdit = resolved.Item.AdditionalTextEdits[0];
Assert.Equal(NormalizeNewlines("using N2;\n\nnamespace N1\r\n{\r\n public class C1\r\n {\r\n public void M(object o)\r\n {\r\n o"),
additionalEdit.NewText);
Assert.Equal(0, additionalEdit.StartLine);
@@ -290,8 +290,8 @@ public static void Test(this object o)
var completions = await FindCompletionsWithImportedAsync(filename, input, host);
var resolved = await ResolveCompletionAsync(completions.Items.First(c => c.InsertText == "Guid"), host);
- Assert.Single(resolved.Item.AdditionalTextEdits.Value);
- var additionalEdit = resolved.Item.AdditionalTextEdits.Value[0];
+ Assert.Single(resolved.Item.AdditionalTextEdits);
+ var additionalEdit = resolved.Item.AdditionalTextEdits[0];
Assert.Equal(NormalizeNewlines("using System;\n\nnamespace N1\r\n{\r\n public class C1\r\n {\r\n public void M(object o)\r\n {\r\n /*Guid*"),
additionalEdit.NewText);
Assert.Equal(0, additionalEdit.StartLine);
@@ -337,8 +337,8 @@ public class C3
var completions = await FindCompletionsWithImportedAsync(filename, input, host);
var resolved = await ResolveCompletionAsync(completions.Items.First(c => c.InsertText == "C2"), host);
- Assert.Single(resolved.Item.AdditionalTextEdits.Value);
- var additionalEdit = resolved.Item.AdditionalTextEdits.Value[0];
+ Assert.Single(resolved.Item.AdditionalTextEdits);
+ var additionalEdit = resolved.Item.AdditionalTextEdits[0];
Assert.Equal(NormalizeNewlines("N2;\nusing N3;\r\nnamespace N1\r\n{\r\n public class C1\r\n {\r\n public void M(object o)\r\n {\r\n "),
additionalEdit.NewText);
Assert.Equal(1, additionalEdit.StartLine);
@@ -631,9 +631,9 @@ class FooChild : Foo
"public override void",
"public override void",
"public override string"},
- completions.Items.Select(c => c.AdditionalTextEdits.Value.Single().NewText));
+ completions.Items.Select(c => c.AdditionalTextEdits.Single().NewText));
- Assert.All(completions.Items.Select(c => c.AdditionalTextEdits.Value.Single()),
+ Assert.All(completions.Items.Select(c => c.AdditionalTextEdits.Single()),
r =>
{
Assert.Equal(9, r.StartLine);
@@ -684,9 +684,9 @@ class CN3 : IN2
"public override int",
"protected override N1.CN1",
"public override string"},
- completions.Items.Select(c => c.AdditionalTextEdits.Value.Single().NewText));
+ completions.Items.Select(c => c.AdditionalTextEdits.Single().NewText));
- Assert.All(completions.Items.Select(c => c.AdditionalTextEdits.Value.Single()),
+ Assert.All(completions.Items.Select(c => c.AdditionalTextEdits.Single()),
r =>
{
Assert.Equal(15, r.StartLine);
@@ -776,9 +776,9 @@ class Derived : Base
"public override int",
"protected override Test",
"public override string"},
- completions.Items.Select(c => c.AdditionalTextEdits.Value.Single().NewText));
+ completions.Items.Select(c => c.AdditionalTextEdits.Single().NewText));
- Assert.All(completions.Items.Select(c => c.AdditionalTextEdits.Value.Single()),
+ Assert.All(completions.Items.Select(c => c.AdditionalTextEdits.Single()),
r =>
{
Assert.Equal(8, r.StartLine);
@@ -790,6 +790,64 @@ class Derived : Base
Assert.All(completions.Items, c => Assert.Equal(InsertTextFormat.Snippet, c.InsertTextFormat));
}
+ [Fact]
+ public async Task OverrideCompletion_TypesNeedImport()
+ {
+ const string baseText = @"
+using System;
+public class Base
+{
+ public virtual Action GetAction(Action a) => null;
+}
+";
+
+ const string derivedText = @"
+public class Derived : Base
+{
+ override $$
+}";
+
+ var completions = await FindCompletionsAsync("derived.cs", derivedText, SharedOmniSharpTestHost, additionalFiles: new[] { new TestFile("base.cs", baseText) });
+ var item = completions.Items.Single(c => c.Label.StartsWith("GetAction"));
+ Assert.Equal("GetAction(System.Action a)", item.Label);
+
+ Assert.Single(item.AdditionalTextEdits);
+ Assert.Equal(NormalizeNewlines("using System;\n\npublic class Derived : Base\r\n{\r\n public override Action"), item.AdditionalTextEdits[0].NewText);
+ Assert.Equal(1, item.AdditionalTextEdits[0].StartLine);
+ 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);
+ }
+
+ [Fact]
+ public async Task OverrideCompletion_FromNullableToNonNullableContext()
+ {
+ const string text = @"
+#nullable enable
+public class Base
+{
+ public virtual object? M1(object? param) => throw null;
+}
+#nullable disable
+public class Derived : Base
+{
+ override $$
+}";
+
+ var completions = await FindCompletionsAsync("derived.cs", text, SharedOmniSharpTestHost);
+ var item = completions.Items.Single(c => c.Label.StartsWith("M1"));
+ Assert.Equal("M1(object? param)", item.Label);
+
+ Assert.Single(item.AdditionalTextEdits);
+ Assert.Equal("public override object", item.AdditionalTextEdits[0].NewText);
+ Assert.Equal(9, item.AdditionalTextEdits[0].StartLine);
+ 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);
+ }
+
[Theory]
[InlineData("dummy.cs")]
[InlineData("dummy.csx")]
@@ -817,6 +875,57 @@ partial class C
Assert.All(completions.Items, c => Assert.Equal(InsertTextFormat.Snippet, c.InsertTextFormat));
}
+ [Fact]
+ public async Task PartialCompletion_TypesNeedImport()
+ {
+ const string file1 = @"
+using System;
+public partial class C
+{
+ partial void M(Action a);
+}
+";
+
+ const string file2 = @"
+public partial class C
+{
+ partial $$
+}";
+
+ var completions = await FindCompletionsAsync("derived.cs", file2, SharedOmniSharpTestHost, additionalFiles: new[] { new TestFile("base.cs", file1) });
+ var item = completions.Items.Single(c => c.Label.StartsWith("M"));
+
+ Assert.Single(item.AdditionalTextEdits);
+ Assert.Equal(NormalizeNewlines("using System;\n\npublic partial class C\r\n{\r\n partial void"), item.AdditionalTextEdits[0].NewText);
+ Assert.Equal(1, item.AdditionalTextEdits[0].StartLine);
+ 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);
+ }
+
+ [Fact]
+ public async Task PartialCompletion_FromNullableToNonNullableContext()
+ {
+ const string text = @"
+#nullable enable
+public partial class C
+{
+ partial void M1(object? param);
+}
+#nullable disable
+public partial class C
+{
+ partial $$
+}";
+
+ var completions = await FindCompletionsAsync("derived.cs", text, SharedOmniSharpTestHost);
+ 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);
+ }
+
[Theory]
[InlineData("dummy.cs")]
[InlineData("dummy.csx")]
@@ -841,9 +950,9 @@ override Ge$$
Assert.Equal(new[] { "public override bool",
"public override int",
"public override string"},
- completions.Items.Select(c => c.AdditionalTextEdits.Value.Single().NewText));
+ completions.Items.Select(c => c.AdditionalTextEdits.Single().NewText));
- Assert.All(completions.Items.Select(c => c.AdditionalTextEdits.Value.Single()),
+ Assert.All(completions.Items.Select(c => c.AdditionalTextEdits.Single()),
r =>
{
Assert.Equal(3, r.StartLine);
@@ -1230,11 +1339,17 @@ public async Task InternalsVisibleToCompletionSkipsMiscProject()
private CompletionService GetCompletionService(OmniSharpTestHost host)
=> host.GetRequestHandler(EndpointName);
- protected async Task FindCompletionsAsync(string filename, string source, OmniSharpTestHost testHost, char? triggerChar = null)
+ protected async Task FindCompletionsAsync(string filename, string source, OmniSharpTestHost testHost, char? triggerChar = null, TestFile[] additionalFiles = null)
{
var testFile = new TestFile(filename, source);
- testHost.AddFilesToWorkspace(testFile);
+ var files = new[] { testFile };
+ if (additionalFiles is object)
+ {
+ files = files.Concat(additionalFiles).ToArray();
+ }
+
+ testHost.AddFilesToWorkspace(files);
var point = testFile.Content.GetPointFromPosition();
var request = new CompletionRequest
@@ -1289,7 +1404,7 @@ private OmniSharpTestHost GetImportCompletionHost()
private static string NormalizeNewlines(string str)
=> str.Replace("\r\n", Environment.NewLine);
- private static void VerifySortOrders(ImmutableArray items)
+ private static void VerifySortOrders(IReadOnlyList items)
{
Assert.All(items, c =>
{
@@ -1300,6 +1415,6 @@ private static void VerifySortOrders(ImmutableArray items)
internal static class CompletionResponseExtensions
{
- public static bool IsSuggestionMode(this CompletionItem item) => (item.CommitCharacters?.IsDefaultOrEmpty ?? true) || !item.CommitCharacters.Contains(' ');
+ public static bool IsSuggestionMode(this CompletionItem item) => !item.CommitCharacters?.Contains(' ') ?? true;
}
}