Update SA1316 to not trigger in tuple types which are part of a membe…

…r's signature, if that member is an override DotNetAnalyzers#3781
bjornhellander · Jan 21, 2024 · 6f94c20 · 6f94c20
1 parent f5f4ca9
commit 6f94c20
Show file tree

Hide file tree

Showing 2 changed files with 195 additions and 0 deletions.
diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp7/NamingRules/SA1316CSharp7UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp7/NamingRules/SA1316CSharp7UnitTests.cs
@@ -401,5 +401,158 @@ public void MethodName((string Name, string Value) obj)
 
             await VerifyCSharpDiagnosticAsync(testCode, DefaultTestSettings, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
         }
+
+        [Theory]
+        [InlineData("(int I, string foo)")]
+        [InlineData("(int I, (bool foo, string S))")]
+        [InlineData("(int foo, string S)[]")]
+        [InlineData("System.Collections.Generic.List<(string foo, bool B)>")]
+        [WorkItem(3781, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3781")]
+        public async Task TestImproperTupleElementNameInOverriddenMethodsParameterTypeAsync(string paramType)
+        {
+            var testCode = $@"
+public class BaseType
+{{
+    public virtual void TestMethod({paramType.Replace("foo", "[|foo|]")} p)
+    {{
+    }}
+}}
+
+public class TestType : BaseType
+{{
+    public override void TestMethod({paramType} p)
+    {{
+    }}
+}}
+";
+
+            await VerifyCSharpDiagnosticAsync(testCode, DefaultTestSettings, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
+        }
+
+        [Theory]
+        [InlineData("(int I, string foo)")]
+        [InlineData("(int I, (bool foo, string S))")]
+        [InlineData("(int foo, string S)[]")]
+        [InlineData("List<(string foo, bool B)>")]
+        [WorkItem(3781, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3781")]
+        public async Task TestImproperTupleElementNameInOverriddenMethodsReturnTypeAsync(string returnType)
+        {
+            var testCode = $@"
+using System.Collections.Generic;
+
+public class BaseType
+{{
+    public virtual {returnType.Replace("foo", "[|foo|]")} TestMethod()
+    {{
+        throw new System.Exception();
+    }}
+}}
+
+public class TestType : BaseType
+{{
+    public override {returnType} TestMethod()
+    {{
+        throw new System.Exception();
+    }}
+}}
+";
+
+            await VerifyCSharpDiagnosticAsync(testCode, DefaultTestSettings, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
+        }
+
+        [Theory]
+        [InlineData("(int I, string foo)")]
+        [InlineData("(int I, (bool foo, string S))")]
+        [InlineData("(int foo, string S)[]")]
+        [InlineData("List<(string foo, bool B)>")]
+        [WorkItem(3781, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3781")]
+        public async Task TestImproperTupleElementNameInOverriddenPropertysTypeAsync(string type)
+        {
+            var testCode = $@"
+using System.Collections.Generic;
+
+public class BaseType
+{{
+    public virtual {type.Replace("foo", "[|foo|]")} TestProperty {{ get; set; }}
+}}
+
+public class TestType : BaseType
+{{
+    public override {type} TestProperty {{ get; set; }}
+}}
+";
+
+            await VerifyCSharpDiagnosticAsync(testCode, DefaultTestSettings, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
+        }
+
+        [Theory]
+        [InlineData("(int I, string foo)")]
+        [InlineData("(int I, (bool foo, string S))")]
+        [InlineData("(int foo, string S)[]")]
+        [InlineData("List<(string foo, bool B)>")]
+        [WorkItem(3781, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3781")]
+        public async Task TestImproperTupleElementNameInOverriddenIndexersReturnTypeAsync(string type)
+        {
+            var testCode = $@"
+using System.Collections.Generic;
+
+public abstract class BaseType
+{{
+    public abstract {type.Replace("foo", "[|foo|]")} this[int i] {{ get; }}
+}}
+
+public class TestType : BaseType
+{{
+    public override {type} this[int i] => throw new System.Exception();
+}}
+";
+
+            await VerifyCSharpDiagnosticAsync(testCode, DefaultTestSettings, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
+        }
+
+        [Fact]
+        [WorkItem(3781, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3781")]
+        public async Task TestImproperVariableTupleElementNameInsideOverriddenMethodAsync()
+        {
+            var testCode = @"
+using System.Collections.Generic;
+
+public abstract class BaseType
+{
+    public virtual void TestMethod()
+    {
+    }
+}
+
+public class TestType : BaseType
+{
+    public override void TestMethod()
+    {
+        (int I, bool [|b|]) x;
+    }
+}
+";
+
+            var fixedCode = @"
+using System.Collections.Generic;
+
+public abstract class BaseType
+{
+    public virtual void TestMethod()
+    {
+    }
+}
+
+public class TestType : BaseType
+{
+    public override void TestMethod()
+    {
+        (int I, bool B) x;
+    }
+}
+";
+
+            await VerifyCSharpFixAsync(testCode, DefaultTestSettings, DiagnosticResult.EmptyDiagnosticResults, fixedCode, CancellationToken.None).ConfigureAwait(false);
+        }
     }
 }
diff --git a/...Analyzers/StyleCop.Analyzers/NamingRules/SA1316TupleElementNamesShouldUseCorrectCasing.cs b/...Analyzers/StyleCop.Analyzers/NamingRules/SA1316TupleElementNamesShouldUseCorrectCasing.cs
@@ -8,6 +8,8 @@ namespace StyleCop.Analyzers.NamingRules
     using System;
     using System.Collections.Immutable;
     using Microsoft.CodeAnalysis;
+    using Microsoft.CodeAnalysis.CSharp;
+    using Microsoft.CodeAnalysis.CSharp.Syntax;
     using Microsoft.CodeAnalysis.Diagnostics;
     using StyleCop.Analyzers.Helpers;
     using StyleCop.Analyzers.Lightup;
@@ -66,12 +68,52 @@ private static void HandleTupleTypeAction(SyntaxNodeAnalysisContext context, Sty
 
             var tupleType = (TupleTypeSyntaxWrapper)context.Node;
 
+            if (IsPartOfMemberOverrideSignature(context.Node))
+            {
+                // In this case, it is not ok to change the tuple element name, since the compiler requires
+                // the names to match the original definition.
+                return;
+            }
+
             foreach (var tupleElement in tupleType.Elements)
             {
                 CheckTupleElement(context, settings, tupleElement);
             }
         }
 
+        private static bool IsPartOfMemberOverrideSignature(SyntaxNode node)
+        {
+            // NOTE: Avoiding a simple FirstAncestorOrSelf<MemberDeclarationSyntax>() call here, since
+            // that would also return true for e.g. tuple variable declarations inside a method body.
+            while (node != null)
+            {
+                switch (node.Kind())
+                {
+                case SyntaxKind.MethodDeclaration:
+                    return ((MethodDeclarationSyntax)node).Modifiers.Any(SyntaxKind.OverrideKeyword);
+
+                case SyntaxKind.IndexerDeclaration:
+                    return ((IndexerDeclarationSyntax)node).Modifiers.Any(SyntaxKind.OverrideKeyword);
+
+                case SyntaxKind.PropertyDeclaration:
+                    return ((PropertyDeclarationSyntax)node).Modifiers.Any(SyntaxKind.OverrideKeyword);
+
+                case SyntaxKind.Parameter:
+                case SyntaxKind.ParameterList:
+                case SyntaxKindEx.TupleElement:
+                case SyntaxKind.TypeArgumentList:
+                case SyntaxKind when node is TypeSyntax:
+                    node = node.Parent;
+                    break;
+
+                default:
+                    return false;
+                }
+            }
+
+            return false;
+        }
+
         private static void HandleTupleExpressionAction(SyntaxNodeAnalysisContext context, StyleCopSettings settings)
         {
             if (!context.SupportsInferredTupleElementNames())