Fix generic binding and block signature merging through a @!parse stub - #1288
Open
apiology wants to merge 14 commits into
Open
Fix generic binding and block signature merging through a @!parse stub#1288apiology wants to merge 14 commits into
apiology wants to merge 14 commits into
Conversation
A class defined in a gem and re-opened elsewhere via @!parse to add @Generic tags produced two pins for the same namespace/method path. ApiMap picked whichever pin loaded first - the gem's own pins load before workspace pins, so the annotation's @Generic declaration and overridden return types were silently ignored. - ApiMap#namespace_pin_for_generics prefers the pin that actually declares generics over an arbitrary .first. - ApiMap::Store#get_methods combines same-path method pins (skipping aliases, since merging an alias pin with a non-alias pin at the same path produces a pin #resolve_method_alias can't trace back to its target, which raises under SOLARGRAPH_ASSERTS=on). Fixes castwide#1286 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LtM8dkYTeQEiyhFu1NLZCB
Confirms combining many pins for the same method path completes quickly rather than hanging - related to the concern that prompted castwide#1186 and castwide#1195 (see comment in spec for why this doesn't reproduce that specific bug, and where the precise regression guard for it already lives). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LtM8dkYTeQEiyhFu1NLZCB
apiology
added a commit
to apiology/solargraph
that referenced
this pull request
Aug 12, 2026
… stub A class defined in a gem and re-opened elsewhere via @!parse to add @Generic tags produced two pins for the same namespace/method path. ApiMap picked whichever pin loaded first - a gem's own pins load before workspace pins, so the annotation's @Generic declaration and overridden return types were silently ignored regardless of file naming/order. - ApiMap#namespace_pin_for_generics prefers the pin that actually declares generics over an arbitrary .first, used at the three call sites that resolve generic type parameters (get_methods, inner_get_methods, inner_get_methods_from_reference). - ApiMap::Store#get_methods combines same-path method pins into one, so a @!parse override's return type isn't shadowed by the plain, undocumented definition. Aliases are excluded from this combination: combining a MethodAlias pin with a non-alias pin at the same path produced a :combined pin that #resolve_method_alias couldn't trace back to its target, raising under SOLARGRAPH_ASSERTS=on. Fixes castwide#1286 Conflict in spec/source_map/clip_spec.rb: both sides independently added a new spec adjacent to the same insertion point - no real overlap, kept both. Verified: spec/source_map/clip_spec.rb, spec/api_map/store_spec.rb, spec/api_map_spec.rb, spec/api_map_method_spec.rb (284 examples, 0 failures, 11 pending), SOLARGRAPH_ASSERTS=on solargraph typecheck --level strong (4 problems, all in lib/solargraph/shell.rb and lib/solargraph/workspace/gemspecs.rb - the same Vernier-gem/ Gem::StubSpecification local-vs-CI environment split already established during the castwide#1240 merge, not new here), and a broader safety net - spec/type_checker, spec/source, spec/pin (612 examples, 0 failures, 18 pending).
Pin::DuckMethod pins (created for `#method_name` duck-type tags, e.g. `@param x [#to_s]`) are constructed without a closure. Method#typify called `closure.gates` unconditionally once see_reference or typify_from_super resolved a type, raising `NoMethodError: undefined method 'gates' for nil` whenever that path was hit on such a pin. Guard it the same way other call sites in this file already do: `closure&.gates || ['']`. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cj8BgHwzHKFsD51H9TfPD5
apiology
added a commit
to apiology/solargraph
that referenced
this pull request
Aug 12, 2026
…osure-less duck-type pins Pin::DuckMethod pins (created for `#method_name` duck-type tags, e.g. `@param x [#to_s]`) are constructed without a closure. Method#typify called `closure.gates` unconditionally once see_reference or typify_from_super resolved a type, raising NoMethodError: undefined method 'gates' for nil whenever that path was hit on such a pin. Guarded it the same way other call sites in this file already do: closure&.gates || ['']. Clean auto-merge, no conflicts. Verified: spec/pin/method_spec.rb (67 examples, 0 failures), and a broader safety net - spec/pin, spec/type_checker (463 examples, 0 failures, 14 pending).
This was referenced Aug 12, 2026
apiology
added a commit
to iftheshoefritz/solargraph-rails
that referenced
this pull request
Aug 19, 2026
…ions t in create_table :things do |t| stopped resolving to TableDefinition in solargraph 0.59.2. The type comes from this gem's create_table annotation, which used to be folded into activerecord's pin for the same path; since castwide/solargraph#1195 removed GemPins.combine_method_pins_by_path from Store#get_methods, both pins survive and activerecord's untyped block wins. Which pin wins is not defined. ApiMap#inner_get_methods sorts method pins by name, and Ruby's sort is unstable, so pins sharing a name compare equal and their order varies with the array. The same annotation wins in a reduced workspace and loses here. Bisected to a4a4d475 (09be4a68, immediately before it, passes). First released in 0.59.2; 0.59.1 passes, and the guard leaves the assertions running there. The versions are listed rather than bounded with >=, so a later release that still has the bug runs the assertions and fails instead of being skipped silently. castwide/solargraph#1288 restores the merge and was verified to make this example pass again. Not pending: branch-castwide-master reports Solargraph::VERSION as 0.60.3, so an unexpectedly-passing example would fail this repo's CI as soon as the fix merges upstream, before any version bump. Verified on Rails 8.0 / Ruby 3.2.8: 0.60.3 and branch-castwide-master give 43 examples, 0 failures; 0.59.1 and 0.58.2 still run the assertions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TY8B7iFysoEXhFF3fbrgrK
apiology
added a commit
to iftheshoefritz/solargraph-rails
that referenced
this pull request
Aug 19, 2026
Keying the guard on Solargraph::VERSION skipped branch-castwide-master too, since master reports 0.60.3. That cell would have stayed skipped after castwide/solargraph#1288 lands, hiding the fix instead of reporting it. Use the CI matrix key the way spec/definitions.rb does, and list only frozen releases. 0.59.2 and 0.60.3 will never gain the fix, so skipping them states a fact; branch keys are absent, so branch-castwide-master runs and fails until the upstream merge, which is the signal we want. Also satisfies Style/IfUnlessModifier, which the previous form tripped. Verified on Rails 8.0 / Ruby 3.2.8: 0.60.3 1 example, 0 failures, 1 pending branch-castwide-master 1 example, 1 failure 0.59.1 1 example, 0 failures Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TY8B7iFysoEXhFF3fbrgrK
apiology
added a commit
to iftheshoefritz/solargraph-rails
that referenced
this pull request
Aug 19, 2026
Removing the ENV errors uncovered the next ones, which the job had been exiting
before it reached:
config/routes.rb:6 Unresolved call to get
db/{cable,cache,queue}_schema.rb:2,14,22 Unresolved call to create_table
Same cause as the migration spec. This gem annotates ActiveRecord::Schema.define
and the routes mapper with @yieldreceiver, and since castwide/solargraph#1195
those annotations are a second pin for the same path and lose to the gem's own,
so the block parameter has no type.
Verified on castwide/solargraph#1288's branch: ActiveRecord::Schema.define goes
from three pins with an untyped rbs pin first to two with the annotation folded
into the winner.
Gated on Solargraph::VERSION like the other allow-lists, so master is covered
while it reports 0.60.3 and the exclusion lifts when that is bumped.
Rails 7.0 was already passing - it generates no Solid Cable/Cache/Queue schema
files.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TY8B7iFysoEXhFF3fbrgrK
apiology
added a commit
to iftheshoefritz/solargraph-rails
that referenced
this pull request
Aug 19, 2026
Removing the ENV errors uncovered the next ones, which the job had been exiting
before it reached:
config/routes.rb:6 Unresolved call to get
db/{cable,cache,queue}_schema.rb:2,14,22 Unresolved call to create_table
Same cause as the migration spec. This gem annotates ActiveRecord::Schema.define
and the routes mapper with @yieldreceiver, and since castwide/solargraph#1195
those annotations are a second pin for the same path and lose to the gem's own,
so the block parameter has no type.
Verified on castwide/solargraph#1288's branch: ActiveRecord::Schema.define goes
from three pins with an untyped rbs pin first to two with the annotation folded
into the winner.
Gated on Solargraph::VERSION like the other allow-lists, so master is covered
while it reports 0.60.3 and the exclusion lifts when that is bumped.
Rails 7.0 was already passing - it generates no Solid Cable/Cache/Queue schema
files.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TY8B7iFysoEXhFF3fbrgrK
apiology
added a commit
to iftheshoefritz/solargraph-rails
that referenced
this pull request
Aug 20, 2026
…x cells (#208) * Add 0.60.3 skip definitions Generated with: ruby script/copy_definitions.rb branch-castwide-master 0.60.3 spec/definitions.rb keys skip lists by MATRIX_SOLARGRAPH_VERSION when set and by Solargraph::VERSION otherwise, so a run against a local solargraph checkout keys on 0.60.3. No definitions file listed 0.60.3, so every method skipped under branch-castwide-master was reported as missing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F1B9SLWrXrKL9SNGy7SXot * Exclude Ruby 3.0 x castwide/solargraph master from CI matrix castwide/solargraph master raised required_ruby_version to '>= 3.1', so `bundle lock` fails version solving on the two Ruby 3.0 matrix rows before any spec runs. Verified required_ruby_version for every solargraph version in the matrix: 0.48.0 (>= 2.4), 0.49.0/0.50.0/0.51.2/0.52.0 (>= 2.6), 0.56.2/0.57.0/0.58.1/0.58.2/0.59.0.dev.1/0.59.0.dev.2 (>= 3.0), and castwide/solargraph branch v0.59 (>= 3.0). All of those still resolve on Ruby 3.0 and keep running. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F1B9SLWrXrKL9SNGy7SXot * Test the released 0.60.3 in CI; drop the superseded v0.59 branch cell Nothing in the matrix ran with MATRIX_SOLARGRAPH_VERSION=0.60.3, so the 0.60.3 skip lists added in the previous commit were exercised only on a local checkout. Adding the version to the matrix makes them checkable: Definitions#assert_matches_definitions leaves @allow_improvements off for any non-`branch-` key, so an entry skipped for 0.60.3 that in fact resolves correctly is reported rather than passing silently. Released 0.60.3 sets required_ruby_version '>= 3.1', so the two Ruby 3.0 cells get the same exclusion castwide/solargraph master already has. branch-castwide-v0.59 is pinned at 735eaa5d (2026-03-26), which is the reproducer commit in castwide/solargraph#1235: the spec suite hangs between core/Hash and core/Integer, and both Ruby 3.0 cells burned ~2h of runner time before being cancelled. The fix, castwide/solargraph#1238, merged to master on 2026-08-03 and was never backported to the v0.59 branch. The `branch-castwide-v0.59` keys left in spec/definitions/*.yml are now unread; removing them is a separate change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TY8B7iFysoEXhFF3fbrgrK * Record what solargraph 0.60.3 infers for 91 method definitions castwide/solargraph#1201 stopped storing literal values in complex types, so 0.59.2 onward widens `true` to `Boolean`, `:activerecord` to `Symbol` and `-1|0|1` to `Integer`. Upstream describes this as disabled for now while castwide/solargraph#1196 is open, so these are skips rather than a rewrite of the shared `types:` list, which the 0.48-0.58.2 cells still need. When 1201 is reverted, a FORCE_UPDATE run removes the skips again via Definitions#remove_skip. Two entries get the type itself corrected instead. `undefined` there was solargraph declining to answer; `void` is the return type, and every version that still answers `undefined` moves into the skip list: ActionController::Base.method_added Rails::Application.inherited ActionController::Base#authenticate_with_http_token infers BasicObject, which Definitions#process_potential_update treats as no better than undefined, so it is skipped. Generated with FORCE_UPDATE=true against solargraph 0.60.3 on Rails 8.0, Ruby 3.2.8. Verified: 0 type mismatches and 0 stale skips on re-run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TY8B7iFysoEXhFF3fbrgrK * Record what castwide/solargraph master infers, and fix five stale types Adds branch-castwide-master to the skip list of 89 definitions whose recorded type is a literal that castwide/solargraph#1201 no longer stores (`true`, `false`, `:activerecord`, `-1|0|1`). The key also covers every other `branch-` version via Definitions#process_single_definition. Five entries had a recorded type that was simply out of date, so they get the type corrected and the versions that still disagree skipped: Module.constants Array<Integer> -> Array<Symbol> Enumerable#sum generic<Elem> -> generic<E> Array#compact_blank Array -> Array<generic<Elem>> File#compact_blank Array<String> -> Array<generic<Elem>> Module.constants returns symbols; Array<Integer> came from the literal inference 1201 removed. The generics are unresolved because Definitions calls Pin#typify without binding a receiver, so Elem has nothing to resolve against - Enumerable.yml and Hash.yml already record them that way, and Array.yml and File.yml were the outliers. Verified against solargraph 0.60.3 and castwide/solargraph master (8fda633) on Rails 8.0, Ruby 3.2.8: 0 type mismatches, 0 stale skips. Both still fail rails_spec.rb:100, which is unrelated to definitions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TY8B7iFysoEXhFF3fbrgrK * Keep compact_blank's resolved types; skip the versions that lost them Array#compact_blank and File#compact_blank now report Array<generic<Elem>>, and recording that would have encoded a regression. Elem is not a type parameter of either class: core/enumerable.rbs:274 module Enumerable[unchecked out E] core/io.rbs:618 include Enumerable[String] (class File < IO) activesupport-7.0.rbs:97 module Enumerable[unchecked out Elem] def compact_blank: () -> Array[Elem] Substitution still works when the parameter name matches core's. File declares no to_a of its own, and `solargraph pin File#to_a --stack --typify` resolves E to String, giving ::Array<::String>. The same command for compact_blank leaves Elem unbound, because gem_rbs_collection reopens Enumerable under a different parameter name than core uses. So Array<String> stays the recorded type for File and Array stays Array, with 0.60.3 and branch-castwide-master skipped until the substitution is fixed. Verified against solargraph 0.60.3 and castwide/solargraph master (8fda633) on Rails 8.0, Ruby 3.2.8: 0 type mismatches, 0 stale skips. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TY8B7iFysoEXhFF3fbrgrK * Skip migration block param assertions on known-broken solargraph versions t in create_table :things do |t| stopped resolving to TableDefinition in solargraph 0.59.2. The type comes from this gem's create_table annotation, which used to be folded into activerecord's pin for the same path; since castwide/solargraph#1195 removed GemPins.combine_method_pins_by_path from Store#get_methods, both pins survive and activerecord's untyped block wins. Which pin wins is not defined. ApiMap#inner_get_methods sorts method pins by name, and Ruby's sort is unstable, so pins sharing a name compare equal and their order varies with the array. The same annotation wins in a reduced workspace and loses here. Bisected to a4a4d475 (09be4a68, immediately before it, passes). First released in 0.59.2; 0.59.1 passes, and the guard leaves the assertions running there. The versions are listed rather than bounded with >=, so a later release that still has the bug runs the assertions and fails instead of being skipped silently. castwide/solargraph#1288 restores the merge and was verified to make this example pass again. Not pending: branch-castwide-master reports Solargraph::VERSION as 0.60.3, so an unexpectedly-passing example would fail this repo's CI as soon as the fix merges upstream, before any version bump. Verified on Rails 8.0 / Ruby 3.2.8: 0.60.3 and branch-castwide-master give 43 examples, 0 failures; 0.59.1 and 0.58.2 still run the assertions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TY8B7iFysoEXhFF3fbrgrK * Scope the migration skip to released versions only Keying the guard on Solargraph::VERSION skipped branch-castwide-master too, since master reports 0.60.3. That cell would have stayed skipped after castwide/solargraph#1288 lands, hiding the fix instead of reporting it. Use the CI matrix key the way spec/definitions.rb does, and list only frozen releases. 0.59.2 and 0.60.3 will never gain the fix, so skipping them states a fact; branch keys are absent, so branch-castwide-master runs and fails until the upstream merge, which is the signal we want. Also satisfies Style/IfUnlessModifier, which the previous form tripped. Verified on Rails 8.0 / Ruby 3.2.8: 0.60.3 1 example, 0 failures, 1 pending branch-castwide-master 1 example, 1 failure 0.59.1 1 example, 0 failures Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TY8B7iFysoEXhFF3fbrgrK * Exclude generated config files from the rails new typecheck ENV[] and ENV.fetch stopped resolving in solargraph 0.59.0.dev.1, so `solargraph typecheck --level strong` reports 10 problems across config/boot.rb, config/puma.rb and config/environments/*.rb in a stock rails new project. Last good release was 0.58.3. YARD derives a Class<ENV> namespace pin from `class << ENV` in pp, which reaches every Rails app via railties -> irb -> pp. That pin shadows the RBS constant, so neither [] nor fetch dispatches. castwide/solargraph#1279 fixes it; verified against this workspace, where its branch reports 0 problems on an ENV probe that gives 2 on released 0.60.3. The job pins castwide master and has no version dimension, so the exclusion is by file rather than by version. Master reports 0.60.3 until the next release, so this will not lift itself when 1279 merges - it has to be removed by hand. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TY8B7iFysoEXhFF3fbrgrK * Add min_rbs to gate a definition on the rbs version rbs 4.0 renamed Enumerable's type parameter from Elem to E, so Enumerable#sum infers generic<Elem> under rbs 3.x and generic<E> under 4.x. The matrix hits both: the workflow runs `bundle update rbs`, which resolves to the newest rbs the cell's Ruby allows, and rbs 4.1.3 requires Ruby >= 3.2. Ruby 3.1 cells stay on rbs 3.10.4. added_in/removed_in gate on Rails and mean the method does not exist there, so neither states this constraint. min_rbs does, and keeps the assertion live on every cell with rbs 4.x instead of dropping it everywhere via skip. Verified on Rails 8.0 / Ruby 3.2.8 / rbs 4.1.3: 43 examples, 0 failures. The rbs 3.x path is unverified locally - spec/rails7 does not reproduce CI's results here, with 12 unrelated core-extension examples failing on undefined. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TY8B7iFysoEXhFF3fbrgrK * Declare return types for inherited and method_added Both inferred void on Rails 8.0 and undefined on Rails 7.x, so recording either in the yml fails half the matrix. Declaring them here makes the type uniform across Rails versions rather than depending on what each one happens to infer. Verified under solargraph 0.60.3 on Rails 7.2 and 8.0: both typify as void. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TY8B7iFysoEXhFF3fbrgrK * Stop skipping inherited and method_added by solargraph version Declaring the return types in the annotations makes both resolve on every solargraph version, so their skip lists - added when older versions genuinely could not infer them - now describe something untrue. The suite flags that deliberately: "marked as skipped ... but is actually present and correct", across 20 cells from 0.48.0 through 0.59.0.dev.2. Also moves the min_rbs gate out of process_single_definition, which was already at its Metrics limits, and silences Lint/MissingSuper on the two callback stubs. Verified on Rails 8.0 / Ruby 3.2.8, full suite: 0.58.2 3 failures before, 1 after (the remaining one is Date#<=>, pre-existing) 0.60.3 0 failures Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TY8B7iFysoEXhFF3fbrgrK * Handle solargraph versions that ship no rbs 0.48.0 predates solargraph's rbs dependency, so RBS::VERSION is undefined and the min_rbs gate raised NameError on four cells. Fall through rather than skipping when the constant is missing, and list 0.48.0 in the entry's skip. A silent skip on undefined would drop the assertion whenever RBS::VERSION goes missing for any reason; listing the one version that has no rbs keeps that case loud. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TY8B7iFysoEXhFF3fbrgrK * Gate compact_blank and Module.constants on rbs 4 Under rbs 3.10.4 the generic substitution in Enumerable#compact_blank works, giving ::Array[untyped] on Array and ::Array[::String] on File, and Module.constants still infers Array<Integer>. Under rbs 4.x those become Array<generic<Elem>> and Array<Symbol>, which is what the entries record. The recorded values were measured on Ruby 3.2 only. The single Ruby 3.1 cell running 0.60.3 is the only one on rbs 3.x, so it was the only one to disagree. min_rbs states the constraint the entries actually carry rather than skipping the solargraph version outright, so every cell with rbs 4.x keeps asserting. Verified on Rails 8.0 / Ruby 3.2.8 / rbs 4.1.3: 43 examples, 0 failures. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TY8B7iFysoEXhFF3fbrgrK * Key the bug allow-lists on the reported solargraph version The migration guard read MATRIX_SOLARGRAPH_VERSION, so branch-castwide-master was a distinct key and never matched the list, leaving that cell red until the upstream fix lands. Read Solargraph::VERSION instead. Master reports 0.60.3 today, so it is covered now and leaves the list the moment castwide bumps the version - at which point a version that still has the bug fails the assertions rather than skipping them. Same treatment for the rails new ENV exclusion, which was unconditional. Verified on Rails 8.0 / Ruby 3.2.8: branch-castwide-master 43 examples, 0 failures, 4 pending 0.58.2 43 examples, 1 failure (Date#<=>, pre-existing) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TY8B7iFysoEXhFF3fbrgrK * Exclude routes and schema files from the rails new typecheck Removing the ENV errors uncovered the next ones, which the job had been exiting before it reached: config/routes.rb:6 Unresolved call to get db/{cable,cache,queue}_schema.rb:2,14,22 Unresolved call to create_table Same cause as the migration spec. This gem annotates ActiveRecord::Schema.define and the routes mapper with @yieldreceiver, and since castwide/solargraph#1195 those annotations are a second pin for the same path and lose to the gem's own, so the block parameter has no type. Verified on castwide/solargraph#1288's branch: ActiveRecord::Schema.define goes from three pins with an untyped rbs pin first to two with the annotation folded into the winner. Gated on Solargraph::VERSION like the other allow-lists, so master is covered while it reports 0.60.3 and the exclusion lifts when that is bumped. Rails 7.0 was already passing - it generates no Solid Cable/Cache/Queue schema files. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TY8B7iFysoEXhFF3fbrgrK --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Cut docstrings and spec comments to the review's per-comment budget (2-6 lines): the combine_duplicate_method_pins and namespace_pin_for_generics docstrings, the alias-combination spec comment, and the "combines many same-path pins" spec comment, which had narrated the history of issues castwide#1186, castwide#1195, and castwide#1238 instead of stating the current constraint. Also replaces "plain" in the cross-file generics spec with what actually makes that fixture plain: no @Generic tag, no @!parse stub.
solargraph typecheck against any project using Forwardable dies before
emitting a single diagnostic:
lib/solargraph/pin/delegated_method.rb:25:in 'initialize':
either :method or :receiver is required (ArgumentError)
from ApiMap#load_with_cache -> catalog -> Store#update ->
combine_duplicate_method_pins -> Pin::Method#combine_with ->
Pin::Base#combine_with.
Pin::Base#combine_with rebuilds the merged pin with
self.class.new(**new_attrs), and new_attrs carries only generic pin
attributes (location, name, closure, comments, visibility, signatures).
Pin::DelegatedMethod#initialize requires exactly one of :method /
:receiver and receives neither, so combining two same-path
DelegatedMethod pins is structurally impossible. This went live when
castwide#1311 started minting DelegatedMethod pins for
def_delegators, which makes duplicate-path groups routine.
combine_duplicate_method_pins already skips groups containing a
Pin::MethodAlias for the same class of reason (a merged pin can't
represent the alias target); DelegatedMethod was never added to that
guard. Extend it rather than teaching DelegatedMethod to merge: a pin
constructed from a :receiver that has since resolved holds both
@receiver_chain and @resolved_method, while initialize forbids passing
both, so any combine_with override would have to discard one pin's
delegation target. When the two pins delegate to different receivers
(reopened class, source-vs-RBS duplicate) that loses information
silently. Keeping both pins preserves it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01H1FEjW6nMpZrWPmeWX9miT
Method docstring was 6 prose lines over the 1-3 line budget; two spec comments restated the example name or the assertion in different words. Neither adds anything the code or the it-title doesn't already say.
…rse' into push-1288-undercover
apiology
added a commit
to iftheshoefritz/solargraph-rails
that referenced
this pull request
Sep 4, 2026
* Add 0.60.4 skip definitions Generated with: ruby script/copy_definitions.rb branch-castwide-master 0.60.4 spec/definitions.rb keys skip lists by MATRIX_SOLARGRAPH_VERSION when set and by Solargraph::VERSION otherwise, so a run against a local solargraph checkout keys on 0.60.4. No definitions file listed 0.60.4, so every method skipped under branch-castwide-master was reported as missing. Rails::Application.<=>.types picked up one incidental YAML re-quote (single to double quotes on the literal "-1") as a side effect of YAML.dump rewriting the whole file; not a semantic change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VLRE2U4AsfZcoV2YiAyhgD * Test the released 0.60.4 in CI Nothing in the matrix ran with MATRIX_SOLARGRAPH_VERSION=0.60.4, so the 0.60.4 skip lists added in the previous commit were exercised only on a local checkout. Adding the version to the matrix makes them checkable. Released 0.60.4 sets required_ruby_version ">= 3.1" (same as 0.60.3 and castwide/solargraph master), so the two Ruby 3.0 cells get the same exclusion those versions already have. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VLRE2U4AsfZcoV2YiAyhgD * Skip the migration block-param regression on 0.60.4 too castwide/solargraph#1288, the fix for the create_table block parameter losing its type (castwide/solargraph#1286), is still open and unmerged. 0.60.4 was cut before it landed, so the migration completion example fails the same way it does on 0.60.3, with an empty completion list instead of ["column"]. Per the comment already on this list, a later version that still has the bug should fail loudly rather than skip silently; verified this one still has it by running the example against 0.60.4 directly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VLRE2U4AsfZcoV2YiAyhgD * Add the missing 0.60.4 skips to core/DateTime.yml core/DateTime.yml is the only definitions file the 0.60.4 generation run never processed: 83 of its entries carry a 0.60.3 skip and none carried a 0.60.4 one, while the 0.60.3 and 0.60.4 skip sets match exactly in all 17 other definition files. CI reported only the first symptom, a missing #utc_to_local_returns_utc_offset_times, because assert_matches_definitions raises on @missing before it prints @incorrect. Added here are the 13 entries solargraph 0.60.4 still gets wrong. The other 70 previously skipped entries are left alone on purpose: 0.60.4 resolves 69 of them correctly and DateTime#quarter needs no skip, so marking any of them skipped would trip the "marked as skipped, but is actually present and correct" check. Derived by running the core/DateTime example against 0.60.4 with a 0.60.3 control run in the same environment, rather than by FORCE_UPDATE, so that no type expectation was rewritten. The 12 other core/*.yml examples pass locally with their CI-generated 0.60.4 skip sets, which is what makes that environment a usable stand-in for CI here. * Exclude the files 0.60.4 still cannot typecheck The workflow installs solargraph from castwide/solargraph master and gates its exclude list on that build reporting exactly 0.60.3. Master released 0.60.4 in 6dcb73338, so the gate stopped firing and the excluded files went back through the strong typecheck: strong typecheck (3.2, 7, 1) config/puma.rb:24: Unresolved call to > 1 problem found in 1 of 24 files. strong typecheck (3.2, 8, 0) db/{cable,cache,queue}_schema.rb: Unresolved call to create_table and add_foreign_key 23 problems found in 3 of 24 files. That unexcluded run enumerates what actually still fails, and it left config/boot.rb, config/environments/*.rb and config/routes.rb clean, so only puma.rb and the schema files are excluded here. Re-listing the whole 0.60.3 set would suppress files that now typecheck. Both causes are traced and linked rather than described. puma.rb is Integer("5") inferring Integer, nil, because Kernel#Integer's String overload takes _ToI and RBS interfaces are matched nominally. The schema files need the block self type on ActiveRecord::Schema.define, which conversion drops. Neither has landed upstream, so the excludes stay until they do. The test stays an exact version match rather than a range, for the reason given on broken_versions in rails_spec.rb: a later version that still has the bug fails loudly instead of being skipped silently. * Exclude config/routes.rb from the 0.60.4 typecheck too The 0.60.4 block carried forward two of the five paths the 0.60.3 block excluded, and one of the three it dropped still fails: config/routes.rb:6: Unresolved call to get 1 problem found in 1 of 23 files. The split was right for the other two. The 0.60.3 comment cited castwide/solargraph#1279 for the ENV failures and #1288 for the yieldreceiver pins. #1279 merged and shipped in 0.60.4, so config/boot.rb and config/environments/*.rb typecheck clean now and were correctly dropped. #1288 is still open, and routes.rb is its case: get reaches the draw block only through this gem's cross-file @yieldreceiver stub on ActionDispatch::Routing::RouteSet#draw. Verified by reproduction rather than inference. A Rails 7.1.6 project built against solargraph 6dcb73338 and this branch, run with the exact CI exclude list, is green with routes.rb excluded and reports the same single error without it. --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
The cross-file @!parse spec called its gem-side source plain_impl and then spent two comment lines explaining what "plain" meant. Rename it to gem_source so the name carries that, and shorten the comment. "pin" as a verb in store_spec collides with Solargraph::Pin, the domain object the surrounding spec is about; say "assert" instead. The combine_duplicate_method_pins docstring ran to seven lines counting its @PARAM and @return tags. Fold the fourth prose line into the third.
namespace_pin_for_generics carried a @type on candidates. Dropping it shows what it was covering: lib/solargraph/api_map.rb:822: Unresolved call to generics on Solargraph::Pin::Base select with an is_a? block does not narrow the returned element type, so the Array stays Array<Pin::Base>. That is a real inference gap and distinct from the flow-sensitive-local narrowing tracked in 1241, 1251, 1254 and 1296; nothing upstream covers it. Replace the cast with an @sg-ignore naming it, so the finding survives rather than being hidden. The DuckMethod example asserted only that typify did not raise. It now asserts the pin has no closure and still resolves to ::String through the core Object#to_s, which is the property that would actually regress. combine_duplicate_method_pins named neither situation that produces duplicate same-path pins; say what they are. The alias and DelegatedMethod reasoning it carried is already stated by the specs that cover those two cases.
The two Timeout.timeout(5) wrappers in store_spec dated from when Pin::Method#combine_same_type_arity_signatures could blow up exponentially. That is fixed and merged, so the wrappers guard nothing and the example is renamed for what it actually asserts: 30 same-path pins combine into one. The timeout require goes with them. namespace_pin_for_generics gains a @todo weighing merging duplicate namespace pins against picking one of them. The DuckMethod example said "core method return type", which names nothing in particular. It resolves through Object#to_s to String; say that.
apiology
marked this pull request as ready for review
September 5, 2026 22:17
A gem's plain &block signature and a workspace @!parse stub's yield-typed one only differ in the block's own type_arity, so combine_signatures_by_type_arity bucketed them as separate overloads and left picking between them to Chain::Call at every call site. Merge them into one signature before that bucketing instead, and make Callable#combine_blocks prefer the block that actually declares parameters rather than choosing arbitrarily. The merged pin is now correct on its own - hover and completion benefit too, not just inference at a call that happens to trigger dispatch_order.
The merge only fired when one side's block declared zero yielded parameters. A block documenting one parameter and a sibling documenting two hit the identical type_arity mismatch and still failed to combine. Compare declared parameter counts instead of emptiness, so any side with fewer yielded parameters loses to one with more, not just the zero case.
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
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.
Fixes #1286
Problem:
A
@generictag that a@!parsestub adds to a class already defined elsewhere doesn't apply, so the type variable never binds and every call downstream of it stops resolving.It happens whenever the plain definition is mapped first — a gem loading ahead of the workspace, or simply appearing earlier in the same file — so an annotation written this way is inert with nothing reported to say so.
Solution:
Choose the namespace pin that actually declares generics rather than whichever loaded first, and combine same-path method pins so a stub's
@returntags survive alongside the gem's own definition.Problem:
A workspace
@!parsestub's block parameter type has no effect when the gem's own doc already declares a return type:The method pin itself carries only one of the two signatures, so hover and completion lose the same information inference does.
Solution:
combine_signatures_by_type_aritybuckets signatures bytype_aritybefore merging, and a block's declared-parameter count changes that bucket, so this adds a pass that merges block-informativeness-only differences first and makesCallable#combine_blocksprefer the more informative block instead of choosing arbitrarily; two related gaps stay open (a stub missing@returnis discarded wholesale bycombine_signatures, anddef self.buildpaired withclass << selfnever combines pins at all).🤖 Generated with Claude Code