feat(analyzers): detect collection IsEqualTo reference equality (TUnitAssertions0016)

[thomhurst]

Summary

• Add TUnitAssertions0016 (Info): flags .IsEqualTo(...) on collection assertion sources where reference equality is used instead of content equivalence.
• Add code fix to rewrite .IsEqualTo(...) to .IsEquivalentTo(...).
• Filters to the generic EqualsAssertion-returning overload so specialized overloads (Count().IsEqualTo(...), DateTimeEqualsAssertion, ...) are not flagged.
• Excludes string, Memory<T>, ReadOnlyMemory<T>, Span<T>, ReadOnlySpan<T> from collection detection.

Test plan

• Analyzer test: List<int>.IsEqualTo raises info
• Analyzer test: string.IsEqualTo not flagged
• Analyzer test: int.IsEqualTo not flagged
• Analyzer test: int[].IsEqualTo raises info
• Analyzer test: Count().IsEqualTo (specialized overload) not flagged
• Code fix test: rewrites IsEqualTo to IsEquivalentTo
• Code fix test: preserves chained .And.IsNotNull() calls
• Code fix test: works on arrays
• 85 total tests pass (67 analyzer + 18 codefixer on net9.0)

[claude]

Code Review: TUnitAssertions0016 — Collection IsEqualTo Reference Equality

Good addition overall. The structure follows existing analyzer patterns well, the code fix is correct and trivia-preserving, and the test matrix is solid. A few things worth discussing:

---

Design Concern: False Positives for Custom Collections with Value Equality

The core issue: IsCollectionWithoutStructuralEquality checks whether a type implements IEnumerable, but it does not check whether the type overrides Equals. This means the analyzer will fire on any custom collection that does implement structural equality via an Equals override:

