Repository: phpactor/phpactor (php)

Issue:
Undefined variable $matches with list unpacking
Hi,

with the following code 

```php
$str = '2023_001';
if(preg_match('#([0-9]+)_([0-9]+)#', $str, $matches) {
  list($a, $b, $c) = $matches;
  dump($a, $b, $c);
}
```
phpactor considers `$matches` as undefined.

![image](https://github.com/phpactor/phpactor/assets/5028767/ea9bac9a-9797-449c-93e8-5c8c719ec5ec)



PR #2291 only fixes the  `preg_match` part.

Pull request description:
Gh 2361: Really _declare_ passed-by-ref vars
Fixes #2361 

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7aaeb2bc..a8b9c973 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,7 +5,8 @@ Changelog
 
 Bug fixes:
 
-  - Fix undefined-var false positives for undeclared variables that have `@var` #2366
+  - Fix undefined-var false positive for undeclared variables that have `@var` #2366
+  - Fix undefined-var false positive for pass by ref (again) #2361
 
 Features:
 
diff --git a/lib/WorseReflection/Bridge/TolerantParser/Diagnostics/UndefinedVariableProvider.php b/lib/WorseReflection/Bridge/TolerantParser/Diagnostics/UndefinedVariableProvider.php
index deb9d2a9..92887247 100644
--- a/lib/WorseReflection/Bridge/TolerantParser/Diagnostics/UndefinedVariableProvider.php
+++ b/lib/WorseReflection/Bridge/TolerantParser/Diagnostics/UndefinedVariableProvider.php
@@ -3,9 +3,6 @@
 namespace Phpactor\WorseReflection\Bridge\TolerantParser\Diagnostics;
 
 use Microsoft\PhpParser\Node;
-use Microsoft\PhpParser\Node\DelimitedList\ArgumentExpressionList;
-use Microsoft\PhpParser\Node\Expression\ArgumentExpression;
-use Microsoft\PhpParser\Node\Expression\CallExpression;
 use Microsoft\PhpParser\Node\Expression\ScopedPropertyAccessExpression;
 use Microsoft\PhpParser\Node\Expression\Variable;
 use Microsoft\PhpParser\Node\PropertyDeclaration;
@@ -13,13 +10,10 @@ use PHPUnit\Framework\Assert;
 use Phpactor\WorseReflection\Core\DiagnosticExample;
 use Phpactor\WorseReflection\Core\DiagnosticProvider;
 use Phpactor\WorseReflection\Core\Diagnostics;
-use Phpactor\WorseReflection\Core\Inference\Context\FunctionCallContext;
-use Phpactor\WorseReflection\Core\Inference\Context\MemberAccessContext;
 use Phpactor\WorseReflection\Core\Inference\Frame;
 use Phpactor\WorseReflection\Core\Inference\NodeContextResolver;
 use Phpactor\WorseReflection\Core\Inference\SuperGlobals;
 use Phpactor\WorseReflection\Core\Inference\Variable as PhpactorVariable;
-use Phpactor\WorseReflection\Core\Reflection\ReflectionMember;
 use Phpactor\WorseReflection\Core\Util\NodeUtil;
 
 /**
@@ -280,6 +274,19 @@ class UndefinedVariableProvider implements DiagnosticProvider
                 Assert::assertCount(0, $diagnostics);
             }
         );
+        yield new DiagnosticExample(
+            title: 'pass by reference introduces var',
+            source: <<<'PHP'
+                <?php
+                function preg_match(string $pattern, string $string, array &$matches): bool {}
+                preg_match('foobar', 'barfoo', $matches);
+                echo $matches[1];
+                PHP,
+            valid: true,
+            assertion: function (Diagnostics $diagnostics): void {
+                Assert::assertCount(0, $diagnostics);
+            }
+        );
         yield new DiagnosticExample(
             title: 'super globals',
             source: <<<'PHP'
@@ -437,50 +444,6 @@ class UndefinedVariableProvider implements DiagnosticProvider
                 return [];
             }
         }
-        $isByReference = function () use ($resolver, $frame, $node) {
-            if (!$node->parent instanceof ArgumentExpression) {
-                return false;
-            }
-            $argument = $node->parent;
-            if (!$argument->parent instanceof ArgumentExpressionList) {
-                return false;
-            }
-
-            $argumentExpressionList = $argument->parent;
-            if (!$argumentExpressionList->parent instanceof CallExpression) {
-                return false;
-            }
-
-            $offset = NodeUtil::argumentOffset($argumentExpressionList, $argument);
-            if ($offset === null) {
-                return false;
-            }
-
-            $call = $argumentExpressionList->parent;
-            $callContext = $resolver->resolveNode($frame, $call);
-            $parameter = null;
-
-            if ($callContext instanceof FunctionCallContext) {
-                $parameter = $callContext->function()->parameters()->at($offset);
-            }
-
-            if (!$parameter && $callContext instanceof MemberAccessContext) {
-                $member = $callContext->accessedMember();
-                if ($member->memberType() === ReflectionMember::TYPE_METHOD) {
-                    $parameter = $member->class()->methods()->get($member->name())->parameters()->at($offset);
-                }
-            }
-
-            if (!$parameter) {
-                return false;
-            }
-
-            return $parameter->byReference();
-        };
-
-        if ($isByReference()) {
-            return false;
-        }
 
         yield new UndefinedVariableDiagnostic(
             NodeUtil::byteOffsetRangeForNode($node),
diff --git a/lib/WorseReflection/Core/Inference/Resolver/MemberAccess/NodeContextFromMemberAccess.php b/lib/WorseReflection/Core/Inference/Resolver/MemberAccess/NodeContextFromMemberAccess.php
index 196fbf68..796c51fd 100644
--- a/lib/WorseReflection/Core/Inference/Resolver/MemberAccess/NodeContextFromMemberAccess.php
+++ b/lib/WorseReflection/Core/Inference/Resolver/MemberAccess/NodeContextFromMemberAccess.php
@@ -16,6 +16,7 @@ use Phpactor\WorseReflection\Core\Inference\GenericMapResolver;
 use Phpactor\WorseReflection\Core\Inference\NodeContextFactory;
 use Phpactor\WorseReflection\Core\Inference\Symbol;
 use Phpactor\WorseReflection\Core\Inference\NodeContext;
+use Phpactor\WorseReflection\Core\Inference\Variable;
 use Phpactor\WorseReflection\Core\Reflection\Collection\ReflectionInterfaceCollection;
 use Phpactor\WorseReflection\Core\Reflection\ReflectionClass;
 use Phpactor\WorseReflection\Core\Reflection\ReflectionClassLike;
@@ -174,6 +175,23 @@ class NodeContextFromMemberAccess
             }
         }
 
+        if ($member instanceof ReflectionMethod && $arguments) {
+            $byReference = $member->parameters()->passedByReference();
+
+            if ($byReference->count()) {
+                foreach ($byReference as $parameter) {
+                    $argument = $arguments->at($parameter->index());
+                    $frame->locals()->set(new Variable(
+                        name: $argument->symbol()->name(),
+                        offset: $argument->symbol()->position()->start()->toInt(),
+                        type: $parameter->type(),
+                        wasAssigned: false /** $wasAssigned bool */,
+                        wasDefined: true /** $wasDefined bool */
+                    ));
+                }
+            }
+        }
+
         $containerType = UnionType::fromTypes(...$types)->reduce();
         return [$containerType, $memberType, $member];
     }
