Skip to content

Commit

Permalink
Merge pull request #74510 from CyrusNajmabadi/sigHelpCrash
Browse files Browse the repository at this point in the history
  • Loading branch information
@CyrusNajmabadi
CyrusNajmabadi authored Jul 23, 2024
2 parents b4808a3 + 62680ba commit 415cea3
Show file tree
Hide file tree
Showing 12 changed files with 119 additions and 77 deletions.
28 changes: 21 additions & 7 deletions src/EditorFeatures/Core.Wpf/SignatureHelp/Controller.Session_ComputeModel.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,8 @@ private async Task<Model> ComputeModelInBackgroundAsync(
currentModel.Provider == provider &&
currentModel.GetCurrentSpanInSubjectBuffer(disconnectedBufferGraph.SubjectBufferSnapshot).Span.Start == items.ApplicableSpan.Start &&
currentModel.Items.IndexOf(currentModel.SelectedItem) == items.SelectedItemIndex &&
currentModel.ArgumentIndex == items.ArgumentIndex &&
currentModel.ArgumentCount == items.ArgumentCount &&
currentModel.SemanticParameterIndex == items.SemanticParameterIndex &&
currentModel.SyntacticArgumentCount == items.SyntacticArgumentCount &&
currentModel.ArgumentName == items.ArgumentName)
{
// The new model is the same as the current model. Return the currentModel
Expand All @@ -112,14 +112,28 @@ private async Task<Model> ComputeModelInBackgroundAsync(

var selectedItem = GetSelectedItem(currentModel, items, provider, out var userSelected);

var model = new Model(disconnectedBufferGraph, items.ApplicableSpan, provider,
items.Items, selectedItem, items.ArgumentIndex, items.ArgumentCount, items.ArgumentName,
selectedParameter: 0, userSelected);
var model = new Model(
disconnectedBufferGraph,
items.ApplicableSpan,
provider,
items.Items,
selectedItem,
items.SemanticParameterIndex,
items.SyntacticArgumentCount,
items.ArgumentName,
selectedParameter: 0,
userSelected);

var syntaxFactsService = document.GetLanguageService<ISyntaxFactsService>();
var isCaseSensitive = syntaxFactsService == null || syntaxFactsService.IsCaseSensitive;
var selection = DefaultSignatureHelpSelector.GetSelection(model.Items,
model.SelectedItem, model.UserSelected, model.ArgumentIndex, model.ArgumentCount, model.ArgumentName, isCaseSensitive);
var selection = DefaultSignatureHelpSelector.GetSelection(
model.Items,
model.SelectedItem,
model.UserSelected,
model.SemanticParameterIndex,
model.SyntacticArgumentCount,
model.ArgumentName,
isCaseSensitive);

return model.WithSelectedItem(selection.SelectedItem, selection.UserSelected)
.WithSelectedParameter(selection.SelectedParameter);
Expand Down
30 changes: 18 additions & 12 deletions src/EditorFeatures/Core.Wpf/SignatureHelp/Controller.Session_UpdateModel.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,13 @@ public static SignatureHelpSelection GetSelection(
IList<SignatureHelpItem> items,
SignatureHelpItem selectedItem,
bool userSelected,
int argumentIndex,
int argumentCount,
int semanticParameterIndex,
int syntacticArgumentCount,
string argumentName,
bool isCaseSensitive)
{
SelectBestItem(ref selectedItem, ref userSelected, items, argumentIndex, argumentCount, argumentName, isCaseSensitive);
var selectedParameter = GetSelectedParameter(selectedItem, argumentIndex, argumentName, isCaseSensitive);
SelectBestItem(ref selectedItem, ref userSelected, items, semanticParameterIndex, syntacticArgumentCount, argumentName, isCaseSensitive);
var selectedParameter = GetSelectedParameter(selectedItem, semanticParameterIndex, argumentName, isCaseSensitive);
return new SignatureHelpSelection(selectedItem, userSelected, selectedParameter);
}

Expand All @@ -67,12 +67,18 @@ private static int GetSelectedParameter(SignatureHelpItem bestItem, int paramete
return parameterIndex;
}

private static void SelectBestItem(ref SignatureHelpItem currentItem, ref bool userSelected,
IList<SignatureHelpItem> filteredItems, int selectedParameter, int argumentCount, string name, bool isCaseSensitive)
private static void SelectBestItem(
ref SignatureHelpItem currentItem,
ref bool userSelected,
IList<SignatureHelpItem> filteredItems,
int semanticParameterIndex,
int syntacticArgumentCount,
string name,
bool isCaseSensitive)
{
// If the current item is still applicable, then just keep it.
if (filteredItems.Contains(currentItem) &&
IsApplicable(currentItem, argumentCount, name, isCaseSensitive))
IsApplicable(currentItem, syntacticArgumentCount, name, isCaseSensitive))
{
// If the current item was user-selected, we keep it as such.
return;
Expand All @@ -86,7 +92,7 @@ private static void SelectBestItem(ref SignatureHelpItem currentItem, ref bool u
// selected parameter was outside the bounds of all methods. i.e. all methods only
// went up to 3 parameters, and selected parameter is 3 or higher. In that case,
// just pick the very last item as it is closest in parameter count.
var result = filteredItems.FirstOrDefault(i => IsApplicable(i, argumentCount, name, isCaseSensitive));
var result = filteredItems.FirstOrDefault(i => IsApplicable(i, syntacticArgumentCount, name, isCaseSensitive));
if (result != null)
{
currentItem = result;
Expand All @@ -97,7 +103,7 @@ private static void SelectBestItem(ref SignatureHelpItem currentItem, ref bool u
// a name.
if (name != null)
{
SelectBestItem(ref currentItem, ref userSelected, filteredItems, selectedParameter, argumentCount, null, isCaseSensitive);
SelectBestItem(ref currentItem, ref userSelected, filteredItems, semanticParameterIndex, syntacticArgumentCount, null, isCaseSensitive);
return;
}

Expand All @@ -112,7 +118,7 @@ private static void SelectBestItem(ref SignatureHelpItem currentItem, ref bool u
currentItem = lastItem;
}

private static bool IsApplicable(SignatureHelpItem item, int argumentCount, string name, bool isCaseSensitive)
private static bool IsApplicable(SignatureHelpItem item, int syntacticArgumentCount, string name, bool isCaseSensitive)
{
// If they provided a name, then the item is only valid if it has a parameter that
// matches that name.
Expand All @@ -126,7 +132,7 @@ private static bool IsApplicable(SignatureHelpItem item, int argumentCount, stri
// parameter index. i.e. if it has 2 parameters and we're at index 0 or 1 then it's
// applicable. However, if it has 2 parameters and we're at index 2, then it's not
// applicable.
if (item.Parameters.Length >= argumentCount)
if (item.Parameters.Length >= syntacticArgumentCount)
{
return true;
}
Expand All @@ -141,7 +147,7 @@ private static bool IsApplicable(SignatureHelpItem item, int argumentCount, stri
// Also, we special case 0. that's because if the user has "Goo(" and goo takes no
// arguments, then we'll see that it's arg count is 0. We still want to consider
// any item applicable here though.
return argumentCount == 0;
return syntacticArgumentCount == 0;
}
}
}
Expand Down
20 changes: 12 additions & 8 deletions src/EditorFeatures/Core.Wpf/SignatureHelp/Model.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,12 @@ internal class Model

/// <summary>UserSelected is true if the SelectedItem is the result of a user selection (up/down arrows).</summary>
public bool UserSelected { get; }
public int ArgumentIndex { get; }
public int ArgumentCount { get; }

/// <inheritdoc cref="SignatureHelpItems.SemanticParameterIndex"/>
public int SemanticParameterIndex { get; }

/// <inheritdoc cref="SignatureHelpItems.SyntacticArgumentCount"/>
public int SyntacticArgumentCount { get; }
public string ArgumentName { get; }
public int? SelectedParameter { get; }
public ISignatureHelpProvider Provider { get; }
Expand All @@ -35,8 +39,8 @@ public Model(
ISignatureHelpProvider provider,
IList<SignatureHelpItem> items,
SignatureHelpItem selectedItem,
int argumentIndex,
int argumentCount,
int semanticParameterIndex,
int syntacticArgumentCount,
string argumentName,
int? selectedParameter,
bool userSelected)
Expand All @@ -51,8 +55,8 @@ public Model(
this.Provider = provider;
this.SelectedItem = selectedItem;
this.UserSelected = userSelected;
this.ArgumentIndex = argumentIndex;
this.ArgumentCount = argumentCount;
this.SemanticParameterIndex = semanticParameterIndex;
this.SyntacticArgumentCount = syntacticArgumentCount;
this.ArgumentName = argumentName;
this.SelectedParameter = selectedParameter;
}
Expand All @@ -61,14 +65,14 @@ public Model WithSelectedItem(SignatureHelpItem selectedItem, bool userSelected)
{
return selectedItem == this.SelectedItem && userSelected == this.UserSelected
? this
: new Model(_disconnectedBufferGraph, TextSpan, Provider, Items, selectedItem, ArgumentIndex, ArgumentCount, ArgumentName, SelectedParameter, userSelected);
: new Model(_disconnectedBufferGraph, TextSpan, Provider, Items, selectedItem, SemanticParameterIndex, SyntacticArgumentCount, ArgumentName, SelectedParameter, userSelected);
}

public Model WithSelectedParameter(int? selectedParameter)
{
return selectedParameter == this.SelectedParameter
? this
: new Model(_disconnectedBufferGraph, TextSpan, Provider, Items, SelectedItem, ArgumentIndex, ArgumentCount, ArgumentName, selectedParameter, UserSelected);
: new Model(_disconnectedBufferGraph, TextSpan, Provider, Items, SelectedItem, SemanticParameterIndex, SyntacticArgumentCount, ArgumentName, selectedParameter, UserSelected);
}

public SnapshotSpan GetCurrentSpanInSubjectBuffer(ITextSnapshot bufferSnapshot)
Expand Down
25 changes: 24 additions & 1 deletion src/EditorFeatures/Test2/IntelliSense/CSharpSignatureHelpCommandHandlerTests.vb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ Namespace Microsoft.CodeAnalysis.Editor.UnitTests.IntelliSense
<[UseExportProvider]>
<Trait(Traits.Feature, Traits.Features.SignatureHelp)>
Public Class CSharpSignatureHelpCommandHandlerTests

<WpfTheory, CombinatorialData>
Public Async Function TestCreateAndDismiss(showCompletionInArgumentLists As Boolean) As Task
Using state = TestStateFactory.CreateCSharpTestState(
Expand Down Expand Up @@ -931,5 +930,29 @@ class C
End Using
End Function

<WpfTheory, CombinatorialData>
<WorkItem("https://github.com/dotnet/roslyn/issues/72012")>
<WorkItem("https://github.com/dotnet/roslyn/issues/74383")>
<WorkItem("https://github.com/dotnet/roslyn/issues/74500")>
Public Async Function TestParameterIndexBeyondSyntacticIndex(showCompletionInArgumentLists As Boolean) As Task
Using state = TestStateFactory.CreateCSharpTestState(
<Document>
class C
{
void Main()
{
M(0, d$$)
}

void M(int a, int b = 0, int c = 0, int d = 0) { }
}
</Document>, showCompletionInArgumentLists:=showCompletionInArgumentLists)

state.SendInvokeSignatureHelp()
Await state.AssertSelectedSignatureHelpItem(displayText:="void C.M(int a, int b = 0, int c = 0, int d = 0)", selectedParameter:="int b = 0")
state.SendTypeChars(":")
Await state.AssertSelectedSignatureHelpItem(displayText:="void C.M(int a, int b = 0, int c = 0, int d = 0)", selectedParameter:="int d = 0")
End Using
End Function
End Class
End Namespace
12 changes: 6 additions & 6 deletions src/EditorFeatures/Test2/IntelliSense/SignatureHelpControllerTests.vb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ Namespace Microsoft.CodeAnalysis.Editor.UnitTests.IntelliSense
slowProvider.Setup(Function(p) p.IsTriggerCharacter(" "c)).Returns(True)
slowProvider.Setup(Function(p) p.IsRetriggerCharacter(" "c)).Returns(True)
slowProvider.Setup(Function(p) p.GetItemsAsync(It.IsAny(Of Document), It.IsAny(Of Integer), It.IsAny(Of SignatureHelpTriggerInfo), options, It.IsAny(Of CancellationToken))) _
.Returns(Task.FromResult(New SignatureHelpItems(CreateItems(2), TextSpan.FromBounds(0, 0), selectedItem:=0, argumentIndex:=0, argumentCount:=0, argumentName:=Nothing)))
.Returns(Task.FromResult(New SignatureHelpItems(CreateItems(2), TextSpan.FromBounds(0, 0), selectedItem:=0, semanticParameterIndex:=0, syntacticArgumentCount:=0, argumentName:=Nothing)))
Dim controller = CreateController(CreateWorkspace(), provider:=slowProvider.Object, waitForPresentation:=True)

' Now force an update to the model that will result in stopping the session
Expand Down Expand Up @@ -115,7 +115,7 @@ Namespace Microsoft.CodeAnalysis.Editor.UnitTests.IntelliSense
slowProvider.Setup(Function(p) p.GetItemsAsync(It.IsAny(Of Document), It.IsAny(Of Integer), It.IsAny(Of SignatureHelpTriggerInfo), options, It.IsAny(Of CancellationToken))) _
.Returns(Function()
mre.WaitOne()
Return Task.FromResult(New SignatureHelpItems(CreateItems(2), TextSpan.FromBounds(0, 0), selectedItem:=0, argumentIndex:=0, argumentCount:=0, argumentName:=Nothing))
Return Task.FromResult(New SignatureHelpItems(CreateItems(2), TextSpan.FromBounds(0, 0), selectedItem:=0, semanticParameterIndex:=0, syntacticArgumentCount:=0, argumentName:=Nothing))
End Function)

GetMocks(controller).PresenterSession.Setup(Sub(p) p.Dismiss())
Expand All @@ -135,7 +135,7 @@ Namespace Microsoft.CodeAnalysis.Editor.UnitTests.IntelliSense
slowProvider.Setup(Function(p) p.GetItemsAsync(It.IsAny(Of Document), It.IsAny(Of Integer), It.IsAny(Of SignatureHelpTriggerInfo), options, It.IsAny(Of CancellationToken))) _
.Returns(Function()
mre.WaitOne()
Return Task.FromResult(New SignatureHelpItems(CreateItems(2), TextSpan.FromBounds(0, 0), selectedItem:=0, argumentIndex:=0, argumentCount:=0, argumentName:=Nothing))
Return Task.FromResult(New SignatureHelpItems(CreateItems(2), TextSpan.FromBounds(0, 0), selectedItem:=0, semanticParameterIndex:=0, syntacticArgumentCount:=0, argumentName:=Nothing))
End Function)

GetMocks(controller).PresenterSession.Setup(Sub(p) p.Dismiss())
Expand All @@ -157,7 +157,7 @@ Namespace Microsoft.CodeAnalysis.Editor.UnitTests.IntelliSense
slowProvider.Setup(Function(p) p.IsTriggerCharacter(" "c)).Returns(True)
slowProvider.Setup(Function(p) p.IsRetriggerCharacter(" "c)).Returns(True)
slowProvider.Setup(Function(p) p.GetItemsAsync(It.IsAny(Of Document), It.IsAny(Of Integer), It.IsAny(Of SignatureHelpTriggerInfo), options, It.IsAny(Of CancellationToken))) _
.Returns(Task.FromResult(New SignatureHelpItems(CreateItems(2), TextSpan.FromBounds(0, 0), selectedItem:=0, argumentIndex:=0, argumentCount:=0, argumentName:=Nothing)))
.Returns(Task.FromResult(New SignatureHelpItems(CreateItems(2), TextSpan.FromBounds(0, 0), selectedItem:=0, semanticParameterIndex:=0, syntacticArgumentCount:=0, argumentName:=Nothing)))

Await threadingContext.JoinableTaskFactory.SwitchToMainThreadAsync()
Dim controller = CreateController(workspace, provider:=slowProvider.Object, waitForPresentation:=True)
Expand All @@ -168,7 +168,7 @@ Namespace Microsoft.CodeAnalysis.Editor.UnitTests.IntelliSense
slowProvider.Setup(Function(p) p.GetItemsAsync(It.IsAny(Of Document), It.IsAny(Of Integer), It.IsAny(Of SignatureHelpTriggerInfo), options, It.IsAny(Of CancellationToken))) _
.Returns(Function()
checkpoint.Task.Wait()
Return Task.FromResult(New SignatureHelpItems(CreateItems(2), TextSpan.FromBounds(0, 2), selectedItem:=0, argumentIndex:=0, argumentCount:=0, argumentName:=Nothing))
Return Task.FromResult(New SignatureHelpItems(CreateItems(2), TextSpan.FromBounds(0, 2), selectedItem:=0, semanticParameterIndex:=0, syntacticArgumentCount:=0, argumentName:=Nothing))
End Function)

Await threadingContext.JoinableTaskFactory.SwitchToMainThreadAsync()
Expand Down Expand Up @@ -355,7 +355,7 @@ Namespace Microsoft.CodeAnalysis.Editor.UnitTests.IntelliSense
Public Function GetItemsAsync(document As Document, position As Integer, triggerInfo As SignatureHelpTriggerInfo, options As MemberDisplayOptions, cancellationToken As CancellationToken) As Task(Of SignatureHelpItems) Implements ISignatureHelpProvider.GetItemsAsync
GetItemsCount += 1
Return Task.FromResult(If(_items.Any(),
New SignatureHelpItems(_items, TextSpan.FromBounds(position, position), selectedItem:=0, argumentIndex:=0, argumentCount:=0, argumentName:=Nothing),
New SignatureHelpItems(_items, TextSpan.FromBounds(position, position), selectedItem:=0, semanticParameterIndex:=0, syntacticArgumentCount:=0, argumentName:=Nothing),
Nothing))
End Function

Expand Down
Loading

0 comments on commit 415cea3

Please sign in to comment.