Skip to content

Allow arguments to satisfy RBS interface-typed parameters - #1228

Closed
apiology wants to merge 7 commits into
castwide:masterfrom
apiology:fix-hash-fetch-generic-rbs-4.1
Closed

Allow arguments to satisfy RBS interface-typed parameters#1228
apiology wants to merge 7 commits into
castwide:masterfrom
apiology:fix-hash-fetch-generic-rbs-4.1

Conversation

@apiology

Copy link
Copy Markdown
Contributor

🤖 Filed by Claude, not Vince — acting on his behalf via his GitHub credentials.

Fixes #1227.

Problem

As of RBS 4.1.0, Hash#fetch's single-argument overload takes its key as the ::Hash::_Key duck-type interface instead of the generic K (see ruby/rbs core/hash.rbs). Pin::Parameter#compatible_arg? had no way to confirm an argument satisfies an interface without an explicit include declaration — which core gems don't add for every class that happens to satisfy Hash::_Key's #hash/#eql? contract — so the single-arg overload was rejected as a non-match for a plain Symbol argument.

Call#inferred_pins then fell back to merging the return types of all of Hash#fetch's overloads (including the unresolved generic<X> placeholder from the block-taking overload) instead of resolving the one overload that actually matched the call, producing a spurious typecheck error like:

Declared type Class<Foo> does not match inferred type Class<Foo>, generic<X> for variable clazz

on any Hash{Symbol => Class<X>}#fetch(:key) call under rbs >= 4.1.0.

Fix

Pass :allow_unmatched_interface into the conformance check in Pin::Parameter#compatible_arg?, so an argument is treated as compatible with an interface-typed parameter rather than rejecting the overload outright. This mirrors how TypeChecker already treats unmatched interfaces when comparing declared vs. inferred types (type_checker.rb:80).

Testing

  • Added a regression spec in spec/type_checker/levels/strong_spec.rb reproducing the exact repro from Hash#fetch on Hash{Symbol => Class<X>} miscomputes generics when rbs >= 4.1.0 #1227; confirmed it fails without the fix and passes with it.
  • bundle exec rspec: 1618 examples, 0 failures (65 pre-existing pending, unrelated to this change).
  • bundle exec rubocop on changed files: no offenses.
  • bundle exec rake typecheck: 548 problems (down from 551 on the same base commit without this change) — the remaining problems are pre-existing RBS 4.1.0 drift unrelated to this fix, out of scope here.

