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..d077cfba5 100644 --- a/rules/CodeQuality/Rector/Class_/NarrowUnusedSetUpDefinedPropertyRector.php +++ b/rules/CodeQuality/Rector/Class_/NarrowUnusedSetUpDefinedPropertyRector.php @@ -6,6 +6,7 @@ use PhpParser\Node; use PhpParser\Node\Expr; +use PhpParser\Node\Expr\Closure; use PhpParser\Node\Expr\PropertyFetch; use PhpParser\Node\Expr\Variable; use PhpParser\Node\Stmt\Class_; @@ -121,6 +122,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 +196,35 @@ private function isPropertyUsedOutsideSetUpClassMethod( return $isPropertyUsed; } + private function isPropertyUsedInNestedClosure(ClassMethod $setUpClassMethod, string $propertyName): bool + { + // 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 ( + $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,