From e2ec322b4ad93cfa9efe22a77a824a7d5a9860a4 Mon Sep 17 00:00:00 2001 From: "gel@microsoft.com" Date: Wed, 23 Aug 2023 16:47:23 -0700 Subject: [PATCH 1/2] Fix import completion in LSP workspace 1. Provide unimported extension methods 2. Fully qualify unimported type in using statment --- .../AsyncCompletion/CommitManager.cs | 5 + .../AsyncCompletion/CompletionSource.cs | 4 +- .../Portable/Completion/CompletionOptions.cs | 6 ++ ...ExtensionMethodImportCompletionProvider.cs | 4 +- .../AbstractImportCompletionProvider.cs | 25 ++--- .../ImportCompletionItem.cs | 6 +- ...tractLspCompletionResultCreationService.cs | 1 - .../Completion/CompletionFeaturesTests.cs | 95 ++++++++++++++++++- .../CSharpDebuggerIntellisenseTests.vb | 57 +++++++++++ 9 files changed, 174 insertions(+), 29 deletions(-) diff --git a/src/EditorFeatures/Core/IntelliSense/AsyncCompletion/CommitManager.cs b/src/EditorFeatures/Core/IntelliSense/AsyncCompletion/CommitManager.cs index 89469ba637c1c..b5d3014a3e200 100644 --- a/src/EditorFeatures/Core/IntelliSense/AsyncCompletion/CommitManager.cs +++ b/src/EditorFeatures/Core/IntelliSense/AsyncCompletion/CommitManager.cs @@ -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; @@ -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)) diff --git a/src/EditorFeatures/Core/IntelliSense/AsyncCompletion/CompletionSource.cs b/src/EditorFeatures/Core/IntelliSense/AsyncCompletion/CompletionSource.cs index cee50b765abb2..25557c77d333f 100644 --- a/src/EditorFeatures/Core/IntelliSense/AsyncCompletion/CompletionSource.cs +++ b/src/EditorFeatures/Core/IntelliSense/AsyncCompletion/CompletionSource.cs @@ -392,7 +392,9 @@ public async Task GetExpandedCompletionContextAsync( options = options with { FilterOutOfScopeLocals = false, - ShowXmlDocCommentCompletion = false + ShowXmlDocCommentCompletion = false, + // Adding import is not allowed in debugger view + CanAddImportStatement = false, }; } diff --git a/src/Features/Core/Portable/Completion/CompletionOptions.cs b/src/Features/Core/Portable/Completion/CompletionOptions.cs index a251bd2866530..6f2b6c928933b 100644 --- a/src/Features/Core/Portable/Completion/CompletionOptions.cs +++ b/src/Features/Core/Portable/Completion/CompletionOptions.cs @@ -36,6 +36,12 @@ internal sealed record class CompletionOptions /// public bool UpdateImportCompletionCacheInBackground { get; init; } = false; + /// + /// Whether completion can add import statement as part of committed change. + /// For example, adding import is not allowed in debugger view. + /// + public bool CanAddImportStatement { get; init; } = true; + public bool FilterOutOfScopeLocals { get; init; } = true; public bool ShowXmlDocCommentCompletion { get; init; } = true; public bool? ShowNewSnippetExperienceUserOption { get; init; } = null; diff --git a/src/Features/Core/Portable/Completion/Providers/ImportCompletionProvider/AbstractExtensionMethodImportCompletionProvider.cs b/src/Features/Core/Portable/Completion/Providers/ImportCompletionProvider/AbstractExtensionMethodImportCompletionProvider.cs index ad42c118b9010..d79c88928b0f2 100644 --- a/src/Features/Core/Portable/Completion/Providers/ImportCompletionProvider/AbstractExtensionMethodImportCompletionProvider.cs +++ b/src/Features/Core/Portable/Completion/Providers/ImportCompletionProvider/AbstractExtensionMethodImportCompletionProvider.cs @@ -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(); diff --git a/src/Features/Core/Portable/Completion/Providers/ImportCompletionProvider/AbstractImportCompletionProvider.cs b/src/Features/Core/Portable/Completion/Providers/ImportCompletionProvider/AbstractImportCompletionProvider.cs index 41550239a3510..e2d921381e5cd 100644 --- a/src/Features/Core/Portable/Completion/Providers/ImportCompletionProvider/AbstractImportCompletionProvider.cs +++ b/src/Features/Core/Portable/Completion/Providers/ImportCompletionProvider/AbstractImportCompletionProvider.cs @@ -202,10 +202,10 @@ public override async Task GetChangeAsync( async Task 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 @@ -242,24 +242,11 @@ private async Task 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(); - if (!documentSupportsFeatureService.SupportsRefactorings(document)) - { - return false; - } - - return true; + return completionOptions?.CanAddImportStatement != false && + document.Project.Solution.Services.GetRequiredService().SupportsRefactorings(document); } private static SyntaxNode CreateImport(Document document, string namespaceName) diff --git a/src/Features/Core/Portable/Completion/Providers/ImportCompletionProvider/ImportCompletionItem.cs b/src/Features/Core/Portable/Completion/Providers/ImportCompletionProvider/ImportCompletionItem.cs index c9ac87149b582..e0ac026c26fe2 100644 --- a/src/Features/Core/Portable/Completion/Providers/ImportCompletionProvider/ImportCompletionItem.cs +++ b/src/Features/Core/Portable/Completion/Providers/ImportCompletionProvider/ImportCompletionItem.cs @@ -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, @@ -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); } } diff --git a/src/Features/LanguageServer/Protocol/Handler/Completion/AbstractLspCompletionResultCreationService.cs b/src/Features/LanguageServer/Protocol/Handler/Completion/AbstractLspCompletionResultCreationService.cs index e08a9c4a6a3df..65596f7530252 100644 --- a/src/Features/LanguageServer/Protocol/Handler/Completion/AbstractLspCompletionResultCreationService.cs +++ b/src/Features/LanguageServer/Protocol/Handler/Completion/AbstractLspCompletionResultCreationService.cs @@ -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); diff --git a/src/Features/LanguageServer/ProtocolUnitTests/Completion/CompletionFeaturesTests.cs b/src/Features/LanguageServer/ProtocolUnitTests/Completion/CompletionFeaturesTests.cs index a419c4f101eb6..b5f0a6a82afa4 100644 --- a/src/Features/LanguageServer/ProtocolUnitTests/Completion/CompletionFeaturesTests.cs +++ b/src/Features/LanguageServer/ProtocolUnitTests/Completion/CompletionFeaturesTests.cs @@ -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() @@ -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); @@ -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.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.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) { diff --git a/src/VisualStudio/Core/Test/DebuggerIntelliSense/CSharpDebuggerIntellisenseTests.vb b/src/VisualStudio/Core/Test/DebuggerIntelliSense/CSharpDebuggerIntellisenseTests.vb index 8ef1dee83adda..5f88fb474c41a 100644 --- a/src/VisualStudio/Core/Test/DebuggerIntelliSense/CSharpDebuggerIntellisenseTests.vb +++ b/src/VisualStudio/Core/Test/DebuggerIntelliSense/CSharpDebuggerIntellisenseTests.vb @@ -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 @@ -758,5 +760,60 @@ $$ Assert.Contains("args", state.GetCurrentViewLineText()) End Using End Function + + + Public Async Function CommitUnimportedTypeShouldFullyQualify(isImmediateWindow As Boolean) As Task + Dim text = + + class Program +{ + static void Main(string[] args) + [|{|] + } +} + + + + 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 + + + Public Async Function ShouldNotProvideUnimportedExtensionMethods(isImmediateWindow As Boolean) As Task + Dim text = + + class Program +{ + static void Main(string[] args) + [|{|] + } +} + + + + 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 From dfa4c4884e11fd1755d9c1716ded320dffe71546 Mon Sep 17 00:00:00 2001 From: "gel@microsoft.com" Date: Thu, 24 Aug 2023 11:20:11 -0700 Subject: [PATCH 2/2] Fix test --- .../ProtocolUnitTests/Completion/CompletionFeaturesTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Features/LanguageServer/ProtocolUnitTests/Completion/CompletionFeaturesTests.cs b/src/Features/LanguageServer/ProtocolUnitTests/Completion/CompletionFeaturesTests.cs index b5f0a6a82afa4..f4210d351dccd 100644 --- a/src/Features/LanguageServer/ProtocolUnitTests/Completion/CompletionFeaturesTests.cs +++ b/src/Features/LanguageServer/ProtocolUnitTests/Completion/CompletionFeaturesTests.cs @@ -149,7 +149,7 @@ void M() Assert.Equal(CompletionItemKind.Class, resolvedItem.Kind); TextEdit expectedAdditionalEdit = isInUsingStatement - ? new() { NewText = "System.Threading.Tasks.Task", Range = new() { Start = new(1, 20), End = new(1, 24) } } + ? 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);