Fix order-dependent generic resolution for same-class union receivers - #1273
Open
apiology wants to merge 2 commits into
Open
Fix order-dependent generic resolution for same-class union receivers#1273apiology wants to merge 2 commits into
apiology wants to merge 2 commits into
Conversation
When a receivers declared type unions multiple instantiations of the same generic class (e.g. Box<Integer>, Box<String>), Chain::Call#resolve looked up a method pin per union member, then deduped the results by path alone. Since both members resolve to the same method path (Box#get) but had already been resolved to different, correct return types for their own context, the dedup silently discarded every member but the first - so the inferred type depended on declaration order instead of being a real union. Fixes castwide#1272 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LoPQ2EZUCHYMwDr13PBsc4
apiology
added a commit
to apiology/solargraph
that referenced
this pull request
Aug 6, 2026
This was referenced Aug 6, 2026
apiology
marked this pull request as ready for review
August 6, 2026 20:28
apiology
marked this pull request as draft
August 6, 2026 20:28
apiology
added a commit
to apiology/solargraph
that referenced
this pull request
Aug 6, 2026
Applied the same fix as castwide#1273 (order-dependent generic resolution for same-class union receivers) to Call#method_stack_pins Intersection branch: both conjunct dedup points now key on [path, return_type.tag] instead of path alone, so a same-class intersection (e.g. Hash{K1=>V1} & Hash{K2=>V2}) no longer silently drops every conjunct but the first. This makes Hash#fetch dispatch order-independent and sound (returns the union of every conjunct plausible result), but not yet precise - true per-key narrowing needs the literal Hash key ("Index" vs "Triggers") to survive Pin::Parameter#typify, and UniqueType#qualify unconditionally widens literal types to their base class. Attempted gating that on a corrected #literal? check (the existing one is unconditionally disabled by castwide#1201, for an unrelated array/tuple-inference reason) but reverted it: the same code path is load-bearing for other tested behavior (RBS `NilClass#to_s: () -> ""` widening to String, true/false -> Boolean consolidation), which broke under the naive fix (spec/rbs_map/core_map_spec.rb:102,114 and spec/parser/flow_sensitive_typing_spec.rb:644). A real fix needs qualify/transform to distinguish a key_types position from a general return-type position, which is a larger change than this commit attempts. Updated the two affected pending specs to describe the current, accurate remaining gap (union-not-precise-narrowing + castwide#1266) instead of the now-fixed order-dependence. Verified: full suite 1688 examples, 1 pre-existing unrelated failure, 0 regressions; rubocop clean (pre-existing offenses untouched). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TZme4n9mb8hGU8mrw94NAV
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LoPQ2EZUCHYMwDr13PBsc4
apiology
marked this pull request as ready for review
August 6, 2026 22:41
apiology
added a commit
to apiology/solargraph
that referenced
this pull request
Aug 6, 2026
apiology
added a commit
to apiology/solargraph
that referenced
this pull request
Aug 17, 2026
Review feedback on castwide#1231: - ComplexType::UniqueType#conforms_to?: the new @ sg-ignore did not use a reason string from the taxonomy in TypeChecker::Rules. It is no longer needed at all - the ignore existed only because `expected_unique_type.class` was interpolated into the raise message after `is_a?(UniqueType)` narrowed the negated branch to nothing. `expected.inspect` already reports the offending value, so the interpolation and the suppression both come out. - ComplexType.close_disjunction: same treatment. `disjuncts.fetch(0)` types as non-nil where `disjuncts.first` did not, so its ignore marker (also off-taxonomy) is gone rather than reworded. - Pin::Signature#key_param_index renamed to #hash_key_param_index - `_Key` is specific to hashtable datatypes. Its doc block is cut to roughly a third: the RBS 4.1.0 shape change stays, the inline code sketch for the post-castwide#1266 structural rewrite drops in favor of a one-line @todo pointing at the PR comment that holds it. - Chain::Call doc blocks for #method_pins_for_binder, #method_stack_pins and #key_verified_conjuncts roughly halved. - Comments describing what the code used to do, rather than what it does, removed from complex_type_spec, call_spec and the strong-level intersection specs. The history they carried: - duck_types_match? previously checked the duck-typed expectation against ComplexType#namespace/#scope, which for an Intersection delegates to the first conjunct, so an intersection was rejected whenever the duck-typed conjunct was not first. - #fetch on Hash{K1=>V1} & Hash{K2=>V2} previously always resolved through the first conjunct's signature; dedup keyed on pin path alone. Fixed the way castwide#1273 fixed it for real unions, then narrowed further by literal key. - Before Signature#hash_key_param_index learned RBS < 4.1's `(K arg0)` shape, the symbol-key spec failed on RBS 3.10.x/4.0.x with an unresolved generic<X> and three spurious "Wrong argument type for Hash#fetch" errors. - The strict-union call_spec gap was found while checking whether the intersection change regressed union semantics; it did not. - Class-level and #conforms_to? doc blocks in UniqueType::Intersection, and the duplicated #mixin_pairing? doc in ComplexType::UniqueType, trimmed for the same reason. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0175gggvr8gZaKQsGFe3e6CT
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #1272.
Chain::Call#resolvelooks up a method pin per union member viabinder.each_unique_type, then deduped the results withpin_groups.flatten.uniq(&:path). When a receiver's declared type unions multiple instantiations of the same generic class (e.g.Box<Integer>, Box<String>), both members resolve to a pin with the samepath(Box#get) but each pin'sreturn_typehas already been correctly resolved for its own context (IntegervsString, respectively - this resolution happens earlier, insideApiMap#get_method_stack). Deduping onpathalone discarded every member but whichever happened to be listed first, so the inferred type depended on declaration order instead of being a real union.The fix dedups on
[path, return_type.tag]instead ofpathalone, so distinctly-resolved pins from different union members are preserved.Test plan
spec/source/chain/call_spec.rbthat checks both declaration orders resolve to the same union{Integer, String}bundle exec rspec- full suite green (0 failures)bundle exec rubocop- clean (pre-existing unrelated offenses left untouched)bundle exec solargraph typecheck --level strict lib/solargraph/source/chain/call.rb- 0 problems