From 0a90013c027048695d3d72a4aadbd221a0f3ae83 Mon Sep 17 00:00:00 2001 From: phpstan-bot <79867460+phpstan-bot@users.noreply.github.com> Date: Sun, 2 Aug 2026 16:40:50 +0000 Subject: [PATCH] Rebuild `GatheringNodeCallback` layers in `VirtualAssignNodeCallback` instead of wrapping them * `VirtualAssignNodeCallback` used to wrap the whole callback chain, which hid any `GatheringNodeCallback` from `FiberNodeScopeResolver::callNodeCallback()`. The gatherer then ran inside the fiber and could be parked past the point where its result is read. * Added `VirtualAssignNodeCallback::create()`, which recursively rebuilds the chain so gathering layers stay on the outside and keep running at the emission position; `NodeScopeResolver::processVirtualAssign()` now uses it. * This restores `hasAssign` on `NoopExpressionNode` for every construct routed through `processVirtualAssign()` - `$this->prop++`/`--`, `++$this->prop`/`--`, `$this->arr['k']++`, by-ref function-call writes, unset-offset and foreach virtual assigns - not just the reported `match` arm case. * Analogous false positives fixed by the same change: `Unused result of "&&"/"||" operator` when the operand only increments a property, `Expression "[$this->a++]" ... does not do anything`, and nested `match` arms. * Probed and found already correct: `LogicalAnd`/`LogicalOr`/`LogicalXor` and ternary statements (reported before the `hasAssign` check on purpose), plain `=`/`+=` property assignments and list destructuring (they emit through the unwrapped callback), and `NoopNodeCallback` (discards every node, so it has nothing to gather). --- src/Analyser/NodeScopeResolver.php | 2 +- src/Analyser/VirtualAssignNodeCallback.php | 23 ++++- .../Analyser/AnalyserIntegrationTest.php | 7 ++ tests/PHPStan/Analyser/data/bug-15038.php | 84 +++++++++++++++++++ 4 files changed, 114 insertions(+), 2 deletions(-) create mode 100644 tests/PHPStan/Analyser/data/bug-15038.php diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index aefe7850be2..90fa162f056 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -4364,7 +4364,7 @@ private function getParameterOutExtensionsType(CallLike $callLike, $calleeReflec public function processVirtualAssign(MutatingScope $scope, ExpressionResultStorage $storage, Node\Stmt $stmt, Expr $var, Expr $assignedExpr, callable $nodeCallback): ExpressionResult { $assignHandler = $this->container->getByType(AssignHandler::class); - $virtualAssignNodeCallback = new VirtualAssignNodeCallback($nodeCallback); + $virtualAssignNodeCallback = VirtualAssignNodeCallback::create($nodeCallback); $target = $assignHandler->prepareTarget( $this, $scope, diff --git a/src/Analyser/VirtualAssignNodeCallback.php b/src/Analyser/VirtualAssignNodeCallback.php index 13d4cff9101..c4c4bdf8164 100644 --- a/src/Analyser/VirtualAssignNodeCallback.php +++ b/src/Analyser/VirtualAssignNodeCallback.php @@ -12,10 +12,31 @@ final class VirtualAssignNodeCallback implements ShallowNodeCallback /** * @param callable(Node $node, Scope $scope): void $originalNodeCallback */ - public function __construct(private mixed $originalNodeCallback) + private function __construct(private mixed $originalNodeCallback) { } + /** + * Rebuilds the chain instead of wrapping it so that GatheringNodeCallback + * layers stay on the outside. Hiding a gatherer behind this filter would let + * FiberNodeScopeResolver defer it into a fiber, and a parked fiber can run + * the gatherer long after the caller already read its result. + * + * @param callable(Node $node, Scope $scope): void $nodeCallback + * @return callable(Node $node, Scope $scope): void + */ + public static function create(callable $nodeCallback): callable + { + if ($nodeCallback instanceof GatheringNodeCallback) { + return new GatheringNodeCallback( + self::create($nodeCallback->getGatherer()), + self::create($nodeCallback->getInner()), + ); + } + + return new self($nodeCallback); + } + public function __invoke(Node $node, Scope $scope): void { if (!$node instanceof PropertyAssignNode && !$node instanceof VariableAssignNode) { diff --git a/tests/PHPStan/Analyser/AnalyserIntegrationTest.php b/tests/PHPStan/Analyser/AnalyserIntegrationTest.php index 18218040b39..4c0601d1280 100644 --- a/tests/PHPStan/Analyser/AnalyserIntegrationTest.php +++ b/tests/PHPStan/Analyser/AnalyserIntegrationTest.php @@ -1616,6 +1616,13 @@ public function testBug15003(): void $this->assertNoErrors($errors); } + #[RequiresPhp('>= 8.4.0')] + public function testBug15038(): void + { + $errors = $this->runAnalyse(__DIR__ . '/data/bug-15038.php'); + $this->assertNoErrors($errors); + } + /** * @param string[]|null $allAnalysedFiles * @return list diff --git a/tests/PHPStan/Analyser/data/bug-15038.php b/tests/PHPStan/Analyser/data/bug-15038.php new file mode 100644 index 00000000000..3a7c78df5be --- /dev/null +++ b/tests/PHPStan/Analyser/data/bug-15038.php @@ -0,0 +1,84 @@ += 8.4 + +namespace Bug15038; + +enum Counter { + case A; + case B; +} + +class Statistics { + public private(set) int $counterA = 0; + public private(set) int $counterB = 0; + + public function inc(Counter $counter): void { + match ($counter) { + Counter::A => $this->counterA++, + Counter::B => $this->counterB++, + }; + } +} + +$s = new Statistics(); +$s->inc(Counter::A); +$s->inc(Counter::A); +$s->inc(Counter::A); + +var_dump($s->counterA); + +class MoreVirtualAssigns +{ + + public int $a = 0; + + public int $b = 0; + + /** @var array{a: int, b: int} */ + public array $arr = ['a' => 0, 'b' => 0]; + + public function postDec(Counter $counter): void + { + match ($counter) { + Counter::A => $this->a--, + Counter::B => $this->b--, + }; + } + + public function preIncDec(Counter $counter): void + { + match ($counter) { + Counter::A => ++$this->a, + Counter::B => --$this->b, + }; + } + + public function offset(Counter $counter): void + { + match ($counter) { + Counter::A => $this->arr['a']++, + Counter::B => $this->arr['b']--, + }; + } + + public function nestedMatch(Counter $counter): void + { + match ($counter) { + Counter::A => match (true) { + default => $this->a++, + }, + Counter::B => $this->b++, + }; + } + + public function booleanOperators(bool $cond): void + { + $cond && ($this->a++ > 0); + $cond || ($this->b-- > 0); + } + + public function arrayLiteral(): void + { + [$this->a++]; + } + +}