Skip to content

Commit

Permalink
Fix S4144 FP: when type constraints are used (#9398)
Browse files Browse the repository at this point in the history
  • Loading branch information
@martin-strecker-sonarsource
martin-strecker-sonarsource authored Jun 20, 2024
1 parent 2513886 commit 33a85fe
Show file tree
Hide file tree
Showing 8 changed files with 449 additions and 84 deletions.
5 changes: 3 additions & 2 deletions analyzers/src/SonarAnalyzer.CSharp/Rules/MethodsShouldNotHaveIdenticalImplementations.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,11 @@ protected override IEnumerable<IMethodDeclaration> 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<ParameterSyntax>(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);

protected override SyntaxToken GetMethodIdentifier(IMethodDeclaration method) =>
Expand Down
118 changes: 76 additions & 42 deletions analyzers/src/SonarAnalyzer.Common/Rules/MethodsShouldNotHaveIdenticalImplementationsBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,58 +18,92 @@
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

namespace SonarAnalyzer.Rules
namespace SonarAnalyzer.Rules;

public abstract class MethodsShouldNotHaveIdenticalImplementationsBase<TSyntaxKind, TMethodDeclarationSyntax> : SonarDiagnosticAnalyzer<TSyntaxKind>
where TSyntaxKind : struct
{
public abstract class MethodsShouldNotHaveIdenticalImplementationsBase<TSyntaxKind, TMethodDeclarationSyntax> : SonarDiagnosticAnalyzer<TSyntaxKind>
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<TMethodDeclarationSyntax> GetMethodDeclarations(SyntaxNode node);
protected abstract SyntaxToken GetMethodIdentifier(TMethodDeclarationSyntax method);
protected abstract bool AreDuplicates(TMethodDeclarationSyntax firstMethod, TMethodDeclarationSyntax secondMethod);
protected abstract IEnumerable<TMethodDeclarationSyntax> 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<TMethodDeclarationSyntax>(GetMethodDeclarations(c.Node));
var methodsToHandle = new LinkedList<TMethodDeclarationSyntax>(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(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<TSyntax>(SeparatedSyntaxList<TSyntax>? leftParameters, SeparatedSyntaxList<TSyntax>? rightParameters)
where TSyntax : SyntaxNode =>
(leftParameters == null && rightParameters == null)
|| (leftParameters != null
&& rightParameters != null
&& leftParameters.Value.Count == rightParameters.Value.Count
&& leftParameters.Value.Select((left, index) => left.IsEquivalentTo(rightParameters.Value[index])).All(x => x));
}
},
SyntaxKinds);

protected virtual bool IsExcludedFromBeingExamined(SonarSyntaxNodeReportingContext context) =>
context.ContainingSymbol.Kind != SymbolKind.NamedType;

protected static bool HaveSameParameters<TSyntax>(SeparatedSyntaxList<TSyntax>? leftParameters, SeparatedSyntaxList<TSyntax>? rightParameters)
where TSyntax : SyntaxNode =>
(leftParameters is null && rightParameters is null) // In VB.Net the parameter list can be omitted
|| (leftParameters?.Count == rightParameters?.Count && HaveSameParameterLists(leftParameters.Value, rightParameters.Value));

protected static bool HaveSameTypeParameters<TSyntax>(SemanticModel model, SeparatedSyntaxList<TSyntax>? firstTypeParameterList, SeparatedSyntaxList<TSyntax>? secondTypeParameterList)
where TSyntax : SyntaxNode
{
var firstSymbols = firstTypeParameterList?.Select(x => model.GetDeclaredSymbol(x)).OfType<ITypeParameterSymbol>() ?? [];
var secondSymbols = secondTypeParameterList?.Select(x => model.GetDeclaredSymbol(x)).OfType<ITypeParameterSymbol>().ToArray() ?? [];
return firstSymbols.All(x => Array.Exists(secondSymbols, secondSymbol => TypeParametersHaveSameNameAndConstraints(x, secondSymbol)));
}

private static bool HaveSameParameterLists<TSyntax>(SeparatedSyntaxList<TSyntax> firstParameters,
SeparatedSyntaxList<TSyntax> secondParameters) where TSyntax : SyntaxNode =>
firstParameters.Equals(secondParameters, (first, second) => first.IsEquivalentTo(second)); // Perf: Syntactic equivalence for all parameters

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 => TypeConstraintsAreSame(x, y)));

private static bool TypeConstraintsAreSame(ITypeSymbol first, ITypeSymbol second) =>
first.Equals(second) // M1<T>(T x) where T: IComparable <-> M2<T>(T x) where T: IComparable
|| AreSameNamedTypeParameters(first, second) // M1<T1, T2>() where T1: T2 <-> M2<T1, T2>() 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 by the TypeConstraintsAreSame call in TypeParametersHaveSameNameAndConstraints.
|| TypesAreSameGenericType(first, second); // M1<T>(T x) where T: IEquatable<T> <-> M2<T>(T x) where T: IEquatable<T>

private static bool TypesAreSameGenericType(ITypeSymbol firstParameterType, ITypeSymbol secondParameterType) =>
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;
}
3 changes: 2 additions & 1 deletion ...yzers/src/SonarAnalyzer.VisualBasic/Rules/MethodsShouldNotHaveIdenticalImplementations.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,11 @@ public sealed class MethodsShouldNotHaveIdenticalImplementations : MethodsShould
protected override IEnumerable<MethodBlockSyntax> GetMethodDeclarations(SyntaxNode node) =>
((TypeBlockSyntax)node).Members.OfType<MethodBlockSyntax>();

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())
&& HaveSameTypeParameters(model, firstMethod.SubOrFunctionStatement?.TypeParameterList?.Parameters, secondMethod.SubOrFunctionStatement?.TypeParameterList?.Parameters)
&& VisualBasicEquivalenceChecker.AreEquivalent(firstMethod.Statements, secondMethod.Statements);

protected override SyntaxToken GetMethodIdentifier(MethodBlockSyntax method) =>
Expand Down
6 changes: 2 additions & 4 deletions analyzers/tests/SonarAnalyzer.Test/Rules/AspNet/UseAspNetModelBindingTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,7 @@ static void StaticLocalFunction(HttpRequest request)
}
"""").Verify();

[TestMethod]
[CombinatorialData]
[CombinatorialDataTestMethod]
public void UseAspNetModelBinding_CompliantAccess(
[DataValues(
"_ = {0}.Keys",
Expand Down Expand Up @@ -293,8 +292,7 @@ public void HandleRequest(HttpRequest request)
}
"""").VerifyNoIssues();

[TestMethod]
[CombinatorialData]
[CombinatorialDataTestMethod]
public void UseAspNetModelBinding_InheritanceAccess(
[DataValues(
": Controller",
Expand Down
Loading

0 comments on commit 33a85fe

Please sign in to comment.