Merge pull request #2703 from vweijsters/fix-2593

vweijsters · web-flow · commit 1147afb331fe · 2018-06-04T20:01:50.000+02:00
Made SA1130 codefix properly expand parameters when only delegate key…
diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/ReadabilityRules/SA1130CodeFixProvider.cs b/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/ReadabilityRules/SA1130CodeFixProvider.cs
@@ -3,6 +3,7 @@
 
 namespace StyleCop.Analyzers.ReadabilityRules
 {
+    using System;
     using System.Collections.Generic;
     using System.Collections.Immutable;
     using System.Composition;
@@ -24,6 +25,8 @@ namespace StyleCop.Analyzers.ReadabilityRules
     [Shared]
     internal class SA1130CodeFixProvider : CodeFixProvider
     {
+        private static readonly SyntaxToken ParameterListSeparator = SyntaxFactory.Token(SyntaxKind.CommaToken).WithTrailingTrivia(SyntaxFactory.Space);
+
         /// <inheritdoc/>
         public override ImmutableArray<string> FixableDiagnosticIds { get; } =
             ImmutableArray.Create(SA1130UseLambdaSyntax.DiagnosticId);
@@ -50,14 +53,38 @@ public override Task RegisterCodeFixesAsync(CodeFixContext context)
             return SpecializedTasks.CompletedTask;
         }
 
-        private static SyntaxNode ReplaceWithLambda(AnonymousMethodExpressionSyntax anonymousMethod)
+        private static SyntaxNode ReplaceWithLambda(SemanticModel semanticModel, AnonymousMethodExpressionSyntax anonymousMethod)
         {
             var parameterList = anonymousMethod.ParameterList;
             SyntaxNode lambdaExpression;
 
             if (parameterList == null)
             {
-                parameterList = SyntaxFactory.ParameterList()
+                ImmutableArray<string> argumentList = default(ImmutableArray<string>);
+
+                switch (anonymousMethod.Parent.Kind())
+                {
+                case SyntaxKind.Argument:
+                    argumentList = GetMethodInvocationArgumentList(semanticModel, anonymousMethod);
+                    break;
+
+                case SyntaxKind.EqualsValueClause:
+                    argumentList = GetEqualsArgumentList(semanticModel, anonymousMethod);
+                    break;
+
+                case SyntaxKind.AddAssignmentExpression:
+                case SyntaxKind.SubtractAssignmentExpression:
+                    argumentList = GetAssignmentArgumentList(semanticModel, anonymousMethod);
+                    break;
+                }
+
+                List<ParameterSyntax> parameters = GenerateUniqueParameterNames(semanticModel, anonymousMethod, argumentList);
+
+                var newList = (parameters.Count > 0)
+                    ? SyntaxFactory.SeparatedList(parameters, Enumerable.Repeat(ParameterListSeparator, parameters.Count - 1))
+                    : SyntaxFactory.SeparatedList<ParameterSyntax>();
+
+                parameterList = SyntaxFactory.ParameterList(newList)
                     .WithLeadingTrivia(anonymousMethod.DelegateKeyword.LeadingTrivia)
                     .WithTrailingTrivia(anonymousMethod.DelegateKeyword.TrailingTrivia);
             }
@@ -106,15 +133,74 @@ private static SyntaxNode ReplaceWithLambda(AnonymousMethodExpressionSyntax anon
                 .WithAdditionalAnnotations(Formatter.Annotation);
         }
 
+        private static ImmutableArray<string> GetMethodInvocationArgumentList(SemanticModel semanticModel, AnonymousMethodExpressionSyntax anonymousMethod)
+        {
+            var argumentSyntax = (ArgumentSyntax)anonymousMethod.Parent;
+            var argumentListSyntax = (ArgumentListSyntax)argumentSyntax.Parent;
+            var originalInvocationExpression = (InvocationExpressionSyntax)argumentListSyntax.Parent;
+
+            var originalSymbolInfo = semanticModel.GetSymbolInfo(originalInvocationExpression);
+            var argumentIndex = argumentListSyntax.Arguments.IndexOf(argumentSyntax);
+            var parameterList = SA1130UseLambdaSyntax.GetDelegateParameterList((IMethodSymbol)originalSymbolInfo.Symbol, argumentIndex);
+            return parameterList.Parameters.Select(p => p.Identifier.ToString()).ToImmutableArray();
+        }
+
+        private static ImmutableArray<string> GetEqualsArgumentList(SemanticModel semanticModel, AnonymousMethodExpressionSyntax anonymousMethod)
+        {
+            var equalsValueClauseSyntax = (EqualsValueClauseSyntax)anonymousMethod.Parent;
+            var variableDeclaration = (VariableDeclarationSyntax)equalsValueClauseSyntax.Parent.Parent;
+
+            var symbol = semanticModel.GetSymbolInfo(variableDeclaration.Type);
+            var namedTypeSymbol = (INamedTypeSymbol)symbol.Symbol;
+            return namedTypeSymbol.DelegateInvokeMethod.Parameters.Select(ps => ps.Name).ToImmutableArray();
+        }
+
+        private static ImmutableArray<string> GetAssignmentArgumentList(SemanticModel semanticModel, AnonymousMethodExpressionSyntax anonymousMethod)
+        {
+            var assignmentExpressionSyntax = (AssignmentExpressionSyntax)anonymousMethod.Parent;
+
+            var symbol = semanticModel.GetSymbolInfo(assignmentExpressionSyntax.Left);
+            var eventSymbol = (IEventSymbol)symbol.Symbol;
+            var namedTypeSymbol = (INamedTypeSymbol)eventSymbol.Type;
+            return namedTypeSymbol.DelegateInvokeMethod.Parameters.Select(ps => ps.Name).ToImmutableArray();
+        }
+
+        private static List<ParameterSyntax> GenerateUniqueParameterNames(SemanticModel semanticModel, AnonymousMethodExpressionSyntax anonymousMethod, ImmutableArray<string> argumentNames)
+        {
+            var parameters = new List<ParameterSyntax>();
+
+            foreach (var argumentName in argumentNames)
+            {
+                var baseName = argumentName;
+                var newName = baseName;
+                var index = 0;
+
+                while (semanticModel.LookupSymbols(anonymousMethod.SpanStart, name: newName).Length > 0)
+                {
+                    index++;
+                    newName = baseName + index;
+                }
+
+                parameters.Add(SyntaxFactory.Parameter(SyntaxFactory.Identifier(newName)).WithType(null));
+            }
+
+            return parameters;
+        }
+
         private static ParameterListSyntax RemoveType(ParameterListSyntax parameterList)
         {
             return parameterList.WithParameters(SyntaxFactory.SeparatedList(parameterList.Parameters.Select(x => RemoveType(x)), parameterList.Parameters.GetSeparators()));
         }
 
         private static ParameterSyntax RemoveType(ParameterSyntax parameterSyntax)
         {
-            var syntax = parameterSyntax.WithType(null)
-                .WithLeadingTrivia(parameterSyntax.Type.GetLeadingTrivia().Concat(parameterSyntax.Type.GetTrailingTrivia()));
+            var syntax = parameterSyntax.WithType(null);
+
+            if (parameterSyntax.Type != null)
+            {
+                syntax = syntax.WithLeadingTrivia(parameterSyntax.Type.GetLeadingTrivia().Concat(parameterSyntax.Type.GetTrailingTrivia()));
+            }
+
             return syntax.WithTrailingTrivia(syntax.GetTrailingTrivia().WithoutTrailingWhitespace())
                 .WithLeadingTrivia(syntax.GetLeadingTrivia().WithoutWhitespace());
         }
@@ -131,10 +217,11 @@ private static bool IsValid(ParameterSyntax parameterSyntax)
         private static async Task<Document> GetTransformedDocumentAsync(Document document, Diagnostic diagnostic, CancellationToken cancellationToken)
         {
             var syntaxRoot = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
+            var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
 
             var anonymousMethod = (AnonymousMethodExpressionSyntax)syntaxRoot.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true);
 
-            var newSyntaxRoot = syntaxRoot.ReplaceNode(anonymousMethod, ReplaceWithLambda(anonymousMethod));
+            var newSyntaxRoot = syntaxRoot.ReplaceNode(anonymousMethod, ReplaceWithLambda(semanticModel, anonymousMethod));
             var newDocument = document.WithSyntaxRoot(newSyntaxRoot.WithoutFormatting());
 
             return newDocument;
@@ -150,17 +237,17 @@ private class FixAll : DocumentBasedFixAllProvider
             protected override async Task<SyntaxNode> FixAllInDocumentAsync(FixAllContext fixAllContext, Document document, ImmutableArray<Diagnostic> diagnostics)
             {
                 var syntaxRoot = await document.GetSyntaxRootAsync(fixAllContext.CancellationToken).ConfigureAwait(false);
+                var semanticModel = await document.GetSemanticModelAsync(fixAllContext.CancellationToken).ConfigureAwait(false);
 
                 var nodes = new List<AnonymousMethodExpressionSyntax>();
 
                 foreach (var diagnostic in diagnostics)
                 {
                     var node = (AnonymousMethodExpressionSyntax)syntaxRoot.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true);
-
                     nodes.Add(node);
                 }
 
-                return syntaxRoot.ReplaceNodes(nodes, (originalNode, rewrittenNode) => ReplaceWithLambda(rewrittenNode));
+                return syntaxRoot.ReplaceNodes(nodes, (originalNode, rewrittenNode) => ReplaceWithLambda(semanticModel, rewrittenNode));
             }
         }
     }
diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1130UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1130UnitTests.cs
@@ -294,6 +294,96 @@ public void Test()
             await this.VerifyCSharpFixAsync(testCode, fixedCode).ConfigureAwait(false);
         }
 
+        /// <summary>
+        /// Verify that expansion of a delegate without parameters will generate a lambda with the necessary parameters.
+        /// </summary>
+        /// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
+        [Fact]
+        [WorkItem(2593, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/2593")]
+        public async Task VerifyThatDelegateExpansionWillNotGenerateInvalidCodeAsync()
+        {
+            var testCode = @"
+using System;
+
+namespace StyleCopDemo
+{
+    class Program
+    {
+        public delegate void TestDelegate(int arg1, double arg2);
+        public static event EventHandler TestEvent;
+
+        static void Main(string[] args)
+        {
+            EventHandler anon = delegate { Console.WriteLine(""hello""); };
+            TestEvent += delegate { Console.WriteLine(""hello""); };
+            TestEvent -= delegate { Console.WriteLine(""hello""); };
+        }
+
+        public static void TestMethod()
+        {
+            object sender = null;
+            EventArgs e = EventArgs.Empty;
+
+            EventHandler anon = delegate { Console.WriteLine(""hello""); };
+
+            TestMethod2(delegate { Console.WriteLine(""hello""); });
+        }
+
+        public static void TestMethod2(TestDelegate testDelegate)
+        {
+        }
+    }
+}
+";
+
+            var fixedCode = @"
+using System;
+
+namespace StyleCopDemo
+{
+    class Program
+    {
+        public delegate void TestDelegate(int arg1, double arg2);
+        public static event EventHandler TestEvent;
+
+        static void Main(string[] args)
+        {
+            EventHandler anon = (sender, e) => { Console.WriteLine(""hello""); };
+            TestEvent += (sender, e) => { Console.WriteLine(""hello""); };
+            TestEvent -= (sender, e) => { Console.WriteLine(""hello""); };
+        }
+
+        public static void TestMethod()
+        {
+            object sender = null;
+            EventArgs e = EventArgs.Empty;
+
+            EventHandler anon = (sender1, e1) => { Console.WriteLine(""hello""); };
+
+            TestMethod2((arg1, arg2) => { Console.WriteLine(""hello""); });
+        }
+
+        public static void TestMethod2(TestDelegate testDelegate)
+        {
+        }
+    }
+}
+";
+
+            DiagnosticResult[] expected =
+            {
+                this.CSharpDiagnostic().WithLocation(13, 33),
+                this.CSharpDiagnostic().WithLocation(14, 26),
+                this.CSharpDiagnostic().WithLocation(15, 26),
+                this.CSharpDiagnostic().WithLocation(23, 33),
+                this.CSharpDiagnostic().WithLocation(25, 25),
+            };
+
+            await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
+            await this.VerifyCSharpDiagnosticAsync(fixedCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
+            await this.VerifyCSharpFixAsync(testCode, fixedCode).ConfigureAwait(false);
+        }
+
         /// <inheritdoc/>
         protected override IEnumerable<DiagnosticAnalyzer> GetCSharpDiagnosticAnalyzers()
         {
diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1130UseLambdaSyntax.cs b/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1130UseLambdaSyntax.cs
