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,10 +92,10 @@ 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];
ITypeSymbol? reportedType = ShouldReport(comparedType, genericEnumerableSymbol)
ITypeSymbol? reportedType = ShouldReport(comparedType, genericEnumerableSymbol, equatableSymbol)
Comment thread
Evangelink marked this conversation as resolved.
Outdated
? comparedType
: GetCollectionArgumentType(invocation, firstParameterName, genericEnumerableSymbol)
?? GetCollectionArgumentType(invocation, "actual", genericEnumerableSymbol);
: GetCollectionArgumentType(invocation, firstParameterName, genericEnumerableSymbol, equatableSymbol)
?? GetCollectionArgumentType(invocation, "actual", genericEnumerableSymbol, equatableSymbol);
Comment thread
Evangelink marked this conversation as resolved.
Outdated

if (reportedType is null)
{
Expand All @@ -104,7 +107,7 @@ targetMethod.Name is not ("AreEqual" or "AreNotEqual") ||
context.ReportDiagnostic(invocation.CreateDiagnostic(Rule, methodName, comparedTypeDisplay));
}

private static ITypeSymbol? GetCollectionArgumentType(IInvocationOperation invocation, string parameterName, INamedTypeSymbol genericEnumerableSymbol)
private static ITypeSymbol? GetCollectionArgumentType(IInvocationOperation invocation, string parameterName, INamedTypeSymbol genericEnumerableSymbol, INamedTypeSymbol? equatableSymbol)
{
IArgumentOperation? argument = invocation.Arguments.FirstOrDefault(arg => arg.Parameter?.Name == parameterName);

Expand All @@ -113,14 +116,48 @@ targetMethod.Name is not ("AreEqual" or "AreNotEqual") ||
// operand to a non-collection type (or vice versa), so the call-site static type — the result
// of the user-defined conversion — is what the user wrote and what we should reason about.
ITypeSymbol? argumentType = argument?.Value.WalkDownBuiltInConversion().Type;
return argumentType is not null && ShouldReport(argumentType, genericEnumerableSymbol)
return argumentType is not null && ShouldReport(argumentType, genericEnumerableSymbol, equatableSymbol)
? argumentType
: null;
}

private static bool ShouldReport(ITypeSymbol comparedType, INamedTypeSymbol genericEnumerableSymbol)
private static bool ShouldReport(ITypeSymbol comparedType, INamedTypeSymbol genericEnumerableSymbol, INamedTypeSymbol? equatableSymbol)
=> comparedType.SpecialType != SpecialType.System_String
&& ImplementsGenericEnumerable(comparedType, genericEnumerableSymbol);
&& ImplementsGenericEnumerable(comparedType, genericEnumerableSymbol)
// When the compared type declares its own equality (implements IEquatable&lt;itself&gt; or overrides
// object.Equals), the author has deliberately opted into a custom, non-reference equality. Assert.AreEqual
// then honors that intent via EqualityComparer&lt;T&gt;.Default, so suggesting a sequence/structural comparison
// would be second-guessing a deliberate decision (see issue #9971). We only warn on collection types that
// fall back to reference equality — the actual footgun this rule exists to catch.
&& !DeclaresOwnEquality(comparedType, equatableSymbol);

// Type parameters can't declare their own equality; their constraints drive ImplementsGenericEnumerable,
// but a bare `where T : IEnumerable<...>` is still the reference-equality footgun we want to flag.
private static bool DeclaresOwnEquality(ITypeSymbol type, INamedTypeSymbol? equatableSymbol)
=> type is INamedTypeSymbol namedType
Comment thread
Evangelink marked this conversation as resolved.
Outdated
&& ((equatableSymbol is not null &&
namedType.AllInterfaces.Any(i =>
Comment thread
Evangelink marked this conversation as resolved.
Outdated
SymbolEqualityComparer.Default.Equals(i.OriginalDefinition, equatableSymbol) &&
i.TypeArguments.Length == 1 &&
SymbolEqualityComparer.Default.Equals(i.TypeArguments[0], namedType)))
|| OverridesObjectEquals(namedType));

private static bool OverridesObjectEquals(INamedTypeSymbol type)
{
for (INamedTypeSymbol? current = type; current is not null && current.SpecialType != SpecialType.System_Object; current = current.BaseType)
Comment thread
Evangelink marked this conversation as resolved.
Outdated
{
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)
Comment thread
Evangelink marked this conversation as resolved.
Outdated
{
return true;
}
}
Comment thread
Evangelink marked this conversation as resolved.
}

return false;
}