As of RBS 4.1.0, Hash#fetch's single-argument overload takes its key
as the ::Hash::_Key duck-type interface instead of the generic K.
Pin::Parameter#compatible_arg? had no way to confirm an argument
satisfies an interface without an explicit `include` declaration
(which core gems don't add for every class that happens to satisfy
Hash::_Key's #hash/#eql? contract), so the overload was rejected as a
non-match. Call#inferred_pins then fell back to merging the return
types of all of Hash#fetch's overloads, including the unresolved
`generic<X>` placeholder from the block-taking overload, producing a
spurious typecheck error on `Hash{Symbol => Class<X>}#fetch`.

Pass :allow_unmatched_interface into the conformance check so an
argument is treated as compatible with an interface-typed parameter,
consistent with how TypeChecker already treats unmatched interfaces
when checking declared vs. inferred types.

Fixes castwide#1227
@apiology

Copy link
Copy Markdown
Contributor Author

🤖 Comment from Claude, not Vince — acting on his behalf via his GitHub credentials.

CI status: 28/29 checks pass, including Solargraph / strong (the typecheck job this issue is about).

The one failure, run_solargraph_rspec_specs, is pre-existing on master and unrelated to this change — it is the downstream solargraph-rspec fork's own Array-generics inference test, already failing on the last 5 pushes to master, including the commit this PR is based on. This PR does not touch that code path.

See https://github.com/castwide/solargraph/actions/runs/30486979645 for that same failure on master.

apiology added 3 commits July 30, 2026 07:44
4.0.0 and 4.0.1 exercise essentially the same behavior in this
matrix; 4.1.0 is the version that introduced the Hash::_Key regression
fixed in this PR and wasn't pinned anywhere in CI, so a future rbs
release could silently drop coverage for it once it's no longer
"latest" in the unpinned jobs (typecheck.yml, plugins.yml).
4.1.1 is now the latest stable release (superseding 4.1.0), and
4.1.1.pre.1 is the latest available prerelease. Testing the current
latest stable/prerelease plus the oldest supported version (3.10.0)
gives better forward coverage than pinning to already-superseded
patch releases.
No prerelease newer than 4.1.1 exists yet (4.1.1.dev.1 and
4.1.1.pre.1 both predate the 4.1.1 stable release), so pinning to it
added no forward coverage over testing 4.1.1 itself.
@apiology
apiology force-pushed the fix-hash-fetch-generic-rbs-4.1 branch from 17ebf7a to 20511d9 Compare July 31, 2026 11:49
@apiology
apiology marked this pull request as draft August 2, 2026 14:42
@apiology
apiology marked this pull request as ready for review August 2, 2026 17:05
@apiology

apiology commented Aug 2, 2026

Copy link
Copy Markdown
Contributor Author

@castwide This one's ready for review

apiology added a commit to apiology/checkoff that referenced this pull request Aug 2, 2026
rbs 4.1.0 changed Hash's key/value generic params to _Key/_Value
duck-type interfaces, exposing a Solargraph bug where Hash#fetch's
inferred return type leaks a generic<X> instead of the declared value
type. Reproduces on vanilla solargraph 0.60.2, unrelated to the
apiology/solargraph fork.

castwide/solargraph#1228 already tracks and fixes this upstream, so
rather than pinning rbs below 4.1.0 to avoid the bug, remove the cap
and add targeted @sg-ignore comments citing the PR at each affected
call site, consistent with how #1229 is already cited elsewhere in
this repo.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B8YdoRihmgAK4MFHjmMECr
apiology added a commit to apiology/checkoff that referenced this pull request Aug 2, 2026
CIs clean-environment Solargraph run caught 6 problems that did not
reproduce in local runs (gem doc cache showed 192 vs CIs 200 gems,
suggesting an environment divergence not worth chasing further):

- attachments.rb: URI::Generic#host nilability now resolves correctly
  under rbs 4.1.1, making the old ignore unneeded; removed it.
- selector_classes/task.rb (3 sites) and subtasks.rb: additional
  Asana::Resources method/block-param return type inference regressed
  under rbs >= 4.1.0, same class of issue as castwide/solargraph#1228;
  added @sg-ignore comments citing that PR.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B8YdoRihmgAK4MFHjmMECr
apiology added a commit to apiology/checkoff that referenced this pull request Aug 3, 2026
rbs 4.1.0 changed Hash's key/value generic params to _Key/_Value
duck-type interfaces, exposing a Solargraph bug where Hash#fetch's
inferred return type leaks a generic<X> instead of the declared value
type. Reproduces on vanilla solargraph 0.60.2, unrelated to the
apiology/solargraph fork.

castwide/solargraph#1228 already tracks and fixes this upstream, so
rather than pinning rbs below 4.1.0 to avoid the bug, remove the cap
and add targeted @sg-ignore comments citing the PR at each affected
call site, consistent with how #1229 is already cited elsewhere in
this repo.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B8YdoRihmgAK4MFHjmMECr
apiology added a commit to apiology/checkoff that referenced this pull request Aug 3, 2026
CIs clean-environment Solargraph run caught 6 problems that did not
reproduce in local runs (gem doc cache showed 192 vs CIs 200 gems,
suggesting an environment divergence not worth chasing further):

- attachments.rb: URI::Generic#host nilability now resolves correctly
  under rbs 4.1.1, making the old ignore unneeded; removed it.
- selector_classes/task.rb (3 sites) and subtasks.rb: additional
  Asana::Resources method/block-param return type inference regressed
  under rbs >= 4.1.0, same class of issue as castwide/solargraph#1228;
  added @sg-ignore comments citing that PR.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B8YdoRihmgAK4MFHjmMECr
apiology added a commit to apiology/checkoff that referenced this pull request Aug 6, 2026
The 26 markers tagged upstream-type-annotation:rbs-4-1-regression
(citing castwide/solargraph#1228) are all instances of the same
Hash#fetch(...)-on-interface-typed-key pattern. #1228 is a narrow,
still-unmerged patch for one specific shape (Hash{Symbol => Class<X>})
and doesn't cover these. The real root cause is a genuine solargraph
engine gap, not missing/wrong RBS data: Hash::_Key's signature is
correct, solargraph just has no structural conformance check for
RBS interface-typed parameters generally (Hash::_Key, _ToAry, _Each,
_ToS, etc.), falling back to a blanket allow instead. That's tracked
generally at castwide/solargraph#1232, which is a better citation and
correctly buckets this as tool-limitation rather than
upstream-type-annotation.

Pure comment relabel; typecheck problem count unchanged (127).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kk5dVYpJr49U3YqNtFHwTL
apiology added a commit to apiology/solargraph that referenced this pull request Aug 8, 2026
…arameters

castwide#1228 fixes the same underlying bug as the already-merged
castwide#1266 (issue castwide#1227: RBS 4.1's Hash#fetch takes its
key as the Hash::_Key duck-type interface instead of a generic,
causing Solargraph to fall back to the unresolved generic<X> from the
block-form overload) but via a different, earlier mechanism: a blanket
:allow_unmatched_interface bypass in Pin::Parameter#compatible_arg?,
rather than castwide#1266's later structural Conformance check.

Verified castwide#1228's own regression test already passes unmodified on
this branch without its compatible_arg? change (isolated it into a
standalone spec file and ran it against HEAD before resolving the
conflict) - castwide#1266's structural interface verification already covers
this case, making castwide#1228's code change redundant here. Kept HEAD's
compatible_arg? as-is (including literal_arg_matches?, from an
earlier-merged PR that castwide#1228's branch, based directly on
castwide/master, never saw) and dropped castwide#1228's interface-bypass hunk
entirely.

Conflict in spec/type_checker/levels/strong_spec.rb: kept castwide#1228's new
regression test (issue castwide#1227) as a sibling of HEAD's intersection-type
test block (from castwide#1231), which castwide#1228's branch also never saw.

.github/workflows/rspec.yml auto-merged cleanly, taking castwide#1228's RBS
matrix bump (4.0.0/4.0.1/4.0.2 -> 3.10.0/4.0.3/4.1.1) - core to what
this PR is actually testing (RBS 4.1's Hash#fetch signature change).

Verified: spec/type_checker/levels/strong_spec.rb, spec/pin/parameter_spec.rb
(104 examples, 0 failures, 5 pending), and a broader safety net -
spec/type_checker, spec/source, spec/source_map/clip_spec.rb,
spec/complex_type, spec/complex_type_spec.rb (807 examples, 0
failures, 35 pending) - all passing locally.
@apiology
apiology marked this pull request as draft August 31, 2026 20:20
Compress the compatible_arg? comment from 7 lines to 3, and drop
the changelog-style multi-paragraph comment on the new Hash#fetch
strong-typecheck spec, moving its reasoning into the example name
instead.
apiology added a commit to apiology/solargraph that referenced this pull request Sep 1, 2026
castwide#1266 fixes the same underlying bug as castwide#1228
(castwide#1228) via real structural
interface conformance instead of a blanket allow_unmatched_interface
flag, making castwide#1228 redundant. These two pieces of castwide#1228 are not
duplicated by castwide#1266 and are carried over before castwide#1228 is closed:

- CI matrix bump in rspec.yml (tests rbs 4.0.3/4.1.1 instead of the
  now-superseded 4.0.0-4.0.2 range)
- End-to-end regression spec reproducing castwide#1227's original repro at
  the typecheck level (structural-rbs-interface-conformance only had
  unit-level specs against ComplexType::Conformance directly)
@apiology

apiology commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Claude: Superseded by #1266 — same fix, done via real structural conformance instead of a blanket allow.

@apiology apiology closed this Sep 1, 2026
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.

Hash#fetch on Hash{Symbol => Class<X>} miscomputes generics when rbs >= 4.1.0

1 participant