Merge pull request #55035 from genlu/fixRazorCompletion

Keep track of trigger location for individual CompletionItem

src/EditorFeatures/Core/Implementation/IntelliSense/AsyncCompletion/CommitManager.cs

@@ -123,10 +123,11 @@ public AsyncCompletionData.CommitResult TryCommit(
                 return new AsyncCompletionData.CommitResult(isHandled: true, AsyncCompletionData.CommitBehavior.None);
             }
 
-            if (!Helpers.TryGetInitialTriggerLocation(session, out var triggerLocation))
+            if (!item.Properties.TryGetProperty(CompletionSource.TriggerLocation, out SnapshotPoint triggerLocation))
             {
                 // Need the trigger snapshot to calculate the span when the commit changes to be applied.
-                // They should always be available from VS. Just to be defensive, if it's not found here, Roslyn should not make a commit.
+                // They should always be available from items provided by Roslyn CompletionSource.
+                // Just to be defensive, if it's not found here, Roslyn should not make a commit.
                 return CommitResultUnhandled;
             }
 

src/EditorFeatures/Core/Implementation/IntelliSense/AsyncCompletion/CompletionSource.cs

@@ -35,6 +35,7 @@ internal sealed class CompletionSource : ForegroundThreadAffinitizedObject, IAsy
     {
         internal const string RoslynItem = nameof(RoslynItem);
         internal const string TriggerLocation = nameof(TriggerLocation);
+        internal const string ExpandedItemTriggerLocation = nameof(ExpandedItemTriggerLocation);
         internal const string CompletionListSpan = nameof(CompletionListSpan);
         internal const string InsertionText = nameof(InsertionText);
         internal const string HasSuggestionItemOptions = nameof(HasSuggestionItemOptions);
@@ -219,7 +220,6 @@ private bool TryInvokeSnippetCompletion(
             if (session is null)
                 throw new ArgumentNullException(nameof(session));
 
-            session.Properties[TriggerLocation] = triggerLocation;
             return GetCompletionContextWorkerAsync(session, trigger, triggerLocation, isExpanded: false, cancellationToken);
         }
 
@@ -231,12 +231,9 @@ private bool TryInvokeSnippetCompletion(
             CancellationToken cancellationToken)
         {
             // We only want to provide expanded items for Roslyn's expander.
-            if ((object)expander == FilterSet.Expander)
+            if ((object)expander == FilterSet.Expander && session.Properties.TryGetProperty(ExpandedItemTriggerLocation, out SnapshotPoint initialTriggerLocation))
             {
-                if (Helpers.TryGetInitialTriggerLocation(session, out var initialTriggerLocation))
-                {
-                    return await GetCompletionContextWorkerAsync(session, intialTrigger, initialTriggerLocation, isExpanded: true, cancellationToken).ConfigureAwait(false);
-                }
+                return await GetCompletionContextWorkerAsync(session, intialTrigger, initialTriggerLocation, isExpanded: true, cancellationToken).ConfigureAwait(false);
             }
 
             return AsyncCompletionData.CompletionContext.Empty;
@@ -297,7 +294,7 @@ private bool TryInvokeSnippetCompletion(
                 foreach (var roslynItem in completionList.Items)
                 {
                     cancellationToken.ThrowIfCancellationRequested();
-                    var item = Convert(document, roslynItem, filterSet);
+                    var item = Convert(document, roslynItem, filterSet, triggerLocation);
                     itemsBuilder.Add(item);
                 }
 
@@ -336,6 +333,16 @@ private bool TryInvokeSnippetCompletion(
                 }
             }
 
+            // We need to remember the trigger location for when a completion service claims expanded items are available
+            // since the initial trigger we are able to get from IAsyncCompletionSession might not be the same (e.g. in projection scenarios)
+            // so when they are requested via expander later, we can retrieve it.
+            // Technically we should save the trigger location for each individual service that made such claim, but in reality only Roslyn's
+            // completion service uses expander, so we can get away with not making such distinction.
+            if (!isExpanded && expandItemsAvailable)
+            {
+                session.Properties[ExpandedItemTriggerLocation] = triggerLocation;
+            }
+
             // It's possible that some providers can provide expanded items, in which case we will need to show expander as unselected.
             return new AsyncCompletionData.CompletionContext(
                 items,
@@ -354,7 +361,7 @@ private bool TryInvokeSnippetCompletion(
                 throw new ArgumentNullException(nameof(item));
 
             if (!item.Properties.TryGetProperty(RoslynItem, out RoslynCompletionItem roslynItem) ||
-                !Helpers.TryGetInitialTriggerLocation(session, out var triggerLocation))
+                !item.Properties.TryGetProperty(TriggerLocation, out SnapshotPoint triggerLocation))
             {
                 return null;
             }
@@ -427,7 +434,8 @@ public VSCompletionItemData(
         private VSCompletionItem Convert(
             Document document,
             RoslynCompletionItem roslynItem,
-            FilterSet filterSet)
+            FilterSet filterSet,
+            SnapshotPoint initialTriggerLocation)
         {
             VSCompletionItemData itemData;
 
@@ -481,6 +489,7 @@ private VSCompletionItem Convert(
                 attributeIcons: itemData.AttributeIcons);
 
             item.Properties.AddProperty(RoslynItem, roslynItem);
+            item.Properties.AddProperty(TriggerLocation, initialTriggerLocation);
 
             return item;
         }

src/EditorFeatures/Core/Implementation/IntelliSense/AsyncCompletion/Helpers.cs

@@ -3,6 +3,7 @@
 // See the LICENSE file in the project root for more information.
 
 using System;
+using System.Collections.Generic;
 using Microsoft.CodeAnalysis.Completion;
 using Microsoft.VisualStudio.Text;
 using EditorAsyncCompletion = Microsoft.VisualStudio.Language.Intellisense.AsyncCompletion;
@@ -141,9 +142,6 @@ internal static bool TextTypedSoFarMatchesItem(RoslynCompletionItem item, string
         internal static bool IsStandardCommitCharacter(char c)
             => c == '\t' || c == '\n' || c == '\0';
 
-        internal static bool TryGetInitialTriggerLocation(EditorAsyncCompletion.IAsyncCompletionSession session, out SnapshotPoint initialTriggerLocation)
-            => session.Properties.TryGetProperty(CompletionSource.TriggerLocation, out initialTriggerLocation);
-
         // This is a temporarily method to support preference of IntelliCode items comparing to non-IntelliCode items.
         // We expect that Editor will introduce this support and we will get rid of relying on the "★" then.
         // We check both the display text and the display text prefix to account for IntelliCode item providers

src/EditorFeatures/Core/Implementation/IntelliSense/AsyncCompletion/ItemManager.cs

@@ -152,13 +152,13 @@ private static readonly ObjectPool<List<MatchResult>> s_listOfMatchResultPool
 
             var filterReason = Helpers.GetFilterReason(data.Trigger);
 
-            // If the session was created/maintained out of Roslyn, e.g. in debugger; no properties are set and we should use data.Snapshot.
-            // However, we prefer using the original snapshot in some projection scenarios.
-            var snapshotForDocument = Helpers.TryGetInitialTriggerLocation(session, out var triggerLocation)
-                ? triggerLocation.Snapshot
+            // We prefer using the original snapshot, which should always be available from items provided by Roslyn's CompletionSource.
+            // Only use data.Snapshot in the theoretically possible but rare case when all items we are handling are from some non-Roslyn CompletionSource.
+            var snapshotForDocument = TryGetInitialTriggerLocation(data, out var intialTriggerLocation)
+                ? intialTriggerLocation.Snapshot
                 : data.Snapshot;
 
-            var document = snapshotForDocument.TextBuffer.AsTextContainer().GetOpenDocumentInCurrentContext();
+            var document = snapshotForDocument?.TextBuffer.AsTextContainer().GetOpenDocumentInCurrentContext();
             var completionService = document?.GetLanguageService<CompletionService>();
             var completionRules = completionService?.GetRules() ?? CompletionRules.Default;
             var completionHelper = document != null ? CompletionHelper.GetHelper(document) : _defaultCompletionHelper;
@@ -264,6 +264,18 @@ private static readonly ObjectPool<List<MatchResult>> s_listOfMatchResultPool
                 s_listOfMatchResultPool.Free(initialListOfItemsToBeIncluded);
             }
 
+            static bool TryGetInitialTriggerLocation(AsyncCompletionSessionDataSnapshot data, out SnapshotPoint intialTriggerLocation)
+            {
+                var firstItem = data.InitialSortedList.FirstOrDefault(static item => item.Properties.ContainsProperty(CompletionSource.TriggerLocation));
+                if (firstItem != null)
+                {
+                    return firstItem.Properties.TryGetProperty(CompletionSource.TriggerLocation, out intialTriggerLocation);
+                }
+
+                intialTriggerLocation = default;
+                return false;
+            }
+
             static bool ShouldBeFilteredOutOfCompletionList(VSCompletionItem item, ImmutableArray<CompletionFilter> activeNonExpanderFilters)
             {
                 if (item.Filters.Any(filter => activeNonExpanderFilters.Contains(filter)))