publicclassMyOrderedSet:IEnumerable<int>{publicoverrideboolEquals(object?obj)=>/* structural comparison */;// ...}awaitAssert.That(newMyOrderedSet()).IsEqualTo(newMyOrderedSet());// ← flagged as Info, but wrong!

The method name IsCollectionWithoutStructuralEquality implies it checks for absence of structural equality, but it actually only checks for presence of IEnumerable. The fix is to also verify that Equals is not overridden beyond object:

privatestaticboolIsCollectionWithoutStructuralEquality(ITypeSymboltype,INamedTypeSymbolienumerable){// ... existing exclusions ...if(!ImplementsIEnumerable(type,ienumerable))returnfalse;// Only flag if Equals is not overridden (i.e., reference equality is used)return!HasEqualsOverride(type);}privatestaticboolHasEqualsOverride(ITypeSymboltype){varcurrent=type;while(currentis not null&&current.SpecialType!=SpecialType.System_Object){if(current.GetMembers("Equals").OfType<IMethodSymbol>().Any(m =>m.IsOverride&&m.Parameters.Length==1&&m.Parameters[0].Type.SpecialType==SpecialType.System_Object))returntrue;current=current.BaseType;}returnfalse;}

This would make the diagnostic semantically accurate and eliminate false positives.

---

Missing Test: Custom IEnumerable with Equals Override

There is no test covering the false-positive case described above — a custom IEnumerable-implementing type that overrides Equals. Adding that test would both document the intended behavior and guard against regressions.

---

Minor: Resources.resx Missing Trailing Newline

The diff shows \ No newline at end of file for Resources.resx. Other entries in that file likely have a trailing newline; this should be consistent.

---

Observation: CompilationStart for IEnumerable Symbol Caching

Wrapping the operation action in RegisterCompilationStartAction to cache the IEnumerable symbol is a good pattern — it avoids re-resolving the symbol on every invocation. This is better than the approach in some other analyzers (e.g., PreferIsNullAnalyzer) that don't cache. Worth keeping, and potentially worth applying to other analyzers in a follow-up.

---

Summary

Overall	Good implementation with one real design gap
Blocking	False-positive for custom IEnumerable types that override Equals — the diagnostic message would be incorrect in that case
Non-blocking	Missing negative test for that case; Resources.resx trailing newline

The fix for the blocking issue is straightforward: add an Equals-override check inside IsCollectionWithoutStructuralEquality. Everything else — the code fix provider, the span reporting, the chained-call preservation, the exclusions for string/Memory/Span — looks correct.

[codacy-production]

Up to standards ✅

🟢 Issues0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics59 complexity

Metric	Results
Complexity	59

View in Codacy

_{TIP This summary will be updated as you push new changes. Give us feedback}

[thomhurst]

Thanks for the review. Addressed in 0d4f6ce51:

• Blocking — false positive on custom collections with Equals override: added OverridesObjectEquals walk up the base chain; IsCollectionWithoutStructuralEquality now returns false when an override is present. Covers records (synthesized Equals(object) override) and hand-rolled overrides.
• Missing test: added CustomEnumerable_With_EqualsOverride_Not_Flagged and Record_Collection_Not_Flagged.
• Resources.resx trailing newline: fixed.

All 69 analyzer tests + 18 code fixer tests pass (net9.0).

[claude]

Code Review: TUnitAssertions0016 — Follow-up (v2)

Previous Review vs. Current Code

My prior review flagged three issues. Looking at the current diff carefully, all three were already present and handled in the original submission — the review was inaccurate. To set the record straight:

Previous concern	Status
False positives for custom IEnumerable with Equals override	Already handled — OverridesObjectEquals was already in the code
Missing test for custom IEnumerable + Equals override	Already present — CustomEnumerable_With_EqualsOverride_Not_Flagged test
Resources.resx trailing newline	Fixed — the diff no longer shows \ No newline at end of file

Apologies for the noise in the prior review.

---

New Concern: IEquatable<T> Gap

OverridesObjectEquals checks for Equals(object) overrides but not for IEquatable\<T\> implementation. This matters because EqualityComparer\<T\>.Default (which TUnit's IsEqualTo likely uses) prefers IEquatable\<T\>.Equals when implemented, making the comparison structural — not reference-based.

A type like this would be falsely flagged:

publicclassMyCollection:IEnumerable<int>,IEquatable<MyCollection>{publicboolEquals(MyCollection?other)=>/* structural */;// Does NOT override Equals(object) separately}awaitAssert.That(newMyCollection()).IsEqualTo(newMyCollection());// ← falsely flagged

Fix — extend the guard in IsCollectionWithoutStructuralEquality:

privatestaticboolImplementsIEquatableOfSelf(INamedTypeSymboltype){returntype.AllInterfaces.Any(i =>i.Name=="IEquatable"&&i.TypeArguments.Length==1&&SymbolEqualityComparer.Default.Equals(i.TypeArguments[0],type));}

And add || ImplementsIEquatableOfSelf(...)" to the return condition alongside OverridesObjectEquals`.

This is a low-probability edge case (most types that implement IEquatable\<T\> also override Equals(object)), but worth closing for correctness.

---

Everything Else Looks Good

• RegistrationCompilationStartAction + caching the IEnumerable symbol: correct and efficient.
• Span reporting (TextSpan.FromBounds) is precise — highlights only IsEqualTo(...) rather than the full chain.
• Code fix correctly replaces the identifier via SyntaxFactory.IdentifierName with trivia preserved, leaving chained calls intact.
• Exclusions for string, Memory\<T\>, Span\<T\>, ReadOnlySpan\<T\>, ReadOnlyMemory\<T\> are correct and tested.
• Record exclusion (Record_Collection_Not_Flagged) correctly handles synthesised Equals from record types.
• WellKnownFixAllProviders.BatchFixer is the right choice for a simple rename fix.

The only actionable gap is the IEquatable\<T\> case. Given that DiagnosticSeverity.Info keeps this non-blocking for users, this is non-blocking for the PR as well — but worth a follow-up or inline fix.

[thomhurst]

Addressed the IEquatable<T> gap in da8001ee1:

• Added ImplementsIEquatableOfSelf walk over AllInterfaces, comparing the type argument to the implementing type with SymbolEqualityComparer.Default.
• IsCollectionWithoutStructuralEquality now gates on both !OverridesObjectEquals && !ImplementsIEquatableOfSelf.
• New test CustomEnumerable_With_IEquatable_Not_Flagged covers the case where IEquatable<self> is implemented without an Equals(object) override.

8/8 CollectionIsEqualToAnalyzerTests pass.

[claude]

Code Review: TUnitAssertions0016 — v3 (current state)

Previous Reviews vs. Current State

The PR has evolved through two prior rounds of review feedback:

Concern	Addressed by
False positives for custom IEnumerable without Equals override (Review 1)	commit 0d4f6ce5 — added OverridesObjectEquals
IEquatable<T> implementations not excluded (Review 2)	commit da8001ee — added ImplementsIEquatableOfSelf

Both concerns are now correctly handled and tested. The analyzer properly skips collections that implement structural equality via either mechanism.

---

Current Code Assessment

IsCollectionWithoutStructuralEquality now correctly returns false when:

• The type is string, Memory<T>, ReadOnlyMemory<T>, Span<T>, or ReadOnlySpan<T>
• The type does not implement IEnumerable
• The type overrides Equals(object)
• The type implements IEquatable<T> of itself

This is semantically accurate. The return condition !OverridesObjectEquals(type) && !ImplementsIEquatableOfSelf(type) is correct and complete.

---

Minor Observation: Redundant Span/Memory Exclusions

Span<T> and ReadOnlySpan<T> are ref structs and cannot implement interfaces — so ImplementsIEnumerable would already return false for them. Similarly, Memory<T> and ReadOnlyMemory<T> do not implement IEnumerable. The explicit exclusions are technically redundant, but they serve as clear documentation of intent and add no cost. Not worth changing.

---

Everything Looks Good

• Analyzer logic: Sound. The CompilationStartAction pattern for caching the IEnumerable symbol is correct and efficient.
• Span reporting: TextSpan.FromBounds(memberAccess.Name.SpanStart, invocationSyntax.Span.End) highlights only IsEqualTo(...) rather than the full chain. Precise and correct.
• Code fix: Simple identifier rename via SyntaxFactory.IdentifierName with WithTriviaFrom is the right approach. Preserves chained calls and does not require any AST restructuring.
• GetFixAllProvider: WellKnownFixAllProviders.BatchFixer is correct for a pure rename fix.
• Test coverage: All meaningful cases are covered — List, array, string, int, Count() overload, record, IEquatable, Equals override.

This is ready to merge.