Do not include commit characters if the typed span is empty (#2569)

* Do not include commit characters if the typed span is empty * Extract TreatAsSuggestion code to a shared helper * Fix Completion tests for new Preselect behavior
OmniSharp · Oct 5, 2023 · 22d97e0 · 22d97e0
1 parent 79106f6
commit 22d97e0
Show file tree

Hide file tree

Showing 6 changed files with 62 additions and 33 deletions.
diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionListBuilder.cs b/src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionListBuilder.cs
@@ -1,16 +1,17 @@
 #nullable enable
 
+using System.Collections.Generic;
+using System.Collections.Immutable;
+using System.Linq;
+using System.Threading.Tasks;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.Completion;
 using Microsoft.CodeAnalysis.Tags;
 using Microsoft.CodeAnalysis.Text;
 using OmniSharp.Models;
 using OmniSharp.Models.v1.Completion;
-using System.Collections.Generic;
-using System.Collections.Immutable;
-using System.Linq;
-using System.Threading.Tasks;
 using CompletionItem = OmniSharp.Models.v1.Completion.CompletionItem;
+using CSharpCompletionItem = Microsoft.CodeAnalysis.Completion.CompletionItem;
 using CSharpCompletionList = Microsoft.CodeAnalysis.Completion.CompletionList;
 using CSharpCompletionService = Microsoft.CodeAnalysis.Completion.CompletionService;
 
@@ -171,5 +172,14 @@ private static CompletionItemKind GetCompletionItemKind(ImmutableArray<string> t
 
             return CompletionItemKind.Text;
         }
+
+        internal static bool ShouldTreatCompletionItemAsSuggestion(CSharpCompletionItem item, TextSpan typedSpan)
+        {
+            // The user hasn't actually typed anything and completion provider does
+            // not request the item be hard-selected.
+            return item.Rules.MatchPriority != MatchPriority.Preselect &&
+                typedSpan.Length == 0 &&
+                item.Rules.SelectionBehavior != CompletionItemSelectionBehavior.HardSelection;
+        }
     }
 }
diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionListBuilder_Async.cs b/src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionListBuilder_Async.cs
@@ -1,15 +1,16 @@
 #nullable enable
 
+using System.Collections.Generic;
+using System.Collections.Immutable;
+using System.Diagnostics;
+using System.Text;
+using System.Threading.Tasks;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.Completion;
 using Microsoft.CodeAnalysis.ExternalAccess.OmniSharp.Completion;
 using Microsoft.CodeAnalysis.Text;
 using OmniSharp.Models;
 using OmniSharp.Models.v1.Completion;
-using System.Collections.Generic;
-using System.Collections.Immutable;
-using System.Diagnostics;
-using System.Threading.Tasks;
 using CompletionItem = OmniSharp.Models.v1.Completion.CompletionItem;
 using CSharpCompletionList = Microsoft.CodeAnalysis.Completion.CompletionList;
 using CSharpCompletionService = Microsoft.CodeAnalysis.Completion.CompletionService;
@@ -95,7 +96,8 @@ await completionService.GetChangeAsync(document, completion),
                         out insertText, out filterText, out sortText, out insertTextFormat, out changeSpan, out additionalTextEdits);
                 }
 
-                var commitCharacters = BuildCommitCharacters(completion.Rules.CommitCharacterRules, isSuggestionMode, commitCharacterRuleCache, commitCharacterRuleBuilder);
+                var treatAsASuggestion = isSuggestionMode || ShouldTreatCompletionItemAsSuggestion(completion, typedSpan);
+                var commitCharacters = BuildCommitCharacters(completion.Rules.CommitCharacterRules, treatAsASuggestion, commitCharacterRuleCache, commitCharacterRuleBuilder);
 
                 completionsBuilder.Add(new CompletionItem
                 {
@@ -108,7 +110,7 @@ await completionService.GetChangeAsync(document, completion),
                     Kind = GetCompletionItemKind(completion.Tags),
                     Detail = completion.InlineDescription,
                     Data = (cacheId, i),
-                    Preselect = completion.Rules.SelectionBehavior == CompletionItemSelectionBehavior.HardSelection,
+                    Preselect = completion.Rules.MatchPriority == MatchPriority.Preselect,
                     CommitCharacters = commitCharacters,
                     HasAfterInsertStep = hasAfterInsertStep,
                 });

diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionListBuilder_Sync.cs b/src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionListBuilder_Sync.cs
@@ -1,20 +1,20 @@
 #nullable enable
 
+using System.Collections.Generic;
+using System.Collections.Immutable;
+using System.Diagnostics;
+using System.Linq;
+using System.Threading.Tasks;
 using Microsoft.CodeAnalysis;
-using Microsoft.CodeAnalysis.ExternalAccess.OmniSharp.Completion;
 using Microsoft.CodeAnalysis.Completion;
