Merge pull request #11836 from CyrusNajmabadi/moveCompletionMethods

Simplify completion helper code.

src/EditorFeatures/CSharpTest/Completion/CompletionProviders/AbstractCSharpCompletionProviderTests.cs

@@ -7,6 +7,7 @@
 using Microsoft.CodeAnalysis.Completion;
 using Microsoft.CodeAnalysis.CSharp;
 using Microsoft.CodeAnalysis.CSharp.Completion;
+using Microsoft.CodeAnalysis.Editor.Implementation.IntelliSense.Completion;
 using Microsoft.CodeAnalysis.Editor.UnitTests.Completion;
 using Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces;
 using Microsoft.CodeAnalysis.Text;
@@ -151,8 +152,7 @@ protected async Task VerifySendEnterThroughToEnterAsync(string initialMarkup, st
                 var completionList = await GetCompletionListAsync(service, document, position, CompletionTrigger.Default);
                 var item = completionList.Items.First(i => i.DisplayText.StartsWith(textTypedSoFar));
 
-                var completionRules = CompletionHelper.GetHelper(document, service);
-                Assert.Equal(expected, completionRules.SendEnterThroughToEditor(item, textTypedSoFar, workspace.Options));
+                Assert.Equal(expected, Controller.SendEnterThroughToEditor(service.GetRules(), item, textTypedSoFar));
             }
         }
 
@@ -216,16 +216,14 @@ protected async Task VerifyCommitCharactersAsync(string initialMarkup, string te
                 var completionList = await GetCompletionListAsync(service, document, position, CompletionTrigger.Default);
                 var item = completionList.Items.First(i => i.DisplayText.StartsWith(textTypedSoFar));
 
-                var completionRules = CompletionHelper.GetHelper(document, service);
-
                 foreach (var ch in validChars)
                 {
-                    Assert.True(completionRules.IsCommitCharacter(item, ch, textTypedSoFar), $"Expected '{ch}' to be a commit character");
+                    Assert.True(Controller.IsCommitCharacter(service.GetRules(), item, ch, textTypedSoFar), $"Expected '{ch}' to be a commit character");
                 }
 
                 foreach (var ch in invalidChars)
                 {
-                    Assert.False(completionRules.IsCommitCharacter(item, ch, textTypedSoFar), $"Expected '{ch}' NOT to be a commit character");
+                    Assert.False(Controller.IsCommitCharacter(service.GetRules(), item, ch, textTypedSoFar), $"Expected '{ch}' NOT to be a commit character");
                 }
             }
         }

src/EditorFeatures/CSharpTest/Completion/CompletionProviders/ObjectInitializerCompletionProviderTests.cs

@@ -4,6 +4,7 @@
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.Completion;
 using Microsoft.CodeAnalysis.CSharp.Completion.Providers;
+using Microsoft.CodeAnalysis.Editor.Implementation.IntelliSense.Completion;
 using Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces;
 using Roslyn.Test.Utilities;
 using Xunit;
@@ -579,9 +580,7 @@ void foo()
                 var completionList = await GetCompletionListAsync(service, document, position, triggerInfo);
                 var item = completionList.Items.First();
 
-                var completionRules = CompletionHelper.GetHelper(document, service);
-
-                Assert.False(completionRules.SendEnterThroughToEditor(item, string.Empty, workspace.Options), "Expected false from SendEnterThroughToEditor()");
+                Assert.False(Controller.SendEnterThroughToEditor(service.GetRules(), item, string.Empty), "Expected false from SendEnterThroughToEditor()");
             }
         }
 

src/EditorFeatures/Core/Extensibility/Completion/CompletionHelper.cs

@@ -313,80 +313,11 @@ protected bool IsArgumentName(CompletionItem item)
             return item.Tags.Contains(CompletionTags.ArgumentName);
         }
 