private static bool HasNullLiteralArgument(IInvocationOperation invocation, string 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
Original file line number Diff line number Diff line change
Expand Up @@ -826,6 +826,160 @@ public sealed class Wrapper
await VerifyCS.VerifyAnalyzerAsync(code);
}

[TestMethod]
public async Task WhenUsingAssertAreEqualOnCollectionImplementingIEquatable_DoNotReportDiagnostic()
{
// The type is a collection but declares its own equality via IEquatable<self>, so Assert.AreEqual
// honors that intentional equality. Suggesting a sequence comparison would second-guess the author (issue #9971).
string code = """
#nullable enable
using System;
using System.Collections;
using System.Collections.Generic;
using Microsoft.VisualStudio.TestTools.UnitTesting;

[TestClass]
public class MyTestClass
{
[TestMethod]
public void MyTestMethod()
{
MyCollection c1 = new();
MyCollection c2 = new();
Assert.AreEqual(c1, c2);
}

private sealed class MyCollection : IEnumerable<int>, IEquatable<MyCollection>
{
public bool Equals(MyCollection? other) => true;

public override bool Equals(object? obj) => obj is MyCollection other && Equals(other);

public override int GetHashCode() => 0;
Comment thread
Evangelink marked this conversation as resolved.
Outdated

public IEnumerator<int> GetEnumerator() => new List<int>().GetEnumerator();

IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
}
}
""";

await VerifyCS.VerifyAnalyzerAsync(code);
}

[TestMethod]
public async Task WhenUsingAssertAreNotEqualOnCollectionImplementingIEquatable_DoNotReportDiagnostic()
{
string code = """
#nullable enable
using System;
using System.Collections;
using System.Collections.Generic;
using Microsoft.VisualStudio.TestTools.UnitTesting;

[TestClass]
public class MyTestClass
{
[TestMethod]
public void MyTestMethod()
{
MyCollection c1 = new();
MyCollection c2 = new();
Assert.AreNotEqual(c1, c2);
}

private sealed class MyCollection : IEnumerable<int>, IEquatable<MyCollection>
{
public bool Equals(MyCollection? other) => true;

public override bool Equals(object? obj) => obj is MyCollection other && Equals(other);

public override int GetHashCode() => 0;

public IEnumerator<int> GetEnumerator() => new List<int>().GetEnumerator();

IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
}
}
""";

await VerifyCS.VerifyAnalyzerAsync(code);
}

[TestMethod]
public async Task WhenUsingAssertAreEqualOnCollectionOverridingObjectEquals_DoNotReportDiagnostic()
{
// Overriding object.Equals is the same intentional-equality signal as IEquatable<self>.
string code = """
#nullable enable
using System.Collections;
using System.Collections.Generic;
using Microsoft.VisualStudio.TestTools.UnitTesting;

[TestClass]
public class MyTestClass
{
[TestMethod]
public void MyTestMethod()
{
MyCollection c1 = new();
MyCollection c2 = new();
Assert.AreEqual(c1, c2);
}

private sealed class MyCollection : IEnumerable<int>
{
public override bool Equals(object? obj) => obj is MyCollection;

public override int GetHashCode() => 0;

public IEnumerator<int> GetEnumerator() => new List<int>().GetEnumerator();

IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
}
}
""";

await VerifyCS.VerifyAnalyzerAsync(code);
}

[TestMethod]
public async Task WhenUsingAssertAreEqualOnCollectionImplementingIEquatableOfOtherType_ReportDiagnostic()
{
// IEquatable<SomethingElse> is not used by EqualityComparer<MyCollection>.Default, so the type still
// falls back to reference equality — the footgun the rule targets.
string code = """
#nullable enable
using System;
using System.Collections;
using System.Collections.Generic;
using Microsoft.VisualStudio.TestTools.UnitTesting;

[TestClass]
public class MyTestClass
{
[TestMethod]
public void MyTestMethod()
{
MyCollection c1 = new();
MyCollection c2 = new();
{|#0:Assert.AreEqual(c1, c2)|};
}

private sealed class MyCollection : IEnumerable<int>, IEquatable<string>
{
public bool Equals(string? other) => false;

public IEnumerator<int> GetEnumerator() => new List<int>().GetEnumerator();

IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
}
}
""";

await VerifyCS.VerifyAnalyzerAsync(code, ExpectedDiagnostic("Assert.AreEqual", "MyCollection"));
}

private static DiagnosticResult ExpectedDiagnostic(string methodName, string typeName)
=> VerifyCS.Diagnostic().WithLocation(0).WithArguments(methodName, typeName);

Expand Down
Loading