Merge pull request #48987 from genlu/EnableImportCompletion

Port #43548 from master-vs-deps to master

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

@@ -26,16 +26,20 @@ public class ExtensionMethodImportCompletionProviderTests : AbstractCSharpComple
 
         private bool? ShowImportCompletionItemsOptionValue { get; set; } = true;
 
+        // -1 would disable timebox, whereas 0 means always timeout.
+        private int TimeoutInMilliseconds { get; set; } = -1;
+
         private bool IsExpandedCompletion { get; set; } = true;
 
-        private bool HideAdvancedMembers { get; set; } = false;
+        private bool HideAdvancedMembers { get; set; }
 
         protected override OptionSet WithChangedOptions(OptionSet options)
         {
             return options
                 .WithChangedOption(CompletionOptions.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp, ShowImportCompletionItemsOptionValue)
                 .WithChangedOption(CompletionServiceOptions.IsExpandedCompletion, IsExpandedCompletion)
-                .WithChangedOption(CompletionOptions.HideAdvancedMembers, LanguageNames.CSharp, HideAdvancedMembers);
+                .WithChangedOption(CompletionOptions.HideAdvancedMembers, LanguageNames.CSharp, HideAdvancedMembers)
+                .WithChangedOption(CompletionServiceOptions.TimeoutInMillisecondsForExtensionMethodImportCompletion, TimeoutInMilliseconds);
         }
 
         protected override TestComposition GetComposition()
@@ -1884,6 +1888,44 @@ await VerifyImportItemIsAbsentAsync(
             }
         }
 
+        [Fact, Trait(Traits.Feature, Traits.Features.Completion)]
+        public async Task TestTimeBox()
+        {
+            var file1 = @"
+using System;
+
+namespace Foo
+{
+    public static class ExtensionClass
+    {
+        public static bool ExtentionMethod(this int x)
+            => true;
+    }
+}";
+            var file2 = @"
+using System;
+
+namespace Baz
+{
+    public class Bat
+    {
+        public void M(int x)
+        {
+            x.$$
+        }
+    }
+}";
+
+            IsExpandedCompletion = false;
+            TimeoutInMilliseconds = 0; //timeout immediately
+            var markup = GetMarkup(file2, file1, ReferenceType.None);
+
+            await VerifyImportItemIsAbsentAsync(
+                 markup,
+                 "ExtentionMethod",
+                 inlineDescription: "Foo");
+        }
+
         private Task VerifyImportItemExistsAsync(string markup, string expectedItem, int glyph, string inlineDescription, string displayTextSuffix = null, string expectedDescriptionOrNull = null)
             => VerifyItemExistsAsync(markup, expectedItem, displayTextSuffix: displayTextSuffix, glyph: glyph, inlineDescription: inlineDescription, expectedDescriptionOrNull: expectedDescriptionOrNull);
 

src/EditorFeatures/Test2/IntelliSense/CSharpCompletionCommandHandlerTests.vb

@@ -6090,6 +6090,7 @@ namespace NS2
 
                 Dim workspace = state.Workspace
                 workspace.TryApplyChanges(workspace.CurrentSolution.WithOptions(workspace.Options _
+                    .WithChangedOption(CompletionServiceOptions.TimeoutInMillisecondsForExtensionMethodImportCompletion, -1) _
                     .WithChangedOption(CompletionOptions.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp, False)))
 
                 ' trigger completion with import completion disabled
@@ -6160,6 +6161,7 @@ namespace NS2
 
                 Dim workspace = state.Workspace
                 workspace.TryApplyChanges(workspace.CurrentSolution.WithOptions(workspace.Options _
+                    .WithChangedOption(CompletionServiceOptions.TimeoutInMillisecondsForExtensionMethodImportCompletion, -1) _
                     .WithChangedOption(CompletionOptions.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp, True)))
 
                 ' trigger completion with import completion enabled
@@ -6203,6 +6205,7 @@ namespace NS1
 
                 Dim workspace = state.Workspace
                 workspace.TryApplyChanges(workspace.CurrentSolution.WithOptions(workspace.Options _
+                    .WithChangedOption(CompletionServiceOptions.TimeoutInMillisecondsForExtensionMethodImportCompletion, -1) _
                     .WithChangedOption(CompletionOptions.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp, True)))
 
                 ' trigger completion with import completion enabled
