Skip to content

Commit

Permalink
Fix import completion in LSP workspace (#69691)
Browse files Browse the repository at this point in the history
* Fix import completion in LSP workspace

1. Provide unimported extension methods
2. Fully qualify unimported type in using statment
  • Loading branch information
genlu authored Aug 24, 2023
1 parent a6fc267 commit 72efaa9
Show file tree
Hide file tree
Showing 9 changed files with 174 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Completion;
using Microsoft.CodeAnalysis.Completion.Providers;
using Microsoft.CodeAnalysis.Completion.Providers.Snippets;
using Microsoft.CodeAnalysis.Editor.Shared.Extensions;
using Microsoft.CodeAnalysis.Editor.Shared.Utilities;
Expand Down Expand Up @@ -212,6 +213,10 @@ private AsyncCompletionData.CommitResult Commit(
if (roslynItem.Flags.IsCached())
roslynItem.Span = completionListSpan;

// Adding import is not allowed in debugger view
if (_textView is IDebuggerTextView)
roslynItem = ImportCompletionItem.MarkItemToAlwaysFullyQualify(roslynItem);

change = completionService.GetChangeAsync(document, roslynItem, commitCharacter, cancellationToken).WaitAndGetResult(cancellationToken);
}
catch (OperationCanceledException e) when (e.CancellationToken != cancellationToken && FatalError.ReportAndCatch(e))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,9 @@ public async Task<VSCompletionContext> GetExpandedCompletionContextAsync(
options = options with
{
FilterOutOfScopeLocals = false,
ShowXmlDocCommentCompletion = false
ShowXmlDocCommentCompletion = false,
// Adding import is not allowed in debugger view
CanAddImportStatement = false,
};
}

Expand Down
6 changes: 6 additions & 0 deletions src/Features/Core/Portable/Completion/CompletionOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@ internal sealed record class CompletionOptions
/// </summary>
public bool UpdateImportCompletionCacheInBackground { get; init; } = false;

/// <summary>
/// Whether completion can add import statement as part of committed change.
/// For example, adding import is not allowed in debugger view.
/// </summary>
public bool CanAddImportStatement { get; init; } = true;

public bool FilterOutOfScopeLocals { get; init; } = true;
public bool ShowXmlDocCommentCompletion { get; init; } = true;
public bool? ShowNewSnippetExperienceUserOption { get; init; } = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@ internal abstract class AbstractExtensionMethodImportCompletionProvider : Abstra
{
protected abstract string GenericSuffix { get; }

// Don't provide unimported extension methods if adding import is not supported,
// since we are current incapable of making a change using its fully qualify form.
protected override bool ShouldProvideCompletion(CompletionContext completionContext, SyntaxContext syntaxContext)
=> syntaxContext.IsRightOfNameSeparator && IsAddingImportsSupported(completionContext.Document);
=> syntaxContext.IsRightOfNameSeparator && IsAddingImportsSupported(completionContext.Document, completionContext.CompletionOptions);

protected override void LogCommit()
=> CompletionProvidersLogger.LogCommitOfExtensionMethodImportCompletionItem();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,10 +202,10 @@ public override async Task<CompletionChange> GetChangeAsync(

async Task<bool> ShouldCompleteWithFullyQualifyTypeName()
{
if (ImportCompletionItem.ShouldAlwaysAddMissingImport(completionItem))
return false;
if (ImportCompletionItem.ShouldAlwaysFullyQualify(completionItem))
return true;

if (!IsAddingImportsSupported(document))
if (!IsAddingImportsSupported(document, completionOptions: null))
return true;

// We might need to qualify unimported types to use them in an import directive, because they only affect members of the containing
Expand Down Expand Up @@ -242,24 +242,11 @@ private async Task<bool> IsInImportsDirectiveAsync(Document document, int positi
&& !IsFinalSemicolonOfUsingOrExtern(node, leftToken);
}

protected static bool IsAddingImportsSupported(Document document)
protected static bool IsAddingImportsSupported(Document document, CompletionOptions? completionOptions)
{
var solution = document.Project.Solution;

// Certain types of workspace don't support document change, e.g. DebuggerIntelliSenseWorkspace
if (!solution.CanApplyChange(ApplyChangesKind.ChangeDocument))
{
return false;
}

// Certain documents, e.g. Razor document, don't support adding imports
var documentSupportsFeatureService = solution.Services.GetRequiredService<IDocumentSupportsFeatureService>();
if (!documentSupportsFeatureService.SupportsRefactorings(document))
{
return false;
}

return true;
return completionOptions?.CanAddImportStatement != false &&
document.Project.Solution.Services.GetRequiredService<IDocumentSupportsFeatureService>().SupportsRefactorings(document);
}

private static SyntaxNode CreateImport(Document document, string namespaceName)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ internal static class ImportCompletionItem
private const string MethodKey = nameof(MethodKey);
private const string ReceiverKey = nameof(ReceiverKey);
private const string OverloadCountKey = nameof(OverloadCountKey);
private const string AlwaysAddMissingImportKey = nameof(AlwaysAddMissingImportKey);
private const string AlwaysFullyQualifyKey = nameof(AlwaysFullyQualifyKey);

public static CompletionItem Create(
string name,
Expand Down Expand Up @@ -211,8 +211,8 @@ private static (ISymbol? symbol, int overloadCount) GetSymbolAndOverloadCount(Co
return (compilation.GetTypeByMetadataName(fullyQualifiedName), 0);
}

public static CompletionItem MarkItemToAlwaysAddMissingImport(CompletionItem item) => item.WithProperties(item.Properties.Add(AlwaysAddMissingImportKey, AlwaysAddMissingImportKey));
public static CompletionItem MarkItemToAlwaysFullyQualify(CompletionItem item) => item.WithProperties(item.Properties.Add(AlwaysFullyQualifyKey, AlwaysFullyQualifyKey));

public static bool ShouldAlwaysAddMissingImport(CompletionItem item) => item.Properties.ContainsKey(AlwaysAddMissingImportKey);
public static bool ShouldAlwaysFullyQualify(CompletionItem item) => item.Properties.ContainsKey(AlwaysFullyQualifyKey);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,6 @@ protected static async Task GetChangeAndPopulateSimpleTextEditAsync(
CancellationToken cancellationToken)
{
Debug.Assert(selectedItem.Flags.IsExpanded());
selectedItem = ImportCompletionItem.MarkItemToAlwaysAddMissingImport(selectedItem);
var completionChange = await completionService.GetChangeAsync(document, selectedItem, cancellationToken: cancellationToken).ConfigureAwait(false);

var sourceText = await document.GetTextAsync(cancellationToken).ConfigureAwait(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,12 @@ public int M()
}

[Theory, CombinatorialData]
public async Task TestImportCompletion(bool mutatingLspWorkspace)
[WorkItem("https://github.com/dotnet/roslyn/issues/68791")]
public async Task TestImportCompletionForType(bool mutatingLspWorkspace, bool isInUsingStatement)
{
var markup =
@"
var markup = isInUsingStatement
? @"global using static Task{|caret:|}"
: @"
class A
{
void M()
Expand Down Expand Up @@ -146,7 +148,10 @@ void M()
Assert.Equal("~Task System.Threading.Tasks", resolvedItem.SortText);
Assert.Equal(CompletionItemKind.Class, resolvedItem.Kind);

var expectedAdditionalEdit = new TextEdit() { NewText = "using System.Threading.Tasks;\r\n\r\n", Range = new() { Start = new(1, 0), End = new(1, 0) } };
TextEdit expectedAdditionalEdit = isInUsingStatement
? new() { NewText = "System.Threading.Tasks.Task", Range = new() { Start = new(0, 20), End = new(0, 24) } }
: new() { NewText = "using System.Threading.Tasks;\r\n\r\n", Range = new() { Start = new(1, 0), End = new(1, 0) } };

AssertJsonEquals(new[] { expectedAdditionalEdit }, resolvedItem.AdditionalTextEdits);

Assert.Null(resolvedItem.LabelDetails.Detail);
Expand All @@ -164,6 +169,88 @@ void M()
AssertJsonEquals(resolvedItem.Documentation, expectedDocumentation);
}

[Theory, CombinatorialData]
[WorkItem("https://github.com/dotnet/roslyn/issues/69576")]
public async Task TestImportCompletionForExtensionMethod(bool mutatingLspWorkspace)
{
var markup =
@"
namespace NS2
{
public static class ExtensionClass
{
public static bool ExtensionMethod(this object o) => true;
}
}
namespace NS1
{
class Program
{
void M(object o)
{
o.{|caret:|}
}
}
}";
await using var testLspServer = await CreateTestLspServerAsync(markup, mutatingLspWorkspace, DefaultClientCapabilities);
var completionParams = CreateCompletionParams(
testLspServer.GetLocations("caret").Single(),
invokeKind: LSP.VSInternalCompletionInvokeKind.Explicit,
triggerCharacter: "\0",
triggerKind: LSP.CompletionTriggerKind.Invoked);

// Make sure the import completion option is on.
testLspServer.TestWorkspace.GlobalOptions.SetGlobalOption(CompletionOptionsStorage.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp, true);
testLspServer.TestWorkspace.GlobalOptions.SetGlobalOption(CompletionOptionsStorage.ForceExpandedCompletionIndexCreation, true);

var document = testLspServer.GetCurrentSolution().Projects.First().Documents.First();

var completionResult = await testLspServer.ExecuteRequestAsync<LSP.CompletionParams, LSP.CompletionList>(LSP.Methods.TextDocumentCompletionName, completionParams, CancellationToken.None).ConfigureAwait(false);
Assert.NotNull(completionResult.ItemDefaults.EditRange);
Assert.NotNull(completionResult.ItemDefaults.Data);
Assert.NotNull(completionResult.ItemDefaults.CommitCharacters);

var actualItem = completionResult.Items.First(i => i.Label == "ExtensionMethod");
Assert.Equal("NS2", actualItem.LabelDetails.Description);
Assert.Equal("~ExtensionMethod NS2", actualItem.SortText);
Assert.Equal(CompletionItemKind.Method, actualItem.Kind);
Assert.Null(actualItem.LabelDetails.Detail);
Assert.Null(actualItem.FilterText);
Assert.Null(actualItem.TextEdit);
Assert.Null(actualItem.TextEditText);
Assert.Null(actualItem.AdditionalTextEdits);
Assert.Null(actualItem.Command);
Assert.Null(actualItem.CommitCharacters);
Assert.Null(actualItem.Data);
Assert.Null(actualItem.Detail);
Assert.Null(actualItem.Documentation);

actualItem.Data = completionResult.ItemDefaults.Data;

var resolvedItem = await testLspServer.ExecuteRequestAsync<LSP.CompletionItem, LSP.CompletionItem>(LSP.Methods.TextDocumentCompletionResolveName, actualItem, CancellationToken.None).ConfigureAwait(false);
Assert.Equal("NS2", resolvedItem.LabelDetails.Description);
Assert.Equal("~ExtensionMethod NS2", resolvedItem.SortText);
Assert.Equal(CompletionItemKind.Method, resolvedItem.Kind);

var expectedAdditionalEdit = new TextEdit() { NewText = "using NS2;\r\n\r\n", Range = new() { Start = new(1, 0), End = new(1, 0) } };
AssertJsonEquals(new[] { expectedAdditionalEdit }, resolvedItem.AdditionalTextEdits);

Assert.Null(resolvedItem.LabelDetails.Detail);
Assert.Null(resolvedItem.FilterText);
Assert.Null(resolvedItem.TextEdit);
Assert.Null(resolvedItem.TextEditText);
Assert.Null(resolvedItem.Command);
Assert.Null(resolvedItem.Detail);

var expectedDocumentation = new MarkupContent()
{
Kind = LSP.MarkupKind.PlainText,
Value = "(extension) bool object.ExtensionMethod()"
};
AssertJsonEquals(resolvedItem.Documentation, expectedDocumentation);
}

[Theory, CombinatorialData]
public async Task TestResolveComplexEdit(bool mutatingLspWorkspace)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
' The .NET Foundation licenses this file to you under the MIT license.
' See the LICENSE file in the project root for more information.

Imports Microsoft.CodeAnalysis
Imports Microsoft.CodeAnalysis.Completion
Imports Microsoft.CodeAnalysis.Test.Utilities
Imports Roslyn.Test.Utilities

Expand Down Expand Up @@ -758,5 +760,60 @@ $$</Document>
Assert.Contains("args", state.GetCurrentViewLineText())
End Using
End Function

<WpfTheory, CombinatorialData>
Public Async Function CommitUnimportedTypeShouldFullyQualify(isImmediateWindow As Boolean) As Task
Dim text = <Workspace>
<Project Language="C#" CommonReferences="true">
<Document>class Program
{
static void Main(string[] args)
[|{|]
}
}</Document>
</Project>
</Workspace>

Using state = TestState.CreateCSharpTestState(text, isImmediateWindow)

state.Workspace.GlobalOptions.SetGlobalOption(CompletionOptionsStorage.ForceExpandedCompletionIndexCreation, True)
state.Workspace.GlobalOptions.SetGlobalOption(CompletionOptionsStorage.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp, True)

state.SendTypeChars("Console")
Await state.WaitForAsynchronousOperationsAsync()
Await state.AssertCompletionSession()
Await state.AssertSelectedCompletionItem("Console", inlineDescription:="System")

state.SendTab()

Assert.Contains("System.Console", state.GetCurrentViewLineText())
End Using
End Function

<WpfTheory, CombinatorialData>
Public Async Function ShouldNotProvideUnimportedExtensionMethods(isImmediateWindow As Boolean) As Task
Dim text = <Workspace>
<Project Language="C#" CommonReferences="true">
<Document>class Program
{
static void Main(string[] args)
[|{|]
}
}</Document>
</Project>
</Workspace>

Using state = TestState.CreateCSharpTestState(text, isImmediateWindow)

state.Workspace.GlobalOptions.SetGlobalOption(CompletionOptionsStorage.ForceExpandedCompletionIndexCreation, True)
state.Workspace.GlobalOptions.SetGlobalOption(CompletionOptionsStorage.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp, True)

state.SendTypeChars("args.To")
Await state.WaitForAsynchronousOperationsAsync()
Await state.AssertCompletionSession()
Await state.AssertCompletionItemsContain("ToString", "")
Await state.AssertCompletionItemsDoNotContainAny("ToArray", "ToDictionary", "ToList")
End Using
End Function
End Class
End Namespace

0 comments on commit 72efaa9

Please sign in to comment.