Skip to content

Commit c75ee35

Browse files
sharwellsharwell
committed
Update SA1408 for relational and logical patterns
1 parent 28b7f55 commit c75ee35

File tree

6 files changed

+215
-10
lines changed

6 files changed

+215
-10
lines changed

‎StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/MaintainabilityRules/SA1407SA1408CodeFixProvider.cs‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ namespace StyleCop.Analyzers.MaintainabilityRules
1414
using Microsoft.CodeAnalysis.CSharp;
1515
using Microsoft.CodeAnalysis.CSharp.Syntax;
1616
using StyleCop.Analyzers.Helpers;
17+
using StyleCop.Analyzers.Lightup;
1718

1819
/// <summary>
1920
/// Implements a code fix for <see cref="SA1407ArithmeticExpressionsMustDeclarePrecedence"/> and <see cref="SA1408ConditionalExpressionsMustDeclarePrecedence"/>.
@@ -59,6 +60,15 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
5960
nameof(SA1407SA1408CodeFixProvider)),
6061
diagnostic);
6162
}
63+
else if (BinaryPatternSyntaxWrapper.IsInstance(node))
64+
{
65+
context.RegisterCodeFix(
66+
CodeAction.Create(
67+
MaintainabilityResources.SA1407SA1408CodeFix,
68+
cancellationToken => GetTransformedDocumentAsync(context.Document, root, (BinaryPatternSyntaxWrapper)node),
69+
nameof(SA1407SA1408CodeFixProvider)),
70+
diagnostic);
71+
}
6272
}
6373
}
6474

@@ -72,5 +82,17 @@ private static Task<Document> GetTransformedDocumentAsync(Document document, Syn
7282

7383
return Task.FromResult(document.WithSyntaxRoot(newSyntaxRoot));
7484
}
85+
86+
private static Task<Document> GetTransformedDocumentAsync(Document document, SyntaxNode root, BinaryPatternSyntaxWrapper syntax)
87+
{
88+
var newNode = (ParenthesizedPatternSyntaxWrapper)SyntaxFactoryEx.ParenthesizedPattern((PatternSyntaxWrapper)syntax.SyntaxNode.WithoutTrivia())
89+
.SyntaxNode
90+
.WithTriviaFrom(syntax)
91+
.WithoutFormatting();
92+
93+
var newSyntaxRoot = root.ReplaceNode(syntax, newNode);
94+
95+
return Task.FromResult(document.WithSyntaxRoot(newSyntaxRoot));
96+
}
7597
}
7698
}

‎StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/MaintainabilityRules/SA1407SA1408FixAllProvider.cs‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ namespace StyleCop.Analyzers.MaintainabilityRules
1313
using Microsoft.CodeAnalysis.CSharp;
1414
using Microsoft.CodeAnalysis.CSharp.Syntax;
1515
using StyleCop.Analyzers.Helpers;
16+
using StyleCop.Analyzers.Lightup;
1617

1718
internal sealed class SA1407SA1408FixAllProvider : DocumentBasedFixAllProvider
1819
{
@@ -44,16 +45,26 @@ protected override async Task<SyntaxNode> FixAllInDocumentAsync(FixAllContext fi
4445

4546
private static SyntaxNode AddParentheses(SyntaxNode node)
4647
{
47-
if (!(node is BinaryExpressionSyntax syntax))
48+
if (node is BinaryExpressionSyntax syntax)
4849
{
49-
return node;
50+
BinaryExpressionSyntax trimmedSyntax = syntax.WithoutTrivia();
51+
52+
return SyntaxFactory.ParenthesizedExpression(trimmedSyntax)
53+
.WithTriviaFrom(syntax)
54+
.WithoutFormatting();
5055
}
5156

52-
BinaryExpressionSyntax trimmedSyntax = syntax.WithoutTrivia();
57+
if (BinaryPatternSyntaxWrapper.IsInstance(node))
58+
{
59+
BinaryPatternSyntaxWrapper trimmedSyntax = (BinaryPatternSyntaxWrapper)node.WithoutTrivia();
60+
61+
return SyntaxFactoryEx.ParenthesizedPattern(trimmedSyntax)
62+
.SyntaxNode
63+
.WithTriviaFrom(node)
64+
.WithoutFormatting();
65+
}
5366

54-
return SyntaxFactory.ParenthesizedExpression(trimmedSyntax)
55-
.WithTriviaFrom(syntax)
56-
.WithoutFormatting();
67+
return node;
5768
}
5869
}
5970
}

