Merge pull request #30997 from genlu/fixSignatureHelper

Fix IndexOutOfRange exception in InvocationExpressionSignatureHelpProvider
dotnet · Nov 8, 2018 · 86d7c79 · 86d7c79
2 parents 151e59e + bc7f565
commit 86d7c79
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 9 deletions.
diff --git a/...EditorFeatures/CSharpTest/SignatureHelp/InvocationExpressionSignatureHelpProviderTests.cs b/...EditorFeatures/CSharpTest/SignatureHelp/InvocationExpressionSignatureHelpProviderTests.cs
@@ -2365,5 +2365,34 @@ int Goo(object x)
 
             await TestAsync(markup, expectedOrderedItems, usePreviousCharAsTrigger: true);
         }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.SignatureHelp)]
+        public async Task PickCorrectOverload_WithCorrectSelectionAfterFilteringOutNoApplicableItems()
+        {
+            var markup = @"
+class Comparer
+{
+    public static bool Equals(object x, object y) => true;
+    public bool Equals(object x) => true;
+    public bool Equals(string x, string y) => true;
+}
+
+class Program
+{
+    static void Main(string x, string y)
+    {
+        var comparer = new Comparer();
+        comparer.Equals(x, y$$);
+    }
+}";
+
+            var expectedOrderedItems = new List<SignatureHelpTestItem>
+            {
+                new SignatureHelpTestItem("bool Comparer.Equals(object x)", currentParameterIndex: 1),
+                new SignatureHelpTestItem("bool Comparer.Equals(string x, string y)", currentParameterIndex: 1, isSelected: true),
+            };
+
+            await TestAsync(markup, expectedOrderedItems);
+        }
     }
 }
diff --git a/src/Features/CSharp/Portable/SignatureHelp/InvocationExpressionSignatureHelpProvider.cs b/src/Features/CSharp/Portable/SignatureHelp/InvocationExpressionSignatureHelpProvider.cs
@@ -95,10 +95,11 @@ protected override async Task<SignatureHelpItems> GetItemsWorkerAsync(Document d
 
             if (methodGroup.Any())
             {
-                var selectedItem = TryGetSelectedIndex(methodGroup, symbolInfo);
+                var (items, selectedItem) =
+                    GetMethodGroupItemsAndSelection(invocationExpression, semanticModel, symbolDisplayService, anonymousTypeDisplayService, documentationCommentFormattingService, within, methodGroup, symbolInfo, cancellationToken);
 
                 return CreateSignatureHelpItems(
-                    GetMethodGroupItems(invocationExpression, semanticModel, symbolDisplayService, anonymousTypeDisplayService, documentationCommentFormattingService, within, methodGroup, cancellationToken),
+                    items,
                     textSpan,
                     GetCurrentArgumentState(root, position, syntaxFacts, textSpan, cancellationToken),
                     selectedItem);

diff --git a/...es/CSharp/Portable/SignatureHelp/InvocationExpressionSignatureHelpProvider_MethodGroup.cs b/...es/CSharp/Portable/SignatureHelp/InvocationExpressionSignatureHelpProvider_MethodGroup.cs
@@ -16,14 +16,15 @@ namespace Microsoft.CodeAnalysis.CSharp.SignatureHelp
 {
     internal partial class InvocationExpressionSignatureHelpProvider
     {
-        private IList<SignatureHelpItem> GetMethodGroupItems(
+        private (IList<SignatureHelpItem>, int?) GetMethodGroupItemsAndSelection(
             InvocationExpressionSyntax invocationExpression,
             SemanticModel semanticModel,
             ISymbolDisplayService symbolDisplayService,
             IAnonymousTypeDisplayService anonymousTypeDisplayService,
             IDocumentationCommentFormattingService documentationCommentFormattingService,
             ISymbol within,
             IEnumerable<IMethodSymbol> methodGroup,
+            SymbolInfo currentSymbol,
             CancellationToken cancellationToken)
         {
             ITypeSymbol throughType = null;
@@ -58,17 +59,18 @@ private IList<SignatureHelpItem> GetMethodGroupItems(
                 methodGroup = methodGroup.Where(m => m.IsStatic);
             }
 
-            var accessibleMethods = methodGroup.Where(m => m.IsAccessibleWithin(within, throughTypeOpt: throughType)).ToList();
-            if (accessibleMethods.Count == 0)
+            var accessibleMethods = methodGroup.Where(m => m.IsAccessibleWithin(within, throughTypeOpt: throughType)).ToImmutableArrayOrEmpty();
+            if (accessibleMethods.Length == 0)
             {
-                return null;
+                return default;
             }
 
             var methodSet = accessibleMethods.ToSet();
-            accessibleMethods = accessibleMethods.Where(m => !IsHiddenByOtherMethod(m, methodSet)).ToList();
+            accessibleMethods = accessibleMethods.Where(m => !IsHiddenByOtherMethod(m, methodSet)).ToImmutableArrayOrEmpty();
 
-            return accessibleMethods.Select(m =>
-                ConvertMethodGroupMethod(m, invocationExpression, semanticModel, symbolDisplayService, anonymousTypeDisplayService, documentationCommentFormattingService, cancellationToken)).ToList();
+            return (accessibleMethods.Select(m =>
+                ConvertMethodGroupMethod(m, invocationExpression, semanticModel, symbolDisplayService, anonymousTypeDisplayService, documentationCommentFormattingService, cancellationToken)).ToList(),
+                TryGetSelectedIndex(accessibleMethods, currentSymbol));
         }
 
         private bool IsHiddenByOtherMethod(IMethodSymbol method, ISet<IMethodSymbol> methodSet)