Fix Pin::Method#== and Pin::Parameter#type_arity_decl overload bugs - #52
Merged
apiology merged 1 commit intoAug 5, 2026
Conversation
Both pre-existing on master, unrelated to any currently open PR: Pin::Method#== (super && other.node == node) is from castwide#930 (2025-05-11) and never compared signatures. Pin::Parameter#type_arity_decl (arity_decl + return_type.items.count.to_s) is from castwide#1177 (2026-05-12), the same commit that added the spec/pin/method_spec.rb "combines signatures by type" test this fix makes pass. Both bugs are dormant on plain master: GemPins.combine_method_pins_by_path, the only caller that exercises this combining logic, was itself removed by castwide#1195 ("Limit pin combination to doc maps"), so this fix has no observable effect and no test to point to on this base until that function and its call site are restored. See PR description for context on where that currently stands. Traced from a CI-only failure on an unrelated integration-testing branch, where a different, in-progress PR stack (apiology/solargraph pin-caching-3/4) happens to re-add GemPins.combine_method_pins_by_path and its caller, waking up both of these bugs: Integer#+ inferred a return type of "Integer, BigDecimal" instead of "Integer" for `x = 0; x += 1; x`, because Pin::Method#== treated two RBS declarations of Integer#+ with different signatures (core Ruby's and the bigdecimal gem's reopening) as equal - both have nil location and identical rdoc-derived comments - so GemPins.combine_method_pins' skip-if-already-identical shortcut fired and one declaration was silently dropped instead of merged. Separately, type_arity_decl grouped signatures for merging by how many types are in each parameter's union rather than the types themselves, so distinct single-type overloads (Integer, Float, Rational, Complex, BigDecimal) bucketed together and had their return types incorrectly unioned. Fixed by comparing actual type tags in type_arity_decl and by including signatures in Pin::Method#==.
apiology
marked this pull request as ready for review
August 5, 2026 02:17
apiology
added a commit
that referenced
this pull request
Aug 5, 2026
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.
Stacked on
pin-caching-3-pincache-corebecause that is what currently makesGemPins.combine_method_pins_by_pathreachable at all - see context below.What this fixes
spec/pin/method_spec.rb's'combines signatures by type'test (added by castwide#1177, alongside the buggytype_arity_declit exercises) expectsInteger#+to combine into more than 3 signatures oncebigdecimal's RBS reopening ofInteger#+is merged with core Ruby's own 4 overloads. It currently gets 1.Two independent, pre-existing bugs, both already on
castwide/master, unrelated to any currently open PR:Pin::Parameter#type_arity_declgrouped overloads for merging byreturn_type.items.count(how many types are in a parameter's union) instead of the types themselves, so single-type overloads forInteger,Float,Rational,Complex, andBigDecimalall looked like the same bucket and had their return types unioned into each other. From Release 0.59.0 castwide/solargraph#1177 (2026-05-12).Pin::Method#==never comparedsignatures- justnode(both nil here) plusPin::Base's owncomments/locationcheck.bigdecimal's reopening reuses Ruby's own rdoc comment forInteger#+verbatim and neither pin sets alocation, so two RBS declarations with completely different signatures compared as==.GemPins.combine_method_pins's skip-if-already-identical optimization then fired and one declaration was silently dropped instead of merged. From Improve ApiMap catalog speed by preserving static pin indexes castwide/solargraph#930 (2025-05-11).Why this is stacked on pin-caching-3-pincache-core
Both bugs are dormant on plain
castwide/master:GemPins.combine_method_pins_by_path, the only caller that exercises this combining logic, was itself removed by castwide#1195 ("Limit pin combination to doc maps"). This branch reintroduces that function and its call site as part of the PinCache rewiring, which is what makes these two bugs observable and testable again.Verification
spec/pin/method_spec.rb:526("combines signatures by type") confirmed red without this fix, green with it, on this exact base.