Skip to content
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,14 @@ public override void Initialize(AnalysisContext context)
return;
}

context.RegisterOperationAction(context => AnalyzeInvocation(context, assertSymbol, genericEnumerableSymbol), OperationKind.Invocation);
// May be null on very old target frameworks; the equality-opt-out check simply becomes a no-op then.
INamedTypeSymbol? equatableSymbol = compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemIEquatable1);

context.RegisterOperationAction(context => AnalyzeInvocation(context, assertSymbol, genericEnumerableSymbol, equatableSymbol), OperationKind.Invocation);
});
}

private static void AnalyzeInvocation(OperationAnalysisContext context, INamedTypeSymbol assertSymbol, INamedTypeSymbol genericEnumerableSymbol)
private static void AnalyzeInvocation(OperationAnalysisContext context, INamedTypeSymbol assertSymbol, INamedTypeSymbol genericEnumerableSymbol, INamedTypeSymbol? equatableSymbol)
{
var invocation = (IInvocationOperation)context.Operation;
IMethodSymbol targetMethod = invocation.TargetMethod;
Expand Down Expand Up @@ -89,6 +92,21 @@ targetMethod.Name is not ("AreEqual" or "AreNotEqual") ||
// patterns where the caller widened to a non-collection type (e.g. `Assert.AreEqual<object>(arr1, arr2)`
// or `Assert.AreEqual((object)arr1, (object)arr2)`) which would otherwise silently use reference equality.
ITypeSymbol comparedType = targetMethod.TypeArguments[0];

// Opt out when the compared type declares its own equality (implements IEquatable&lt;itself&gt; or overrides
// object.Equals). The author has then deliberately chosen a custom, non-reference equality that Assert.AreEqual
// honors via EqualityComparer&lt;T&gt;.Default, so suggesting a sequence/structural comparison would second-guess
// a deliberate decision (see issue #9971). Two things are important here:
// * The check must be based on the *selected generic type argument* T, not the argument's runtime collection
// type. Widening to a base type (e.g. Assert.AreEqual&lt;object&gt;(collection, collection)) discards the
// collection's IEquatable&lt;self&gt; and falls back to reference equality — exactly the footgun the rule targets.
// * It must not apply when the caller supplies an explicit comparer, because then EqualityComparer&lt;T&gt;.Default
// (and therefore the type's own equality) is not used at all.
if (!HasComparerArgument(invocation) && DeclaresOwnEquality(comparedType, equatableSymbol))
{
return;
}

ITypeSymbol? reportedType = ShouldReport(comparedType, genericEnumerableSymbol)
? comparedType
: GetCollectionArgumentType(invocation, firstParameterName, genericEnumerableSymbol)
Expand All @@ -104,6 +122,19 @@ targetMethod.Name is not ("AreEqual" or "AreNotEqual") ||
context.ReportDiagnostic(invocation.CreateDiagnostic(Rule, methodName, comparedTypeDisplay));
}

private static bool HasComparerArgument(IInvocationOperation invocation)
{
foreach (IArgumentOperation argument in invocation.Arguments)
{
if (argument.Parameter?.Name == "comparer")
{
return true;
}
Comment thread
Evangelink marked this conversation as resolved.
Outdated
}
Comment thread
github-code-quality[bot] marked this conversation as resolved.
Fixed

return false;
}

