Skip to content

Commit 16aeec9

Browse files
sharwellsharwell
authored
Merge pull request #2809 from pdelvo/sa1130constructor
Make sure the SA1130 exception also works on other invocable expressions
2 parents bc6bb96 + 62ea3e6 commit 16aeec9

File tree

3 files changed

+271
-17
lines changed

3 files changed

+271
-17
lines changed

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -136,12 +136,12 @@ private static SyntaxNode ReplaceWithLambda(SemanticModel semanticModel, Anonymo
136136
private static ImmutableArray<string> GetMethodInvocationArgumentList(SemanticModel semanticModel, AnonymousMethodExpressionSyntax anonymousMethod)
137137
{
138138
var argumentSyntax = (ArgumentSyntax)anonymousMethod.Parent;
139-
var argumentListSyntax = (ArgumentListSyntax)argumentSyntax.Parent;
140-
var originalInvocationExpression = (InvocationExpressionSyntax)argumentListSyntax.Parent;
139+
var argumentListSyntax = (BaseArgumentListSyntax)argumentSyntax.Parent;
140+
var originalInvocableExpression = argumentListSyntax.Parent;
141141

142-
var originalSymbolInfo = semanticModel.GetSymbolInfo(originalInvocationExpression);
142+
var originalSymbolInfo = semanticModel.GetSymbolInfo(originalInvocableExpression);
143143
var argumentIndex = argumentListSyntax.Arguments.IndexOf(argumentSyntax);
144-
var parameterList = SA1130UseLambdaSyntax.GetDelegateParameterList((IMethodSymbol)originalSymbolInfo.Symbol, argumentIndex);
144+
var parameterList = SA1130UseLambdaSyntax.GetDelegateParameterList(originalSymbolInfo.Symbol, argumentIndex);
145145
return parameterList.Parameters.Select(p => p.Identifier.ToString()).ToImmutableArray();
146146
}
147147

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

Lines changed: 237 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -446,5 +446,242 @@ public void Test()
446446

447447
await VerifyCSharpFixAsync(testCode, expected, fixedCode, CancellationToken.None).ConfigureAwait(false);
448448
}
449+
450+
[Fact]
451+
public async Task TestDelegateUseAsConstructorArgumentsAsync()
452+
{
453+
var testCode = @"
454+
using System;
455+
public class TypeName
456+
{
457+
public TypeName(Action argument)
458+
{
459+
460+
}
461+
462+
public void Test()
463+
{
464+
new TypeName(delegate { });
465+
}
466+
}";
467+
string fixedCode = @"
468+
using System;
469+
public class TypeName
470+
{
471+
public TypeName(Action argument)
472+
{
473+
474+
}
475+
476+
public void Test()
477+
{
478+
new TypeName(() => { });
479+
}
480+
}";
481+
var expected = Diagnostic().WithLocation(12, 22);
482+
await VerifyCSharpFixAsync(testCode, expected, fixedCode, CancellationToken.None).ConfigureAwait(false);
483+
}
484+
485+
[Fact]
486+
public async Task TestDelegateUseAsConstructorArgumentsWithConflictingExpressionOverloadAsync()
487+
{
488+
var testCode = @"
489+
using System;
490+
using System.Linq.Expressions;
491+
public class TypeName
492+
{
493+
public TypeName(Action argument)
494+
{
495+
496+
}
497+
498+
public TypeName(Expression<Action> argument)
499+
{
500+
501+
}
502+
503+
public void Test()
504+
{
505+
new TypeName(delegate { });
506+
}
507+
}";
508+
await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
509+
}
510+
511+
[Fact]
512+
public async Task TestDelegateUseAsConstructorArgumentsWithNonConflictingExpressionOverloadAsync()
513+
{
514+
var testCode = @"
515+
using System;
516+
using System.Linq.Expressions;
517+
public class TypeName
518+
{
519+
public TypeName(Action argument)
520+
{
521+
522+
}
523+
524+
public TypeName(Expression<Func<int>> argument)
525+
{
526+
527+
}
528+
529+
public void Test()
530+
{
531+
new TypeName(delegate { });
532+
}
533+
}";
534+
var fixedCode = @"
535+
using System;
536+
using System.Linq.Expressions;
537+
public class TypeName
538+
{
539+
public TypeName(Action argument)
540+
{
541+
542+
}
543+
544+
public TypeName(Expression<Func<int>> argument)
545+
{
546+
547+
}
548+
549+
public void Test()
550+
{
551+
new TypeName(() => { });
552+
}
553+
}";
554+
var expected = Diagnostic().WithLocation(18, 22);
555+
await VerifyCSharpFixAsync(testCode, expected, fixedCode, CancellationToken.None).ConfigureAwait(false);
556+
}
557+
558+
[Fact]
559+
public async Task TestDelegateUseAsIndexerArgumentsAsync()
560+
{
561+
var testCode = @"
562+
using System;
563+
public class TypeName
564+
{
565+
public int this[Action argument]
566+
{
567+
get { return 0; }
568+
}
569+
570+
public void Test()
571+
{
572+
int _ = this[delegate { }];
573+
}
574+
}";
575+
string fixedCode = @"
576+
using System;
577+
public class TypeName
578+
{
579+
public int this[Action argument]
580+
{
581+
get { return 0; }
582+
}
583+
584+
public void Test()
585+
{
586+
int _ = this[() => { }];
587+
}
588+
}";
589+
var expected = Diagnostic().WithLocation(12, 22);
590+
await VerifyCSharpFixAsync(testCode, expected, fixedCode, CancellationToken.None).ConfigureAwait(false);
591+
}
592+
593+
[Fact]
594+
public async Task TestDelegateUseAsIndexerArgumentsWithConflictingExpressionOverloadAsync()
595+
{
596+
var testCode = @"
597+
using System;
598+
using System.Linq.Expressions;
599+
public class TypeName
600+
{
601+
public int this[Action argument]
602+
{
603+
get { return 0; }
604+
}
605+
public int this[Expression<Action> argument]
606+
{
607+
get { return 0; }
608+
}
609+
public void Test()
610+
{
611+
int _ = this[delegate { }];
612+
}
613+
}";
614+
await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
615+
}
616+
617+
[Fact]
618+
public async Task TestDelegateUseAsIndexerArgumentsWithNonConflictingExpressionOverloadAsync()
619+
{
620+
var testCode = @"
621+
using System;
622+
using System.Linq.Expressions;
623+
public class TypeName
624+
{
625+
public int this[Action argument]
626+
{
627+
get { return 0; }
628+
}
629+
630+
public int this[Expression<Func<int>> argument]
631+
{
632+
get { return 0; }
633+
}
634+
635+
public void Test()
636+
{
637+
int _ = this[delegate { }];
638+
}
639+
}";
640+
var fixedCode = @"
641+
using System;
642+
using System.Linq.Expressions;
643+
public class TypeName
644+
{
645+
public int this[Action argument]
646+
{
647+
get { return 0; }
648+
}
649+
650+
public int this[Expression<Func<int>> argument]
651+
{
652+
get { return 0; }
653+
}
654+
655+
public void Test()
656+
{
657+
int _ = this[() => { }];
658+
}
659+
}";
660+
var expected = Diagnostic().WithLocation(18, 22);
661+
await VerifyCSharpFixAsync(testCode, expected, fixedCode, CancellationToken.None).ConfigureAwait(false);
662+
}
663+
664+
[Fact]
665+
public async Task TestInvalidOverloadAsync()
666+
{
667+
var testCode = @"
668+
using System;
669+
using System.Linq.Expressions;
670+
public unsafe class TypeName
671+
{
672+
void Method(int* data) { throw null; }
673+
void Caller() => Method(delegate { });
674+
}";
675+
var fixedCode = @"
676+
using System;
677+
using System.Linq.Expressions;
678+
public unsafe class TypeName
679+
{
680+
void Method(int* data) { throw null; }
681+
void Caller() => Method(delegate { });
682+
}";
683+
var expected = DiagnosticResult.CompilerError("CS1660").WithLocation(7, 29);
684+
await VerifyCSharpFixAsync(testCode, expected, fixedCode, CancellationToken.None).ConfigureAwait(false);
685+
}
449686
}
450687
}

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

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -50,16 +50,32 @@ public override void Initialize(AnalysisContext context)
5050
/// <summary>
5151
/// Gets the delegate parameter list from a method symbol and the argument index.
5252
/// </summary>
53-
/// <param name="methodSymbol">The symbol containing information about the method invocation.</param>
53+
/// <param name="symbol">The symbol containing information about the method invocation.</param>
5454
/// <param name="argumentIndex">The index of the argument containing the delegate.</param>
5555
/// <returns>A parameter list for the delegate parameters.</returns>
56-
internal static ParameterListSyntax GetDelegateParameterList(IMethodSymbol methodSymbol, int argumentIndex)
56+
internal static ParameterListSyntax GetDelegateParameterList(ISymbol symbol, int argumentIndex)
5757
{
58-
var realIndex = Math.Min(argumentIndex, methodSymbol.Parameters.Length - 1);
58+
ImmutableArray<IParameterSymbol> parameterList;
59+
int realIndex;
60+
INamedTypeSymbol delegateType;
61+
if (symbol is IMethodSymbol methodSymbol)
62+
{
63+
parameterList = methodSymbol.Parameters;
64+
}
65+
else if (symbol is IPropertySymbol propertySymbol)
66+
{
67+
parameterList = propertySymbol.Parameters;
68+
}
69+
else
70+
{
71+
throw new InvalidOperationException("This should be unreachable.");
72+
}
73+
74+
realIndex = Math.Min(argumentIndex, parameterList.Length - 1);
5975

60-
var type = methodSymbol.Parameters[realIndex].Type;
76+
var type = parameterList[realIndex].Type;
6177

62-
if (methodSymbol.Parameters[realIndex].IsParams)
78+
if (parameterList[realIndex].IsParams)
6379
{
6480
if (type is IArrayTypeSymbol arrayTypeSymbol)
6581
{
@@ -73,7 +89,8 @@ internal static ParameterListSyntax GetDelegateParameterList(IMethodSymbol metho
7389
}
7490
}
7591

76-
var delegateType = (INamedTypeSymbol)type;
92+
delegateType = (INamedTypeSymbol)type;
93+
7794
var delegateParameters = delegateType.DelegateInvokeMethod.Parameters;
7895

7996
var syntaxParameters = GetSyntaxParametersFromSymbolParameters(delegateParameters);
@@ -111,24 +128,24 @@ private static void HandleAnonymousMethodExpression(SyntaxNodeAnalysisContext co
111128
private static bool HandleMethodInvocation(SemanticModel semanticModel, AnonymousMethodExpressionSyntax anonymousMethod, ArgumentSyntax argumentSyntax)
112129
{
113130
// invocation -> argument list -> argument -> anonymous method
114-
var argumentListSyntax = argumentSyntax?.Parent as ArgumentListSyntax;
131+
var argumentListSyntax = argumentSyntax?.Parent as BaseArgumentListSyntax;
115132

116-
if (argumentListSyntax?.Parent is InvocationExpressionSyntax originalInvocationExpression)
133+
if (argumentListSyntax != null)
117134
{
118-
SymbolInfo originalSymbolInfo = semanticModel.GetSymbolInfo(originalInvocationExpression);
135+
var originalInvocableExpression = argumentListSyntax.Parent;
136+
SymbolInfo originalSymbolInfo = semanticModel.GetSymbolInfo(originalInvocableExpression);
137+
Location location = originalInvocableExpression.GetLocation();
119138

120139
if (originalSymbolInfo.Symbol == null)
121140
{
122141
// The most likely cause for this is an overload resolution failure.
123142
return false;
124143
}
125144

126-
Location location = originalInvocationExpression.GetLocation();
127-
128145
var argumentIndex = argumentListSyntax.Arguments.IndexOf(argumentSyntax);
129146

130147
// 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.
131-
var parameterList = GetDelegateParameterList((IMethodSymbol)originalSymbolInfo.Symbol, argumentIndex);
148+
var parameterList = GetDelegateParameterList(originalSymbolInfo.Symbol, argumentIndex);
132149

133150
if (parameterList == null)
134151
{
@@ -144,7 +161,7 @@ private static bool HandleMethodInvocation(SemanticModel semanticModel, Anonymou
144161
SyntaxFactory.Token(SyntaxKind.EqualsGreaterThanToken),
145162
anonymousMethod.Body);
146163

147-
var invocationExpression = originalInvocationExpression.ReplaceNode(anonymousMethod, lambdaExpression);
164+
var invocationExpression = originalInvocableExpression.ReplaceNode(anonymousMethod, lambdaExpression);
148165
SymbolInfo newSymbolInfo = semanticModel.GetSpeculativeSymbolInfo(location.SourceSpan.Start, invocationExpression, SpeculativeBindingOption.BindAsExpression);
149166

150167
if (!originalSymbolInfo.Symbol.Equals(newSymbolInfo.Symbol))

0 commit comments

Comments
 (0)