From a83e943963fd130c0ba8ef91a6546c9f5b8e84ba Mon Sep 17 00:00:00 2001 From: "gel@microsoft.com" Date: Fri, 28 Jan 2022 14:08:11 -0800 Subject: [PATCH] Fix --- ...bjectInitializerCompletionProviderTests.cs | 2 +- .../Test/Completion/CompletionServiceTests.cs | 2 +- .../IntelliSense/CompletionServiceTests.vb | 1 - .../CompletionServiceTests_Exclusivitiy.vb | 1 - .../AbstractCompletionProviderTests.cs | 4 +- ...bjectInitializerCompletionProviderTests.vb | 2 +- .../Portable/Completion/CompletionContext.cs | 19 +++- .../Portable/Completion/CompletionList.cs | 2 + .../CompletionServiceWithProviders.cs | 96 +++++-------------- 9 files changed, 49 insertions(+), 80 deletions(-) diff --git a/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/ObjectInitializerCompletionProviderTests.cs b/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/ObjectInitializerCompletionProviderTests.cs index 4dde8138a4d5e..11326f4e2f938 100644 --- a/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/ObjectInitializerCompletionProviderTests.cs +++ b/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/ObjectInitializerCompletionProviderTests.cs @@ -1206,7 +1206,7 @@ private async Task VerifyExclusiveAsync(string markup, bool exclusive) var service = GetCompletionService(document.Project); var completionList = await GetCompletionListAsync(service, document, position, triggerInfo); - if (completionList != null) + if (!completionList.IsEmpty) { Assert.True(exclusive == completionList.GetTestAccessor().IsExclusive, "group.IsExclusive == " + completionList.GetTestAccessor().IsExclusive); } diff --git a/src/EditorFeatures/Test/Completion/CompletionServiceTests.cs b/src/EditorFeatures/Test/Completion/CompletionServiceTests.cs index 1708c4fc99589..09d4e4f5d4369 100644 --- a/src/EditorFeatures/Test/Completion/CompletionServiceTests.cs +++ b/src/EditorFeatures/Test/Completion/CompletionServiceTests.cs @@ -47,7 +47,7 @@ void Method() { var caretPosition = workspace.DocumentWithCursor.CursorPosition ?? throw new InvalidOperationException(); var completions = await completionService.GetCompletionsAsync(document, caretPosition, CompletionOptions.Default); - Assert.NotNull(completions); + Assert.False(completions.IsEmpty); var item = Assert.Single(completions.Items.Where(item => item.ProviderName == typeof(DebugAssertTestCompletionProvider).FullName)); Assert.Equal("Assertion failed", item.DisplayText); } diff --git a/src/EditorFeatures/Test2/IntelliSense/CompletionServiceTests.vb b/src/EditorFeatures/Test2/IntelliSense/CompletionServiceTests.vb index 5f015df1c04bd..577d040b0d858 100644 --- a/src/EditorFeatures/Test2/IntelliSense/CompletionServiceTests.vb +++ b/src/EditorFeatures/Test2/IntelliSense/CompletionServiceTests.vb @@ -35,7 +35,6 @@ Namespace Microsoft.CodeAnalysis.Editor.UnitTests.IntelliSense Dim list = Await completionService.GetCompletionsAsync( document, caretPosition:=0, options:=CompletionOptions.Default, trigger:=CompletionTrigger.Invoke) - Assert.NotNull(list) Assert.NotEmpty(list.Items) Assert.True(list.Items.Length = 1, "Completion list contained more than one item") Assert.Equal("Completion Item From Test Completion Provider", list.Items.First.DisplayText) diff --git a/src/EditorFeatures/Test2/IntelliSense/CompletionServiceTests_Exclusivitiy.vb b/src/EditorFeatures/Test2/IntelliSense/CompletionServiceTests_Exclusivitiy.vb index 0ab7fcdbcb11d..0c4c18ed4293e 100644 --- a/src/EditorFeatures/Test2/IntelliSense/CompletionServiceTests_Exclusivitiy.vb +++ b/src/EditorFeatures/Test2/IntelliSense/CompletionServiceTests_Exclusivitiy.vb @@ -42,7 +42,6 @@ Namespace Microsoft.CodeAnalysis.Editor.UnitTests.IntelliSense Dim list = Await completionService.GetCompletionsAsync( document, caretPosition:=0, options:=CompletionOptions.Default, trigger:=CompletionTrigger.Invoke) - Assert.NotNull(list) Assert.NotEmpty(list.Items) Assert.True(list.Items.Length = 2, "Completion List does not contain exactly two items.") Assert.Equal(String.Format(CompletionItemExclusive, 2), list.Items.First.DisplayText) diff --git a/src/EditorFeatures/TestUtilities/Completion/AbstractCompletionProviderTests.cs b/src/EditorFeatures/TestUtilities/Completion/AbstractCompletionProviderTests.cs index 03fa7b9864b84..792a40c8ae276 100644 --- a/src/EditorFeatures/TestUtilities/Completion/AbstractCompletionProviderTests.cs +++ b/src/EditorFeatures/TestUtilities/Completion/AbstractCompletionProviderTests.cs @@ -159,7 +159,7 @@ private protected async Task CheckResultsAsync( var displayOptions = SymbolDescriptionOptions.From(document.Project); var completionService = GetCompletionService(document.Project); var completionList = await GetCompletionListAsync(completionService, document, position, trigger, options); - var items = completionList == null ? ImmutableArray.Empty : completionList.Items; + var items = completionList.Items; if (hasSuggestionModeItem != null) { @@ -1114,7 +1114,7 @@ protected async Task VerifyCommitCharactersAsync(string initialMarkup, string te var completionService = GetCompletionService(document.Project); var completionList = await GetCompletionListAsync(completionService, document, position, trigger); - return completionList == null ? ImmutableArray.Empty : completionList.Items; + return completionList.Items; } } } diff --git a/src/EditorFeatures/VisualBasicTest/Completion/CompletionProviders/ObjectInitializerCompletionProviderTests.vb b/src/EditorFeatures/VisualBasicTest/Completion/CompletionProviders/ObjectInitializerCompletionProviderTests.vb index eb2636346cdd2..b012dc1a2a484 100644 --- a/src/EditorFeatures/VisualBasicTest/Completion/CompletionProviders/ObjectInitializerCompletionProviderTests.vb +++ b/src/EditorFeatures/VisualBasicTest/Completion/CompletionProviders/ObjectInitializerCompletionProviderTests.vb @@ -461,7 +461,7 @@ End Program Dim document = workspace.CurrentSolution.GetDocument(hostDocument.Id) Dim service = GetCompletionService(document.Project) Dim completionList = Await GetCompletionListAsync(service, document, caretPosition, RoslynCompletion.CompletionTrigger.Invoke) - Assert.True(completionList Is Nothing OrElse completionList.GetTestAccessor().IsExclusive, "Expected always exclusive") + Assert.True(completionList.IsEmpty OrElse completionList.GetTestAccessor().IsExclusive, "Expected always exclusive") End Using End Function diff --git a/src/Features/Core/Portable/Completion/CompletionContext.cs b/src/Features/Core/Portable/Completion/CompletionContext.cs index 1f962b9f35d42..b6492fbeeff89 100644 --- a/src/Features/Core/Portable/Completion/CompletionContext.cs +++ b/src/Features/Core/Portable/Completion/CompletionContext.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.Collections.Immutable; +using System.Diagnostics; using System.Threading; using Microsoft.CodeAnalysis.Options; using Microsoft.CodeAnalysis.Text; @@ -20,6 +21,7 @@ public sealed class CompletionContext private CompletionItem? _suggestionModeItem; private OptionSet? _lazyOptionSet; + private bool _isExclusive; internal CompletionProvider Provider { get; } @@ -71,8 +73,23 @@ public sealed class CompletionContext /// /// Set to true if the items added here should be the only items presented to the user. + /// Expand items should never be exclusive. /// - public bool IsExclusive { get; set; } + public bool IsExclusive + { + get + { + return _isExclusive && !Provider.IsExpandItemProvider; + } + + set + { + if (value) + Debug.Assert(!Provider.IsExpandItemProvider); + + _isExclusive = value; + } + } /// /// Creates a instance. diff --git a/src/Features/Core/Portable/Completion/CompletionList.cs b/src/Features/Core/Portable/Completion/CompletionList.cs index 2995327ac6ef2..539495669bf06 100644 --- a/src/Features/Core/Portable/Completion/CompletionList.cs +++ b/src/Features/Core/Portable/Completion/CompletionList.cs @@ -157,6 +157,8 @@ public CompletionList WithSuggestionModeItem(CompletionItem suggestionModeItem) default, default, CompletionRules.Default, suggestionModeItem: null, isExclusive: false); + internal bool IsEmpty => Items.IsEmpty && SuggestionModeItem is null; + internal TestAccessor GetTestAccessor() => new(this); diff --git a/src/Features/Core/Portable/Completion/CompletionServiceWithProviders.cs b/src/Features/Core/Portable/Completion/CompletionServiceWithProviders.cs index 67cadb9081723..651a44e5eeb08 100644 --- a/src/Features/Core/Portable/Completion/CompletionServiceWithProviders.cs +++ b/src/Features/Core/Portable/Completion/CompletionServiceWithProviders.cs @@ -127,12 +127,14 @@ private protected async Task GetCompletionsWithAvailabilityOfExp document, caretPosition, trigger, options, defaultItemSpan, triggeredProviders, cancellationToken).ConfigureAwait(false); // Nothing to do if we didn't even get any regular items back (i.e. 0 items or suggestion item only.) - if (!triggeredContexts.ContainsNonSuggestionModeItem) + if (!triggeredContexts.Any(cc => cc.Items.Count > 0)) return CompletionList.Empty; - // See if there were completion contexts provided that were exclusive. If so, then that's all we'll return. - if (triggeredContexts.ContainsExclusiveContext) - return MergeAndPruneCompletionLists(triggeredContexts.GetExclusiveContexts(), defaultItemSpan, options, isExclusive: true); + // See if there were completion contexts provided that were exclusive. If so, then + // that's all we'll return. + var exclusiveContexts = triggeredContexts.Where(t => t.IsExclusive).ToImmutableArray(); + if (!exclusiveContexts.IsEmpty) + return MergeAndPruneCompletionLists(exclusiveContexts, defaultItemSpan, options, isExclusive: true); // Great! We had some items. Now we want to see if any of the other providers // would like to augment the completion list. For example, we might trigger @@ -148,31 +150,31 @@ private protected async Task GetCompletionsWithAvailabilityOfExp // Providers are ordered, but we processed them in our own order. Ensure that the // groups are properly ordered based on the original providers. var completionProviderToIndex = GetCompletionProviderToIndex(providers); - var allContexts = triggeredContexts.NonEmptyContexts.Concat(augmentingContexts.NonEmptyContexts) + var allContexts = triggeredContexts.Concat(augmentingContexts) .Sort((p1, p2) => completionProviderToIndex[p1.Provider] - completionProviderToIndex[p2.Provider]); return MergeAndPruneCompletionLists(allContexts, defaultItemSpan, options, isExclusive: false); ImmutableArray GetTriggeredProviders( - Document document, ConcatImmutableArray allProviders, int caretPosition, CompletionOptions options, CompletionTrigger trigger, ImmutableHashSet? roles, SourceText text) + Document document, ConcatImmutableArray providers, int caretPosition, CompletionOptions options, CompletionTrigger trigger, ImmutableHashSet? roles, SourceText text) { switch (trigger.Kind) { case CompletionTriggerKind.Insertion: case CompletionTriggerKind.Deletion: - var shouldTrigger = ShouldTypingTriggerCompletionWithoutAskingProviders(trigger, options); - if (shouldTrigger.HasValue && !shouldTrigger.Value) - return ImmutableArray.Empty; - var triggeredProviders = allProviders.Where(p => p.ShouldTriggerCompletion(document.Project.LanguageServices, text, caretPosition, trigger, options)).ToImmutableArrayOrEmpty(); - Debug.Assert(ValidatePossibleTriggerCharacterSet(trigger.Kind, triggeredProviders, document, text, caretPosition, options)); + if (ShouldTriggerCompletion(document.Project, document.Project.LanguageServices, text, caretPosition, trigger, options, roles)) + { + var triggeredProviders = providers.Where(p => p.ShouldTriggerCompletion(document.Project.LanguageServices, text, caretPosition, trigger, options)).ToImmutableArrayOrEmpty(); + + Debug.Assert(ValidatePossibleTriggerCharacterSet(trigger.Kind, triggeredProviders, document, text, caretPosition, options)); + return triggeredProviders.IsEmpty ? providers.ToImmutableArray() : triggeredProviders; + } - return triggeredProviders.Length == 0 - ? allProviders.ToImmutableArray() - : triggeredProviders; + return ImmutableArray.Empty; default: - return allProviders.ToImmutableArray(); + return providers.ToImmutableArray(); } } @@ -206,11 +208,8 @@ public sealed override bool ShouldTriggerCompletion(SourceText text, int caretPo return ShouldTriggerCompletion(document?.Project, languageServices, text, caretPosition, trigger, completionOptions, roles); } - /// - /// A preliminary quick check to see if we can decide whether to trigger completion without asking each individual providers. - /// Returning null means the decision needs to be made by providers. - /// - private bool? ShouldTypingTriggerCompletionWithoutAskingProviders(CompletionTrigger trigger, CompletionOptions options) + internal sealed override bool ShouldTriggerCompletion( + Project? project, HostLanguageServices languageServices, SourceText text, int caretPosition, CompletionTrigger trigger, CompletionOptions options, ImmutableHashSet? roles = null) { if (!options.TriggerOnTyping) { @@ -222,16 +221,6 @@ public sealed override bool ShouldTriggerCompletion(SourceText text, int caretPo return char.IsLetterOrDigit(trigger.Character) || trigger.Character == '.'; } - return null; - } - - internal sealed override bool ShouldTriggerCompletion( - Project? project, HostLanguageServices languageServices, SourceText text, int caretPosition, CompletionTrigger trigger, CompletionOptions options, ImmutableHashSet? roles = null) - { - var shouldTrigger = ShouldTypingTriggerCompletionWithoutAskingProviders(trigger, options); - if (shouldTrigger.HasValue) - return shouldTrigger.Value; - var providers = _providerManager.GetFilteredProviders(project, roles, trigger, options); return providers.Any(p => p.ShouldTriggerCompletion(languageServices, text, caretPosition, trigger, options)); } @@ -278,7 +267,10 @@ private static bool ValidatePossibleTriggerCharacterSet(CompletionTriggerKind co return true; } - private static async Task ComputeNonEmptyCompletionContextsAsync( + private static bool HasAnyItems(CompletionContext cc) + => cc.Items.Count > 0 || cc.SuggestionModeItem != null; + + private static async Task> ComputeNonEmptyCompletionContextsAsync( Document document, int caretPosition, CompletionTrigger trigger, CompletionOptions options, TextSpan defaultItemSpan, ImmutableArray providers, @@ -293,7 +285,7 @@ private static async Task ComputeNonEmptyCompl } var completionContexts = await Task.WhenAll(completionContextTasks).ConfigureAwait(false); - return new(completionContexts); + return completionContexts.Where(HasAnyItems).ToImmutableArray(); } private CompletionList MergeAndPruneCompletionLists( @@ -510,46 +502,6 @@ System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() } } - private readonly struct AggregatedCompletionContextsData - { - public ImmutableArray NonEmptyContexts { get; } - - public bool ContainsNonSuggestionModeItem { get; } - public bool ContainsExclusiveContext { get; } - - public bool IsEmpty => NonEmptyContexts.Length == 0; - - public AggregatedCompletionContextsData(CompletionContext[] allContexts) - { - var containsNonSuggestionModeItem = false; - var containsExclusiveContext = false; - var builder = ArrayBuilder.GetInstance(allContexts.Length); - - foreach (var context in allContexts) - { - if (context.Items.Count > 0) - { - containsNonSuggestionModeItem = true; - builder.Add(context); - } - else if (context.SuggestionModeItem != null) - { - builder.Add(context); - } - - if (context.IsExclusive) - containsExclusiveContext = true; - } - - NonEmptyContexts = builder.ToImmutableAndFree(); - ContainsNonSuggestionModeItem = containsNonSuggestionModeItem; - ContainsExclusiveContext = containsExclusiveContext; - } - - public ImmutableArray GetExclusiveContexts() - => ContainsExclusiveContext ? NonEmptyContexts.WhereAsArray(c => c.IsExclusive) : ImmutableArray.Empty; - } - internal TestAccessor GetTestAccessor() => new(this);