+using Microsoft.CodeAnalysis.ExternalAccess.OmniSharp.Completion;
 using Microsoft.CodeAnalysis.Text;
 using OmniSharp.Extensions;
 using OmniSharp.Models;
 using OmniSharp.Models.v1.Completion;
 using OmniSharp.Roslyn.CSharp.Helpers;
 using OmniSharp.Roslyn.Utilities;
 using OmniSharp.Utilities;
-using System.Collections.Generic;
-using System.Collections.Immutable;
-using System.Diagnostics;
-using System.Linq;
-using System.Threading.Tasks;
 using CompletionItem = OmniSharp.Models.v1.Completion.CompletionItem;
 using CSharpCompletionItem = Microsoft.CodeAnalysis.Completion.CompletionItem;
 using CSharpCompletionList = Microsoft.CodeAnalysis.Completion.CompletionList;
@@ -110,7 +110,8 @@ internal static partial class CompletionListBuilder
                         }
                 }
 
-                var commitCharacters = BuildCommitCharacters(completion.Rules.CommitCharacterRules, isSuggestionMode, commitCharacterRuleCache, commitCharacterRuleBuilder);
+                var treatAsASuggestion = isSuggestionMode || ShouldTreatCompletionItemAsSuggestion(completion, typedSpan);
+                var commitCharacters = BuildCommitCharacters(completion.Rules.CommitCharacterRules, treatAsASuggestion, commitCharacterRuleCache, commitCharacterRuleBuilder);
 
                 completionsBuilder.Add(new CompletionItem
                 {
@@ -123,7 +124,7 @@ internal static partial class CompletionListBuilder
                     Kind = GetCompletionItemKind(completion.Tags),
                     Detail = completion.InlineDescription,
                     Data = (cacheId, i),
-                    Preselect = completion.Rules.SelectionBehavior == CompletionItemSelectionBehavior.HardSelection,
+                    Preselect = completion.Rules.MatchPriority == MatchPriority.Preselect,
                     CommitCharacters = commitCharacters,
                 });
             }

diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionService.cs
@@ -8,8 +8,8 @@
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis;
-using Microsoft.CodeAnalysis.ExternalAccess.OmniSharp.Completion;
 using Microsoft.CodeAnalysis.Completion;
+using Microsoft.CodeAnalysis.ExternalAccess.OmniSharp.Completion;
 using Microsoft.CodeAnalysis.Text;
 using Microsoft.Extensions.Logging;
 using OmniSharp.Extensions;

diff --git a/tests/OmniSharp.Lsp.Tests/OmnisharpCompletionHandlerFacts.cs b/tests/OmniSharp.Lsp.Tests/OmnisharpCompletionHandlerFacts.cs
@@ -569,13 +569,7 @@ public MyClass2()
             var item = completions.Items.First(c => c.Label == "text:");
             Assert.NotNull(item);
             Assert.Equal("text", item.TextEdit.TextEdit.NewText);
-            Assert.All(completions.Items, c =>
-            {
-                if (c.Label == "ToString")
-                    Assert.True(c.Preselect);
-                else
-                    Assert.False(c.Preselect);
-            });
+            Assert.All(completions.Items, c => Assert.False(c.Preselect));
         }
 
         [Theory]

diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/CompletionFacts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/CompletionFacts.cs
@@ -588,13 +588,7 @@ public MyClass2()
             var item = completions.Items.First(c => c.Label == "text:");
             Assert.NotNull(item);
             Assert.Equal("text", item.TextEdit.NewText);
-            Assert.All(completions.Items, c =>
-            {
-                if (c.Label == "ToString")
-                    Assert.True(c.Preselect);
-                else
-                    Assert.False(c.Preselect);
-            });
+            Assert.All(completions.Items, c => Assert.False(c.Preselect));
         }
 
         [Theory]
@@ -2268,6 +2262,34 @@ public async Task ReplacesUpUntilCursorInMiddleOfWord(string filename)
             });
         }
 
+        [Theory]
+        [InlineData("dummy.cs", true)]
+        [InlineData("dummy.cs", false)]
+        [InlineData("dummy.csx", true)]
+        [InlineData("dummy.csx", false)]
+        public async Task SoftSelectionWhenFilterTextIsEmpty(string filename, bool useAsyncCompletion)
+        {
+            const string input = @"
+using System;
+using System.Text;
+public class A
+{
+    public void M(string someText)
+    {
+        var x = new StringBuilder();
+        x.Append($$
+    }
+}";
+
+            using var host = useAsyncCompletion ? GetAsyncCompletionAndImportCompletionHost() : GetImportCompletionHost();
+
+            var completions = await FindCompletionsAsync(filename, input, host, '(');
+            var someTextItem = completions.Items.First(item => item.Label == "someText");
+
+            Assert.Null(someTextItem.CommitCharacters);
+            Assert.False(someTextItem.Preselect);
+        }
+
         private CompletionService GetCompletionService(OmniSharpTestHost host)
             => host.GetRequestHandler<CompletionService>(EndpointName);