Skip to content

Commit 9f833fb

Browse files
vweijstersvweijsters
committed
Made SA1130 codefix properly expand parameters when only delegate keyword is used
1 parent 9296fa2 commit 9f833fb

3 files changed

Lines changed: 266 additions & 28 deletions

File tree

‎StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/ReadabilityRules/SA1130CodeFixProvider.cs‎

Lines changed: 52 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
namespace StyleCop.Analyzers.ReadabilityRules
55
{
6+
using System;
67
using System.Collections.Generic;
78
using System.Collections.Immutable;
89
using System.Composition;
@@ -24,6 +25,8 @@ namespace StyleCop.Analyzers.ReadabilityRules
2425
[Shared]
2526
internal class SA1130CodeFixProvider : CodeFixProvider
2627
{
28+
private static readonly SyntaxToken ParameterListSeparator = SyntaxFactory.Token(SyntaxKind.CommaToken).WithTrailingTrivia(SyntaxFactory.Space);
29+
2730
/// <inheritdoc/>
2831
public override ImmutableArray<string> FixableDiagnosticIds { get; } =
2932
ImmutableArray.Create(SA1130UseLambdaSyntax.DiagnosticId);
@@ -50,14 +53,24 @@ public override Task RegisterCodeFixesAsync(CodeFixContext context)
5053
return SpecializedTasks.CompletedTask;
5154
}
5255

53-
private static SyntaxNode ReplaceWithLambda(AnonymousMethodExpressionSyntax anonymousMethod)
56+
private static SyntaxNode ReplaceWithLambda(SemanticModel semanticModel, AnonymousMethodExpressionSyntax anonymousMethod, string argumentsProperty)
5457
{
5558
var parameterList = anonymousMethod.ParameterList;
5659
SyntaxNode lambdaExpression;
5760

5861
if (parameterList == null)
5962
{
60-
parameterList = SyntaxFactory.ParameterList()
63+
SeparatedSyntaxList<ParameterSyntax> newList = default(SeparatedSyntaxList<ParameterSyntax>);
64+
65+
if (!string.IsNullOrEmpty(argumentsProperty))
66+
{
67+
var argumentNames = argumentsProperty.Split(',');
68+
List<ParameterSyntax> parameters = GenerateUniqueParameterNames(semanticModel, anonymousMethod, argumentNames);
69+
70+
newList = SyntaxFactory.SeparatedList(parameters, Enumerable.Repeat(ParameterListSeparator, parameters.Count - 1));
71+
}
72+
73+
parameterList = SyntaxFactory.ParameterList(newList)
6174
.WithLeadingTrivia(anonymousMethod.DelegateKeyword.LeadingTrivia)
6275
.WithTrailingTrivia(anonymousMethod.DelegateKeyword.TrailingTrivia);
6376
}
@@ -106,15 +119,42 @@ private static SyntaxNode ReplaceWithLambda(AnonymousMethodExpressionSyntax anon
106119
.WithAdditionalAnnotations(Formatter.Annotation);
107120
}
108121

122+
private static List<ParameterSyntax> GenerateUniqueParameterNames(SemanticModel semanticModel, AnonymousMethodExpressionSyntax anonymousMethod, string[] argumentNames)
123+
{
124+
var parameters = new List<ParameterSyntax>();
125+
126+
foreach (var argumentName in argumentNames)
127+
{
128+
var baseName = argumentName;
129+
var newName = baseName;
130+
var index = 0;
131+
132+
while (semanticModel.LookupSymbols(anonymousMethod.SpanStart, name: newName).Length > 0)
133+
{
134+
index++;
135+
newName = baseName + index;
136+
}
137+
138+
parameters.Add(SyntaxFactory.Parameter(SyntaxFactory.Identifier(newName)).WithType(null));
139+
}
140+
141+
return parameters;
142+
}
143+
109144
private static ParameterListSyntax RemoveType(ParameterListSyntax parameterList)
110145
{
111146
return parameterList.WithParameters(SyntaxFactory.SeparatedList(parameterList.Parameters.Select(x => RemoveType(x)), parameterList.Parameters.GetSeparators()));
112147
}
113148

114149
private static ParameterSyntax RemoveType(ParameterSyntax parameterSyntax)
115150
{
116-
var syntax = parameterSyntax.WithType(null)
117-
.WithLeadingTrivia(parameterSyntax.Type.GetLeadingTrivia().Concat(parameterSyntax.Type.GetTrailingTrivia()));
151+
var syntax = parameterSyntax.WithType(null);
152+
153+
if (parameterSyntax.Type != null)
154+
{
155+
syntax = syntax.WithLeadingTrivia(parameterSyntax.Type.GetLeadingTrivia().Concat(parameterSyntax.Type.GetTrailingTrivia()));
156+
}
157+
118158
return syntax.WithTrailingTrivia(syntax.GetTrailingTrivia().WithoutTrailingWhitespace())
119159
.WithLeadingTrivia(syntax.GetLeadingTrivia().WithoutWhitespace());
120160
}
@@ -131,10 +171,12 @@ private static bool IsValid(ParameterSyntax parameterSyntax)
131171
private static async Task<Document> GetTransformedDocumentAsync(Document document, Diagnostic diagnostic, CancellationToken cancellationToken)
132172
{
133173
var syntaxRoot = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
174+
var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
134175

135176
var anonymousMethod = (AnonymousMethodExpressionSyntax)syntaxRoot.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true);
136177

137-
var newSyntaxRoot = syntaxRoot.ReplaceNode(anonymousMethod, ReplaceWithLambda(anonymousMethod));
178+
var argumentsProperty = diagnostic.Properties.ContainsKey(SA1130UseLambdaSyntax.DelegateArgumentNamesProperty) ? diagnostic.Properties[SA1130UseLambdaSyntax.DelegateArgumentNamesProperty] : string.Empty;
179+
var newSyntaxRoot = syntaxRoot.ReplaceNode(anonymousMethod, ReplaceWithLambda(semanticModel, anonymousMethod, argumentsProperty));
138180
var newDocument = document.WithSyntaxRoot(newSyntaxRoot.WithoutFormatting());
139181

140182
return newDocument;
@@ -150,17 +192,21 @@ private class FixAll : DocumentBasedFixAllProvider
150192
protected override async Task<SyntaxNode> FixAllInDocumentAsync(FixAllContext fixAllContext, Document document, ImmutableArray<Diagnostic> diagnostics)
151193
{
152194
var syntaxRoot = await document.GetSyntaxRootAsync(fixAllContext.CancellationToken).ConfigureAwait(false);
195+
var semanticModel = await document.GetSemanticModelAsync(fixAllContext.CancellationToken).ConfigureAwait(false);
153196

154197
var nodes = new List<AnonymousMethodExpressionSyntax>();
198+
var nodeProperties = new Dictionary<AnonymousMethodExpressionSyntax, string>();
155199

156200
foreach (var diagnostic in diagnostics)
157201
{
202+
var argumentsProperty = diagnostic.Properties.ContainsKey(SA1130UseLambdaSyntax.DelegateArgumentNamesProperty) ? diagnostic.Properties[SA1130UseLambdaSyntax.DelegateArgumentNamesProperty] : string.Empty;
158203
var node = (AnonymousMethodExpressionSyntax)syntaxRoot.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true);
159204

160205
nodes.Add(node);
206+
nodeProperties.Add(node, argumentsProperty);
161207
}
162208

163-
return syntaxRoot.ReplaceNodes(nodes, (originalNode, rewrittenNode) => ReplaceWithLambda(rewrittenNode));
209+
return syntaxRoot.ReplaceNodes(nodes, (originalNode, rewrittenNode) => ReplaceWithLambda(semanticModel, rewrittenNode, nodeProperties[rewrittenNode]));
164210
}
165211
}
166212
}

