Skip to content

Commit 7033f3a

Browse files
authored
Merge pull request #4058 from sharwell/relational-logical-patterns
Support relational and logical patterns from C# 9
2 parents e6480ae + 3e180ad commit 7033f3a

File tree

12 files changed

+417
-34
lines changed

12 files changed

+417
-34
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/SA1119CSharp9UnitTests.cs

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,5 +183,68 @@ public object GetValue(bool flag)
183183
FixedCode = fixedCode,
184184
}.RunAsync(CancellationToken.None).ConfigureAwait(false);
185185
}
186+
187+
[Fact]
188+
[WorkItem(3968, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3968")]
189+
public async Task TestOuterParenthesesAroundParenthesizedPatternAreRemovedAsync()
190+
{
191+
const string testCode = @"
192+
class C
193+
{
194+
void M(int value)
195+
{
196+
if ({|#0:{|#1:(|}(value is (> 0 and < 5)){|#2:)|}|})
197+
{
198+
}
199+
}
200+
}";
201+
202+
const string fixedCode = @"
203+
class C
204+
{
205+
void M(int value)
206+
{
207+
if (value is (> 0 and < 5))
208+
{
209+
}
210+
}
211+
}";
212+
213+
await new CSharpTest()
214+
{
215+
NumberOfIncrementalIterations = 2,
216+
NumberOfFixAllIterations = 2,
217+
TestCode = testCode,
218+
ExpectedDiagnostics =
219+
{
220+
Diagnostic(DiagnosticId).WithLocation(0),
221+
Diagnostic(ParenthesesDiagnosticId).WithLocation(1),
222+
Diagnostic(ParenthesesDiagnosticId).WithLocation(2),
223+
},
224+
FixedCode = fixedCode,
225+
}.RunAsync(CancellationToken.None).ConfigureAwait(false);
226+
}
227+
228+
/// <summary>
229+
/// Verifies that parentheses required to clarify precedence within patterns are not removed.
230+
/// </summary>
231+
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
232+
[Fact]
233+
[WorkItem(3968, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3968")]
234+
public async Task TestClarifyingPatternParenthesesAreNotRemovedAsync()
235+
{
236+
const string testCode = @"
237+
class C
238+
{
239+
void M(int value)
240+
{
241+
if (value is (> 0 and < 5) or 10)
242+
{
243+
}
244+
}
245+
}";
246+
247+
await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
248+
}
186249
}
187250
}

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.Test.CSharp9/SpacingRules/SA1000CSharp9UnitTests.cs

Lines changed: 29 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ namespace StyleCop.Analyzers.Test.CSharp9.SpacingRules
77
using Microsoft.CodeAnalysis.Testing;
88
using StyleCop.Analyzers.Test.CSharp8.SpacingRules;
99
using Xunit;
10-
1110
using static StyleCop.Analyzers.Test.Verifiers.StyleCopCodeFixVerifier<
1211
StyleCop.Analyzers.SpacingRules.SA1000KeywordsMustBeSpacedCorrectly,
1312
StyleCop.Analyzers.SpacingRules.TokenSpacingCodeFixProvider>;
@@ -31,47 +30,58 @@ public async Task TestTargetTypedNewInConditionalExpressionAsync()
3130
await this.TestKeywordStatementAsync(statement, DiagnosticResult.EmptyDiagnosticResults, statement).ConfigureAwait(false);
3231
}
3332

34-
[Fact]
33+
[Theory]
3534
[WorkItem(3508, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3508")]
36-
public async Task TestIsBeforeRelationalPatternAsync()
35+
[InlineData("<")]
36+
[InlineData("<=")]
37+
[InlineData(">")]
38+
[InlineData(">=")]
39+
public async Task TestIsBeforeRelationalPatternAsync(string @operator)
3740
{
38-
var statementWithoutSpace = "_ = 1 {|#0:is|}>1;";
39-
var statementWithSpace = "_ = 1 is >1;";
41+
var statementWithoutSpace = $"_ = 1 {{|#0:is|}}{@operator}1;";
42+
var statementWithSpace = $"_ = 1 is {@operator}1;";
4043

4144
var expected = Diagnostic().WithArguments("is", string.Empty, "followed").WithLocation(0);
4245
await this.TestKeywordStatementAsync(statementWithoutSpace, expected, statementWithSpace).ConfigureAwait(false);
4346
}
4447

45-
[Fact]
48+
[Theory]
4649
[WorkItem(3508, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3508")]
47-
public async Task TestNotBeforeRelationalPatternAsync()
50+
[InlineData("<")]
51+
[InlineData("<=")]
52+
[InlineData(">")]
53+
[InlineData(">=")]
54+
public async Task TestNotBeforeRelationalPatternAsync(string relationalOperator)
4855
{
49-
var statementWithoutSpace = "_ = 1 is {|#0:not|}>1;";
50-
var statementWithSpace = "_ = 1 is not >1;";
56+
var statementWithoutSpace = $"_ = 1 is {{|#0:not|}}{relationalOperator}1;";
57+
var statementWithSpace = $"_ = 1 is not {relationalOperator}1;";
5158

5259
var expected = Diagnostic().WithArguments("not", string.Empty, "followed").WithLocation(0);
5360
await this.TestKeywordStatementAsync(statementWithoutSpace, expected, statementWithSpace).ConfigureAwait(false);
5461
}
5562

56-
[Fact]
63+
[Theory]
5764
[WorkItem(3508, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3508")]
58-
public async Task TestAndBeforeRelationalPatternAsync()
65+
[CombinatorialData]
66+
public async Task TestAndBeforeRelationalPatternAsync(
67+
[CombinatorialValues("and", "or")] string logicalOperator,
68+
[CombinatorialValues("<", "<=", ">", ">=")] string relationalOperator)
5969
{
60-
var statementWithoutSpace = "_ = 1 is 1 {|#0:and|}>0;";
61-
var statementWithSpace = "_ = 1 is 1 and >0;";
70+
var statementWithoutSpace = $"_ = (int?)1 is not null {{|#0:{logicalOperator}|}}{relationalOperator}1;";
71+
var statementWithSpace = $"_ = (int?)1 is not null {logicalOperator} {relationalOperator}1;";
6272

63-
var expected = Diagnostic().WithArguments("and", string.Empty, "followed").WithLocation(0);
73+
var expected = Diagnostic().WithArguments(logicalOperator, string.Empty, "followed").WithLocation(0);
6474
await this.TestKeywordStatementAsync(statementWithoutSpace, expected, statementWithSpace).ConfigureAwait(false);
6575
}
6676

6777
[Fact]
68-
[WorkItem(3508, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3508")]
69-
public async Task TestOrBeforeRelationalPatternAsync()
78+
[WorkItem(3968, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3968")]
79+
public async Task TestNotBeforeConstantPatternMissingSpaceAsync()
7080
{
71-
var statementWithoutSpace = "_ = 1 is 1 {|#0:or|}>1;";
72-
var statementWithSpace = "_ = 1 is 1 or >1;";
81+
var statementWithoutSpace = "_ = new object() is {|#0:not|}(null);";
82+
var statementWithSpace = "_ = new object() is not (null);";
7383

74-
var expected = Diagnostic().WithArguments("or", string.Empty, "followed").WithLocation(0);
84+
var expected = Diagnostic().WithArguments("not", string.Empty, "followed").WithLocation(0);
7585
await this.TestKeywordStatementAsync(statementWithoutSpace, expected, statementWithSpace).ConfigureAwait(false);
7686
}
7787
}

StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp9/SpacingRules/SA1003CSharp9UnitTests.cs

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,5 +46,51 @@ void M(bool flag)
4646

4747
await VerifyCSharpFixAsync(testCode, expected, fixedCode, CancellationToken.None).ConfigureAwait(false);
4848
}
49+
50+
[Fact]
51+
[WorkItem(3968, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3968")]
52+
public async Task TestRelationalPatternsAreValidatedAsync()
53+
{
54+
const string testCode = @"
55+
class C
56+
{
57+
void M(int value)
58+
{
59+
_ = value is {|#0:>|}5;
60+
_ = value is{|#1:<|} 5;
61+
_ = value is ( < 5); // Validated by SA1008
62+
_ = value is (< 5);
63+
_ = value is {|#2:<=|}5;
64+
_ = value is {|#3:>=|}5;
65+
_ = value is {|#4:>|}
66+
5;
67+
}
68+
}";
69+
70+
const string fixedCode = @"
71+
class C
72+
{
73+
void M(int value)
74+
{
75+
_ = value is > 5;
76+
_ = value is < 5;
77+
_ = value is ( < 5); // Validated by SA1008
78+
_ = value is (< 5);
79+
_ = value is <= 5;
80+
_ = value is >= 5;
81+
_ = value is > 5;
82+
}
83+
}";
84+
85+
DiagnosticResult[] expected =
86+
{
87+
Diagnostic(DescriptorFollowedByWhitespace).WithLocation(0).WithArguments(">"),
88+
Diagnostic(DescriptorPrecededByWhitespace).WithLocation(1).WithArguments("<"),
89+
Diagnostic(DescriptorFollowedByWhitespace).WithLocation(2).WithArguments("<="),
90+
Diagnostic(DescriptorFollowedByWhitespace).WithLocation(3).WithArguments(">="),
91+
Diagnostic(DescriptorNotAtEndOfLine).WithLocation(4).WithArguments(">"),
92+
};
93+
await VerifyCSharpFixAsync(testCode, expected, fixedCode, CancellationToken.None).ConfigureAwait(false);
94+
}
4995
}
5096
}

StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp9/SpacingRules/SA1008CSharp9UnitTests.cs

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,5 +110,46 @@ public async Task TestDeconstructionInTopLevelProgramAsync(string prefix)
110110
FixedCode = fixedCode,
111111
}.RunAsync(CancellationToken.None).ConfigureAwait(false);
112112
}
113+
114+
[Fact]
115+
[WorkItem(3968, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3968")]
116+
public async Task TestLogicalPatternsWithParenthesesAsync()
117+
{
118+
const string testCode = @"
119+
class C
120+
{
121+
void M(int value)
122+
{
123+
_ = value is not{|#0:(|}> 0);
124+
_ = value is > 0 and{|#1:(|}< 5);
125+
_ = value is > 0 and {|#2:(|} < 5);
126+
_ = value is > 10 or{|#3:(|}< 5);
127+
_ = value is > 10 or {|#4:(|} < 5);
128+
}
129+
}";
130+
131+
const string fixedCode = @"
132+
class C
133+
{
134+
void M(int value)
135+
{
136+
_ = value is not (> 0);
137+
_ = value is > 0 and (< 5);
138+
_ = value is > 0 and (< 5);
139+
_ = value is > 10 or (< 5);
140+
_ = value is > 10 or (< 5);
141+
}
142+
}";
143+
144+
DiagnosticResult[] expected =
145+
{
146+
Diagnostic(DescriptorPreceded).WithLocation(0),
147+
Diagnostic(DescriptorPreceded).WithLocation(1),
148+
Diagnostic(DescriptorNotFollowed).WithLocation(2),
149+
Diagnostic(DescriptorPreceded).WithLocation(3),
150+
Diagnostic(DescriptorNotFollowed).WithLocation(4),
151+
};
152+
await VerifyCSharpFixAsync(testCode, expected, fixedCode, CancellationToken.None).ConfigureAwait(false);
153+
}
113154
}
114155
}

0 commit comments

Comments
 (0)