Skip to content

Model TypeError throw points for illegal array key offsets - #5977

Open
gnutix wants to merge 2 commits into
phpstan:2.3.xfrom
gnutix:typeerror-array-key-throw-points
Open

gnutix wants to merge 2 commits into
phpstan:2.3.xfrom
gnutix:typeerror-array-key-throw-points

Conversation

@gnutix

@gnutix gnutix commented Jul 2, 2026 •

Copy link
Copy Markdown
Contributor

Closes phpstan/phpstan#6970

Root cause

Since PHP 8.0, using an array or an object as an array (or string) offset throws TypeError instead of emitting an "Illegal offset type" warning (Reclassifying engine warnings RFC — the "Illegal offset type" / "Illegal offset type in isset or empty" / "Illegal offset type in unset" diagnostics). PHPStan did not model this anywhere: the ArrayDimFetch branch of the assignment handling only collected throw points from evaluating the root variable, the dimension expressions, the assigned expression, and a synthetic offsetSet() call for ArrayAccess containers. Nothing represented the engine's own TypeError for an illegal key, so CatchWithUnthrownExceptionRule reported Dead catch - TypeError is never thrown in the try block on code that can genuinely throw — while InvalidKeyInArrayDimFetchRule was simultaneously warning about Possibly invalid array key type mixed for the same expression.

The same gap existed for reads ($arr[$key], including under ??, isset(), empty()), for unset($arr[$key]), for string offset access and for array literal keys ([$key => $value]) — it is one coherent hole (the engine throws from the same offset-legality check in all of these), so this PR covers all of them together rather than splitting.

The fix

  • New PhpVersion::throwsTypeErrorForIllegalOffsets() (>= 80000) gates everything: on PHP 7.x these operations only raised a warning, so the dead-catch report was correct there and must stay.
  • New IllegalOffsetTypeHelper::mayOffsetThrowTypeError(): an offset may throw only if array|object is not definitively ruled out for it — float, bool and null keys are coerced with at most a deprecation, resource keys with a warning, so those never throw.
  • Write side (AssignHandler): prepareTarget() collects every link of the chain whose offset may be illegal (PreparedAssignTarget::getIllegalOffsetDimFetches()), and applyWrite() creates an InternalThrowPoint::createExplicit(..., TypeError, ...) for each of them after the assigned value was evaluated, with the root variable defined as an array when it was undefined before. That is the engine's order: the right-hand side is evaluated first, then the container is fetched for writing — which turns an undefined/null root into [] — and only then is the offset rejected (see the table below). So after a caught TypeError, variables assigned in the right-hand side and the auto-vivified container are definitely defined. Skipped when the offset expression is null ($arr[] = never throws for the offset) and when the container is definitely ArrayAccess (the write goes through offsetSet(), whose throw points are already modelled by the existing synthetic call — including built-ins like ArrayIterator which throw from inside offsetSet()). Compound assignments ($arr[$key] .= …) flow through the same path and are covered automatically, as are list destructuring targets and string offset writes ($str[$key] = 'x'). For $arr[$key] ??= … on a container that is definitely an array or a string, the throw point stays before the right-hand side: the isset()-style read throws first and the right-hand side is never evaluated. The variable flow of an assignment or a compound assignment records the throw, so UnusedVariableRule knows a catch block can observe the values assigned before the statement.
  • Read side (ArrayDimFetchHandler::composeResult()): same throw point when the container may be an array or a string and the offset may be illegal. Reads on null/other scalars only emit a warning and evaluate to null (verified below), so containers that are definitely neither array nor string are skipped. Reads under ?? and unset() are covered automatically because those handlers process the inner ArrayDimFetch through the same code path — and PHP does throw there (null coalescing suppresses undefined-key notices, not illegal-offset TypeErrors).
  • isset() / empty(): on an array container they throw ("in isset or empty"), but on a string container they just answer false / true. IssetHandler and EmptyHandler process their subject with a new ExpressionContext::enterIssetTarget(), and the read side skips the throw point for a string-only container there. Only the outermost link is affected — isset($str[$obj][0]) reads $str[$obj] first, which throws.
  • Array literals (ArrayHandler): [$key => $value] throws once the item's value was evaluated when the key may be an array or an object; the throw point and its variable flow are added there, gated the same way.