private static ITypeSymbol? GetCollectionArgumentType(IInvocationOperation invocation, string parameterName, INamedTypeSymbol genericEnumerableSymbol)
{
IArgumentOperation? argument = invocation.Arguments.FirstOrDefault(arg => arg.Parameter?.Name == parameterName);
Expand All @@ -122,6 +153,109 @@ private static bool ShouldReport(ITypeSymbol comparedType, INamedTypeSymbol gene
=> comparedType.SpecialType != SpecialType.System_String
&& ImplementsGenericEnumerable(comparedType, genericEnumerableSymbol);

private static bool DeclaresOwnEquality(ITypeSymbol type, INamedTypeSymbol? equatableSymbol)
{
switch (type)
{
case INamedTypeSymbol namedType:
return ImplementsSelfEquatable(namedType, namedType, equatableSymbol) || OverridesObjectEquals(namedType);

// EqualityComparer&lt;T&gt;.Default honors a `where T : IEquatable<T>` constraint, and a class constraint that
// overrides object.Equals is inherited by every T. A bare `where T : IEnumerable<...>` constraint has
// neither, so it stays the reference-equality footgun we still want to flag.
//
// The equality target stays `typeParameter` (T) while we traverse constraints. We must NOT recurse with the
// constraint as the new target: for `where T : ISelf` with `ISelf : IEquatable<ISelf>`, a concrete T need
// not implement IEquatable&lt;T&gt;, so EqualityComparer&lt;T&gt;.Default would still use reference equality.
case ITypeParameterSymbol typeParameter:
foreach (ITypeSymbol constraintType in typeParameter.ConstraintTypes)
{
if (constraintType is INamedTypeSymbol namedConstraint &&
(ImplementsSelfEquatable(namedConstraint, typeParameter, equatableSymbol) || OverridesObjectEquals(namedConstraint)))
Comment thread
Evangelink marked this conversation as resolved.
Outdated
{
return true;
}
}

return false;

default:
return false;
}
}

// Returns true when `candidate` is or implements IEquatable&lt;equalityType&gt;, i.e. the equality contract that
// EqualityComparer&lt;equalityType&gt;.Default would dispatch to. `equalityType` is the type whose default comparer
// is used (the compared type or the type parameter); `candidate` is the type (or constraint) we inspect.
private static bool ImplementsSelfEquatable(INamedTypeSymbol candidate, ITypeSymbol equalityType, INamedTypeSymbol? equatableSymbol)
{
if (equatableSymbol is null)
{
return false;
}

// The candidate can be the IEquatable&lt;T&gt; interface itself (e.g. a `where T : IEquatable<T>` constraint)
// or a type that lists it among its implemented interfaces (e.g. `class C : IEquatable<C>`).
if (IsEquatableOf(candidate, equalityType, equatableSymbol))
{
return true;
}

foreach (INamedTypeSymbol implemented in candidate.AllInterfaces)
{
if (IsEquatableOf(implemented, equalityType, equatableSymbol))
{
return true;
}
}
Comment thread
Evangelink marked this conversation as resolved.

return false;
}

private static bool IsEquatableOf(INamedTypeSymbol type, ITypeSymbol equalityType, INamedTypeSymbol equatableSymbol)
=> SymbolEqualityComparer.Default.Equals(type.OriginalDefinition, equatableSymbol)
&& type.TypeArguments.Length == 1
&& SymbolEqualityComparer.Default.Equals(type.TypeArguments[0], equalityType);
Comment thread
Evangelink marked this conversation as resolved.

private static bool OverridesObjectEquals(INamedTypeSymbol type)
{
// Stop before object AND ValueType: System.ValueType overrides object.Equals, so walking into it would treat
// every struct that implements IEnumerable<T> as declaring its own equality even when it has no Equals of its
// own (its backing array/list field would then be compared by reference). An explicit struct override is found
// on the concrete type before we reach ValueType.
for (INamedTypeSymbol? current = type;
current is not null && current.SpecialType is not (SpecialType.System_Object or SpecialType.System_ValueType or SpecialType.System_Enum);
current = current.BaseType)
{
foreach (ISymbol member in current.GetMembers(nameof(Equals)))
{
if (member is IMethodSymbol { IsOverride: true, Parameters.Length: 1, ReturnType.SpecialType: SpecialType.System_Boolean } method &&
method.Parameters[0].Type.SpecialType == SpecialType.System_Object &&
OverrideRootIsObjectEquals(method))
{
return true;
}
}
Comment thread
Evangelink marked this conversation as resolved.
}

return false;
}

// `IsOverride` only proves the method overrides *some* virtual slot. A base class can declare
// `new virtual bool Equals(object)`, and a derived override of that member is not an override of
// object.Equals — EqualityComparer&lt;T&gt;.Default still dispatches the unchanged object.Equals slot
// (reference equality). Follow the override chain to its root and require it to be object.Equals.
private static bool OverrideRootIsObjectEquals(IMethodSymbol method)
{
IMethodSymbol root = method;
while (root.OverriddenMethod is { } overridden)
{
root = overridden;
}

return root.ContainingType?.SpecialType == SpecialType.System_Object;
}

private static bool HasNullLiteralArgument(IInvocationOperation invocation, string parameterName)
{
IArgumentOperation? argument = invocation.Arguments.FirstOrDefault(arg => arg.Parameter?.Name == parameterName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ internal static class WellKnownTypeNames
public const string SystemFunc1 = "System.Func`1";
public const string SystemIAsyncDisposable = "System.IAsyncDisposable";
public const string SystemIDisposable = "System.IDisposable";
public const string SystemIEquatable1 = "System.IEquatable`1";
public const string SystemLinqEnumerable = "System.Linq.Enumerable";
public const string SystemLinqExpressionsExpression1 = "System.Linq.Expressions.Expression`1";
public const string SystemOperatingSystem = "System.OperatingSystem";
Expand Down
Loading
Loading