From 898ab8853a90f7f301646f8f72b2c00c778f4829 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Mon, 29 Jun 2026 17:50:14 +0200 Subject: [PATCH 1/2] [CodeQuality] Skip NarrowUnusedSetUpDefinedPropertyRector for property used in nested closure The rule turned a setUp() property into a local variable while rewriting every $this->property reference inside setUp(), including references inside nested closures (e.g. resolver callbacks stored for later execution). A closure does not capture the local variable (no 'use' binding is added), so at call time the variable was undefined and the closure silently returned null. Skip narrowing when the property is referenced inside a Closure or ArrowFunction within setUp(). --- .../skip_property_used_in_closure.php.inc | 25 ++++++++++++ ...NarrowUnusedSetUpDefinedPropertyRector.php | 38 +++++++++++++++++++ 2 files changed, 63 insertions(+) create mode 100644 rules-tests/CodeQuality/Rector/Class_/NarrowUnusedSetUpDefinedPropertyRector/Fixture/skip_property_used_in_closure.php.inc diff --git a/rules-tests/CodeQuality/Rector/Class_/NarrowUnusedSetUpDefinedPropertyRector/Fixture/skip_property_used_in_closure.php.inc b/rules-tests/CodeQuality/Rector/Class_/NarrowUnusedSetUpDefinedPropertyRector/Fixture/skip_property_used_in_closure.php.inc new file mode 100644 index 000000000..a9a4865f1 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Class_/NarrowUnusedSetUpDefinedPropertyRector/Fixture/skip_property_used_in_closure.php.inc @@ -0,0 +1,25 @@ +dataSource = [1, 2, 3]; + + $this->resolver = function () { + return $this->dataSource; + }; + } + + public function test() + { + $resolver = $this->resolver; + $this->assertSame([1, 2, 3], $resolver()); + } +} diff --git a/rules/CodeQuality/Rector/Class_/NarrowUnusedSetUpDefinedPropertyRector.php b/rules/CodeQuality/Rector/Class_/NarrowUnusedSetUpDefinedPropertyRector.php index 423da88af..265748c9e 100644 --- a/rules/CodeQuality/Rector/Class_/NarrowUnusedSetUpDefinedPropertyRector.php +++ b/rules/CodeQuality/Rector/Class_/NarrowUnusedSetUpDefinedPropertyRector.php @@ -6,6 +6,8 @@ use PhpParser\Node; use PhpParser\Node\Expr; +use PhpParser\Node\Expr\ArrowFunction; +use PhpParser\Node\Expr\Closure; use PhpParser\Node\Expr\PropertyFetch; use PhpParser\Node\Expr\Variable; use PhpParser\Node\Stmt\Class_; @@ -121,6 +123,12 @@ public function refactor(Node $node): ?Node continue; } + // referenced inside a nested closure that may run after setUp() - turning it into a local + // variable would leave it out of the closure scope, as there is no "use" binding to add it + if ($this->isPropertyUsedInNestedClosure($setUpClassMethod, $propertyName)) { + continue; + } + $hasChanged = true; unset($node->stmts[$key]); @@ -189,6 +197,36 @@ private function isPropertyUsedOutsideSetUpClassMethod( return $isPropertyUsed; } + private function isPropertyUsedInNestedClosure(ClassMethod $setUpClassMethod, string $propertyName): bool + { + $closures = $this->nodeFinder->find( + $setUpClassMethod, + static fn (Node $node): bool => $node instanceof Closure || $node instanceof ArrowFunction + ); + + foreach ($closures as $closure) { + $usedPropertyFetch = $this->nodeFinder->findFirst($closure, function (Node $node) use ( + $propertyName + ): bool { + if (! $node instanceof PropertyFetch) { + return false; + } + + if (! $this->isName($node->var, 'this')) { + return false; + } + + return $this->isName($node->name, $propertyName); + }); + + if ($usedPropertyFetch instanceof PropertyFetch) { + return true; + } + } + + return false; + } + private function shouldSkipProperty( bool $isFinalClass, Property $property, From 6d1f7a3f98df6570b8a15ea03bcead896c7bde37 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Mon, 29 Jun 2026 17:58:48 +0200 Subject: [PATCH 2/2] fixup! [CodeQuality] Skip NarrowUnusedSetUpDefinedPropertyRector for property used in nested closure --- .../Class_/NarrowUnusedSetUpDefinedPropertyRector.php | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/rules/CodeQuality/Rector/Class_/NarrowUnusedSetUpDefinedPropertyRector.php b/rules/CodeQuality/Rector/Class_/NarrowUnusedSetUpDefinedPropertyRector.php index 265748c9e..d077cfba5 100644 --- a/rules/CodeQuality/Rector/Class_/NarrowUnusedSetUpDefinedPropertyRector.php +++ b/rules/CodeQuality/Rector/Class_/NarrowUnusedSetUpDefinedPropertyRector.php @@ -6,7 +6,6 @@ use PhpParser\Node; use PhpParser\Node\Expr; -use PhpParser\Node\Expr\ArrowFunction; use PhpParser\Node\Expr\Closure; use PhpParser\Node\Expr\PropertyFetch; use PhpParser\Node\Expr\Variable; @@ -199,10 +198,9 @@ private function isPropertyUsedOutsideSetUpClassMethod( private function isPropertyUsedInNestedClosure(ClassMethod $setUpClassMethod, string $propertyName): bool { - $closures = $this->nodeFinder->find( - $setUpClassMethod, - static fn (Node $node): bool => $node instanceof Closure || $node instanceof ArrowFunction - ); + // only regular closures are unsafe: they do not capture outer variables without an + // explicit "use" binding. Arrow functions auto-capture by value, so narrowing is safe there. + $closures = $this->nodeFinder->findInstanceOf($setUpClassMethod, Closure::class); foreach ($closures as $closure) { $usedPropertyFetch = $this->nodeFinder->findFirst($closure, function (Node $node) use (