Say what proxy equality actually promises, and test it - #84
Conversation
The contract is between proxies: two are equal when the objects they wrap are, and that is symmetric and agrees with GetHashCode - verified for the same target and for distinct-but-equal targets with value semantics of their own, including finding each other in a dictionary. GetHashCode returning the target's hash is right for exactly that purpose. Comparing a proxy against a *raw* object is a convenience on top, and the only asymmetric part: proxy.Equals(target) holds, target.Equals(proxy) cannot, because the target is an ordinary object that knows nothing of proxies. So #75 was accurate about the mechanics and wrong about the significance - nothing is broken, one direction was never promised. Both doc comments now say which is which, and the tests cover the contract rather than only the same-target case. Also pinned: two proxies over one target are equal even presenting different interfaces, which is the part worth changing later - #83, for 9.0. 217 on net8.0 and net10.0, 223 on net47, 128 on browser-wasm. Closes #75. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016mwfq4oeZW8SiD4HjTHYdg
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The equality documentation should qualify behavior for uninitialized/orphan proxies.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
Clarifies proxy equality semantics and adds contract tests without changing runtime behavior.
Changes:
- Adds value-semantic fixtures and equality/dictionary tests.
- Tests equality across different proxy interfaces.
- Clarifies equality documentation.
| File | Description |
|---|---|
Tests/UnitTestImpromptuInterface/Support/SupportDefinitions.cs |
Adds ValuePoco for equality tests. |
Tests/UnitTestImpromptuInterface/Contracts.cs |
Adds proxy equality and dictionary contract tests. |
ImpromptuInterface/src/EmitProxy/ActLikeProxy.cs |
Clarifies proxy equality documentation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| /// Two proxies are equal when the objects they wrap are. That comparison is symmetric | ||
| /// and agrees with <see cref="GetHashCode"/>, so proxies work as dictionary keys and | ||
| /// find each other there. |
…rgets The doc comment said two proxies are equal when their targets are, that it agrees with GetHashCode, and that proxies therefore work as dictionary keys - stated without qualification. Orphan proxies are a counterexample: two share the one stand-in so they compare equal, while GetHashCode throws, so they cannot be keys at all. Qualified to proxies that have targets, with the orphan case spelled out as what it is - a quirk of an object that already throws on every member, rather than a contract - and pinned by a test that says so, pointing at #83. 217 on net8.0 and net10.0, 223 on net47, 128 on browser-wasm. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016mwfq4oeZW8SiD4HjTHYdg
|
Right, and precisely put — fixed in the commit above. I wrote the promise unqualified: "two proxies are equal when the objects they wrap are… so proxies work as dictionary keys and find each other there." An orphan proxy is a counterexample, exactly as described: two share the one stand-in so the typed overload returns It now reads "two proxies that have targets", and the remark spells the exception out rather than leaving it implicit:
And the orphan test now pins that pair of facts together ( Worth noting the same fact reached this PR from two directions: the #81 review found it independently in the code, and I recorded it there as pre-existing — master had it too, via 217 on net8.0 and net10.0, 223 on net47, 128 on browser-wasm. |

Closes #75. Documentation and tests only — no behaviour change, which is what 8.1 is for.
#75 was right about the mechanics and wrong about the significance
I filed it as "proxy equality is asymmetric, so a proxy does not find its target in a dictionary", implying a broken promise. Reading the code again, the promise is between proxies, and it is kept:
a.Equals(b)/b.Equals(a)True/TrueTruea, looked up by proxybThe typed overload — the one carrying the original "equivalent to the objects they proxy" comment — compares targets, so both sides run the same logic and the result is symmetric and hash-consistent.
GetHashCodereturning the target's hash is exactly right for that: it is what puts two proxies over one target in the same bucket.Comparing a proxy to a raw object is a convenience added by the
Equals(object)overload, and it is the only asymmetric part — inherently, since the target is an ordinary object that knows nothing of proxies. Nothing is broken; one direction was never promised.What changed
Contracts.csonly half covered: distinct-but-equal targets, both directions, hash agreement, and the dictionary lookup.Equalsit must joinGetHashCodetoo.217 on net8.0 and net10.0, 223 on net47, 128 on browser-wasm.
🤖 Generated with Claude Code
https://claude.ai/code/session_016mwfq4oeZW8SiD4HjTHYdg