From b60f5748251a26566447c2918f09e8532b9175aa Mon Sep 17 00:00:00 2001 From: Dmitry Turin Date: Mon, 14 Sep 2015 17:30:00 +0300 Subject: [PATCH 1/3] Implemented SA1117 diagnostic + unit tests --- .../ReadabilityRules/SA1117UnitTests.cs | 233 +++++++++++++ .../StyleCop.Analyzers.Test.csproj | 1 + .../ReadabilityResources.Designer.cs | 2 +- .../ReadabilityResources.resx | 2 +- ...rametersMustBeOnSameLineOrSeparateLines.cs | 313 +++++++++++++++++- 5 files changed, 542 insertions(+), 9 deletions(-) create mode 100644 StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1117UnitTests.cs diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1117UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1117UnitTests.cs new file mode 100644 index 000000000..8cca226f6 --- /dev/null +++ b/StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1117UnitTests.cs @@ -0,0 +1,233 @@ +// Copyright (c) Tunnel Vision Laboratories, LLC. All Rights Reserved. +// Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information. + +namespace StyleCop.Analyzers.Test.ReadabilityRules +{ + using System.Collections.Generic; + using System.Threading; + using System.Threading.Tasks; + using Microsoft.CodeAnalysis.Diagnostics; + using StyleCop.Analyzers.ReadabilityRules; + using TestHelper; + using Xunit; + + public class SA1117UnitTests : DiagnosticVerifier + { + public static IEnumerable GetTestDeclarations(string delimiter) + { + yield return new object[] { $"public Foo(int a, int b,{delimiter} string s) {{ }}" }; + yield return new object[] { $"public object Bar(int a, int b,{delimiter} string s) => null;" }; + yield return new object[] { $"public object this[int a, int b,{delimiter} string s] => null;" }; + yield return new object[] { $"public delegate void Bar(int a, int b,{delimiter} string s);" }; + } + + public static IEnumerable GetTestConstructorInitializers(string delimiter) + { + yield return new object[] { $"this(42, 43, {delimiter} \"hello\")" }; + yield return new object[] { $"base(42, 43, {delimiter} \"hello\")" }; + } + + public static IEnumerable GetTestExpressions(string delimiter) + { + yield return new object[] { $"Bar(1, 2, {delimiter} 2)", 13 }; + yield return new object[] { $"System.Action func = (int x, int y, {delimiter} int z) => Bar(x, y, z)", 41 }; + yield return new object[] { $"System.Action func = delegate(int x, int y, {delimiter} int z) {{ Bar(x, y, z); }}", 49 }; + yield return new object[] { $"new System.DateTime(2015, 9, {delimiter} 14)", 20 }; + yield return new object[] { $"var arr = new string[2, 2, {delimiter} 2];", 30 }; + yield return new object[] { $"char cc = (new char[3, 3, 3])[2, 2,{delimiter} 2];", 36 }; + yield return new object[] { $"char? c = (new char[3, 3, 3])?[2, 2,{delimiter} 2];", 37 }; + yield return new object[] { $"long ll = this[2, 2,{delimiter} 2];", 24 }; + } + + public static IEnumerable ValidTestExpressions() + { + yield return new object[] { $"System.Action func = () => Bar(0, 2, 3)", 0 }; + yield return new object[] { $"System.Action func = x => Bar(x, 2, 3)", 0 }; + yield return new object[] { $"System.Action func = delegate {{ Bar(0, 0, 0); }}", 0 }; + } + + [Theory] + [MemberData(nameof(GetTestDeclarations), "")] + public async Task TestValidDeclarationAsync(string declaration) + { + var testCode = $@" +class Foo +{{ + {declaration} +}}"; + await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false); + } + + [Theory] + [MemberData(nameof(GetTestDeclarations), "\r\n")] + public async Task TestInvalidDeclarationAsync(string declaration) + { + var testCode = $@" +class Foo +{{ + {declaration} +}}"; + + DiagnosticResult expected = this.CSharpDiagnostic().WithLocation(5, 2); + await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); + } + + [Theory] + [MemberData(nameof(GetTestConstructorInitializers), "")] + public async Task TestValidConstructorInitializerAsync(string initializer) + { + var testCode = $@" +class Base +{{ + public Base(int a, int b, string s) + {{ + }} +}} + +class Derived : Base +{{ + public Derived() + : {initializer} + {{ + }} + + public Derived(int i, int j, string z) + : base(i, j, z) + {{ + }} +}}"; + + await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false); + } + + [Theory] + [MemberData(nameof(GetTestConstructorInitializers), "\r\n")] + public async Task TestInvalidConstructorInitializerAsync(string initializer) + { + var testCode = $@" +class Base +{{ + public Base(int a, int b, string s) + {{ + }} +}} + +class Derived : Base +{{ + public Derived() + : {initializer} + {{ + }} + + public Derived(int i, int j, string z) + : base(i, j, z) + {{ + }} +}}"; + + DiagnosticResult expected = this.CSharpDiagnostic().WithLocation(13, 2); + await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); + } + + [Theory] + [MemberData(nameof(GetTestExpressions), "")] + [MemberData(nameof(ValidTestExpressions))] + public async Task TestValidExpressionAsync(string expression, int column) + { + var testCode = $@" +class Foo +{{ + public void Bar(int i, int j, int k) + {{ + }} + + public void Baz() + {{ + {expression}; + }} + + public long this[int a, int b, int s] => a + b + s; +}}"; + + await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false); + } + + [Theory] + [MemberData(nameof(GetTestExpressions), "\r\n")] + public async Task TestInvalidExpressionAsync(string expression, int column) + { + var testCode = $@" +class Foo +{{ + public void Bar(int i, int j, int k) + {{ + }} + + public void Baz() + {{ + {expression}; + }} + + public long this[int a, int b, int s] => a + b + s; +}}"; + + DiagnosticResult expected = this.CSharpDiagnostic().WithLocation(11, 2); + await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); + } + + [Fact] + public async Task TestValidAttributeAsync() + { + var testCode = @" +[System.AttributeUsage(System.AttributeTargets.Class)] +public class MyAttribute : System.Attribute +{ + public MyAttribute(int a, int b, int c) + { + } +} + +[MyAttribute(1, 2, 3)] +class Foo +{ +} + +// This is a regression test for https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/1211 +[System.Obsolete] +class ObsoleteType +{ +}"; + + await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false); + } + + [Fact] + public async Task TestInvalidAttributeAsync() + { + var testCode = @" +[System.AttributeUsage(System.AttributeTargets.Class)] +public class MyAttribute : System.Attribute +{ + public MyAttribute(int a, int b, int c) + { + } +} + +[MyAttribute( + 1, + 2, 3)] +class Foo +{ +}"; + + DiagnosticResult expected = this.CSharpDiagnostic().WithLocation(12, 8); + await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); + } + + /// + protected override IEnumerable GetCSharpDiagnosticAnalyzers() + { + yield return new SA1117ParametersMustBeOnSameLineOrSeparateLines(); + } + } +} diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/StyleCop.Analyzers.Test.csproj b/StyleCop.Analyzers/StyleCop.Analyzers.Test/StyleCop.Analyzers.Test.csproj index aa48d68ed..9254b7f6c 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.Test/StyleCop.Analyzers.Test.csproj +++ b/StyleCop.Analyzers/StyleCop.Analyzers.Test/StyleCop.Analyzers.Test.csproj @@ -289,6 +289,7 @@ + diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/ReadabilityResources.Designer.cs b/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/ReadabilityResources.Designer.cs index 75270e9f3..ef0ba7a5e 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/ReadabilityResources.Designer.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/ReadabilityResources.Designer.cs @@ -611,7 +611,7 @@ internal static string SA1117Description { } /// - /// Looks up a localized string similar to . + /// Looks up a localized string similar to The parameters must all be placed on the same line or alternatively, each parameter must be placed on its own line.. /// internal static string SA1117MessageFormat { get { diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/ReadabilityResources.resx b/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/ReadabilityResources.resx index 6192dee5e..af8245de5 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/ReadabilityResources.resx +++ b/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/ReadabilityResources.resx @@ -301,7 +301,7 @@ The parameters to a C# method or indexer call or declaration are not all on the same line or each on a separate line. - + The parameters must all be placed on the same line or alternatively, each parameter must be placed on its own line. Parameters must be on same line or separate lines diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1117ParametersMustBeOnSameLineOrSeparateLines.cs b/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1117ParametersMustBeOnSameLineOrSeparateLines.cs index 446030875..8d3fecf6a 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1117ParametersMustBeOnSameLineOrSeparateLines.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1117ParametersMustBeOnSameLineOrSeparateLines.cs @@ -3,9 +3,13 @@ namespace StyleCop.Analyzers.ReadabilityRules { + using System; using System.Collections.Immutable; using Microsoft.CodeAnalysis; + using Microsoft.CodeAnalysis.CSharp; + using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Diagnostics; + using StyleCop.Analyzers.Helpers; /// /// The parameters to a C# method or indexer call or declaration are not all on the same line or each on a separate @@ -55,24 +59,319 @@ public class SA1117ParametersMustBeOnSameLineOrSeparateLines : DiagnosticAnalyze private static readonly string HelpLink = "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1117.md"; private static readonly DiagnosticDescriptor Descriptor = - new DiagnosticDescriptor(DiagnosticId, Title, MessageFormat, AnalyzerCategory.ReadabilityRules, DiagnosticSeverity.Warning, AnalyzerConstants.DisabledNoTests, Description, HelpLink); + new DiagnosticDescriptor(DiagnosticId, Title, MessageFormat, AnalyzerCategory.ReadabilityRules, DiagnosticSeverity.Warning, AnalyzerConstants.EnabledByDefault, Description, HelpLink); private static readonly ImmutableArray SupportedDiagnosticsValue = ImmutableArray.Create(Descriptor); /// - public override ImmutableArray SupportedDiagnostics + public override ImmutableArray SupportedDiagnostics => SupportedDiagnosticsValue; + + /// + public override void Initialize(AnalysisContext context) + { + context.RegisterCompilationStartAction(HandleCompilationStart); + } + + private static void HandleCompilationStart(CompilationStartAnalysisContext context) + { + context.RegisterSyntaxNodeActionHonorExclusions( + HandleBaseMethodDeclaration, + new[] { SyntaxKind.ConstructorDeclaration, SyntaxKind.MethodDeclaration }); + + context.RegisterSyntaxNodeActionHonorExclusions( + HandleConstructorInitializer, + new[] { SyntaxKind.BaseConstructorInitializer, SyntaxKind.ThisConstructorInitializer }); + + context.RegisterSyntaxNodeActionHonorExclusions(HandleDelegateDeclaration, SyntaxKind.DelegateDeclaration); + context.RegisterSyntaxNodeActionHonorExclusions(HandleIndexerDeclaration, SyntaxKind.IndexerDeclaration); + context.RegisterSyntaxNodeActionHonorExclusions(HandleInvocation, SyntaxKind.InvocationExpression); + context.RegisterSyntaxNodeActionHonorExclusions(HandleObjectCreation, SyntaxKind.ObjectCreationExpression); + context.RegisterSyntaxNodeActionHonorExclusions(HandleElementAccess, SyntaxKind.ElementAccessExpression); + context.RegisterSyntaxNodeActionHonorExclusions(HandleElementBinding, SyntaxKind.ElementBindingExpression); + context.RegisterSyntaxNodeActionHonorExclusions(HandleArrayCreation, SyntaxKind.ArrayCreationExpression); + context.RegisterSyntaxNodeActionHonorExclusions(HandleAttribute, SyntaxKind.Attribute); + context.RegisterSyntaxNodeActionHonorExclusions(HandleAnonymousMethod, SyntaxKind.AnonymousMethodExpression); + context.RegisterSyntaxNodeActionHonorExclusions(HandleLambda, SyntaxKind.ParenthesizedLambdaExpression); + } + + private static void HandleBaseMethodDeclaration(SyntaxNodeAnalysisContext context) + { + var declaration = (BaseMethodDeclarationSyntax)context.Node; + HandleParameterListSyntax(context, declaration.ParameterList); + } + + private static void HandleInvocation(SyntaxNodeAnalysisContext context) { - get + var invocation = (InvocationExpressionSyntax)context.Node; + HandleArgumentListSyntax(context, invocation.ArgumentList); + } + + private static void HandleObjectCreation(SyntaxNodeAnalysisContext context) + { + var objectCreation = (ObjectCreationExpressionSyntax)context.Node; + HandleArgumentListSyntax(context, objectCreation.ArgumentList); + } + + private static void HandleIndexerDeclaration(SyntaxNodeAnalysisContext context) + { + var indexerDeclaration = (IndexerDeclarationSyntax)context.Node; + BracketedParameterListSyntax argumentListSyntax = indexerDeclaration.ParameterList; + SeparatedSyntaxList arguments = argumentListSyntax.Parameters; + + if (arguments.Count > 2) { - return SupportedDiagnosticsValue; + Analyze(context, argumentListSyntax.Parameters); } } - /// - public override void Initialize(AnalysisContext context) + private static void HandleElementAccess(SyntaxNodeAnalysisContext context) + { + var elementAccess = (ElementAccessExpressionSyntax)context.Node; + HandleBracketedArgumentListSyntax(context, elementAccess.ArgumentList); + } + + private static void HandleArrayCreation(SyntaxNodeAnalysisContext context) + { + var arrayCreation = (ArrayCreationExpressionSyntax)context.Node; + + foreach (var rankSpecifier in arrayCreation.Type.RankSpecifiers) + { + SeparatedSyntaxList sizes = rankSpecifier.Sizes; + if (sizes.Count < 3) + { + continue; + } + + ExpressionSyntax previousParameter = sizes[1]; + int firstParameterLine = sizes[0].GetLine(); + int previousLine = previousParameter.GetLine(); + Func lineCondition; + + if (firstParameterLine == previousLine) + { + // arguments must be on same line + lineCondition = (param1Line, param2Line) => param1Line != param2Line; + } + else if (previousLine - firstParameterLine == 1) + { + // each argument must be on its own line + lineCondition = (param1Line, param2Line) => param1Line == param2Line; + } + else + { + // SA1115 should be handled first + return; + } + + for (int i = 2; i < sizes.Count; ++i) + { + ExpressionSyntax currentParameter = sizes[i]; + int currentLine = currentParameter.GetLine(); + + if (lineCondition(previousLine, currentLine)) + { + context.ReportDiagnostic(Diagnostic.Create(Descriptor, currentParameter.GetLocation())); + return; + } + + previousLine = currentLine; + } + } + } + + private static void HandleAttribute(SyntaxNodeAnalysisContext context) + { + var attribute = (AttributeSyntax)context.Node; + AttributeArgumentListSyntax argumentListSyntax = attribute.ArgumentList; + if (argumentListSyntax == null) + { + return; + } + + SeparatedSyntaxList arguments = argumentListSyntax.Arguments; + if (arguments.Count < 3) + { + return; + } + + AttributeArgumentSyntax previousParameter = arguments[1]; + int firstParameterLine = arguments[0].GetLine(); + int previousLine = previousParameter.GetLine(); + Func lineCondition; + + if (firstParameterLine == previousLine) + { + // arguments must be on same line + lineCondition = (param1Line, param2Line) => param1Line != param2Line; + } + else if (previousLine - firstParameterLine == 1) + { + // each argument must be on its own line + lineCondition = (param1Line, param2Line) => param1Line == param2Line; + } + else + { + // SA1115 should be handled first + return; + } + + for (int i = 2; i < arguments.Count; ++i) + { + AttributeArgumentSyntax currentParameter = arguments[i]; + int currentLine = currentParameter.GetLine(); + + if (lineCondition(previousLine, currentLine)) + { + context.ReportDiagnostic(Diagnostic.Create(Descriptor, currentParameter.GetLocation())); + return; + } + + previousLine = currentLine; + } + } + + private static void HandleAnonymousMethod(SyntaxNodeAnalysisContext context) + { + var anonymousMethod = (AnonymousMethodExpressionSyntax)context.Node; + HandleParameterListSyntax(context, anonymousMethod.ParameterList); + } + + private static void HandleLambda(SyntaxNodeAnalysisContext context) { - // TODO: Implement analysis + var parenthesizedLambda = (ParenthesizedLambdaExpressionSyntax)context.Node; + HandleParameterListSyntax(context, parenthesizedLambda.ParameterList); + } + + private static void HandleDelegateDeclaration(SyntaxNodeAnalysisContext context) + { + var delegateDeclaration = (DelegateDeclarationSyntax)context.Node; + HandleParameterListSyntax(context, delegateDeclaration.ParameterList); + } + + private static void HandleConstructorInitializer(SyntaxNodeAnalysisContext context) + { + var constructorInitializer = (ConstructorInitializerSyntax)context.Node; + HandleArgumentListSyntax(context, constructorInitializer.ArgumentList); + } + + private static void HandleElementBinding(SyntaxNodeAnalysisContext context) + { + var elementBinding = (ElementBindingExpressionSyntax)context.Node; + HandleBracketedArgumentListSyntax(context, elementBinding.ArgumentList); + } + + private static void HandleArgumentListSyntax(SyntaxNodeAnalysisContext context, ArgumentListSyntax argumentList) + { + if (argumentList == null) + { + return; + } + + SeparatedSyntaxList arguments = argumentList.Arguments; + if (arguments.Count > 2) + { + Analyze(context, arguments); + } + } + + private static void HandleParameterListSyntax(SyntaxNodeAnalysisContext context, ParameterListSyntax parameterList) + { + if (parameterList == null) + { + return; + } + + SeparatedSyntaxList parameters = parameterList.Parameters; + if (parameters.Count > 2) + { + Analyze(context, parameters); + } + } + + private static void HandleBracketedArgumentListSyntax(SyntaxNodeAnalysisContext context, BracketedArgumentListSyntax bracketedArgumentList) + { + SeparatedSyntaxList arguments = bracketedArgumentList.Arguments; + if (arguments.Count > 2) + { + Analyze(context, arguments); + } + } + + private static void Analyze(SyntaxNodeAnalysisContext context, SeparatedSyntaxList parameters) + { + ParameterSyntax previousParameter = parameters[1]; + int firstParameterLine = parameters[0].GetLine(); + int previousLine = previousParameter.GetLine(); + Func lineCondition; + + if (firstParameterLine == previousLine) + { + // parameters must be on same line + lineCondition = (param1Line, param2Line) => param1Line != param2Line; + } + else if (previousLine - firstParameterLine == 1) + { + // each parameter must be on its own line + lineCondition = (param1Line, param2Line) => param1Line == param2Line; + } + else + { + // SA1115 should be handled first + return; + } + + for (int i = 2; i < parameters.Count; ++i) + { + ParameterSyntax currentParameter = parameters[i]; + int currentLine = currentParameter.GetLine(); + + if (lineCondition(previousLine, currentLine)) + { + context.ReportDiagnostic(Diagnostic.Create(Descriptor, currentParameter.GetLocation())); + return; + } + + previousLine = currentLine; + } + } + + private static void Analyze(SyntaxNodeAnalysisContext context, SeparatedSyntaxList arguments) + { + ArgumentSyntax previousParameter = arguments[1]; + int firstParameterLine = arguments[0].GetLine(); + int previousLine = previousParameter.GetLine(); + Func lineCondition; + + if (firstParameterLine == previousLine) + { + // arguments must be on same line + lineCondition = (param1Line, param2Line) => param1Line != param2Line; + } + else if (previousLine - firstParameterLine == 1) + { + // each argument must be on its own line + lineCondition = (param1Line, param2Line) => param1Line == param2Line; + } + else + { + // SA1115 should be handled first + return; + } + + for (int i = 2; i < arguments.Count; ++i) + { + ArgumentSyntax currentParameter = arguments[i]; + int currentLine = currentParameter.GetLine(); + + if (lineCondition(previousLine, currentLine)) + { + context.ReportDiagnostic(Diagnostic.Create(Descriptor, currentParameter.GetLocation())); + return; + } + + previousLine = currentLine; + } } } } From 25bd95c011561d2b720e4b87a6d96ce2c5d168a4 Mon Sep 17 00:00:00 2001 From: Dmitry Turin Date: Fri, 18 Sep 2015 21:56:56 +0300 Subject: [PATCH 2/3] Address PR feedback --- .../ReadabilityRules/SA1117UnitTests.cs | 18 +++++ ...rametersMustBeOnSameLineOrSeparateLines.cs | 69 +++++++------------ 2 files changed, 43 insertions(+), 44 deletions(-) diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1117UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1117UnitTests.cs index 8cca226f6..71cb0c0c4 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1117UnitTests.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1117UnitTests.cs @@ -44,10 +44,28 @@ public static IEnumerable ValidTestExpressions() yield return new object[] { $"System.Action func = () => Bar(0, 2, 3)", 0 }; yield return new object[] { $"System.Action func = x => Bar(x, 2, 3)", 0 }; yield return new object[] { $"System.Action func = delegate {{ Bar(0, 0, 0); }}", 0 }; + yield return new object[] { "var weird = new int[10][,,,];", 0 }; + } + + public static IEnumerable ValidTestDeclarations() + { + yield return new object[] + { + $@"public Foo( + int a, int b, string s) {{ }}" + }; + yield return new object[] + { + $@"public Foo( + int a, + int b, + string s) {{ }}" + }; } [Theory] [MemberData(nameof(GetTestDeclarations), "")] + [MemberData(nameof(ValidTestDeclarations))] public async Task TestValidDeclarationAsync(string declaration) { var testCode = $@" diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1117ParametersMustBeOnSameLineOrSeparateLines.cs b/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1117ParametersMustBeOnSameLineOrSeparateLines.cs index 8d3fecf6a..c7bffc2d8 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1117ParametersMustBeOnSameLineOrSeparateLines.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1117ParametersMustBeOnSameLineOrSeparateLines.cs @@ -61,11 +61,8 @@ public class SA1117ParametersMustBeOnSameLineOrSeparateLines : DiagnosticAnalyze private static readonly DiagnosticDescriptor Descriptor = new DiagnosticDescriptor(DiagnosticId, Title, MessageFormat, AnalyzerCategory.ReadabilityRules, DiagnosticSeverity.Warning, AnalyzerConstants.EnabledByDefault, Description, HelpLink); - private static readonly ImmutableArray SupportedDiagnosticsValue = - ImmutableArray.Create(Descriptor); - /// - public override ImmutableArray SupportedDiagnostics => SupportedDiagnosticsValue; + public override ImmutableArray SupportedDiagnostics { get; } = ImmutableArray.Create(Descriptor); /// public override void Initialize(AnalysisContext context) @@ -151,17 +148,12 @@ private static void HandleArrayCreation(SyntaxNodeAnalysisContext context) if (firstParameterLine == previousLine) { // arguments must be on same line - lineCondition = (param1Line, param2Line) => param1Line != param2Line; - } - else if (previousLine - firstParameterLine == 1) - { - // each argument must be on its own line lineCondition = (param1Line, param2Line) => param1Line == param2Line; } else { - // SA1115 should be handled first - return; + // each argument must be on its own line + lineCondition = (param1Line, param2Line) => param1Line != param2Line; } for (int i = 2; i < sizes.Count; ++i) @@ -171,11 +163,12 @@ private static void HandleArrayCreation(SyntaxNodeAnalysisContext context) if (lineCondition(previousLine, currentLine)) { - context.ReportDiagnostic(Diagnostic.Create(Descriptor, currentParameter.GetLocation())); - return; + previousLine = currentLine; + continue; } - previousLine = currentLine; + context.ReportDiagnostic(Diagnostic.Create(Descriptor, currentParameter.GetLocation())); + return; } } } @@ -203,17 +196,12 @@ private static void HandleAttribute(SyntaxNodeAnalysisContext context) if (firstParameterLine == previousLine) { // arguments must be on same line - lineCondition = (param1Line, param2Line) => param1Line != param2Line; - } - else if (previousLine - firstParameterLine == 1) - { - // each argument must be on its own line lineCondition = (param1Line, param2Line) => param1Line == param2Line; } else { - // SA1115 should be handled first - return; + // each argument must be on its own line + lineCondition = (param1Line, param2Line) => param1Line != param2Line; } for (int i = 2; i < arguments.Count; ++i) @@ -223,11 +211,12 @@ private static void HandleAttribute(SyntaxNodeAnalysisContext context) if (lineCondition(previousLine, currentLine)) { - context.ReportDiagnostic(Diagnostic.Create(Descriptor, currentParameter.GetLocation())); - return; + previousLine = currentLine; + continue; } - previousLine = currentLine; + context.ReportDiagnostic(Diagnostic.Create(Descriptor, currentParameter.GetLocation())); + return; } } @@ -308,17 +297,12 @@ private static void Analyze(SyntaxNodeAnalysisContext context, SeparatedSyntaxLi if (firstParameterLine == previousLine) { // parameters must be on same line - lineCondition = (param1Line, param2Line) => param1Line != param2Line; - } - else if (previousLine - firstParameterLine == 1) - { - // each parameter must be on its own line lineCondition = (param1Line, param2Line) => param1Line == param2Line; } else { - // SA1115 should be handled first - return; + // each parameter must be on its own line + lineCondition = (param1Line, param2Line) => param1Line != param2Line; } for (int i = 2; i < parameters.Count; ++i) @@ -328,11 +312,12 @@ private static void Analyze(SyntaxNodeAnalysisContext context, SeparatedSyntaxLi if (lineCondition(previousLine, currentLine)) { - context.ReportDiagnostic(Diagnostic.Create(Descriptor, currentParameter.GetLocation())); - return; + previousLine = currentLine; + continue; } - previousLine = currentLine; + context.ReportDiagnostic(Diagnostic.Create(Descriptor, currentParameter.GetLocation())); + return; } } @@ -346,17 +331,12 @@ private static void Analyze(SyntaxNodeAnalysisContext context, SeparatedSyntaxLi if (firstParameterLine == previousLine) { // arguments must be on same line - lineCondition = (param1Line, param2Line) => param1Line != param2Line; - } - else if (previousLine - firstParameterLine == 1) - { - // each argument must be on its own line lineCondition = (param1Line, param2Line) => param1Line == param2Line; } else { - // SA1115 should be handled first - return; + // each argument must be on its own line + lineCondition = (param1Line, param2Line) => param1Line != param2Line; } for (int i = 2; i < arguments.Count; ++i) @@ -366,11 +346,12 @@ private static void Analyze(SyntaxNodeAnalysisContext context, SeparatedSyntaxLi if (lineCondition(previousLine, currentLine)) { - context.ReportDiagnostic(Diagnostic.Create(Descriptor, currentParameter.GetLocation())); - return; + previousLine = currentLine; + continue; } - previousLine = currentLine; + context.ReportDiagnostic(Diagnostic.Create(Descriptor, currentParameter.GetLocation())); + return; } } } From a92d65ab4e10b3697cdd6ddf9779fa2403fe4b03 Mon Sep 17 00:00:00 2001 From: Dmitry Turin Date: Fri, 18 Sep 2015 23:34:36 +0300 Subject: [PATCH 3/3] Fix SA1117 message format --- .../ReadabilityRules/ReadabilityResources.Designer.cs | 2 +- .../ReadabilityRules/ReadabilityResources.resx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/ReadabilityResources.Designer.cs b/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/ReadabilityResources.Designer.cs index ef0ba7a5e..12b927fb3 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/ReadabilityResources.Designer.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/ReadabilityResources.Designer.cs @@ -611,7 +611,7 @@ internal static string SA1117Description { } /// - /// Looks up a localized string similar to The parameters must all be placed on the same line or alternatively, each parameter must be placed on its own line.. + /// Looks up a localized string similar to The parameters must all be placed on the same line or each parameter must be placed on its own line.. /// internal static string SA1117MessageFormat { get { diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/ReadabilityResources.resx b/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/ReadabilityResources.resx index af8245de5..bbc955f3c 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/ReadabilityResources.resx +++ b/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/ReadabilityResources.resx @@ -301,7 +301,7 @@ The parameters to a C# method or indexer call or declaration are not all on the same line or each on a separate line. - The parameters must all be placed on the same line or alternatively, each parameter must be placed on its own line. + The parameters must all be placed on the same line or each parameter must be placed on its own line. Parameters must be on same line or separate lines