Fix PHPRedis hSet/Nx function signatures#5734
Closed
DKhalil wants to merge 657 commits into
Closed
Conversation
`array_intersect_key(\$this, \$other)` keeps entries from `\$this` whose keys appear in `\$other`. The unsealed extras in `\$this` survive only at keys that `\$other` can also have — narrow the unsealed key type to the intersection of `\$this->unsealed[0]` and `\$other->getIterableKeyType()`. If the intersection is `Never`, drop the unsealed slot entirely. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`array_reverse` only permutes element positions. The unsealed slot describes "zero or more extras at unspecified positions" — that property is unchanged by reversal, so pass `\$this->unsealed` through to the result. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`traverse()` already invokes the callback on the unsealed value type (commit history c. \`traverse\` unsealed support). Its \`traverseSimultaneously\` sibling skipped that step — only the explicit \`valueTypes\` were paired with \`\$right->getOffsetValueType()\`, leaving the unsealed value untransformed. Pair the unsealed value with \`\$right->getIterableValueType()\` and thread the result back into the new \`unsealed\` tuple, mirroring how \`traverse()\` handles it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
\`array_pop\` removes the last element. On a sealed array the last explicit key is the one that disappears, but with real unsealed extras on the source the popped element might come from the unsealed range instead — the actual value might have zero extras (so a trailing explicit key is popped) or one+ extras (so an extra is popped, leaving the explicit keys intact). Encode the union: when the source is unsealed, the trailing \`\$length\` explicit keys become optional and the unsealed slot is preserved. Sealed inputs keep the previous "hard-remove last keys" behaviour. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
\`array_shift\` removes the first element. With explicit keys present on the source, the shifted element is always one of them — the unsealed extras live "after" the explicit keys in insertion order and are untouched by the operation. Pass \`\$this->unsealed\` into the builder so the result keeps its zero-or-more extras. (Re-indexing of int keys doesn't change the unsealed range — a \`<int, V>\` slot stays \`<int, V>\`.) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`array_keys($source)` / `array_values($source)` produce a list whose explicit values come from `$source`'s keys/values. With real unsealed extras on `$source`, the source's unsealed *key* type becomes the new unsealed value for `array_keys`, and the source's unsealed *value* type becomes the new unsealed value for `array_values`. The new unsealed key range is `int<0, max>` — the conventional shape for list-style extras, which also collapses the describe output to the `<value>` short form. Previously the shared helper passed `$this->unsealed` through verbatim (the two `// todo unsealed` markers); for an unsealed source that produced a result with the *source's* unsealed key/value in the *result's* unsealed slot — semantically nonsense. Pass an explicit `$unsealedSourceType` so each caller injects the right projection. `getKeysArrayFiltered` flows through the same helper and benefits implicitly: its `getIterableValueType()` now includes the right unsealed value contribution. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`array_slice($arr, $offset, $length)` returns up to `$length` elements starting at `$offset`. When the slice fits within the source's explicit keys, the result is sealed as before. When the requested length runs past the explicit keys, the trailing slots could be filled by the source's unsealed extras (or be absent) — carry the unsealed slot through to the result so those potential extras are preserved. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`array_chunk` with a constant length precisely enumerates the chunks
for sealed CATs: each chunk is a known slice of the source. With
real unsealed extras the source has an unknown number of trailing
entries that could form additional partial or full chunks — the
precise enumeration would lie about the chunk count.
Fall back to `traitChunkArray`, which yields the general
`non-empty-list<array{...source-shape...}>` form. Less precise but
correct.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…form `ConstantArrayTypeBuilder::getArray()` in the degrade-to-general-array branch builds `new ArrayType(union($keyTypes), union($valueTypes))` from the explicit slots only — the unsealed extras' key/value contribution was silently dropped. Union the unsealed key/value into the degraded `ArrayType`'s key and value unions when the builder carries real extras. Fixes shuffleArray (and any other call site that round-trips through `degradeToGeneralArray()`) on unsealed sources, where the unsealed value type now reaches the final list/array form. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`array_splice` removes a slice at an explicit offset and inserts a replacement at that position. Real unsealed extras live at positions past the explicit keys, so they're not affected by either operation; the re-indexing of int keys also leaves a `<int, V>` unsealed range unchanged. Pass the source's unsealed tuple into the builder before producing the result for each replacement variant. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Twelve cases pinning the current behaviour: empty arrays (legacy null vs. bleeding-edge NeverType marker), sealed inputs at each precision (lessSpecific, moreSpecific, templateArgument), single and multi-key shapes, list shape, all-optional keys, and unsealed inputs — including the "no explicit keys + real extras" case where the current implementation returns `$this` unchanged instead of generalizing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…nged
`generalize()` early-returned `$this` when `count($keyTypes) === 0`,
which is correct for actually empty sealed arrays but wrong for a
CAT with no explicit keys but real unsealed extras
(`array{...<int, 'foo'>}` — type-checked construction; the builder
normally collapses this shape to `ArrayType`, but it can still
appear through `recreate()` paths). The constant values inside the
unsealed slot survived `generalize()` untouched.
Two fixes, mirroring each other:
1. Only early-return for "no explicit keys *and* not unsealed".
When the CAT carries real extras, proceed through the normal
path so the unsealed key/value are generalized through
`getIterableKeyType()` / `getIterableValueType()`.
2. Switch the `NonEmptyArrayType` gate from
`keyTypesCount > optionalKeysCount` to `isIterableAtLeastOnce()->yes()`.
The old gate was a proxy for "the array is definitely non-empty"
that only looked at explicit keys; for "no explicit keys + real
extras" it would incorrectly require the accessory. The
`isIterableAtLeastOnce()` answer is what we actually want — and
for sealed shapes it matches the previous gate exactly.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`generalizeValues()` walked the explicit `valueTypes` only — for an unsealed source the unsealed value type (e.g. `'foo'`) survived unchanged, leaving the result inconsistent: explicit values generalized to `int`/`string`/etc. but the unsealed slot still carried a constant. Generalize the unsealed value too, mirroring the explicit loop. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rically The empty-keys block unconditionally overrode `$isList` from the unsealed key type, while the non-empty path only filled it in when null — an asymmetry. No caller actually passes a non-null `$isList` for an empty CAT, so the override and the only-when-null path were equivalent today, but the rule was inconsistent on its face. Wrap both branches in `if ($isList === null)` so the constructor consistently says "trust the caller when given, infer from shape otherwise". Also drop the stale `makeList()` TODO — the comment was speculating about validation that would require `isList=Maybe` co-occurring with a non-list-compatible shape, a state that no source-level flow actually constructs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`array_sum` summed only the explicit `getValueTypes()` of a constant
array, silently dropping the unsealed extras — `array_sum(array{1, 2,
...<int, int>})` inferred `3` even though the real sum is `3` plus
zero-or-more further ints. That's unsound, not just imprecise.
Add two result variants for unsealed inputs: the zero-extras case
(the exact explicit sum, so a float unsealed value can't erase the
precise int sum) and the one-or-more-extras case (explicit sum plus
the unsealed value type multiplied by an unbounded count). Sealed
constant arrays are unaffected.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`inferConstantType` built the exact result string from the explicit
`getValueTypes()` only — `implode(',', array{'a', 'b', ...<int,
string>})` inferred `'a,b'` even though the unsealed extras can append
further segments. That's an unsound constant fold.
Bail out of the constant fold when the input `isUnsealed()->yes()` and
fall through to the accessory-based result, which only keeps what's
provable (e.g. `non-falsy-string` when the separator is non-falsy and
the explicit prefix guarantees at least one separator).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`processArrayType` fed only the explicit `getValueTypes()` into the
min/max comparison, so `min(array{1, 2, ...<int, int>})` inferred `1`
and `max(...)` inferred `2` — unsound, since the unsealed extras can
be any int above or below the explicit entries.
Append the unsealed value type to the comparison arguments when the
constant array `isUnsealed()->yes()`, so the result widens to cover
the extras (`int` here). Sealed constant arrays keep their exact
min/max.
`vsprintf` was on the same verify list but is already sound: it reads
only the positions the format references (extras beyond them are
ignored) and bails to a general string when a referenced position
falls outside the explicit keys.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`ArrayCombineHelper` paired only the explicit `getValueTypes()` of the
keys and values arrays, dropping unsealed extras —
`array_combine(array{'a', 'b', ...<int, string>}, array{1, 2, ...<int,
int>})` inferred a sealed `array{a: 1, b: 2}` even though the matching
extras pair up into further entries.
When both inputs are unsealed (their extra counts are both unbounded
and must match for the call to succeed), attach the result's unsealed
slot: the keys' unsealed value (as an array key) mapped to the values'
unsealed value. If only one side is unsealed the sealed side caps the
size, so no extras survive and the result stays sealed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`findConstantStrings` walked a constant array's explicit
`getValueTypes()` to collect the variable names, so an unsealed names
array — `compact(array{'foo', 'bar', ...<int, string>})` — produced a
sealed `array{foo: ..., bar: ...}`, ignoring that the unsealed extras
are further unknown variable names.
Return `null` (give up the precise shape) when the names array is
`isUnsealed()->yes()`, so the caller falls back to the general
`array<string, mixed>` signature.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…_encoding()`
The PHP 8 "can this return false?" check counted only the explicit
`getValueTypes()` of the from-encoding array, so a one-explicit-key
unsealed array — `mb_convert_encoding($s, 'UTF-8', array{'FOO',
...<int, string>})` — was treated as a single guaranteed encoding and
the `false` branch was dropped. The unsealed extras can supply further
candidates, making it an auto-detect list that may fail.
Extend the `count(...) > 1` gate with `|| isUnsealed()->yes()` so the
result keeps `false` as a possible outcome.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…quals()`
`equals()` already documented that a null unsealed slot (legacy /
pre-bleeding-edge, `isUnsealed()` = `Maybe`) and the
`[NeverType, NeverType]` sealed marker (bleeding edge, `isUnsealed()`
= `No`) both mean "no real extras" and should compare equal. But the
implementation used `isUnsealed()->no()`, which is `false` for the
null case and `true` for the marker — so a legacy-null shape and a
marker-sealed shape of otherwise-identical structure compared
*unequal*.
This surfaced as `TypeToPhpDocNodeTest::testToPhpDocNode` failing only
under old PHPUnit (PHP < 8.2): there the data provider runs before the
test container enables bleeding edge, so the directly-constructed
expected type gets a null slot while the round-trip-parsed type
(built after bleeding edge is on) gets the marker. New PHPUnit runs
data providers after container init, so both sides got the marker and
the bug stayed hidden.
Compare on `isUnsealed()->yes()` ("has real extras") instead, so null
and marker are both treated as sealed and only genuine extras are
compared. Added a timing-independent unit test that constructs both
forms directly via `BleedingEdgeToggle`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ization
When `generalizeType` merges two sealed constant-array shapes on the
fall-through ArrayType path, it kept the literal key union (so loop-bounded
counter keys stayed within range) but still recursed into `generalizeType`
for the values — which applies the loop-widening heuristic and widened e.g.
`int<0, 5>` to `int<0, max>`. Keep the literal value union too (with a
fallback to generalization when it outgrows the shape limit), so values like
loop-built `array{int<0, 5>, int<1, 6>}` keep their bounds.
Scoped to sealed shapes so the general `generalize()` widening contract for
legacy arrays (ScopeTest::testGeneralize) is unaffected.
Refs phpstan/phpstan#13647
Refs phpstan/phpstan#13637
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
6 participants
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.
Updating the Redis::hSetNx, RedisCluster::hSetNx, RedisCluster::hSet signatures to match the documented ones:
https://phpredis.github.io/phpredis/RedisCluster.html#hSetNx