Skip to content

Commit 6f94c20

Browse files
bjornhellanderbjornhellander
committed
Update SA1316 to not trigger in tuple types which are part of a member's signature, if that member is an override
#3781
1 parent f5f4ca9 commit 6f94c20

File tree

2 files changed

+195
-0
lines changed

2 files changed

+195
-0
lines changed

‎StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp7/NamingRules/SA1316CSharp7UnitTests.cs‎

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -401,5 +401,158 @@ public void MethodName((string Name, string Value) obj)
401401

402402
await VerifyCSharpDiagnosticAsync(testCode, DefaultTestSettings, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
403403
}
404+
405+
[Theory]
406+
[InlineData("(int I, string foo)")]
407+
[InlineData("(int I, (bool foo, string S))")]
408+
[InlineData("(int foo, string S)[]")]
409+
[InlineData("System.Collections.Generic.List<(string foo, bool B)>")]
410+
[WorkItem(3781, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3781")]
411+
public async Task TestImproperTupleElementNameInOverriddenMethodsParameterTypeAsync(string paramType)
412+
{
413+
var testCode = $@"
414+
public class BaseType
415+
{{
416+
public virtual void TestMethod({paramType.Replace("foo", "[|foo|]")} p)
417+
{{
418+
}}
419+
}}
420+
421+
public class TestType : BaseType
422+
{{
423+
public override void TestMethod({paramType} p)
424+
{{
425+
}}
426+
}}
427+
";
428+
429+
await VerifyCSharpDiagnosticAsync(testCode, DefaultTestSettings, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
430+
}
431+
432+
[Theory]
433+
[InlineData("(int I, string foo)")]
434+
[InlineData("(int I, (bool foo, string S))")]
435+
[InlineData("(int foo, string S)[]")]
436+
[InlineData("List<(string foo, bool B)>")]
437+
[WorkItem(3781, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3781")]
438+
public async Task TestImproperTupleElementNameInOverriddenMethodsReturnTypeAsync(string returnType)
439+
{
440+
var testCode = $@"
441+
using System.Collections.Generic;
442+
443+
public class BaseType
444+
{{
445+
public virtual {returnType.Replace("foo", "[|foo|]")} TestMethod()
446+
{{
447+
throw new System.Exception();
448+
}}
449+
}}
450+
451+
public class TestType : BaseType
452+
{{
453+
public override {returnType} TestMethod()
454+
{{
455+
throw new System.Exception();
456+
}}
457+
}}
458+
";
459+
460+
await VerifyCSharpDiagnosticAsync(testCode, DefaultTestSettings, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
461+
}
462+
463+
[Theory]
464+
[InlineData("(int I, string foo)")]
465+
[InlineData("(int I, (bool foo, string S))")]
466+
[InlineData("(int foo, string S)[]")]
467+
[InlineData("List<(string foo, bool B)>")]
468+
[WorkItem(3781, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3781")]
469+
public async Task TestImproperTupleElementNameInOverriddenPropertysTypeAsync(string type)
470+
{
471+
var testCode = $@"
472+
using System.Collections.Generic;
473+
474+
public class BaseType
475+
{{
476+
public virtual {type.Replace("foo", "[|foo|]")} TestProperty {{ get; set; }}
477+
}}
478+
479+
public class TestType : BaseType
480+
{{
481+
public override {type} TestProperty {{ get; set; }}
482+
}}
483+
";
484+
485+
await VerifyCSharpDiagnosticAsync(testCode, DefaultTestSettings, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
486+
}
487+
488+
[Theory]
489+
[InlineData("(int I, string foo)")]
490+
[InlineData("(int I, (bool foo, string S))")]
491+
[InlineData("(int foo, string S)[]")]
492+
[InlineData("List<(string foo, bool B)>")]
493+
[WorkItem(3781, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3781")]
494+
public async Task TestImproperTupleElementNameInOverriddenIndexersReturnTypeAsync(string type)
495+
{
496+
var testCode = $@"
497+
using System.Collections.Generic;
498+
499+
public abstract class BaseType
500+
{{
501+
public abstract {type.Replace("foo", "[|foo|]")} this[int i] {{ get; }}
502+
}}
503+
504+
public class TestType : BaseType
505+
{{
506+
public override {type} this[int i] => throw new System.Exception();
507+
}}
508+
";
509+
510+
await VerifyCSharpDiagnosticAsync(testCode, DefaultTestSettings, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
511+
}
512+
513+
[Fact]
514+
[WorkItem(3781, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3781")]
515+
public async Task TestImproperVariableTupleElementNameInsideOverriddenMethodAsync()
516+
{
517+
var testCode = @"
518+
using System.Collections.Generic;
519+
520+
public abstract class BaseType
521+
{
522+
public virtual void TestMethod()
523+
{
524+
}
525+
}
526+
527+
public class TestType : BaseType
528+
{
529+
public override void TestMethod()
530+
{
531+
(int I, bool [|b|]) x;
532+
}
533+
}
534+
";
535+
536+
var fixedCode = @"
537+
using System.Collections.Generic;
538+
539+
public abstract class BaseType
540+
{
541+
public virtual void TestMethod()
542+
{
543+
}
544+
}
545+
546+
public class TestType : BaseType
547+
{
548+
public override void TestMethod()
549+
{
550+
(int I, bool B) x;
551+
}
552+
}
553+
";
554+
555+
await VerifyCSharpFixAsync(testCode, DefaultTestSettings, DiagnosticResult.EmptyDiagnosticResults, fixedCode, CancellationToken.None).ConfigureAwait(false);
556+
}
404557
}
405558
}

‎StyleCop.Analyzers/StyleCop.Analyzers/NamingRules/SA1316TupleElementNamesShouldUseCorrectCasing.cs‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ namespace StyleCop.Analyzers.NamingRules
88
using System;
99
using System.Collections.Immutable;
1010
using Microsoft.CodeAnalysis;
11+
using Microsoft.CodeAnalysis.CSharp;
12+
using Microsoft.CodeAnalysis.CSharp.Syntax;
1113
using Microsoft.CodeAnalysis.Diagnostics;
1214
using StyleCop.Analyzers.Helpers;
1315
using StyleCop.Analyzers.Lightup;
@@ -66,12 +68,52 @@ private static void HandleTupleTypeAction(SyntaxNodeAnalysisContext context, Sty
6668

6769
var tupleType = (TupleTypeSyntaxWrapper)context.Node;
6870

71+
if (IsPartOfMemberOverrideSignature(context.Node))
72+
{
73+
// In this case, it is not ok to change the tuple element name, since the compiler requires
74+
// the names to match the original definition.
75+
return;
76+
}
77+
6978
foreach (var tupleElement in tupleType.Elements)
7079
{
7180
CheckTupleElement(context, settings, tupleElement);
7281
}
7382
}
7483

84+
private static bool IsPartOfMemberOverrideSignature(SyntaxNode node)
85+
{
86+
// NOTE: Avoiding a simple FirstAncestorOrSelf<MemberDeclarationSyntax>() call here, since
87+
// that would also return true for e.g. tuple variable declarations inside a method body.
88+
while (node != null)
89+
{
90+
switch (node.Kind())
91+
{
92+
case SyntaxKind.MethodDeclaration:
93+
return ((MethodDeclarationSyntax)node).Modifiers.Any(SyntaxKind.OverrideKeyword);
94+
95+
case SyntaxKind.IndexerDeclaration:
96+
return ((IndexerDeclarationSyntax)node).Modifiers.Any(SyntaxKind.OverrideKeyword);
97+
98+
case SyntaxKind.PropertyDeclaration:
99+
return ((PropertyDeclarationSyntax)node).Modifiers.Any(SyntaxKind.OverrideKeyword);
100+
101+
case SyntaxKind.Parameter:
102+
case SyntaxKind.ParameterList:
103+
case SyntaxKindEx.TupleElement:
104+
case SyntaxKind.TypeArgumentList:
105+
case SyntaxKind when node is TypeSyntax:
106+
node = node.Parent;
107+
break;
108+
109+
default:
110+
return false;
111+
}
112+
}
113+
114+
return false;
115+
}
116+
75117
private static void HandleTupleExpressionAction(SyntaxNodeAnalysisContext context, StyleCopSettings settings)
76118
{
77119
if (!context.SupportsInferredTupleElementNames())

0 commit comments

Comments
 (0)