-        /// <summary>
-        /// Returns true if the character is one that can commit the specified completion item. A
-        /// character will be checked to see if it should filter an item.  If not, it will be checked
-        /// to see if it should commit that item.  If it does neither, then completion will be
-        /// dismissed.
-        /// </summary>
-        public virtual bool IsCommitCharacter(CompletionItem item, char ch, string textTypedSoFar, string textTypedWithChar = null)
+        private static bool TextTypedSoFarMatchesItem(CompletionItem item, char ch, string textTypedSoFar)
         {
-            // general rule: if the filtering text exactly matches the start of the item then it must be a filter character
-            textTypedWithChar = textTypedWithChar ?? textTypedSoFar + ch;
-            if (item.DisplayText.StartsWith(textTypedWithChar, StringComparison.CurrentCultureIgnoreCase)
-                || item.FilterText.StartsWith(textTypedWithChar, StringComparison.CurrentCultureIgnoreCase))
-            {
-                return false;
-            }
-
-            foreach (var rule in item.Rules.CommitCharacterRules)
-            {
-                switch (rule.Kind)
-                {
-                    case CharacterSetModificationKind.Add:
-                        if (rule.Characters.IndexOf(ch) >= 0)
-                            return true;
-                        break;
-
-                    case CharacterSetModificationKind.Remove:
-                        if (rule.Characters.IndexOf(ch) >= 0)
-                            return false;
-                        break;
-
-                    case CharacterSetModificationKind.Replace:
-                        return rule.Characters.IndexOf(ch) >= 0;
-                }
-            }
-
-            return _rules.DefaultCommitCharacters.IndexOf(ch) >= 0;
-        }
-
-        /// <summary>
-        /// Returns true if the character typed should be used to filter the specified completion
-        /// item.  A character will be checked to see if it should filter an item.  If not, it will be
-        /// checked to see if it should commit that item.  If it does neither, then completion will
-        /// be dismissed.
-        /// </summary>
-        public virtual bool IsFilterCharacter(CompletionItem item, char ch, string textTypedSoFar, string textTypedWithChar = null)
-        {
-            // general rule: if the filtering text exactly matches the start of the item then it must be a filter character
-            textTypedWithChar = textTypedWithChar ?? textTypedSoFar + ch;
-            if (item.DisplayText.StartsWith(textTypedWithChar, StringComparison.CurrentCultureIgnoreCase)
-                || item.FilterText.StartsWith(textTypedWithChar, StringComparison.CurrentCultureIgnoreCase))
-            {
-                return false;
-            }
-
-            foreach (var rule in item.Rules.FilterCharacterRules)
-            {
-                switch (rule.Kind)
-                {
-                    case CharacterSetModificationKind.Add:
-                        if (rule.Characters.IndexOf(ch) >= 0)
-                            return true;
-                        break;
-
-                    case CharacterSetModificationKind.Remove:
-                        if (rule.Characters.IndexOf(ch) >= 0)
-                            return false;
-                        break;
-
-                    case CharacterSetModificationKind.Replace:
-                        return rule.Characters.IndexOf(ch) >= 0;
-                }
-            }
-
-            return false;
+            var textTypedWithChar = textTypedSoFar + ch;
+            return item.DisplayText.StartsWith(textTypedWithChar, StringComparison.CurrentCultureIgnoreCase) ||
+                item.FilterText.StartsWith(textTypedWithChar, StringComparison.CurrentCultureIgnoreCase);
         }
 
         private static StringComparison GetComparision(bool isCaseSensitive)
@@ -394,31 +325,6 @@ private static StringComparison GetComparision(bool isCaseSensitive)
             return isCaseSensitive? StringComparison.CurrentCulture: StringComparison.CurrentCultureIgnoreCase;
         }
 
-        /// <summary>
-        /// Returns true if the enter key that was typed should also be sent through to the editor
-        /// after committing the provided completion item.
-        /// </summary>
-        public virtual bool SendEnterThroughToEditor(CompletionItem item, string textTypedSoFar, OptionSet options)
-        {
-            var rule = item.Rules.EnterKeyRule;
-            if (rule == EnterKeyRule.Default)
-            {
-                rule = _rules.DefaultEnterKeyRule;
-            }
-
-            switch (rule)
-            {
-                default:
-                case EnterKeyRule.Default:
-                case EnterKeyRule.Never:
-                    return false;
-                case EnterKeyRule.Always:
-                    return true;
-                case EnterKeyRule.AfterFullyTypedWord:
-                    return item.DisplayText == textTypedSoFar;
-            }
-        }
-
         /// <summary>
         /// Returns true if the completion item should be "soft" selected, or false if it should be "hard"
         /// selected.
