Merge signatures only on matching parameter types - #95
Closed
apiology wants to merge 10 commits into
Closed
Conversation
Combining two method pins that describe the same arity with different
parameter types should keep one signature per parameter type, so a call
with a Float argument still selects the Float return. Today the two
collapse into a single signature whose parameter type is whichever side
came first and whose return type is the union of both:
(Integer) -> Integer combined with (Float) -> Float
=> (Integer) -> Integer, Float
Pin::Method#combine_same_type_arity_signatures already means to prevent
this: it merges two signatures only when the merge leaves type_arity
unchanged, precisely so parameter types can go on choosing the return
type. That check cannot fire. Pin::Parameter#combine_with returns self
unless the other parameter shares its closure, and a parameter pin's
closure is its enclosing signature pin, so parameters from two
separately-parsed method pins never combine. Their types never widen,
type_arity never changes, and the guard sees a merge that looks free.
The example asserts the desired behavior and is marked pending until
parameter types can widen across method pins.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VNEbPE8fjo8Ub7EXsBkbJJ
Combining two method pins collapsed signatures describing different
parameter types into one, so a call could no longer use its argument
types to pick a return type:
(Integer) -> Integer combine_with (Float) -> Float
=> (Integer) -> Integer, Float
combine_same_type_arity_signatures meant to prevent exactly that: it
merged only when combining left type_arity unchanged, type_arity being
the parameter decl markers plus a count of each parameter type's union
members. That count cannot change on this path. Parameter#combine_with
returns self unless the other parameter shares its closure, and a
parameter pin's closure is its enclosing signature pin, which differs
between two separately parsed methods. The combined signature therefore
always carried the left side's parameter types, type_arity always
matched, and the guard always merged.
type_arity was doing two jobs at once. Bucketing now keys on arity, the
parameter shape by itself, and the merge test is a direct comparison of
the two input signatures: matching generics, matching block, and every
parameter type equal by rooted_tags. Comparing the inputs rather than
the combined result is what lets the test fire at all. Return types
stay out of it, since widening one is the point of merging.
full_type_arity, type_arity and type_arity_decl are left in place; no
caller remains outside full_type_arity, which itself has none.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VNEbPE8fjo8Ub7EXsBkbJJ
Merging on matching parameter types keeps the Integer and Float signatures apart, so the example passes and RSpec fails it as a pending test that succeeded. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VNEbPE8fjo8Ub7EXsBkbJJ
Integer#+ declares four RBS signatures differing only in parameter type. Merging them into one would leave a single return type and lose the distinction the declarations exist to make. The spec drives a real RBS-backed ApiMap and infers at four call sites, asserting Integer, Float, Rational and Complex in turn - one per signature. Nothing else in the suite covers dispatch end to end; the existing coverage checks the signature count on the pin, which stays correct even if inference collapses downstream. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VNEbPE8fjo8Ub7EXsBkbJJ
The existing end-to-end spec drives Integer#+, which is core RBS. Pin::Method#combine_with runs only from GemPins.combine_method_pins, so core method pins never reach combine_signatures at all and that spec passes with or without the parameter-type guard. Reproduce the merge where it actually happens: two Pin::Method pins for one method, combined the way a gem yardoc and its RBS collection entry are, with same-arity signatures differing only in parameter type. Master collapses them into one signature and Clip#infer returns the union of both return types; the guard keeps them apart so the Float argument still selects String. The same collapse is observable on the date gem today, where Date#upto(Date.new) infers Enumerator<Date, Date>, Date instead of Enumerator<Date, Date>. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VNEbPE8fjo8Ub7EXsBkbJJ
combine_same_arity_signatures merges two same-arity signatures only when Signature#same_parameter_types? agrees, and that compares param.return_type.rooted_tags. YARD pin types are still unqualified when GemPins.combine runs while RBS types are rooted, so a YARD (String) -> Boolean and an RBS (::String) -> ::Boolean compare unequal and both survive, where they name the same types and should collapse to one. Measured over the 73 gems carrying both a YARD and an RBS-collection cache: 18059 method paths appear in both, 2818 reach the by-arity merge, and 867 produce different signature sets between this branch and master. Over 800 of those 867 are this rooted-versus-unrooted spelling difference rather than a real type difference, and nothing in the suite catches it. The example asserts the collapse to a single signature and stays agnostic on which spelling survives: combine_return_type picks the unrooted side today, while a qualify-before-compare fix would keep the rooted one, and pinning either would leave the example pending forever under the other. It fails on the signature count, so RSpec reports it pending rather than passing unexpectedly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VNEbPE8fjo8Ub7EXsBkbJJ
combine_same_arity_signatures merges two same-arity signatures only when Signature#same_parameter_types? agrees, and that compares param.return_type.rooted_tags. YARD pin types are unqualified when GemPins.combine runs while RBS types are rooted, so a YARD (Integer) -> String and an RBS (::Integer) -> ::String compare unequal and both survive, where they name one type and one overload. The previous example asserted signatures.length on the combined pin. That is the point of explosion rather than the consequence, and it would keep reporting pending even if the consequence were fixed some other way. Replace it with the symptom a user meets: typing Widget.new.scan( puts two entries labelled scan(count) in the parameter-hints popup, because TextDocument::SignatureHelp feeds Clip#signify results through Pin::Method#signature_help, which emits one entry per signature. The example asserts a single entry and names no type, so a qualify-before-compare fix and an unroot-before-compare fix both satisfy it. Confirmed against real gem data. Loading the cached YARD and RBS pins for the 62 gems in this namespace and running GemPins.combine leaves 88 method paths whose surviving signatures become identical once the leading :: is stripped from every parameter type. AST::Node#to_sexp is the clearest: (::Integer) -> ::String alongside (Integer) -> String, whose Pin::Method#detail then renders (*) => String instead of (count) => String in hover and in the completion list. Three other candidate symptoms show no difference and are not asserted. Clip#infer returns ::String either way, since the union is qualified before it reaches the caller. Strong typecheck reports no problems either way. Both signify and define collapse to one signature once a real argument is present, because overload selection picks the matching one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VNEbPE8fjo8Ub7EXsBkbJJ
999d703 pins this defect at Clip#signify, one layer below the protocol. Add a second pending example driving the same input through TextDocument::SignatureHelp, so it fails where an editor actually reads the result: message.result[:signatures] carries two entries labelled scan(count) where one describes the overload. Getting a combined pin into a Host-driven spec needs a route that survives Library#sync_catalog, which rebuilds the ApiMap from the bench on every request. Injecting pins with ApiMap#index, as the clip_spec example does, is wiped by that rebuild. A Convention supplying the pins in its Environ is carried through it, and spec/source_map_spec.rb already registers one that way. The unregister call sits in an ensure block rather than at the end of the example. A pending example stops at the raised expectation, so a trailing unregister would never run and Widget#scan would be injected into every source map mapped later in the same process. Verified against a control: two pins both spelling the parameter ::Integer collapse to a single scan(count) entry through this same harness, so the duplication comes from the rooted-unrooted comparison rather than from the Convention route. The assertion names only the label, so a qualify-before-compare fix and an unroot-before-compare fix both satisfy it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VNEbPE8fjo8Ub7EXsBkbJJ
Pin::Method#combine_same_arity_signatures merges two signatures only
when Callable#same_parameter_types? says their parameter types agree.
That comparison keyed on ComplexType#rooted_tags, but the two pins
GemPins.combine hands it come from different sources: YARD types are
still unqualified, RBS types are rooted. So (String) and (::String) -
one type, two spellings - compared unequal and both signatures survived.
AST::Node#to_sexp came out of combine with two, where master had one:
signatures=2 detail="(*) => String"
params=["::Integer"] return="::String"
params=["Integer"] return="String"
88 method paths across this workspace's cached gems show that shape.
Key on ComplexType#tags instead. It renders every union member and
every subtype unrooted, so Array<::String> and Array<String> both come
out "Array<String>", while ::Foo::Bar still renders "Foo::Bar" and stays
distinct from "Bar". Qualifying the YARD types up front was ruled out:
at both pin-creation and combine time the map lacks dependency-gem
namespaces, and a name like Parser::Source::Range mis-resolves to core
::Range rather than failing.
The two examples that pinned the gap are unpended here rather than in a
follow-up commit, so that no commit reports them as passing
unexpectedly.
Pin::Parameter#type_arity_decl, Pin::Callable#type_arity and Pin::Callable#full_type_arity existed to decide whether combining two signatures would lose the mapping from specific parameter types to specific return types. That guard now runs through same_parameter_types?, which compares parameter type tags directly, so the three became unreachable except from each other. Pin::Parameter#arity_decl stays: Pin::Callable#arity still uses it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VNEbPE8fjo8Ub7EXsBkbJJ
apiology
added this pull request to stack #101
September 9, 2026 13:33
Owner
Author
|
Claude: Reopened upstream as castwide#1356; closing this one so the work has a single home. |
apiology
removed this pull request from stack #101
September 10, 2026 03:02
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
1 participant
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.
This PR was written by Claude Code on behalf of @apiology.
Problem: Combining two methods that declare the same parameter arity with different parameter types collapses them into one signature and drops one of the parameter types, so an argument type can no longer select its own return type.
Any method whose overloads dispatch on parameter type is affected, which is how RBS distinguishes overloads from one another.
Solution: Replace the per-parameter union-member count that stood in for "did the parameter-to-return mapping survive" — it compared the combined signature against its own input, and
Pin::Parameter#combine_withreturns the left side unchanged when the two parameters have different closures, so the count never moved — withCallable#same_parameter_types?, which compares the two inputs by generics, block and per-parameterrooted_tags, leaving the return type free to widen.