Repository: phpactor/phpactor (php)

Issue:
Cannot infer `foreach` item type from docblock 
@dantleech thanks for hint from #1687 - more trivial example can be used here.

Given https://github.com/phpactor/phpactor/blob/master/lib/WorseReflection/Tests/Inference/foreach/assigns_type_to_item.test remove the only real assignment:
```php
<?php

/** @var int[] $items */

foreach ($items as $item) {
    wrAssertType("int", $item);
}
```
and run `./vendor/bin/phpunit  lib/WorseReflection/Tests/Inference/SelfTest.php`. https://github.com/phpactor/phpactor/compare/master...przepompownia:infer-foreach-item-from-ddocblock?expand=1

![image](https://user-images.githubusercontent.com/11404453/172360449-8d2f6a59-2928-47fb-90af-fcba327dad33.png)
![image](https://user-images.githubusercontent.com/11404453/172360554-1cb19956-2838-4a75-a5b0-43c71abdb8a4.png)
The variable defined in docblock is recognized independently of real assignment:
![image](https://user-images.githubusercontent.com/11404453/172361238-815af9ad-20e3-49a2-93c7-bca093e4456e.png)



Pull request description:
Use vardoc buffer to keep track of @var assignments
Fixes #1708 and #1707


Patch:
diff --git a/lib/WorseReflection/Core/Inference/Frame.php b/lib/WorseReflection/Core/Inference/Frame.php
index 16916aba..ee3aca9b 100644
--- a/lib/WorseReflection/Core/Inference/Frame.php
+++ b/lib/WorseReflection/Core/Inference/Frame.php
@@ -31,6 +31,8 @@ class Frame
 
     private int $version = 1;
 
+    private VarDocBuffer $varDocBuffer;
+
     public function __construct(
         string $name,
         LocalAssignments $locals = null,
@@ -41,6 +43,7 @@ class Frame
         $this->properties = $properties ?: PropertyAssignments::create();
         $this->locals = $locals ?: LocalAssignments::create();
         $this->problems = $problems ?: Problems::create();
+        $this->varDocBuffer = new VarDocBuffer();
         $this->parent = $parent;
         $this->name = $name;
     }
@@ -159,10 +162,16 @@ class Frame
     public function version(): string
     {
         return sprintf(
-            '%s-%s-%s',
+            '%s-%s-%s-%s',
             $this->locals()->version(),
             $this->properties()->version(),
+            $this->varDocBuffer()->version(),
             $this->version
         );
     }
+
+    public function varDocBuffer(): VarDocBuffer
+    {
+        return $this->varDocBuffer;
+    }
 }
diff --git a/lib/WorseReflection/Core/Inference/NodeContextFactory.php b/lib/WorseReflection/Core/Inference/NodeContextFactory.php
index 61d0579f..471a0d2e 100644
--- a/lib/WorseReflection/Core/Inference/NodeContextFactory.php
+++ b/lib/WorseReflection/Core/Inference/NodeContextFactory.php
@@ -66,7 +66,7 @@ class NodeContextFactory
                 [
                     'symbol_type' => Symbol::VARIABLE
                 ]
-            )->withIssue(sprintf('Variable "%s" is undefined', $varName));
+            );
         }
 
         $variable = $variables->last();
diff --git a/lib/WorseReflection/Core/Inference/Resolver/VariableResolver.php b/lib/WorseReflection/Core/Inference/Resolver/VariableResolver.php
index 99f75ebc..386f911f 100644
--- a/lib/WorseReflection/Core/Inference/Resolver/VariableResolver.php
+++ b/lib/WorseReflection/Core/Inference/Resolver/VariableResolver.php
@@ -15,6 +15,7 @@ use Phpactor\WorseReflection\Core\Inference\Symbol;
 use Phpactor\WorseReflection\Core\Inference\NodeContextResolver;
 use Phpactor\WorseReflection\Core\Inference\TypeAssertion;
 use Phpactor\WorseReflection\Core\Inference\TypeCombinator;
+use Phpactor\WorseReflection\Core\Inference\Variable as PhpactorVariable;
 use Phpactor\WorseReflection\Core\Type;
 use Phpactor\WorseReflection\Core\TypeFactory;
 use Phpactor\WorseReflection\Core\Type\MissingType;
@@ -46,12 +47,13 @@ class VariableResolver implements Resolver
 
         $variableName = $node->getText();
         $frameVariable = $frame->locals()->byName($variableName)->lessThanOrEqualTo($node->getStartPosition())->lastOrNull();
+
         $type = new MissingType();
         if ($frameVariable) {
             $type = $frameVariable->type();
         }
 
-        return NodeContextFactory::forVariableAt(
+        $context = NodeContextFactory::forVariableAt(
             $frame,
             $node->getStartPosition(),
             $node->getEndPosition(),
@@ -64,6 +66,15 @@ class VariableResolver implements Resolver
             },
             fn (Type $type) => TypeCombinator::intersection(TypeFactory::unionEmpty(), $type),
         ))->withType($type);