@@ -432,22 +338,5 @@ protected bool IsObjectCreationItem(CompletionItem item)
         {
             return item.Tags.Contains(CompletionTags.ObjectCreation);
         }
-
-        public static async Task<TextChange> GetTextChangeAsync(
-            CompletionService service, Document document, CompletionItem item, 
-            char? commitKey = null, CancellationToken cancellationToken = default(CancellationToken))
-        {
-            var change = await service.GetChangeAsync(document, item, commitKey, cancellationToken).ConfigureAwait(false);
-
-            // normally the items that produce multiple changes are not expecting to trigger the behaviors that rely on looking at the text
-            if (change.TextChanges.Length == 1)
-            {
-                return change.TextChanges[0];
-            }
-            else
-            {
-                return new TextChange(item.Span, item.DisplayText);
-            }
-        }
     }
 }
\ No newline at end of file

src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_ReturnKey.cs

 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
 using System;
+using Microsoft.CodeAnalysis.Completion;
 using Microsoft.CodeAnalysis.Editor.Commands;
 
 namespace Microsoft.CodeAnalysis.Editor.Implementation.IntelliSense.Completion
@@ -82,8 +83,6 @@ private void CommitOnEnter(out bool sendThrough, out bool committed)
                 return;
             }
 
-            var helper = GetCompletionHelper();
-
             if (sendThrough)
             {
                 // Get the text that the user has currently entered into the buffer
@@ -91,15 +90,37 @@ private void CommitOnEnter(out bool sendThrough, out bool committed)
                 var textTypedSoFar = model.GetCurrentTextInSnapshot(
                     viewSpan, this.TextView.TextSnapshot, this.GetCaretPointInViewBuffer());
 
-                var options = GetOptions();
-                if (options != null)
-                {
-                    sendThrough = helper.SendEnterThroughToEditor(model.SelectedItem.Item, textTypedSoFar, options);
-                }
+                var service = GetCompletionService();
+                sendThrough = SendEnterThroughToEditor(
+                     service.GetRules(), model.SelectedItem.Item, textTypedSoFar);
             }
 
             this.Commit(model.SelectedItem, model, commitChar: null);
             committed = true;
         }
+
+        /// <summary>
+        /// Internal for testing purposes only.
+        /// </summary>
+        internal static bool SendEnterThroughToEditor(CompletionRules rules, CompletionItem item, string textTypedSoFar)
+        {
+            var rule = item.Rules.EnterKeyRule;
+            if (rule == EnterKeyRule.Default)
+            {
+                rule = rules.DefaultEnterKeyRule;
+            }
+
+            switch (rule)
+            {
+                default:
+                case EnterKeyRule.Default:
+                case EnterKeyRule.Never:
+                    return false;
+                case EnterKeyRule.Always:
+                    return true;
+                case EnterKeyRule.AfterFullyTypedWord:
+                    return item.DisplayText == textTypedSoFar;
+            }
+        }
     }
 }

src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TypeChar.cs

@@ -311,14 +311,49 @@ private bool IsCommitCharacter(char ch)
                 return char.IsLetterOrDigit(ch);
             }
 
