Fixed false positive for ambiguous method overloads in SA1410

Author: vweijsters

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

@@ -99,6 +99,109 @@ 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; });
+    }
+
+    public void TestMethod2(Delegate1 d)
+    {
+    }
+
+    public void TestMethod2(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; });
+    }
+
+    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)
+    {
+    }
+}
+";
+
+            DiagnosticResult[] expectedResults =
+            {
+                this.CSharpDiagnostic().WithLocation(8, 34),
+                this.CSharpDiagnostic().WithLocation(9, 37),
+                this.CSharpDiagnostic().WithLocation(10, 34),
+                this.CSharpDiagnostic().WithLocation(11, 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

@@ -5,6 +5,8 @@ namespace StyleCop.Analyzers.MaintainabilityRules
 {
     using System;
     using System.Collections.Immutable;
+    using System.Linq;
+    using System.Threading;
     using Microsoft.CodeAnalysis;
     using Microsoft.CodeAnalysis.CSharp;
     using Microsoft.CodeAnalysis.CSharp.Syntax;
@@ -81,8 +83,59 @@ 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, argumentListSyntax.Arguments.IndexOf(argumentSyntax), argumentListSyntax.Parent))
+                    {
+                        return;
+                    }
+
+                    break;
+                }
+            }
+
             // Remove delegate parenthesis when possible
             context.ReportDiagnostic(Diagnostic.Create(Descriptor, syntax.ParameterList.GetLocation()));
         }
+
+        private static bool HasAmbiguousOverload(SyntaxNodeAnalysisContext context, int parameterIndex, SyntaxNode methodCallSyntax)
+        {
+            var methodSymbol = (IMethodSymbol)context.SemanticModel.GetSymbolInfo(methodCallSyntax, context.CancellationToken).Symbol;
+
+            var nameOverloads = methodSymbol.ContainingType.GetMembers(methodSymbol.Name);
+            var parameterCountMatchingOverloads = nameOverloads.OfType<IMethodSymbol>().Where(symbol => (symbol != methodSymbol) && (symbol.Parameters.Length == methodSymbol.Parameters.Length));
+
+            foreach (var overload in parameterCountMatchingOverloads)
+            {
+                var isAmbiguousOverload = true;
+
+                for (var i = 0; isAmbiguousOverload && (i < methodSymbol.Parameters.Length); i++)
+                {
+                    if (i == parameterIndex)
+                    {
+                        isAmbiguousOverload = overload.Parameters[i].Type.TypeKind == TypeKind.Delegate;
+                    }
+                    else
+                    {
+                        isAmbiguousOverload = methodSymbol.Parameters[i].Type == overload.Parameters[i].Type;
+                    }
+                }
+
+                if (isAmbiguousOverload)
+                {
+                    return true;
+                }
+            }
+
+            return false;
+        }
     }
 }