‎StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp9/MaintainabilityRules/SA1408CSharp9UnitTests.cs‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,62 @@
33

44
namespace StyleCop.Analyzers.Test.CSharp9.MaintainabilityRules
55
{
6+
using System.Threading;
7+
using System.Threading.Tasks;
8+
using Microsoft.CodeAnalysis.Testing;
69
using StyleCop.Analyzers.Test.CSharp8.MaintainabilityRules;
10+
using Xunit;
11+
12+
using static StyleCop.Analyzers.Test.Verifiers.StyleCopCodeFixVerifier<
13+
StyleCop.Analyzers.MaintainabilityRules.SA1408ConditionalExpressionsMustDeclarePrecedence,
14+
StyleCop.Analyzers.MaintainabilityRules.SA1407SA1408CodeFixProvider>;
715

816
public partial class SA1408CSharp9UnitTests : SA1408CSharp8UnitTests
917
{
18+
[Fact]
19+
[WorkItem(3968, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3968")]
20+
public async Task TestLogicalPatternsDeclarePrecedenceAsync()
21+
{
22+
const string testCode = @"
23+
class C
24+
{
25+
bool M(int value) => value is {|#0:> 0 and < 5|} or 10;
26+
}";
27+
const string fixedCode = @"
28+
class C
29+
{
30+
bool M(int value) => value is (> 0 and < 5) or 10;
31+
}";
32+
33+
DiagnosticResult expected = Diagnostic().WithLocation(0);
34+
35+
await VerifyCSharpFixAsync(testCode, expected, fixedCode, CancellationToken.None).ConfigureAwait(false);
36+
}
37+
38+
[Fact]
39+
[WorkItem(3968, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3968")]
40+
public async Task TestPatternAndWithLogicalOrIsIgnoredAsync()
41+
{
42+
const string testCode = @"
43+
class C
44+
{
45+
bool M(int value, bool flag) => flag || value is > 0 and < 5;
46+
}";
47+
48+
await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
49+
}
50+
51+
[Fact]
52+
[WorkItem(3968, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3968")]
53+
public async Task TestPatternOrWithLogicalAndIsIgnoredAsync()
54+
{
55+
const string testCode = @"
56+
class C
57+
{
58+
bool M(int value, bool flag) => flag && value is > 0 or < 5;
59+
}";
60+
61+
await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
62+
}
1063
}
1164
}

‎StyleCop.Analyzers/StyleCop.Analyzers/Lightup/SyntaxFactoryEx.cs‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ internal static class SyntaxFactoryEx
1919
private static readonly Func<SyntaxToken, SeparatedSyntaxListWrapper<SubpatternSyntaxWrapper>, SyntaxToken, CSharpSyntaxNode> PositionalPatternClauseAccessor2;
2020
private static readonly Func<SeparatedSyntaxListWrapper<SubpatternSyntaxWrapper>, CSharpSyntaxNode> PropertyPatternClauseAccessor1;
2121
private static readonly Func<SyntaxToken, SeparatedSyntaxListWrapper<SubpatternSyntaxWrapper>, SyntaxToken, CSharpSyntaxNode> PropertyPatternClauseAccessor2;
22+
private static readonly Func<CSharpSyntaxNode, CSharpSyntaxNode> ParenthesizedPatternAccessor1;
23+
private static readonly Func<SyntaxToken, CSharpSyntaxNode, SyntaxToken, CSharpSyntaxNode> ParenthesizedPatternAccessor3;
2224
private static readonly Func<TypeSyntax, CSharpSyntaxNode> TupleElementAccessor1;
2325
private static readonly Func<TypeSyntax, SyntaxToken, CSharpSyntaxNode> TupleElementAccessor2;
2426
private static readonly Func<SeparatedSyntaxList<ArgumentSyntax>, ExpressionSyntax> TupleExpressionAccessor1;
@@ -132,6 +134,55 @@ static SyntaxFactoryEx()
132134
PropertyPatternClauseAccessor2 = ThrowNotSupportedOnFallback<SyntaxToken, SeparatedSyntaxListWrapper<SubpatternSyntaxWrapper>, SyntaxToken, TypeSyntax>(nameof(SyntaxFactory), nameof(PropertyPatternClause));
133135
}
134136

137+
var parenthesizedPatternMethods = typeof(SyntaxFactory).GetTypeInfo().GetDeclaredMethods(nameof(ParenthesizedPattern));
138+
var parenthesizedPatternMethod = parenthesizedPatternMethods.FirstOrDefault(method => method.GetParameters().Length == 1 && method.GetParameters()[0].ParameterType == SyntaxWrapperHelper.GetWrappedType(typeof(PatternSyntaxWrapper)));
139+
if (parenthesizedPatternMethod is object)
140+
{
141+
var patternParameter = Expression.Parameter(typeof(CSharpSyntaxNode), "pattern");
142+
Expression<Func<CSharpSyntaxNode, CSharpSyntaxNode>> expression =
143+
Expression.Lambda<Func<CSharpSyntaxNode, CSharpSyntaxNode>>(
144+
Expression.Call(
145+
parenthesizedPatternMethod,
146+
Expression.Convert(
147+
patternParameter,
148+
parenthesizedPatternMethod.GetParameters()[0].ParameterType)),
149+
patternParameter);
150+
ParenthesizedPatternAccessor1 = expression.Compile();
151+
}
152+
else
153+
{
154+
ParenthesizedPatternAccessor1 = ThrowNotSupportedOnFallback<CSharpSyntaxNode, CSharpSyntaxNode>(nameof(SyntaxFactory), nameof(ParenthesizedPattern));
155+
}
156+
157+
parenthesizedPatternMethod = parenthesizedPatternMethods.FirstOrDefault(method => method.GetParameters().Length == 3
158+
&& method.GetParameters()[0].ParameterType == typeof(SyntaxToken)
159+
&& method.GetParameters()[1].ParameterType == typeof(SeparatedSyntaxList<>).MakeGenericType(SyntaxWrapperHelper.GetWrappedType(typeof(SubpatternSyntaxWrapper)))
160+
&& method.GetParameters()[2].ParameterType == typeof(SyntaxToken));
161+
if (parenthesizedPatternMethod is object)
162+
{
163+
var openParenTokenParameter = Expression.Parameter(typeof(SyntaxToken), "openParenToken");
164+
var patternParameter = Expression.Parameter(typeof(CSharpSyntaxNode), "pattern");
165+
var closeParenTokenParameter = Expression.Parameter(typeof(SyntaxToken), "closeParenToken");
166+
167+
Expression<Func<SyntaxToken, CSharpSyntaxNode, SyntaxToken, CSharpSyntaxNode>> expression =
168+
Expression.Lambda<Func<SyntaxToken, CSharpSyntaxNode, SyntaxToken, CSharpSyntaxNode>>(
169+
Expression.Call(
170+
parenthesizedPatternMethod,
171+
openParenTokenParameter,
172+
Expression.Convert(
173+
patternParameter,
174+
parenthesizedPatternMethod.GetParameters()[1].ParameterType),
175+
closeParenTokenParameter),
176+
openParenTokenParameter,
177+
patternParameter,
178+
closeParenTokenParameter);
179+
ParenthesizedPatternAccessor3 = expression.Compile();
180+
}
181+
else
182+
{
183+
ParenthesizedPatternAccessor3 = ThrowNotSupportedOnFallback<SyntaxToken, CSharpSyntaxNode, SyntaxToken, TypeSyntax>(nameof(SyntaxFactory), nameof(ParenthesizedPattern));
184+
}
185+
135186
var tupleElementMethods = typeof(SyntaxFactory).GetTypeInfo().GetDeclaredMethods(nameof(TupleElement));
136187
var tupleElementMethod = tupleElementMethods.FirstOrDefault(method => method.GetParameters().Length == 1 && method.GetParameters()[0].ParameterType == typeof(TypeSyntax));
137188
if (tupleElementMethod is object)
@@ -276,6 +327,16 @@ public static PropertyPatternClauseSyntaxWrapper PropertyPatternClause(SyntaxTok
276327
return (PropertyPatternClauseSyntaxWrapper)PropertyPatternClauseAccessor2(openBraceToken, subpatterns, closeBraceToken);
277328
}
278329

330+
public static ParenthesizedPatternSyntaxWrapper ParenthesizedPattern(PatternSyntaxWrapper pattern)
331+
{
332+
return (ParenthesizedPatternSyntaxWrapper)ParenthesizedPatternAccessor1(pattern);
333+
}
334+
335+
public static ParenthesizedPatternSyntaxWrapper ParenthesizedPattern(SyntaxToken openParenToken, PatternSyntaxWrapper pattern, SyntaxToken closeParenToken)
336+
{
337+
return (ParenthesizedPatternSyntaxWrapper)ParenthesizedPatternAccessor3(openParenToken, pattern, closeParenToken);
338+
}
339+
279340
public static TupleElementSyntaxWrapper TupleElement(TypeSyntax type)
280341
{
281342
return (TupleElementSyntaxWrapper)TupleElementAccessor1(type);

‎StyleCop.Analyzers/StyleCop.Analyzers/MaintainabilityRules/SA1408ConditionalExpressionsMustDeclarePrecedence.cs‎

Lines changed: 50 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ namespace StyleCop.Analyzers.MaintainabilityRules
1111
using Microsoft.CodeAnalysis.CSharp;
1212
using Microsoft.CodeAnalysis.CSharp.Syntax;
1313
using Microsoft.CodeAnalysis.Diagnostics;
14+
using StyleCop.Analyzers.Lightup;
1415

1516
/// <summary>
1617
/// A C# statement contains a complex conditional expression which omits parenthesis around operators.
@@ -71,7 +72,11 @@ internal class SA1408ConditionalExpressionsMustDeclarePrecedence : DiagnosticAna
7172
private static readonly ImmutableArray<SyntaxKind> HandledBinaryExpressionKinds =
7273
ImmutableArray.Create(SyntaxKind.LogicalAndExpression, SyntaxKind.LogicalOrExpression);
7374

75+
private static readonly ImmutableArray<SyntaxKind> HandledBinaryPatternKinds =
76+
ImmutableArray.Create(SyntaxKindEx.AndPattern, SyntaxKindEx.OrPattern);
77+
7478
private static readonly Action<SyntaxNodeAnalysisContext> BinaryExpressionAction = HandleBinaryExpression;
79+
private static readonly Action<SyntaxNodeAnalysisContext> BinaryPatternAction = HandleBinaryPattern;
7580

7681
/// <inheritdoc/>
7782
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } =
@@ -84,6 +89,7 @@ public override void Initialize(AnalysisContext context)
8489
context.EnableConcurrentExecution();
8590

8691
context.RegisterSyntaxNodeAction(BinaryExpressionAction, HandledBinaryExpressionKinds);
92+
context.RegisterSyntaxNodeAction(BinaryPatternAction, HandledBinaryPatternKinds);
8793
}
8894

8995
private static void HandleBinaryExpression(SyntaxNodeAnalysisContext context)
@@ -93,7 +99,7 @@ private static void HandleBinaryExpression(SyntaxNodeAnalysisContext context)
9399
if (binSyntax.Left is BinaryExpressionSyntax left)
94100
{
95101
// Check if the operations are of the same kind
96-
if (left.OperatorToken.IsKind(SyntaxKind.AmpersandAmpersandToken) || left.OperatorToken.IsKind(SyntaxKind.BarBarToken))
102+
if (IsLogicalOperator(left.OperatorToken))
97103
{
98104
if (!IsSameFamily(binSyntax.OperatorToken, left.OperatorToken))
99105
{
@@ -105,7 +111,7 @@ private static void HandleBinaryExpression(SyntaxNodeAnalysisContext context)
105111
if (binSyntax.Right is BinaryExpressionSyntax right)
106112
{
107113
// Check if the operations are of the same kind
108-
if (right.OperatorToken.IsKind(SyntaxKind.AmpersandAmpersandToken) || right.OperatorToken.IsKind(SyntaxKind.BarBarToken))
114+
if (IsLogicalOperator(right.OperatorToken))
109115
{
110116
if (!IsSameFamily(binSyntax.OperatorToken, right.OperatorToken))
111117
{
@@ -115,10 +121,50 @@ private static void HandleBinaryExpression(SyntaxNodeAnalysisContext context)
115121
}
116122
}
117123

124+
private static void HandleBinaryPattern(SyntaxNodeAnalysisContext context)
125+
{
126+
var binaryPattern = (BinaryPatternSyntaxWrapper)context.Node;
127+
128+
if (BinaryPatternSyntaxWrapper.IsInstance(binaryPattern.Left.SyntaxNode))
129+
{
130+
var left = (BinaryPatternSyntaxWrapper)binaryPattern.Left;
131+
if (IsLogicalOperator(left.OperatorToken) && !IsSameFamily(binaryPattern.OperatorToken, left.OperatorToken))
132+
{
133+
context.ReportDiagnostic(Diagnostic.Create(Descriptor, left.SyntaxNode.GetLocation()));
134+
}
135+
}
136+
137+
if (BinaryPatternSyntaxWrapper.IsInstance(binaryPattern.Right.SyntaxNode))
138+
{
139+
var right = (BinaryPatternSyntaxWrapper)binaryPattern.Right;
140+
if (IsLogicalOperator(right.OperatorToken) && !IsSameFamily(binaryPattern.OperatorToken, right.OperatorToken))
141+
{
142+
context.ReportDiagnostic(Diagnostic.Create(Descriptor, right.SyntaxNode.GetLocation()));
143+
}
144+
}
145+
}
146+
147+
private static bool IsLogicalOperator(SyntaxToken operatorToken)
148+
{
149+
return IsAndOperator(operatorToken) || IsOrOperator(operatorToken);
150+
}
151+
118152
private static bool IsSameFamily(SyntaxToken operatorToken1, SyntaxToken operatorToken2)
119153
{
120-
return (operatorToken1.IsKind(SyntaxKind.AmpersandAmpersandToken) && operatorToken2.IsKind(SyntaxKind.AmpersandAmpersandToken))
121-
|| (operatorToken1.IsKind(SyntaxKind.BarBarToken) && operatorToken2.IsKind(SyntaxKind.BarBarToken));
154+
return (IsAndOperator(operatorToken1) && IsAndOperator(operatorToken2))
155+
|| (IsOrOperator(operatorToken1) && IsOrOperator(operatorToken2));
156+
}
157+
158+
private static bool IsAndOperator(SyntaxToken operatorToken)
159+
{
160+
return operatorToken.IsKind(SyntaxKind.AmpersandAmpersandToken)
161+
|| operatorToken.IsKind(SyntaxKindEx.AndKeyword);
162+
}
163+
164+
private static bool IsOrOperator(SyntaxToken operatorToken)
165+
{
166+
return operatorToken.IsKind(SyntaxKind.BarBarToken)
167+
|| operatorToken.IsKind(SyntaxKindEx.OrKeyword);
122168
}
123169
}
124170
}

‎documentation/SA1408.md‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,18 @@ if (x || (y && z && a) || b)
4747
}
4848
```
4949

50+
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:
51+
52+
```csharp
53+
if (value is > 0 and < 5 or 10)
54+
{
55+
}
56+
57+
if (value is (> 0 and < 5) or 10)
58+
{
59+
}
60+
```
61+
5062
Inserting parenthesis makes the code more obvious and easy to understand, and removes the need for the reader to make assumptions about the code.
5163

5264
## How to fix violations

0 commit comments

Comments
 (0)