From 67110e8b1970dbe4b916575d0c31d3e73b340a67 Mon Sep 17 00:00:00 2001 From: Vincent Weijsters Date: Sun, 24 Feb 2019 16:19:49 +0100 Subject: [PATCH 1/3] Fixed SA1130 code fix for return statements and arrow expression clauses --- .../ReadabilityRules/SA1130CodeFixProvider.cs | 24 +++++- .../ReadabilityRules/SA1130UnitTests.cs | 75 ++++++++++++++++++- 2 files changed, 94 insertions(+), 5 deletions(-) diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/ReadabilityRules/SA1130CodeFixProvider.cs b/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/ReadabilityRules/SA1130CodeFixProvider.cs index 9d66d8cc4..88cd190f6 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/ReadabilityRules/SA1130CodeFixProvider.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/ReadabilityRules/SA1130CodeFixProvider.cs @@ -72,6 +72,7 @@ private static SyntaxNode ReplaceWithLambda(SemanticModel semanticModel, Anonymo { var parameterList = anonymousMethod.ParameterList; SyntaxNode lambdaExpression; + SyntaxToken arrowToken; if (parameterList == null) { @@ -98,6 +99,11 @@ private static SyntaxNode ReplaceWithLambda(SemanticModel semanticModel, Anonymo argumentList = list.Value; break; + + case SyntaxKind.ArrowExpressionClause: + case SyntaxKind.ReturnStatement: + argumentList = GetMemberReturnTypeArgumentList(semanticModel, anonymousMethod); + break; } List parameters = GenerateUniqueParameterNames(semanticModel, anonymousMethod, argumentList); @@ -107,12 +113,17 @@ private static SyntaxNode ReplaceWithLambda(SemanticModel semanticModel, Anonymo : SyntaxFactory.SeparatedList(); parameterList = SyntaxFactory.ParameterList(newList) - .WithLeadingTrivia(anonymousMethod.DelegateKeyword.LeadingTrivia) + .WithLeadingTrivia(anonymousMethod.DelegateKeyword.LeadingTrivia); + + arrowToken = SyntaxFactory.Token(SyntaxKind.EqualsGreaterThanToken) .WithTrailingTrivia(anonymousMethod.DelegateKeyword.TrailingTrivia); } else { parameterList = parameterList.WithLeadingTrivia(anonymousMethod.DelegateKeyword.TrailingTrivia); + + arrowToken = SyntaxFactory.Token(SyntaxKind.EqualsGreaterThanToken) + .WithTrailingTrivia(SyntaxFactory.ElasticSpace); } foreach (var parameter in parameterList.Parameters) @@ -123,9 +134,6 @@ private static SyntaxNode ReplaceWithLambda(SemanticModel semanticModel, Anonymo } } - var arrowToken = SyntaxFactory.Token(SyntaxKind.EqualsGreaterThanToken) - .WithTrailingTrivia(SyntaxFactory.ElasticSpace); - if (parameterList.Parameters.Count == 1) { var parameterSyntax = RemoveType(parameterList.Parameters[0]); @@ -193,6 +201,14 @@ private static ImmutableArray GetEqualsArgumentList(SemanticModel semant return namedTypeSymbol.DelegateInvokeMethod.Parameters.Select(ps => ps.Name).ToImmutableArray(); } + private static ImmutableArray GetMemberReturnTypeArgumentList(SemanticModel semanticModel, AnonymousMethodExpressionSyntax anonymousMethod) + { + var enclosingSymbol = semanticModel.GetEnclosingSymbol(anonymousMethod.Parent.SpanStart); + var returnType = (INamedTypeSymbol)((IMethodSymbol)enclosingSymbol).ReturnType; + + return returnType.DelegateInvokeMethod.Parameters.Select(ps => ps.Name).ToImmutableArray(); + } + private static List GenerateUniqueParameterNames(SemanticModel semanticModel, AnonymousMethodExpressionSyntax anonymousMethod, ImmutableArray argumentNames) { var parameters = new List(); diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1130UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1130UnitTests.cs index d43b84ba3..6c8085db7 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1130UnitTests.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1130UnitTests.cs @@ -276,7 +276,7 @@ public class TypeName { public void Test() { - Action action1 = /*a*/()/*b*/ => { }; + Action action1 = /*a*/() =>/*b*/{ }; Action action2 = /*a*//*b*/(/*c*/)/*d*/ => { }; Action action3 = /*a*//*b*//*c*//*d*/i/*e*//*f*/ => { }; Action> action4 = i => { }; @@ -710,5 +710,78 @@ static void Main(string[] args) await VerifyCSharpFixAsync(testCode, expected, testCode, CancellationToken.None).ConfigureAwait(false); } + + [Fact] + [WorkItem(2902, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/2902")] + public async Task VerifyThatCodeFixDoesNotCrashOnDelegateReturnAsync() + { + var testCode = @"using System; +public class TestClass +{ + public static EventHandler TestMethod1() + { + return delegate + { + }; + } + + public static EventHandler TestMethod2() => delegate + { + }; + + public static EventHandler TestProperty1 + { + get + { + return delegate + { + }; + } + } + + public static EventHandler TestProperty2 => delegate + { + }; +}"; + + var fixedCode = @"using System; +public class TestClass +{ + public static EventHandler TestMethod1() + { + return (sender, e) => + { + }; + } + + public static EventHandler TestMethod2() => (sender, e) => + { + }; + + public static EventHandler TestProperty1 + { + get + { + return (sender, e) => + { + }; + } + } + + public static EventHandler TestProperty2 => (sender, e) => + { + }; +}"; + + DiagnosticResult[] expected = + { + Diagnostic().WithLocation(6, 16), + Diagnostic().WithLocation(11, 49), + Diagnostic().WithLocation(19, 20), + Diagnostic().WithLocation(25, 49), + }; + + await VerifyCSharpFixAsync(testCode, expected, fixedCode, CancellationToken.None).ConfigureAwait(false); + } } } From 77ee36f1187eccbbdee6a36b563cf56a34d05c52 Mon Sep 17 00:00:00 2001 From: Vincent Weijsters Date: Wed, 27 Feb 2019 11:18:29 +0100 Subject: [PATCH 2/3] Added test cases for event initializers --- .../ReadabilityRules/SA1130UnitTests.cs | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1130UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1130UnitTests.cs index 6c8085db7..bf687acb1 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1130UnitTests.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1130UnitTests.cs @@ -783,5 +783,34 @@ public static EventHandler TestProperty1 await VerifyCSharpFixAsync(testCode, expected, fixedCode, CancellationToken.None).ConfigureAwait(false); } + + [Fact] + [WorkItem(2902, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/2902")] + public async Task VerifyThatEventInitializersWorkAsExpectedAsync() + { + var testCode = @"using System; +public class TestClass +{ + public static event EventHandler StaticEvent = delegate { }; + public event EventHandler InstanceEvent = delegate { }; +} +"; + + var fixedCode = @"using System; +public class TestClass +{ + public static event EventHandler StaticEvent = (sender, e) => { }; + public event EventHandler InstanceEvent = (sender, e) => { }; +} +"; + + DiagnosticResult[] expected = + { + Diagnostic().WithLocation(4, 52), + Diagnostic().WithLocation(5, 47), + }; + + await VerifyCSharpFixAsync(testCode, expected, fixedCode, CancellationToken.None).ConfigureAwait(false); + } } } From ffe11e1fdb5bea0f60e2c17efc5f873f6c5b06f8 Mon Sep 17 00:00:00 2001 From: Vincent Weijsters Date: Fri, 1 Mar 2019 16:40:12 +0100 Subject: [PATCH 3/3] Added test for invalid construction and local function --- .../ReadabilityRules/SA1130CodeFixProvider.cs | 10 +++-- .../SA1130CSharp7UnitTests.cs | 38 +++++++++++++++++++ .../ReadabilityRules/SA1130UnitTests.cs | 28 ++++++++++++++ 3 files changed, 73 insertions(+), 3 deletions(-) diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/ReadabilityRules/SA1130CodeFixProvider.cs b/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/ReadabilityRules/SA1130CodeFixProvider.cs index 88cd190f6..60b6ed3b2 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/ReadabilityRules/SA1130CodeFixProvider.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/ReadabilityRules/SA1130CodeFixProvider.cs @@ -103,6 +103,11 @@ private static SyntaxNode ReplaceWithLambda(SemanticModel semanticModel, Anonymo case SyntaxKind.ArrowExpressionClause: case SyntaxKind.ReturnStatement: argumentList = GetMemberReturnTypeArgumentList(semanticModel, anonymousMethod); + if (argumentList.IsEmpty) + { + return null; + } + break; } @@ -204,9 +209,8 @@ private static ImmutableArray GetEqualsArgumentList(SemanticModel semant private static ImmutableArray GetMemberReturnTypeArgumentList(SemanticModel semanticModel, AnonymousMethodExpressionSyntax anonymousMethod) { var enclosingSymbol = semanticModel.GetEnclosingSymbol(anonymousMethod.Parent.SpanStart); - var returnType = (INamedTypeSymbol)((IMethodSymbol)enclosingSymbol).ReturnType; - - return returnType.DelegateInvokeMethod.Parameters.Select(ps => ps.Name).ToImmutableArray(); + var returnType = ((IMethodSymbol)enclosingSymbol).ReturnType as INamedTypeSymbol; + return (returnType == null) ? ImmutableArray.Empty : returnType.DelegateInvokeMethod.Parameters.Select(ps => ps.Name).ToImmutableArray(); } private static List GenerateUniqueParameterNames(SemanticModel semanticModel, AnonymousMethodExpressionSyntax anonymousMethod, ImmutableArray argumentNames) diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp7/ReadabilityRules/SA1130CSharp7UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp7/ReadabilityRules/SA1130CSharp7UnitTests.cs index 880861ffd..b99e21eb9 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp7/ReadabilityRules/SA1130CSharp7UnitTests.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp7/ReadabilityRules/SA1130CSharp7UnitTests.cs @@ -3,9 +3,47 @@ namespace StyleCop.Analyzers.Test.CSharp7.ReadabilityRules { + using System.Threading; + using System.Threading.Tasks; + using Microsoft.CodeAnalysis.Testing; using StyleCop.Analyzers.Test.ReadabilityRules; + using Xunit; + using static StyleCop.Analyzers.Test.Verifiers.StyleCopCodeFixVerifier< + StyleCop.Analyzers.ReadabilityRules.SA1130UseLambdaSyntax, + StyleCop.Analyzers.ReadabilityRules.SA1130CodeFixProvider>; public class SA1130CSharp7UnitTests : SA1130UnitTests { + [Fact] + [WorkItem(2902, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/2902")] + public async Task VerifyLocalFunctionAsync() + { + var testCode = @"using System; +public class TestClass +{ + public void TestMethod() + { + EventHandler LocalTestFunction() => delegate { }; + } +} +"; + + var fixedCode = @"using System; +public class TestClass +{ + public void TestMethod() + { + EventHandler LocalTestFunction() => (sender, e) => { }; + } +} +"; + + DiagnosticResult[] expected = + { + Diagnostic().WithLocation(6, 45), + }; + + await VerifyCSharpFixAsync(testCode, expected, fixedCode, CancellationToken.None).ConfigureAwait(false); + } } } diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1130UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1130UnitTests.cs index bf687acb1..eaae1cbc5 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1130UnitTests.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1130UnitTests.cs @@ -812,5 +812,33 @@ public class TestClass await VerifyCSharpFixAsync(testCode, expected, fixedCode, CancellationToken.None).ConfigureAwait(false); } + + [Fact] + [WorkItem(2902, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/2902")] + public async Task VerifyInvalidCodeConstructionsAsync() + { + var testCode = @"using System; +public class TestClass +{ + public static EventHandler[] TestMethod() => delegate { }; +} +"; + + DiagnosticResult[] expected = + { + Diagnostic().WithSpan(4, 50, 4, 58), + DiagnosticResult.CompilerError("CS1660").WithMessage("Cannot convert anonymous method to type 'EventHandler[]' because it is not a delegate type").WithSpan(4, 50, 4, 62), + }; + + var test = new CSharpTest + { + TestCode = testCode, + FixedCode = testCode, + }; + + test.ExpectedDiagnostics.AddRange(expected); + test.RemainingDiagnostics.AddRange(expected); + await test.RunAsync(CancellationToken.None).ConfigureAwait(false); + } } }