fix(DefinitionResolver): find variables in sibling children (#568)

Fixes #566

fixtures/completion/foreach.php

@@ -38,3 +38,9 @@ foreach ($bar->test() as $value) {
 
 foreach ($unknownArray as $member->access => $unknown) {
     $unkno
+
+foreach ($loop as $loop) {
+}
+ 
+foreach ($loop->getArray() as $loop) {
+}

src/DefinitionResolver.php

@@ -545,6 +545,15 @@ class DefinitionResolver
         } else {
             throw new \InvalidArgumentException('$var must be Variable, Param or ClosureUse, not ' . get_class($var));
         }
+        if (empty($name)) {
+            return null;
+        }
+
+        $shouldDescend = function ($nodeToDescand) {
+            // Make sure not to decend into functions or classes (they represent a scope boundary)
+            return !($nodeToDescand instanceof PhpParser\FunctionLike || $nodeToDescand instanceof PhpParser\ClassLike);
+        };
+
         // Traverse the AST up
         do {
             // If a function is met, check the parameters and use statements
@@ -569,39 +578,55 @@ class DefinitionResolver
                 break;
             }
 
-            // If we get to a ForeachStatement, check the keys and values
+            // Check each previous sibling node and their descendents for a variable assignment to that variable
-            if ($n instanceof Node\Statement\ForeachStatement) {
+            // Each previous sibling could contain a declaration of the variable
-                if ($n->foreachKey instanceof Node\Expression\Variable
-                    && $n->foreachKey->expression->getName() === $name
-                ) {
-                    return $n->foreachKey;
-                }
-                if ($n->foreachValue
-                    && $n->foreachValue->expression instanceof Node\Expression\Variable
-                    && $n->foreachValue->expression->getName() === $name
-                ) {
-                    return $n->foreachValue;
-                }
-            }
-
-            // Check each previous sibling node for a variable assignment to that variable
             while (($prevSibling = $n->getPreviousSibling()) !== null && $n = $prevSibling) {
-                if ($n instanceof Node\Statement\ExpressionStatement) {
+
-                    $n = $n->expression;
+                // Check the sibling itself
-                }
+                if (self::isVariableDeclaration($n, $name)) {
-                if (
-                    // TODO - clean this up
-                    ($n instanceof Node\Expression\AssignmentExpression && $n->operator->kind === PhpParser\TokenKind::EqualsToken)
-                    && $n->leftOperand instanceof Node\Expression\Variable && $n->leftOperand->getName() === $name
-                ) {
                     return $n;
                 }
+
+                // Check descendant of this sibling (e.g. the children of a previous if block)
+                foreach ($n->getDescendantNodes($shouldDescend) as $descendant) {
+                    if (self::isVariableDeclaration($descendant, $name)) {
+                        return $descendant;
+                    }
+                }
             }
         } while (isset($n) && $n = $n->parent);
         // Return null if nothing was found
         return null;
     }
 
+    /**
+     * Checks whether the given Node declares the given variable name
+     *
+     * @param Node $n The Node to check
+     * @param string $name The name of the wanted variable
+     * @return bool
+     */
+    private static function isVariableDeclaration(Node $n, string $name)
+    {
+        if (
+            // TODO - clean this up
+            ($n instanceof Node\Expression\AssignmentExpression && $n->operator->kind === PhpParser\TokenKind::EqualsToken)
+            && $n->leftOperand instanceof Node\Expression\Variable && $n->leftOperand->getName() === $name
+        ) {
+            return true;
+        }
+
+        if (
+            ($n instanceof Node\ForeachValue || $n instanceof Node\ForeachKey)
+            && $n->expression instanceof Node\Expression\Variable
+            && $n->expression->getName() === $name
+        ) {
+            return true;
+        }
+
+        return false;
+    }
+
     /**
      * Given an expression node, resolves that expression recursively to a type.
      * If the type could not be resolved, returns Types\Mixed_.