From e4141f19604a3c5c8953cbfffa69d471bb86e6bc Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Tue, 4 Jun 2024 17:15:00 +0200 Subject: [PATCH 01/33] Fix via semantic model type check --- ...dsShouldNotHaveIdenticalImplementations.cs | 4 ++-- ...ouldNotHaveIdenticalImplementationsBase.cs | 21 ++++++++++++------- ...dsShouldNotHaveIdenticalImplementations.cs | 4 ++-- ...NotHaveIdenticalImplementations.CSharp9.cs | 4 ++-- 4 files changed, 20 insertions(+), 13 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/MethodsShouldNotHaveIdenticalImplementations.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/MethodsShouldNotHaveIdenticalImplementations.cs index c661d761922..07dc6a1adee 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/MethodsShouldNotHaveIdenticalImplementations.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/MethodsShouldNotHaveIdenticalImplementations.cs @@ -40,10 +40,10 @@ protected override IEnumerable GetMethodDeclarations(SyntaxN ? ((CompilationUnitSyntax)node).GetMethodDeclarations() : ((TypeDeclarationSyntax)node).GetMethodDeclarations(); - protected override bool AreDuplicates(IMethodDeclaration firstMethod, IMethodDeclaration secondMethod) => + protected override bool AreDuplicates(SemanticModel model, IMethodDeclaration firstMethod, IMethodDeclaration secondMethod) => firstMethod is { Body: { Statements: { Count: > 1 } } } && firstMethod.Identifier.ValueText != secondMethod.Identifier.ValueText - && HaveSameParameters(firstMethod.ParameterList.Parameters, secondMethod.ParameterList.Parameters) + && HaveSameParameters(model, firstMethod.ParameterList.Parameters, secondMethod.ParameterList.Parameters) && firstMethod.Body.IsEquivalentTo(secondMethod.Body, false); protected override SyntaxToken GetMethodIdentifier(IMethodDeclaration method) => diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs index 42596e50c4b..3223c086ae5 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs @@ -29,7 +29,7 @@ public abstract class MethodsShouldNotHaveIdenticalImplementationsBase GetMethodDeclarations(SyntaxNode node); protected abstract SyntaxToken GetMethodIdentifier(TMethodDeclarationSyntax method); - protected abstract bool AreDuplicates(TMethodDeclarationSyntax firstMethod, TMethodDeclarationSyntax secondMethod); + protected abstract bool AreDuplicates(SemanticModel model, TMethodDeclarationSyntax firstMethod, TMethodDeclarationSyntax secondMethod); protected override string MessageFormat => "Update this method so that its implementation is not identical to '{0}'."; @@ -50,7 +50,7 @@ protected override void Initialize(SonarAnalysisContext context) => { var method = methodsToHandle.First.Value; methodsToHandle.RemoveFirst(); - var duplicates = methodsToHandle.Where(x => AreDuplicates(method, x)).ToList(); + var duplicates = methodsToHandle.Where(x => AreDuplicates(c.SemanticModel, method, x)).ToList(); foreach (var duplicate in duplicates) { @@ -64,12 +64,19 @@ protected override void Initialize(SonarAnalysisContext context) => protected virtual bool IsExcludedFromBeingExamined(SonarSyntaxNodeReportingContext context) => context.ContainingSymbol.Kind != SymbolKind.NamedType; - protected static bool HaveSameParameters(SeparatedSyntaxList? leftParameters, SeparatedSyntaxList? rightParameters) + protected static bool HaveSameParameters(SemanticModel model, SeparatedSyntaxList? leftParameters, SeparatedSyntaxList? rightParameters) where TSyntax : SyntaxNode => - (leftParameters == null && rightParameters == null) - || (leftParameters != null - && rightParameters != null + (leftParameters is null && rightParameters is null) + || (leftParameters is not null + && rightParameters is not null && leftParameters.Value.Count == rightParameters.Value.Count - && leftParameters.Value.Select((left, index) => left.IsEquivalentTo(rightParameters.Value[index])).All(x => x)); + && (leftParameters.Value.Count == 0 + || (leftParameters.Value.Zip(rightParameters.Value, (left, right) => left.IsEquivalentTo(right)).All(x => x) + && leftParameters.Value.Zip(rightParameters.Value, (left, right) => HaveSameParameterType(model, left, right)).Any(x => x)))); + + private static bool HaveSameParameterType(SemanticModel model, SyntaxNode left, SyntaxNode right) => + model.GetDeclaredSymbol(left) is IParameterSymbol { Type: { } leftParameterType } + && model.GetDeclaredSymbol(right) is IParameterSymbol { Type: { } rightParameterType } + && leftParameterType.Equals(rightParameterType); } } diff --git a/analyzers/src/SonarAnalyzer.VisualBasic/Rules/MethodsShouldNotHaveIdenticalImplementations.cs b/analyzers/src/SonarAnalyzer.VisualBasic/Rules/MethodsShouldNotHaveIdenticalImplementations.cs index 512fbaf115e..d7426844620 100644 --- a/analyzers/src/SonarAnalyzer.VisualBasic/Rules/MethodsShouldNotHaveIdenticalImplementations.cs +++ b/analyzers/src/SonarAnalyzer.VisualBasic/Rules/MethodsShouldNotHaveIdenticalImplementations.cs @@ -30,10 +30,10 @@ public sealed class MethodsShouldNotHaveIdenticalImplementations : MethodsShould protected override IEnumerable GetMethodDeclarations(SyntaxNode node) => ((TypeBlockSyntax)node).Members.OfType(); - protected override bool AreDuplicates(MethodBlockSyntax firstMethod, MethodBlockSyntax secondMethod) => + protected override bool AreDuplicates(SemanticModel model, MethodBlockSyntax firstMethod, MethodBlockSyntax secondMethod) => firstMethod.Statements.Count > 1 && firstMethod.GetIdentifierText() != secondMethod.GetIdentifierText() - && HaveSameParameters(firstMethod.GetParameters(), secondMethod.GetParameters()) + && HaveSameParameters(model, firstMethod.GetParameters(), secondMethod.GetParameters()) && VisualBasicEquivalenceChecker.AreEquivalent(firstMethod.Statements, secondMethod.Statements); protected override SyntaxToken GetMethodIdentifier(MethodBlockSyntax method) => diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/MethodsShouldNotHaveIdenticalImplementations.CSharp9.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/MethodsShouldNotHaveIdenticalImplementations.CSharp9.cs index f9fbec5f2d2..32895b67880 100644 --- a/analyzers/tests/SonarAnalyzer.Test/TestCases/MethodsShouldNotHaveIdenticalImplementations.CSharp9.cs +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/MethodsShouldNotHaveIdenticalImplementations.CSharp9.cs @@ -95,13 +95,13 @@ public static class TypeConstraints public static int Use(T? value) where T : class => 2; - public static void First(T? value) where T : struct // Secondary + public static void First(T? value) where T : struct { var x = Use(value); Console.WriteLine(x); } - public static void Second(T? value) where T : class // Noncompliant - FP, method looks the same but different overloads are called due to the type constraints used. See: https://github.com/SonarSource/sonar-dotnet/issues/7068 + public static void Second(T? value) where T : class // Compliant, method looks the same but different overloads are called due to the type constraints used. See: https://github.com/SonarSource/sonar-dotnet/issues/7068 { var x = Use(value); Console.WriteLine(x); From 3fc4b3e0775751dd81864d509f820997c069b954 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Tue, 4 Jun 2024 17:23:37 +0200 Subject: [PATCH 02/33] Refactor --- ...ouldNotHaveIdenticalImplementationsBase.cs | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs index 3223c086ae5..79c825ab095 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs @@ -18,6 +18,8 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ +using Google.Protobuf.WellKnownTypes; + namespace SonarAnalyzer.Rules { public abstract class MethodsShouldNotHaveIdenticalImplementationsBase : SonarDiagnosticAnalyzer @@ -66,13 +68,17 @@ protected virtual bool IsExcludedFromBeingExamined(SonarSyntaxNodeReportingConte protected static bool HaveSameParameters(SemanticModel model, SeparatedSyntaxList? leftParameters, SeparatedSyntaxList? rightParameters) where TSyntax : SyntaxNode => - (leftParameters is null && rightParameters is null) - || (leftParameters is not null - && rightParameters is not null - && leftParameters.Value.Count == rightParameters.Value.Count - && (leftParameters.Value.Count == 0 - || (leftParameters.Value.Zip(rightParameters.Value, (left, right) => left.IsEquivalentTo(right)).All(x => x) - && leftParameters.Value.Zip(rightParameters.Value, (left, right) => HaveSameParameterType(model, left, right)).Any(x => x)))); + (leftParameters is null && rightParameters is null) // VB.Net + || (leftParameters is { Count: { } leftCount } leftParams + && rightParameters is { Count: { } rightCount } rightParams + && leftCount == rightCount + && (leftCount == 0 || HaveSameParameterLists(model, leftParams, rightParams))); + + private static bool HaveSameParameterLists(SemanticModel model, + SeparatedSyntaxList leftParameters, + SeparatedSyntaxList rightParameters) where TSyntax : SyntaxNode => + leftParameters.Zip(rightParameters, (left, right) => left.IsEquivalentTo(right)).All(x => x) // Perf: Syntactic equivalence for all parameters first + && leftParameters.Zip(rightParameters, (left, right) => HaveSameParameterType(model, left, right)).Any(x => x); // Also make sure the parameter types are same private static bool HaveSameParameterType(SemanticModel model, SyntaxNode left, SyntaxNode right) => model.GetDeclaredSymbol(left) is IParameterSymbol { Type: { } leftParameterType } From baf9a5f403d7eab5e98c4f45334fcf5ac479bc17 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Tue, 4 Jun 2024 17:24:52 +0200 Subject: [PATCH 03/33] Fix --- .../Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs index 79c825ab095..388fbc712c8 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs @@ -78,7 +78,7 @@ private static bool HaveSameParameterLists(SemanticModel model, SeparatedSyntaxList leftParameters, SeparatedSyntaxList rightParameters) where TSyntax : SyntaxNode => leftParameters.Zip(rightParameters, (left, right) => left.IsEquivalentTo(right)).All(x => x) // Perf: Syntactic equivalence for all parameters first - && leftParameters.Zip(rightParameters, (left, right) => HaveSameParameterType(model, left, right)).Any(x => x); // Also make sure the parameter types are same + && leftParameters.Zip(rightParameters, (left, right) => HaveSameParameterType(model, left, right)).All(x => x); // Also make sure the parameter types are same private static bool HaveSameParameterType(SemanticModel model, SyntaxNode left, SyntaxNode right) => model.GetDeclaredSymbol(left) is IParameterSymbol { Type: { } leftParameterType } From 3b3d68f7c941745af1f0af0e6d2ba2254fe57468 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Tue, 4 Jun 2024 17:29:25 +0200 Subject: [PATCH 04/33] Unsued using --- .../Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs index 388fbc712c8..fcf07a3992a 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs @@ -18,8 +18,6 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -using Google.Protobuf.WellKnownTypes; - namespace SonarAnalyzer.Rules { public abstract class MethodsShouldNotHaveIdenticalImplementationsBase : SonarDiagnosticAnalyzer From 017049713c3992a6ae920d8b26c9086857ac4248 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Wed, 5 Jun 2024 11:22:30 +0200 Subject: [PATCH 05/33] Add proper support for type parameters and add more tests --- ...ouldNotHaveIdenticalImplementationsBase.cs | 125 ++++++++------ ...ouldNotHaveIdenticalImplementationsTest.cs | 160 +++++++++++++++--- ...NotHaveIdenticalImplementations.CSharp9.cs | 21 +++ 3 files changed, 230 insertions(+), 76 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs index fcf07a3992a..ea7226dc91c 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs @@ -18,69 +18,86 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -namespace SonarAnalyzer.Rules +namespace SonarAnalyzer.Rules; + +public abstract class MethodsShouldNotHaveIdenticalImplementationsBase : SonarDiagnosticAnalyzer + where TSyntaxKind : struct { - public abstract class MethodsShouldNotHaveIdenticalImplementationsBase : SonarDiagnosticAnalyzer - where TSyntaxKind : struct - { - private const string DiagnosticId = "S4144"; + private const string DiagnosticId = "S4144"; - protected abstract TSyntaxKind[] SyntaxKinds { get; } + protected abstract TSyntaxKind[] SyntaxKinds { get; } - protected abstract IEnumerable GetMethodDeclarations(SyntaxNode node); - protected abstract SyntaxToken GetMethodIdentifier(TMethodDeclarationSyntax method); - protected abstract bool AreDuplicates(SemanticModel model, TMethodDeclarationSyntax firstMethod, TMethodDeclarationSyntax secondMethod); + protected abstract IEnumerable GetMethodDeclarations(SyntaxNode node); + protected abstract SyntaxToken GetMethodIdentifier(TMethodDeclarationSyntax method); + protected abstract bool AreDuplicates(SemanticModel model, TMethodDeclarationSyntax firstMethod, TMethodDeclarationSyntax secondMethod); - protected override string MessageFormat => "Update this method so that its implementation is not identical to '{0}'."; + protected override string MessageFormat => "Update this method so that its implementation is not identical to '{0}'."; - protected MethodsShouldNotHaveIdenticalImplementationsBase() : base(DiagnosticId) { } + protected MethodsShouldNotHaveIdenticalImplementationsBase() : base(DiagnosticId) { } - protected override void Initialize(SonarAnalysisContext context) => - context.RegisterNodeAction(Language.GeneratedCodeRecognizer, - c => + protected override void Initialize(SonarAnalysisContext context) => + context.RegisterNodeAction(Language.GeneratedCodeRecognizer, + c => + { + if (IsExcludedFromBeingExamined(c)) { - if (IsExcludedFromBeingExamined(c)) - { - return; - } + return; + } - var methodsToHandle = new LinkedList(GetMethodDeclarations(c.Node)); + var methodsToHandle = new LinkedList(GetMethodDeclarations(c.Node)); - while (methodsToHandle.Any()) + while (methodsToHandle.Any()) + { + var method = methodsToHandle.First.Value; + methodsToHandle.RemoveFirst(); + var duplicates = methodsToHandle.Where(x => AreDuplicates(c.SemanticModel, method, x)).ToList(); + + foreach (var duplicate in duplicates) { - var method = methodsToHandle.First.Value; - methodsToHandle.RemoveFirst(); - var duplicates = methodsToHandle.Where(x => AreDuplicates(c.SemanticModel, method, x)).ToList(); - - foreach (var duplicate in duplicates) - { - methodsToHandle.Remove(duplicate); - c.ReportIssue(SupportedDiagnostics[0], GetMethodIdentifier(duplicate), [GetMethodIdentifier(method).ToSecondaryLocation()], GetMethodIdentifier(method).ValueText); - } + methodsToHandle.Remove(duplicate); + c.ReportIssue(SupportedDiagnostics[0], GetMethodIdentifier(duplicate), [GetMethodIdentifier(method).ToSecondaryLocation()], GetMethodIdentifier(method).ValueText); } - }, - SyntaxKinds); - - protected virtual bool IsExcludedFromBeingExamined(SonarSyntaxNodeReportingContext context) => - context.ContainingSymbol.Kind != SymbolKind.NamedType; - - protected static bool HaveSameParameters(SemanticModel model, SeparatedSyntaxList? leftParameters, SeparatedSyntaxList? rightParameters) - where TSyntax : SyntaxNode => - (leftParameters is null && rightParameters is null) // VB.Net - || (leftParameters is { Count: { } leftCount } leftParams - && rightParameters is { Count: { } rightCount } rightParams - && leftCount == rightCount - && (leftCount == 0 || HaveSameParameterLists(model, leftParams, rightParams))); - - private static bool HaveSameParameterLists(SemanticModel model, - SeparatedSyntaxList leftParameters, - SeparatedSyntaxList rightParameters) where TSyntax : SyntaxNode => - leftParameters.Zip(rightParameters, (left, right) => left.IsEquivalentTo(right)).All(x => x) // Perf: Syntactic equivalence for all parameters first - && leftParameters.Zip(rightParameters, (left, right) => HaveSameParameterType(model, left, right)).All(x => x); // Also make sure the parameter types are same - - private static bool HaveSameParameterType(SemanticModel model, SyntaxNode left, SyntaxNode right) => - model.GetDeclaredSymbol(left) is IParameterSymbol { Type: { } leftParameterType } - && model.GetDeclaredSymbol(right) is IParameterSymbol { Type: { } rightParameterType } - && leftParameterType.Equals(rightParameterType); - } + } + }, + SyntaxKinds); + + protected virtual bool IsExcludedFromBeingExamined(SonarSyntaxNodeReportingContext context) => + context.ContainingSymbol.Kind != SymbolKind.NamedType; + + protected static bool HaveSameParameters(SemanticModel model, SeparatedSyntaxList? leftParameters, SeparatedSyntaxList? rightParameters) + where TSyntax : SyntaxNode => + (leftParameters is null && rightParameters is null) // In VB.Net the parameter list can be omitted + || (leftParameters?.Count == rightParameters?.Count && HaveSameParameterLists(model, leftParameters.Value, rightParameters.Value)); + + private static bool HaveSameParameterLists(SemanticModel model, + SeparatedSyntaxList leftParameters, + SeparatedSyntaxList rightParameters) where TSyntax : SyntaxNode => + leftParameters.Zip(rightParameters, (left, right) => left.IsEquivalentTo(right)).All(x => x) // Perf: Syntactic equivalence for all parameters first + && leftParameters.Zip(rightParameters, (left, right) => HaveSameParameterType(model, left, right)).All(x => x); // Also make sure the parameter types are same + + private static bool HaveSameParameterType(SemanticModel model, SyntaxNode left, SyntaxNode right) => + model.GetDeclaredSymbol(left) is IParameterSymbol { Type: { } leftParameterType } + && model.GetDeclaredSymbol(right) is IParameterSymbol { Type: { } rightParameterType } + && TypesAreSame(leftParameterType, rightParameterType); + + private static bool TypesAreSame(ITypeSymbol leftParameterType, ITypeSymbol rightParameterType) => + leftParameterType.Equals(rightParameterType) // M1(int x) <-> M2(int x) + || TypesAreSameTypeParameters(leftParameterType, rightParameterType) // M1(T x) where T: class <-> M2(T x) where T: class + || TypesAreSameGenericType(leftParameterType, rightParameterType); // M1(IEnumerable x) where T: class <-> M2(IEnumerable x) where T: class + + private static bool TypesAreSameGenericType(ITypeSymbol leftParameterType, ITypeSymbol rightParameterType) => + leftParameterType is INamedTypeSymbol { IsGenericType: true } namedTypeLeft + && rightParameterType is INamedTypeSymbol { IsGenericType: true } namedTypeRight + && namedTypeLeft.TypeParameters.Length == namedTypeRight.TypeParameters.Length + && namedTypeLeft.TypeParameters.Zip(namedTypeRight.TypeParameters, (left, right) => TypesAreSame(left, right)).All(x => x); + + private static bool TypesAreSameTypeParameters(ITypeSymbol leftParameterType, ITypeSymbol rightParameterType) => + leftParameterType is ITypeParameterSymbol left + && rightParameterType is ITypeParameterSymbol right + && left.HasConstructorConstraint == right.HasConstructorConstraint + && left.HasReferenceTypeConstraint == right.HasReferenceTypeConstraint + && left.HasValueTypeConstraint == right.HasValueTypeConstraint + && left.HasUnmanagedTypeConstraint() == right.HasUnmanagedTypeConstraint() + && left.ConstraintTypes.Length == right.ConstraintTypes.Length + && left.ConstraintTypes.All(x => right.ConstraintTypes.Any(y => TypesAreSame(x, y))); } diff --git a/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs b/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs index bc19e086790..8ab0646541e 100644 --- a/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs +++ b/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs @@ -22,36 +22,152 @@ using CS = SonarAnalyzer.Rules.CSharp; using VB = SonarAnalyzer.Rules.VisualBasic; -namespace SonarAnalyzer.Test.Rules +namespace SonarAnalyzer.Test.Rules; + +[TestClass] +public class MethodsShouldNotHaveIdenticalImplementationsTest { - [TestClass] - public class MethodsShouldNotHaveIdenticalImplementationsTest - { - private readonly VerifierBuilder builderCS = new VerifierBuilder(); - private readonly VerifierBuilder builderVB = new VerifierBuilder(); + private readonly VerifierBuilder builderCS = new VerifierBuilder(); + private readonly VerifierBuilder builderVB = new VerifierBuilder(); + + [TestMethod] + public void MethodsShouldNotHaveIdenticalImplementations() => + builderCS.AddPaths("MethodsShouldNotHaveIdenticalImplementations.cs").WithOptions(ParseOptionsHelper.FromCSharp8).Verify(); + + [DataTestMethod] + [DataRow("", "")] + [DataRow("where T: struct", "where T: struct")] + [DataRow("where T: class", "where T: class")] + [DataRow("where T: unmanaged", "where T: unmanaged")] + [DataRow("where T: new()", "where T: new()")] + [DataRow("where T: IEquatable, IComparable", "where T: System.IComparable, IEquatable")] + public void MethodsShouldNotHaveIdenticalImplementations_MethodTypeParameters_NonCompliant(string constraint1, string constraint2) => + builderCS.AddSnippet($$""" + using System; + public static class TypeConstraints + { + public static bool Compare1(T? value1, T value2) {{constraint1}} // Secondary + { + Console.WriteLine(value1); + Console.WriteLine(value2); + return true; + } + + public static bool Compare2(T? value1, T value2) {{constraint2}} // Noncompliant + { + Console.WriteLine(value1); + Console.WriteLine(value2); + return true; + } + } + """).WithOptions(ParseOptionsHelper.FromCSharp9).Verify(); + + [TestMethod] + [CombinatorialData] + public void MethodsShouldNotHaveIdenticalImplementations_MethodTypeParameters_Compliant( + [DataValues("where T: struct", "where T: class")] string constraint1, + [DataValues("", "where T: unmanaged", "where T: new()")] string constraint2) => + builderCS.AddSnippet($$""" + using System; + public static class TypeConstraints + { + public static bool Compare1(T? value1, T value2) {{constraint1}} + { + Console.WriteLine(value1); + Console.WriteLine(value2); + return true; + } + + public static bool Compare2(T? value1, T value2) {{constraint2}} + { + Console.WriteLine(value1); + Console.WriteLine(value2); + return true; + } + } + """).WithOptions(ParseOptionsHelper.FromCSharp9).VerifyNoIssues(); + + [DataTestMethod] + [DataRow("", "")] + [DataRow("where TKey: struct", "where TKey: struct")] + [DataRow("where TKey: struct where TValue: class", "where TKey: struct where TValue: class")] + [DataRow("where TValue: class where TKey: struct", "where TKey: struct where TValue: class")] + [DataRow("where TKey: class", "where TKey: class")] + [DataRow("where TKey: unmanaged", "where TKey: unmanaged")] + [DataRow("where TKey: new()", "where TKey: new()")] + [DataRow("where TKey: IEquatable, IComparable", "where TKey: System.IComparable, IEquatable")] + [DataRow("where TKey: IEquatable, IComparable where TValue: IComparable", " where TValue: System.IComparable where TKey: System.IComparable, IEquatable")] + public void MethodsShouldNotHaveIdenticalImplementations_MethodTypeParameters_Dictionary_NonCompliant(string constraint1, string constraint2) => + builderCS.AddSnippet($$""" + using System; + using System.Collections.Generic; + public static class TypeConstraints + { + public static bool Test1(IDictionary dict) {{constraint1}} // Secondary + { + Console.WriteLine(dict); + Console.WriteLine(dict); + return true; + } + + public static bool Test2(IDictionary dict) {{constraint1}} // Noncompliant + { + Console.WriteLine(dict); + Console.WriteLine(dict); + return true; + } + } + """).WithOptions(ParseOptionsHelper.FromCSharp9).Verify(); + + [DataTestMethod] + [DataRow("")] + [DataRow("where TKey: struct")] + [DataRow("where TKey: struct where TValue: class")] + [DataRow("where TValue: class where TKey: struct")] + [DataRow("where TKey: class")] + [DataRow("where TKey: unmanaged")] + [DataRow("where TKey: new()")] + [DataRow("where TKey: IEquatable, IComparable")] + [DataRow("where TKey: IEquatable, IComparable where TValue: IComparable")] + public void MethodsShouldNotHaveIdenticalImplementations_ClassTypeParameters_Dictionary_NonCompliant(string constraint) => + builderCS.AddSnippet($$""" + using System; + using System.Collections.Generic; + public class TypeConstraints {{constraint}} + { + public static bool Test1(IDictionary dict) // Secondary + { + Console.WriteLine(dict); + Console.WriteLine(dict); + return true; + } - [TestMethod] - public void MethodsShouldNotHaveIdenticalImplementations() => - builderCS.AddPaths("MethodsShouldNotHaveIdenticalImplementations.cs").WithOptions(ParseOptionsHelper.FromCSharp8).Verify(); + public static bool Test2(IDictionary dict) // Noncompliant + { + Console.WriteLine(dict); + Console.WriteLine(dict); + return true; + } + } + """).WithOptions(ParseOptionsHelper.FromCSharp9).Verify(); #if NET - [TestMethod] - public void MethodsShouldNotHaveIdenticalImplementations_CSharp9() => - builderCS.AddPaths("MethodsShouldNotHaveIdenticalImplementations.CSharp9.cs").WithTopLevelStatements().Verify(); + [TestMethod] + public void MethodsShouldNotHaveIdenticalImplementations_CSharp9() => + builderCS.AddPaths("MethodsShouldNotHaveIdenticalImplementations.CSharp9.cs").WithTopLevelStatements().Verify(); - [TestMethod] - public void MethodsShouldNotHaveIdenticalImplementations_CSharp10() => - builderCS.AddPaths("MethodsShouldNotHaveIdenticalImplementations.CSharp10.cs").WithLanguageVersion(LanguageVersion.CSharp10).Verify(); + [TestMethod] + public void MethodsShouldNotHaveIdenticalImplementations_CSharp10() => + builderCS.AddPaths("MethodsShouldNotHaveIdenticalImplementations.CSharp10.cs").WithLanguageVersion(LanguageVersion.CSharp10).Verify(); - [TestMethod] - public void MethodsShouldNotHaveIdenticalImplementations_CSharp11() => - builderCS.AddPaths("MethodsShouldNotHaveIdenticalImplementations.CSharp11.cs").WithLanguageVersion(LanguageVersion.CSharp11).Verify(); + [TestMethod] + public void MethodsShouldNotHaveIdenticalImplementations_CSharp11() => + builderCS.AddPaths("MethodsShouldNotHaveIdenticalImplementations.CSharp11.cs").WithLanguageVersion(LanguageVersion.CSharp11).Verify(); #endif - [TestMethod] - public void MethodsShouldNotHaveIdenticalImplementations_VB() => - builderVB.AddPaths("MethodsShouldNotHaveIdenticalImplementations.vb").Verify(); - } + [TestMethod] + public void MethodsShouldNotHaveIdenticalImplementations_VB() => + builderVB.AddPaths("MethodsShouldNotHaveIdenticalImplementations.vb").Verify(); } diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/MethodsShouldNotHaveIdenticalImplementations.CSharp9.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/MethodsShouldNotHaveIdenticalImplementations.CSharp9.cs index 32895b67880..940c616b7cb 100644 --- a/analyzers/tests/SonarAnalyzer.Test/TestCases/MethodsShouldNotHaveIdenticalImplementations.CSharp9.cs +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/MethodsShouldNotHaveIdenticalImplementations.CSharp9.cs @@ -106,4 +106,25 @@ public static void Second(T? value) where T : class // Compliant, method loo var x = Use(value); Console.WriteLine(x); } + + public static bool Compare1(T? value1, T value2) // Secondary + { + Console.WriteLine(value1); + Console.WriteLine(value2); + return true; + } + + public static bool Compare2(T? value1, T value2) // Noncompliant + { + Console.WriteLine(value1); + Console.WriteLine(value2); + return true; + } + + public static bool Compare3(T? value1, T value2) where T : System.IComparable // Compliant. Parameter type constraints don't match + { + Console.WriteLine(value1); + Console.WriteLine(value2); + return true; + } } From 798bb24ce790e0f369b4c235702e1463d10369a1 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Wed, 5 Jun 2024 11:31:17 +0200 Subject: [PATCH 06/33] Better comments --- .../MethodsShouldNotHaveIdenticalImplementationsBase.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs index ea7226dc91c..8b1d72c0ce8 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs @@ -81,9 +81,10 @@ private static bool HaveSameParameterType(SemanticModel model, SyntaxNode left, && TypesAreSame(leftParameterType, rightParameterType); private static bool TypesAreSame(ITypeSymbol leftParameterType, ITypeSymbol rightParameterType) => - leftParameterType.Equals(rightParameterType) // M1(int x) <-> M2(int x) + leftParameterType.Equals(rightParameterType) // M1(int x) <-> M2(int x) || TypesAreSameTypeParameters(leftParameterType, rightParameterType) // M1(T x) where T: class <-> M2(T x) where T: class - || TypesAreSameGenericType(leftParameterType, rightParameterType); // M1(IEnumerable x) where T: class <-> M2(IEnumerable x) where T: class + // T of M1 is a different symbol than T of M2, but if they have the same constraints, they can be interchanged + || TypesAreSameGenericType(leftParameterType, rightParameterType); // M1(IEnumerable x) where T: class <-> M2(IEnumerable x) where T: class private static bool TypesAreSameGenericType(ITypeSymbol leftParameterType, ITypeSymbol rightParameterType) => leftParameterType is INamedTypeSymbol { IsGenericType: true } namedTypeLeft From f7cc661a4708b538f7d5b3e57049cb7648019b95 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Wed, 5 Jun 2024 11:38:03 +0200 Subject: [PATCH 07/33] Test improvements --- .../Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs | 2 +- .../MethodsShouldNotHaveIdenticalImplementations.CSharp9.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs b/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs index 8ab0646541e..bbe570cf48b 100644 --- a/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs +++ b/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs @@ -110,7 +110,7 @@ public static bool Test1(IDictionary dict) {{constra return true; } - public static bool Test2(IDictionary dict) {{constraint1}} // Noncompliant + public static bool Test2(IDictionary dict) {{constraint1}} // Noncompliant { Console.WriteLine(dict); Console.WriteLine(dict); diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/MethodsShouldNotHaveIdenticalImplementations.CSharp9.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/MethodsShouldNotHaveIdenticalImplementations.CSharp9.cs index 940c616b7cb..f2a9e783858 100644 --- a/analyzers/tests/SonarAnalyzer.Test/TestCases/MethodsShouldNotHaveIdenticalImplementations.CSharp9.cs +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/MethodsShouldNotHaveIdenticalImplementations.CSharp9.cs @@ -101,7 +101,7 @@ public static void First(T? value) where T : struct Console.WriteLine(x); } - public static void Second(T? value) where T : class // Compliant, method looks the same but different overloads are called due to the type constraints used. See: https://github.com/SonarSource/sonar-dotnet/issues/7068 + public static void Second(T? value) where T : class // Compliant, method looks the same but different overloads are called due to the type constraints used. { var x = Use(value); Console.WriteLine(x); From 08d5da47f741f0650fa40754319c29ef189e7bb4 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Wed, 5 Jun 2024 11:44:35 +0200 Subject: [PATCH 08/33] fix test --- .../Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs b/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs index bbe570cf48b..653af7f8ee1 100644 --- a/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs +++ b/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs @@ -110,7 +110,7 @@ public static bool Test1(IDictionary dict) {{constra return true; } - public static bool Test2(IDictionary dict) {{constraint1}} // Noncompliant + public static bool Test2(IDictionary dict) {{constraint2}} // Noncompliant { Console.WriteLine(dict); Console.WriteLine(dict); From fccc74d92247c9b49e8cf61cb3fffe07a96029db Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Wed, 5 Jun 2024 11:46:04 +0200 Subject: [PATCH 09/33] Make CombinatorialDataAttribute derive from TestMethodAttribute --- .../Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs | 1 - .../Common/CombinatorialDataAttribute.cs | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs b/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs index 653af7f8ee1..adb51a0e022 100644 --- a/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs +++ b/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs @@ -62,7 +62,6 @@ public static bool Compare2(T? value1, T value2) {{constraint2}} // Noncompli } """).WithOptions(ParseOptionsHelper.FromCSharp9).Verify(); - [TestMethod] [CombinatorialData] public void MethodsShouldNotHaveIdenticalImplementations_MethodTypeParameters_Compliant( [DataValues("where T: struct", "where T: class")] string constraint1, diff --git a/analyzers/tests/SonarAnalyzer.TestFramework/Common/CombinatorialDataAttribute.cs b/analyzers/tests/SonarAnalyzer.TestFramework/Common/CombinatorialDataAttribute.cs index 739146e06dc..28409a5dec2 100644 --- a/analyzers/tests/SonarAnalyzer.TestFramework/Common/CombinatorialDataAttribute.cs +++ b/analyzers/tests/SonarAnalyzer.TestFramework/Common/CombinatorialDataAttribute.cs @@ -35,7 +35,7 @@ public DataValuesAttribute(params object[] values) } [AttributeUsage(AttributeTargets.Method)] -public sealed class CombinatorialDataAttribute : Attribute, ITestDataSource +public sealed class CombinatorialDataAttribute : TestMethodAttribute, ITestDataSource { public IEnumerable GetData(MethodInfo methodInfo) { From c37817180a0aa688b3fdd64998e46500acf51be8 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Thu, 6 Jun 2024 16:38:56 +0200 Subject: [PATCH 10/33] wip: Fix.. --- ...dsShouldNotHaveIdenticalImplementations.cs | 3 +- ...ouldNotHaveIdenticalImplementationsBase.cs | 72 ++++++++++++------- ...ouldNotHaveIdenticalImplementationsTest.cs | 33 +++++++++ 3 files changed, 82 insertions(+), 26 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/MethodsShouldNotHaveIdenticalImplementations.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/MethodsShouldNotHaveIdenticalImplementations.cs index 07dc6a1adee..54974a6ebd7 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/MethodsShouldNotHaveIdenticalImplementations.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/MethodsShouldNotHaveIdenticalImplementations.cs @@ -43,7 +43,8 @@ protected override IEnumerable GetMethodDeclarations(SyntaxN protected override bool AreDuplicates(SemanticModel model, IMethodDeclaration firstMethod, IMethodDeclaration secondMethod) => firstMethod is { Body: { Statements: { Count: > 1 } } } && firstMethod.Identifier.ValueText != secondMethod.Identifier.ValueText - && HaveSameParameters(model, firstMethod.ParameterList.Parameters, secondMethod.ParameterList.Parameters) + && HaveSameParameters(model, firstMethod.ParameterList?.Parameters, secondMethod.ParameterList?.Parameters) + && HaveSameTypeParameters(model, firstMethod.TypeParameterList?.Parameters, secondMethod.TypeParameterList?.Parameters) && firstMethod.Body.IsEquivalentTo(secondMethod.Body, false); protected override SyntaxToken GetMethodIdentifier(IMethodDeclaration method) => diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs index 8b1d72c0ce8..eb3d520f630 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs @@ -72,33 +72,55 @@ protected static bool HaveSameParameters(SemanticModel model, Separated private static bool HaveSameParameterLists(SemanticModel model, SeparatedSyntaxList leftParameters, SeparatedSyntaxList rightParameters) where TSyntax : SyntaxNode => - leftParameters.Zip(rightParameters, (left, right) => left.IsEquivalentTo(right)).All(x => x) // Perf: Syntactic equivalence for all parameters first - && leftParameters.Zip(rightParameters, (left, right) => HaveSameParameterType(model, left, right)).All(x => x); // Also make sure the parameter types are same - - private static bool HaveSameParameterType(SemanticModel model, SyntaxNode left, SyntaxNode right) => - model.GetDeclaredSymbol(left) is IParameterSymbol { Type: { } leftParameterType } - && model.GetDeclaredSymbol(right) is IParameterSymbol { Type: { } rightParameterType } - && TypesAreSame(leftParameterType, rightParameterType); - - private static bool TypesAreSame(ITypeSymbol leftParameterType, ITypeSymbol rightParameterType) => + leftParameters.Equals(rightParameters, (left, right) => left.IsEquivalentTo(right)); // Perf: Syntactic equivalence for all parameters + + protected static bool HaveSameTypeParameters(SemanticModel model, SeparatedSyntaxList? firstTypeParameterList, SeparatedSyntaxList? secondTypeParameterList) + where TSyntax : SyntaxNode + { + HashSet visited = []; + var firstSymbols = firstTypeParameterList?.Select(x => model.GetDeclaredSymbol(x)).OfType() ?? []; + var secondSymbols = secondTypeParameterList?.Select(x => model.GetDeclaredSymbol(x)).OfType().ToArray() ?? []; + return firstSymbols.All(x => secondSymbols.Any(secondSymbol => TypesAreSame(visited, x, secondSymbol))); + } + + private static bool TypesAreSame(HashSet visited, ITypeSymbol leftParameterType, ITypeSymbol rightParameterType) => leftParameterType.Equals(rightParameterType) // M1(int x) <-> M2(int x) - || TypesAreSameTypeParameters(leftParameterType, rightParameterType) // M1(T x) where T: class <-> M2(T x) where T: class - // T of M1 is a different symbol than T of M2, but if they have the same constraints, they can be interchanged - || TypesAreSameGenericType(leftParameterType, rightParameterType); // M1(IEnumerable x) where T: class <-> M2(IEnumerable x) where T: class + || TypesAreSameTypeParameters(visited, leftParameterType, rightParameterType) // M1(T x) where T: class <-> M2(T x) where T: class + // T of M1 is a different symbol than T of M2, but if they have the same constraints, they can be interchanged + || TypesAreSameGenericType(visited, leftParameterType, rightParameterType); // M1(IEnumerable x) where T: class <-> M2(IEnumerable x) where T: class - private static bool TypesAreSameGenericType(ITypeSymbol leftParameterType, ITypeSymbol rightParameterType) => + private static bool TypesAreSameGenericType(HashSet visited, ITypeSymbol leftParameterType, ITypeSymbol rightParameterType) => leftParameterType is INamedTypeSymbol { IsGenericType: true } namedTypeLeft && rightParameterType is INamedTypeSymbol { IsGenericType: true } namedTypeRight - && namedTypeLeft.TypeParameters.Length == namedTypeRight.TypeParameters.Length - && namedTypeLeft.TypeParameters.Zip(namedTypeRight.TypeParameters, (left, right) => TypesAreSame(left, right)).All(x => x); - - private static bool TypesAreSameTypeParameters(ITypeSymbol leftParameterType, ITypeSymbol rightParameterType) => - leftParameterType is ITypeParameterSymbol left - && rightParameterType is ITypeParameterSymbol right - && left.HasConstructorConstraint == right.HasConstructorConstraint - && left.HasReferenceTypeConstraint == right.HasReferenceTypeConstraint - && left.HasValueTypeConstraint == right.HasValueTypeConstraint - && left.HasUnmanagedTypeConstraint() == right.HasUnmanagedTypeConstraint() - && left.ConstraintTypes.Length == right.ConstraintTypes.Length - && left.ConstraintTypes.All(x => right.ConstraintTypes.Any(y => TypesAreSame(x, y))); + && namedTypeLeft.TypeArguments.Length == namedTypeRight.TypeArguments.Length + && namedTypeLeft.TypeArguments.Equals(namedTypeRight.TypeArguments, (x, y) => TypesAreSameTypeParameters(visited, x, y)); + + private static bool TypesAreSameTypeParameters(HashSet visited, ITypeSymbol leftParameterType, ITypeSymbol rightParameterType) + { + if (leftParameterType.Equals(rightParameterType)) + { + return true; + } + var leftNew = visited.Add(leftParameterType); + var rightNew = visited.Add(rightParameterType); + if (leftNew != rightNew) + { + return false; + } + else if (leftNew) + { + return leftParameterType is ITypeParameterSymbol left + && rightParameterType is ITypeParameterSymbol right + && left.HasConstructorConstraint == right.HasConstructorConstraint + && left.HasReferenceTypeConstraint == right.HasReferenceTypeConstraint + && left.HasValueTypeConstraint == right.HasValueTypeConstraint + && left.HasUnmanagedTypeConstraint() == right.HasUnmanagedTypeConstraint() + && left.ConstraintTypes.Length == right.ConstraintTypes.Length + && left.ConstraintTypes.All(x => right.ConstraintTypes.Any(y => TypesAreSame(visited, x, y))); + } + else + { + return true; + } + } } diff --git a/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs b/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs index adb51a0e022..903e7e79e58 100644 --- a/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs +++ b/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs @@ -118,6 +118,39 @@ public static bool Test2(IDictionary dict) {{constra } """).WithOptions(ParseOptionsHelper.FromCSharp9).Verify(); + [DataTestMethod] + [DataRow("", "where TKey: struct")] + [DataRow("where TKey: struct", "")] + [DataRow("where TKey: struct", "where TKey: class")] + [DataRow("where TKey: struct where TValue: class", "where TKey: class where TValue: class")] + [DataRow("where TValue: class where TKey: struct", "where TKey: class where TValue: struct")] + [DataRow("where TKey: class", "where TKey: class, new()")] + [DataRow("where TKey: unmanaged", "where TKey: struct")] + [DataRow("where TKey: new()", "where TKey: IComparable, new()")] + [DataRow("where TKey: IEquatable, IComparable", "where TKey: System.IComparable")] + [DataRow("where TKey: IEquatable, IComparable where TValue: IComparable", " where TKey: System.IComparable where TValue: System.IComparable, IEquatable")] + public void MethodsShouldNotHaveIdenticalImplementations_MethodTypeParameters_Dictionary_Compliant(string constraint1, string constraint2) => + builderCS.AddSnippet($$""" + using System; + using System.Collections.Generic; + public static class TypeConstraints + { + public static bool Test1(IDictionary dict) {{constraint1}} + { + Console.WriteLine(dict); + Console.WriteLine(dict); + return true; + } + + public static bool Test2(IDictionary dict) {{constraint2}} + { + Console.WriteLine(dict); + Console.WriteLine(dict); + return true; + } + } + """).WithOptions(ParseOptionsHelper.FromCSharp9).VerifyNoIssues(); + [DataTestMethod] [DataRow("")] [DataRow("where TKey: struct")] From de614408fe814458c184cac31c1e327af1b44c3a Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Fri, 7 Jun 2024 13:03:56 +0200 Subject: [PATCH 11/33] Rename to CombinatorialDataTestMethod --- .../Rules/AspNet/UseAspNetModelBindingTest.cs | 6 ++---- ...sShouldNotHaveIdenticalImplementationsTest.cs | 2 +- ... CombinatorialDataTestMethodAttributeTest.cs} | 16 ++++++++-------- ...s => CombinatorialDataTestMethodAttribute.cs} | 2 +- 4 files changed, 12 insertions(+), 14 deletions(-) rename analyzers/tests/SonarAnalyzer.TestFramework.Test/Common/{CombinatorialDataAttributeTest.cs => CombinatorialDataTestMethodAttributeTest.cs} (92%) rename analyzers/tests/SonarAnalyzer.TestFramework/Common/{CombinatorialDataAttribute.cs => CombinatorialDataTestMethodAttribute.cs} (96%) diff --git a/analyzers/tests/SonarAnalyzer.Test/Rules/AspNet/UseAspNetModelBindingTest.cs b/analyzers/tests/SonarAnalyzer.Test/Rules/AspNet/UseAspNetModelBindingTest.cs index bc91ac48b3e..115cc45a5df 100644 --- a/analyzers/tests/SonarAnalyzer.Test/Rules/AspNet/UseAspNetModelBindingTest.cs +++ b/analyzers/tests/SonarAnalyzer.Test/Rules/AspNet/UseAspNetModelBindingTest.cs @@ -135,8 +135,7 @@ static void StaticLocalFunction(HttpRequest request) } """").Verify(); - [TestMethod] - [CombinatorialData] + [CombinatorialDataTestMethod] public void UseAspNetModelBinding_CompliantAccess( [DataValues( "_ = {0}.Keys", @@ -293,8 +292,7 @@ public void HandleRequest(HttpRequest request) } """").VerifyNoIssues(); - [TestMethod] - [CombinatorialData] + [CombinatorialDataTestMethod] public void UseAspNetModelBinding_InheritanceAccess( [DataValues( ": Controller", diff --git a/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs b/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs index 903e7e79e58..bc57b8bf3c1 100644 --- a/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs +++ b/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs @@ -62,7 +62,7 @@ public static bool Compare2(T? value1, T value2) {{constraint2}} // Noncompli } """).WithOptions(ParseOptionsHelper.FromCSharp9).Verify(); - [CombinatorialData] + [CombinatorialDataTestMethod] public void MethodsShouldNotHaveIdenticalImplementations_MethodTypeParameters_Compliant( [DataValues("where T: struct", "where T: class")] string constraint1, [DataValues("", "where T: unmanaged", "where T: new()")] string constraint2) => diff --git a/analyzers/tests/SonarAnalyzer.TestFramework.Test/Common/CombinatorialDataAttributeTest.cs b/analyzers/tests/SonarAnalyzer.TestFramework.Test/Common/CombinatorialDataTestMethodAttributeTest.cs similarity index 92% rename from analyzers/tests/SonarAnalyzer.TestFramework.Test/Common/CombinatorialDataAttributeTest.cs rename to analyzers/tests/SonarAnalyzer.TestFramework.Test/Common/CombinatorialDataTestMethodAttributeTest.cs index 6fa7d4f8838..eae9bfcd981 100644 --- a/analyzers/tests/SonarAnalyzer.TestFramework.Test/Common/CombinatorialDataAttributeTest.cs +++ b/analyzers/tests/SonarAnalyzer.TestFramework.Test/Common/CombinatorialDataTestMethodAttributeTest.cs @@ -36,7 +36,7 @@ public static void Initialize(TestContext context) } [TestMethod] - [CombinatorialData] + [CombinatorialDataTestMethod] #pragma warning disable S2699 // Tests should include assertions. Assertion happens in cleanup public void Combinatorial([DataValues(1, 2, 3)] int x, [DataValues(-1, -2, -3)] int y) #pragma warning restore S2699 @@ -73,7 +73,7 @@ public static void Initialize(TestContext context) } [TestMethod] - [CombinatorialData] + [CombinatorialDataTestMethod] #pragma warning disable S2699 // Tests should include assertions. Assertion happens in cleanup public void Combinatorial([DataValues(1, 2, 3)] int x, [DataValues("A", "B")] string y, [DataValues(true, false)] bool z) #pragma warning restore S2699 @@ -107,7 +107,7 @@ public class CombinatorialDataAttributeTest_AttributeTest [TestMethod] public void Combinatorial_Test() { - var attribute = new CombinatorialDataAttribute(); + var attribute = new CombinatorialDataTestMethodAttribute(); var data = attribute.GetData(typeof(CombinatorialDataAttributeTest_AttributeTest).GetMethod(nameof(Combinatorial))); data.Should().BeEquivalentTo([ [1, "A", true], @@ -128,7 +128,7 @@ public void Combinatorial_Test() [TestMethod] public void MissingAttribute_Test() { - var attribute = new CombinatorialDataAttribute(); + var attribute = new CombinatorialDataTestMethodAttribute(); var data = () => attribute.GetData(typeof(CombinatorialDataAttributeTest_AttributeTest).GetMethod(nameof(MissingAttribute))).ToList(); data.Should().Throw().WithMessage("Combinatorial test requires all parameters to have the [DataValues] attribute set"); } @@ -136,7 +136,7 @@ public void MissingAttribute_Test() [TestMethod] public void EmptyDataValuesAttribute_Test() { - var attribute = new CombinatorialDataAttribute(); + var attribute = new CombinatorialDataTestMethodAttribute(); var data = () => attribute.GetData(typeof(CombinatorialDataAttributeTest_AttributeTest).GetMethod(nameof(EmptyDataValuesAttribute))).ToList(); data.Should().Throw().WithMessage("[DataValues] attribute must have values set for all parameters"); } @@ -144,7 +144,7 @@ public void EmptyDataValuesAttribute_Test() [TestMethod] public void NoParameters_Test() { - var attribute = new CombinatorialDataAttribute(); + var attribute = new CombinatorialDataTestMethodAttribute(); var data = () => attribute.GetData(typeof(CombinatorialDataAttributeTest_AttributeTest).GetMethod(nameof(NoParameters))).ToList(); data.Should().Throw().WithMessage("Combinatorial test must specify parameters with [DataValues] attributes"); } @@ -157,7 +157,7 @@ public void NoParameters_Test() [DataRow("Test (a,b,1)", "a", "b", 1)] public void GetDisplayName_Test(string expected, params object[] arguments) { - var attribute = new CombinatorialDataAttribute(); + var attribute = new CombinatorialDataTestMethodAttribute(); var methodInfo = Substitute.For(); methodInfo.Name.Returns("Test"); var actual = attribute.GetDisplayName(methodInfo, arguments); @@ -167,7 +167,7 @@ public void GetDisplayName_Test(string expected, params object[] arguments) [TestMethod] public void GetDisplayName_Null() { - var attribute = new CombinatorialDataAttribute(); + var attribute = new CombinatorialDataTestMethodAttribute(); var methodInfo = Substitute.For(); methodInfo.Name.Returns("Test"); var actual = attribute.GetDisplayName(methodInfo, null); diff --git a/analyzers/tests/SonarAnalyzer.TestFramework/Common/CombinatorialDataAttribute.cs b/analyzers/tests/SonarAnalyzer.TestFramework/Common/CombinatorialDataTestMethodAttribute.cs similarity index 96% rename from analyzers/tests/SonarAnalyzer.TestFramework/Common/CombinatorialDataAttribute.cs rename to analyzers/tests/SonarAnalyzer.TestFramework/Common/CombinatorialDataTestMethodAttribute.cs index 28409a5dec2..5815482580e 100644 --- a/analyzers/tests/SonarAnalyzer.TestFramework/Common/CombinatorialDataAttribute.cs +++ b/analyzers/tests/SonarAnalyzer.TestFramework/Common/CombinatorialDataTestMethodAttribute.cs @@ -35,7 +35,7 @@ public DataValuesAttribute(params object[] values) } [AttributeUsage(AttributeTargets.Method)] -public sealed class CombinatorialDataAttribute : TestMethodAttribute, ITestDataSource +public sealed class CombinatorialDataTestMethodAttribute : TestMethodAttribute, ITestDataSource { public IEnumerable GetData(MethodInfo methodInfo) { From 8605bd4b11fd59d9908a033763a39c2298999521 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Fri, 7 Jun 2024 13:53:05 +0200 Subject: [PATCH 12/33] Remove unsed parameter --- ...dsShouldNotHaveIdenticalImplementations.cs | 2 +- ...ouldNotHaveIdenticalImplementationsBase.cs | 72 ++++++++----------- 2 files changed, 29 insertions(+), 45 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/MethodsShouldNotHaveIdenticalImplementations.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/MethodsShouldNotHaveIdenticalImplementations.cs index 54974a6ebd7..41d057f34cf 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/MethodsShouldNotHaveIdenticalImplementations.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/MethodsShouldNotHaveIdenticalImplementations.cs @@ -43,7 +43,7 @@ protected override IEnumerable GetMethodDeclarations(SyntaxN protected override bool AreDuplicates(SemanticModel model, IMethodDeclaration firstMethod, IMethodDeclaration secondMethod) => firstMethod is { Body: { Statements: { Count: > 1 } } } && firstMethod.Identifier.ValueText != secondMethod.Identifier.ValueText - && HaveSameParameters(model, firstMethod.ParameterList?.Parameters, secondMethod.ParameterList?.Parameters) + && HaveSameParameters(firstMethod.ParameterList?.Parameters, secondMethod.ParameterList?.Parameters) && HaveSameTypeParameters(model, firstMethod.TypeParameterList?.Parameters, secondMethod.TypeParameterList?.Parameters) && firstMethod.Body.IsEquivalentTo(secondMethod.Body, false); diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs index eb3d520f630..3ce7a9c2748 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs @@ -64,63 +64,47 @@ protected override void Initialize(SonarAnalysisContext context) => protected virtual bool IsExcludedFromBeingExamined(SonarSyntaxNodeReportingContext context) => context.ContainingSymbol.Kind != SymbolKind.NamedType; - protected static bool HaveSameParameters(SemanticModel model, SeparatedSyntaxList? leftParameters, SeparatedSyntaxList? rightParameters) + protected static bool HaveSameParameters(SeparatedSyntaxList? leftParameters, SeparatedSyntaxList? rightParameters) where TSyntax : SyntaxNode => (leftParameters is null && rightParameters is null) // In VB.Net the parameter list can be omitted - || (leftParameters?.Count == rightParameters?.Count && HaveSameParameterLists(model, leftParameters.Value, rightParameters.Value)); - - private static bool HaveSameParameterLists(SemanticModel model, - SeparatedSyntaxList leftParameters, - SeparatedSyntaxList rightParameters) where TSyntax : SyntaxNode => - leftParameters.Equals(rightParameters, (left, right) => left.IsEquivalentTo(right)); // Perf: Syntactic equivalence for all parameters + || (leftParameters?.Count == rightParameters?.Count && HaveSameParameterLists(leftParameters.Value, rightParameters.Value)); protected static bool HaveSameTypeParameters(SemanticModel model, SeparatedSyntaxList? firstTypeParameterList, SeparatedSyntaxList? secondTypeParameterList) where TSyntax : SyntaxNode { - HashSet visited = []; var firstSymbols = firstTypeParameterList?.Select(x => model.GetDeclaredSymbol(x)).OfType() ?? []; var secondSymbols = secondTypeParameterList?.Select(x => model.GetDeclaredSymbol(x)).OfType().ToArray() ?? []; - return firstSymbols.All(x => secondSymbols.Any(secondSymbol => TypesAreSame(visited, x, secondSymbol))); + return firstSymbols.All(x => secondSymbols.Any(secondSymbol => TypesAreSame(x, secondSymbol))); } - private static bool TypesAreSame(HashSet visited, ITypeSymbol leftParameterType, ITypeSymbol rightParameterType) => + private static bool HaveSameParameterLists(SeparatedSyntaxList leftParameters, + SeparatedSyntaxList rightParameters) where TSyntax : SyntaxNode => + leftParameters.Equals(rightParameters, (left, right) => left.IsEquivalentTo(right)); // Perf: Syntactic equivalence for all parameters + + private static bool TypesAreSame(ITypeSymbol leftParameterType, ITypeSymbol rightParameterType) => leftParameterType.Equals(rightParameterType) // M1(int x) <-> M2(int x) - || TypesAreSameTypeParameters(visited, leftParameterType, rightParameterType) // M1(T x) where T: class <-> M2(T x) where T: class - // T of M1 is a different symbol than T of M2, but if they have the same constraints, they can be interchanged - || TypesAreSameGenericType(visited, leftParameterType, rightParameterType); // M1(IEnumerable x) where T: class <-> M2(IEnumerable x) where T: class + || TypesAreSameTypeParameters(leftParameterType, rightParameterType) // M1(T x) where T: class <-> M2(T x) where T: class + // T of M1 is a different symbol than T of M2, but if they have the same constraints, they can be interchanged + || TypesAreSameGenericType(leftParameterType, rightParameterType); // M1(IEnumerable x) where T: class <-> M2(IEnumerable x) where T: class - private static bool TypesAreSameGenericType(HashSet visited, ITypeSymbol leftParameterType, ITypeSymbol rightParameterType) => + private static bool TypesAreSameGenericType(ITypeSymbol leftParameterType, ITypeSymbol rightParameterType) => leftParameterType is INamedTypeSymbol { IsGenericType: true } namedTypeLeft && rightParameterType is INamedTypeSymbol { IsGenericType: true } namedTypeRight && namedTypeLeft.TypeArguments.Length == namedTypeRight.TypeArguments.Length - && namedTypeLeft.TypeArguments.Equals(namedTypeRight.TypeArguments, (x, y) => TypesAreSameTypeParameters(visited, x, y)); - - private static bool TypesAreSameTypeParameters(HashSet visited, ITypeSymbol leftParameterType, ITypeSymbol rightParameterType) - { - if (leftParameterType.Equals(rightParameterType)) - { - return true; - } - var leftNew = visited.Add(leftParameterType); - var rightNew = visited.Add(rightParameterType); - if (leftNew != rightNew) - { - return false; - } - else if (leftNew) - { - return leftParameterType is ITypeParameterSymbol left - && rightParameterType is ITypeParameterSymbol right - && left.HasConstructorConstraint == right.HasConstructorConstraint - && left.HasReferenceTypeConstraint == right.HasReferenceTypeConstraint - && left.HasValueTypeConstraint == right.HasValueTypeConstraint - && left.HasUnmanagedTypeConstraint() == right.HasUnmanagedTypeConstraint() - && left.ConstraintTypes.Length == right.ConstraintTypes.Length - && left.ConstraintTypes.All(x => right.ConstraintTypes.Any(y => TypesAreSame(visited, x, y))); - } - else - { - return true; - } - } + && namedTypeLeft.TypeArguments.Equals(namedTypeRight.TypeArguments, (x, y) => + x is ITypeParameterSymbol a ? + y is ITypeParameterSymbol b && a.Name == b.Name + : TypesAreSame(x, y)); + + private static bool TypesAreSameTypeParameters(ITypeSymbol leftParameterType, ITypeSymbol rightParameterType) => + leftParameterType.Equals(rightParameterType) + || (leftParameterType is ITypeParameterSymbol left + && rightParameterType is ITypeParameterSymbol right + && left.Name == right.Name + && left.HasConstructorConstraint == right.HasConstructorConstraint + && left.HasReferenceTypeConstraint == right.HasReferenceTypeConstraint + && left.HasValueTypeConstraint == right.HasValueTypeConstraint + && left.HasUnmanagedTypeConstraint() == right.HasUnmanagedTypeConstraint() + && left.ConstraintTypes.Length == right.ConstraintTypes.Length + && left.ConstraintTypes.All(x => right.ConstraintTypes.Any(y => TypesAreSame(x, y)))); } From a45e584589fe453ccde59836c5c384d8920c136a Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Fri, 7 Jun 2024 13:53:17 +0200 Subject: [PATCH 13/33] Add support for VB --- .../Rules/MethodsShouldNotHaveIdenticalImplementations.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/analyzers/src/SonarAnalyzer.VisualBasic/Rules/MethodsShouldNotHaveIdenticalImplementations.cs b/analyzers/src/SonarAnalyzer.VisualBasic/Rules/MethodsShouldNotHaveIdenticalImplementations.cs index d7426844620..5aa8d86f6ca 100644 --- a/analyzers/src/SonarAnalyzer.VisualBasic/Rules/MethodsShouldNotHaveIdenticalImplementations.cs +++ b/analyzers/src/SonarAnalyzer.VisualBasic/Rules/MethodsShouldNotHaveIdenticalImplementations.cs @@ -33,7 +33,8 @@ protected override IEnumerable GetMethodDeclarations(SyntaxNo protected override bool AreDuplicates(SemanticModel model, MethodBlockSyntax firstMethod, MethodBlockSyntax secondMethod) => firstMethod.Statements.Count > 1 && firstMethod.GetIdentifierText() != secondMethod.GetIdentifierText() - && HaveSameParameters(model, firstMethod.GetParameters(), secondMethod.GetParameters()) + && HaveSameParameters(firstMethod.GetParameters(), secondMethod.GetParameters()) + && HaveSameTypeParameters(model, firstMethod.SubOrFunctionStatement?.TypeParameterList?.Parameters, secondMethod.SubOrFunctionStatement?.TypeParameterList?.Parameters) && VisualBasicEquivalenceChecker.AreEquivalent(firstMethod.Statements, secondMethod.Statements); protected override SyntaxToken GetMethodIdentifier(MethodBlockSyntax method) => From 8f70d1acc82e5cc74c209b6a8b0d4f374a0d6728 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Fri, 7 Jun 2024 15:18:16 +0200 Subject: [PATCH 14/33] Add VB tests --- ...ouldNotHaveIdenticalImplementationsTest.cs | 95 ++++++++++++++++++- 1 file changed, 94 insertions(+), 1 deletion(-) diff --git a/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs b/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs index bc57b8bf3c1..6dfd49166cc 100644 --- a/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs +++ b/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs @@ -40,7 +40,7 @@ public void MethodsShouldNotHaveIdenticalImplementations() => [DataRow("where T: class", "where T: class")] [DataRow("where T: unmanaged", "where T: unmanaged")] [DataRow("where T: new()", "where T: new()")] - [DataRow("where T: IEquatable, IComparable", "where T: System.IComparable, IEquatable")] + [DataRow("where T: IEquatable, IComparable", "where T: System.IComparable, IEquatable")] public void MethodsShouldNotHaveIdenticalImplementations_MethodTypeParameters_NonCompliant(string constraint1, string constraint2) => builderCS.AddSnippet($$""" using System; @@ -118,6 +118,35 @@ public static bool Test2(IDictionary dict) {{constra } """).WithOptions(ParseOptionsHelper.FromCSharp9).Verify(); + [DataTestMethod] + [DataRow("Of TKey, TValue", "Of TKey, TValue")] + [DataRow("Of TKey As Structure, TValue", "Of TKey As Structure, TValue")] + [DataRow("Of TKey As Structure, TValue As Class", "Of TKey As Structure, TValue As Class")] + [DataRow("Of TValue As Class, TKey As Structure", "Of TKey As Structure, TValue As Class")] + [DataRow("Of TKey As {Class}, TValue", "Of TKey As Class, TValue")] + [DataRow("Of TKey As {New}, TValue", "Of TKey As New, TValue")] + [DataRow("Of TKey As {IEquatable(Of TKey), IComparable}, TValue", "Of TKey As {System.IComparable, IEquatable(Of TKey)}, TValue")] + [DataRow("Of TKey As {IEquatable(Of TKey), IComparable}, TValue As IComparable", "Of TValue As IComparable, TKey As {System.IComparable, IEquatable(Of TKey)}")] + public void MethodsShouldNotHaveIdenticalImplementations_MethodTypeParameters_Dictionary_VB_NonCompliant(string constraint1, string constraint2) => + builderVB.AddSnippet($$""" + Imports System + Imports System.Collections.Generic + + Class TypeConstraints + Function Test1({{constraint1}})(dict As IDictionary(Of TKey, TValue)) As Boolean ' Secondary + Console.WriteLine(dict) + Console.WriteLine(dict) + Return True + End Function + + Function Test2({{constraint2}})(dict As IDictionary(Of TKey, TValue)) As Boolean ' Noncompliant + Console.WriteLine(dict) + Console.WriteLine(dict) + Return True + End Function + End Class + """).Verify(); + [DataTestMethod] [DataRow("", "where TKey: struct")] [DataRow("where TKey: struct", "")] @@ -129,6 +158,9 @@ public static bool Test2(IDictionary dict) {{constra [DataRow("where TKey: new()", "where TKey: IComparable, new()")] [DataRow("where TKey: IEquatable, IComparable", "where TKey: System.IComparable")] [DataRow("where TKey: IEquatable, IComparable where TValue: IComparable", " where TKey: System.IComparable where TValue: System.IComparable, IEquatable")] + [DataRow("where TKey: TValue")] + [DataRow("where TKey: TValue where TValue: IComparable")] + [DataRow("where TKey: IEquatable where TValue: IComparable")] public void MethodsShouldNotHaveIdenticalImplementations_MethodTypeParameters_Dictionary_Compliant(string constraint1, string constraint2) => builderCS.AddSnippet($$""" using System; @@ -151,6 +183,36 @@ public static bool Test2(IDictionary dict) {{constra } """).WithOptions(ParseOptionsHelper.FromCSharp9).VerifyNoIssues(); + [DataTestMethod] + [DataRow("Of TKey, TValue", "Of TKey, TValue As Structure")] + + //[DataRow("Of TKey As Structure, TValue", "Of TKey As Structure, TValue")] + //[DataRow("Of TKey As Structure, TValue As Class", "Of TKey As Structure, TValue As Class")] + //[DataRow("Of TValue As Class, TKey As Structure", "Of TKey As Structure, TValue As Class")] + //[DataRow("Of TKey As {Class}, TValue", "Of TKey As Class, TValue")] + //[DataRow("Of TKey As {New}, TValue", "Of TKey As New, TValue")] + //[DataRow("Of TKey As {IEquatable(Of TKey), IComparable}, TValue", "Of TKey As {System.IComparable, IEquatable(Of TKey)}, TValue")] + //[DataRow("Of TKey As {IEquatable(Of TKey), IComparable}, TValue As IComparable", "Of TValue As IComparable, TKey As {System.IComparable, IEquatable(Of TKey)}")] + public void MethodsShouldNotHaveIdenticalImplementations_MethodTypeParameters_Dictionary_VB_Compliant(string constraint1, string constraint2) => + builderVB.AddSnippet($$""" + Imports System + Imports System.Collections.Generic + + Class TypeConstraints + Function Test1({{constraint1}})(dict As IDictionary(Of TKey, TValue)) As Boolean + Console.WriteLine(dict) + Console.WriteLine(dict) + Return True + End Function + + Function Test2({{constraint2}})(dict As IDictionary(Of TKey, TValue)) As Boolean + Console.WriteLine(dict) + Console.WriteLine(dict) + Return True + End Function + End Class + """).VerifyNoIssues(); + [DataTestMethod] [DataRow("")] [DataRow("where TKey: struct")] @@ -161,6 +223,9 @@ public static bool Test2(IDictionary dict) {{constra [DataRow("where TKey: new()")] [DataRow("where TKey: IEquatable, IComparable")] [DataRow("where TKey: IEquatable, IComparable where TValue: IComparable")] + [DataRow("where TKey: TValue")] + [DataRow("where TKey: TValue where TValue: IComparable")] + [DataRow("where TKey: IEquatable where TValue: IComparable")] public void MethodsShouldNotHaveIdenticalImplementations_ClassTypeParameters_Dictionary_NonCompliant(string constraint) => builderCS.AddSnippet($$""" using System; @@ -183,6 +248,34 @@ public static bool Test2(IDictionary dict) // Noncompliant } """).WithOptions(ParseOptionsHelper.FromCSharp9).Verify(); + [DataTestMethod] + [DataRow("where TSelf: IEqualityOperators")] + [DataRow("where TSelf: IEqualityOperators, TResult")] + [DataRow("where TSelf: IEqualityOperators where TResult: IEqualityOperators")] + [DataRow("where TSelf: IComparisonOperators")] + [DataRow("where TSelf: IComparisonOperators, TResult")] + public void MethodsShouldNotHaveIdenticalImplementations_SelfTypes_NonCompliant(string constraint) => + builderCS.AddSnippet($$""" + using System; + using System.Numerics; + public class TypeConstraints + { + public static bool Test1(IEqualityOperators x) {{constraint}} // Secondary + { + Console.WriteLine(x); + Console.WriteLine(x); + return true; + } + + public static bool Test2(IEqualityOperators x) {{constraint}} // Noncompliant + { + Console.WriteLine(x); + Console.WriteLine(x); + return true; + } + } + """).WithOptions(ParseOptionsHelper.FromCSharp9).Verify(); + #if NET [TestMethod] From 16bcfde49200c06cbb02c7f25f897ef5f9f669b1 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Fri, 7 Jun 2024 15:23:07 +0200 Subject: [PATCH 15/33] More VB tests --- ...ethodsShouldNotHaveIdenticalImplementationsTest.cs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs b/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs index 6dfd49166cc..d01953e62f9 100644 --- a/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs +++ b/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs @@ -185,14 +185,9 @@ public static bool Test2(IDictionary dict) {{constra [DataTestMethod] [DataRow("Of TKey, TValue", "Of TKey, TValue As Structure")] - - //[DataRow("Of TKey As Structure, TValue", "Of TKey As Structure, TValue")] - //[DataRow("Of TKey As Structure, TValue As Class", "Of TKey As Structure, TValue As Class")] - //[DataRow("Of TValue As Class, TKey As Structure", "Of TKey As Structure, TValue As Class")] - //[DataRow("Of TKey As {Class}, TValue", "Of TKey As Class, TValue")] - //[DataRow("Of TKey As {New}, TValue", "Of TKey As New, TValue")] - //[DataRow("Of TKey As {IEquatable(Of TKey), IComparable}, TValue", "Of TKey As {System.IComparable, IEquatable(Of TKey)}, TValue")] - //[DataRow("Of TKey As {IEquatable(Of TKey), IComparable}, TValue As IComparable", "Of TValue As IComparable, TKey As {System.IComparable, IEquatable(Of TKey)}")] + [DataRow("Of TKey, TValue As Class", "Of TKey, TValue As Structure")] + [DataRow("Of TKey As Structure, TValue", "Of TKey, TValue As Structure")] + [DataRow("Of TKey As {New, IComparable}, TValue", "Of TKey As New, TValue")] public void MethodsShouldNotHaveIdenticalImplementations_MethodTypeParameters_Dictionary_VB_Compliant(string constraint1, string constraint2) => builderVB.AddSnippet($$""" Imports System From 093915bf7ddca7dab963a6cccdc13bff7fb92a8f Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Fri, 7 Jun 2024 15:28:47 +0200 Subject: [PATCH 16/33] Correct tests --- .../MethodsShouldNotHaveIdenticalImplementationsTest.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs b/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs index d01953e62f9..0be42be4d2a 100644 --- a/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs +++ b/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs @@ -88,6 +88,9 @@ public static bool Compare2(T? value1, T value2) {{constraint2}} [DataTestMethod] [DataRow("", "")] + [DataRow("where TKey: TValue", "where TKey: TValue")] + [DataRow("where TKey: TValue where TValue: IComparable", "where TKey: TValue where TValue: IComparable")] + [DataRow("where TKey: IEquatable where TValue: IComparable", "where TKey: IEquatable where TValue: IComparable")] [DataRow("where TKey: struct", "where TKey: struct")] [DataRow("where TKey: struct where TValue: class", "where TKey: struct where TValue: class")] [DataRow("where TValue: class where TKey: struct", "where TKey: struct where TValue: class")] @@ -158,9 +161,6 @@ End Class [DataRow("where TKey: new()", "where TKey: IComparable, new()")] [DataRow("where TKey: IEquatable, IComparable", "where TKey: System.IComparable")] [DataRow("where TKey: IEquatable, IComparable where TValue: IComparable", " where TKey: System.IComparable where TValue: System.IComparable, IEquatable")] - [DataRow("where TKey: TValue")] - [DataRow("where TKey: TValue where TValue: IComparable")] - [DataRow("where TKey: IEquatable where TValue: IComparable")] public void MethodsShouldNotHaveIdenticalImplementations_MethodTypeParameters_Dictionary_Compliant(string constraint1, string constraint2) => builderCS.AddSnippet($$""" using System; From 586d42beec63353705a61cd8d5833f98669261e5 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Mon, 10 Jun 2024 09:55:55 +0200 Subject: [PATCH 17/33] Net only test --- .../Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs b/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs index 0be42be4d2a..c737c7a5967 100644 --- a/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs +++ b/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs @@ -243,6 +243,8 @@ public static bool Test2(IDictionary dict) // Noncompliant } """).WithOptions(ParseOptionsHelper.FromCSharp9).Verify(); +#if NET + [DataTestMethod] [DataRow("where TSelf: IEqualityOperators")] [DataRow("where TSelf: IEqualityOperators, TResult")] @@ -271,8 +273,6 @@ public static bool Test2(IEqualityOperators builderCS.AddPaths("MethodsShouldNotHaveIdenticalImplementations.CSharp9.cs").WithTopLevelStatements().Verify(); From a26c3ce19765eda221b51c5633282924e7b2e96f Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Mon, 10 Jun 2024 13:35:29 +0200 Subject: [PATCH 18/33] Fix code smells and formatting --- ...ouldNotHaveIdenticalImplementationsBase.cs | 28 ++++++++++--------- .../CombinatorialDataTestMethodAttribute.cs | 5 ++-- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs index 3ce7a9c2748..d2d5e4f8a3e 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs @@ -74,7 +74,7 @@ protected static bool HaveSameTypeParameters(SemanticModel model, Separ { var firstSymbols = firstTypeParameterList?.Select(x => model.GetDeclaredSymbol(x)).OfType() ?? []; var secondSymbols = secondTypeParameterList?.Select(x => model.GetDeclaredSymbol(x)).OfType().ToArray() ?? []; - return firstSymbols.All(x => secondSymbols.Any(secondSymbol => TypesAreSame(x, secondSymbol))); + return firstSymbols.All(x => Array.Exists(secondSymbols, secondSymbol => TypesAreSame(x, secondSymbol))); } private static bool HaveSameParameterLists(SeparatedSyntaxList leftParameters, @@ -92,19 +92,21 @@ private static bool TypesAreSameGenericType(ITypeSymbol leftParameterType, IType && rightParameterType is INamedTypeSymbol { IsGenericType: true } namedTypeRight && namedTypeLeft.TypeArguments.Length == namedTypeRight.TypeArguments.Length && namedTypeLeft.TypeArguments.Equals(namedTypeRight.TypeArguments, (x, y) => - x is ITypeParameterSymbol a ? - y is ITypeParameterSymbol b && a.Name == b.Name + x is ITypeParameterSymbol a + ? y is ITypeParameterSymbol b && a.Name == b.Name : TypesAreSame(x, y)); private static bool TypesAreSameTypeParameters(ITypeSymbol leftParameterType, ITypeSymbol rightParameterType) => - leftParameterType.Equals(rightParameterType) - || (leftParameterType is ITypeParameterSymbol left - && rightParameterType is ITypeParameterSymbol right - && left.Name == right.Name - && left.HasConstructorConstraint == right.HasConstructorConstraint - && left.HasReferenceTypeConstraint == right.HasReferenceTypeConstraint - && left.HasValueTypeConstraint == right.HasValueTypeConstraint - && left.HasUnmanagedTypeConstraint() == right.HasUnmanagedTypeConstraint() - && left.ConstraintTypes.Length == right.ConstraintTypes.Length - && left.ConstraintTypes.All(x => right.ConstraintTypes.Any(y => TypesAreSame(x, y)))); + leftParameterType.Equals(rightParameterType) || TypeParametersHaveSameNameAndConstraints(leftParameterType, rightParameterType); + + private static bool TypeParametersHaveSameNameAndConstraints(ITypeSymbol leftParameterType, ITypeSymbol rightParameterType) => + leftParameterType is ITypeParameterSymbol left + && rightParameterType is ITypeParameterSymbol right + && left.Name == right.Name + && left.HasConstructorConstraint == right.HasConstructorConstraint + && left.HasReferenceTypeConstraint == right.HasReferenceTypeConstraint + && left.HasValueTypeConstraint == right.HasValueTypeConstraint + && left.HasUnmanagedTypeConstraint() == right.HasUnmanagedTypeConstraint() + && left.ConstraintTypes.Length == right.ConstraintTypes.Length + && left.ConstraintTypes.All(x => right.ConstraintTypes.Any(y => TypesAreSame(x, y))); } diff --git a/analyzers/tests/SonarAnalyzer.TestFramework/Common/CombinatorialDataTestMethodAttribute.cs b/analyzers/tests/SonarAnalyzer.TestFramework/Common/CombinatorialDataTestMethodAttribute.cs index 5815482580e..810096eefba 100644 --- a/analyzers/tests/SonarAnalyzer.TestFramework/Common/CombinatorialDataTestMethodAttribute.cs +++ b/analyzers/tests/SonarAnalyzer.TestFramework/Common/CombinatorialDataTestMethodAttribute.cs @@ -63,6 +63,9 @@ public IEnumerable GetData(MethodInfo methodInfo) } } + public string GetDisplayName(MethodInfo methodInfo, object[] data) => + $"{methodInfo.Name} ({(data is not null ? string.Join(",", data) : null)})"; + private static object[] CreateArguments(object[][] valuesPerParameter, int[] parameterIndices) { var arg = new object[parameterIndices.Length]; @@ -74,6 +77,4 @@ private static object[] CreateArguments(object[][] valuesPerParameter, int[] par return arg; } - public string GetDisplayName(MethodInfo methodInfo, object[] data) => - $"{methodInfo.Name} ({(data is not null ? string.Join(",", data) : null)})"; } From 3c35dac7888ad0cba90d06d5e6422c98f4f86c3b Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Mon, 10 Jun 2024 14:02:56 +0200 Subject: [PATCH 19/33] Add all combinations for constraints --- ...ouldNotHaveIdenticalImplementationsTest.cs | 25 +++++++++++++------ .../CombinatorialDataTestMethodAttribute.cs | 1 - 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs b/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs index c737c7a5967..f72291649fc 100644 --- a/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs +++ b/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs @@ -63,28 +63,39 @@ public static bool Compare2(T? value1, T value2) {{constraint2}} // Noncompli """).WithOptions(ParseOptionsHelper.FromCSharp9).Verify(); [CombinatorialDataTestMethod] - public void MethodsShouldNotHaveIdenticalImplementations_MethodTypeParameters_Compliant( - [DataValues("where T: struct", "where T: class")] string constraint1, - [DataValues("", "where T: unmanaged", "where T: new()")] string constraint2) => - builderCS.AddSnippet($$""" + public void MethodsShouldNotHaveIdenticalImplementations_MethodTypeParameters( + [DataValues("", "where T: struct", "where T: class", "where T: unmanaged", "where T: new()", "where T: class, new()")] string constraint1, + [DataValues("", "where T: struct", "where T: class", "where T: unmanaged", "where T: new()", "where T: class, new()")] string constraint2) + { + var nonCompliant = constraint1 == constraint2; + var builder = builderCS.AddSnippet($$""" using System; public static class TypeConstraints { - public static bool Compare1(T? value1, T value2) {{constraint1}} + public static bool Compare1(T? value1, T value2) {{constraint1}} {{(nonCompliant ? "// Secondary" : string.Empty)}} { Console.WriteLine(value1); Console.WriteLine(value2); return true; } - public static bool Compare2(T? value1, T value2) {{constraint2}} + public static bool Compare2(T? value1, T value2) {{constraint2}} {{(nonCompliant ? "// Noncompliant" : string.Empty)}} { Console.WriteLine(value1); Console.WriteLine(value2); return true; } } - """).WithOptions(ParseOptionsHelper.FromCSharp9).VerifyNoIssues(); + """).WithOptions(ParseOptionsHelper.FromCSharp9); + if (nonCompliant) + { + builder.Verify(); + } + else + { + builder.VerifyNoIssues(); + } + } [DataTestMethod] [DataRow("", "")] diff --git a/analyzers/tests/SonarAnalyzer.TestFramework/Common/CombinatorialDataTestMethodAttribute.cs b/analyzers/tests/SonarAnalyzer.TestFramework/Common/CombinatorialDataTestMethodAttribute.cs index 810096eefba..b26915f6850 100644 --- a/analyzers/tests/SonarAnalyzer.TestFramework/Common/CombinatorialDataTestMethodAttribute.cs +++ b/analyzers/tests/SonarAnalyzer.TestFramework/Common/CombinatorialDataTestMethodAttribute.cs @@ -76,5 +76,4 @@ private static object[] CreateArguments(object[][] valuesPerParameter, int[] par return arg; } - } From 7a4d9338e4e6147d9268f0c1846bc8eb6e654469 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Wed, 12 Jun 2024 10:05:41 +0200 Subject: [PATCH 20/33] Review --- .../MethodsShouldNotHaveIdenticalImplementationsBase.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs index d2d5e4f8a3e..52a13e3da89 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs @@ -74,7 +74,7 @@ protected static bool HaveSameTypeParameters(SemanticModel model, Separ { var firstSymbols = firstTypeParameterList?.Select(x => model.GetDeclaredSymbol(x)).OfType() ?? []; var secondSymbols = secondTypeParameterList?.Select(x => model.GetDeclaredSymbol(x)).OfType().ToArray() ?? []; - return firstSymbols.All(x => Array.Exists(secondSymbols, secondSymbol => TypesAreSame(x, secondSymbol))); + return firstSymbols.All(x => Array.Exists(secondSymbols, secondSymbol => TypesAreEquivalentTypeParameters(x, secondSymbol))); } private static bool HaveSameParameterLists(SeparatedSyntaxList leftParameters, @@ -83,8 +83,8 @@ private static bool HaveSameParameterLists(SeparatedSyntaxList private static bool TypesAreSame(ITypeSymbol leftParameterType, ITypeSymbol rightParameterType) => leftParameterType.Equals(rightParameterType) // M1(int x) <-> M2(int x) - || TypesAreSameTypeParameters(leftParameterType, rightParameterType) // M1(T x) where T: class <-> M2(T x) where T: class - // T of M1 is a different symbol than T of M2, but if they have the same constraints, they can be interchanged + || TypesAreEquivalentTypeParameters(leftParameterType, rightParameterType) // M1(T x) where T: class <-> M2(T x) where T: class + // T of M1 is a different symbol than T of M2, but if they have the same constraints, they can be interchanged || TypesAreSameGenericType(leftParameterType, rightParameterType); // M1(IEnumerable x) where T: class <-> M2(IEnumerable x) where T: class private static bool TypesAreSameGenericType(ITypeSymbol leftParameterType, ITypeSymbol rightParameterType) => @@ -96,7 +96,7 @@ x is ITypeParameterSymbol a ? y is ITypeParameterSymbol b && a.Name == b.Name : TypesAreSame(x, y)); - private static bool TypesAreSameTypeParameters(ITypeSymbol leftParameterType, ITypeSymbol rightParameterType) => + private static bool TypesAreEquivalentTypeParameters(ITypeSymbol leftParameterType, ITypeSymbol rightParameterType) => leftParameterType.Equals(rightParameterType) || TypeParametersHaveSameNameAndConstraints(leftParameterType, rightParameterType); private static bool TypeParametersHaveSameNameAndConstraints(ITypeSymbol leftParameterType, ITypeSymbol rightParameterType) => From 01944a9a880abf440cca81d8aff406cfab521b0c Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Wed, 12 Jun 2024 10:14:42 +0200 Subject: [PATCH 21/33] Move test cases and add a further nested example --- ...ouldNotHaveIdenticalImplementationsTest.cs | 53 +++++++++---------- 1 file changed, 25 insertions(+), 28 deletions(-) diff --git a/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs b/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs index f72291649fc..53a8f477f01 100644 --- a/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs +++ b/analyzers/tests/SonarAnalyzer.Test/Rules/MethodsShouldNotHaveIdenticalImplementationsTest.cs @@ -34,34 +34,6 @@ public class MethodsShouldNotHaveIdenticalImplementationsTest public void MethodsShouldNotHaveIdenticalImplementations() => builderCS.AddPaths("MethodsShouldNotHaveIdenticalImplementations.cs").WithOptions(ParseOptionsHelper.FromCSharp8).Verify(); - [DataTestMethod] - [DataRow("", "")] - [DataRow("where T: struct", "where T: struct")] - [DataRow("where T: class", "where T: class")] - [DataRow("where T: unmanaged", "where T: unmanaged")] - [DataRow("where T: new()", "where T: new()")] - [DataRow("where T: IEquatable, IComparable", "where T: System.IComparable, IEquatable")] - public void MethodsShouldNotHaveIdenticalImplementations_MethodTypeParameters_NonCompliant(string constraint1, string constraint2) => - builderCS.AddSnippet($$""" - using System; - public static class TypeConstraints - { - public static bool Compare1(T? value1, T value2) {{constraint1}} // Secondary - { - Console.WriteLine(value1); - Console.WriteLine(value2); - return true; - } - - public static bool Compare2(T? value1, T value2) {{constraint2}} // Noncompliant - { - Console.WriteLine(value1); - Console.WriteLine(value2); - return true; - } - } - """).WithOptions(ParseOptionsHelper.FromCSharp9).Verify(); - [CombinatorialDataTestMethod] public void MethodsShouldNotHaveIdenticalImplementations_MethodTypeParameters( [DataValues("", "where T: struct", "where T: class", "where T: unmanaged", "where T: new()", "where T: class, new()")] string constraint1, @@ -97,6 +69,31 @@ public static bool Compare2(T? value1, T value2) {{constraint2}} {{(nonCompli } } + [DataTestMethod] + [DataRow("where T: IEquatable, IComparable", "where T: System.IComparable, IEquatable")] + [DataRow("where T: List>, IList, IComparable", "where T: List>, IComparable, IList")] + public void MethodsShouldNotHaveIdenticalImplementations_MethodTypeParameters_NonCompliant(string constraint1, string constraint2) => + builderCS.AddSnippet($$""" + using System; + using System.Collections.Generic; + public static class TypeConstraints + { + public static bool Compare1(T? value1, T value2) {{constraint1}} // Secondary + { + Console.WriteLine(value1); + Console.WriteLine(value2); + return true; + } + + public static bool Compare2(T? value1, T value2) {{constraint2}} // Noncompliant + { + Console.WriteLine(value1); + Console.WriteLine(value2); + return true; + } + } + """).WithOptions(ParseOptionsHelper.FromCSharp9).Verify(); + [DataTestMethod] [DataRow("", "")] [DataRow("where TKey: TValue", "where TKey: TValue")] From 7fc57b4bc4a8232e79712d04e9b83256929a1389 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Wed, 12 Jun 2024 12:06:00 +0200 Subject: [PATCH 22/33] Codecoverage: More tests --- ...NotHaveIdenticalImplementations.CSharp9.cs | 63 ++++++++++++++++++- 1 file changed, 61 insertions(+), 2 deletions(-) diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/MethodsShouldNotHaveIdenticalImplementations.CSharp9.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/MethodsShouldNotHaveIdenticalImplementations.CSharp9.cs index f2a9e783858..c30fa1e4782 100644 --- a/analyzers/tests/SonarAnalyzer.Test/TestCases/MethodsShouldNotHaveIdenticalImplementations.CSharp9.cs +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/MethodsShouldNotHaveIdenticalImplementations.CSharp9.cs @@ -107,14 +107,14 @@ public static void Second(T? value) where T : class // Compliant, method loo Console.WriteLine(x); } - public static bool Compare1(T? value1, T value2) // Secondary + public static bool Compare1(T? value1, T value2) // Secondary [Compare] { Console.WriteLine(value1); Console.WriteLine(value2); return true; } - public static bool Compare2(T? value1, T value2) // Noncompliant + public static bool Compare2(T? value1, T value2) // Noncompliant [Compare] { Console.WriteLine(value1); Console.WriteLine(value2); @@ -127,4 +127,63 @@ public static bool Compare3(T? value1, T value2) where T : System.IComparable Console.WriteLine(value2); return true; } + + public static bool Equal1(T t1, T t2) where T : System.IEquatable // Secondary [Equal] + { + Console.WriteLine(t1); + Console.WriteLine(t2); + return true; + } + + public static bool Equal2(T t1, T t2) where T : System.IEquatable // Noncompliant [Equal] + { + Console.WriteLine(t1); + Console.WriteLine(t2); + return true; + } + + public static bool Equal3(T t1, T t2) where T : System.IEquatable // Compliant. The type constraint is different + { + Console.WriteLine(t1); + Console.WriteLine(t2); + return true; + } +} + +public class TypeConstraintsOnGenericClass +{ + public void ConstraintByTClass1() where TMethod : TClass // Secondary [ConstraintByTClass] + { + Console.WriteLine("a"); + Console.WriteLine("b"); + Console.WriteLine("c"); + } + + public void ConstraintByTClass2() where TMethod : TClass // Noncompliant [ConstraintByTClass] + { + Console.WriteLine("a"); + Console.WriteLine("b"); + Console.WriteLine("c"); + } + + public void ConstraintByTClass3() // Compliant + { + Console.WriteLine("a"); + Console.WriteLine("b"); + Console.WriteLine("c"); + } + + public void ConstraintByTClass4() where TMethod : IEquatable // Compliant + { + Console.WriteLine("a"); + Console.WriteLine("b"); + Console.WriteLine("c"); + } + + public void ConstraintByTClass5() where TMethod : struct, TClass // Compliant + { + Console.WriteLine("a"); + Console.WriteLine("b"); + Console.WriteLine("c"); + } } From 50c9dde18a2af74aab6b123db0f0e099a899dd3c Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Wed, 12 Jun 2024 13:09:54 +0200 Subject: [PATCH 23/33] Simplify --- .../MethodsShouldNotHaveIdenticalImplementationsBase.cs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs index 52a13e3da89..15d039b17dd 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs @@ -74,7 +74,7 @@ protected static bool HaveSameTypeParameters(SemanticModel model, Separ { var firstSymbols = firstTypeParameterList?.Select(x => model.GetDeclaredSymbol(x)).OfType() ?? []; var secondSymbols = secondTypeParameterList?.Select(x => model.GetDeclaredSymbol(x)).OfType().ToArray() ?? []; - return firstSymbols.All(x => Array.Exists(secondSymbols, secondSymbol => TypesAreEquivalentTypeParameters(x, secondSymbol))); + return firstSymbols.All(x => Array.Exists(secondSymbols, secondSymbol => TypeParametersHaveSameNameAndConstraints(x, secondSymbol))); } private static bool HaveSameParameterLists(SeparatedSyntaxList leftParameters, @@ -83,7 +83,7 @@ private static bool HaveSameParameterLists(SeparatedSyntaxList private static bool TypesAreSame(ITypeSymbol leftParameterType, ITypeSymbol rightParameterType) => leftParameterType.Equals(rightParameterType) // M1(int x) <-> M2(int x) - || TypesAreEquivalentTypeParameters(leftParameterType, rightParameterType) // M1(T x) where T: class <-> M2(T x) where T: class + || TypeParametersHaveSameNameAndConstraints(leftParameterType, rightParameterType) // M1(T x) where T: class <-> M2(T x) where T: class // T of M1 is a different symbol than T of M2, but if they have the same constraints, they can be interchanged || TypesAreSameGenericType(leftParameterType, rightParameterType); // M1(IEnumerable x) where T: class <-> M2(IEnumerable x) where T: class @@ -96,9 +96,6 @@ x is ITypeParameterSymbol a ? y is ITypeParameterSymbol b && a.Name == b.Name : TypesAreSame(x, y)); - private static bool TypesAreEquivalentTypeParameters(ITypeSymbol leftParameterType, ITypeSymbol rightParameterType) => - leftParameterType.Equals(rightParameterType) || TypeParametersHaveSameNameAndConstraints(leftParameterType, rightParameterType); - private static bool TypeParametersHaveSameNameAndConstraints(ITypeSymbol leftParameterType, ITypeSymbol rightParameterType) => leftParameterType is ITypeParameterSymbol left && rightParameterType is ITypeParameterSymbol right From 1ed56ed7974d8999144db8ebc7e9e183f89da50d Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Wed, 12 Jun 2024 13:12:52 +0200 Subject: [PATCH 24/33] Formatting --- .../Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs index 15d039b17dd..295e11b0552 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs @@ -84,7 +84,7 @@ private static bool HaveSameParameterLists(SeparatedSyntaxList private static bool TypesAreSame(ITypeSymbol leftParameterType, ITypeSymbol rightParameterType) => leftParameterType.Equals(rightParameterType) // M1(int x) <-> M2(int x) || TypeParametersHaveSameNameAndConstraints(leftParameterType, rightParameterType) // M1(T x) where T: class <-> M2(T x) where T: class - // T of M1 is a different symbol than T of M2, but if they have the same constraints, they can be interchanged + // T of M1 is a different symbol than T of M2, but if they have the same constraints, they can be traded || TypesAreSameGenericType(leftParameterType, rightParameterType); // M1(IEnumerable x) where T: class <-> M2(IEnumerable x) where T: class private static bool TypesAreSameGenericType(ITypeSymbol leftParameterType, ITypeSymbol rightParameterType) => From 8f9ffc3a904b7b650e3a33f841aef96fb5312f1f Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Wed, 12 Jun 2024 13:13:41 +0200 Subject: [PATCH 25/33] Grammar --- .../Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs index 295e11b0552..bb599007d64 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs @@ -84,7 +84,7 @@ private static bool HaveSameParameterLists(SeparatedSyntaxList private static bool TypesAreSame(ITypeSymbol leftParameterType, ITypeSymbol rightParameterType) => leftParameterType.Equals(rightParameterType) // M1(int x) <-> M2(int x) || TypeParametersHaveSameNameAndConstraints(leftParameterType, rightParameterType) // M1(T x) where T: class <-> M2(T x) where T: class - // T of M1 is a different symbol than T of M2, but if they have the same constraints, they can be traded + // T of M1 is a different symbol than T of M2, but if they have the same constraints and can be traded || TypesAreSameGenericType(leftParameterType, rightParameterType); // M1(IEnumerable x) where T: class <-> M2(IEnumerable x) where T: class private static bool TypesAreSameGenericType(ITypeSymbol leftParameterType, ITypeSymbol rightParameterType) => From 6a3504d7086cf49b7d33d56dd77d128baf723ba5 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Wed, 12 Jun 2024 13:17:14 +0200 Subject: [PATCH 26/33] Better is ITypeParameterSymbol check --- .../Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs index bb599007d64..693088507b9 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs @@ -92,8 +92,8 @@ private static bool TypesAreSameGenericType(ITypeSymbol leftParameterType, IType && rightParameterType is INamedTypeSymbol { IsGenericType: true } namedTypeRight && namedTypeLeft.TypeArguments.Length == namedTypeRight.TypeArguments.Length && namedTypeLeft.TypeArguments.Equals(namedTypeRight.TypeArguments, (x, y) => - x is ITypeParameterSymbol a - ? y is ITypeParameterSymbol b && a.Name == b.Name + x is ITypeParameterSymbol || y is ITypeParameterSymbol + ? x is ITypeParameterSymbol a && y is ITypeParameterSymbol b && a.Name == b.Name : TypesAreSame(x, y)); private static bool TypeParametersHaveSameNameAndConstraints(ITypeSymbol leftParameterType, ITypeSymbol rightParameterType) => From 90d582b829bce97bc37183c8eab6ed6e99928559 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Wed, 12 Jun 2024 13:19:17 +0200 Subject: [PATCH 27/33] Better names --- ...ouldNotHaveIdenticalImplementationsBase.cs | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs index 693088507b9..e4a71491238 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs @@ -77,33 +77,33 @@ protected static bool HaveSameTypeParameters(SemanticModel model, Separ return firstSymbols.All(x => Array.Exists(secondSymbols, secondSymbol => TypeParametersHaveSameNameAndConstraints(x, secondSymbol))); } - private static bool HaveSameParameterLists(SeparatedSyntaxList leftParameters, - SeparatedSyntaxList rightParameters) where TSyntax : SyntaxNode => - leftParameters.Equals(rightParameters, (left, right) => left.IsEquivalentTo(right)); // Perf: Syntactic equivalence for all parameters + private static bool HaveSameParameterLists(SeparatedSyntaxList firstParameters, + SeparatedSyntaxList secondParameters) where TSyntax : SyntaxNode => + firstParameters.Equals(secondParameters, (first, second) => first.IsEquivalentTo(second)); // Perf: Syntactic equivalence for all parameters - private static bool TypesAreSame(ITypeSymbol leftParameterType, ITypeSymbol rightParameterType) => - leftParameterType.Equals(rightParameterType) // M1(int x) <-> M2(int x) - || TypeParametersHaveSameNameAndConstraints(leftParameterType, rightParameterType) // M1(T x) where T: class <-> M2(T x) where T: class + private static bool TypesAreSame(ITypeSymbol firstParameterType, ITypeSymbol secondParameterType) => + firstParameterType.Equals(secondParameterType) // M1(int x) <-> M2(int x) + || TypeParametersHaveSameNameAndConstraints(firstParameterType, secondParameterType) // M1(T x) where T: class <-> M2(T x) where T: class // T of M1 is a different symbol than T of M2, but if they have the same constraints and can be traded - || TypesAreSameGenericType(leftParameterType, rightParameterType); // M1(IEnumerable x) where T: class <-> M2(IEnumerable x) where T: class + || TypesAreSameGenericType(firstParameterType, secondParameterType); // M1(IEnumerable x) where T: class <-> M2(IEnumerable x) where T: class - private static bool TypesAreSameGenericType(ITypeSymbol leftParameterType, ITypeSymbol rightParameterType) => - leftParameterType is INamedTypeSymbol { IsGenericType: true } namedTypeLeft - && rightParameterType is INamedTypeSymbol { IsGenericType: true } namedTypeRight + private static bool TypesAreSameGenericType(ITypeSymbol firstParameterType, ITypeSymbol secondParameterType) => + firstParameterType is INamedTypeSymbol { IsGenericType: true } namedTypeLeft + && secondParameterType is INamedTypeSymbol { IsGenericType: true } namedTypeRight && namedTypeLeft.TypeArguments.Length == namedTypeRight.TypeArguments.Length && namedTypeLeft.TypeArguments.Equals(namedTypeRight.TypeArguments, (x, y) => x is ITypeParameterSymbol || y is ITypeParameterSymbol ? x is ITypeParameterSymbol a && y is ITypeParameterSymbol b && a.Name == b.Name : TypesAreSame(x, y)); - private static bool TypeParametersHaveSameNameAndConstraints(ITypeSymbol leftParameterType, ITypeSymbol rightParameterType) => - leftParameterType is ITypeParameterSymbol left - && rightParameterType is ITypeParameterSymbol right - && left.Name == right.Name - && left.HasConstructorConstraint == right.HasConstructorConstraint - && left.HasReferenceTypeConstraint == right.HasReferenceTypeConstraint - && left.HasValueTypeConstraint == right.HasValueTypeConstraint - && left.HasUnmanagedTypeConstraint() == right.HasUnmanagedTypeConstraint() - && left.ConstraintTypes.Length == right.ConstraintTypes.Length - && left.ConstraintTypes.All(x => right.ConstraintTypes.Any(y => TypesAreSame(x, y))); + private static bool TypeParametersHaveSameNameAndConstraints(ITypeSymbol firstParameterType, ITypeSymbol secondParameterType) => + firstParameterType is ITypeParameterSymbol first + && secondParameterType is ITypeParameterSymbol second + && first.Name == second.Name + && first.HasConstructorConstraint == second.HasConstructorConstraint + && first.HasReferenceTypeConstraint == second.HasReferenceTypeConstraint + && first.HasValueTypeConstraint == second.HasValueTypeConstraint + && first.HasUnmanagedTypeConstraint() == second.HasUnmanagedTypeConstraint() + && first.ConstraintTypes.Length == second.ConstraintTypes.Length + && first.ConstraintTypes.All(x => second.ConstraintTypes.Any(y => TypesAreSame(x, y))); } From b9e7640a17e9ae11e81497d6bc30b3a36df796c9 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Thu, 13 Jun 2024 16:40:40 +0200 Subject: [PATCH 28/33] Simplify --- ...ouldNotHaveIdenticalImplementationsBase.cs | 38 +++++++++---------- 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs index e4a71491238..d3f279ed725 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs @@ -81,29 +81,27 @@ private static bool HaveSameParameterLists(SeparatedSyntaxList SeparatedSyntaxList secondParameters) where TSyntax : SyntaxNode => firstParameters.Equals(secondParameters, (first, second) => first.IsEquivalentTo(second)); // Perf: Syntactic equivalence for all parameters - private static bool TypesAreSame(ITypeSymbol firstParameterType, ITypeSymbol secondParameterType) => - firstParameterType.Equals(secondParameterType) // M1(int x) <-> M2(int x) - || TypeParametersHaveSameNameAndConstraints(firstParameterType, secondParameterType) // M1(T x) where T: class <-> M2(T x) where T: class - // T of M1 is a different symbol than T of M2, but if they have the same constraints and can be traded - || TypesAreSameGenericType(firstParameterType, secondParameterType); // M1(IEnumerable x) where T: class <-> M2(IEnumerable x) where T: class - - private static bool TypesAreSameGenericType(ITypeSymbol firstParameterType, ITypeSymbol secondParameterType) => - firstParameterType is INamedTypeSymbol { IsGenericType: true } namedTypeLeft - && secondParameterType is INamedTypeSymbol { IsGenericType: true } namedTypeRight - && namedTypeLeft.TypeArguments.Length == namedTypeRight.TypeArguments.Length - && namedTypeLeft.TypeArguments.Equals(namedTypeRight.TypeArguments, (x, y) => - x is ITypeParameterSymbol || y is ITypeParameterSymbol - ? x is ITypeParameterSymbol a && y is ITypeParameterSymbol b && a.Name == b.Name - : TypesAreSame(x, y)); - - private static bool TypeParametersHaveSameNameAndConstraints(ITypeSymbol firstParameterType, ITypeSymbol secondParameterType) => - firstParameterType is ITypeParameterSymbol first - && secondParameterType is ITypeParameterSymbol second - && first.Name == second.Name + private static bool TypeParametersHaveSameNameAndConstraints(ITypeParameterSymbol first, ITypeParameterSymbol second) => + first.Name == second.Name && first.HasConstructorConstraint == second.HasConstructorConstraint && first.HasReferenceTypeConstraint == second.HasReferenceTypeConstraint && first.HasValueTypeConstraint == second.HasValueTypeConstraint && first.HasUnmanagedTypeConstraint() == second.HasUnmanagedTypeConstraint() && first.ConstraintTypes.Length == second.ConstraintTypes.Length - && first.ConstraintTypes.All(x => second.ConstraintTypes.Any(y => TypesAreSame(x, y))); + && first.ConstraintTypes.All(x => second.ConstraintTypes.Any(y => TypeConstraintsAreSame(x, y))); + + private static bool TypeConstraintsAreSame(ITypeSymbol first, ITypeSymbol second) => + first.Equals(second) // M1(T x) where T: IComparable <-> M2(T x) where T: IComparable + || AreSameNamedTypeParameters(first, second) // M1() where T1: T2 <-> M2() where T1: T2 + // T2 of M1 is a different symbol than T2 of M2, but if they have the same name they can be traded + || TypesAreSameGenericType(first, second); // M1(IEnumerable x) where T: class <-> M2(IEnumerable x) where T: class + + private static bool TypesAreSameGenericType(ITypeSymbol firstParameterType, ITypeSymbol secondParameterType) => + firstParameterType is INamedTypeSymbol { IsGenericType: true } namedTypeLeft + && secondParameterType is INamedTypeSymbol { IsGenericType: true } namedTypeRight + && namedTypeLeft.TypeArguments.Length == namedTypeRight.TypeArguments.Length + && namedTypeLeft.TypeArguments.Equals(namedTypeRight.TypeArguments, TypeConstraintsAreSame); + + private static bool AreSameNamedTypeParameters(ITypeSymbol first, ITypeSymbol second) => + first is ITypeParameterSymbol x && second is ITypeParameterSymbol y && x.Name == y.Name; } From f17f3b351bc30074cba9b3764afec8cf7985bb19 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Thu, 13 Jun 2024 16:44:46 +0200 Subject: [PATCH 29/33] Add OriginalDefinition check and test --- .../MethodsShouldNotHaveIdenticalImplementationsBase.cs | 9 +++++---- ...thodsShouldNotHaveIdenticalImplementations.CSharp9.cs | 7 +++++++ 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs index d3f279ed725..0114b927468 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs @@ -97,10 +97,11 @@ private static bool TypeConstraintsAreSame(ITypeSymbol first, ITypeSymbol second || TypesAreSameGenericType(first, second); // M1(IEnumerable x) where T: class <-> M2(IEnumerable x) where T: class private static bool TypesAreSameGenericType(ITypeSymbol firstParameterType, ITypeSymbol secondParameterType) => - firstParameterType is INamedTypeSymbol { IsGenericType: true } namedTypeLeft - && secondParameterType is INamedTypeSymbol { IsGenericType: true } namedTypeRight - && namedTypeLeft.TypeArguments.Length == namedTypeRight.TypeArguments.Length - && namedTypeLeft.TypeArguments.Equals(namedTypeRight.TypeArguments, TypeConstraintsAreSame); + firstParameterType is INamedTypeSymbol { IsGenericType: true } namedTypeFirst + && secondParameterType is INamedTypeSymbol { IsGenericType: true } namedTypeSecond + && namedTypeFirst.OriginalDefinition.Equals(namedTypeSecond.OriginalDefinition) + && namedTypeFirst.TypeArguments.Length == namedTypeSecond.TypeArguments.Length + && namedTypeFirst.TypeArguments.Equals(namedTypeSecond.TypeArguments, TypeConstraintsAreSame); private static bool AreSameNamedTypeParameters(ITypeSymbol first, ITypeSymbol second) => first is ITypeParameterSymbol x && second is ITypeParameterSymbol y && x.Name == y.Name; diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/MethodsShouldNotHaveIdenticalImplementations.CSharp9.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/MethodsShouldNotHaveIdenticalImplementations.CSharp9.cs index c30fa1e4782..87d05ca5c61 100644 --- a/analyzers/tests/SonarAnalyzer.Test/TestCases/MethodsShouldNotHaveIdenticalImplementations.CSharp9.cs +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/MethodsShouldNotHaveIdenticalImplementations.CSharp9.cs @@ -148,6 +148,13 @@ public static bool Equal3(T t1, T t2) where T : System.IEquatable // Com Console.WriteLine(t2); return true; } + + public static bool Equal4(T t1, T t2) where T : System.IComparable // Compliant. The type constraint is different + { + Console.WriteLine(t1); + Console.WriteLine(t2); + return true; + } } public class TypeConstraintsOnGenericClass From 18aab61c4632eebc0985e67eb003143b79e356c7 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Thu, 13 Jun 2024 16:47:29 +0200 Subject: [PATCH 30/33] Better comment --- .../Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs index 0114b927468..ebb919dd997 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs @@ -93,7 +93,8 @@ private static bool TypeParametersHaveSameNameAndConstraints(ITypeParameterSymbo private static bool TypeConstraintsAreSame(ITypeSymbol first, ITypeSymbol second) => first.Equals(second) // M1(T x) where T: IComparable <-> M2(T x) where T: IComparable || AreSameNamedTypeParameters(first, second) // M1() where T1: T2 <-> M2() where T1: T2 - // T2 of M1 is a different symbol than T2 of M2, but if they have the same name they can be traded + // T2 of M1 is a different symbol than T2 of M2, but if they have the same name they can be interchanged. + // T2 equivalency is checked as well in HaveSameTypeParameters || TypesAreSameGenericType(first, second); // M1(IEnumerable x) where T: class <-> M2(IEnumerable x) where T: class private static bool TypesAreSameGenericType(ITypeSymbol firstParameterType, ITypeSymbol secondParameterType) => From 0288dd8d03cf4c10d74179d14eedb3fd5337699c Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Thu, 13 Jun 2024 16:49:06 +0200 Subject: [PATCH 31/33] Correct comment --- .../Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs index ebb919dd997..f30b930cda8 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs @@ -95,7 +95,7 @@ private static bool TypeConstraintsAreSame(ITypeSymbol first, ITypeSymbol second || AreSameNamedTypeParameters(first, second) // M1() where T1: T2 <-> M2() where T1: T2 // T2 of M1 is a different symbol than T2 of M2, but if they have the same name they can be interchanged. // T2 equivalency is checked as well in HaveSameTypeParameters - || TypesAreSameGenericType(first, second); // M1(IEnumerable x) where T: class <-> M2(IEnumerable x) where T: class + || TypesAreSameGenericType(first, second); // M1(T x) where T: IEquatable <-> M2(T x) where T: IEquatable private static bool TypesAreSameGenericType(ITypeSymbol firstParameterType, ITypeSymbol secondParameterType) => firstParameterType is INamedTypeSymbol { IsGenericType: true } namedTypeFirst From 2cdf1b3633d7587d256b12c4a6ded5e733891d69 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Wed, 19 Jun 2024 15:19:16 +0200 Subject: [PATCH 32/33] Fix name --- .../Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs index f30b930cda8..00f37aa9d5e 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs @@ -94,7 +94,7 @@ private static bool TypeConstraintsAreSame(ITypeSymbol first, ITypeSymbol second first.Equals(second) // M1(T x) where T: IComparable <-> M2(T x) where T: IComparable || AreSameNamedTypeParameters(first, second) // M1() where T1: T2 <-> M2() where T1: T2 // T2 of M1 is a different symbol than T2 of M2, but if they have the same name they can be interchanged. - // T2 equivalency is checked as well in HaveSameTypeParameters + // T2 equivalency is checked as well in TypeConstraintsAreSame || TypesAreSameGenericType(first, second); // M1(T x) where T: IEquatable <-> M2(T x) where T: IEquatable private static bool TypesAreSameGenericType(ITypeSymbol firstParameterType, ITypeSymbol secondParameterType) => From 9b25eb75b0dab8adc8cdf46453694c24e08df072 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Thu, 20 Jun 2024 15:40:52 +0200 Subject: [PATCH 33/33] Comment --- .../Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs index 00f37aa9d5e..918495c90a3 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs @@ -94,7 +94,7 @@ private static bool TypeConstraintsAreSame(ITypeSymbol first, ITypeSymbol second first.Equals(second) // M1(T x) where T: IComparable <-> M2(T x) where T: IComparable || AreSameNamedTypeParameters(first, second) // M1() where T1: T2 <-> M2() where T1: T2 // T2 of M1 is a different symbol than T2 of M2, but if they have the same name they can be interchanged. - // T2 equivalency is checked as well in TypeConstraintsAreSame + // T2 equivalency is checked as well by the TypeConstraintsAreSame call in TypeParametersHaveSameNameAndConstraints. || TypesAreSameGenericType(first, second); // M1(T x) where T: IEquatable <-> M2(T x) where T: IEquatable private static bool TypesAreSameGenericType(ITypeSymbol firstParameterType, ITypeSymbol secondParameterType) =>