Conversation
prepend_to_pin and extend_to_pin computed generic_values from the mixin type arguments and then dropped the value on the floor, so a `prepend Foo[Integer]` or `extend Foo[String]` in RBS produced a reference pin with no type arguments at all. The generic module was mixed in as if it were bare, and the instantiated element type was lost. RuboCop flagged both lines as Lint/UselessAssignment, which was the only signal the bug existed. Pass generic_values through to the constructor, the way the sibling include_to_pin already does. Pin::Reference accepts the keyword already, so Prepend and Extend needed no widening. Also switch both from all_params.map(&:rooted_tags) to map(&:to_s), matching every live caller of build_type in this file. The two differ: rooted_tags emits a leading :: that no other reference pin carries, and Reference#type re-parses the joined values, so the include path format is the one already proven to round-trip. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Include, Prepend and Extend reference pins, and Interface and Module namespace pins, took their name from decl.name.relative!.to_s, which yields a name relative to wherever the declaration sat. Superclass reference pins already used the rooted form, so the two kinds disagreed. Name them from the built type instead - rooted_name for references, fqns for namespaces - and build every generic_values list from rooted_tags rather than to_s, so a pin carries a name that does not depend on the context it was created in. Prepend and Extend were also dropping their generic values entirely; pass them through.
convert_self_type_to_pins was the one reference pin still named from the unrooted form, so it disagreed with the Include pins built everywhere else in this file.
The reference pins built by prepend_to_pin and extend_to_pin now pass their generic values through, but nothing exercised that. Add an RBS fixture declaring a generic module that a class both prepends and extends, and assert the resulting pins carry the type argument. The extend case needs the negative form as well: add_mixins emits its own Extend pin for the same mixin and has always set generic values on it, so asserting only that some Extend pin carries ::Integer passes even without the change. Asserting that no Extend pin is left with an empty list is what actually covers extend_to_pin.
Carrying generic_values onto the Prepend and Extend reference pins had no user-visible effect on its own: ApiMap#inner_get_methods resolved both through the plain inner_get_methods, which performs no generic substitution. A method on a generic prepended module inferred generic<T> rather than the type argument the RBS declaration supplied. Route prepends and extends through inner_get_methods_from_reference, the same path includes and superclasses already take, so the reference type resolves its generics against the namespace pin before its methods are pulled in. namespace_pin is nil whenever the namespace has no Pin::Namespace, and all four call sites already passed a possibly-nil value; widening the three @PARAM tags to match clears four pre-existing strong-typecheck problems (591 to 587 locally, none introduced). Replace the two pin-inspection specs with specs on inferred method return types, which is where the behavior is observable. The included case is a control: it passed before this change and still passes.
RBS documents a trailing type-argument annotation on mixins, written
`include Enumerable #[String]`. Solargraph read that annotation only
after a superclass, so a generic module mixed into Ruby source resolved
its methods to the bare type variable rather than the argument given.
Read it in the include, prepend and extend processors and pass the
values through as generic_values, which Pin::Reference already renders
into the reference type.
Hand the argument list to RBS::Parser and RbsTranslator rather than
splitting it locally, so a nested argument converts to Solargraph
syntax: Hash[String, Integer] becomes Hash{String => Integer}. The
superclass path, which does split locally, still truncates that form at
its first bracket.
Follow RBS on the two rules it defines for the annotation: one module
argument per call, and no space between the hash mark and the bracket.
Undercover reported parse_type_args at 75 percent: nothing exercised the branch where RBS refuses the annotation. Add a spec whose brackets are balanced, so the regex accepts them, but whose contents are not a type.
apiology
marked this pull request as ready for review
September 7, 2026 15:34
These three @PARAM tags had been widened to accept nil, which cleared six Solargraph complaints by declaring the nil legitimate rather than by stopping it. Restore them so the gap stays visible. The nil has a single source. Boolean has no Pin::Namespace at all, yet qualify_superclass('Boolean') still returns 'Object', so the superclass branch of inner_get_methods calls onward with a nil namespace_pin. Instrumenting the full suite counted four such calls out of 35,837, every one of them Boolean. Six errors come back, four already present before this branch and two the branch adds by routing prepends and extends through inner_get_methods_from_reference. All six are accurate: nil does reach those parameters. Fixing that means giving Boolean a namespace pin, not describing the nil as valid input.
A reference pin's name is a rooted name, so its type arguments should be rooted too. to_s drops that, which makes the stored value depend on where it happened to be read from. This also moves include_to_pin, which was already on to_s, so all three mixin builders agree. No measured difference: strong typecheck stays at 593 and the suite stays at 1657 examples. The rootedness matters for what the value means, not for anything currently exercised.
The mixin processors and the superclass processor each had their own
scan for a trailing #[...] annotation, with the same offset and
end-of-line scaffolding written twice.
Move the regex and both helpers into ParserGem::NodeMethods, which both
processors already include, and have each call trailing_rbs_type_args.
The superclass path gains what the mixin one already had: a recursive
bracket match, so a nested argument is not cut off at its first bracket,
and conversion through RBS::Parser and RbsTranslator, so RBS syntax
becomes Solargraph syntax. class Foo < Array #[Hash[String, Integer]]
gave Array<Hash[String and now gives Array<Hash{String => Integer}>.
Strong typecheck drops 593 to 591 with the duplicated code.
Three sites still built generic_values with to_s: the self-type include, the superclass pin in the class declaration path, and add_mixins. All feed the same field on the same kind of pin as the three mixin builders already moved, so a reference pin's type arguments are now rooted wherever they are built. The superclass one already said so: name: type.rooted_name carries the comment "reference pins use rooted names", six lines under a to_s that produced an unrooted one. Strong typecheck stays at 591 and the suite stays at 1658 examples.
castwide#1344 already passes generic_values through on the prepend and extend pins and builds every generic_values list from rooted_tags, which this branch had arrived at independently. Take its conversions.rb wholesale, so the file is now identical to that branch and this one carries none of it. What remains here is the part castwide#1344 does not do: routing prepends and extends through inner_get_methods_from_reference so those values actually resolve, and reading RBS's inline #[...] annotation on mixins through a parser shared with the superclass path.
apiology
changed the title
These same three @PARAM tags were widened to accept nil, then reverted back to strict types in 5f4eb8f, on the reasoning that the nil source (Boolean lacks a Pin::Namespace) should be fixed at its root rather than described as valid input. A second, permanent source of nil exists alongside Boolean: ActiveSupportConcern guesses a "<Mod>::ClassMethods" submodule path that often does not exist, and inner_get_methods correctly returns zero methods for it without namespace_pin ever being non-nil. Unlike Boolean, there is no namespace pin to add here - a nonexistent submodule is supposed to resolve to nothing, forever. Restore the nil so the type matches both call sites' real behavior. Verified by comparing typecheck output before and after this revert on lib/solargraph/api_map.rb, lib/solargraph/complex_type.rb and lib/solargraph/complex_type/unique_type.rb: it removes exactly the four "Wrong argument type ... namespace_pin expected Solargraph::Pin::Base, received Solargraph::Pin::Base, nil" errors at the inner_get_methods_from_reference call sites, and introduces none.
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: a generic module mixed in with
prependorextendloses its type argument.RBS's inline form of the same annotation -
prepend Prependable #[Integer]- was read only after a superclass, so a Ruby class could not express any of these relationships at all.Solution: name reference pins from
type.rooted_nameand namespace pins fromfqns, build everygeneric_valueslist fromrooted_tags, pass those values through on prepend and extend and route both throughinner_get_methods_from_referenceso they resolve, and read the trailing#[...]annotation in the three mixin processors through a parser shared with the superclass path.