Skip to content

Walk each isset() subject narrowed by the subjects before it - #7062

Merged
ondrejmirtes merged 1 commit into
2.3.xfrom
isset-narrow-each-subject-by-previous
Nov 14, 2026
Merged

ondrejmirtes merged 1 commit into
2.3.xfrom
isset-narrow-each-subject-by-previous

Conversation

@ondrejmirtes

@ondrejmirtes ondrejmirtes commented Nov 13, 2026 •

Copy link
Copy Markdown
Member

Closes phpstan/phpstan#15729

isset() evaluates its arguments left to right and stops at the first one that isn't set, so isset($key, $map[$key]) never reads $map[$key] with $key = null. IssetHandler walked every subject on the same scope, so InvalidKeyInArrayDimFetchRule saw the second subject's offset as string|null and reported "Possibly invalid array key type string|null" whenever PHP 8.5 is in the analysed version range. Since 8cf6b64 that includes any project whose range reaches 8.5.

Each subject after the first is now walked on the scope narrowed by the previous subject being set (DefaultNarrowingHelper::createIssetTruthyChainTypes(), the same truthy narrowing the handler already composes), like isset($a) && isset($b). Each subject's ensured non-nullability is reverted right after its own walk, and its chain results are captured then. The offsetExists() throw-point block is unchanged. The walk may stop after any subject, so the resulting scope merges the scope after each subject. The truthy/falsey narrowing of the whole isset() is unchanged.

The turbo-ext mirror (turbo-ext/src/IssetHandler.cpp) gets the same change.

Verification:

  • Regression test in InvalidKeyInArrayDimFetchRuleConfigPhpTest (PHP version range 7.4–8.5). It fails without the fix (lines 8 and 20 reported) and passes with it. isset($map[$key], $key) still reports, since there the key is checked only after the offset.
  • Full make tests, with and without the extension loaded. The regression test also passes with only the native port applied.
  • turbo-ext/tests/walk-trace.php --shards=8 gives identical traces, and smoke.php and signature-parity.php pass. The strict -Werror build is clean.
  • make phpstan and phpcs are clean.

Benchmark: tests/bench/data/bug-15061.php (a 26-subject isset($entity['a'], ..., $entity['z'])) goes from 3.166B to 3.920B instructions (+23.8%), and its numbers are accepted from this PR's tally-results. Each subject is now narrowed by the one before it, so $entity's native type (mixed) is intersected with one more hasOffset() per subject, and TypeCombinator::intersect() re-normalizes the growing (non-empty-array&hasOffset(...)...)|(ArrayAccess&hasOffset(...)...) each time. The equivalent isset($entity['a']) && ... && isset($entity['z']) chain does exactly the same narrowing. Measured locally on 2.3.x it costs about what the new isset() costs (within ~3% under a sampling profiler), so this is the price of narrowing like && and not something specific to IssetHandler.

@ondrejmirtes
ondrejmirtes force-pushed the isset-narrow-each-subject-by-previous branch from 967564f to 33f97a9 Compare November 14, 2026 09:28
isset() evaluates its arguments left to right and stops at the first
one that is not set, so isset($key, $map[$key]) never reads
$map[$key] with $key = null. IssetHandler walked every subject on the
same scope, so rules saw the offset of the second subject as possibly
null - with PHP 8.5 in the analysed version range, "Possibly invalid
array key type string|null".

Each subject after the first is now walked on the scope narrowed by
the previous subject being set, like isset($a) && isset($b). The walk
may stop after any subject, so the resulting scope merges the scopes
after each subject. The turbo-ext mirror gets the same change.

Amp-Thread-ID: https://ampcode.com/threads/T-01a1d722-733e-76bf-aded-6de7e1ddd980
@ondrejmirtes
ondrejmirtes force-pushed the isset-narrow-each-subject-by-previous branch from 33f97a9 to 0c8b6b9 Compare November 14, 2026 09:40
@ondrejmirtes
ondrejmirtes merged commit 7ef85f5 into 2.3.x Nov 14, 2026
101 of 106 checks passed
@ondrejmirtes
ondrejmirtes deleted the isset-narrow-each-subject-by-previous branch November 14, 2026 09:41
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.

isset($key, $map[$key]) reports "Possibly invalid array key type string|null" for PHP 8.5

1 participant