Replace Location with value-equatable LocationInfo — fix incremental cache invalidation on Clone

[Skymly]

Summary

• Introduce LocationInfo, a value-equatable struct that replaces Location in all incremental generator models. Location uses reference equality, which breaks model value equality when Compilation.Clone() creates new Location objects — this forced AssertCacheHit to use the same compilation (not Clone), weakening the cache test.
• Replace Location with LocationInfo in 9 record model types: KeyedRegistration, HandlerRegistration, CompositeRegistration, DecoratorRegistration, EventHandlerRegistration, SingletonTargetInfo, StateMachineModel, TransitionModel, StateParentModel.
• Update DiagnosticInfo to use LocationInfo? with value equality instead of ReferenceEquals.
• Fix List<T> return types in KeyedRegistration and HandlerRegistration transforms to EquatableArray<T> — List<T> uses reference equality, which broke caching even with LocationInfo. EquatableArray<T> provides value equality for correct incremental caching.
• AssertCacheHit now uses Compilation.Clone() for the second run, verifying that LocationInfo value equality holds across cloned compilations.
• Add LocationInfo_Equals_AfterClone test verifying LocationInfo equality after Clone.

What this fixes

Before: AssertCacheHit had this comment:

We use the same compilation (not Clone) because Location uses reference equality: Clone forces the transform to re-execute, producing new Location objects that break model value equality even though the semantic content is identical.

After: All 7 cache tests pass with Clone(), proving that LocationInfo value equality holds across cloned compilations. This is a stronger test that verifies the incremental pipeline works correctly even when Roslyn internally clones compilations.

Design

LocationInfo stores FilePath (string), TextSpan (int Start + Length), and LineSpan (LinePositionSpan) — all value types with structural equality. The ToLocation() method reconstructs a Location for diagnostic reporting via Location.Create(filePath, textSpan, lineSpan).

Related Issue

Closes #215

Solution module

• SourceGenerators

Type of change

• Bug fix (incremental cache invalidation)
• Test improvement (stronger cache test with Clone)

Test plan

• dotnet build DesignPatterns.slnx -c Release — 0 errors, 0 warnings
• dotnet test DesignPatterns.slnx -c Release --no-build — all 642 tests pass
• All 7 CachesOnSecondRun tests pass with Compilation.Clone() (previously used same compilation)
• New LocationInfo_Equals_AfterClone test verifies LocationInfo equality after Clone

Breaking changes

• None (internal model types only, no public API changes)

[Skymly]

Review: PR #216 — Replace Location with value-equatable LocationInfo

This is a well-executed fix for a subtle but important incremental pipeline bug. The root cause analysis (Location reference equality + List reference equality breaking cache on Clone) is correct, and the fix is comprehensive. A few observations:

Issues

1. GetHashCode uses non-ordinal string hashing
Equals uses StringComparison.Ordinal for FilePath, but GetHashCode uses FilePath?.GetHashCode() which uses the current culture's hash. On .NET 5+ this is typically the same, but on netstandard2.0 the default string.GetHashCode() is culture-sensitive and can differ from ordinal. If two LocationInfo instances have the same ordinal-equal FilePath but different culture-based hash codes, they'd be Equals but have different hashes — violating the hash contract.

Fix: use StringComparer.Ordinal.GetHashCode(FilePath ?? "") instead of FilePath?.GetHashCode().

2. No test for ToLocation() round-trip
LocationInfo_Equals_AfterClone verifies value equality survives Clone, but there's no test that ToLocation() produces a Location with the correct FilePath, SourceSpan, and LineSpan. Since ToLocation() creates an ExternalFileLocation (not a syntax-tree-backed location), it's worth a unit test to confirm the round-trip preserves the diagnostic coordinates. This matters because if ToLocation() returns wrong coordinates, diagnostics will point to the wrong location in the IDE.

3. DiagnosticInfo.MessageArgsEqual uses EqualityComparer<object?>.Default
This is pre-existing (not introduced by this PR), but worth noting: MessageArgs are object?[] and compared with default equality. If a message arg is a struct (e.g. int order value), EqualityComparer<object?>.Default boxes and uses value equality — this works. But if an arg is a string, it uses string equality — also fine. No action needed, just documenting for future awareness.

Observations (not blocking)

4. LocationInfo constructor calls GetLineSpan() twice

FilePath = location.SourceTree?.FilePath ?? location.GetLineSpan().Path;
TextSpan = location.SourceSpan;
LineSpan = location.GetLineSpan().Span;

GetLineSpan() is called twice when SourceTree is non-null (once for the fallback, once for LineSpan). Minor perf nit — could cache var lineSpan = location.GetLineSpan(); and reuse. Not critical since this runs in transform, not hot path.

5. ResolvedTransition is still a class (reference equality)
ResolvedTransition was updated to use LocationInfo for its Location property, but it remains a sealed class with reference equality. This is fine because ResolvedTransition is only used inside Execute (validation phase), not in the incremental pipeline — the pipeline carries Result<StateMachineModel> where StateMachineModel is a record. No action needed, just confirming the analysis.

6. EquatableArray<T> constraint is where T : IEquatable<T>
All model records implement IEquatable<T> via record's auto-generated equality. LocationInfo implements IEquatable<LocationInfo>. DiagnosticInfo implements IEquatable<DiagnosticInfo>. This is correct.

What's good

• Root cause analysis is thorough: both Location reference equality AND List<T> reference equality were identified as causes.
• LocationInfo is a readonly struct with IEquatable<T> — optimal for incremental pipeline value equality.
• ToLocation() correctly creates ExternalFileLocation via Location.Create(filePath, textSpan, lineSpan) — sufficient for Diagnostic.Create.
• All 9 model records updated consistently.
• AssertCacheHit upgraded to use Clone() — this is a stronger test that would have caught the original bug.
• LocationInfo_Equals_AfterClone test directly validates the core fix.
• Transform return types changed from List<T> to EquatableArray<T> in RegistrationGeneratorHelper.Transform and HandlerOrderGenerator.Transform — this was the other half of the fix.
• XML doc on LocationInfo clearly explains why it exists and the Clone scenario.

Summary

The fix is correct and comprehensive. Issue #1 (hash code) is a real correctness bug on netstandard2.0 that should be fixed. Issue #2 (missing round-trip test) is a test gap worth filling. The rest are observations.

[Skymly]

Review issues fixed in latest commit

1. GetHashCode now uses StringComparer.Ordinal — Equals uses StringComparison.Ordinal for FilePath, but GetHashCode previously used FilePath?.GetHashCode() which is culture-sensitive on netstandard2.0. Now uses StringComparer.Ordinal.GetHashCode(FilePath ?? string.Empty) to match Equals. This was a real correctness bug that could violate the hash/equals contract on netstandard2.0.
2. GetLineSpan() cached in constructor — Was called twice when SourceTree is non-null; now cached in a local.
3. ToLocation() round-trip test added — LocationInfo_ToLocation_RoundTripsCoordinates verifies that ToLocation() reconstructs a Location with the correct FilePath, SourceSpan, and LineSpan.
4. Null-location edge case test added — LocationInfo_NullLocation_ToLocationReturnsNull verifies that new LocationInfo(null).ToLocation() returns null without crashing.

All 133 tests pass (was 131, +2 new tests).