@@ -6638,6 +6641,7 @@ namespace NS2
 
                 Dim workspace = state.Workspace
                 workspace.TryApplyChanges(workspace.CurrentSolution.WithOptions(workspace.Options _
+                    .WithChangedOption(CompletionServiceOptions.TimeoutInMillisecondsForExtensionMethodImportCompletion, -1) _
                     .WithChangedOption(CompletionOptions.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp, True)))
 
                 state.SendInvokeCompletionList()
@@ -6712,6 +6716,7 @@ namespace NS2
 
                 Dim workspace = state.Workspace
                 workspace.TryApplyChanges(workspace.CurrentSolution.WithOptions(workspace.Options _
+                    .WithChangedOption(CompletionServiceOptions.TimeoutInMillisecondsForExtensionMethodImportCompletion, -1) _
                     .WithChangedOption(CompletionOptions.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp, True)))
 
                 state.SendInvokeCompletionList()
@@ -6786,6 +6791,7 @@ namespace NS2
 
                 Dim workspace = state.Workspace
                 workspace.TryApplyChanges(workspace.CurrentSolution.WithOptions(workspace.Options _
+                    .WithChangedOption(CompletionServiceOptions.TimeoutInMillisecondsForExtensionMethodImportCompletion, -1) _
                     .WithChangedOption(CompletionOptions.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp, True)))
 
                 state.SendInvokeCompletionList()
@@ -6860,6 +6866,7 @@ namespace NS2
 
                 Dim workspace = state.Workspace
                 workspace.TryApplyChanges(workspace.CurrentSolution.WithOptions(workspace.Options _
+                    .WithChangedOption(CompletionServiceOptions.TimeoutInMillisecondsForExtensionMethodImportCompletion, -1) _
                     .WithChangedOption(CompletionOptions.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp, True)))
 
                 state.SendInvokeCompletionList()
@@ -6935,6 +6942,7 @@ namespace NS2
 
                 Dim workspace = state.Workspace
                 workspace.TryApplyChanges(workspace.CurrentSolution.WithOptions(workspace.Options _
+                    .WithChangedOption(CompletionServiceOptions.TimeoutInMillisecondsForExtensionMethodImportCompletion, -1) _
                     .WithChangedOption(CompletionOptions.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp, True)))
 
                 state.SendInvokeCompletionList()
@@ -6980,6 +6988,7 @@ namespace OtherNS
 
                 Dim workspace = state.Workspace
                 workspace.TryApplyChanges(workspace.CurrentSolution.WithOptions(workspace.Options _
+                    .WithChangedOption(CompletionServiceOptions.TimeoutInMillisecondsForExtensionMethodImportCompletion, -1) _
                     .WithChangedOption(CompletionOptions.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp, True)))
 
                 state.SendInvokeCompletionList()
@@ -7099,6 +7108,7 @@ namespace NS2
                 Await state.AssertNoCompletionSession()
 
                 workspace.TryApplyChanges(workspace.CurrentSolution.WithOptions(workspace.Options _
+                    .WithChangedOption(CompletionServiceOptions.TimeoutInMillisecondsForExtensionMethodImportCompletion, -1) _
                     .WithChangedOption(CompletionOptions.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp, True)))
 
                 state.SendTypeChars("mytask")

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

@@ -14,14 +14,16 @@ Namespace Microsoft.CodeAnalysis.Editor.VisualBasic.UnitTests.Completion.Complet
 
         Private Property IsExpandedCompletion As Boolean = True
 
-        Private Property ShowImportCompletionItemsOptionValue As Boolean = True
-
         ' -1 would disable timebox, whereas 0 means always timeout.
         Private Property TimeoutInMilliseconds As Integer = -1
 
+        Private Property ShowImportCompletionItemsOptionValue As Boolean = True
+
         Protected Overrides Function WithChangedOptions(options As OptionSet) As OptionSet
             Return options _
-                .WithChangedOption(CompletionOptions.ShowItemsFromUnimportedNamespaces, LanguageNames.VisualBasic, ShowImportCompletionItemsOptionValue).WithChangedOption(CompletionServiceOptions.IsExpandedCompletion, IsExpandedCompletion)
+                .WithChangedOption(CompletionOptions.ShowItemsFromUnimportedNamespaces, LanguageNames.VisualBasic, ShowImportCompletionItemsOptionValue) _
+                .WithChangedOption(CompletionServiceOptions.IsExpandedCompletion, IsExpandedCompletion) _
+                .WithChangedOption(CompletionServiceOptions.TimeoutInMillisecondsForExtensionMethodImportCompletion, TimeoutInMilliseconds)
         End Function
 
         Protected Overrides Function GetComposition() As TestComposition

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

