diff --git a/src/EditorFeatures/Core/IntelliSense/AsyncCompletion/CompletionSource.cs b/src/EditorFeatures/Core/IntelliSense/AsyncCompletion/CompletionSource.cs index 83b13b75663a4..6b71911bec093 100644 --- a/src/EditorFeatures/Core/IntelliSense/AsyncCompletion/CompletionSource.cs +++ b/src/EditorFeatures/Core/IntelliSense/AsyncCompletion/CompletionSource.cs @@ -194,7 +194,7 @@ private bool TryInvokeSnippetCompletion( } var syntaxFacts = services.GetService(); - // Snippets are included if the user types: + // Snippets are included if the user types: // If at least one condition for snippets do not hold, bail out. if (syntaxFacts == null || caretPoint < 3 || @@ -240,10 +240,14 @@ public async Task GetCompletionContextAsync( // #1 is the essence of completion so we'd always wait until its task is completed and return the results. However, because we have // a really tight perf budget in completion, and computing those items in #2 could be expensive especially in a large solution // (e.g. requires syntax/symbol indices and/or runs in OOP,) we decide to kick off the computation in parallel when completion is - // triggered, but only include its results if it's completed by the time task #1 is completed, otherwise we don't wait on it and - // just return items from #1 immediately. Task #2 will still be running in the background (until session is dismissed/committed,) - // and we'd check back to see if it's completed whenever we have a chance to update the completion list, i.e. when user typed another - // character, a filter was selected, etc. If so, those items will be added as part of the refresh. + // triggered, but only include its results if: + // + // (a) it's completed by the time task #1 is completed and + // (b) including them won't interfere with users' ability to browse the list (e.g. when the list is too long since filter text is short) + // + // Otherwise we don't wait on it and return items from #1 immediately. Task #2 will still be running in the background + // (until session is dismissed/committed) and we'd check back to see if it's completed whenever we have a chance to update the completion list, + // i.e. when user typed another character, a filter was selected, etc. If so, those items will be added as part of the refresh. // // The reason of adopting this approach is we want to minimize typing delays. There are two ways user might perceive a delay in typing. // First, they could see a delay between typing a character and completion list being displayed if they want to examine the items available. @@ -254,12 +258,12 @@ public async Task GetCompletionContextAsync( // // This approach would ensure the computation of #2 will not be the cause of such delays, with the obvious trade off of potentially not providing // expanded items until later (or never) in a completion session even if the feature is enabled. Note that in most cases we'd expect task #2 to finish - // in time and complete result would be available from the start of the session. However, even in the case only partial result is returned at the start, - // we still believe this is acceptable given how critical perf is in typing scenario. + // in time and complete result would be available when it's most likely needed (see `ShouldHideExpandedItems` helper in ItemManager for details.) + // However, even in the case only partial result is returned at the start, we still believe this is acceptable given how critical perf is in typing scenario. // Additionally, expanded items are usually considered complementary. The need for them only rise occasionally (it's rare when users need to add imports,) // and when they are needed, our hypothesis is because of their more intrusive nature (adding an import to the document) users would more likely to // contemplate such action thus typing slower before commit and/or spending more time examining the list, which give us some opportunities - // to still provide those items later before they are truly required. + // to still provide those items later before they are truly required. var showCompletionItemFilters = _editorOptionsService.GlobalOptions.GetOption(CompletionViewOptionsStorage.ShowCompletionItemFilters, document.Project.Language); var options = _editorOptionsService.GlobalOptions.GetCompletionOptions(document.Project.Language) with @@ -296,7 +300,7 @@ public async Task GetCompletionContextAsync( // If the core items are exclusive, we don't include expanded items. // This would cancel expandedItemsTask. if (sessionData.IsExclusive) - { + { _ = _expandedItemsTaskCancellationSeries.CreateNext(CancellationToken.None); } else @@ -345,26 +349,26 @@ public async Task GetExpandedCompletionContextAsync( if (sessionData.CombinedSortedList is null) { - // We only reach here when expanded items are disabled, but user requested them explicitly via expander. - // In this case, enable expanded items and trigger the completion only for them. - var document = initialTriggerLocation.Snapshot.GetOpenDocumentInCurrentContextWithChanges(); - if (document != null) - { - // User selected expander explicitly, which means we need to collect and return - // items from unimported namespace (and only those items) regardless of whether it's enabled. - var options = _editorOptionsService.GlobalOptions.GetCompletionOptions(document.Project.Language) with + // We only reach here when expanded items are disabled, but user requested them explicitly via expander. + // In this case, enable expanded items and trigger the completion only for them. + var document = initialTriggerLocation.Snapshot.GetOpenDocumentInCurrentContextWithChanges(); + if (document != null) { - ShowItemsFromUnimportedNamespaces = true, - ExpandedCompletionBehavior = ExpandedCompletionMode.ExpandedItemsOnly - }; + // User selected expander explicitly, which means we need to collect and return + // items from unimported namespace (and only those items) regardless of whether it's enabled. + var options = _editorOptionsService.GlobalOptions.GetCompletionOptions(document.Project.Language) with + { + ShowItemsFromUnimportedNamespaces = true, + ExpandedCompletionBehavior = ExpandedCompletionMode.ExpandedItemsOnly + }; - var (context, completionList) = await GetCompletionContextWorkerAsync(session, document, initialTrigger, initialTriggerLocation, options, cancellationToken).ConfigureAwait(false); - UpdateSessionData(session, sessionData, completionList, initialTriggerLocation); + var (context, completionList) = await GetCompletionContextWorkerAsync(session, document, initialTrigger, initialTriggerLocation, options, cancellationToken).ConfigureAwait(false); + UpdateSessionData(session, sessionData, completionList, initialTriggerLocation); - return context; + return context; + } } } - } return VSCompletionContext.Empty; } diff --git a/src/EditorFeatures/Core/IntelliSense/AsyncCompletion/ItemManager.CompletionListUpdater.cs b/src/EditorFeatures/Core/IntelliSense/AsyncCompletion/ItemManager.CompletionListUpdater.cs index 76729b8579339..3219cdae38211 100644 --- a/src/EditorFeatures/Core/IntelliSense/AsyncCompletion/ItemManager.CompletionListUpdater.cs +++ b/src/EditorFeatures/Core/IntelliSense/AsyncCompletion/ItemManager.CompletionListUpdater.cs @@ -234,12 +234,6 @@ private bool ShouldDismissCompletionListImmediately() } return false; - - static bool IsAfterDot(ITextSnapshot snapshot, ITrackingSpan applicableToSpan) - { - var position = applicableToSpan.GetStartPoint(snapshot).Position; - return position > 0 && snapshot[position - 1] == '.'; - } } private void AddCompletionItems(List list, ThreadLocal threadLocalPatternMatchHelper, CancellationToken cancellationToken) diff --git a/src/EditorFeatures/Core/IntelliSense/AsyncCompletion/ItemManager.cs b/src/EditorFeatures/Core/IntelliSense/AsyncCompletion/ItemManager.cs index ec7633797068c..0478b39d872eb 100644 --- a/src/EditorFeatures/Core/IntelliSense/AsyncCompletion/ItemManager.cs +++ b/src/EditorFeatures/Core/IntelliSense/AsyncCompletion/ItemManager.cs @@ -11,6 +11,7 @@ using Microsoft.CodeAnalysis.Options; using Microsoft.VisualStudio.Language.Intellisense.AsyncCompletion; using Microsoft.VisualStudio.Language.Intellisense.AsyncCompletion.Data; +using Microsoft.VisualStudio.Text; using Roslyn.Utilities; using RoslynCompletionItem = Microsoft.CodeAnalysis.Completion.CompletionItem; @@ -20,9 +21,17 @@ namespace Microsoft.CodeAnalysis.Editor.Implementation.IntelliSense.AsyncComplet { internal sealed partial class ItemManager : IAsyncCompletionItemManager2 { + /// + /// The threshold for us to consider exclude (potentially large amount of) expanded items from completion list. + /// Showing a large amount of expanded items to user would introduce noise and render the list too long to browse. + /// Not processing those expanded items also has perf benefit (e.g. matching and highlighting could be expensive.) + /// We set it to 2 because it's common to use filter of length 2 for camel case match, e.g. `AB` for `ArrayBuiler`. + /// + internal const int FilterTextLengthToExcludeExpandedItemsExclusive = 2; + private readonly RecentItemsManager _recentItemsManager; private readonly EditorOptionsService _editorOptionsService; - + internal ItemManager(RecentItemsManager recentItemsManager, EditorOptionsService editorOptionsService) { _recentItemsManager = recentItemsManager; @@ -93,7 +102,7 @@ private static SegmentedList SortCompletionItems(AsyncCompleti data = new AsyncCompletionSessionDataSnapshot(sessionData.CombinedSortedList, data.Snapshot, data.Trigger, data.InitialTrigger, data.SelectedFilters, data.IsSoftSelected, data.DisplaySuggestionItem, data.Defaults); } - else if (sessionData.ExpandedItemsTask != null) + else if (!ShouldHideExpandedItems() && sessionData.ExpandedItemsTask is not null) { var task = sessionData.ExpandedItemsTask; @@ -133,6 +142,21 @@ private static SegmentedList SortCompletionItems(AsyncCompleti { AsyncCompletionLogger.LogItemManagerUpdateDataPoint(stopwatch.Elapsed, isCanceled: cancellationToken.IsCancellationRequested); } + + // Don't hide expanded items if all these conditions are met: + // 1. filter text length >= 2 (it's common to use filter of length 2 for camel case match, e.g. `AB` for `ArrayBuiler`) + // 2. the completion is triggered in the context of listing members (it usually has much fewer items and more often used for browsing purpose) + // 3. defaults is not empty (it might suggests an expanded item) + bool ShouldHideExpandedItems() + => session.ApplicableToSpan.GetText(data.Snapshot).Length < FilterTextLengthToExcludeExpandedItemsExclusive + && !IsAfterDot(data.Snapshot, session.ApplicableToSpan) + && data.Defaults.IsEmpty; + } + + private static bool IsAfterDot(ITextSnapshot snapshot, ITrackingSpan applicableToSpan) + { + var position = applicableToSpan.GetStartPoint(snapshot).Position; + return position > 0 && snapshot[position - 1] == '.'; } private sealed class VSItemComparer : IComparer diff --git a/src/EditorFeatures/Test2/IntelliSense/CSharpCompletionCommandHandlerTests.vb b/src/EditorFeatures/Test2/IntelliSense/CSharpCompletionCommandHandlerTests.vb index 1057e5227e90a..622cebf979074 100644 --- a/src/EditorFeatures/Test2/IntelliSense/CSharpCompletionCommandHandlerTests.vb +++ b/src/EditorFeatures/Test2/IntelliSense/CSharpCompletionCommandHandlerTests.vb @@ -6860,7 +6860,7 @@ namespace NS1 { void M() { - $$ + Un$$ } } } @@ -7111,7 +7111,6 @@ public class AA Await state.SendInvokeCompletionListAndWaitForUiRenderAsync() - ' import completion is disabled, so we shouldn't have expander selected by default state.AssertCompletionItemExpander(isAvailable:=True, isSelected:=False) Await state.AssertCompletionItemsContain("DDProp1", "") Await state.AssertCompletionItemsDoNotContainAny("DD") @@ -7954,7 +7953,7 @@ namespace NS1 { void M() { - $$ + unimportedtype$$ } } } @@ -7973,16 +7972,7 @@ namespace <%= containingNamespace %> state.Workspace.GlobalOptions.SetGlobalOption(CompletionOptionsStorage.ForceExpandedCompletionIndexCreation, True) state.Workspace.GlobalOptions.SetGlobalOption(CompletionOptionsStorage.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp, True) - Await state.SendInvokeCompletionListAndWaitForUiRenderAsync() - - ' make sure expander is selected - state.AssertCompletionItemExpander(isAvailable:=True, isSelected:=True) - - state.SendEscape() - Await state.AssertNoCompletionSession() - - state.SendTypeChars("unimportedtype") - Await state.WaitForAsynchronousOperationsAsync() + Await state.SendCommitUniqueCompletionListItemAsync() ' make sure expander is selected state.AssertCompletionItemExpander(isAvailable:=True, isSelected:=True) @@ -8021,7 +8011,7 @@ namespace NS1 { void M() { - $$ + mytask$$ } } @@ -8042,16 +8032,7 @@ namespace NS2 state.Workspace.GlobalOptions.SetGlobalOption(CompletionOptionsStorage.ForceExpandedCompletionIndexCreation, True) state.Workspace.GlobalOptions.SetGlobalOption(CompletionOptionsStorage.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp, True) - Await state.SendInvokeCompletionListAndWaitForUiRenderAsync() - - ' make sure expander is selected - state.AssertCompletionItemExpander(isAvailable:=True, isSelected:=True) - - state.SendEscape() - Await state.AssertNoCompletionSession() - - state.SendTypeChars("mytask") - Await state.WaitForAsynchronousOperationsAsync() + Await state.SendCommitUniqueCompletionListItemAsync() ' make sure expander is selected state.AssertCompletionItemExpander(isAvailable:=True, isSelected:=True) @@ -10449,7 +10430,7 @@ class C Public Async Function TestNonBlockingExpandCompletionDoesNotChangeItemOrder() As Task Using state = TestStateFactory.CreateCSharpTestState( - $$ + Test$$ , extraExportedTypes:={GetType(TestProvider)}.ToList()) @@ -10482,7 +10463,7 @@ class C ' Now delayed expand item task is completed, following up by typing and delete a character to trigger ' update so the list would contains all items - Await state.SendTypeCharsAndWaitForUiRenderAsync("t") + Await state.SendTypeCharsAndWaitForUiRenderAsync("U") Await state.AssertCompletionItemsContain("TestUnimportedItem", "") state.AssertCompletionItemExpander(isAvailable:=True, isSelected:=True) @@ -10496,7 +10477,7 @@ class C state.SendEscape() Await state.AssertNoCompletionSession() - ' Now disable expand item delay, so intial trigger should contain full list + ' Now disable expand item delay, so initial trigger should contain full list state.TextView.Options.SetOptionValue(DefaultOptions.ResponsiveCompletionOptionId, False) state.SendInvokeCompletionList() diff --git a/src/EditorFeatures/Test2/IntelliSense/CSharpCompletionCommandHandlerTests_DefaultsSource.vb b/src/EditorFeatures/Test2/IntelliSense/CSharpCompletionCommandHandlerTests_DefaultsSource.vb index 32467d086f0fd..a05f6b8a780ab 100644 --- a/src/EditorFeatures/Test2/IntelliSense/CSharpCompletionCommandHandlerTests_DefaultsSource.vb +++ b/src/EditorFeatures/Test2/IntelliSense/CSharpCompletionCommandHandlerTests_DefaultsSource.vb @@ -4,11 +4,16 @@ Imports System.Collections.Immutable Imports System.Composition +Imports System.Threading Imports Microsoft.CodeAnalysis.Completion Imports Microsoft.CodeAnalysis.Editor.Implementation.IntelliSense.AsyncCompletion Imports Microsoft.CodeAnalysis.Host.Mef Imports Microsoft.CodeAnalysis.Options Imports Microsoft.CodeAnalysis.Text +Imports Microsoft.CodeAnalysis.Shared.TestHooks +Imports Microsoft.CodeAnalysis.PooledObjects +Imports Microsoft.CodeAnalysis.CSharp.Completion +Imports Microsoft.CodeAnalysis.Completion.Providers Imports Microsoft.VisualStudio.Language.Intellisense.AsyncCompletion Imports Microsoft.VisualStudio.Text.Editor @@ -469,5 +474,215 @@ class Test Assert.Contains("if", committedLine, StringComparison.Ordinal) End Using End Function + + + + + Public Async Function TestAutoHidingExpandedItems(afterDot As Boolean, responsiveTypingEnabled As Boolean) As Task + Dim text As String + If afterDot Then + text = "this." + Else + text = String.Empty + End If + + Using state = TestStateFactory.CreateCSharpTestState( + + <%= text %>$$ + , + excludedTypes:={GetType(CSharpCompletionService.Factory)}.ToList(), + extraExportedTypes:={GetType(MockCompletionServiceFactory)}.ToList()) + + Dim workspace = state.Workspace + workspace.GlobalOptions.SetGlobalOption(CompletionOptionsStorage.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp, True) + + state.TextView.Options.SetOptionValue(DefaultOptions.ResponsiveCompletionOptionId, responsiveTypingEnabled) + + Dim completionService = DirectCast(workspace.Services.GetLanguageServices(LanguageNames.CSharp).GetRequiredService(Of CompletionService)(), MockCompletionServiceFactory.Service) + + If Not responsiveTypingEnabled And afterDot Then + ' we'd block on expanded items when responsive completion is disabled and triggered after a dot + ' so make sure the checkpoint is released first + completionService.ExpandedItemsCheckpoint.Release() + + Await state.SendInvokeCompletionListAndWaitForUiRenderAsync() + state.AssertCompletionItemExpander(isAvailable:=True, isSelected:=True) + Dim items = state.GetCompletionItems() + Assert.Equal(completionService.RegularCount + completionService.ExpandedCount, items.Count) + Assert.Equal(completionService.ExpandedCount, items.Where(Function(x) x.Flags.IsExpanded()).Count()) + Else + ' we blocked the expanded item task, so only regular items in the list + ' this is true even with responsive typing disabled, since the filter text is empty. + Await state.SendInvokeCompletionListAndWaitForUiRenderAsync() + state.AssertCompletionItemExpander(isAvailable:=True, isSelected:=False) + Dim items = state.GetCompletionItems() + Assert.Equal(completionService.RegularCount, items.Count) + Assert.False(items.Any(Function(i) i.Flags.IsExpanded())) + + ' make sure expanded item task completes + completionService.ExpandedItemsCheckpoint.Release() + Dim session = Await state.GetCompletionSession() + Dim sessionData = CompletionSessionData.GetOrCreateSessionData(session) + Assert.NotNull(sessionData.ExpandedItemsTask) + Await sessionData.ExpandedItemsTask + End If + + Dim typedChars = "Item" + For i = 0 To typedChars.Length - 1 + Await state.SendTypeCharsAndWaitForUiRenderAsync(typedChars(i)) + Dim items = state.GetCompletionItems() + Dim expandedCount As Integer + + If (Not afterDot And i < ItemManager.FilterTextLengthToExcludeExpandedItemsExclusive - 1) Then + expandedCount = 0 + Else + expandedCount = completionService.ExpandedCount + End If + + Assert.True((completionService.RegularCount + expandedCount) = items.Count, $"Typed char: '{typedChars(i)}', expected: {completionService.RegularCount + expandedCount}, actual: {items.Count} ") + Assert.Equal(expandedCount, items.Where(Function(x) x.Flags.IsExpanded()).Count()) + Next + + ' once we show expanded items, we will not hide it in the same session, even if filter text became shorter + For i = 1 To 4 + state.SendBackspace() + Dim items = state.GetCompletionItems() + Assert.True((completionService.RegularCount + completionService.ExpandedCount) = items.Count, $"Backspace number: {i}expected: {completionService.RegularCount + completionService.ExpandedCount}, actual: {items.Count} ") + Assert.Equal(completionService.ExpandedCount, items.Where(Function(x) x.Flags.IsExpanded()).Count()) + Next + End Using + End Function + + + Public Async Function TestAutoHidingExpandedItemsWithDefaults() As Task + Using state = TestStateFactory.CreateCSharpTestState( + +$$ + , + excludedTypes:={GetType(CSharpCompletionService.Factory)}.ToList(), + extraExportedTypes:={GetType(MockCompletionServiceFactory), GetType(MockDefaultSource)}.ToList()) + + Dim workspace = state.Workspace + workspace.GlobalOptions.SetGlobalOption(CompletionOptionsStorage.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp, True) + state.TextView.Options.SetOptionValue(DefaultOptions.ResponsiveCompletionOptionId, True) + + Dim completionService = DirectCast(workspace.Services.GetLanguageServices(LanguageNames.CSharp).GetRequiredService(Of CompletionService)(), MockCompletionServiceFactory.Service) + MockDefaultSource.Defaults = ImmutableArray.Create("ItemExpanded1") + + ' we blocked the expanded item task, so only regular items in the list + Await state.SendInvokeCompletionListAndWaitForUiRenderAsync() + state.AssertCompletionItemExpander(isAvailable:=True, isSelected:=False) + Dim items = state.GetCompletionItems() + Assert.Equal(completionService.RegularCount, items.Count) + Assert.False(items.Any(Function(i) i.Flags.IsExpanded())) + + ' make sure expanded item task completes + completionService.ExpandedItemsCheckpoint.Release() + Dim session = Await state.GetCompletionSession() + Dim sessionData = CompletionSessionData.GetOrCreateSessionData(session) + Assert.NotNull(sessionData.ExpandedItemsTask) + Await sessionData.ExpandedItemsTask + + Await state.SendInvokeCompletionListAndWaitForUiRenderAsync() + Await state.AssertSelectedCompletionItem("★ ItemExpanded1", isHardSelected:=False) + End Using + End Function + + + Private Class MockCompletionServiceFactory + Implements ILanguageServiceFactory + + Private ReadOnly _listenerProvider As IAsynchronousOperationListenerProvider + + + + Public Sub New(listenerProvider As IAsynchronousOperationListenerProvider) + _listenerProvider = listenerProvider + End Sub + + Public Function CreateLanguageService(languageServices As CodeAnalysis.Host.HostLanguageServices) As CodeAnalysis.Host.ILanguageService Implements ILanguageServiceFactory.CreateLanguageService + Return New Service(languageServices.LanguageServices.SolutionServices, _listenerProvider) + End Function + + Public Class Service + Inherits CompletionService + + Public Property RegularCount As Integer = 10 + Public Property ExpandedCount As Integer = 10 + + Public ExpandedItemsCheckpoint As New Checkpoint() + + Public Sub New(services As CodeAnalysis.Host.SolutionServices, listenerProvider As IAsynchronousOperationListenerProvider) + MyBase.New(services, listenerProvider) + End Sub + + Public Overrides ReadOnly Property Language As String + Get + Return LanguageNames.CSharp + End Get + End Property + + Friend Overrides Function GetRules(options As CompletionOptions) As CompletionRules + Return CompletionRules.Default + End Function + + Friend Overrides Async Function GetCompletionsAsync(document As Document, + caretPosition As Integer, + options As CompletionOptions, + passThroughOptions As OptionSet, + Optional trigger As CompletionTrigger = Nothing, + Optional roles As ImmutableHashSet(Of String) = Nothing, + Optional cancellationToken As CancellationToken = Nothing) As Task(Of CompletionList) + + Dim text = Await document.GetTextAsync(cancellationToken).ConfigureAwait(False) + Dim defaultItemSpan = GetDefaultCompletionListSpan(text, caretPosition) + + Dim builder = ArrayBuilder(Of CompletionItem).GetInstance(RegularCount + ExpandedCount) + If (options.ExpandedCompletionBehavior = ExpandedCompletionMode.AllItems) Then + CreateRegularItems(builder, RegularCount) + Await CreateExpandedItems(builder, ExpandedCount) + ElseIf (options.ExpandedCompletionBehavior = ExpandedCompletionMode.ExpandedItemsOnly) Then + Await CreateExpandedItems(builder, ExpandedCount) + Else + CreateRegularItems(builder, RegularCount) + End If + + Return CompletionList.Create(defaultItemSpan, builder.ToImmutableAndFree()) + End Function + + Friend Overrides Function ShouldTriggerCompletion(project As Project, + languageServices As CodeAnalysis.Host.LanguageServices, + text As SourceText, + caretPosition As Integer, + trigger As CompletionTrigger, + options As CompletionOptions, + passThroughOptions As OptionSet, + Optional roles As ImmutableHashSet(Of String) = Nothing) As Boolean + Return True + End Function + + Private Async Function CreateExpandedItems(builder As ArrayBuilder(Of CompletionItem), count As Integer) As Task + Await ExpandedItemsCheckpoint.Task + For i = 1 To count + Dim item = ImportCompletionItem.Create( + $"ItemExpanded{i}", + arity:=0, + containingNamespace:="NS", + glyph:=Glyph.ClassPublic, + genericTypeSuffix:=String.Empty, + flags:=CompletionItemFlags.Expanded, + extensionMethodData:=Nothing, + includedInTargetTypeCompletion:=True) + builder.Add(item) + Next + End Function + + Private Shared Sub CreateRegularItems(builder As ArrayBuilder(Of CompletionItem), count As Integer) + For i = 1 To count + builder.Add(CompletionItem.Create($"ItemRegular{i}")) + Next + End Sub + End Class + End Class End Class End Namespace