Merge pull request #74510 from CyrusNajmabadi/sigHelpCrash

Author: CyrusNajmabadi

src/EditorFeatures/Core.Wpf/SignatureHelp/Controller.Session_ComputeModel.cs

@@ -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
@@ -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);

src/EditorFeatures/Core.Wpf/SignatureHelp/Controller.Session_UpdateModel.cs

@@ -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);
                 }
 
@@ -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;
@@ -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;
@@ -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;
                     }
 
@@ -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.
@@ -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;
                     }
@@ -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;
                 }
             }
         }

src/EditorFeatures/Core.Wpf/SignatureHelp/Model.cs

@@ -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; }
@@ -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)
@@ -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;
         }
@@ -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)

src/EditorFeatures/Test2/IntelliSense/CSharpSignatureHelpCommandHandlerTests.vb

@@ -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(
@@ -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

src/EditorFeatures/Test2/IntelliSense/SignatureHelpControllerTests.vb

@@ -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
@@ -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())
@@ -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())
@@ -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)
@@ -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()
@@ -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