Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
using Microsoft.CodeAnalysis.Collections;
using Microsoft.CodeAnalysis.Completion;
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.PatternMatching;
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.Text;
Expand Down Expand Up @@ -159,7 +160,7 @@ public CompletionListUpdater(
// take into consideration for things like CompletionTrigger, MatchPriority, MRU, etc.
var initialSelection = InitialTriggerReason == CompletionTriggerReason.Backspace || InitialTriggerReason == CompletionTriggerReason.Deletion
? HandleDeletionTrigger(itemsToBeIncluded, cancellationToken)
: HandleNormalFiltering(itemsToBeIncluded, cancellationToken);
: await HandleNormalFilteringAsync(itemsToBeIncluded, cancellationToken).ConfigureAwait(false);

if (!initialSelection.HasValue)
return null;
Expand Down Expand Up @@ -339,7 +340,7 @@ int DefaultIndexToFabricatedOriginalSortedIndex(int i)
=> i - _snapshotData.Defaults.Length;
}

private ItemSelection? HandleNormalFiltering(IReadOnlyList<MatchResult> matchResults, CancellationToken cancellationToken)
private async Task<ItemSelection?> HandleNormalFilteringAsync(IReadOnlyList<MatchResult> matchResults, CancellationToken cancellationToken)
{
Debug.Assert(matchResults.Count > 0);
var filteredMatchResultsBuilder = s_listOfMatchResultPool.Allocate();
Expand Down Expand Up @@ -428,7 +429,7 @@ int DefaultIndexToFabricatedOriginalSortedIndex(int i)
return null;
}

var isHardSelection = IsHardSelection(bestOrFirstMatchResult.CompletionItem, bestOrFirstMatchResult.ShouldBeConsideredMatchingFilterText);
var isHardSelection = await IsHardSelectionAsync(bestOrFirstMatchResult, cancellationToken).ConfigureAwait(false);
var updateSelectionHint = isHardSelection ? UpdateSelectionHint.Selected : UpdateSelectionHint.SoftSelected;

return new(selectedItemIndex, updateSelectionHint, uniqueItem);
Expand Down Expand Up @@ -755,13 +756,15 @@ private static bool TryGetInitialTriggerLocation(AsyncCompletionSessionDataSnaps
return false;
}

private bool IsHardSelection(RoslynCompletionItem item, bool matchedFilterText)
private async Task<bool> IsHardSelectionAsync(MatchResult selectedItem, CancellationToken cancellationToken)
{
if (_hasSuggestedItemOptions)
{
return false;
}

var item = selectedItem.CompletionItem;

// We don't have a builder and we have a best match. Normally this will be hard
// selected, except for a few cases. Specifically, if no filter text has been
// provided, and this is not a preselect match then we will soft select it. This
Expand Down Expand Up @@ -808,11 +811,21 @@ private bool IsHardSelection(RoslynCompletionItem item, bool matchedFilterText)

// If the user moved the caret left after they started typing, the 'best' match may not match at all
// against the full text span that this item would be replacing.
if (!matchedFilterText)
if (!selectedItem.ShouldBeConsideredMatchingFilterText)
{
return false;
}

// When trying to type something like `public TBuilder GetBuilder<TBuilder>()`, right after `public TBuilder$` is typed
// We don't want an item like `TypeBuilder` to be hard-selected. Otherwise, typing `space` would automatically change `TBuilder` to `TypeBuilder`,
if (_completionService is not null &&
MatchesTypeParameterPattern(_filterText) &&
selectedItem.PatternMatch.HasValue &&
selectedItem.PatternMatch.Value.Kind > PatternMatchKind.Prefix)
{
return !await _completionService.IsSpeculativeTypeParameterContextAsync(_document!, item.Span.Start, cancellationToken).ConfigureAwait(false);
}

// There was either filter text, or this was a preselect match. In either case, we
// can hard select this.
return true;
Expand All @@ -830,6 +843,16 @@ private static bool IsPotentialFilterCharacter(char c)
|| c == '_';
}

