Skip to content

Fix order-dependent generic resolution for same-class union receivers - #1273

Open
apiology wants to merge 2 commits into
castwide:masterfrom
apiology:worktree-fix-1272-generic-union-resolution
Open

Fix order-dependent generic resolution for same-class union receivers#1273
apiology wants to merge 2 commits into
castwide:masterfrom
apiology:worktree-fix-1272-generic-union-resolution

Conversation

@apiology

@apiology apiology commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #1272.

Chain::Call#resolve looks up a method pin per union member via binder.each_unique_type, then deduped the results with pin_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 same path (Box#get) but each pin's return_type has already been correctly resolved for its own context (Integer vs String, respectively - this resolution happens earlier, inside ApiMap#get_method_stack). Deduping on path alone 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 of path alone, so distinctly-resolved pins from different union members are preserved.

# @generic T
class Box
  # @return [generic<T>]
  def get; end
end

# @param b [Box<Integer>, Box<String>]
def process(b)
  b.get # now infers `Integer, String` regardless of declared union order
end

Test plan

  • Added a regression test in spec/source/chain/call_spec.rb that 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

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
@apiology apiology closed this Aug 6, 2026
@apiology apiology reopened this Aug 6, 2026
@apiology
apiology marked this pull request as ready for review August 6, 2026 20:28
@apiology
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
@apiology apiology closed this Aug 6, 2026
@apiology apiology reopened this Aug 6, 2026
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LoPQ2EZUCHYMwDr13PBsc4
@apiology
apiology marked this pull request as ready for review August 6, 2026 22:41
@apiology apiology closed this Aug 6, 2026
@apiology apiology reopened this Aug 6, 2026
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
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.

Same-class generic resolution binds to the first union member instead of the actual member

1 participant