Infer precise Hash{K => V} types from hash literals - #1352
Open
apiology wants to merge 18 commits into
Open
Conversation
Pin::Parameter#compatible_arg? rejected any argument against an RBS-interface-typed parameter (e.g. Hash#fetch's _Key) unless the argument's type was nominally included via CoreFills::INCLUDES. Since Hash#fetch's non-block, non-default overload takes a _Key-typed parameter, no overload of a plain h.fetch(k) call ever matched, and Pin::Method#return_type fell back to unioning every overload's return type together - including the unbound generic X from the two overloads that require a default value or a block. Add :allow_unmatched_interface to the rules passed to conforms_to? in compatible_arg?, matching the leniency TypeChecker itself already applies by default (see Rules#require_interfaces_resolved?) everywhere except the :alpha level. Fixes castwide#1227
Chain::Hash#resolve returned the bare, unparameterized ::Hash type
for every hash literal, so a chained fetch/[] off a hash literal
leaked generic<T>/generic<U> instead of resolving to a concrete
value type. Chain::Array already infers Array<T> from its elements
the same way; this brings Chain::Hash to parity by chaining each
pair's key and value node (NodeChainer#hash_pairs) and inferring
Hash{K=>V} from their actual types (Chain::Hash#inferred_type).
Stacked on castwide#65 (fix-issue-1227), which added the
_receiver_path param to Chain::Hash#resolve that this commit merges
with.
Follow-on to PR 65: this covers the generic<T>/generic<U> leak PR 65
doesn't, tracked as tool-limitation:pr-65-follow-on.
apiology/solargraph-rspec:fix-stale-hash-literal-assertions updates two convention_spec.rb assertions this PR's own hash-literal inference fix made more precise. Not opened as a PR yet - too early for castwide/solargraph-rspec until this PR itself is ready.
Compress three new docstrings/tags added by this PR down to the 1-3 line budget, keeping only the non-obvious why.
run_solargraph_rspec_specs cloned lekemula/solargraph-rspec's default
branch unpinned, so it tested this branch's Hash record inference
against expectations written before records were inferred:
expected: "Hash"
got: "Hash{String => String}"
That is better inference meeting an older spec, not a regression.
lekemula/solargraph-rspec#36 raises those expectations; clone its head
branch until it merges. The original clone line stays in place,
commented out, so reverting this is a one-line change.
Drop the commented-out checkout of fix-stale-hash-literal-assertions:
it names an earlier attempt at this same problem, predating pull/36,
and would otherwise leave two comment layers naming two branches.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TfUfWzAWx3NTkJ79AtGeA9
castwide#1266 already fixes the Hash#fetch generic leak this addition targeted, via real structural conformance instead of a blanket allow. Mark the regression spec pending on that PR and let compatible_arg? go back to its narrower rule set.
Hash#fetch only takes its key as the _Key interface (rather than the generic K) as of RBS 4.1.0, so the leak this spec regresses against does not reproduce on older RBS. Marking it unconditionally pending made RSpec fail it on rbs 3.10.0 in CI, since a pending example that passes is itself a failure. Gate the pending call on RBS::VERSION so the spec only expects the failure where it actually occurs.
key_types and value_types start empty, so Solargraph has nothing to infer their element type from - the same reason pairs already carries a @type tag two lines above. Both feed straight into UniqueType.new(key_types, subtypes:), which documents both as Array<ComplexType>.
Every other Chain subclass's resolve takes exactly (api_map, name_pin, locals), matching Link#resolve and both call sites in chain.rb. This was the only one carrying a fourth parameter, and nothing in the method body used it.
Both cited castwide#1223 (still open), suppressing an argument-type mismatch on the AST node's own children before hash_pairs existed. Solargraph now resolves both calls cleanly without them; typecheck stays at the same pre-existing 15 problems elsewhere in the file, none new.
response = { jsonrpc: '2.0', id: id } now infers Hash{Symbol =>
String, Integer} instead of bare ::Hash, so assigning result/error
into it needs the full value-type union stated explicitly.
Assigning result or error still fails even with that union declared:
each is itself a union containing nil, and conformance checking
rejects a nil-bearing union against a target union that also contains
nil, even though every member is present in the target. Verified
against castwide#1223 (stacked: #40) -
checked out that branch, reproduced the exact case standalone, 0
problems found. Not yet merged upstream, so sg-ignore until it lands.
result[:capabilities] returns V | nil per Hash#[]'s own RBS signature, so every merge!/[]= off it was unresolved once result's value type stopped being bare ::Hash. fetch(:capabilities) raises instead of returning nil, matching the actual guarantee (the key was just set two lines above) and clearing all 13 problems.
Position#to_hash returns { line: Integer, character: Integer }, not
the bare Hash it was declared as. Range#to_hash was declared
Hash{Symbol => Position}, but its literal calls start.to_hash and
ending.to_hash - it never returned a Position at either key. Both
tags now match what the methods actually return.
@Level = LEVELS[LEVELS.values.index(@rank)] indexed a Symbol-keyed Hash with an Integer - always nil, confirmed empirically. Rewritten to keep the input level directly instead of round-tripping through rank; @rank now uses fetch to state the non-nil guarantee the LEVELS.key?(level) branch already establishes. Hash#fetch(key), called with exactly one argument and no block, still leaks generic<X> from its other two overloads into the return type - a distinct, minimal, reproducible bug from the Hash{K=>V} precision work, confirmed fixed on castwide#1266's branch.
set_result's nil literal and normalize's Array(Integer, Integer) element access both typecheck clean once castwide#1223 lands; until then Solargraph reports NilClass instead of nil and an unnarrowed nil on tuple element access. Both pre-existing on master.
apiology
marked this pull request as ready for review
September 9, 2026 15:15
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.
A Hash literal infers only the bare
::Hash— far less precise than an Array literal, which already infersArray<T>from its elements.That imprecision costs anything downstream that needs the Hash's real key/value types to type-check —
#fetch,#[],#each, and the per-conjunct Hash-record dispatch in #1231.Solution
NodeChainer#hash_pairschains each key/value pair of a hash literal, andChain::Hash#inferred_typeinfersHash{K => V}from those pairs' own inferred types, mirroring howChain::Arrayalready infersArray<T>. Theplugins.ymlrepoint tests this against lekemula/solargraph-rspec#36, which expects the added precision.Dropped the
:allow_unmatched_interfaceaddition toPin::Parameter#compatible_arg?that was bundled here to fix #1227 — #1266 already fixes that properly via real structural conformance, and #1231's own Hash-record dispatch is the more complete answer for the record-shaped case. The #1227 regression spec is nowpending, citing #1266.