‎StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1130UnitTests.cs‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,96 @@ public void Test()
294294
await this.VerifyCSharpFixAsync(testCode, fixedCode).ConfigureAwait(false);
295295
}
296296

297+
/// <summary>
298+
/// Verify that expansion of a delegate without parameters will generate a lambda with the necessary parameters.
299+
/// </summary>
300+
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
301+
[Fact]
302+
[WorkItem(2593, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/2593")]
303+
public async Task VerifyThatDelegateExpansionWillNotGenerateInvalidCodeAsync()
304+
{
305+
var testCode = @"
306+
using System;
307+
308+
namespace StyleCopDemo
309+
{
310+
class Program
311+
{
312+
public delegate void TestDelegate(int arg1, double arg2);
313+
public static event EventHandler TestEvent;
314+
315+
static void Main(string[] args)
316+
{
317+
EventHandler anon = delegate { Console.WriteLine(""hello""); };
318+
TestEvent += delegate { Console.WriteLine(""hello""); };
319+
TestEvent -= delegate { Console.WriteLine(""hello""); };
320+
}
321+
322+
public static void TestMethod()
323+
{
324+
object sender = null;
325+
EventArgs e = EventArgs.Empty;
326+
327+
EventHandler anon = delegate { Console.WriteLine(""hello""); };
328+
329+
TestMethod2(delegate { Console.WriteLine(""hello""); });
330+
}
331+
332+
public static void TestMethod2(TestDelegate testDelegate)
333+
{
334+
}
335+
}
336+
}
337+
";
338+
339+
var fixedCode = @"
340+
using System;
341+
342+
namespace StyleCopDemo
343+
{
344+
class Program
345+
{
346+
public delegate void TestDelegate(int arg1, double arg2);
347+
public static event EventHandler TestEvent;
348+
349+
static void Main(string[] args)
350+
{
351+
EventHandler anon = (sender, e) => { Console.WriteLine(""hello""); };
352+
TestEvent += (sender, e) => { Console.WriteLine(""hello""); };
353+
TestEvent -= (sender, e) => { Console.WriteLine(""hello""); };
354+
}
355+
356+
public static void TestMethod()
357+
{
358+
object sender = null;
359+
EventArgs e = EventArgs.Empty;
360+
361+
EventHandler anon = (sender1, e1) => { Console.WriteLine(""hello""); };
362+
363+
TestMethod2((arg1, arg2) => { Console.WriteLine(""hello""); });
364+
}
365+
366+
public static void TestMethod2(TestDelegate testDelegate)
367+
{
368+
}
369+
}
370+
}
371+
";
372+
373+
DiagnosticResult[] expected =
374+
{
375+
this.CSharpDiagnostic().WithLocation(13, 33),
376+
this.CSharpDiagnostic().WithLocation(14, 26),
377+
this.CSharpDiagnostic().WithLocation(15, 26),
378+
this.CSharpDiagnostic().WithLocation(23, 33),
379+
this.CSharpDiagnostic().WithLocation(25, 25),
380+
};
381+
382+
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
383+
await this.VerifyCSharpDiagnosticAsync(fixedCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
384+
await this.VerifyCSharpFixAsync(testCode, fixedCode).ConfigureAwait(false);
385+
}
386+
297387
/// <inheritdoc/>
298388
protected override IEnumerable<DiagnosticAnalyzer> GetCSharpDiagnosticAnalyzers()
299389
{

‎StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1130UseLambdaSyntax.cs‎

Lines changed: 124 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ namespace StyleCop.Analyzers.ReadabilityRules
55
{
66
using System;
77
using System.Collections.Immutable;
8+
using System.Linq;
89
using Microsoft.CodeAnalysis;
910
using Microsoft.CodeAnalysis.CSharp;
1011
using Microsoft.CodeAnalysis.CSharp.Syntax;
@@ -21,6 +22,12 @@ internal class SA1130UseLambdaSyntax : DiagnosticAnalyzer
2122
/// The ID for diagnostics produced by the <see cref="SA1130UseLambdaSyntax"/> analyzer.
2223
/// </summary>
2324
public const string DiagnosticId = "SA1130";
25+
26+
/// <summary>
27+
/// Property identifier used to pass information to the codefix.
28+
/// </summary>
29+
internal const string DelegateArgumentNamesProperty = "DelegateArgumentNames";
30+
2431
private static readonly LocalizableString Title = new LocalizableResourceString(nameof(ReadabilityResources.SA1130Title), ReadabilityResources.ResourceManager, typeof(ReadabilityResources));
2532
private static readonly LocalizableString MessageFormat = new LocalizableResourceString(nameof(ReadabilityResources.SA1130MessageFormat), ReadabilityResources.ResourceManager, typeof(ReadabilityResources));
2633
private static readonly LocalizableString Description = new LocalizableResourceString(nameof(ReadabilityResources.SA1130Description), ReadabilityResources.ResourceManager, typeof(ReadabilityResources));
@@ -47,35 +54,130 @@ public override void Initialize(AnalysisContext context)
4754

4855
private static void HandleAnonymousMethodExpression(SyntaxNodeAnalysisContext context)
4956
{
57+
var diagnosticProperties = ImmutableDictionary.CreateBuilder<string, string>();
58+
59+
bool reportDiagnostic = true;
5060
var anonymousMethod = (AnonymousMethodExpressionSyntax)context.Node;
5161

52-
if (anonymousMethod.Parent.IsKind(SyntaxKind.Argument))
62+
switch (anonymousMethod.Parent.Kind())
5363
{
54-
// invocation -> argument list -> argument -> anonymous method
55-
if (anonymousMethod?.Parent?.Parent?.Parent is InvocationExpressionSyntax originalInvocationExpression)
64+
case SyntaxKind.Argument:
65+
reportDiagnostic = HandleMethodInvocation(context.SemanticModel, anonymousMethod, (ArgumentSyntax)anonymousMethod.Parent, diagnosticProperties);
66+
break;
67+
68+
case SyntaxKind.EqualsValueClause:
69+
reportDiagnostic = HandleAssignment(context.SemanticModel, (EqualsValueClauseSyntax)anonymousMethod.Parent, diagnosticProperties);
70+
break;
71+
72+
case SyntaxKind.AddAssignmentExpression:
73+
case SyntaxKind.SubtractAssignmentExpression:
74+
reportDiagnostic = HandleAssignmentExpression(context.SemanticModel, anonymousMethod, (AssignmentExpressionSyntax)anonymousMethod.Parent, diagnosticProperties);
75+
break;
76+
}
77+
78+
if (reportDiagnostic)
79+
{
80+
context.ReportDiagnostic(Diagnostic.Create(Descriptor, anonymousMethod.DelegateKeyword.GetLocation(), diagnosticProperties.ToImmutable()));
81+
}
82+
}
83+
84+
private static bool HandleMethodInvocation(SemanticModel semanticModel, AnonymousMethodExpressionSyntax anonymousMethod, ArgumentSyntax argumentSyntax, ImmutableDictionary<string, string>.Builder propertiesBuilder)
85+
{
86+
// invocation -> argument list -> argument -> anonymous method
87+
var argumentListSyntax = argumentSyntax?.Parent as ArgumentListSyntax;
88+
89+
var originalInvocationExpression = argumentListSyntax?.Parent as InvocationExpressionSyntax;
90+
if (originalInvocationExpression != null)
91+
{
92+
SymbolInfo originalSymbolInfo = semanticModel.GetSymbolInfo(originalInvocationExpression);
93+
Location location = originalInvocationExpression.GetLocation();
94+
95+
var argumentIndex = argumentListSyntax.Arguments.IndexOf(argumentSyntax);
96+
var parameterList = GetDelegateParameterList(originalSymbolInfo, argumentIndex);
97+
98+
// In some cases passing a delegate as an argument to a method is required to call the right overload
99+
// When there is an other overload that takes an expression.
100+
var lambdaExpression = SyntaxFactory.ParenthesizedLambdaExpression(
101+
anonymousMethod.AsyncKeyword,
102+
parameterList,
103+
SyntaxFactory.Token(SyntaxKind.EqualsGreaterThanToken),
104+
anonymousMethod.Body);
105+
106+
var invocationExpression = originalInvocationExpression.ReplaceNode(anonymousMethod, lambdaExpression);
107+
SymbolInfo newSymbolInfo = semanticModel.GetSpeculativeSymbolInfo(location.SourceSpan.Start, invocationExpression, SpeculativeBindingOption.BindAsExpression);
108+
109+
if (originalSymbolInfo.Symbol != newSymbolInfo.Symbol)
56110
{
57-
// In some cases passing a delegate as an argument to a method is required to call the right overload
58-
// When there is an other overload that takes an expression.
59-
var lambdaExpression = SyntaxFactory.ParenthesizedLambdaExpression(
60-
anonymousMethod.AsyncKeyword,
61-
anonymousMethod.ParameterList ?? EmptyParameterList,
62-
SyntaxFactory.Token(SyntaxKind.EqualsGreaterThanToken),
63-
anonymousMethod.Body);
64-
65-
var invocationExpression = originalInvocationExpression.ReplaceNode(anonymousMethod, lambdaExpression);
66-
67-
SymbolInfo originalSymbolInfo = context.SemanticModel.GetSymbolInfo(originalInvocationExpression);
68-
Location location = originalInvocationExpression.GetLocation();
69-
SymbolInfo newSymbolInfo = context.SemanticModel.GetSpeculativeSymbolInfo(location.SourceSpan.Start, invocationExpression, SpeculativeBindingOption.BindAsExpression);
70-
71-
if (originalSymbolInfo.Symbol != newSymbolInfo.Symbol)
72-
{
73-
return;
74-
}
111+
return false;
75112
}
113+
114+
var parameterNames = parameterList.Parameters.Select(p => p.Identifier.ToString());
115+
propertiesBuilder.Add(DelegateArgumentNamesProperty, string.Join(",", parameterNames));
116+
}
117+
118+
return true;
119+
}
120+
121+
private static bool HandleAssignment(SemanticModel semanticModel, EqualsValueClauseSyntax equalsValueClauseSyntax, ImmutableDictionary<string, string>.Builder propertiesBuilder)
122+
{
123+
var variableDeclaration = (VariableDeclarationSyntax)equalsValueClauseSyntax.Parent.Parent;
124+
var symbol = semanticModel.GetSymbolInfo(variableDeclaration.Type);
125+
126+
var namedTypeSymbol = symbol.Symbol as INamedTypeSymbol;
127+
if (namedTypeSymbol?.TypeKind == TypeKind.Delegate)
128+
{
129+
var delegateParameters = namedTypeSymbol.DelegateInvokeMethod.Parameters;
130+
propertiesBuilder.Add(DelegateArgumentNamesProperty, string.Join(",", delegateParameters.Select(ps => ps.Name)));
131+
return true;
76132
}
77133

78-
context.ReportDiagnostic(Diagnostic.Create(Descriptor, anonymousMethod.DelegateKeyword.GetLocation()));
134+
return false;
135+
}
136+
137+
private static bool HandleAssignmentExpression(SemanticModel semanticModel, AnonymousMethodExpressionSyntax anonymousMethod, AssignmentExpressionSyntax assignmentExpressionSyntax, ImmutableDictionary<string, string>.Builder propertiesBuilder)
138+
{
139+
var symbol = semanticModel.GetSymbolInfo(assignmentExpressionSyntax.Left);
140+
141+
var eventSymbol = symbol.Symbol as IEventSymbol;
142+
if (eventSymbol?.Type.TypeKind == TypeKind.Delegate)
143+
{
144+
var delegateParameters = ((INamedTypeSymbol)eventSymbol.Type).DelegateInvokeMethod.Parameters;
145+
propertiesBuilder.Add(DelegateArgumentNamesProperty, string.Join(",", delegateParameters.Select(ps => ps.Name)));
146+
return true;
147+
}
148+
149+
return false;
150+
}
151+
152+
private static ParameterListSyntax GetDelegateParameterList(SymbolInfo originalSymbolInfo, int argumentIndex)
153+
{
154+
// Determine the parameter list from the method that is invoked, as delegates without parameters are allowed, but they cannot be replaced by a lambda without parameters.
155+
var methodSymbol = (IMethodSymbol)originalSymbolInfo.Symbol;
156+
var delegateType = (INamedTypeSymbol)methodSymbol.Parameters[argumentIndex].Type;
157+
var delegateParameters = delegateType.DelegateInvokeMethod.Parameters;
158+
159+
var syntaxParameters = GetSyntaxParametersFromSymbolParameters(delegateParameters);
160+
161+
return SyntaxFactory.ParameterList(SyntaxFactory.SeparatedList(syntaxParameters));
162+
}
163+
164+
private static ImmutableArray<ParameterSyntax> GetSyntaxParametersFromSymbolParameters(ImmutableArray<IParameterSymbol> symbolParameters)
165+
{
166+
var result = ImmutableArray.CreateBuilder<ParameterSyntax>(symbolParameters.Length);
167+
168+
foreach (var symbolParameter in symbolParameters)
169+
{
170+
var syntaxParameter = GetParameterSyntaxFromParameterSymbol(symbolParameter);
171+
result.Add(syntaxParameter);
172+
}
173+
174+
return result.ToImmutable();
175+
}
176+
177+
private static ParameterSyntax GetParameterSyntaxFromParameterSymbol(IParameterSymbol symbolParameter)
178+
{
179+
return SyntaxFactory.Parameter(SyntaxFactory.Identifier(symbolParameter.Name))
180+
.WithType(SyntaxFactory.ParseTypeName(symbolParameter.Type.Name));
79181
}
80182
}
81183
}

0 commit comments

Comments
 (0)