Skip to content

Stop dropping union members that differ only by type parameter - #1273

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

Stop dropping union members that differ only by type parameter#1273
apiology wants to merge 8 commits into
castwide:masterfrom
apiology:worktree-fix-1272-generic-union-resolution

Conversation

@apiology

@apiology apiology commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Problem: A method called on a variable typed as two instantiations of the same generic class returns only one of them — the rest of the union is silently dropped.

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

# @param b [Box<Integer>, Box<String>]
def one_order(b)
  b.get  # => Integer   (expected Integer, String)
end

# @param b [Box<String>, Box<Integer>]
def other_order(b)
  b.get  # => String    (expected Integer, String)
end

Every union of same-class generics is affected, no declaration order returns the full union, and the truncated result reads as a legitimate concrete type rather than an error.

Solution: Dedup the per-member method pins on both path and return_type.rooted_tags instead of path alone, using rooted_tags rather than tag because ComplexType defines no #tag and so falls through to its first union member.

Fixes #1272.

🤖 Generated with Claude Code

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
@apiology
apiology marked this pull request as draft August 31, 2026 20:29
Drop the bare issue-reference comment above a non-pending spec, and
replace the block-local @PARAM tag with a proper @sg-ignore: the
receiver's element type resolves fine for Array#first but isn't
propagated into uniq's block parameter, a real Solargraph inference
gap rather than something a YARD tag should paper over.
ComplexType has no #tag of its own, so return_type.tag fell through
method_missing to @Items.first.tag. On a union return type the key
therefore saw only the first member: "Integer, String" and
"Integer, Float" both keyed as "Integer", so two pins whose resolved
return types genuinely differ were collapsed into one.

rooted_tags maps every member and keeps the namespace rooting, so the
key reflects the whole resolved type.

Also drops the parenthetical from the ignore marker and cuts the
comment to three lines, per review.

Typecheck is unchanged by this: 591 problems before and after in the
same worktree, zero added and zero removed.
Reorder so the comment states the action before the rationale, per
review.

Also replaces the ignore marker's reason. The previous wording blamed
uniq, but the block param is untyped because Array#flatten is declared
in core RBS as returning a bare Array: arr.flatten infers to ::Array
where arr.compact infers to ::Array<::String>. With no element type on
the receiver there is nothing for uniq's block param to bind to.

No open PR covers this. The nearest two address different shapes:
castwide#1326 is a method-level generic bound by an argument
alongside a block, and castwide#1274 is a generic return
type lost when the declaring method takes a block. Left as prose rather
than citing a URL that does not match.
@apiology apiology changed the title Fix order-dependent generic resolution for same-class union receivers Stop dropping union members that differ only by type parameter Sep 5, 2026
Three lines became one. The removed clause, "keying on path alone
drops all but the first", described what the code used to do rather
than what it does, so it belongs here rather than in the source.

That prior version keyed the uniq on path only, which collapsed the
per-member pins whose resolved return types differ, and this branch
changed the key to path plus return type. A reader coming to the
method cold does not need that history; they need to know duplicates
are possible at all, which the surviving line states.
Trim the marker to its reason and register that reason in the
catalogued list, so it groups with the other tracked strings on a
census instead of reading as one-off wording.

It joins "flow sensitive typing could handle", which is where the
comparable core-typing gaps already sit -- "Need better handling of
#compact" and "Should better support meaning of '&' in RBS" are the
same shape -- and that group's total goes from 96 to 97.
@apiology
apiology marked this pull request as ready for review September 6, 2026 01:24
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