Conform Hash to Enumerable of key/value pairs, not just values - #69
Draft
apiology wants to merge 10 commits into
Draft
Conform Hash to Enumerable of key/value pairs, not just values#69apiology wants to merge 10 commits into
apiology wants to merge 10 commits into
Conversation
Hash is 2-arity (key_types, subtypes), but the Enumerable/_Each ancestor it satisfies structurally is 1-arity: Hash#each yields [key, value] pairs, so Hash includes Enumerable[Array[K, V]], not Enumerable[V]. Conformance previously compared a Hash raw key_types/subtypes directly against a 1-arity expectation once erased_type_conforms? established Hash <: Enumerable via inheritance, so URI.encode_www_form(token: abc) failed strong typecheck even though RBS declares encode_www_form(enum: Enumerable[[_ToS, _ToS]]) and a Hash is Enumerable at runtime. Reshape the Hash into the [key, value] tuple its ancestor actually sees before continuing the parameter comparison.
The pair-reshape in Conformance#hash_viewed_as_pairs? checked
inferred.name == 'Hash' before comparing key_types/subtypes as a
pair. That name check was tighter than the actual discriminator:
parameters_type == :hash is set purely by the YARD `{K => V}` tag
substring in UniqueType.parse, with no restriction to the class
name Hash, so any class documented that way is 2-arity and gets
the same reshape need against a lower-arity ancestor.
RBS translation (rbs_translator.rb, rbs_map/conversions.rb) only
ever sets parameters_type: :hash when base == 'Hash', so in
practice, with stdlib/gem RBS types, Hash is the only type that
reaches this path today. The check is written for the general
case anyway, since YARD-documented classes are not limited that
way.
Renamed hash_viewed_as_pairs?/hash_as_pairs to
pair_shaped_viewed_as_pairs?/pair_shaped_as_pairs to match. Added
a conforms_to_spec.rb case with a non-Hash PairBag class including
Enumerable, parsed via the {K => V} tag, to prove the structural
check actually handles a type the old name check would have
missed: with inferred.name == 'Hash' still in place, that case
returns false; dropping it returns true and leaves the existing
Hash{K => V} case unchanged.
Conformance#pair_shaped_viewed_as_pairs? only reshaped
:hash-parameterized types (Hash{K => V} and the YARD {K => V} tag)
into a [key, value] tuple before comparing against a lower-arity
Enumerable ancestor. A class declared with ordinary <A, B> generic
syntax (parameters_type == :list) that structurally includes
Enumerable[[A, B]] hit the same raw-params-vs-tuple mismatch, but
was never reshaped, so it failed to conform.
Generalize the check to any inferred type with 2+ ordered params
compared against a differently-sized expectation, and build the
tuple from key_types/subtypes for :hash or from all_params for
:list, in a single pair_shaped_as_pairs.
pair_shaped_viewed_as_pairs? fired on arity mismatch alone, for any ancestor erased_type_conforms? found via inheritance - not just Enumerable/_Each. A 2-arity type including some other 1-arity generic module (unrelated to the tuple semantics) could spuriously conform whenever that module happened to be parametrized with an Array type matching the wrapped tuple shape. Gate the reshape on expected.name being Enumerable or _Each, the two names the method's own comment already claims to cover, matching the precedent list in unique_type.rb's implicit_union?. The legitimate Pair/Triple specs use bare `include Enumerable` with no declared type binding, so there is no RBS include-generic-value data available to check the tuple binding directly for those cases - a name allowlist is the check that's actually available. One constructed case (a hash-shaped type against a 1-arity ancestor unrelated to its key/value types) is left `pending`: blocking the reshape there falls through to key_types_conform?/subtypes_conform?, which conflates a hash-shaped type's value type with a list-shaped ancestor's single param positionally. That fallback path is unchanged baseline code (git diff against the branch's merge-base shows the pair_shaped_* methods are the entire diff this branch makes to this file), so the false positive predates this branch and is out of scope here.
Comment blocks in this PR ran well past the 1-3 line budget per method/example, and several restated what the code or the it title already showed. Condense each to the non-obvious why, dropping restatement of the mechanism. The pending spec's explanatory comment also carried changelog content (a `git diff <merge-base>..273c7e6` citation and "before this PR existed" / "out of scope for this PR" framing) confirming the fallback path in key_types_conform?/subtypes_conform? predates this PR's pair_shaped_* additions. That verification belongs here, not in the comment: this PR's only change to conformance.rb is the pair_shaped_* methods and their call site: the key_types_conform?/ subtypes_conform? fallback is unmodified baseline behavior.
…ash-enumerable-pairs-onto-master
Conformance reshaped a Hash into key/value pairs only for ancestors
listed in TUPLE_YIELDING_ANCESTOR_NAMES, a hard-coded %w[Enumerable
_Each]. The list was standing in for a property the ancestor already
declares, so it covered exactly two names and no user-defined module.
Two derivations replace it. Where the ancestry declares its arguments,
ApiMap#type_as_ancestor resolves them against the inferred type, so
Hash's `include Enumerable[[K, V]]` turns Hash{String => Integer} into
Enumerable<Array(String, Integer)> with nothing guessed. Where it
declares none, as a bare `include Enumerable` does,
ApiMap#yields_type_parameter? asks whether the ancestor's single type
parameter appears as a block parameter in one of its own signatures -
true of Enumerable via #map, false of a module that yields nothing.
Both read data RbsMap::Conversions already records; only the query was
missing. _Each drops out of the code entirely: it is a
RbsMap::CoreFills synthetic include, and Hash never had one.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WJfMsNRKVrBMkSfXvsEiT3
undercover reported the method at 88.89%: the trailing `nil`, reached
when the ancestry names no match, had no test exercising it. The check
runs under continue-on-error, so the job reported success anyway.
The paired positive example also records what the derivation produces,
which nothing else asserts directly - Hash{String => Integer} resolving
through `include Enumerable[[K, V]]` to Enumerable<Array(String, Integer)>.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WJfMsNRKVrBMkSfXvsEiT3
YARD reads a tag's name as its first whitespace-delimited token, and Pin::Closure#generics maps those names straight through, so `@generic A, B` produced generics ["A,"] - one entry, comma kept. `generic<A>` then missed on the index lookup and `generic<B>` was absent, leaving both unresolved with nothing reported. Six classes across these examples used the one-line form. Outcomes are unchanged, because they exercise the reshape through the type tag's all_params rather than through the namespace's generics - which is why nothing caught it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WJfMsNRKVrBMkSfXvsEiT3
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.
Problem: URI.encode_www_form takes Enumerable[[_ToS, _ToS]] per RBS core. Solargraph's strict typecheck rejects a real Hash literal there anyway, in code that's run correctly for years:
test/unit/sources/test_airnow.rb:18: Wrong argument type for URI.encode_www_form: enum expected Enumerable<Array(_ToS, _ToS)>, received Hash{Symbol => Float, String, Integer}
body: URI.encode_www_form(latitude: 38.8857022, longitude: -77.0937858, stateCode: 'VA', maxDistance: 50)
Confirmed directly: URI.encode_www_form(token: 'abc', user: 'x') returns "token=abc&user=x" at runtime -- a Hash always satisfies this contract.
Cause: ComplexType::Conformance#conforms_to_unique_type? checks Hash<K,V> against Enumerable<Array(_ToS,_ToS)> by comparing V alone against the expected tuple type, instead of reshaping the hash into [K,V] pairs first:
Solution: Conformance#pair_shaped_viewed_as_pairs? detects when inferred has 2+ ordered generic params being checked against a different-arity ancestor in TUPLE_YIELDING_ANCESTOR_NAMES (Enumerable, _Each) -- the convention for "all of my own params yielded together as one tuple," per Hash's own core signature
include Enumerable[[K, V]]. pair_shaped_as_pairs then reshapes inferred into that ancestor parametrized with the tuple its params form, and conformance is re-checked against that.This PR was written by Claude Code on behalf of @apiology.