Correct code fix for SA1130 to add parenthesis around lambda when the…

… original delegate expression is part of a cast expression #3510
DotNetAnalyzers · Apr 15, 2023 · 6a1c3ef · 6a1c3ef
1 parent ccaac20
commit 6a1c3ef
Show file tree

Hide file tree

Showing 3 changed files with 92 additions and 10 deletions.
diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/ReadabilityRules/SA1130CodeFixProvider.cs b/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/ReadabilityRules/SA1130CodeFixProvider.cs
@@ -72,7 +72,7 @@ private static async Task<bool> CanFixAsync(CodeFixContext context, Diagnostic d
         private static SyntaxNode ReplaceWithLambda(SemanticModel semanticModel, AnonymousMethodExpressionSyntax anonymousMethod)
         {
             var parameterList = anonymousMethod.ParameterList;
-            SyntaxNode lambdaExpression;
+            ExpressionSyntax lambdaExpression;
             SyntaxToken arrowToken;
 
             if (parameterList == null)
@@ -91,16 +91,17 @@ private static SyntaxNode ReplaceWithLambda(SemanticModel semanticModel, Anonymo
 
                 case SyntaxKind.AddAssignmentExpression:
                 case SyntaxKind.SubtractAssignmentExpression:
-                    var list = GetAssignmentArgumentList(semanticModel, anonymousMethod);
-
-                    if (list == null)
                     {
-                        return null;
+                        var list = GetAssignmentArgumentList(semanticModel, anonymousMethod);
+                        if (list == null)
+                        {
+                            return null;
+                        }
+
+                        argumentList = list.Value;
+                        break;
                     }
 
-                    argumentList = list.Value;
-                    break;
-
                 case SyntaxKind.ArrowExpressionClause:
                 case SyntaxKind.ReturnStatement:
                     argumentList = GetMemberReturnTypeArgumentList(semanticModel, anonymousMethod);
@@ -110,6 +111,18 @@ private static SyntaxNode ReplaceWithLambda(SemanticModel semanticModel, Anonymo
                     }
 
                     break;
+
+                case SyntaxKind.CastExpression:
+                    {
+                        var list = GetCastTypeArgumentList(semanticModel, anonymousMethod);
+                        if (list == null)
+                        {
+                            return null;
+                        }
+
+                        argumentList = list.Value;
+                        break;
+                    }
                 }
 
                 List<ParameterSyntax> parameters = GenerateUniqueParameterNames(semanticModel, anonymousMethod, argumentList);
@@ -165,6 +178,13 @@ private static SyntaxNode ReplaceWithLambda(SemanticModel semanticModel, Anonymo
                 lambdaExpression = SyntaxFactory.ParenthesizedLambdaExpression(anonymousMethod.AsyncKeyword, parameterListSyntax, arrowToken, anonymousMethod.Body);
             }
 
+            if (anonymousMethod.Parent.IsKind(SyntaxKind.CastExpression))
+            {
+                // In this case, the lambda needs enclosing parenthesis to be syntactically correct
+                lambdaExpression = SyntaxFactory.ParenthesizedExpression(lambdaExpression);
+            }
+
+            // TODO: No tests require this annotation. Can it be removed?
             return lambdaExpression
                 .WithAdditionalAnnotations(Formatter.Annotation);
         }
@@ -213,6 +233,21 @@ private static ImmutableArray<string> GetMemberReturnTypeArgumentList(SemanticMo
             return !(((IMethodSymbol)enclosingSymbol).ReturnType is INamedTypeSymbol returnType) ? ImmutableArray<string>.Empty : returnType.DelegateInvokeMethod.Parameters.Select(ps => ps.Name).ToImmutableArray();
         }
 
+        private static ImmutableArray<string>? GetCastTypeArgumentList(SemanticModel semanticModel, AnonymousMethodExpressionSyntax anonymousMethod)
+        {
+            var castExpression = (CastExpressionSyntax)anonymousMethod.Parent;
+
+            var symbol = semanticModel.GetSymbolInfo(castExpression.Type);
+            var namedTypeSymbol = symbol.Symbol as INamedTypeSymbol;
+            var parameters = namedTypeSymbol?.DelegateInvokeMethod?.Parameters;
+            if (parameters == null)
+            {
+                return null;
+            }
+
+            return parameters.Value.Select(ps => ps.Name).ToImmutableArray();
+        }
+
         private static List<ParameterSyntax> GenerateUniqueParameterNames(SemanticModel semanticModel, AnonymousMethodExpressionSyntax anonymousMethod, ImmutableArray<string> argumentNames)
         {
             var parameters = new List<ParameterSyntax>();
@@ -306,7 +341,7 @@ protected override async Task<SyntaxNode> FixAllInDocumentAsync(FixAllContext fi
                         return rewrittenNode;
                     }
 
-                    return newNode;
+                    return newNode.WithoutFormatting();
                 });
             }
         }

diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1130UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1130UnitTests.cs
@@ -963,5 +963,52 @@ private void Test2(string description = null, Func<object, string> resolve = nul
 
             await VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
         }
+
+        [Theory]
+        [InlineData(
+            "(Func<int>)[|delegate|] { return 1; }",
+            "(Func<int>)(() => { return 1; })")]
+        [InlineData(
+            "(Func<int>)[|delegate|]() { return 1; }",
+            "(Func<int>)(() => { return 1; })")]
+        [InlineData(
+            "(Func<int, int>)[|delegate|] { return 1; }",
+            "(Func<int, int>)(arg => { return 1; })")]
+        [InlineData(
+            "(Func<int, int>)[|delegate|](int x) { return 1; }",
+            "(Func<int, int>)(x => { return 1; })")]
+        [InlineData(
+            "(Func<int, int, int>)[|delegate|] { return 1; }",
+            "(Func<int, int, int>)((arg1, arg2) => { return 1; })")]
+        [InlineData(
+            "(Func<int, int, int>)[|delegate|](int x, int y) { return 1; }",
+            "(Func<int, int, int>)((x, y) => { return 1; })")]
+        [WorkItem(3510, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3510")]
+        public async Task TestDelegateUsedInCastAsync(string testExpression, string fixedExpression)
+        {
+            var testCode = $@"
+using System;
+
+public class TypeName
+{{
+    public void Test()
+    {{
+        var z = {testExpression};
+    }}
+}}";
+
+            var fixedCode = $@"
+using System;
+
+public class TypeName
+{{
+    public void Test()
+    {{
+        var z = {fixedExpression};
+    }}
+}}";
+
+            await VerifyCSharpFixAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, fixedCode, CancellationToken.None).ConfigureAwait(false);
+        }
     }
 }
diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1130UseLambdaSyntax.cs b/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1130UseLambdaSyntax.cs
@@ -168,7 +168,7 @@ private static bool HandleMethodInvocation(SemanticModel semanticModel, Anonymou
 
                 if (parameterList == null)
                 {
-                    // This might happen if the call was using params witha type unknown to the analyzer, e.g. params Span<T>.
+                    // This might happen if the call was using params with a type unknown to the analyzer, e.g. params Span<T>.
                     return false;
                 }