[CodeQuality] Skip NarrowUnusedSetUpDefinedPropertyRector for property used in nested closure (#693)

* [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().

* fixup! [CodeQuality] Skip NarrowUnusedSetUpDefinedPropertyRector for property used in nested closure

Author: TomasVotruba

rules-tests/CodeQuality/Rector/Class_/NarrowUnusedSetUpDefinedPropertyRector/Fixture/skip_property_used_in_closure.php.inc

@@ -0,0 +1,25 @@
+<?php
+
+namespace Rector\PHPUnit\Tests\CodeQuality\Rector\Class_\NarrowUnusedSetUpDefinedPropertyRector\Fixture;
+
+use PHPUnit\Framework\TestCase;
+
+final class SkipPropertyUsedInClosure extends TestCase
+{
+    private array $dataSource;
+
+    protected function setUp(): void
+    {
+        $this->dataSource = [1, 2, 3];
+
+        $this->resolver = function () {
+            return $this->dataSource;
+        };
+    }
+
+    public function test()
+    {
+        $resolver = $this->resolver;
+        $this->assertSame([1, 2, 3], $resolver());
+    }
+}

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,