private static bool MatchesTypeParameterPattern(string text)
{
if (string.IsNullOrEmpty(text))
return false;

// This is just a very simple heuristic to catch common cases where user is typing a type parameter name in .NET
// Pattern: starts with 'T', and optionally followed by an uppercase letter
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps mention this is just a weak heuristic. But it helps given general .Net naming patterns.

return text == "T" || text.Length >= 2 && text[0] == 'T' && char.IsUpper(text[1]);
}

private ItemSelection UpdateSelectionBasedOnSuggestedDefaults(IReadOnlyList<MatchResult> items, ItemSelection itemSelection, CancellationToken cancellationToken)
{
// Editor doesn't provide us a list of "default" items, or we select SuggestionItem (because we are in suggestion mode and have no match in the list)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13210,5 +13210,124 @@ public class TestClass1
Await state.AssertSelectedCompletionItem("ref")
End Using
End Function

<WpfTheory, CombinatorialData>
Public Async Function TestTypeParameterPattern_InMemberDeclaration(
showCompletionInArgumentLists As Boolean,
<CombinatorialValues("TBu", "Func<TBu")> typeParameter As String) As Task ' not testing "(TBu" here because `TBuilder` would not be in the completion list in this case
Using state = TestStateFactory.CreateCSharpTestState(
<Document>
public class TypeBuilder { }
public class C
{
public <%= typeParameter %>$$ GetBuilder&lt;TBuilder&gt;() { }
}
</Document>,
showCompletionInArgumentLists:=showCompletionInArgumentLists)

state.SendInvokeCompletionList()
Await state.AssertSelectedCompletionItem(displayText:="TBuilder", isHardSelected:=True) ' hard-select TBuilder type parameter (which is in the completion list)
End Using
End Function

<WpfTheory, CombinatorialData>
Public Async Function TestTypeParameterPatternInTuple_InMemberDeclaration(
showCompletionInArgumentLists As Boolean) As Task
Using state = TestStateFactory.CreateCSharpTestState(
<Document>
public class TypeBuilder { }
public class C
{
public (TBu$$ GetBuilder&lt;TBuilder&gt;() { }
}
</Document>,
showCompletionInArgumentLists:=showCompletionInArgumentLists)

state.SendInvokeCompletionList()
Await state.AssertSelectedCompletionItem(displayText:="TypeBuilder", isSoftSelected:=True) ' soft-select TypeBuilder type parameter (because `TBuilder` is not in the completion list)
End Using
End Function

<WpfTheory, CombinatorialData>
Public Async Function TestSpeculativeTypeParameterPattern_InMemberDeclaration(
showCompletionInArgumentLists As Boolean,
<CombinatorialValues("TBu", "(TBu", "Func<TBu", "delegate TBu")> typeParameter As String) As Task
Using state = TestStateFactory.CreateCSharpTestState(
<Document>
public class TypeBuilder { }
public class C
{
public <%= typeParameter %>$$
}
</Document>,
showCompletionInArgumentLists:=showCompletionInArgumentLists)

state.SendInvokeCompletionList()
Await state.AssertSelectedCompletionItem(displayText:="TypeBuilder", isSoftSelected:=True) ' soft-select TypeBuilder class
End Using
End Function

<WpfTheory, CombinatorialData>
Public Async Function TestSpeculativeTypeParameterPattern_InMemberDeclarationNoModifier(
showCompletionInArgumentLists As Boolean,
<CombinatorialValues("TBu", "(TBu", "Func<TBu", "delegate TBu")> typeParameter As String) As Task
Using state = TestStateFactory.CreateCSharpTestState(
<Document>
public class TypeBuilder { }
public class C
{
<%= typeParameter %>$$
}
</Document>,
showCompletionInArgumentLists:=showCompletionInArgumentLists)

state.SendInvokeCompletionList()
Await state.AssertSelectedCompletionItem(displayText:="TypeBuilder", isSoftSelected:=True) ' soft-select TypeBuilder class
End Using
End Function

<WpfTheory, CombinatorialData>
Public Async Function TestTypeParameterPattern_InStatement(
showCompletionInArgumentLists As Boolean,
<CombinatorialValues("TBu", "(TBu", "Func<TBu")> typeParameter As String) As Task
Using state = TestStateFactory.CreateCSharpTestState(
<Document>
public class TypeBuilder { }
public class C
{
public bool M&lt;TBuilder&gt;()
{
<%= typeParameter %>$$
}
}
</Document>,
showCompletionInArgumentLists:=showCompletionInArgumentLists)

state.SendInvokeCompletionList()
Await state.AssertSelectedCompletionItem(displayText:="TBuilder", isHardSelected:=True) ' hard-select TBuilder type parameter
End Using
End Function

<WpfTheory, CombinatorialData>
Public Async Function TestSpeculativeTypeParameterPattern_InStatement(
showCompletionInArgumentLists As Boolean,
<CombinatorialValues("TBu", "(TBu", "Func<TBu")> typeParameter As String) As Task
Using state = TestStateFactory.CreateCSharpTestState(
<Document>
public class TypeBuilder { }
public class C
{
public bool M()
{
<%= typeParameter %>$$
}
}
</Document>,
showCompletionInArgumentLists:=showCompletionInArgumentLists)

state.SendInvokeCompletionList()
Await state.AssertSelectedCompletionItem(displayText:="TypeBuilder", isHardSelected:=True) ' hard-select TypeBuilder class
End Using
End Function
End Class
End Namespace
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@
using System;
using System.Composition;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Completion;
using Microsoft.CodeAnalysis.CSharp.Completion.Providers;
using Microsoft.CodeAnalysis.Host;
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.Shared.TestHooks;
using Microsoft.CodeAnalysis.Text;

Expand Down Expand Up @@ -65,4 +67,22 @@ internal override CompletionRules GetRules(CompletionOptions options)

return newRules;
}

internal override async Task<bool> IsSpeculativeTypeParameterContextAsync(Document document, int position, CancellationToken cancellationToken)
{
var syntaxTree = await document.GetRequiredSyntaxTreeAsync(cancellationToken).ConfigureAwait(false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doc why 'true' for inMemberDeclarationOnly


// Because it's less likely the user wants to type a (undeclared) type parameter when they are inside a method body, treating them so
// might intefere with user intention. For example, while it's fine to provide a speculative `T` item in a statement context,
// since typing 2 characters would filter it out, but for selection, we don't want to soft-select item `TypeBuilder`after `TB`
// is typed in the example below (as if user want to add `TBuilder` to method declaration later):
//
// class C
// {
// void M()
// {
// TB$$
// }
return CompletionUtilities.IsSpeculativeTypeParameterContext(syntaxTree, position, semanticModel: null, includeStatementContexts: false, cancellationToken);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
using System.Threading;
using Microsoft.CodeAnalysis.Completion;
using Microsoft.CodeAnalysis.CSharp.Extensions;
using Microsoft.CodeAnalysis.CSharp.Extensions.ContextQuery;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.CSharp.Utilities;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.Shared.Extensions.ContextQuery;
using Microsoft.CodeAnalysis.Text;
Expand Down Expand Up @@ -238,4 +240,123 @@ public static TextSpan GetTargetSelectionSpanForInsertedMember(SyntaxNode caretT
throw ExceptionUtilities.Unreachable();
}
}

/// <summary>
/// Determines whether the specified position in the syntax tree is a valid context for speculatively typing
/// a type parameter (which might be undeclared yet). This handles cases where the user may be in the middle of typing a generic type, tuple
/// and ref type as well.
///
/// For example, when you typed `public TBuilder$$`, you might want to type `public TBuilder M&lt;TBuilder&gt;(){}`,
/// so TBuilder is a valid speculative type parameter context.
/// </summary>
public static bool IsSpeculativeTypeParameterContext(SyntaxTree syntaxTree, int position, SemanticModel? semanticModel, bool includeStatementContexts, CancellationToken cancellationToken)
{
var spanStart = position;

// We could be in the middle of a ref/generic/tuple type, instead of a simple T case.
// If we managed to walk out and get a different SpanStart, we treat it as a simple $$T case.
while (true)
{
var oldSpanStart = spanStart;

spanStart = WalkOutOfGenericType(syntaxTree, spanStart, semanticModel, cancellationToken);
spanStart = WalkOutOfTupleType(syntaxTree, spanStart, cancellationToken);
spanStart = WalkOutOfRefType(syntaxTree, spanStart, cancellationToken);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you make these helpers into local functions, you can just rewrite this as:

spanStart = WalkOutOfGeneric(WalkOutOfTuple(WalkOutOfRef(spanStart)))


if (spanStart == oldSpanStart)
{
break;
}
}

var token = syntaxTree.FindTokenOnLeftOfPosition(spanStart, cancellationToken);

// Always want to allow in member declaration and delegate return type context, for example:
// class C
// {
// public T$$
// }
//
// delegate T$$
if (syntaxTree.IsMemberDeclarationContext(spanStart, context: null, SyntaxKindSet.AllMemberModifiers, SyntaxKindSet.NonEnumTypeDeclarations, canBePartial: true, cancellationToken) ||
syntaxTree.IsGlobalMemberDeclarationContext(spanStart, SyntaxKindSet.AllGlobalMemberModifiers, cancellationToken) ||
syntaxTree.IsDelegateReturnTypeContext(spanStart, token))
{
return true;
}

// Because it's less likely the user wants to type a (undeclared) type parameter when they are inside a method body, treating them so
// might intefere with user intention. For example, while it's fine to provide a speculative `T` item in a statement context,
// since typing 2 characters would filter it out, but for selection, we don't want to soft-select item `TypeBuilder`after `TB`
// is typed in the example below (as if user want to add `TBuilder` to method declaration later):
//
// class C
// {
// void M()
// {
// TB$$
// }
if (includeStatementContexts)
{
return syntaxTree.IsStatementContext(spanStart, token, cancellationToken) ||
syntaxTree.IsGlobalStatementContext(spanStart, cancellationToken);
}

return false;

static int WalkOutOfGenericType(SyntaxTree syntaxTree, int position, SemanticModel? semanticModel, CancellationToken cancellationToken)
{
var spanStart = position;
var token = syntaxTree.FindTokenOnLeftOfPosition(position, cancellationToken);

if (syntaxTree.IsGenericTypeArgumentContext(position, token, cancellationToken, semanticModel))
{
if (syntaxTree.IsInPartiallyWrittenGeneric(spanStart, cancellationToken, out var nameToken))
{
spanStart = nameToken.SpanStart;
}

// If the user types Goo<T, automatic brace completion will insert the close brace
// and the generic won't be "partially written".
if (spanStart == position)
{
spanStart = token.GetAncestor<GenericNameSyntax>()?.SpanStart ?? spanStart;
}

var tokenLeftOfGenericName = syntaxTree.FindTokenOnLeftOfPosition(spanStart, cancellationToken);
if (tokenLeftOfGenericName.IsKind(SyntaxKind.DotToken) && tokenLeftOfGenericName.Parent.IsKind(SyntaxKind.QualifiedName))
{
spanStart = tokenLeftOfGenericName.Parent.SpanStart;
}
}

return spanStart;
}

static int WalkOutOfRefType(SyntaxTree syntaxTree, int position, CancellationToken cancellationToken)
{
var prevToken = syntaxTree.FindTokenOnLeftOfPosition(position, cancellationToken)
.GetPreviousTokenIfTouchingWord(position);

if (prevToken.Kind() is SyntaxKind.RefKeyword or SyntaxKind.ReadOnlyKeyword && prevToken.Parent.IsKind(SyntaxKind.RefType))
{
return prevToken.SpanStart;
}

return position;
}

static int WalkOutOfTupleType(SyntaxTree syntaxTree, int position, CancellationToken cancellationToken)
{
var prevToken = syntaxTree.FindTokenOnLeftOfPosition(position, cancellationToken)
.GetPreviousTokenIfTouchingWord(position);

if (prevToken.IsPossibleTupleOpenParenOrComma())
{
return prevToken.Parent!.SpanStart;
}

return position;
}
}
}
Loading
Loading