Check keyword arguments passed through a ** splat - #63
Draft
apiology wants to merge 2 commits into
Draft
Conversation
ComplexType::UniqueType#qualify widened every literal type to its class,
so a record type written as Hash{:a => Integer} & Hash{:b => String}
arrived at any consumer as Hash{Symbol => Integer} & Hash{Symbol =>
String} - the keys, which are the whole content of a record type, were
gone by the time inference finished.
Literal keys are now qualified by hand and kept; literals in any other
position still widen, which is what castwide#1201 disabled literal inference
for. Intersection holds conjuncts rather than key_types/subtypes, so it
qualifies those instead of inheriting the walk.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015AsvDi68YqsKoBtS2kg9ch
--level strong reported every required keyword of a call as missing when
the keywords came through a ** splat of a hash whose type records no
keys, e.g. foo(**args) where args is a Hash{Symbol => Integer}. The keys
are not in the type, so the checker could not see whether they were
supplied; reporting them as absent was a claim it had no basis for.
Two behaviours replace that one. When the splatted value has a record
type - Hash{:a => Integer} & Hash{:b => String} - its keys are checked
like literal keywords: required keywords must be present, values must
conform to the parameter types, and a key the method does not accept is
reported as unrecognized. When the type records no keys, the call gets
Cannot verify keyword arguments to #foo: the ** splat is
Hash{Symbol => Integer}, which does not record its keys, so required
keyword arguments a, b cannot be checked - give the splatted value a
record type (e.g. Hash{:a => Object}) to check it
once per call rather than one "missing keyword" per parameter.
convert_hash also discarded literal keys whenever a kwsplat shared the
hash node, so foo(a: 1, **args) lost a, and it turned the splatted
variable's own name into a key, which is what produced "Unrecognized
keyword argument kwargs" on super(name: name, **kwargs) - the two
@sg-ignore comments that suppressed it are gone. hash_is_splatted? now
looks at every child rather than only the last.
Two @sg-ignore comments on the calls that destructure
keyword_splat_types record a separate Solargraph defect: multiple
assignment from an Array(A, B) return type gives every variable the type
of the first element. Declaring the type of par cleared eight existing
findings in kwarg_problems_for and left one about the declaration.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015AsvDi68YqsKoBtS2kg9ch
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.
Passing a hash through
**currently produces one falsemissing keyword argumentfinding per declared keyword, because the splatted hash's keys are invisible to the checker. This makes those calls checkable instead: a record-typed splat has its required keywords, value types and unrecognised keys verified for real, and an opaque splat produces a single diagnostic in place of N false ones — only where a required keyword is supplied by neither a literal key nor a record type. An opaque splat into a method with only optional keywords stays silent.The enabling change is in
ComplexType::UniqueType#qualify, which widened every literal, soHash{:a => Integer}reached the checker asHash{Symbol => Integer}and record types were erased before anything could use them. Preserving literals wholesale broke 11 specs, so this narrows it to keys only.Intersectiongets its ownqualify, since it holds conjuncts rather than key types and subtypes.Stacked on castwide#1231 and cannot merge before it.
This PR was written by Claude Code on behalf of @apiology.