Skip to content

Commit

Permalink
Fix
Browse files Browse the repository at this point in the history
  • Loading branch information
@genlu
genlu committed Feb 17, 2022
1 parent 0b272b5 commit 32069d8
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 41 deletions.
27 changes: 14 additions & 13 deletions src/EditorFeatures/Core/Implementation/IntelliSense/AsyncCompletion/CommitManager.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,8 @@ public bool ShouldCommitCompletion(
return false;
}

var sessionData = CompletionSessionData.GetOrCreateSessionData(session);
return !sessionData.ExcludedCommitCharacters.HasValue
|| !sessionData.ExcludedCommitCharacters.Value.Contains(typedChar);
return !(session.Properties.TryGetProperty(CompletionSource.ExcludedCommitCharacters, out ImmutableArray<char> excludedCommitCharacter)
&& excludedCommitCharacter.Contains(typedChar));
}

public AsyncCompletionData.CommitResult TryCommit(
Expand All @@ -106,16 +105,12 @@ public AsyncCompletionData.CommitResult TryCommit(
return CommitResultUnhandled;
}

if (!CompletionItemData.TryGetData(item, out var itemData) || !itemData.IsProvidedByRoslynCompletionSource)
if (!CompletionItemData.TryGetData(item, out var itemData))
{
// Roslyn should not be called if the item committing was not provided by Roslyn.
return CommitResultUnhandled;
}

// Need the trigger snapshot to calculate the span when the commit changes to be applied.
// They should always be available from items provided by Roslyn CompletionSource.
Contract.ThrowIfFalse(itemData.TriggerLocation.HasValue);

var filterText = session.ApplicableToSpan.GetText(session.ApplicableToSpan.TextBuffer.CurrentSnapshot) + typeChar;
if (Helpers.IsFilterCharacter(itemData.RoslynItem, typeChar, filterText))
{
Expand All @@ -125,7 +120,6 @@ public AsyncCompletionData.CommitResult TryCommit(

var options = _globalOptions.GetCompletionOptions(document.Project.Language);
var serviceRules = completionService.GetRules(options);
var sessionData = CompletionSessionData.GetOrCreateSessionData(session);

// We can be called before for ShouldCommitCompletion. However, that call does not provide rules applied for the completion item.
// Now we check for the commit character in the context of Rules that could change the list of commit characters.
Expand All @@ -137,19 +131,26 @@ public AsyncCompletionData.CommitResult TryCommit(
return new AsyncCompletionData.CommitResult(isHandled: true, AsyncCompletionData.CommitBehavior.None);
}

if (!sessionData.CompletionListSpan.HasValue)
if (!itemData.TriggerLocation.HasValue)
{
// Need the trigger snapshot to calculate the span when the commit changes to be applied.
// 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;
}

var completionListSpan = sessionData.CompletionListSpan.Value;

var triggerDocument = itemData.TriggerLocation.Value.Snapshot.GetOpenDocumentInCurrentContextWithChanges();
if (triggerDocument == null)
{
return CommitResultUnhandled;
}

var sessionData = CompletionSessionData.GetOrCreateSessionData(session);
if (!sessionData.CompletionListSpan.HasValue)
{
return CommitResultUnhandled;
}

// Telemetry
if (sessionData.TargetTypeFilterExperimentEnabled)
{
Expand All @@ -166,7 +167,7 @@ public AsyncCompletionData.CommitResult TryCommit(
var commitChar = typeChar == '\0' ? null : (char?)typeChar;
return Commit(
session, triggerDocument, completionService, subjectBuffer,
itemData.RoslynItem, completionListSpan, commitChar, itemData.TriggerLocation.Value.Snapshot, serviceRules,
itemData.RoslynItem, sessionData.CompletionListSpan.Value, commitChar, itemData.TriggerLocation.Value.Snapshot, serviceRules,
filterText, cancellationToken);
}

Expand Down
2 changes: 1 addition & 1 deletion src/EditorFeatures/Core/Implementation/IntelliSense/AsyncCompletion/CompletionItemData.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public static bool TryGetData(CompletionItem vsCompletionitem, out CompletionIte

public static RoslynCompletionItem GetOrAddDummyRoslynItem(CompletionItem vsItem)
{
if (vsItem.Properties.TryGetProperty(RoslynCompletionItemData, out CompletionItemData data))
if (TryGetData(vsItem, out var data))
return data.RoslynItem;

var roslynItem = CreateDummyRoslynItem(vsItem);
Expand Down
9 changes: 4 additions & 5 deletions src/EditorFeatures/Core/Implementation/IntelliSense/AsyncCompletion/CompletionSessionData.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,10 @@ internal sealed class CompletionSessionData
public bool TargetTypeFilterSelected { get; set; }
public bool HasSuggestionItemOptions { get; set; }

public Optional<SnapshotPoint> ExpandedItemTriggerLocation { get; set; }
public Optional<TextSpan> CompletionListSpan { get; set; }
public Optional<ImmutableArray<CompletionItem>> CombinedSortedList { get; set; }
public Optional<Task<(CompletionContext, RoslynCompletionList)>> ExpandedItemsTask { get; set; }
public Optional<ImmutableArray<char>> ExcludedCommitCharacters { get; set; }
public SnapshotPoint? ExpandedItemTriggerLocation { get; set; }
public TextSpan? CompletionListSpan { get; set; }
public ImmutableArray<CompletionItem>? CombinedSortedList { get; set; }
public Task<(CompletionContext, RoslynCompletionList)>? ExpandedItemsTask { get; set; }

private CompletionSessionData()
{
Expand Down
31 changes: 17 additions & 14 deletions src/EditorFeatures/Core/Implementation/IntelliSense/AsyncCompletion/CompletionSource.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ internal sealed class CompletionSource : ForegroundThreadAffinitizedObject, IAsy
internal const string PotentialCommitCharacters = nameof(PotentialCommitCharacters);
internal const string NonBlockingCompletion = nameof(NonBlockingCompletion);

// Don't change this property! Editor code currently has a dependency on it.
internal const string ExcludedCommitCharacters = nameof(ExcludedCommitCharacters);

private static readonly ImmutableArray<ImageElement> s_warningImageAttributeImagesArray =
ImmutableArray.Create(new ImageElement(Glyph.CompletionWarning.GetImageId(), EditorFeaturesResources.Warning_image_element));

Expand Down Expand Up @@ -263,7 +266,7 @@ public async Task<VSCompletionContext> GetCompletionContextAsync(
var (context, list) = await GetCompletionContextWorkerAsync(document, trigger, triggerLocation,
options with { ExpandedCompletionBehavior = ExpandedCompletionMode.NonExpandedItemsOnly }, cancellationToken).ConfigureAwait(false);

UpdateSessionData(sessionData, list, triggerLocation);
UpdateSessionData(session, sessionData, list, triggerLocation);
return context;
}
else if (!_responsiveCompletionEnabled)
Expand All @@ -273,7 +276,7 @@ public async Task<VSCompletionContext> GetCompletionContextAsync(
var (context, list) = await GetCompletionContextWorkerAsync(document, trigger, triggerLocation,
options with { ExpandedCompletionBehavior = ExpandedCompletionMode.AllItems }, cancellationToken).ConfigureAwait(false);

UpdateSessionData(sessionData, list, triggerLocation);
UpdateSessionData(session, sessionData, list, triggerLocation);
AsyncCompletionLogger.LogImportCompletionGetContext(isBlocking: true, delayed: false);
return context;
}
Expand Down Expand Up @@ -301,13 +304,13 @@ public async Task<VSCompletionContext> GetCompletionContextAsync(
// Now trigger and wait for core providers to return;
var (nonExpandedContext, nonExpandedCompletionList) = await GetCompletionContextWorkerAsync(document, trigger, triggerLocation,
options with { ExpandedCompletionBehavior = ExpandedCompletionMode.NonExpandedItemsOnly }, cancellationToken).ConfigureAwait(false);
UpdateSessionData(sessionData, nonExpandedCompletionList, triggerLocation);
UpdateSessionData(session, sessionData, nonExpandedCompletionList, triggerLocation);

if (expandedItemsTask.IsCompleted)
{
// the task of expanded item is completed, get the result and combine it with result of non-expanded items.
var (expandedContext, expandedCompletionList) = await expandedItemsTask.ConfigureAwait(false);
UpdateSessionData(sessionData, expandedCompletionList, triggerLocation);
UpdateSessionData(session, sessionData, expandedCompletionList, triggerLocation);
AsyncCompletionLogger.LogImportCompletionGetContext(isBlocking: false, delayed: false);

return CombineCompletionContext(nonExpandedContext, expandedContext);
Expand All @@ -319,7 +322,7 @@ public async Task<VSCompletionContext> GetCompletionContextAsync(
// after core providers completed (instead of how long it takes end-to-end).
stopwatch.Start();

sessionData.ExpandedItemsTask = new(expandedItemsTask);
sessionData.ExpandedItemsTask = expandedItemsTask;
AsyncCompletionLogger.LogImportCompletionGetContext(isBlocking: false, delayed: true);

return nonExpandedContext;
Expand Down Expand Up @@ -364,15 +367,15 @@ public async Task<VSCompletionContext> GetExpandedCompletionContextAsync(

// It's possible we didn't provide expanded items at the beginning of completion session because it was slow even if the feature is enabled.
// ExpandedItemsTask would be available in this case, so we just need to return its result.
if (sessionData.ExpandedItemsTask.HasValue)
if (sessionData.ExpandedItemsTask != null)
{
// Make sure the task is removed when returning expanded items,
// so duplicated items won't be added in subsequent list updates.
var task = sessionData.ExpandedItemsTask.Value;
sessionData.ExpandedItemsTask = new();
var task = sessionData.ExpandedItemsTask;
sessionData.ExpandedItemsTask = null;

var (expandedContext, expandedCompletionList) = await task.ConfigureAwait(false);
UpdateSessionData(sessionData, expandedCompletionList, initialTriggerLocation);
UpdateSessionData(session, sessionData, expandedCompletionList, initialTriggerLocation);
return expandedContext;
}

Expand All @@ -390,7 +393,7 @@ public async Task<VSCompletionContext> GetExpandedCompletionContextAsync(
};

var (context, completionList) = await GetCompletionContextWorkerAsync(document, intialTrigger, initialTriggerLocation, options, cancellationToken).ConfigureAwait(false);
UpdateSessionData(sessionData, completionList, initialTriggerLocation);
UpdateSessionData(session, sessionData, completionList, initialTriggerLocation);

return context;
}
Expand Down Expand Up @@ -446,7 +449,7 @@ public async Task<VSCompletionContext> GetExpandedCompletionContextAsync(
return (new(items, suggestionItemOptions, selectionHint: AsyncCompletionData.InitialSelectionHint.SoftSelection, filters), completionList);
}

private static void UpdateSessionData(CompletionSessionData sessionData, CompletionList completionList, SnapshotPoint triggerLocation)
private static void UpdateSessionData(IAsyncCompletionSession session, CompletionSessionData sessionData, CompletionList completionList, SnapshotPoint triggerLocation)
{
// Store around the span this completion list applies to. We'll use this later
// to pass this value in when we're committing a completion list item.
Expand All @@ -461,12 +464,12 @@ private static void UpdateSessionData(CompletionSessionData sessionData, Complet
var excludedCommitCharacters = GetExcludedCommitCharacters(completionList.Items);
if (excludedCommitCharacters.Length > 0)
{
if (sessionData.ExcludedCommitCharacters.HasValue)
if (session.Properties.TryGetProperty(ExcludedCommitCharacters, out ImmutableArray<char> excludedCommitCharactersBefore))
{
excludedCommitCharacters = excludedCommitCharacters.Union(sessionData.ExcludedCommitCharacters.Value).ToImmutableArray();
excludedCommitCharacters = excludedCommitCharacters.Union(excludedCommitCharactersBefore).ToImmutableArray();
}

sessionData.ExcludedCommitCharacters = excludedCommitCharacters;
session.Properties[ExcludedCommitCharacters] = excludedCommitCharacters;
}

// We need to remember the trigger location for when a completion service claims expanded items are available
Expand Down
9 changes: 4 additions & 5 deletions src/EditorFeatures/Core/Implementation/IntelliSense/AsyncCompletion/ItemManager.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
using Microsoft.VisualStudio.Language.Intellisense.AsyncCompletion;
using Microsoft.VisualStudio.Language.Intellisense.AsyncCompletion.Data;
using Roslyn.Utilities;
using RoslynCompletionItem = Microsoft.CodeAnalysis.Completion.CompletionItem;
using VSCompletionItem = Microsoft.VisualStudio.Language.Intellisense.AsyncCompletion.Data.CompletionItem;

namespace Microsoft.CodeAnalysis.Editor.Implementation.IntelliSense.AsyncCompletion
Expand Down Expand Up @@ -70,14 +69,14 @@ public Task<ImmutableArray<VSCompletionItem>> SortCompletionListAsync(
data = new AsyncCompletionSessionDataSnapshot(sessionData.CombinedSortedList.Value, data.Snapshot, data.Trigger, data.InitialTrigger, data.SelectedFilters,
data.IsSoftSelected, data.DisplaySuggestionItem, data.Defaults);
}
else if (sessionData.ExpandedItemsTask.HasValue)
else if (sessionData.ExpandedItemsTask != null)
{
var task = sessionData.ExpandedItemsTask.Value;
var task = sessionData.ExpandedItemsTask;
if (task.Status == TaskStatus.RanToCompletion)
{
// Make sure the task is removed when Adding expanded items,
// so duplicated items won't be added in subsequent list updates.
sessionData.ExpandedItemsTask = new();
sessionData.ExpandedItemsTask = null;

var (expandedContext, _) = await task.ConfigureAwait(false);
if (expandedContext.Items.Length > 0)
Expand All @@ -89,7 +88,7 @@ public Task<ImmutableArray<VSCompletionItem>> SortCompletionListAsync(
var combinedList = itemsBuilder.MoveToImmutable();

// Add expanded items into a combined list, and save it to be used for future updates during the same session.
sessionData.CombinedSortedList = new(combinedList);
sessionData.CombinedSortedList = combinedList;
var combinedFilterStates = FilterSet.CombineFilterStates(expandedContext.Filters, data.SelectedFilters);

data = new AsyncCompletionSessionDataSnapshot(combinedList, data.Snapshot, data.Trigger, data.InitialTrigger, combinedFilterStates,
Expand Down
6 changes: 3 additions & 3 deletions src/EditorFeatures/Test2/IntelliSense/CSharpCompletionCommandHandlerTests.vb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9702,7 +9702,7 @@ class C

Dim session = Await state.GetCompletionSession()
Dim sessionData = CompletionSessionData.GetOrCreateSessionData(session)
Dim expandTask = sessionData.ExpandedItemsTask.GetValueOrNull()
Dim expandTask = sessionData.ExpandedItemsTask

Assert.NotNull(expandTask)
Assert.False(expandTask.IsCompleted)
Expand Down Expand Up @@ -9761,7 +9761,7 @@ class C

Dim session = Await state.GetCompletionSession()
Dim sessionData = CompletionSessionData.GetOrCreateSessionData(session)
Dim expandTask = sessionData.ExpandedItemsTask.GetValueOrNull()
Dim expandTask = sessionData.ExpandedItemsTask

Assert.NotNull(expandTask)
Assert.False(expandTask.IsCompleted)
Expand Down Expand Up @@ -9821,7 +9821,7 @@ class C

Dim session = Await state.GetCompletionSession()
Dim sessionData = CompletionSessionData.GetOrCreateSessionData(session)
Dim expandTask = sessionData.ExpandedItemsTask.GetValueOrNull()
Dim expandTask = sessionData.ExpandedItemsTask

Assert.NotNull(expandTask)
Assert.False(expandTask.IsCompleted)
Expand Down

0 comments on commit 32069d8

Please sign in to comment.