-            var helper = GetCompletionHelper();
-            if (helper != null)
+            var completionService = GetCompletionService();
+            var filterText = GetCurrentFilterText(model, model.SelectedItem.Item);
+            return IsCommitCharacter(
+                completionService.GetRules(), model.SelectedItem.Item, ch, filterText);
+        }
+
+        /// <summary>
+        /// Internal for testing purposes only.
+        /// </summary>
+        internal static bool IsCommitCharacter(
+            CompletionRules completionRules, CompletionItem item, char ch, string filterText)
+        {
+            // general rule: if the filtering text exactly matches the start of the item then it must be a filter character
+            if (TextTypedSoFarMatchesItem(item, ch, textTypedSoFar: filterText))
             {
-                var filterText = GetCurrentFilterText(model, model.SelectedItem.Item);
-                return helper.IsCommitCharacter(model.SelectedItem.Item, ch, filterText);
+                return false;
             }
 
-            return false;
+            foreach (var rule in item.Rules.CommitCharacterRules)
+            {
+                switch (rule.Kind)
+                {
+                    case CharacterSetModificationKind.Add:
+                        if (rule.Characters.Contains(ch))
+                        {
+                            return true;
+                        }
+                        continue;
+
+                    case CharacterSetModificationKind.Remove:
+                        if (rule.Characters.Contains(ch))
+                        {
+                            return false;
+                        }
+                        continue;
+
+                    case CharacterSetModificationKind.Replace:
+                        return rule.Characters.Contains(ch);
+                }
+            }
+
+            // Fall back to the default rules for this language's completion service.
+            return completionRules.DefaultCommitCharacters.IndexOf(ch) >= 0;
         }
 
         private bool IsFilterCharacter(char ch)
@@ -337,11 +372,49 @@ private bool IsFilterCharacter(char ch)
                 return char.IsLetterOrDigit(ch);
             }
 