+
+        $varDocType = $frame->varDocBuffer()->yank($variableName);
+
+        if (null !== $varDocType) {
+            $context = $context->withType($varDocType);
+            $this->applyVarDoc($context, $frame, $varDocType);
+        }
+
+        return $context;
     }
 
     private function resolvePropertyVariable(NodeContextResolver $resolver, Variable $node): NodeContext
@@ -101,4 +112,14 @@ class VariableResolver implements Resolver
             $info->symbol()->name()
         );
     }
+
+    private function applyVarDoc(NodeContext $context, Frame $frame, Type $varDocType): void
+    {
+        foreach ($frame->locals()->byName($context->symbol()->name())->equalTo($context->symbol()->position()->start()) as $existing) {
+            assert($existing instanceof PhpactorVariable);
+            $frame->locals()->replace($existing, $existing->withType($context->type()));
+            return;
+        }
+        $frame->locals()->set(PhpactorVariable::fromSymbolContext($context));
+    }
 }
diff --git a/lib/WorseReflection/Core/Inference/VarDocBuffer.php b/lib/WorseReflection/Core/Inference/VarDocBuffer.php
new file mode 100644
index 00000000..c923f936
--- /dev/null
+++ b/lib/WorseReflection/Core/Inference/VarDocBuffer.php
@@ -0,0 +1,38 @@
+<?php
+
+namespace Phpactor\WorseReflection\Core\Inference;
+
+use Phpactor\WorseReflection\Core\Type;
+
+class VarDocBuffer
+{
+    /**
+     * @var array<string,Type>
+     */
+    private array $buffer = [];
+
+    private int $version = 0;
+
+    public function set(string $name, Type $type): void
+    {
+        $this->version++;
+        $this->buffer[$name] = $type;
+    }
+
+    public function yank(string $name): ?Type
+    {
+        if (!isset($this->buffer[$name])) {
+            return null;
+        }
+        $type = $this->buffer[$name];
+
+        unset($this->buffer[$name]);
+
+        return $type;
+    }
+
+    public function version(): int
+    {
+        return $this->version;
+    }
+}
diff --git a/lib/WorseReflection/Core/Inference/Variable.php b/lib/WorseReflection/Core/Inference/Variable.php
index f2140106..ef55875b 100644
--- a/lib/WorseReflection/Core/Inference/Variable.php
+++ b/lib/WorseReflection/Core/Inference/Variable.php
@@ -26,7 +26,7 @@ final class Variable
         $this->wasAssigned = $wasAssigned;
     }
 
-    public function __toString()
+    public function __toString(): string
     {
         return sprintf('%s#%s %s %s', $this->name, $this->offset, $this->type, $this->classType ? $this->classType->__toString() : '');
     }
diff --git a/lib/WorseReflection/Core/Inference/Walker/PassThroughWalker.php b/lib/WorseReflection/Core/Inference/Walker/PassThroughWalker.php
index fb543234..673de2dd 100644
--- a/lib/WorseReflection/Core/Inference/Walker/PassThroughWalker.php
+++ b/lib/WorseReflection/Core/Inference/Walker/PassThroughWalker.php
@@ -6,6 +6,7 @@ use Microsoft\PhpParser\Node;
 use Microsoft\PhpParser\Node\CatchClause;
 use Microsoft\PhpParser\Node\Expression\BinaryExpression;
 use Microsoft\PhpParser\Node\Expression\CallExpression;
+use Microsoft\PhpParser\Node\Expression\Variable;
 use Microsoft\PhpParser\Node\Expression\YieldExpression;
 use Microsoft\PhpParser\Node\Statement\ForeachStatement;
 use Microsoft\PhpParser\Node\Statement\IfStatementNode;
@@ -31,7 +32,8 @@ class PassThroughWalker implements Walker
             CatchClause::class,
             BinaryExpression::class,
             CallExpression::class,
-            AssignmentExpression::class
+            AssignmentExpression::class,
+            Variable::class,
         ];
     }
 
diff --git a/lib/WorseReflection/Core/Inference/Walker/VariableWalker.php b/lib/WorseReflection/Core/Inference/Walker/VariableWalker.php
index 795ff49d..21fb21f3 100644
--- a/lib/WorseReflection/Core/Inference/Walker/VariableWalker.php
+++ b/lib/WorseReflection/Core/Inference/Walker/VariableWalker.php
@@ -4,31 +4,21 @@ namespace Phpactor\WorseReflection\Core\Inference\Walker;
 
 use Microsoft\PhpParser\Token;
 use Phpactor\WorseReflection\Bridge\TolerantParser\Reflection\ReflectionScope;