@@ -2,8 +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.
 
-#nullable disable
-
 using Microsoft.CodeAnalysis.Options;
 
 namespace Microsoft.CodeAnalysis.Completion
@@ -21,5 +19,12 @@ public static readonly Option2<bool> IsExpandedCompletion
         /// </summary>
         public static readonly Option2<bool> DisallowAddingImports
             = new(nameof(CompletionServiceOptions), nameof(DisallowAddingImports), defaultValue: false);
+
+        /// <summary>
+        /// Timeout value used for time-boxing completion of unimported extension methods.
+        /// Value less than 0 means no timebox; value == 0 means immediate timeout (for testing purpose)
+        /// </summary>
+        public static readonly Option2<int> TimeoutInMillisecondsForExtensionMethodImportCompletion
+            = new(nameof(CompletionServiceOptions), nameof(TimeoutInMillisecondsForExtensionMethodImportCompletion), defaultValue: 500);
     }
 }

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

@@ -31,8 +31,6 @@ namespace Microsoft.CodeAnalysis.Completion
     /// </summary>
     public abstract partial class CompletionServiceWithProviders : CompletionService, IEqualityComparer<ImmutableHashSet<string>>
     {
-        private static readonly Func<string, List<CompletionItem>> s_createList = _ => new List<CompletionItem>();
-
         private readonly object _gate = new();
 
         private readonly ConditionalWeakTable<IReadOnlyList<AnalyzerReference>, StrongBox<ImmutableArray<CompletionProvider>>> _projectCompletionProvidersMap
@@ -436,8 +434,7 @@ private CompletionList MergeAndPruneCompletionLists(
             // changed it 'wins' and picks the span that will be used for all items in the completion
             // list.  If no contexts changed it, then just use the default span provided by the service.
             var finalCompletionListSpan = completionContexts.FirstOrDefault(c => c.CompletionListSpan != defaultSpan)?.CompletionListSpan ?? defaultSpan;
-
-            var displayNameToItemsMap = new Dictionary<string, List<CompletionItem>>();
+            using var displayNameToItemsMap = new DisplayNameToItemsMap(this);
             CompletionItem suggestionModeItem = null;
 
             foreach (var context in completionContexts)
@@ -447,55 +444,31 @@ private CompletionList MergeAndPruneCompletionLists(
                 foreach (var item in context.Items)
                 {
                     Debug.Assert(item != null);
-                    AddToDisplayMap(item, displayNameToItemsMap);
+                    displayNameToItemsMap.Add(item);
                 }
 
                 // first one wins
                 suggestionModeItem ??= context.SuggestionModeItem;
             }
 
-            if (displayNameToItemsMap.Count == 0)
+            if (displayNameToItemsMap.IsEmpty)
             {
                 return CompletionList.Empty;
             }
 
             // TODO(DustinCa): Revisit performance of this.
-            var totalItems = displayNameToItemsMap.Values.Flatten().ToList();
-            totalItems.Sort();
+            using var _ = ArrayBuilder<CompletionItem>.GetInstance(displayNameToItemsMap.Count, out var builder);
+            builder.AddRange(displayNameToItemsMap);
+            builder.Sort();
 
             return CompletionList.Create(
                 finalCompletionListSpan,
-                totalItems.ToImmutableArray(),
+                builder.ToImmutable(),
                 GetRules(),
                 suggestionModeItem,
                 isExclusive);
         }
 
-        private void AddToDisplayMap(
-            CompletionItem item,
-            Dictionary<string, List<CompletionItem>> displayNameToItemsMap)
-        {
-            var sameNamedItems = displayNameToItemsMap.GetOrAdd(item.GetEntireDisplayText(), s_createList);
-
-            // If two items have the same display text choose which one to keep.
-            // If they don't actually match keep both.
-
-            for (var i = 0; i < sameNamedItems.Count; i++)
-            {
-                var existingItem = sameNamedItems[i];
-
-                Debug.Assert(item.GetEntireDisplayText() == existingItem.GetEntireDisplayText());
-
-                if (ItemsMatch(item, existingItem))
-                {
-                    sameNamedItems[i] = GetBetterItem(item, existingItem);
-                    return;
-                }
-            }
-
-            sameNamedItems.Add(item);
-        }
-
         /// <summary>
         /// Determines if the items are similar enough they should be represented by a single item in the list.
         /// </summary>
@@ -650,6 +623,100 @@ int IEqualityComparer<ImmutableHashSet<string>>.GetHashCode(ImmutableHashSet<str
             return hash;
         }
 
+        private class DisplayNameToItemsMap : IEnumerable<CompletionItem>, IDisposable
+        {
+            // We might need to handle large amount of items with import completion enabled,
+            // so use a dedicated pool to minimize array allocations.
+            // Set the size of pool to a small number 5 because we don't expect more than a
+            // couple of callers at the same time.
+            private static readonly ObjectPool<Dictionary<string, object>> s_uniqueSourcesPool
+                = new(factory: () => new(), size: 5);
+
+            private readonly Dictionary<string, object> _displayNameToItemsMap;
+            private readonly CompletionServiceWithProviders _service;
+
+            public int Count { get; private set; }
+
+            public DisplayNameToItemsMap(CompletionServiceWithProviders service)
+            {
+                _service = service;
+                _displayNameToItemsMap = s_uniqueSourcesPool.Allocate();
+            }
+
+            public void Dispose()
+            {
+                _displayNameToItemsMap.Clear();
+                s_uniqueSourcesPool.Free(_displayNameToItemsMap);
+            }
+
+            public bool IsEmpty => _displayNameToItemsMap.Count == 0;
+
+            public void Add(CompletionItem item)
+            {
+                var entireDisplayText = item.GetEntireDisplayText();
+
+                if (!_displayNameToItemsMap.TryGetValue(entireDisplayText, out var value))
+                {
+                    Count++;
+                    _displayNameToItemsMap.Add(entireDisplayText, item);
+                    return;
+                }
+
+                // If two items have the same display text choose which one to keep.
+                // If they don't actually match keep both.
+                if (value is CompletionItem sameNamedItem)
+                {
+                    if (_service.ItemsMatch(item, sameNamedItem))
+                    {
+                        _displayNameToItemsMap[entireDisplayText] = _service.GetBetterItem(item, sameNamedItem);
+                        return;
+                    }
+
+                    Count++;
+                    // Matching items should be rare, no need to use object pool for this.
+                    _displayNameToItemsMap[entireDisplayText] = new List<CompletionItem>() { sameNamedItem, item };
+                }
+                else if (value is List<CompletionItem> sameNamedItems)
+                {
+                    for (var i = 0; i < sameNamedItems.Count; i++)
+                    {
+                        var existingItem = sameNamedItems[i];
+                        if (_service.ItemsMatch(item, existingItem))
+                        {
+                            sameNamedItems[i] = _service.GetBetterItem(item, existingItem);
+                            return;
+                        }
+                    }
+
+                    Count++;
+                    sameNamedItems.Add(item);
+                }
+            }
+
+            public IEnumerator<CompletionItem> GetEnumerator()
+            {
+                foreach (var value in _displayNameToItemsMap.Values)
+                {
+                    if (value is CompletionItem sameNamedItem)
+                    {
+                        yield return sameNamedItem;
+                    }
+                    else if (value is List<CompletionItem> sameNamedItems)
+                    {
+                        foreach (var item in sameNamedItems)
+                        {
+                            yield return item;
+                        }
+                    }
+                }
+            }
+
+            System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator()
+            {
+                return GetEnumerator();
+            }
+        }
+
         internal TestAccessor GetTestAccessor()
             => new(this);
 

src/Features/Core/Portable/Completion/Log/CompletionProvidersLogger.cs

@@ -37,6 +37,7 @@ internal enum ActionInfo
             ExtensionMethodCompletionCreateItemsTicks,
             CommitsOfExtensionMethodImportCompletionItem,
             ExtensionMethodCompletionPartialResultCount,
+            ExtensionMethodCompletionTimeoutCount,
         }
 
         internal static void LogTypeImportCompletionTicksDataPoint(int count)
@@ -81,6 +82,9 @@ internal static void LogCommitOfExtensionMethodImportCompletionItem() =>
         internal static void LogExtensionMethodCompletionPartialResultCount() =>
             s_logAggregator.IncreaseCount((int)ActionInfo.ExtensionMethodCompletionPartialResultCount);
 
+        internal static void LogExtensionMethodCompletionTimeoutCount() =>
+            s_logAggregator.IncreaseCount((int)ActionInfo.ExtensionMethodCompletionTimeoutCount);
+
         internal static void ReportTelemetry()
         {
             Logger.Log(FunctionId.Intellisense_CompletionProviders_Data, KeyValueLogMessage.Create(m =>