Merge pull request #2577 from vweijsters/fix-2572

Fixed false positive for ambiguous method overloads in SA1410

Author: sharwell

StyleCop.Analyzers/StyleCop.Analyzers.Test/MaintainabilityRules/SA1410UnitTests.cs

@@ -99,6 +99,134 @@ public void Bar()
             await this.VerifyCSharpFixAsync(oldSource, newSource, cancellationToken: CancellationToken.None).ConfigureAwait(false);
         }
 
+        /// <summary>
+        /// Verify that no diagnostic is produced when removing the parentheses from the delegate statement
+        /// would result in ambiguity on which method overload to call.
+        /// </summary>
+        /// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
+        [Fact]
+        [WorkItem(2572, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/2572")]
+        public async Task TestMethodOverloadAmbiguityAsync()
+        {
+            var testCode = @"public class TestClass
+{
+    public delegate bool Delegate1();
+    public delegate bool Delegate2(int x);
+
+    public void TestMethod()
+    {
+        var thread = new System.Threading.Thread(delegate()
+        {
+            // ...
+        });
+
+        this.TestMethod2(delegate() { return true; });
+        this.TestMethod3(delegate() { return true; });
+    }
+
+    public void TestMethod2(Delegate1 d)
+    {
+    }
+
+    public void TestMethod2(Delegate2 d)
+    {
+    }
+}
+
+public static class TestClassExtensions
+{
+    public static void TestMethod3(this TestClass obj, TestClass.Delegate1 d)
+    {
+    }
+
+    public static void TestMethod3(this TestClass obj, TestClass.Delegate2 d)
+    {
+    }
+}
+";
+
+            await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
+        }
+
+        /// <summary>
+        /// Verify that the ambiguity detection is specific enough
+        /// </summary>
+        /// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
+        [Fact]
+        [WorkItem(2572, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/2572")]
+        public async Task TestMethodOverloadNonAmbiguityAsync()
+        {
+            var testCode = @"public class TestClass
+{
+    public delegate bool Delegate1();
+    public delegate bool Delegate2(int x);
+
+    public void TestMethod()
+    {
+        this.TestMethod2(delegate() { return true; });
+        this.TestMethod3(0, delegate() { return true; });
+        this.TestMethod4(delegate() { return true; }, 1);
+        this.TestMethod5(delegate() { return true; });
+        this.TestMethod6(delegate() { return true; });
+    }
+
+    public void TestMethod2(Delegate1 d)
+    {
+    }
+
+    public void TestMethod2(int d)
+    {
+    }
+
+    public void TestMethod3(int x, Delegate1 d)
+    {
+    }
+
+    public void TestMethod3(double x, Delegate2 d)
+    {
+    }
+
+    public void TestMethod4(Delegate1 d, int x)
+    {
+    }
+
+    public void TestMethod4(Delegate2 d, double x)
+    {
+    }
+
+    public void TestMethod5(Delegate1 d)
+    {
+    }
+
+    public void TestMethod5(Delegate2 d, double x)
+    {
+    }
+
+    public void TestMethod6(Delegate1 d)
+    {
+    }
+}
+
+public static class TestClassExtensions
+{
+    public static void TestMethod6(this TestClass obj, TestClass.Delegate2 d)
+    {
+    }
+}
+";
+
+            DiagnosticResult[] expectedResults =
+            {
+                this.CSharpDiagnostic().WithLocation(8, 34),
+                this.CSharpDiagnostic().WithLocation(9, 37),
+                this.CSharpDiagnostic().WithLocation(10, 34),
+                this.CSharpDiagnostic().WithLocation(11, 34),
+                this.CSharpDiagnostic().WithLocation(12, 34),
+            };
+
+            await this.VerifyCSharpDiagnosticAsync(testCode, expectedResults, CancellationToken.None).ConfigureAwait(false);
+        }
+
         protected override IEnumerable<DiagnosticAnalyzer> GetCSharpDiagnosticAnalyzers()
         {
             yield return new SA1410RemoveDelegateParenthesisWhenPossible();

StyleCop.Analyzers/StyleCop.Analyzers/MaintainabilityRules/SA1410RemoveDelegateParenthesisWhenPossible.cs

@@ -81,8 +81,34 @@ private static void HandleAnonymousMethodExpression(SyntaxNodeAnalysisContext co
                 return;
             }
 
+            // if the delegate is passed as a parameter, verify that there is no ambiguity.
+            if (syntax.Parent.IsKind(SyntaxKind.Argument))
+            {
+                var argumentSyntax = (ArgumentSyntax)syntax.Parent;
+                var argumentListSyntax = (ArgumentListSyntax)argumentSyntax.Parent;
+
+                switch (argumentListSyntax.Parent.Kind())
+                {
+                case SyntaxKind.ObjectCreationExpression:
+                case SyntaxKind.InvocationExpression:
+                    if (HasAmbiguousOverload(context, syntax, argumentListSyntax.Parent))
+                    {
+                        return;
+                    }
+
+                    break;
+                }
+            }
+
             // Remove delegate parenthesis when possible
             context.ReportDiagnostic(Diagnostic.Create(Descriptor, syntax.ParameterList.GetLocation()));
         }
+
+        private static bool HasAmbiguousOverload(SyntaxNodeAnalysisContext context, AnonymousMethodExpressionSyntax anonymousMethodExpression, SyntaxNode methodCallSyntax)
+        {
+            var nodeForSpeculation = methodCallSyntax.ReplaceNode(anonymousMethodExpression, anonymousMethodExpression.WithParameterList(null));
+            var speculativeSymbolInfo = context.SemanticModel.GetSpeculativeSymbolInfo(methodCallSyntax.SpanStart, nodeForSpeculation, SpeculativeBindingOption.BindAsExpression);
+            return speculativeSymbolInfo.Symbol == null;
+        }
     }
 }