diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp7/NamingRules/SA1316CSharp7UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp7/NamingRules/SA1316CSharp7UnitTests.cs index 5e784d66a..d6be0e77c 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp7/NamingRules/SA1316CSharp7UnitTests.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp7/NamingRules/SA1316CSharp7UnitTests.cs @@ -5,11 +5,13 @@ namespace StyleCop.Analyzers.Test.CSharp7.NamingRules { + using System.Collections.Generic; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.Testing; using StyleCop.Analyzers.NamingRules; + using StyleCop.Analyzers.Test.Helpers; using Xunit; using static StyleCop.Analyzers.Test.Helpers.LanguageVersionTestExtensions; using static StyleCop.Analyzers.Test.Verifiers.StyleCopCodeFixVerifier< @@ -94,6 +96,14 @@ public class SA1316CSharp7UnitTests } "; + public static IEnumerable TypesWithOneLowerCaseTupleElement { get; } = new[] + { + new[] { "(int I, string foo)" }, + new[] { "(int I, (bool foo, string S))" }, + new[] { "(int foo, string S)[]" }, + new[] { "List<(string S, bool foo)>" }, + }; + /// /// Validates the properly named tuple element names will not produce diagnostics. /// @@ -401,5 +411,403 @@ public void MethodName((string Name, string Value) obj) await VerifyCSharpDiagnosticAsync(testCode, DefaultTestSettings, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false); } + + [Theory] + [MemberData(nameof(TypesWithOneLowerCaseTupleElement))] + [WorkItem(3781, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3781")] + public async Task TestImproperTupleElementNameInOverriddenMethodsParameterTypeAsync(string type) + { + var testCode = $@" +using System.Collections.Generic; + +public class BaseType +{{ + public virtual void TestMethod({type.Replace("foo", "[|foo|]")} p) + {{ + }} +}} + +public class TestType : BaseType +{{ + public override void TestMethod({type} p) + {{ + }} +}} +"; + + await VerifyCSharpDiagnosticAsync(testCode, DefaultTestSettings, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false); + } + + [Theory] + [MemberData(nameof(TypesWithOneLowerCaseTupleElement))] + [WorkItem(3781, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3781")] + public async Task TestImproperTupleElementNameInExplicitlyImplementedMethodsParameterTypeAsync(string type) + { + var testCode = $@" +using System.Collections.Generic; + +public interface TestInterface +{{ + void TestMethod({type.Replace("foo", "[|foo|]")} p); +}} + +public class TestType : TestInterface +{{ + void TestInterface.TestMethod({type} p) + {{ + }} +}} +"; + + await VerifyCSharpDiagnosticAsync(testCode, DefaultTestSettings, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false); + } + + [Theory] + [MemberData(nameof(TypesWithOneLowerCaseTupleElement))] + [WorkItem(3781, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3781")] + public async Task TestImproperTupleElementNameInInterfaceImplementedMethodsParameterTypeAsync(string type) + { + var testCode = $@" +using System.Collections.Generic; + +public interface TestInterface +{{ + void TestMethod({type.Replace("foo", "[|foo|]")} p); +}} + +public class TestType : TestInterface +{{ + public void TestMethod({type} p) + {{ + }} +}} +"; + + await VerifyCSharpDiagnosticAsync(testCode, DefaultTestSettings, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false); + } + + [Theory] + [MemberData(nameof(TypesWithOneLowerCaseTupleElement))] + [WorkItem(3781, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3781")] + public async Task TestImproperTupleElementNameInOverriddenMethodsReturnTypeAsync(string type) + { + var testCode = $@" +using System.Collections.Generic; + +public class BaseType +{{ + public virtual {type.Replace("foo", "[|foo|]")} TestMethod() + {{ + throw new System.Exception(); + }} +}} + +public class TestType : BaseType +{{ + public override {type} TestMethod() + {{ + throw new System.Exception(); + }} +}} +"; + + await VerifyCSharpDiagnosticAsync(testCode, DefaultTestSettings, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false); + } + + [Theory] + [MemberData(nameof(TypesWithOneLowerCaseTupleElement))] + [WorkItem(3781, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3781")] + public async Task TestImproperTupleElementNameInExplicitlyImplementedMethodsReturnTypeAsync(string type) + { + var testCode = $@" +using System.Collections.Generic; + +public interface TestInterface +{{ + {type.Replace("foo", "[|foo|]")} TestMethod(); +}} + +public class TestType : TestInterface +{{ + {type} TestInterface.TestMethod() + {{ + throw new System.Exception(); + }} +}} +"; + + await VerifyCSharpDiagnosticAsync(testCode, DefaultTestSettings, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false); + } + + [Theory] + [MemberData(nameof(TypesWithOneLowerCaseTupleElement))] + [WorkItem(3781, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3781")] + public async Task TestImproperTupleElementNameInInterfaceImplementedMethodsReturnTypeAsync(string type) + { + var testCode = $@" +using System.Collections.Generic; + +public interface TestInterface +{{ + {type.Replace("foo", "[|foo|]")} TestMethod(); +}} + +public class TestType : TestInterface +{{ + public {type} TestMethod() + {{ + throw new System.Exception(); + }} +}} +"; + + await VerifyCSharpDiagnosticAsync(testCode, DefaultTestSettings, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false); + } + + [Theory] + [MemberData(nameof(TypesWithOneLowerCaseTupleElement))] + [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] + [MemberData(nameof(TypesWithOneLowerCaseTupleElement))] + [WorkItem(3781, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3781")] + public async Task TestImproperTupleElementNameInExplicitlyImplementedPropertysTypeAsync(string type) + { + var testCode = $@" +using System.Collections.Generic; + +public interface TestInterface +{{ + {type.Replace("foo", "[|foo|]")} TestProperty {{ get; set; }} +}} + +public class TestType : TestInterface +{{ + {type} TestInterface.TestProperty {{ get; set; }} +}} +"; + + await VerifyCSharpDiagnosticAsync(testCode, DefaultTestSettings, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false); + } + + [Theory] + [MemberData(nameof(TypesWithOneLowerCaseTupleElement))] + [WorkItem(3781, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3781")] + public async Task TestImproperTupleElementNameInInterfaceImplementedPropertysTypeAsync(string type) + { + var testCode = $@" +using System.Collections.Generic; + +public interface TestInterface +{{ + {type.Replace("foo", "[|foo|]")} TestProperty {{ get; set; }} +}} + +public class TestType : TestInterface +{{ + public {type} TestProperty {{ get; set; }} +}} +"; + + await VerifyCSharpDiagnosticAsync(testCode, DefaultTestSettings, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false); + } + + [Theory] + [MemberData(nameof(TypesWithOneLowerCaseTupleElement))] + [WorkItem(3781, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3781")] + public async Task TestImproperTupleElementNameInOverriddenEventsTypeAsync(string type) + { + var testCode = $@" +using System; +using System.Collections.Generic; + +public class BaseType +{{ + public virtual event Action<{type.Replace("foo", "[|foo|]")}> TestEvent {{ add {{}} remove {{}} }} +}} + +public class TestType : BaseType +{{ + public override event Action<{type}> TestEvent {{ add {{}} remove {{}} }} +}} +"; + + await VerifyCSharpDiagnosticAsync(testCode, DefaultTestSettings, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false); + } + + [Theory] + [MemberData(nameof(TypesWithOneLowerCaseTupleElement))] + [WorkItem(3781, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3781")] + public async Task TestImproperTupleElementNameInExplicitlyImplementedEventsTypeAsync(string type) + { + var testCode = $@" +using System; +using System.Collections.Generic; + +public interface TestInterface +{{ + event Action<{type.Replace("foo", "[|foo|]")}> TestEvent; +}} + +public class TestType : TestInterface +{{ + event Action<{type}> TestInterface.TestEvent {{ add {{}} remove {{}} }} +}} +"; + + await VerifyCSharpDiagnosticAsync(testCode, DefaultTestSettings, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false); + } + + [Theory] + [MemberData(nameof(TypesWithOneLowerCaseTupleElement))] + [WorkItem(3781, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3781")] + public async Task TestImproperTupleElementNameInInterfaceImplementedEventsTypeAsync(string type) + { + var testCode = $@" +using System; +using System.Collections.Generic; + +public interface TestInterface +{{ + event Action<{type.Replace("foo", "[|foo|]")}> TestEvent; +}} + +public class TestType : TestInterface +{{ + public event Action<{type}> TestEvent {{ add {{}} remove {{}} }} +}} +"; + + await VerifyCSharpDiagnosticAsync(testCode, DefaultTestSettings, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false); + } + + [Theory] + [MemberData(nameof(TypesWithOneLowerCaseTupleElement))] + [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); + } + + [Theory] + [MemberData(nameof(TypesWithOneLowerCaseTupleElement))] + [WorkItem(3781, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3781")] + public async Task TestImproperTupleElementNameInExplicitlyImplementedIndexersReturnTypeAsync(string type) + { + var testCode = $@" +using System.Collections.Generic; + +public interface TestInterface +{{ + {type.Replace("foo", "[|foo|]")} this[int i] {{ get; }} +}} + +public class TestType : TestInterface +{{ + {type} TestInterface.this[int i] => throw new System.Exception(); +}} +"; + + await VerifyCSharpDiagnosticAsync(testCode, DefaultTestSettings, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false); + } + + [Theory] + [MemberData(nameof(TypesWithOneLowerCaseTupleElement))] + [WorkItem(3781, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3781")] + public async Task TestImproperTupleElementNameInInterfaceImplementedIndexersReturnTypeAsync(string type) + { + var testCode = $@" +using System.Collections.Generic; + +public interface TestInterface +{{ + {type.Replace("foo", "[|foo|]")} this[int i] {{ get; }} +}} + +public class TestType : TestInterface +{{ + public {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/StyleCop.Analyzers/StyleCop.Analyzers/NamingRules/SA1316TupleElementNamesShouldUseCorrectCasing.cs b/StyleCop.Analyzers/StyleCop.Analyzers/NamingRules/SA1316TupleElementNamesShouldUseCorrectCasing.cs index 8e42a63ad..634776519 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/NamingRules/SA1316TupleElementNamesShouldUseCorrectCasing.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers/NamingRules/SA1316TupleElementNamesShouldUseCorrectCasing.cs @@ -7,7 +7,10 @@ namespace StyleCop.Analyzers.NamingRules { using System; using System.Collections.Immutable; + using System.Diagnostics; using Microsoft.CodeAnalysis; + using Microsoft.CodeAnalysis.CSharp; + using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Diagnostics; using StyleCop.Analyzers.Helpers; using StyleCop.Analyzers.Lightup; @@ -90,7 +93,7 @@ private static void HandleTupleExpressionAction(SyntaxNodeAnalysisContext contex var inferredMemberName = SyntaxFactsEx.TryGetInferredMemberName(argument.NameColon?.Name ?? argument.Expression); if (inferredMemberName != null) { - CheckName(context, settings, inferredMemberName, argument.Expression.GetLocation(), false); + CheckName(context, settings, tupleElement: null, inferredMemberName, argument.Expression.GetLocation(), false); } } } @@ -102,10 +105,10 @@ private static void CheckTupleElement(SyntaxNodeAnalysisContext context, StyleCo return; } - CheckName(context, settings, tupleElement.Identifier.ValueText, tupleElement.Identifier.GetLocation(), true); + CheckName(context, settings, tupleElement.SyntaxNode, tupleElement.Identifier.ValueText, tupleElement.Identifier.GetLocation(), true); } - private static void CheckName(SyntaxNodeAnalysisContext context, StyleCopSettings settings, string tupleElementName, Location location, bool prepareCodeFix) + private static void CheckName(SyntaxNodeAnalysisContext context, StyleCopSettings settings, SyntaxNode tupleElement, string tupleElementName, Location location, bool prepareCodeFix) { if (tupleElementName == "_") { @@ -130,18 +133,116 @@ private static void CheckName(SyntaxNodeAnalysisContext context, StyleCopSetting break; } - if (reportDiagnostic) + if (!reportDiagnostic) { - var diagnosticProperties = ImmutableDictionary.CreateBuilder(); + return; + } + + if (!CanTupleElementNameBeChanged(context, tupleElement)) + { + return; + } + + var diagnosticProperties = ImmutableDictionary.CreateBuilder(); + + if (prepareCodeFix) + { + var fixedName = fixedFirstChar + tupleElementName.Substring(1); + diagnosticProperties.Add(ExpectedTupleElementNameKey, fixedName); + } + + context.ReportDiagnostic(Diagnostic.Create(Descriptor, location, diagnosticProperties.ToImmutableDictionary())); + } - if (prepareCodeFix) + /// + /// When overriding a base class or implementing an interface, the compiler requires the names to match + /// the original definition. This method checks if we are allowed to change the name of the specified tuple element. + /// + private static bool CanTupleElementNameBeChanged(SyntaxNodeAnalysisContext context, SyntaxNode tupleElement) + { + var memberDeclaration = GetAncestorMemberDeclaration(tupleElement); + if (memberDeclaration == null) + { + return true; + } + + if (IsOverrideOrExplicitInterfaceImplementation(memberDeclaration)) + { + return false; + } + + var symbol = context.SemanticModel.GetDeclaredSymbol(memberDeclaration); + if (NamedTypeHelpers.IsImplementingAnInterfaceMember(symbol)) + { + return false; + } + + return true; + } + + /// + /// Returns the ancestor , if the node is part of its "signature", + /// otherwise returns null. + /// + private static MemberDeclarationSyntax GetAncestorMemberDeclaration(SyntaxNode node) + { + // NOTE: Avoiding a simple FirstAncestorOrSelf() call here, since + // that would also return true for e.g. tuple variable declarations inside a method body. + while (node != null) + { + switch (node.Kind()) { - var fixedName = fixedFirstChar + tupleElementName.Substring(1); - diagnosticProperties.Add(ExpectedTupleElementNameKey, fixedName); + case SyntaxKind.MethodDeclaration: + case SyntaxKind.PropertyDeclaration: + case SyntaxKind.EventDeclaration: + case SyntaxKind.IndexerDeclaration: + return (MemberDeclarationSyntax)node; + + case SyntaxKind.Parameter: + case SyntaxKind.ParameterList: + case SyntaxKindEx.TupleElement: + case SyntaxKind.TypeArgumentList: + case SyntaxKind when node is TypeSyntax: + node = node.Parent; + break; + + default: + return null; } + } + + return null; + } - context.ReportDiagnostic(Diagnostic.Create(Descriptor, location, diagnosticProperties.ToImmutableDictionary())); + private static bool IsOverrideOrExplicitInterfaceImplementation(MemberDeclarationSyntax memberDeclaration) + { + bool result; + + switch (memberDeclaration.Kind()) + { + case SyntaxKind.MethodDeclaration: + var methodDeclaration = (MethodDeclarationSyntax)memberDeclaration; + result = + methodDeclaration.Modifiers.Any(SyntaxKind.OverrideKeyword) || + methodDeclaration.ExplicitInterfaceSpecifier != null; + break; + + case SyntaxKind.PropertyDeclaration: + case SyntaxKind.EventDeclaration: + case SyntaxKind.IndexerDeclaration: + var basePropertyDeclaration = (BasePropertyDeclarationSyntax)memberDeclaration; + result = + basePropertyDeclaration.Modifiers.Any(SyntaxKind.OverrideKeyword) || + basePropertyDeclaration.ExplicitInterfaceSpecifier != null; + break; + + default: + Debug.Assert(false, $"Unhandled type {memberDeclaration.Kind()}"); + result = false; + break; } + + return result; } } }