Skip to content

Fix MA0065/MA0066 never checking for a GetHashCode override - #1391

Merged
meziantou merged 1 commit into
mainfrom
feature/ma0065-ma0066-gethashcode-bug-085dd7
Sep 6, 2026
Merged

meziantou merged 1 commit into
mainfrom
feature/ma0065-ma0066-gethashcode-bug-085dd7

Conversation

@meziantou

Copy link
Copy Markdown
Owner

What

DoNotUseDefaultEqualsOnValueTypeAnalyzer resolved ValueTypeGetHashCodeSymbol with nameof(ValueType.Equals) instead of nameof(ValueType.GetHashCode):

ValueTypeEqualsSymbol      = ValueTypeSymbol.GetMembers(nameof(ValueType.Equals))...;
ValueTypeGetHashCodeSymbol = ValueTypeSymbol.GetMembers(nameof(ValueType.Equals))...;  // bug

Both fields held the same symbol, so the two checks in HasDefaultEqualsOrHashCodeImplementations both tested for an Equals override and GetHashCode was never looked at.

Why it matters

A struct that overrides Equals but not GetHashCode still uses the reflection-based ValueType.GetHashCode — precisely the performance problem MA0065 and MA0066 exist to catch — yet was reported by neither rule:

#pragma warning disable CS0659
struct Test { public override bool Equals(object o) => throw null; }

_ = new Test().GetHashCode();                              // was missing MA0065
_ = new System.Collections.Generic.Dictionary<Test, int>(); // was missing MA0066

Every such struct escaped both rules. The Equals half of the check was never broken, so this is purely a set of missed diagnostics — no false positives were being produced.

Tests

Added to DoNotUseDefaultEqualsOnValueTypeAnalyzerTests:

  • GetHashCode_OnlyEqualsOverriden — MA0065 on a GetHashCode() call
  • Constructor_OnlyEqualsOverriden — MA0066 on HashSet<T> / Dictionary<TKey, TValue>
  • Equals_OnlyGetHashCodeOverriden — guards the symmetric case (passed before the fix too)

Verification

  • Reverting the fix makes 3 of the 4 new tests fail, confirming they pin the bug.
  • With the fix, 44/44 tests in the class pass on all five Roslyn versions (4.8, 4.14, 5.0, 5.6, 5.9).
  • Full suite on the default version (roslyn5.9): 3913/3913 pass.
  • dotnet run --project src/DocumentationGenerator exits 0 with no markdown changes. The MA0065/MA0066 docs already describe checking both Equals and GetHashCode, so the fix brings the implementation in line with the documented behavior and no doc update was needed.

Reviewer note

This makes the rules report in cases they previously stayed silent on, so consumers with structs that override only Equals may see new MA0065/MA0066 warnings after upgrading. That is the intended behavior of both rules.

A copy-paste bug resolved ValueTypeGetHashCodeSymbol from
ValueTypeSymbol.GetMembers(nameof(ValueType.Equals)) instead of
nameof(ValueType.GetHashCode). Both fields held the same symbol, so
HasDefaultEqualsOrHashCodeImplementations tested for an Equals override
twice and never looked at GetHashCode.

As a result, a struct that overrides Equals but not GetHashCode still uses
the reflection-based ValueType.GetHashCode -- the exact performance problem
these rules exist to catch -- yet was reported by neither MA0065 nor MA0066.
@meziantou
meziantou merged commit 9347e36 into main Sep 6, 2026
13 checks passed
@meziantou
meziantou deleted the feature/ma0065-ma0066-gethashcode-bug-085dd7 branch September 6, 2026 04:16
This was referenced Sep 6, 2026
This was referenced Sep 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant