From c8d8c11a11e8a85cb74cd59685c5bb21b84735f2 Mon Sep 17 00:00:00 2001 From: phpstan-bot <79867460+phpstan-bot@users.noreply.github.com> Date: Sat, 24 Oct 2026 20:50:38 +0000 Subject: [PATCH] Re-process a stored scope-changing expression at a foreign ask position instead of re-anchoring it - NodeScopeResolver::processExprNode() answered an on-demand walk from a node's stored ExpressionResult re-anchored with atAskPosition(), which sets both the before and the after scope to the asking scope. For a node that changed the scope (assignment, assign-op, ++/--, list destructuring, ...) this dropped its effect: when ArrayMapFunctionReturnTypeExtension priced the synthetic `$callback(...)` call, the closure body's `$id = f()` was answered from storage without defining `$id`, so only the `if ($id === null)` branch survived and the callback seemed to return `null` only. - Such a node (stored after-scope !== before-scope) is now re-processed on the asking scope; its pure subnodes still answer from storage. Closures and arrow functions keep the existing answer (their stored result is their converged walk). The consume mode is unchanged. - Mirrored in turbo-ext/src/NodeScopeResolver.cpp (processExprNodeBody()). - Probed `+=`, `??=`, `++`, by-ref out parameter (preg_match), list destructuring, arrow function with an assignment and an array_filter() callback: `+=`, `++` and list destructuring had the same bug; the others already worked and are kept as coverage of the same path. --- src/Analyser/NodeScopeResolver.php | 12 ++ tests/PHPStan/Analyser/nsrt/bug-15491.php | 162 ++++++++++++++++++ .../Rules/Functions/ArrayFilterRuleTest.php | 5 + .../Rules/Functions/data/bug-15491.php | 71 ++++++++ turbo-ext/src/NodeScopeResolver.cpp | 14 ++ 5 files changed, 264 insertions(+) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-15491.php create mode 100644 tests/PHPStan/Rules/Functions/data/bug-15491.php diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index b85f94a6b81..2a607baa993 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -786,6 +786,18 @@ public function processExprNode( return $storedResult; } + // re-anchoring drops what the node did to the scope (an assignment + // inside a closure body an extension prices on demand) - such a + // node is re-processed on the asking scope instead + if ( + !$consume + && $storedResult->getScope() !== $storedResult->getBeforeScope() + && !$expr instanceof Expr\Closure + && !$expr instanceof Expr\ArrowFunction + ) { + return $this->processExprNodeInternal($stmt, $expr, $scope, $storage, $nodeCallback, $context); + } + $reanchored = $storedResult->atAskPosition($scope); if ($consume) { // the re-anchored view IS this walk's result for the node diff --git a/tests/PHPStan/Analyser/nsrt/bug-15491.php b/tests/PHPStan/Analyser/nsrt/bug-15491.php new file mode 100644 index 00000000000..c38a17992bc --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-15491.php @@ -0,0 +1,162 @@ + $data + * @return array + */ +function withCoalesce(array $data): array +{ + $data['ids'] = array_filter( + array_map( + function (string $alias): ?string { + $id = idFromAlias($alias); + if ($id === null) { + echo 'invalid'; + } + return $id; + }, + explode('|', (string) ($data['options'] ?? '')) + ) + ); + assertType('array, non-falsy-string>', $data['ids']); + + return $data; +} + +/** + * @template T + * @param T $x + * @return T + */ +function identity($x) +{ + return $x; +} + +/** + * @param mixed $x + * @return list + */ +function strings($x): array +{ + return []; +} + +/** @param array $data */ +function assignOp(array $data): void +{ + $x = identity(array_map( + function (string $alias): int { + $i = 0; + $i += strlen($alias); + if ($i === 0) { + echo 'empty'; + } + return $i; + }, + strings($data['options'] ?? '') + )); + assertType('list>', $x); +} + +/** @param array $data */ +function coalesceAssign(array $data): void +{ + $x = identity(array_map( + function (string $alias): ?string { + $id = null; + $id ??= idFromAlias($alias); + if ($id === null) { + echo 'invalid'; + } + return $id; + }, + strings($data['options'] ?? '') + )); + assertType('list', $x); +} + +/** @param array $data */ +function increment(array $data): void +{ + $x = identity(array_map( + function (string $alias): int { + $i = 0; + $i++; + if ($i === 0) { + echo 'never'; + } + return $i; + }, + strings($data['options'] ?? '') + )); + assertType('list<1>', $x); +} + +/** @param array $data */ +function byRefOutParameter(array $data): void +{ + $x = identity(array_map( + function (string $alias): ?string { + $m = null; + preg_match('/a/', $alias, $m); + if ($m === null) { + echo 'never'; + } + return $m[0] ?? null; + }, + strings($data['options'] ?? '') + )); + assertType('list', $x); +} + +/** @param array $data */ +function listAssign(array $data): void +{ + $x = identity(array_map( + function (string $alias): ?string { + [$id] = [idFromAlias($alias)]; + if ($id === null) { + echo 'invalid'; + } + return $id; + }, + strings($data['options'] ?? '') + )); + assertType('list', $x); +} + +/** @param array $data */ +function arrowFunction(array $data): void +{ + $x = identity(array_map( + fn (string $alias): ?string => ($id = idFromAlias($alias)) === null ? $id : $id, + strings($data['options'] ?? '') + )); + assertType('list', $x); +} + +/** @param array $data */ +function arrayFilterCallback(array $data): void +{ + $x = identity(array_filter( + strings($data['options'] ?? ''), + function (string $alias): bool { + $id = idFromAlias($alias); + if ($id === null) { + echo 'invalid'; + } + return $id !== null; + } + )); + assertType('array, string>', $x); +} diff --git a/tests/PHPStan/Rules/Functions/ArrayFilterRuleTest.php b/tests/PHPStan/Rules/Functions/ArrayFilterRuleTest.php index 96be9603a85..10f5dbea47b 100644 --- a/tests/PHPStan/Rules/Functions/ArrayFilterRuleTest.php +++ b/tests/PHPStan/Rules/Functions/ArrayFilterRuleTest.php @@ -99,4 +99,9 @@ public function testBug2065WithoutPhpDocTypesAsCertain(): void $this->analyse([__DIR__ . '/data/bug-array-filter.php'], []); } + public function testBug15491(): void + { + $this->analyse([__DIR__ . '/data/bug-15491.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Functions/data/bug-15491.php b/tests/PHPStan/Rules/Functions/data/bug-15491.php new file mode 100644 index 00000000000..5a7818e0a61 --- /dev/null +++ b/tests/PHPStan/Rules/Functions/data/bug-15491.php @@ -0,0 +1,71 @@ + $data + * @return array + */ +function withCoalesce(array $data): array +{ + $data['ids'] = array_filter( + array_map( + function (string $alias): ?string { + $id = idFromAlias($alias); + if ($id === null) { + echo 'invalid'; + } + return $id; + }, + explode('|', (string) ($data['options'] ?? '')) + ) + ); + + return $data; +} + +/** + * @param array $data + * @return array + */ +function withoutCoalesce(array $data): array +{ + $data['ids'] = array_filter( + array_map( + function (string $alias): ?string { + $id = idFromAlias($alias); + if ($id === null) { + echo 'invalid'; + } + return $id; + }, + explode('|', (string) $data['options']) + ) + ); + + return $data; +} + +/** + * @param array $data + * @return array + */ +function withoutIf(array $data): array +{ + $data['ids'] = array_filter( + array_map( + function (string $alias): ?string { + $id = idFromAlias($alias); + return $id; + }, + explode('|', (string) ($data['options'] ?? '')) + ) + ); + + return $data; +} diff --git a/turbo-ext/src/NodeScopeResolver.cpp b/turbo-ext/src/NodeScopeResolver.cpp index b2d0b6b4a62..9847711096d 100644 --- a/turbo-ext/src/NodeScopeResolver.cpp +++ b/turbo-ext/src/NodeScopeResolver.cpp @@ -1963,6 +1963,20 @@ class NodeScopeResolver if (UNEXPECTED(beforeScope.isUndef())) return zv::Val(); if (beforeScope.ref().isObject() && Z_OBJ_P(beforeScope.raw()) == Z_OBJ_P(scope)) return storedResult; + // re-anchoring drops what the node did to the scope - such a + // node is re-processed on the asking scope instead + if (!consume) { + zv::Val afterScope = expressionResultScope(storedResult.raw()); + if (UNEXPECTED(afterScope.isUndef())) return zv::Val(); + bool sameScope = afterScope.ref().isObject() && beforeScope.ref().isObject() && Z_OBJ_P(afterScope.raw()) == Z_OBJ_P(beforeScope.raw()); + if (!sameScope) { + bool isClosure; + if (UNEXPECTED(!isInstance(Z_OBJ_P(expr), PT_CLASS_CLOSURE_EXPR, isClosure))) return zv::Val(); + if (!isClosure && UNEXPECTED(!isInstance(Z_OBJ_P(expr), PT_CLASS_ARROW_FUNCTION, isClosure))) return zv::Val(); + if (!isClosure) return processExprNodeInternal(stmt, expr, scope, storage, nodeCallback, context); + } + } + zv::Val reanchored = pt_expression_result_at_ask_position(storedResult.raw(), scope); if (UNEXPECTED(reanchored.isUndef())) return zv::Val(); if (consume && UNEXPECTED(!thisStoreExpressionResult(storage, expr, reanchored.raw()))) return zv::Val();