Skip to content

Commit aff686a

Browse files
sharwellsharwell
authored
Merge pull request #2478 from sharwell/fix-tuple-type-parens
Fix tuple type parentheses handling
2 parents 45c5de7 + 0ccaad4 commit aff686a

6 files changed

Lines changed: 317 additions & 9 deletions

File tree

‎StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp7/SpacingRules/SA1008CSharp7UnitTests.cs‎

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,112 @@ public class TestClass
136136
await this.VerifyCSharpFixAsync(testCode, fixedCode, numberOfFixAllIterations: 2).ConfigureAwait(false);
137137
}
138138

139+
[Fact]
140+
[WorkItem(2472, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/2472")]
141+
public async Task TestTupleArrayParameterTypeAsync()
142+
{
143+
var testCode = @"namespace TestNamespace
144+
{
145+
public class TestClass
146+
{
147+
public int TestMethod1( ( int, int)[] arg) => 0;
148+
public int TestMethod2( (int, int)[] arg) => 0;
149+
public int TestMethod3(( int, int)[] arg) => 0;
150+
151+
public int TestMethod4((int, int) arg1, params( int, int)[] arg2) => 0;
152+
public int TestMethod5((int, int) arg1, params(int, int)[] arg2) => 0;
153+
public int TestMethod6((int, int) arg1, params ( int, int)[] arg2) => 0;
154+
}
155+
}
156+
";
157+
158+
var fixedCode = @"namespace TestNamespace
159+
{
160+
public class TestClass
161+
{
162+
public int TestMethod1((int, int)[] arg) => 0;
163+
public int TestMethod2((int, int)[] arg) => 0;
164+
public int TestMethod3((int, int)[] arg) => 0;
165+
166+
public int TestMethod4((int, int) arg1, params (int, int)[] arg2) => 0;
167+
public int TestMethod5((int, int) arg1, params (int, int)[] arg2) => 0;
168+
public int TestMethod6((int, int) arg1, params (int, int)[] arg2) => 0;
169+
}
170+
}
171+
";
172+
173+
DiagnosticResult[] expectedDiagnostic =
174+
{
175+
// TestMethod1
176+
this.CSharpDiagnostic(DescriptorNotFollowed).WithLocation(5, 31),
177+
this.CSharpDiagnostic(DescriptorNotFollowed).WithLocation(5, 33),
178+
179+
// TestMethod2
180+
this.CSharpDiagnostic(DescriptorNotFollowed).WithLocation(6, 31),
181+
182+
// TestMethod3
183+
this.CSharpDiagnostic(DescriptorNotFollowed).WithLocation(7, 32),
184+
185+
// TestMethod4
186+
this.CSharpDiagnostic(DescriptorPreceded).WithLocation(9, 55),
187+
this.CSharpDiagnostic(DescriptorNotFollowed).WithLocation(9, 55),
188+
189+
// TestMethod5
190+
this.CSharpDiagnostic(DescriptorPreceded).WithLocation(10, 55),
191+
192+
// TestMethod6
193+
this.CSharpDiagnostic(DescriptorNotFollowed).WithLocation(11, 56),
194+
};
195+
196+
await this.VerifyCSharpDiagnosticAsync(testCode, expectedDiagnostic, CancellationToken.None).ConfigureAwait(false);
197+
await this.VerifyCSharpDiagnosticAsync(fixedCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
198+
await this.VerifyCSharpFixAsync(testCode, fixedCode, numberOfFixAllIterations: 2).ConfigureAwait(false);
199+
}
200+
201+
[Fact]
202+
[WorkItem(2472, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/2472")]
203+
public async Task TestTupleOutParametersAsync()
204+
{
205+
var testCode = @"namespace TestNamespace
206+
{
207+
public class TestClass
208+
{
209+
public int TestMethod1(out( int, int)[] arg) => throw null;
210+
public int TestMethod2(out(int, int)[] arg) => throw null;
211+
public int TestMethod3(out ( int, int)[] arg) => throw null;
212+
}
213+
}
214+
";
215+
216+
var fixedCode = @"namespace TestNamespace
217+
{
218+
public class TestClass
219+
{
220+
public int TestMethod1(out (int, int)[] arg) => throw null;
221+
public int TestMethod2(out (int, int)[] arg) => throw null;
222+
public int TestMethod3(out (int, int)[] arg) => throw null;
223+
}
224+
}
225+
";
226+
227+
DiagnosticResult[] expectedDiagnostic =
228+
{
229+
// TestMethod1
230+
this.CSharpDiagnostic(DescriptorPreceded).WithLocation(5, 35),
231+
this.CSharpDiagnostic(DescriptorNotFollowed).WithLocation(5, 35),
232+
233+
// TestMethod2
234+
this.CSharpDiagnostic(DescriptorPreceded).WithLocation(6, 35),
235+
236+
// TestMethod3
237+
this.CSharpDiagnostic(DescriptorNotFollowed).WithLocation(7, 36),
238+
};
239+
240+
await this.VerifyCSharpDiagnosticAsync(testCode, expectedDiagnostic, CancellationToken.None).ConfigureAwait(false);
241+
await this.VerifyCSharpDiagnosticAsync(fixedCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
242+
await this.VerifyCSharpFixAsync(testCode, fixedCode, numberOfFixAllIterations: 2).ConfigureAwait(false);
243+
}
244+
139245
/// <summary>
140246
/// Verifies that spacing around tuple return types is handled properly.
141247
/// </summary>
@@ -273,6 +379,50 @@ public class TestClass
273379
await this.VerifyCSharpFixAsync(testCode, fixedCode, numberOfFixAllIterations: 2).ConfigureAwait(false);
274380
}
275381

382+
[Fact]
383+
[WorkItem(2472, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/2472")]
384+
public async Task TestNullableTupleReturnTypeAsync()
385+
{
386+
var testCode = @"namespace TestNamespace
387+
{
388+
public class TestClass
389+
{
390+
public( int, int)? TestMethod1() => default( ( int, int)?);
391+
public(int, int)? TestMethod2() => default( (int, int)?);
392+
public ( int, int)? TestMethod3() => default(( int, int)?);
393+
}
394+
}
395+
";
396+
397+
var fixedCode = @"namespace TestNamespace
398+
{
399+
public class TestClass
400+
{
401+
public (int, int)? TestMethod1() => default((int, int)?);
402+
public (int, int)? TestMethod2() => default((int, int)?);
403+
public (int, int)? TestMethod3() => default((int, int)?);
404+
}
405+
}
406+
";
407+
408+
DiagnosticResult[] expectedDiagnostic =
409+
{
410+
// TestMethod1, TestMethod2, TestMethod3
411+
this.CSharpDiagnostic(DescriptorPreceded).WithLocation(5, 15),
412+
this.CSharpDiagnostic(DescriptorNotFollowed).WithLocation(5, 15),
413+
this.CSharpDiagnostic(DescriptorNotFollowed).WithLocation(5, 52),
414+
this.CSharpDiagnostic(DescriptorNotFollowed).WithLocation(5, 54),
415+
this.CSharpDiagnostic(DescriptorPreceded).WithLocation(6, 15),
416+
this.CSharpDiagnostic(DescriptorNotFollowed).WithLocation(6, 51),
417+
this.CSharpDiagnostic(DescriptorNotFollowed).WithLocation(7, 16),
418+
this.CSharpDiagnostic(DescriptorNotFollowed).WithLocation(7, 54),
419+
};
420+
421+
await this.VerifyCSharpDiagnosticAsync(testCode, expectedDiagnostic, CancellationToken.None).ConfigureAwait(false);
422+
await this.VerifyCSharpDiagnosticAsync(fixedCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
423+
await this.VerifyCSharpFixAsync(testCode, fixedCode, numberOfFixAllIterations: 2).ConfigureAwait(false);
424+
}
425+
276426
/// <summary>
277427
/// Verifies that spacing for tuple expressions is handled properly.
278428
/// </summary>

‎StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp7/SpacingRules/SA1009CSharp7UnitTests.cs‎

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -453,6 +453,68 @@ public class TestClass
453453
await this.VerifyCSharpFixAsync(testCode, fixedCode).ConfigureAwait(false);
454454
}
455455

456+
[Fact]
457+
[WorkItem(2476, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/2476")]
458+
public async Task TestNullableAndArrayTupleReturnTypeAsync()
459+
{
460+
var testCode = @"namespace TestNamespace
461+
{
462+
public class TestClass
463+
{
464+
public (int, int ) ? TestMethod1() => default((int, int ) ?);
465+
public (int, int )? TestMethod2() => default((int, int )?);
466+
public (int, int) ? TestMethod3() => default((int, int) ?);
467+
468+
public (int, int ) [] TestMethod4() => default((int, int ) []);
469+
public (int, int )[] TestMethod5() => default((int, int )[]);
470+
public (int, int) [] TestMethod6() => default((int, int) []);
471+
}
472+
}
473+
";
474+
475+
var fixedCode = @"namespace TestNamespace
476+
{
477+
public class TestClass
478+
{
479+
public (int, int)? TestMethod1() => default((int, int)?);
480+
public (int, int)? TestMethod2() => default((int, int)?);
481+
public (int, int)? TestMethod3() => default((int, int)?);
482+
483+
public (int, int)[] TestMethod4() => default((int, int)[]);
484+
public (int, int)[] TestMethod5() => default((int, int)[]);
485+
public (int, int)[] TestMethod6() => default((int, int)[]);
486+
}
487+
}
488+
";
489+
490+
DiagnosticResult[] expectedDiagnostic =
491+
{
492+
// TestMethod1, TestMethod2, TestMethod3
493+
this.CSharpDiagnostic().WithArguments(" not", "preceded").WithLocation(5, 26),
494+
this.CSharpDiagnostic().WithArguments(" not", "followed").WithLocation(5, 26),
495+
this.CSharpDiagnostic().WithArguments(" not", "preceded").WithLocation(5, 65),
496+
this.CSharpDiagnostic().WithArguments(" not", "followed").WithLocation(5, 65),
497+
this.CSharpDiagnostic().WithArguments(" not", "preceded").WithLocation(6, 26),
498+
this.CSharpDiagnostic().WithArguments(" not", "preceded").WithLocation(6, 64),
499+
this.CSharpDiagnostic().WithArguments(" not", "followed").WithLocation(7, 25),
500+
this.CSharpDiagnostic().WithArguments(" not", "followed").WithLocation(7, 63),
501+
502+
// TestMethod4, TestMethod5, TestMethod6
503+
this.CSharpDiagnostic().WithArguments(" not", "preceded").WithLocation(9, 26),
504+
this.CSharpDiagnostic().WithArguments(" not", "followed").WithLocation(9, 26),
505+
this.CSharpDiagnostic().WithArguments(" not", "preceded").WithLocation(9, 66),
506+
this.CSharpDiagnostic().WithArguments(" not", "followed").WithLocation(9, 66),
507+
this.CSharpDiagnostic().WithArguments(" not", "preceded").WithLocation(10, 26),
508+
this.CSharpDiagnostic().WithArguments(" not", "preceded").WithLocation(10, 65),
509+
this.CSharpDiagnostic().WithArguments(" not", "followed").WithLocation(11, 25),
510+
this.CSharpDiagnostic().WithArguments(" not", "followed").WithLocation(11, 64),
511+
};
512+
513+
await this.VerifyCSharpDiagnosticAsync(testCode, expectedDiagnostic, CancellationToken.None).ConfigureAwait(false);
514+
await this.VerifyCSharpDiagnosticAsync(fixedCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
515+
await this.VerifyCSharpFixAsync(testCode, fixedCode).ConfigureAwait(false);
516+
}
517+
456518
/// <summary>
457519
/// Verifies that spacing for tuple expressions is handled properly.
458520
/// </summary>
@@ -796,6 +858,72 @@ public class TestClass
796858
await this.VerifyCSharpFixAsync(testCode, fixedCode).ConfigureAwait(false);
797859
}
798860

861+
[Fact]
862+
[WorkItem(2476, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/2476")]
863+
public async Task TestNullableAndArrayTupleTypesAsGenericArgumentsAsync()
864+
{
865+
var testCode = @"using System;
866+
867+
namespace TestNamespace
868+
{
869+
public class TestClass
870+
{
871+
static Func<(int, int ) ?, (int, int ) ?> Function1;
872+
static Func<(int, int )?, (int, int )?> Function2;
873+
static Func<(int, int) ?, (int, int) ?> Function3;
874+
875+
static Func<(int, int ) [], (int, int ) []> Function4;
876+
static Func<(int, int )[], (int, int )[]> Function5;
877+
static Func<(int, int) [], (int, int) []> Function6;
878+
}
879+
}
880+
";
881+
882+
var fixedCode = @"using System;
883+
884+
namespace TestNamespace
885+
{
886+
public class TestClass
887+
{
888+
static Func<(int, int)?, (int, int)?> Function1;
889+
static Func<(int, int)?, (int, int)?> Function2;
890+
static Func<(int, int)?, (int, int)?> Function3;
891+
892+
static Func<(int, int)[], (int, int)[]> Function4;
893+
static Func<(int, int)[], (int, int)[]> Function5;
894+
static Func<(int, int)[], (int, int)[]> Function6;
895+
}
896+
}
897+
";
898+
899+
DiagnosticResult[] expectedDiagnostics =
900+
{
901+
// Function1, Function2, Function3
902+
this.CSharpDiagnostic().WithArguments(" not", "preceded").WithLocation(7, 31),
903+
this.CSharpDiagnostic().WithArguments(" not", "followed").WithLocation(7, 31),
904+
this.CSharpDiagnostic().WithArguments(" not", "preceded").WithLocation(7, 46),
905+
this.CSharpDiagnostic().WithArguments(" not", "followed").WithLocation(7, 46),
906+
this.CSharpDiagnostic().WithArguments(" not", "preceded").WithLocation(8, 31),
907+
this.CSharpDiagnostic().WithArguments(" not", "preceded").WithLocation(8, 45),
908+
this.CSharpDiagnostic().WithArguments(" not", "followed").WithLocation(9, 30),
909+
this.CSharpDiagnostic().WithArguments(" not", "followed").WithLocation(9, 44),
910+
911+
// Function4, Function5, Function6
912+
this.CSharpDiagnostic().WithArguments(" not", "preceded").WithLocation(11, 31),
913+
this.CSharpDiagnostic().WithArguments(" not", "followed").WithLocation(11, 31),
914+
this.CSharpDiagnostic().WithArguments(" not", "preceded").WithLocation(11, 47),
915+
this.CSharpDiagnostic().WithArguments(" not", "followed").WithLocation(11, 47),
916+
this.CSharpDiagnostic().WithArguments(" not", "preceded").WithLocation(12, 31),
917+
this.CSharpDiagnostic().WithArguments(" not", "preceded").WithLocation(12, 46),
918+
this.CSharpDiagnostic().WithArguments(" not", "followed").WithLocation(13, 30),
919+
this.CSharpDiagnostic().WithArguments(" not", "followed").WithLocation(13, 45),
920+
};
921+
922+
await this.VerifyCSharpDiagnosticAsync(testCode, expectedDiagnostics, CancellationToken.None).ConfigureAwait(false);
923+
await this.VerifyCSharpDiagnosticAsync(fixedCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
924+
await this.VerifyCSharpFixAsync(testCode, fixedCode).ConfigureAwait(false);
925+
}
926+
799927
/// <summary>
800928
/// Verifies that spacing for tuple variable declarations is handled properly.
801929
/// </summary>

‎StyleCop.Analyzers/StyleCop.Analyzers.Test/Helpers/MetadataReferences.cs‎

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,16 +26,25 @@ internal static class MetadataReferences
2626

2727
static MetadataReferences()
2828
{
29-
Assembly systemRuntime = AppDomain.CurrentDomain.GetAssemblies().SingleOrDefault(x => x.GetName().Name == "System.Runtime");
30-
if (systemRuntime != null)
29+
if (typeof(string).Assembly.GetType("System.ValueTuple", false) != null)
3130
{
32-
SystemRuntimeReference = MetadataReference.CreateFromFile(systemRuntime.Location);
31+
// mscorlib contains ValueTuple, so no need to add a separate reference
32+
SystemRuntimeReference = null;
33+
SystemValueTupleReference = null;
3334
}
34-
35-
Assembly systemValueTuple = AppDomain.CurrentDomain.GetAssemblies().SingleOrDefault(x => x.GetName().Name == "System.ValueTuple");
36-
if (systemValueTuple != null)
35+
else
3736
{
38-
SystemValueTupleReference = MetadataReference.CreateFromFile(systemValueTuple.Location);
37+
Assembly systemRuntime = AppDomain.CurrentDomain.GetAssemblies().SingleOrDefault(x => x.GetName().Name == "System.Runtime");
38+
if (systemRuntime != null)
39+
{
40+
SystemRuntimeReference = MetadataReference.CreateFromFile(systemRuntime.Location);
41+
}
42+
43+
Assembly systemValueTuple = AppDomain.CurrentDomain.GetAssemblies().SingleOrDefault(x => x.GetName().Name == "System.ValueTuple");
44+
if (systemValueTuple != null)
45+
{
46+
SystemValueTupleReference = MetadataReference.CreateFromFile(systemValueTuple.Location);
47+
}
3948
}
4049
}
4150
}

‎StyleCop.Analyzers/StyleCop.Analyzers/Helpers/TypeSyntaxHelper.cs‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,24 @@ namespace StyleCop.Analyzers.Helpers
99

1010
internal static class TypeSyntaxHelper
1111
{
12+
public static TypeSyntax GetContainingNotEnclosingType(this TypeSyntax syntax)
13+
{
14+
while (true)
15+
{
16+
switch (syntax.Parent.Kind())
17+
{
18+
case SyntaxKind.ArrayType:
19+
case SyntaxKind.NullableType:
20+
case SyntaxKind.PointerType:
21+
syntax = (TypeSyntax)syntax.Parent;
22+
break;
23+
24+
default:
25+
return syntax;
26+
}
27+
}
28+
}
29+
1230
public static bool IsReturnType(this TypeSyntax syntax)
1331
{
1432
switch (syntax.Parent.Kind())

‎StyleCop.Analyzers/StyleCop.Analyzers/SpacingRules/SA1008OpeningParenthesisMustBeSpacedCorrectly.cs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -237,9 +237,11 @@ private static void HandleOpenParenToken(SyntaxTreeAnalysisContext context, Synt
237237

238238
case SyntaxKindEx.TupleType:
239239
// Comma covers tuple types in parameters and nested within other tuple types.
240+
// 'out', 'ref', 'in', 'params' parameters are covered by IsKeywordKind.
240241
// Return types are handled by a helper.
241242
haveLeadingSpace = prevToken.IsKind(SyntaxKind.CommaToken)
242-
|| ((TypeSyntax)token.Parent).IsReturnType();
243+
|| SyntaxFacts.IsKeywordKind(prevToken.Kind())
244+
|| ((TypeSyntax)token.Parent).GetContainingNotEnclosingType().IsReturnType();
243245
break;
244246
}
245247

‎StyleCop.Analyzers/StyleCop.Analyzers/SpacingRules/SA1009ClosingParenthesisMustBeSpacedCorrectly.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,8 @@ private static void HandleCloseParenToken(SyntaxTreeAnalysisContext context, Syn
109109
}
110110
else
111111
{
112-
precedesStickyCharacter = false;
112+
// A space follows unless this is a nullable tuple type
113+
precedesStickyCharacter = nextToken.Parent.IsKind(SyntaxKind.NullableType);
113114
}
114115

115116
break;

0 commit comments

Comments
 (0)