-            var helper = GetCompletionHelper();
-            if (helper != null)
+            var filterText = GetCurrentFilterText(model, model.SelectedItem.Item);
+            return IsFilterCharacter(model.SelectedItem.Item, ch, filterText);
+        }
+
+        private static bool TextTypedSoFarMatchesItem(CompletionItem item, char ch, string textTypedSoFar)
+        {
+            var textTypedWithChar = textTypedSoFar + ch;
+            return item.DisplayText.StartsWith(textTypedWithChar, StringComparison.CurrentCultureIgnoreCase) ||
+                item.FilterText.StartsWith(textTypedWithChar, StringComparison.CurrentCultureIgnoreCase);
+        }
+
+        /// <summary>
+        /// Internal for testing purposes only.
+        /// </summary>
+        internal static bool IsFilterCharacter(CompletionItem item, char ch, string filterText)
+        {
+            // general rule: if the filtering text exactly matches the start of the item then it must be a filter character
+            if (TextTypedSoFarMatchesItem(item, ch, textTypedSoFar: filterText))
+            {
+                return false;
+            }
+
+            foreach (var rule in item.Rules.FilterCharacterRules)
             {
-                var filterText = GetCurrentFilterText(model, model.SelectedItem.Item);
-                return helper.IsFilterCharacter(model.SelectedItem.Item, ch, filterText);
+                switch (rule.Kind)
+                {
+                    case CharacterSetModificationKind.Add:
+                        if (rule.Characters.Contains(ch))
+                        {
+                            return true;
+                        }
+                        continue;
+
+                    case CharacterSetModificationKind.Remove:
+                        if (rule.Characters.Contains(ch))
+                        {
+                            return false;
+                        }
+                        continue;
+
+                    case CharacterSetModificationKind.Replace:
+                        return rule.Characters.Contains(ch);
+                }
             }
 
             return false;

src/EditorFeatures/Core/Implementation/IntelliSense/Completion/DescriptionModifyingPresentationItem.cs

@@ -7,6 +7,7 @@
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.Completion;
 using Microsoft.CodeAnalysis.Snippets;
+using Microsoft.CodeAnalysis.Text;
 
 namespace Microsoft.CodeAnalysis.Editor.Implementation.IntelliSense.Completion
 {
@@ -34,7 +35,7 @@ public override async Task<CompletionDescription> GetDescriptionAsync(Document d
             var description = await this.CompletionService.GetDescriptionAsync(document, this.Item, cancellationToken).ConfigureAwait(false);
             var parts = description.TaggedParts;
 
-            var change = await CompletionHelper.GetTextChangeAsync(this.CompletionService, document, this.Item, '\t').ConfigureAwait(false);
+            var change = await GetTextChangeAsync(this.CompletionService, document, this.Item, '\t').ConfigureAwait(false);
             var insertionText = change.NewText;
 
             var note = string.Empty;
@@ -55,5 +56,25 @@ public override async Task<CompletionDescription> GetDescriptionAsync(Document d
 
             return description.WithTaggedParts(parts);
         }
+
+        /// <summary>
+        /// Internal for testing purposes only.
+        /// </summary>
+        internal static async Task<TextChange> GetTextChangeAsync(
+            CompletionService service, Document document, CompletionItem item,
+            char? commitKey = null, CancellationToken cancellationToken = default(CancellationToken))
+        {
+            var change = await service.GetChangeAsync(document, item, commitKey, cancellationToken).ConfigureAwait(false);
+
+            // normally the items that produce multiple changes are not expecting to trigger the behaviors that rely on looking at the text
+            if (change.TextChanges.Length == 1)
+            {
+                return change.TextChanges[0];
+            }
+            else
+            {
+                return new TextChange(item.Span, item.DisplayText);
+            }
+        }
     }
 }

src/EditorFeatures/Test/Completion/AbstractCompletionProviderTests.cs

@@ -6,13 +6,12 @@
 using System.Security;
 using System.Threading;
 using System.Threading.Tasks;
-using System.Windows.Threading;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.Completion;
+using Microsoft.CodeAnalysis.Editor.Implementation.IntelliSense.Completion;
 using Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces;
 using Microsoft.CodeAnalysis.LanguageServices;
 using Microsoft.CodeAnalysis.Options;
-using Microsoft.CodeAnalysis.Shared.Extensions;
 using Microsoft.CodeAnalysis.Text;
 using Microsoft.VisualStudio.Language.Intellisense;
 using Microsoft.VisualStudio.Text;
@@ -313,7 +312,7 @@ private async Task VerifyCustomCommitProviderCheckResultsAsync(Document document
             {
                 var completionRules = GetCompletionHelper(document, service);
                 var textView = (await WorkspaceFixture.GetWorkspaceAsync()).Documents.Single().GetTextView();
-                VerifyCustomCommitWorker(customCommitCompletionProvider, firstItem, completionRules, textView, textBuffer, codeBeforeCommit, expectedCodeAfterCommit, commitChar);
+                VerifyCustomCommitWorker(service, customCommitCompletionProvider, firstItem, completionRules, textView, textBuffer, codeBeforeCommit, expectedCodeAfterCommit, commitChar);
             }
             else
             {
@@ -333,9 +332,7 @@ private async Task VerifyCustomCommitProviderCheckResultsAsync(Document document
             string actualExpectedCode = null;
             MarkupTestFile.GetPosition(expectedCodeAfterCommit, out actualExpectedCode, out expectedCaretPosition);
 
-            CompletionHelper completionRules = GetCompletionHelper(document, service);
-
-            if (commitChar.HasValue && !completionRules.IsCommitCharacter(completionItem, commitChar.Value, string.Empty))
+            if (commitChar.HasValue && !Controller.IsCommitCharacter(service.GetRules(), completionItem, commitChar.Value, string.Empty))
             {
                 Assert.Equal(codeBeforeCommit, actualExpectedCode);
                 return;
@@ -359,6 +356,7 @@ private async Task VerifyCustomCommitProviderCheckResultsAsync(Document document
         }
 
         internal virtual void VerifyCustomCommitWorker(
+            CompletionService service,
             ICustomCommitCompletionProvider customCommitCompletionProvider,
             CompletionItem completionItem,
             CompletionHelper completionRules,
@@ -372,7 +370,7 @@ private async Task VerifyCustomCommitProviderCheckResultsAsync(Document document
             string actualExpectedCode = null;
             MarkupTestFile.GetPosition(expectedCodeAfterCommit, out actualExpectedCode, out expectedCaretPosition);
 
-            if (commitChar.HasValue && !completionRules.IsCommitCharacter(completionItem, commitChar.Value, string.Empty))
+            if (commitChar.HasValue && !Controller.IsCommitCharacter(service.GetRules(), completionItem, commitChar.Value, string.Empty))
             {
                 Assert.Equal(codeBeforeCommit, actualExpectedCode);
                 return;
@@ -407,7 +405,8 @@ private async Task VerifyCustomCommitProviderCheckResultsAsync(Document document
             }
         }
 
-        private async Task VerifyProviderCommitCheckResultsAsync(Document document, int position, string itemToCommit, string expectedCodeAfterCommit, char? commitCharOpt, string textTypedSoFar)
+        private async Task VerifyProviderCommitCheckResultsAsync(
+            Document document, int position, string itemToCommit, string expectedCodeAfterCommit, char? commitCharOpt, string textTypedSoFar)
         {
             var workspace = await WorkspaceFixture.GetWorkspaceAsync();
             var textBuffer = workspace.Documents.Single().TextBuffer;
@@ -422,9 +421,10 @@ private async Task VerifyProviderCommitCheckResultsAsync(Document document, int
 
             var text = await document.GetTextAsync();
 
-            if (commitChar == '\t' || completionRules.IsCommitCharacter(firstItem, commitChar, textTypedSoFar))
+            if (commitChar == '\t' || Controller.IsCommitCharacter(service.GetRules(), firstItem, commitChar, textTypedSoFar))
             {
-                var textChange = CompletionHelper.GetTextChangeAsync(service, document, firstItem, commitChar).Result;
+                var textChange = await DescriptionModifyingPresentationItem.GetTextChangeAsync(
+                    service, document, firstItem, commitChar);
 
                 // Adjust TextChange to include commit character, so long as it isn't TAB.
                 if (commitChar != '\t')

src/EditorFeatures/VisualBasicTest/Completion/CompletionProviders/AbstractVisualBasicCompletionProviderTests.vb

@@ -3,6 +3,7 @@
 Imports System.Collections.Immutable
 Imports Microsoft.CodeAnalysis
 Imports Microsoft.CodeAnalysis.Completion
+Imports Microsoft.CodeAnalysis.Editor.Implementation.IntelliSense.Completion
 Imports Microsoft.CodeAnalysis.Editor.UnitTests.Completion
 Imports Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces
 Imports Microsoft.CodeAnalysis.Text
@@ -121,8 +122,7 @@ Namespace Microsoft.CodeAnalysis.Editor.VisualBasic.UnitTests.Completion.Complet
                 Dim completionList = Await GetCompletionListAsync(service, document, position, CompletionTrigger.Default)
                 Dim item = completionList.Items.First(Function(i) i.DisplayText.StartsWith(textTypedSoFar))
 
-                Dim helper = CompletionHelper.GetHelper(document, service)
-                Assert.Equal(expected, helper.SendEnterThroughToEditor(item, textTypedSoFar, document.Options))
+                Assert.Equal(expected, Controller.SendEnterThroughToEditor(service.GetRules(), item, textTypedSoFar))
             End Using
         End Function
 
@@ -145,11 +145,11 @@ Namespace Microsoft.CodeAnalysis.Editor.VisualBasic.UnitTests.Completion.Complet
                 Dim helper = CompletionHelper.GetHelper(document, service)
 
                 For Each ch In chars
-                    Assert.True(helper.IsCommitCharacter(item, ch, textTypedSoFar), $"Expected '{ch}' to be a commit character")
+                    Assert.True(Controller.IsCommitCharacter(service.GetRules(), item, ch, textTypedSoFar), $"Expected '{ch}' to be a commit character")
                 Next
 
                 Dim chr = "x"c
-                Assert.False(helper.IsCommitCharacter(item, chr, textTypedSoFar), $"Expected '{chr}' NOT to be a commit character")
+                Assert.False(Controller.IsCommitCharacter(service.GetRules(), item, chr, textTypedSoFar), $"Expected '{chr}' NOT to be a commit character")
             End Using
 
         End Function