Merge pull request #47480 from allisonchou/RevertCommitCharactersLSP

Revert LSP commit characters

src/Features/Core/Portable/Completion/CompletionTrigger.cs

@@ -6,16 +6,6 @@
 
 namespace Microsoft.CodeAnalysis.Completion
 {
-    /// <summary>
-    /// The action that triggered completion to start.
-    /// </summary>
-    /// <remarks>
-    /// NOTE: Roslyn's LSP completion implementation uses this struct. If a new property is added, either:
-    ///     1: The property's type must be serializable
-    ///     OR
-    ///     2. LSP will need to be updated to not use CompletionTrigger - see
-    ///        Features\LanguageServer\Protocol\Handler\Completion\CompletionResolveData.cs
-    /// </remarks>
     public readonly struct CompletionTrigger
     {
         /// <summary>

src/Features/LanguageServer/Protocol/Extensions/ProtocolConversions.cs

@@ -10,17 +10,14 @@
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.DocumentHighlighting;
-using Microsoft.CodeAnalysis.Elfie.Diagnostics;
 using Microsoft.CodeAnalysis.Host;
-using Microsoft.CodeAnalysis.Internal.Log;
 using Microsoft.CodeAnalysis.NavigateTo;
 using Microsoft.CodeAnalysis.Tags;
 using Microsoft.CodeAnalysis.Text;
 using Microsoft.VisualStudio.LanguageServer.Protocol;
 using Microsoft.VisualStudio.Text.Adornments;
 using Microsoft.VisualStudio.Utilities;
 using Roslyn.Utilities;
-using Logger = Microsoft.CodeAnalysis.Internal.Log.Logger;
 using LSP = Microsoft.VisualStudio.LanguageServer.Protocol;
 
 namespace Microsoft.CodeAnalysis.LanguageServer
@@ -68,23 +65,6 @@ internal static class ProtocolConversions
             { WellKnownTags.NuGet, LSP.CompletionItemKind.Text }
         };
 
-        // TO-DO: More LSP.CompletionTriggerKind mappings are required to properly map to Roslyn CompletionTriggerKinds.
-        // https://dev.azure.com/devdiv/DevDiv/_workitems/edit/1178726
-        public static Completion.CompletionTriggerKind LSPToRoslynCompletionTriggerKind(LSP.CompletionTriggerKind triggerKind)
-        {
-            switch (triggerKind)
-            {
-                case LSP.CompletionTriggerKind.Invoked:
-                    return Completion.CompletionTriggerKind.Invoke;
-                case LSP.CompletionTriggerKind.TriggerCharacter:
-                    return Completion.CompletionTriggerKind.Insertion;
-                default:
-                    // LSP added a TriggerKind that we need to support.
-                    Logger.Log(FunctionId.LSPCompletion_MissingLSPCompletionTriggerKind);
-                    return Completion.CompletionTriggerKind.Invoke;
-            }
-        }
-
         public static Uri GetUriFromFilePath(string? filePath)
         {
             if (filePath is null)

src/Features/LanguageServer/Protocol/Handler/Completion/CompletionHandler.cs

@@ -5,19 +5,16 @@
 #nullable enable
 
 using System;
-using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Composition;
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.Completion;
-using Microsoft.CodeAnalysis.Completion.Providers;
 using Microsoft.CodeAnalysis.Editor.Shared.Extensions;
 using Microsoft.CodeAnalysis.Host.Mef;
-using Microsoft.CodeAnalysis.PooledObjects;
+using Microsoft.CodeAnalysis.LanguageServer.CustomProtocol;
 using Microsoft.VisualStudio.Text.Adornments;
-using Roslyn.Utilities;
 using LSP = Microsoft.VisualStudio.LanguageServer.Protocol;
 
 namespace Microsoft.CodeAnalysis.LanguageServer.Handler
@@ -29,20 +26,10 @@ namespace Microsoft.CodeAnalysis.LanguageServer.Handler
     [ExportLspMethod(LSP.Methods.TextDocumentCompletionName)]
     internal class CompletionHandler : AbstractRequestHandler<LSP.CompletionParams, LSP.CompletionItem[]>
     {
-        private readonly ImmutableHashSet<string> _csTriggerCharacters;
-        private readonly ImmutableHashSet<string> _vbTriggerCharacters;
-
         [ImportingConstructor]
         [Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
-        public CompletionHandler(
-            ILspSolutionProvider solutionProvider,
-            [ImportMany] IEnumerable<Lazy<CompletionProvider, CompletionProviderMetadata>> completionProviders)
-            : base(solutionProvider)
+        public CompletionHandler(ILspSolutionProvider solutionProvider) : base(solutionProvider)
         {
-            _csTriggerCharacters = completionProviders.Where(lz => lz.Metadata.Language == LanguageNames.CSharp).SelectMany(
-                lz => GetTriggerCharacters(lz.Value)).Select(c => c.ToString()).ToImmutableHashSet();
-            _vbTriggerCharacters = completionProviders.Where(lz => lz.Metadata.Language == LanguageNames.VisualBasic).SelectMany(
-                lz => GetTriggerCharacters(lz.Value)).Select(c => c.ToString()).ToImmutableHashSet();
         }
 
         public override async Task<LSP.CompletionItem[]> HandleRequestAsync(LSP.CompletionParams request, RequestContext context, CancellationToken cancellationToken)
@@ -53,16 +40,6 @@ public CompletionHandler(
                 return Array.Empty<LSP.CompletionItem>();
             }
 
-            // C# and VB share the same LSP language server, and thus share the same default trigger characters.
-            // We need to ensure the trigger character is valid in the document's language. For example, the '{'
-            // character, while a trigger character in VB, is not a trigger character in C#.
-            var triggerCharacter = char.Parse(request.Context.TriggerCharacter);
-            if (request.Context.TriggerKind == LSP.CompletionTriggerKind.TriggerCharacter && !char.IsLetterOrDigit(triggerCharacter) &&
-                !IsValidTriggerCharacterForDocument(document, request.Context.TriggerCharacter))
-            {
-                return Array.Empty<LSP.CompletionItem>();
-            }
-
             var position = await document.GetPositionFromLinePositionAsync(ProtocolConversions.PositionToLinePosition(request.Position), cancellationToken).ConfigureAwait(false);
 
             // Filter out snippets as they are not supported in the LSP client
@@ -81,124 +58,44 @@ public CompletionHandler(
                 .WithChangedOption(CompletionServiceOptions.DisallowAddingImports, true);
 
             var completionService = document.Project.LanguageServices.GetRequiredService<CompletionService>();
-
-            // TO-DO: More LSP.CompletionTriggerKind mappings are required to properly map to Roslyn CompletionTriggerKinds.
-            // https://dev.azure.com/devdiv/DevDiv/_workitems/edit/1178726
-            var triggerKind = ProtocolConversions.LSPToRoslynCompletionTriggerKind(request.Context.TriggerKind);
-            var completionTrigger = new CompletionTrigger(triggerKind, triggerCharacter);
-
-            var list = await completionService.GetCompletionsAsync(document, position, completionTrigger, options: completionOptions, cancellationToken: cancellationToken).ConfigureAwait(false);
+            var list = await completionService.GetCompletionsAsync(document, position, options: completionOptions, cancellationToken: cancellationToken).ConfigureAwait(false);
             if (list == null)
             {
                 return Array.Empty<LSP.CompletionItem>();
             }
 
             var lspVSClientCapability = context.ClientCapabilities?.HasVisualStudioLspCapability() == true;
 
-            return list.Items.Select(item => CreateLSPCompletionItem(request, item, lspVSClientCapability, completionTrigger)).ToArray();
+            return list.Items.Select(item => CreateLSPCompletionItem(request, item, lspVSClientCapability)).ToArray();
 
-            // Local functions
-            bool IsValidTriggerCharacterForDocument(Document document, string triggerCharacter)
-            {
-                if (document.Project.Language == LanguageNames.CSharp)
-                {
-                    return _csTriggerCharacters.Contains(triggerCharacter);
-                }
-                else if (document.Project.Language == LanguageNames.VisualBasic)
-                {
-                    return _vbTriggerCharacters.Contains(triggerCharacter);
-                }
-
-                return true;
-            }
-
-            static LSP.CompletionItem CreateLSPCompletionItem(
-                LSP.CompletionParams request,
-                CompletionItem item,
-                bool useVSCompletionItem,
-                CompletionTrigger completionTrigger)
+            // local functions
+            static LSP.CompletionItem CreateLSPCompletionItem(LSP.CompletionParams request, CompletionItem item, bool useVSCompletionItem)
             {
                 if (useVSCompletionItem)
                 {
-                    var vsCompletionItem = CreateCompletionItem<LSP.VSCompletionItem>(request, item, completionTrigger);
+                    var vsCompletionItem = CreateCompletionItem<LSP.VSCompletionItem>(request, item);
                     vsCompletionItem.Icon = new ImageElement(item.Tags.GetFirstGlyph().GetImageId());
                     return vsCompletionItem;
                 }
                 else
                 {
-                    var roslynCompletionItem = CreateCompletionItem<LSP.CompletionItem>(request, item, completionTrigger);
+                    var roslynCompletionItem = CreateCompletionItem<RoslynCompletionItem>(request, item);
+                    roslynCompletionItem.Tags = item.Tags.ToArray();
                     return roslynCompletionItem;
                 }
             }
 
-            static TCompletionItem CreateCompletionItem<TCompletionItem>(
-                LSP.CompletionParams request,
-                CompletionItem item,
-                CompletionTrigger completionTrigger) where TCompletionItem : LSP.CompletionItem, new()
-            {
-                var completeDisplayText = item.DisplayTextPrefix + item.DisplayText + item.DisplayTextSuffix;
-                var completionItem = new TCompletionItem
+            static TCompletionItem CreateCompletionItem<TCompletionItem>(LSP.CompletionParams request, CompletionItem item) where TCompletionItem : LSP.CompletionItem, new()
+                => new TCompletionItem
                 {
-                    Label = completeDisplayText,
-                    InsertText = item.Properties.ContainsKey("InsertionText") ? item.Properties["InsertionText"] : completeDisplayText,
+                    Label = item.DisplayTextPrefix + item.DisplayText + item.DisplayTextSuffix,
+                    InsertText = item.Properties.ContainsKey("InsertionText") ? item.Properties["InsertionText"] : item.DisplayText,
                     SortText = item.SortText,
                     FilterText = item.FilterText,
                     Kind = GetCompletionKind(item.Tags),
-                    Data = new CompletionResolveData
-                    {
-                        TextDocument = request.TextDocument,
-                        Position = request.Position,
-                        DisplayText = item.DisplayText,
-                        CompletionTrigger = completionTrigger,
-                    },
+                    Data = new CompletionResolveData { TextDocument = request.TextDocument, Position = request.Position, DisplayText = item.DisplayText },
                     Preselect = item.Rules.SelectionBehavior == CompletionItemSelectionBehavior.HardSelection,
-                    CommitCharacters = GetCommitCharacters(item)
                 };
-
-                return completionItem;
-            }
-
-            static string[]? GetCommitCharacters(CompletionItem item)
-            {
-                var commitCharacterRules = item.Rules.CommitCharacterRules;
-
-                // If the item doesn't have any special rules, just use the default commit characters.
-                if (commitCharacterRules.IsEmpty)
-                {
-                    return null;
-                }
-
-                using var _ = PooledHashSet<char>.GetInstance(out var commitCharacters);
-                commitCharacters.AddAll(CompletionRules.Default.DefaultCommitCharacters);
-                foreach (var rule in commitCharacterRules)
-                {
-                    switch (rule.Kind)
-                    {
-                        case CharacterSetModificationKind.Add:
-                            commitCharacters.UnionWith(rule.Characters);
-                            continue;
-                        case CharacterSetModificationKind.Remove:
-                            commitCharacters.ExceptWith(rule.Characters);
-                            continue;
-                        case CharacterSetModificationKind.Replace:
-                            commitCharacters.Clear();
-                            commitCharacters.AddRange(rule.Characters);
-                            break;
-                    }
-                }
-
-                return commitCharacters.Select(c => c.ToString()).ToArray();
-            }
-        }
-
-        internal static ImmutableHashSet<char> GetTriggerCharacters(CompletionProvider provider)
-        {
-            if (provider is LSPCompletionProvider lspProvider)
-            {
-                return lspProvider.TriggerCharacters;
-            }
-
-            return ImmutableHashSet<char>.Empty;
         }
 
         private static LSP.CompletionItemKind GetCompletionKind(ImmutableArray<string> tags)

src/Features/LanguageServer/Protocol/Handler/Completion/CompletionResolveData.cs

@@ -2,7 +2,6 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-using Microsoft.CodeAnalysis.Completion;
 using Microsoft.VisualStudio.LanguageServer.Protocol;
 
 namespace Microsoft.CodeAnalysis.LanguageServer.Handler
@@ -14,7 +13,5 @@ internal class CompletionResolveData
         public Position Position { get; set; }
 
         public string DisplayText { get; set; }
-
-        public CompletionTrigger CompletionTrigger { get; set; }
     }
 }

src/Features/LanguageServer/Protocol/Handler/Completion/CompletionResolveHandler.cs

@@ -52,7 +52,7 @@ public CompletionResolveHandler(ILspSolutionProvider solutionProvider) : base(so
             var position = await document.GetPositionFromLinePositionAsync(ProtocolConversions.PositionToLinePosition(data.Position), cancellationToken).ConfigureAwait(false);
 
             var completionService = document.Project.LanguageServices.GetRequiredService<CompletionService>();
-            var list = await completionService.GetCompletionsAsync(document, position, data.CompletionTrigger, cancellationToken: cancellationToken).ConfigureAwait(false);
+            var list = await completionService.GetCompletionsAsync(document, position, cancellationToken: cancellationToken).ConfigureAwait(false);
             if (list == null)
             {
                 return completionItem;

src/Features/LanguageServer/Protocol/Handler/Initialize/InitializeHandler.cs

@@ -36,9 +36,7 @@ public InitializeHandler([ImportMany] IEnumerable<Lazy<CompletionProvider, Compl
 
         public Task<LSP.InitializeResult> HandleRequestAsync(LSP.InitializeParams request, RequestContext context, CancellationToken cancellationToken)
         {
-            var commitCharacters = CompletionRules.Default.DefaultCommitCharacters.Select(c => c.ToString()).ToArray();
-            var triggerCharacters = _completionProviders.SelectMany(
-                lz => CompletionHandler.GetTriggerCharacters(lz.Value)).Distinct().Select(c => c.ToString()).ToArray();
+            var triggerCharacters = _completionProviders.SelectMany(lz => GetTriggerCharacters(lz.Value)).Distinct().Select(c => c.ToString()).ToArray();
 
             return Task.FromResult(new LSP.InitializeResult
             {
@@ -49,12 +47,7 @@ public InitializeHandler([ImportMany] IEnumerable<Lazy<CompletionProvider, Compl
                     ImplementationProvider = true,
                     CodeActionProvider = new LSP.CodeActionOptions { CodeActionKinds = new[] { CodeActionKind.QuickFix, CodeActionKind.Refactor } },
                     CodeActionsResolveProvider = true,
-                    CompletionProvider = new LSP.CompletionOptions
-                    {
-                        ResolveProvider = true,
-                        AllCommitCharacters = commitCharacters,
-                        TriggerCharacters = triggerCharacters
-                    },
+                    CompletionProvider = new LSP.CompletionOptions { ResolveProvider = true, TriggerCharacters = triggerCharacters },
                     SignatureHelpProvider = new LSP.SignatureHelpOptions { TriggerCharacters = new[] { "(", "," } },
                     DocumentSymbolProvider = true,
                     WorkspaceSymbolProvider = true,
@@ -73,5 +66,15 @@ public InitializeHandler([ImportMany] IEnumerable<Lazy<CompletionProvider, Compl
                 }
             });
         }
+
+        private static ImmutableHashSet<char> GetTriggerCharacters(CompletionProvider provider)
+        {
+            if (provider is LSPCompletionProvider lspProvider)
+            {
+                return lspProvider.TriggerCharacters;
+            }
+
+            return ImmutableHashSet<char>.Empty;
+        }
     }
 }

src/Features/LanguageServer/ProtocolUnitTests/Completion/CompletionResolveTests.cs

@@ -28,19 +28,13 @@ void M()
             using var workspace = CreateTestWorkspace(markup, out var locations);
             var tags = new string[] { "Class", "Internal" };
             var completionParams = CreateCompletionParams(locations["caret"].Single(), "\0", LSP.CompletionTriggerKind.Invoked);
-            var commitCharacters = new string[]
-                {
-                    " ", "{", "}", "[", "]", "(", ")", ".", ",", ":",
-                    ";", "+", "-", "*", "/", "%", "&", "|", "^", "!",
-                    "~", "=", "<", ">", "?", "@", "#", "'", "\"", "\\"
-                };
             var completionItem = CreateCompletionItem
-                ("A", LSP.CompletionItemKind.Class, tags, completionParams, commitCharacters: commitCharacters);
+                ("A", LSP.CompletionItemKind.Class, tags, completionParams);
             var description = new ClassifiedTextElement(CreateClassifiedTextRunForClass("A"));
             var clientCapabilities = new LSP.VSClientCapabilities { SupportsVisualStudioExtensions = true };
 
             var expected = CreateResolvedCompletionItem(
-                "A", LSP.CompletionItemKind.Class, null, completionParams, description, "class A", null, commitCharacters);
+                "A", LSP.CompletionItemKind.Class, null, completionParams, description, "class A", null);
 
             var results = (LSP.VSCompletionItem)await RunResolveCompletionItemAsync(workspace.CurrentSolution, completionItem, clientCapabilities);
             AssertJsonEquals(expected, results);

src/Features/LanguageServer/ProtocolUnitTests/Completion/CompletionTests.cs

@@ -28,13 +28,7 @@ void M()
             using var workspace = CreateTestWorkspace(markup, out var locations);
             var completionParams = CreateCompletionParams(
                 locations["caret"].Single(), triggerCharacter: "\0", triggerKind: LSP.CompletionTriggerKind.Invoked);
-            var expected = CreateCompletionItem("A", LSP.CompletionItemKind.Class, new string[] { "Class", "Internal" },
-                completionParams, commitCharacters: new string[]
-                {
-                    " ", "{", "}", "[", "]", "(", ")", ".", ",", ":",
-                    ";", "+", "-", "*", "/", "%", "&", "|", "^", "!",
-                    "~", "=", "<", ">", "?", "@", "#", "'", "\"", "\\"
-                });
+            var expected = CreateCompletionItem("A", LSP.CompletionItemKind.Class, new string[] { "Class", "Internal" }, completionParams);
 
             var results = await RunGetCompletionsAsync(workspace.CurrentSolution, completionParams).ConfigureAwait(false);
             AssertJsonEquals(expected, results.First());
@@ -98,7 +92,7 @@ void M()
             using var workspace = CreateTestWorkspace(markup, out var locations);
             var completionParams = CreateCompletionParams(locations["caret"].Single(), triggerCharacter: "\0", LSP.CompletionTriggerKind.Invoked);
             var expected = CreateCompletionItem("A", LSP.CompletionItemKind.Class, new string[] { "Class", "Internal" },
-                completionParams, preselect: true, commitCharacters: new string[] { "", "(", "[", "{" });
+                completionParams, preselect: true);
 
             var results = await RunGetCompletionsAsync(workspace.CurrentSolution, completionParams).ConfigureAwait(false);
             AssertJsonEquals(expected, results.First());