Skip to content

Correct code fix for SA1130 when delegate expression is part of a cast expression #3516

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 21, 2023
Merged

Correct code fix for SA1130 when delegate expression is part of a cast expression #3516

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 44 additions & 9 deletions StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/ReadabilityRules/SA1130CodeFixProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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>();
Expand Down Expand Up @@ -306,7 +341,7 @@ protected override async Task<SyntaxNode> FixAllInDocumentAsync(FixAllContext fi
return rewrittenNode;
}

return newNode;
return newNode.WithoutFormatting();
});
}
}
Expand Down
47 changes: 47 additions & 0 deletions StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1130UnitTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
2 changes: 1 addition & 1 deletion StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1130UseLambdaSyntax.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down