This mirrors the fix for the identical bug shape with typed property assignment: phpstan/phpstan#9146, fixed in #4981 (commit 88b41d4), which added InternalThrowPoint::createExplicit($scope, new ObjectType(TypeError::class), $assignedExpr, false) in the PropertyFetch branch of the same method — also after the assigned value. The new rule test is modeled on testBug9146.

Runtime semantics verification (php -r, PHP 8.2, 8.3, 8.4 and 8.5 — the messages read "Illegal offset type" before 8.3, the behaviour is the same)

operation result
$array[$object] = 'x' / $array[[1]] = 'x' TypeError
$array[true] = 'x' / $array[1.5] = 'x' / $array[null] = 'x' no throw (coerced)
$array[$resource] = 'x' no throw (warning, cast to int)
$array[dim()] = $x = rhs() dim(), then rhs(), then TypeError — $x is defined
$undefined[$object] = 1 / [$undefined[$object]] = [1] / $undefined[$object] .= 1 TypeError, $undefined === [] afterwards
$undefined[1][$object] = 1 TypeError, $undefined === [1 => []] afterwards
$array[$object] ??= $x = 1 TypeError from the read, $x not assigned
$undefined[$object] ??= $x = 1 $x assigned, then TypeError from the write, $undefined === []
$x = $array[$object] TypeError
$array[$object] ?? 'default' / $string[$object] ?? 'default' TypeError
isset($array[$object]) / empty($array[$object]) TypeError ("in isset or empty")
isset($string[$object]) / empty($string[$object]) no throw (false / true)
isset($string[$object][0]) TypeError (the inner read)
unset($array[$object]) TypeError ("Cannot unset offset")
$string[$object] = 'x' / $x = $string[$object] TypeError
[$object => 1] / [$array => 1] TypeError
$null[$object] = 1 / $false[$object] = 1 TypeError (after auto-vivification)
$x = $null[$object] / $x = $int[$object] no throw (warning, null)
isset($null[$object]) / $undefined[$object] ?? 'd' no throw (false / 'd')
$int[$object] = 1 Error "Cannot use a scalar value as an array" (not TypeError)
$arrayIterator[$object] = 1 / unset($arrayIterator[$object]) TypeError from inside offsetSet()/offsetUnset() (covered by the existing synthetic method call modelling)

On PHP 7.4 all of the array-offset cases above emit Warning: Illegal offset type (or the isset/unset variants) instead of throwing — hence the version gate. The boundary is PHP 8.0.0 per the engine warnings RFC and the PHP 8.0 UPGRADING notes.

Turbo

The changed classes are shadowed by the turbo extension, so the native mirrors change in the same commit:

  • ArrayDimFetchHandler.cpp (read side, isset()/empty() refinement, the PhpVersion constructor parameter), AssignHandler.cpp (collecting the links in prepareTarget(), the throw points in applyWrite(), the write's variable flow), AssignOpHandler.cpp (the compound assignment's variable flow), ArrayHandler.cpp (array literal keys, the PhpVersion constructor parameter), IssetHandler.cpp / EmptyHandler.cpp (the isset subject context).
  • IllegalOffsetTypeHelper.cpp (new), ExpressionContext.cpp (enterIssetTarget() / isIssetTarget()), PreparedAssignTarget.cpp (the new slot and getter), the throwsTypeErrorForIllegalOffsets entry in PhpVersionAccess.cpp, and the regenerated declarations.

Verified with the strict build, smoke.php, side-by-side.php, signature-parity.php, walk-trace.php over the default corpus, identical --error-format=raw output with the extension loaded and not loaded, and the full test suite in both modes. The follow-up commit bumps TurboExtensionEnabler::EXPECTED_EXTENSION_VERSION.

