From 599a5133cd113822e8886b7404608b8516c55fc9 Mon Sep 17 00:00:00 2001
From: Tomas Votruba <tomas.vot@gmail.com>
Date: Mon, 10 Jun 2019 01:47:23 +0200
Subject: [PATCH] resolve foreach scope for
 AddDefaultValueForUndefinedVariableRector

---
 ...DefaultValueForUndefinedVariableRector.php | 50 +++++++++++++++++--
 ...ultValueForUndefinedVariableRectorTest.php |  1 +
 .../Fixture/skip_foreach_assign.php.inc       | 18 +++++++
 3 files changed, 65 insertions(+), 4 deletions(-)
 create mode 100644 packages/Php/tests/Rector/FunctionLike/AddDefaultValueForUndefinedVariableRector/Fixture/skip_foreach_assign.php.inc

diff --git a/packages/Php/src/Rector/FunctionLike/AddDefaultValueForUndefinedVariableRector.php b/packages/Php/src/Rector/FunctionLike/AddDefaultValueForUndefinedVariableRector.php
index 6ea5baf069ca..b63f6ec00de2 100644
--- a/packages/Php/src/Rector/FunctionLike/AddDefaultValueForUndefinedVariableRector.php
+++ b/packages/Php/src/Rector/FunctionLike/AddDefaultValueForUndefinedVariableRector.php
@@ -10,6 +10,7 @@
 use PhpParser\Node\FunctionLike;
 use PhpParser\Node\Stmt\ClassMethod;
 use PhpParser\Node\Stmt\Expression;
+use PhpParser\Node\Stmt\Foreach_;
 use PhpParser\Node\Stmt\Function_;
 use PhpParser\Node\Stmt\Static_;
 use PhpParser\Node\Stmt\StaticVar;
@@ -32,6 +33,11 @@ final class AddDefaultValueForUndefinedVariableRector extends AbstractRector
      */
     private $undefinedVariables = [];
 
+    /**
+     * @var string[]
+     */
+    private $definedVariables = [];
+
     public function getDefinition(): RectorDefinition
     {
         return new RectorDefinition('Adds default value for undefined variable', [
@@ -79,6 +85,7 @@ public function getNodeTypes(): array
      */
     public function refactor(Node $node): ?Node
     {
+        $this->definedVariables = [];
         $this->undefinedVariables = [];
 
         $this->traverseNodesWithCallable((array) $node->stmts, function (Node $node): ?int {
@@ -87,6 +94,8 @@ public function refactor(Node $node): ?Node
                 return NodeTraverser::STOP_TRAVERSAL;
             }
 
+            $this->collectDefinedVariablesFromForeach($node);
+
             if (! $node instanceof Variable) {
                 return null;
             }
@@ -95,10 +104,8 @@ public function refactor(Node $node): ?Node
                 return null;
             }
 
+            /** @var string $variableName */
             $variableName = $this->getName($node);
-            if ($variableName === null) {
-                return null;
-            }
 
             // defined 100 %
             /** @var Scope $nodeScope */
@@ -120,6 +127,10 @@ public function refactor(Node $node): ?Node
 
         $variablesInitiation = [];
         foreach ($this->undefinedVariables as $undefinedVariable) {
+            if (in_array($undefinedVariable, $this->definedVariables, true)) {
+                continue;
+            }
+
             $variablesInitiation[] = new Expression(new Assign(new Variable($undefinedVariable), $this->createNull()));
         }
 
@@ -165,6 +176,37 @@ private function shouldSkipVariable(Variable $variable): bool
         /** @var Scope|null $nodeScope */
         $nodeScope = $variable->getAttribute(AttributeKey::SCOPE);
 
-        return $nodeScope === null;
+        if ($nodeScope === null) {
+            return true;
+        }
+
+        $variableName = $this->getName($variable);
+        if ($variableName === null) {
+            return true;
+        }
+
+        return false;
+    }
+
+    private function collectDefinedVariablesFromForeach(Node $node): void
+    {
+        if (! $node instanceof Foreach_) {
+            return;
+        }
+
+        $this->traverseNodesWithCallable($node->stmts, function (Node $node): void {
+            if ($node instanceof Assign || $node instanceof Node\Expr\AssignRef) {
+                if (! $node->var instanceof Variable) {
+                    return;
+                }
+
+                $variableName = $this->getName($node->var);
+                if ($variableName === null) {
+                    return;
+                }
+
+                $this->definedVariables[] = $variableName;
+            }
+        });
     }
 }
diff --git a/packages/Php/tests/Rector/FunctionLike/AddDefaultValueForUndefinedVariableRector/AddDefaultValueForUndefinedVariableRectorTest.php b/packages/Php/tests/Rector/FunctionLike/AddDefaultValueForUndefinedVariableRector/AddDefaultValueForUndefinedVariableRectorTest.php
index 0113847d8919..f9d502c87d23 100644
--- a/packages/Php/tests/Rector/FunctionLike/AddDefaultValueForUndefinedVariableRector/AddDefaultValueForUndefinedVariableRectorTest.php
+++ b/packages/Php/tests/Rector/FunctionLike/AddDefaultValueForUndefinedVariableRector/AddDefaultValueForUndefinedVariableRectorTest.php
@@ -20,6 +20,7 @@ public function test(): void
             __DIR__ . '/Fixture/take_static_into_account.php.inc',
             // @see https://3v4l.org/ObtMn
             __DIR__ . '/Fixture/skip_list.php.inc',
+            __DIR__ . '/Fixture/skip_foreach_assign.php.inc',
         ]);
     }
 
diff --git a/packages/Php/tests/Rector/FunctionLike/AddDefaultValueForUndefinedVariableRector/Fixture/skip_foreach_assign.php.inc b/packages/Php/tests/Rector/FunctionLike/AddDefaultValueForUndefinedVariableRector/Fixture/skip_foreach_assign.php.inc
new file mode 100644
index 000000000000..e9bba8abe5a6
--- /dev/null
+++ b/packages/Php/tests/Rector/FunctionLike/AddDefaultValueForUndefinedVariableRector/Fixture/skip_foreach_assign.php.inc
@@ -0,0 +1,18 @@
+<?php
+
+namespace Rector\Php\Tests\Rector\FunctionLike\AddDefaultValueForUndefinedVariableRector\Fixture;
+
+class SkipForeachAssign
+{
+    public function run(array $nodes)
+    {
+        $abstracts = array();
+
+        // Abstract numbering definition
+        if (true) {
+            foreach ($nodes as $node) {
+                $abstract = &$abstracts[$node];
+            }
+        }
+    }
+}