diff --git a/lib/WorseReflection/Core/Inference/Resolver/QualifiedNameResolver.php b/lib/WorseReflection/Core/Inference/Resolver/QualifiedNameResolver.php
index b193eb44..34390951 100644
--- a/lib/WorseReflection/Core/Inference/Resolver/QualifiedNameResolver.php
+++ b/lib/WorseReflection/Core/Inference/Resolver/QualifiedNameResolver.php
@@ -18,6 +18,7 @@ use Phpactor\WorseReflection\Core\Inference\NodeContextFactory;
 use Phpactor\WorseReflection\Core\Inference\Resolver;
 use Phpactor\WorseReflection\Core\Inference\Symbol;
 use Phpactor\WorseReflection\Core\Inference\NodeContextResolver;
+use Phpactor\WorseReflection\Core\Inference\Variable;
 use Phpactor\WorseReflection\Core\Name;
 use Phpactor\WorseReflection\Core\TypeFactory;
 use Phpactor\WorseReflection\Core\Type\ReflectedClassType;
@@ -139,12 +140,33 @@ class QualifiedNameResolver implements Resolver
 
         $stub = $this->registry->get($name->short());
 
-        if ($stub) {
+        $byReference = $function->parameters()->passedByReference();
+        $arguments = null;
+
+        if ($byReference->count()) {
             $arguments = FunctionArguments::fromList(
                 $resolver,
                 $frame,
                 $parent->argumentExpressionList
             );
+            foreach ($byReference as $parameter) {
+                $argument = $arguments->at($parameter->index());
+                $frame->locals()->set(new Variable(
+                    name: $argument->symbol()->name(),
+                    offset: $argument->symbol()->position()->start()->toInt(),
+                    type: $parameter->type(),
+                    wasAssigned: false /** $wasAssigned bool */,
+                    wasDefined: true /** $wasDefined bool */
+                ));
+            }
+        }
+
+        if ($stub) {
+            $arguments = $arguments ?: FunctionArguments::fromList(
+                $resolver,
+                $frame,
+                $parent->argumentExpressionList
+            );
             return $stub->resolve($frame, $context, $arguments);
         }
 
diff --git a/lib/WorseReflection/Core/Reflection/Collection/ReflectionParameterCollection.php b/lib/WorseReflection/Core/Reflection/Collection/ReflectionParameterCollection.php
index 964185f5..d64fb8a3 100644
--- a/lib/WorseReflection/Core/Reflection/Collection/ReflectionParameterCollection.php
+++ b/lib/WorseReflection/Core/Reflection/Collection/ReflectionParameterCollection.php
@@ -103,4 +103,11 @@ final class ReflectionParameterCollection extends AbstractReflectionCollection
 
         return null;
     }
+
+    public function passedByReference(): PhpactorReflectionParameterCollection
+    {
+        return new self(array_filter($this->items, function (PhpactorReflectionParameter $parameter) {
+            return $parameter->byReference();
+        }));
+    }
 }


Test patch:
diff --git a/lib/WorseReflection/Tests/Inference/variable/pass-by-ref.test b/lib/WorseReflection/Tests/Inference/variable/pass-by-ref.test
new file mode 100644
index 00000000..fe4408f6
--- /dev/null
+++ b/lib/WorseReflection/Tests/Inference/variable/pass-by-ref.test
@@ -0,0 +1,10 @@
+<?php
+
+function foo(?string &$bar): void
+{
+    $bar ='foo';
+}
+
+foo($bar);
+
+wrAssertType('?string', $bar);