Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Update SA1316 to not trigger in tuple types which are part of a membe…
Browse files Browse the repository at this point in the history
…r's signature, if that member is an override or implements an interface

DotNetAnalyzers#3781
bjornhellander committed Mar 23, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
1 parent c9debeb commit 7ea2d3f
Showing 2 changed files with 518 additions and 9 deletions.
Original file line number Diff line number Diff line change
@@ -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<object[]> 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)>" },
};

/// <summary>
/// Validates the properly named tuple element names will not produce diagnostics.
/// </summary>
@@ -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);
}
}
}
Original file line number Diff line number Diff line change
@@ -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<string, string>();
return;
}

if (!CanTupleElementNameBeChanged(context, tupleElement))
{
return;
}

var diagnosticProperties = ImmutableDictionary.CreateBuilder<string, string>();

if (prepareCodeFix)
{
var fixedName = fixedFirstChar + tupleElementName.Substring(1);
diagnosticProperties.Add(ExpectedTupleElementNameKey, fixedName);
}

context.ReportDiagnostic(Diagnostic.Create(Descriptor, location, diagnosticProperties.ToImmutableDictionary()));
}

if (prepareCodeFix)
/// <summary>
/// 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.
/// </summary>
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;
}

/// <summary>
/// Returns the ancestor <see cref="MemberDeclarationSyntax"/>, if the node is part of its "signature",
/// otherwise returns null.
/// </summary>
private static MemberDeclarationSyntax GetAncestorMemberDeclaration(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())
{
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;
}
}
}

0 comments on commit 7ea2d3f

Please sign in to comment.