-use Phpactor\WorseReflection\Core\Inference\Symbol;
-use Phpactor\WorseReflection\Core\Inference\NodeContextFactory;
-use Phpactor\WorseReflection\Core\Inference\Variable as WorseVariable;
 use Microsoft\PhpParser\Node;
 use Microsoft\PhpParser\Node\Expression\Variable;
 use Phpactor\WorseReflection\Core\Inference\Frame;
 use Phpactor\WorseReflection\Core\DocBlock\DocBlockFactory;
 use Phpactor\WorseReflection\Core\Inference\FrameResolver;
 use Phpactor\WorseReflection\Core\DocBlock\DocBlockVar;
-use Phpactor\WorseReflection\Core\Inference\Variable as PhpactorVariable;
 use Phpactor\WorseReflection\Core\Inference\Walker;
 use Phpactor\WorseReflection\Core\Reflection\ReflectionScope as PhpactorReflectionScope;
 use Phpactor\WorseReflection\Core\Type;
-use Phpactor\WorseReflection\Core\TypeFactory;
 use Phpactor\WorseReflection\Core\TypeResolver\DefaultTypeResolver;
 
 class VariableWalker implements Walker
 {
     private DocBlockFactory $docblockFactory;
     
-    /**
-     * @var array<string,Type>
-     */
-    private array $injectedTypes = [];
-    
     public function __construct(
         DocBlockFactory $docblockFactory
     ) {
@@ -46,44 +36,21 @@ class VariableWalker implements Walker
         $scope = new ReflectionScope($resolver->reflector(), $node);
         $docblockType = $this->injectVariablesFromComment($scope, $frame, $node);
 
-        if (!$node instanceof Variable) {
+        if (null === $docblockType) {
             return $frame;
         }
 
-        $token = $node->name;
-        if (false === $token instanceof Token) {
+        if (!$node instanceof Variable) {
             return $frame;
         }
 
-        $context = NodeContextFactory::create(
-            (string)$token->getText($node->getFileContents()),
-            $node->getStartPosition(),
-            $node->getEndPosition(),
-            [
-                'symbol_type' => Symbol::VARIABLE,
-            ]
-        );
-
-        $symbolName = $context->symbol()->name();
-
-        if (!isset($this->injectedTypes[$symbolName]) && !($docblockType->isDefined())) {
+        $token = $node->name;
+        if (false === $token instanceof Token) {
             return $frame;
         }
 
-        if (isset($this->injectedTypes[$symbolName])) {
-            $docblockType = $this->injectedTypes[$symbolName];
-            unset($this->injectedTypes[$symbolName]);
-        }
-
-        $context = $context->withType($docblockType);
-        $locals = $frame->locals();
-        foreach ($locals->byName($symbolName)->equalTo($context->symbol()->position()->start()) as $existing) {
-            assert($existing instanceof PhpactorVariable);
-            // TODO: not sure this will work as expected
-            $locals->replace($existing, $existing->withType($context->type()));
-            return $frame;
-        }
-        $frame->locals()->set(WorseVariable::fromSymbolContext($context));
+        $name = (string)$token->getText($node->getFileContents());
+        $frame->varDocBuffer()->set($name, $docblockType);
 
         return $frame;
     }
@@ -93,13 +60,13 @@ class VariableWalker implements Walker
         return $frame;
     }
 
-    private function injectVariablesFromComment(PhpactorReflectionScope $scope, Frame $frame, Node $node): Type
+    private function injectVariablesFromComment(PhpactorReflectionScope $scope, Frame $frame, Node $node): ?Type
     {
         $comment = $node->getLeadingCommentAndWhitespaceText();
         $docblock = $this->docblockFactory->create($comment)->withTypeResolver(new DefaultTypeResolver($scope));
 
         if (false === $docblock->isDefined()) {
-            return TypeFactory::undefined();
+            return null;
         }
 
         $vars = $docblock->vars();
@@ -113,9 +80,9 @@ class VariableWalker implements Walker
                 return $type;
             }
 
-            $this->injectedTypes[ltrim($var->name(), '$')] = $type;
+            $frame->varDocBuffer()->set('$' . $var->name(), $type);
         }
 
-        return TypeFactory::undefined();
+        return null;
     }
 }


Test patch:
diff --git a/lib/WorseReflection/Tests/Inference/foreach/gh-1708.test b/lib/WorseReflection/Tests/Inference/foreach/gh-1708.test
new file mode 100644
index 00000000..bb35beb1
--- /dev/null
+++ b/lib/WorseReflection/Tests/Inference/foreach/gh-1708.test
@@ -0,0 +1,6 @@
+<?php
+
+/** @var int[] $items */
+foreach ($items as $item) {
+    wrAssertType("int", $item);
+}
diff --git a/lib/WorseReflection/Tests/Inference/type/gh-1707.test b/lib/WorseReflection/Tests/Inference/type/gh-1707.test
new file mode 100644
index 00000000..58617b44
--- /dev/null
+++ b/lib/WorseReflection/Tests/Inference/type/gh-1707.test
@@ -0,0 +1,8 @@
+<?php
+
+/** @var array{name:string,age:int} */
+$shape = [];
+
+$tmp = $shape; // only suggest $shape
+
+wrAssertType('array{name:string,age:int}', $tmp);