Test expectation updates

  • tests/PHPStan/Analyser/nsrt/throw-points/array-dim-fetch.php ([][doesntThrow()];) and array.php ([doesntThrow() => 1, $foo = 1];) move to the version-split illegal-array-offset-php7.php / illegal-array-offset-php8.php: the helper doesntThrow() returns mixed, so on PHP 8 the read and the literal key can throw before $foo is assigned (Maybe), on PHP 7 they cannot (Yes).
  • The write cases from assign.php / assign-op.php ($foo[doesntThrow()] = 0;, [$foo[doesntThrow()]] = 1;, $foo[doesntThrow()] .= 0;) moved to the same split and stay Yes on both: the write throws only after $foo was turned into an array.
  • tests/PHPStan/Levels/data/arrayOffsetAccess-4.json: the "Expression "$a[$objectOrInt]" on a separate line does not do anything." family disappears for every offset that may be an array or an object (16 entries). A statement whose expression has an explicit throw point is not a no-op — ExpressionHandler emits NoopExpressionNode only when there are no impure points and no explicit throw points — and $a[$mixed]; can now throw. Offsets that are provably legal ($a[42], …) are still reported.

New tests: testBug6970 in CatchWithUnthrownExceptionRuleTest (the original issue reproducer plus minimal cases for each context — write, compound assign, read, ??, isset, empty, unset, string offset read/write, array literals — with int-keyed and $arr[] = counterparts asserting the dead catch is still reported when the offset is provably legal, and the string-container isset()/empty() cases asserting it is reported there too); variable certainty cases in illegal-array-offset-php8.php for the evaluation order (right-hand side, auto-vivified root, .=, ??=, array literals, isset()/empty() of string offsets); testIllegalOffsetThrowReachesCatch in UnusedVariableRuleTest; and IllegalOffsetTypeHelperTest.

Ecosystem impact

The integration test against shopware/shopware reports one new error, a true positive: tests/integration/Core/Content/Seo/SeoUrlGeneratorTest.php declares @throws void on a logger's log(mixed $level, string|\Stringable $message, …) that does $this->logs[$level][$message][] = $context; — a Stringable key throws TypeError (Cannot access offset of type Stringable@anonymous on array). The shopware baseline in phpstan/phpstan's integration tests needs that entry.

Companion baseline update: phpstan/phpstan#15470 (merge together with this PR).

Alternatives / follow-ups

  • A non-numeric string offset on a string ($string['foo'], read or write) also throws TypeError ("Cannot access offset of type string on string") on PHP 8, while a leading-numeric one ('1x') only warns. That is a different trigger — the container being a string and the key a non-numeric string — so it is left for a follow-up.
  • A simpler variant records the write-side throw point before the right-hand side (where the dimensions are evaluated). It needs neither the new PreparedAssignTarget field nor the vivified-root scope, but after a caught TypeError it leaves variables assigned in the right-hand side and an auto-vivified container maybe-undefined, which turns into "Variable might not be defined" false positives.

🤖 Generated with Claude Code

https://claude.ai/code/session_01QW8VaDtn3cMdQowPSh1xGG

@gnutix
gnutix changed the base branch from 2.1.x to 2.2.x July 4, 2026 19:47
@gnutix
gnutix force-pushed the typeerror-array-key-throw-points branch 3 times, most recently from b0e6316 to 5f483b5 Compare July 5, 2026 07:49
@gnutix
gnutix force-pushed the typeerror-array-key-throw-points branch 3 times, most recently from ab4e62d to 897a3d3 Compare July 5, 2026 11:39
@gnutix
gnutix force-pushed the typeerror-array-key-throw-points branch from 897a3d3 to c19e5b3 Compare October 12, 2026 16:32
@gnutix
gnutix changed the base branch from 2.2.x to 2.3.x October 12, 2026 16:32
gnutix and others added 2 commits October 12, 2026 21:19
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QW8VaDtn3cMdQowPSh1xGG
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QW8VaDtn3cMdQowPSh1xGG
@gnutix
gnutix force-pushed the typeerror-array-key-throw-points branch from c19e5b3 to 8d336a5 Compare October 12, 2026 19:27
gnutix added a commit to gnutix/phpstan that referenced this pull request Oct 12, 2026
phpstan/phpstan-src#5977 models the TypeError PHP 8 throws for an
illegal array key. Shopware's SeoUrlGeneratorTest logger declares
@throws void but writes $this->logs[$level][$message][] with a
string|Stringable key, and a Stringable key throws. A true positive.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QW8VaDtn3cMdQowPSh1xGG
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.

Accessing an array's value using a Generator's key (which can be another type than "array-key") could potentially trigger a TypeError

1 participant