diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/MaintainabilityRules/SA1407SA1408CodeFixProvider.cs b/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/MaintainabilityRules/SA1407SA1408CodeFixProvider.cs index f4dd7a710..1ed724315 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/MaintainabilityRules/SA1407SA1408CodeFixProvider.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/MaintainabilityRules/SA1407SA1408CodeFixProvider.cs @@ -14,6 +14,7 @@ namespace StyleCop.Analyzers.MaintainabilityRules using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; using StyleCop.Analyzers.Helpers; + using StyleCop.Analyzers.Lightup; /// /// Implements a code fix for and . @@ -59,6 +60,15 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) nameof(SA1407SA1408CodeFixProvider)), diagnostic); } + else if (BinaryPatternSyntaxWrapper.IsInstance(node)) + { + context.RegisterCodeFix( + CodeAction.Create( + MaintainabilityResources.SA1407SA1408CodeFix, + cancellationToken => GetTransformedDocumentAsync(context.Document, root, (BinaryPatternSyntaxWrapper)node), + nameof(SA1407SA1408CodeFixProvider)), + diagnostic); + } } } @@ -72,5 +82,17 @@ private static Task GetTransformedDocumentAsync(Document document, Syn return Task.FromResult(document.WithSyntaxRoot(newSyntaxRoot)); } + + private static Task GetTransformedDocumentAsync(Document document, SyntaxNode root, BinaryPatternSyntaxWrapper syntax) + { + var newNode = (ParenthesizedPatternSyntaxWrapper)SyntaxFactoryEx.ParenthesizedPattern((PatternSyntaxWrapper)syntax.SyntaxNode.WithoutTrivia()) + .SyntaxNode + .WithTriviaFrom(syntax) + .WithoutFormatting(); + + var newSyntaxRoot = root.ReplaceNode(syntax, newNode); + + return Task.FromResult(document.WithSyntaxRoot(newSyntaxRoot)); + } } } diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/MaintainabilityRules/SA1407SA1408FixAllProvider.cs b/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/MaintainabilityRules/SA1407SA1408FixAllProvider.cs index 79a549386..861344d85 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/MaintainabilityRules/SA1407SA1408FixAllProvider.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/MaintainabilityRules/SA1407SA1408FixAllProvider.cs @@ -13,6 +13,7 @@ namespace StyleCop.Analyzers.MaintainabilityRules using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; using StyleCop.Analyzers.Helpers; + using StyleCop.Analyzers.Lightup; internal sealed class SA1407SA1408FixAllProvider : DocumentBasedFixAllProvider { @@ -44,16 +45,26 @@ protected override async Task FixAllInDocumentAsync(FixAllContext fi private static SyntaxNode AddParentheses(SyntaxNode node) { - if (!(node is BinaryExpressionSyntax syntax)) + if (node is BinaryExpressionSyntax syntax) { - return node; + BinaryExpressionSyntax trimmedSyntax = syntax.WithoutTrivia(); + + return SyntaxFactory.ParenthesizedExpression(trimmedSyntax) + .WithTriviaFrom(syntax) + .WithoutFormatting(); } - BinaryExpressionSyntax trimmedSyntax = syntax.WithoutTrivia(); + if (BinaryPatternSyntaxWrapper.IsInstance(node)) + { + BinaryPatternSyntaxWrapper trimmedSyntax = (BinaryPatternSyntaxWrapper)node.WithoutTrivia(); + + return SyntaxFactoryEx.ParenthesizedPattern(trimmedSyntax) + .SyntaxNode + .WithTriviaFrom(node) + .WithoutFormatting(); + } - return SyntaxFactory.ParenthesizedExpression(trimmedSyntax) - .WithTriviaFrom(syntax) - .WithoutFormatting(); + return node; } } } diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp9/MaintainabilityRules/SA1119CSharp9UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp9/MaintainabilityRules/SA1119CSharp9UnitTests.cs index 8da24321e..fd23ce2cd 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp9/MaintainabilityRules/SA1119CSharp9UnitTests.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp9/MaintainabilityRules/SA1119CSharp9UnitTests.cs @@ -183,5 +183,68 @@ public object GetValue(bool flag) FixedCode = fixedCode, }.RunAsync(CancellationToken.None).ConfigureAwait(false); } + + [Fact] + [WorkItem(3968, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3968")] + public async Task TestOuterParenthesesAroundParenthesizedPatternAreRemovedAsync() + { + const string testCode = @" +class C +{ + void M(int value) + { + if ({|#0:{|#1:(|}(value is (> 0 and < 5)){|#2:)|}|}) + { + } + } +}"; + + const string fixedCode = @" +class C +{ + void M(int value) + { + if (value is (> 0 and < 5)) + { + } + } +}"; + + await new CSharpTest() + { + NumberOfIncrementalIterations = 2, + NumberOfFixAllIterations = 2, + TestCode = testCode, + ExpectedDiagnostics = + { + Diagnostic(DiagnosticId).WithLocation(0), + Diagnostic(ParenthesesDiagnosticId).WithLocation(1), + Diagnostic(ParenthesesDiagnosticId).WithLocation(2), + }, + FixedCode = fixedCode, + }.RunAsync(CancellationToken.None).ConfigureAwait(false); + } + + /// + /// Verifies that parentheses required to clarify precedence within patterns are not removed. + /// + /// A representing the asynchronous unit test. + [Fact] + [WorkItem(3968, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3968")] + public async Task TestClarifyingPatternParenthesesAreNotRemovedAsync() + { + const string testCode = @" +class C +{ + void M(int value) + { + if (value is (> 0 and < 5) or 10) + { + } + } +}"; + + await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false); + } } } diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp9/MaintainabilityRules/SA1408CSharp9UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp9/MaintainabilityRules/SA1408CSharp9UnitTests.cs index 2a2cfc1b0..43764d3a4 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp9/MaintainabilityRules/SA1408CSharp9UnitTests.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp9/MaintainabilityRules/SA1408CSharp9UnitTests.cs @@ -3,9 +3,62 @@ namespace StyleCop.Analyzers.Test.CSharp9.MaintainabilityRules { + using System.Threading; + using System.Threading.Tasks; + using Microsoft.CodeAnalysis.Testing; using StyleCop.Analyzers.Test.CSharp8.MaintainabilityRules; + using Xunit; + + using static StyleCop.Analyzers.Test.Verifiers.StyleCopCodeFixVerifier< + StyleCop.Analyzers.MaintainabilityRules.SA1408ConditionalExpressionsMustDeclarePrecedence, + StyleCop.Analyzers.MaintainabilityRules.SA1407SA1408CodeFixProvider>; public partial class SA1408CSharp9UnitTests : SA1408CSharp8UnitTests { + [Fact] + [WorkItem(3968, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3968")] + public async Task TestLogicalPatternsDeclarePrecedenceAsync() + { + const string testCode = @" +class C +{ + bool M(int value) => value is {|#0:> 0 and < 5|} or 10; +}"; + const string fixedCode = @" +class C +{ + bool M(int value) => value is (> 0 and < 5) or 10; +}"; + + DiagnosticResult expected = Diagnostic().WithLocation(0); + + await VerifyCSharpFixAsync(testCode, expected, fixedCode, CancellationToken.None).ConfigureAwait(false); + } + + [Fact] + [WorkItem(3968, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3968")] + public async Task TestPatternAndWithLogicalOrIsIgnoredAsync() + { + const string testCode = @" +class C +{ + bool M(int value, bool flag) => flag || value is > 0 and < 5; +}"; + + await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false); + } + + [Fact] + [WorkItem(3968, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3968")] + public async Task TestPatternOrWithLogicalAndIsIgnoredAsync() + { + const string testCode = @" +class C +{ + bool M(int value, bool flag) => flag && value is > 0 or < 5; +}"; + + await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false); + } } } diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp9/SpacingRules/SA1000CSharp9UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp9/SpacingRules/SA1000CSharp9UnitTests.cs index 9e4ec26ee..0a1076a6b 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp9/SpacingRules/SA1000CSharp9UnitTests.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp9/SpacingRules/SA1000CSharp9UnitTests.cs @@ -7,7 +7,6 @@ namespace StyleCop.Analyzers.Test.CSharp9.SpacingRules using Microsoft.CodeAnalysis.Testing; using StyleCop.Analyzers.Test.CSharp8.SpacingRules; using Xunit; - using static StyleCop.Analyzers.Test.Verifiers.StyleCopCodeFixVerifier< StyleCop.Analyzers.SpacingRules.SA1000KeywordsMustBeSpacedCorrectly, StyleCop.Analyzers.SpacingRules.TokenSpacingCodeFixProvider>; @@ -31,47 +30,58 @@ public async Task TestTargetTypedNewInConditionalExpressionAsync() await this.TestKeywordStatementAsync(statement, DiagnosticResult.EmptyDiagnosticResults, statement).ConfigureAwait(false); } - [Fact] + [Theory] [WorkItem(3508, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3508")] - public async Task TestIsBeforeRelationalPatternAsync() + [InlineData("<")] + [InlineData("<=")] + [InlineData(">")] + [InlineData(">=")] + public async Task TestIsBeforeRelationalPatternAsync(string @operator) { - var statementWithoutSpace = "_ = 1 {|#0:is|}>1;"; - var statementWithSpace = "_ = 1 is >1;"; + var statementWithoutSpace = $"_ = 1 {{|#0:is|}}{@operator}1;"; + var statementWithSpace = $"_ = 1 is {@operator}1;"; var expected = Diagnostic().WithArguments("is", string.Empty, "followed").WithLocation(0); await this.TestKeywordStatementAsync(statementWithoutSpace, expected, statementWithSpace).ConfigureAwait(false); } - [Fact] + [Theory] [WorkItem(3508, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3508")] - public async Task TestNotBeforeRelationalPatternAsync() + [InlineData("<")] + [InlineData("<=")] + [InlineData(">")] + [InlineData(">=")] + public async Task TestNotBeforeRelationalPatternAsync(string relationalOperator) { - var statementWithoutSpace = "_ = 1 is {|#0:not|}>1;"; - var statementWithSpace = "_ = 1 is not >1;"; + var statementWithoutSpace = $"_ = 1 is {{|#0:not|}}{relationalOperator}1;"; + var statementWithSpace = $"_ = 1 is not {relationalOperator}1;"; var expected = Diagnostic().WithArguments("not", string.Empty, "followed").WithLocation(0); await this.TestKeywordStatementAsync(statementWithoutSpace, expected, statementWithSpace).ConfigureAwait(false); } - [Fact] + [Theory] [WorkItem(3508, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3508")] - public async Task TestAndBeforeRelationalPatternAsync() + [CombinatorialData] + public async Task TestAndBeforeRelationalPatternAsync( + [CombinatorialValues("and", "or")] string logicalOperator, + [CombinatorialValues("<", "<=", ">", ">=")] string relationalOperator) { - var statementWithoutSpace = "_ = 1 is 1 {|#0:and|}>0;"; - var statementWithSpace = "_ = 1 is 1 and >0;"; + var statementWithoutSpace = $"_ = (int?)1 is not null {{|#0:{logicalOperator}|}}{relationalOperator}1;"; + var statementWithSpace = $"_ = (int?)1 is not null {logicalOperator} {relationalOperator}1;"; - var expected = Diagnostic().WithArguments("and", string.Empty, "followed").WithLocation(0); + var expected = Diagnostic().WithArguments(logicalOperator, string.Empty, "followed").WithLocation(0); await this.TestKeywordStatementAsync(statementWithoutSpace, expected, statementWithSpace).ConfigureAwait(false); } [Fact] - [WorkItem(3508, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3508")] - public async Task TestOrBeforeRelationalPatternAsync() + [WorkItem(3968, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3968")] + public async Task TestNotBeforeConstantPatternMissingSpaceAsync() { - var statementWithoutSpace = "_ = 1 is 1 {|#0:or|}>1;"; - var statementWithSpace = "_ = 1 is 1 or >1;"; + var statementWithoutSpace = "_ = new object() is {|#0:not|}(null);"; + var statementWithSpace = "_ = new object() is not (null);"; - var expected = Diagnostic().WithArguments("or", string.Empty, "followed").WithLocation(0); + var expected = Diagnostic().WithArguments("not", string.Empty, "followed").WithLocation(0); await this.TestKeywordStatementAsync(statementWithoutSpace, expected, statementWithSpace).ConfigureAwait(false); } } diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp9/SpacingRules/SA1003CSharp9UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp9/SpacingRules/SA1003CSharp9UnitTests.cs index 212a5b8b5..6a24915eb 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp9/SpacingRules/SA1003CSharp9UnitTests.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp9/SpacingRules/SA1003CSharp9UnitTests.cs @@ -46,5 +46,51 @@ void M(bool flag) await VerifyCSharpFixAsync(testCode, expected, fixedCode, CancellationToken.None).ConfigureAwait(false); } + + [Fact] + [WorkItem(3968, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3968")] + public async Task TestRelationalPatternsAreValidatedAsync() + { + const string testCode = @" +class C +{ + void M(int value) + { + _ = value is {|#0:>|}5; + _ = value is{|#1:<|} 5; + _ = value is ( < 5); // Validated by SA1008 + _ = value is (< 5); + _ = value is {|#2:<=|}5; + _ = value is {|#3:>=|}5; + _ = value is {|#4:>|} + 5; + } +}"; + + const string fixedCode = @" +class C +{ + void M(int value) + { + _ = value is > 5; + _ = value is < 5; + _ = value is ( < 5); // Validated by SA1008 + _ = value is (< 5); + _ = value is <= 5; + _ = value is >= 5; + _ = value is > 5; + } +}"; + + DiagnosticResult[] expected = + { + Diagnostic(DescriptorFollowedByWhitespace).WithLocation(0).WithArguments(">"), + Diagnostic(DescriptorPrecededByWhitespace).WithLocation(1).WithArguments("<"), + Diagnostic(DescriptorFollowedByWhitespace).WithLocation(2).WithArguments("<="), + Diagnostic(DescriptorFollowedByWhitespace).WithLocation(3).WithArguments(">="), + Diagnostic(DescriptorNotAtEndOfLine).WithLocation(4).WithArguments(">"), + }; + await VerifyCSharpFixAsync(testCode, expected, fixedCode, CancellationToken.None).ConfigureAwait(false); + } } } diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp9/SpacingRules/SA1008CSharp9UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp9/SpacingRules/SA1008CSharp9UnitTests.cs index e98818ab7..2b1c749db 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp9/SpacingRules/SA1008CSharp9UnitTests.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp9/SpacingRules/SA1008CSharp9UnitTests.cs @@ -110,5 +110,46 @@ public async Task TestDeconstructionInTopLevelProgramAsync(string prefix) FixedCode = fixedCode, }.RunAsync(CancellationToken.None).ConfigureAwait(false); } + + [Fact] + [WorkItem(3968, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3968")] + public async Task TestLogicalPatternsWithParenthesesAsync() + { + const string testCode = @" +class C +{ + void M(int value) + { + _ = value is not{|#0:(|}> 0); + _ = value is > 0 and{|#1:(|}< 5); + _ = value is > 0 and {|#2:(|} < 5); + _ = value is > 10 or{|#3:(|}< 5); + _ = value is > 10 or {|#4:(|} < 5); + } +}"; + + const string fixedCode = @" +class C +{ + void M(int value) + { + _ = value is not (> 0); + _ = value is > 0 and (< 5); + _ = value is > 0 and (< 5); + _ = value is > 10 or (< 5); + _ = value is > 10 or (< 5); + } +}"; + + DiagnosticResult[] expected = + { + Diagnostic(DescriptorPreceded).WithLocation(0), + Diagnostic(DescriptorPreceded).WithLocation(1), + Diagnostic(DescriptorNotFollowed).WithLocation(2), + Diagnostic(DescriptorPreceded).WithLocation(3), + Diagnostic(DescriptorNotFollowed).WithLocation(4), + }; + await VerifyCSharpFixAsync(testCode, expected, fixedCode, CancellationToken.None).ConfigureAwait(false); + } } } diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/Lightup/SyntaxFactoryEx.cs b/StyleCop.Analyzers/StyleCop.Analyzers/Lightup/SyntaxFactoryEx.cs index fbcfcfa90..dcc691ba6 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/Lightup/SyntaxFactoryEx.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers/Lightup/SyntaxFactoryEx.cs @@ -19,6 +19,8 @@ internal static class SyntaxFactoryEx private static readonly Func, SyntaxToken, CSharpSyntaxNode> PositionalPatternClauseAccessor2; private static readonly Func, CSharpSyntaxNode> PropertyPatternClauseAccessor1; private static readonly Func, SyntaxToken, CSharpSyntaxNode> PropertyPatternClauseAccessor2; + private static readonly Func ParenthesizedPatternAccessor1; + private static readonly Func ParenthesizedPatternAccessor3; private static readonly Func TupleElementAccessor1; private static readonly Func TupleElementAccessor2; private static readonly Func, ExpressionSyntax> TupleExpressionAccessor1; @@ -132,6 +134,55 @@ static SyntaxFactoryEx() PropertyPatternClauseAccessor2 = ThrowNotSupportedOnFallback, SyntaxToken, TypeSyntax>(nameof(SyntaxFactory), nameof(PropertyPatternClause)); } + var parenthesizedPatternMethods = typeof(SyntaxFactory).GetTypeInfo().GetDeclaredMethods(nameof(ParenthesizedPattern)); + var parenthesizedPatternMethod = parenthesizedPatternMethods.FirstOrDefault(method => method.GetParameters().Length == 1 && method.GetParameters()[0].ParameterType == SyntaxWrapperHelper.GetWrappedType(typeof(PatternSyntaxWrapper))); + if (parenthesizedPatternMethod is object) + { + var patternParameter = Expression.Parameter(typeof(CSharpSyntaxNode), "pattern"); + Expression> expression = + Expression.Lambda>( + Expression.Call( + parenthesizedPatternMethod, + Expression.Convert( + patternParameter, + parenthesizedPatternMethod.GetParameters()[0].ParameterType)), + patternParameter); + ParenthesizedPatternAccessor1 = expression.Compile(); + } + else + { + ParenthesizedPatternAccessor1 = ThrowNotSupportedOnFallback(nameof(SyntaxFactory), nameof(ParenthesizedPattern)); + } + + parenthesizedPatternMethod = parenthesizedPatternMethods.FirstOrDefault(method => method.GetParameters().Length == 3 + && method.GetParameters()[0].ParameterType == typeof(SyntaxToken) + && method.GetParameters()[1].ParameterType == typeof(SeparatedSyntaxList<>).MakeGenericType(SyntaxWrapperHelper.GetWrappedType(typeof(SubpatternSyntaxWrapper))) + && method.GetParameters()[2].ParameterType == typeof(SyntaxToken)); + if (parenthesizedPatternMethod is object) + { + var openParenTokenParameter = Expression.Parameter(typeof(SyntaxToken), "openParenToken"); + var patternParameter = Expression.Parameter(typeof(CSharpSyntaxNode), "pattern"); + var closeParenTokenParameter = Expression.Parameter(typeof(SyntaxToken), "closeParenToken"); + + Expression> expression = + Expression.Lambda>( + Expression.Call( + parenthesizedPatternMethod, + openParenTokenParameter, + Expression.Convert( + patternParameter, + parenthesizedPatternMethod.GetParameters()[1].ParameterType), + closeParenTokenParameter), + openParenTokenParameter, + patternParameter, + closeParenTokenParameter); + ParenthesizedPatternAccessor3 = expression.Compile(); + } + else + { + ParenthesizedPatternAccessor3 = ThrowNotSupportedOnFallback(nameof(SyntaxFactory), nameof(ParenthesizedPattern)); + } + var tupleElementMethods = typeof(SyntaxFactory).GetTypeInfo().GetDeclaredMethods(nameof(TupleElement)); var tupleElementMethod = tupleElementMethods.FirstOrDefault(method => method.GetParameters().Length == 1 && method.GetParameters()[0].ParameterType == typeof(TypeSyntax)); if (tupleElementMethod is object) @@ -276,6 +327,16 @@ public static PropertyPatternClauseSyntaxWrapper PropertyPatternClause(SyntaxTok return (PropertyPatternClauseSyntaxWrapper)PropertyPatternClauseAccessor2(openBraceToken, subpatterns, closeBraceToken); } + public static ParenthesizedPatternSyntaxWrapper ParenthesizedPattern(PatternSyntaxWrapper pattern) + { + return (ParenthesizedPatternSyntaxWrapper)ParenthesizedPatternAccessor1(pattern); + } + + public static ParenthesizedPatternSyntaxWrapper ParenthesizedPattern(SyntaxToken openParenToken, PatternSyntaxWrapper pattern, SyntaxToken closeParenToken) + { + return (ParenthesizedPatternSyntaxWrapper)ParenthesizedPatternAccessor3(openParenToken, pattern, closeParenToken); + } + public static TupleElementSyntaxWrapper TupleElement(TypeSyntax type) { return (TupleElementSyntaxWrapper)TupleElementAccessor1(type); diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/MaintainabilityRules/SA1408ConditionalExpressionsMustDeclarePrecedence.cs b/StyleCop.Analyzers/StyleCop.Analyzers/MaintainabilityRules/SA1408ConditionalExpressionsMustDeclarePrecedence.cs index e5b1dc7fc..5c1a98976 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/MaintainabilityRules/SA1408ConditionalExpressionsMustDeclarePrecedence.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers/MaintainabilityRules/SA1408ConditionalExpressionsMustDeclarePrecedence.cs @@ -11,6 +11,7 @@ namespace StyleCop.Analyzers.MaintainabilityRules using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Diagnostics; + using StyleCop.Analyzers.Lightup; /// /// A C# statement contains a complex conditional expression which omits parenthesis around operators. @@ -71,7 +72,11 @@ internal class SA1408ConditionalExpressionsMustDeclarePrecedence : DiagnosticAna private static readonly ImmutableArray HandledBinaryExpressionKinds = ImmutableArray.Create(SyntaxKind.LogicalAndExpression, SyntaxKind.LogicalOrExpression); + private static readonly ImmutableArray HandledBinaryPatternKinds = + ImmutableArray.Create(SyntaxKindEx.AndPattern, SyntaxKindEx.OrPattern); + private static readonly Action BinaryExpressionAction = HandleBinaryExpression; + private static readonly Action BinaryPatternAction = HandleBinaryPattern; /// public override ImmutableArray SupportedDiagnostics { get; } = @@ -84,6 +89,7 @@ public override void Initialize(AnalysisContext context) context.EnableConcurrentExecution(); context.RegisterSyntaxNodeAction(BinaryExpressionAction, HandledBinaryExpressionKinds); + context.RegisterSyntaxNodeAction(BinaryPatternAction, HandledBinaryPatternKinds); } private static void HandleBinaryExpression(SyntaxNodeAnalysisContext context) @@ -93,7 +99,7 @@ private static void HandleBinaryExpression(SyntaxNodeAnalysisContext context) if (binSyntax.Left is BinaryExpressionSyntax left) { // Check if the operations are of the same kind - if (left.OperatorToken.IsKind(SyntaxKind.AmpersandAmpersandToken) || left.OperatorToken.IsKind(SyntaxKind.BarBarToken)) + if (IsLogicalOperator(left.OperatorToken)) { if (!IsSameFamily(binSyntax.OperatorToken, left.OperatorToken)) { @@ -105,7 +111,7 @@ private static void HandleBinaryExpression(SyntaxNodeAnalysisContext context) if (binSyntax.Right is BinaryExpressionSyntax right) { // Check if the operations are of the same kind - if (right.OperatorToken.IsKind(SyntaxKind.AmpersandAmpersandToken) || right.OperatorToken.IsKind(SyntaxKind.BarBarToken)) + if (IsLogicalOperator(right.OperatorToken)) { if (!IsSameFamily(binSyntax.OperatorToken, right.OperatorToken)) { @@ -115,10 +121,50 @@ private static void HandleBinaryExpression(SyntaxNodeAnalysisContext context) } } + private static void HandleBinaryPattern(SyntaxNodeAnalysisContext context) + { + var binaryPattern = (BinaryPatternSyntaxWrapper)context.Node; + + if (BinaryPatternSyntaxWrapper.IsInstance(binaryPattern.Left.SyntaxNode)) + { + var left = (BinaryPatternSyntaxWrapper)binaryPattern.Left; + if (IsLogicalOperator(left.OperatorToken) && !IsSameFamily(binaryPattern.OperatorToken, left.OperatorToken)) + { + context.ReportDiagnostic(Diagnostic.Create(Descriptor, left.SyntaxNode.GetLocation())); + } + } + + if (BinaryPatternSyntaxWrapper.IsInstance(binaryPattern.Right.SyntaxNode)) + { + var right = (BinaryPatternSyntaxWrapper)binaryPattern.Right; + if (IsLogicalOperator(right.OperatorToken) && !IsSameFamily(binaryPattern.OperatorToken, right.OperatorToken)) + { + context.ReportDiagnostic(Diagnostic.Create(Descriptor, right.SyntaxNode.GetLocation())); + } + } + } + + private static bool IsLogicalOperator(SyntaxToken operatorToken) + { + return IsAndOperator(operatorToken) || IsOrOperator(operatorToken); + } + private static bool IsSameFamily(SyntaxToken operatorToken1, SyntaxToken operatorToken2) { - return (operatorToken1.IsKind(SyntaxKind.AmpersandAmpersandToken) && operatorToken2.IsKind(SyntaxKind.AmpersandAmpersandToken)) - || (operatorToken1.IsKind(SyntaxKind.BarBarToken) && operatorToken2.IsKind(SyntaxKind.BarBarToken)); + return (IsAndOperator(operatorToken1) && IsAndOperator(operatorToken2)) + || (IsOrOperator(operatorToken1) && IsOrOperator(operatorToken2)); + } + + private static bool IsAndOperator(SyntaxToken operatorToken) + { + return operatorToken.IsKind(SyntaxKind.AmpersandAmpersandToken) + || operatorToken.IsKind(SyntaxKindEx.AndKeyword); + } + + private static bool IsOrOperator(SyntaxToken operatorToken) + { + return operatorToken.IsKind(SyntaxKind.BarBarToken) + || operatorToken.IsKind(SyntaxKindEx.OrKeyword); } } } diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/SpacingRules/SA1003SymbolsMustBeSpacedCorrectly.cs b/StyleCop.Analyzers/StyleCop.Analyzers/SpacingRules/SA1003SymbolsMustBeSpacedCorrectly.cs index 102957e39..4c9e4ded3 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/SpacingRules/SA1003SymbolsMustBeSpacedCorrectly.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers/SpacingRules/SA1003SymbolsMustBeSpacedCorrectly.cs @@ -140,6 +140,7 @@ internal class SA1003SymbolsMustBeSpacedCorrectly : DiagnosticAnalyzer private static readonly Action LambdaExpressionAction = HandleLambdaExpression; private static readonly Action ArrowExpressionClauseAction = HandleArrowExpressionClause; private static readonly Action RangeExpressionAction = HandleRangeExpression; + private static readonly Action RelationalPatternAction = HandleRelationalPattern; private static readonly Action SwitchExpressionArmAction = HandleSwitchExpressionArm; /// @@ -218,6 +219,7 @@ public override void Initialize(AnalysisContext context) context.RegisterSyntaxNodeAction(LambdaExpressionAction, SyntaxKinds.LambdaExpression); context.RegisterSyntaxNodeAction(ArrowExpressionClauseAction, SyntaxKind.ArrowExpressionClause); context.RegisterSyntaxNodeAction(RangeExpressionAction, SyntaxKindEx.RangeExpression); + context.RegisterSyntaxNodeAction(RelationalPatternAction, SyntaxKindEx.RelationalPattern); context.RegisterSyntaxNodeAction(SwitchExpressionArmAction, SyntaxKindEx.SwitchExpressionArm); } @@ -256,11 +258,6 @@ private static void HandleBinaryExpression(SyntaxNodeAnalysisContext context) private static void HandleRangeExpression(SyntaxNodeAnalysisContext context) { - if (!RangeExpressionSyntaxWrapper.IsInstance(context.Node)) - { - return; - } - var rangeExpression = (RangeExpressionSyntaxWrapper)context.Node; var hasLeftOperand = rangeExpression.LeftOperand != null; var hasRightOperand = rangeExpression.RightOperand != null; @@ -346,6 +343,22 @@ private static void HandlePrefixUnaryExpression(SyntaxNodeAnalysisContext contex } } + private static void HandleRelationalPattern(SyntaxNodeAnalysisContext context) + { + var relationalPattern = (RelationalPatternSyntaxWrapper)context.Node; + var operatorToken = relationalPattern.OperatorToken; + var precedingToken = operatorToken.GetPreviousToken(); + + if (precedingToken.IsKind(SyntaxKind.OpenParenToken)) + { + // Spacing next to the opening parenthesis is handled by SA1008; only enforce trailing whitespace here. + CheckTokenTrailingWhitespace(context, operatorToken, allowAtEndOfLine: false, withTrailingWhitespace: true); + return; + } + + CheckToken(context, operatorToken, withLeadingWhitespace: true, allowAtEndOfLine: false, withTrailingWhitespace: true); + } + private static void HandlePostfixUnaryExpression(SyntaxNodeAnalysisContext context) { var unaryExpression = (PostfixUnaryExpressionSyntax)context.Node; diff --git a/documentation/SA1003.md b/documentation/SA1003.md index 606de8bdb..12c32a6b8 100644 --- a/documentation/SA1003.md +++ b/documentation/SA1003.md @@ -38,6 +38,11 @@ rules apply: each arrow should have a single space on both sides. var description = value switch { 0 => "zero", _ => "nonzero" }; ``` +Relational operators inside pattern matching expressions follow the same rules when they appear after keywords such as +`is`. For example, in a relational pattern the `>` in `value is > 5` must be surrounded by spaces: `value is > 5`. When +a relational pattern is wrapped in parentheses, it should still avoid spaces immediately after `(` or before `)`, +consistent with the parenthesis rules described in SA1008. + In contrast, unary operators should be preceded by a single space, but should never be followed by any space. For example: ```csharp diff --git a/documentation/SA1408.md b/documentation/SA1408.md index 13fcb83a6..0b5439406 100644 --- a/documentation/SA1408.md +++ b/documentation/SA1408.md @@ -47,6 +47,18 @@ if (x || (y && z && a) || b) } ``` +Starting with pattern combinators introduced in C# 9, `and` and `or` patterns are treated the same way as `&&` and `||`. Mixing them without parentheses will produce a diagnostic, for example: + +```csharp +if (value is > 0 and < 5 or 10) +{ +} + +if (value is (> 0 and < 5) or 10) +{ +} +``` + Inserting parenthesis makes the code more obvious and easy to understand, and removes the need for the reader to make assumptions about the code. ## How to fix violations