Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix import completion in LSP workspace #69691

Merged
merged 2 commits into from
Aug 24, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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(1, 20), End = new(1, 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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to ensure these options are also turned on in vscode? Or is it covered by
image

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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