From 0fa7cbeb021c2dbcb0f9dc6b59058a8219e462fd Mon Sep 17 00:00:00 2001
From: Matthew Brown <github@muglug.com>
Date: Mon, 16 Jul 2018 22:48:53 -0400
Subject: [PATCH] Fix #895 - check __call magic method args

---
 .../Expression/Call/MethodCallChecker.php     | 54 ++++++++++++-------
 tests/ClassTest.php                           | 19 +++++++
 tests/MagicMethodAnnotationTest.php           | 24 ++++-----
 tests/MethodCallTest.php                      |  4 +-
 4 files changed, 68 insertions(+), 33 deletions(-)

diff --git a/src/Psalm/Checker/Statements/Expression/Call/MethodCallChecker.php b/src/Psalm/Checker/Statements/Expression/Call/MethodCallChecker.php
index 779b06b85..b962fe8bb 100644
--- a/src/Psalm/Checker/Statements/Expression/Call/MethodCallChecker.php
+++ b/src/Psalm/Checker/Statements/Expression/Call/MethodCallChecker.php
@@ -93,10 +93,12 @@ class MethodCallChecker extends \Psalm\Checker\Statements\Expression\CallChecker
 
         $source = $statements_checker->getSource();
 
+        $args = $stmt->args;
+
         if (!$context->check_methods || !$context->check_classes) {
             if (self::checkFunctionArguments(
                 $statements_checker,
-                $stmt->args,
+                $args,
                 null,
                 null,
                 $context
@@ -217,7 +219,7 @@ class MethodCallChecker extends \Psalm\Checker\Statements\Expression\CallChecker
 
                             if (self::checkFunctionArguments(
                                 $statements_checker,
-                                $stmt->args,
+                                $args,
                                 null,
                                 null,
                                 $context
@@ -318,7 +320,7 @@ class MethodCallChecker extends \Psalm\Checker\Statements\Expression\CallChecker
 
                                 if (self::checkFunctionArguments(
                                     $statements_checker,
-                                    $stmt->args,
+                                    $args,
                                     $pseudo_method_storage->params,
                                     $method_id,
                                     $context
@@ -330,7 +332,7 @@ class MethodCallChecker extends \Psalm\Checker\Statements\Expression\CallChecker
 
                                 if (self::checkFunctionLikeArgumentsMatch(
                                     $statements_checker,
-                                    $stmt->args,
+                                    $args,
                                     null,
                                     $pseudo_method_storage->params,
                                     $pseudo_method_storage,
@@ -356,7 +358,7 @@ class MethodCallChecker extends \Psalm\Checker\Statements\Expression\CallChecker
                             } else {
                                 if (self::checkFunctionArguments(
                                     $statements_checker,
-                                    $stmt->args,
+                                    $args,
                                     null,
                                     null,
                                     $context
@@ -374,8 +376,22 @@ class MethodCallChecker extends \Psalm\Checker\Statements\Expression\CallChecker
                         $has_valid_method_call_type = true;
                         $existent_method_ids[] = $method_id;
 
-                        $return_type = Type::getMixed();
-                        continue;
+                        $array_values = array_map(
+                            /**
+                             * @return PhpParser\Node\Expr\ArrayItem
+                             */
+                            function (PhpParser\Node\Arg $arg) {
+                                return new PhpParser\Node\Expr\ArrayItem($arg->value);
+                            },
+                            $args
+                        );
+
+                        $args = [
+                            new PhpParser\Node\Arg(new PhpParser\Node\Scalar\String_($method_name_lc)),
+                            new PhpParser\Node\Arg(new PhpParser\Node\Expr\Array_($array_values)),
+                        ];
+
+                        $method_id = $fq_class_name . '::__call';
                     }
                 }
 
@@ -424,7 +440,7 @@ class MethodCallChecker extends \Psalm\Checker\Statements\Expression\CallChecker
 
                             if (self::checkFunctionArguments(
                                 $statements_checker,
-                                $stmt->args,
+                                $args,
                                 $pseudo_method_storage->params,
                                 $method_id,
                                 $context
@@ -436,7 +452,7 @@ class MethodCallChecker extends \Psalm\Checker\Statements\Expression\CallChecker
 
                             if (self::checkFunctionLikeArgumentsMatch(
                                 $statements_checker,
-                                $stmt->args,
+                                $args,
                                 null,
                                 $pseudo_method_storage->params,
                                 $pseudo_method_storage,
@@ -513,7 +529,7 @@ class MethodCallChecker extends \Psalm\Checker\Statements\Expression\CallChecker
 
                 if (self::checkMethodArgs(
                     $method_id,
-                    $stmt->args,
+                    $args,
                     $class_template_params,
                     $context,
                     $code_location,
@@ -549,11 +565,11 @@ class MethodCallChecker extends \Psalm\Checker\Statements\Expression\CallChecker
                         }
                     } else {
                         if ($call_map_id === 'domnode::appendchild'
-                            && isset($stmt->args[0]->value->inferredType)
-                            && $stmt->args[0]->value->inferredType->hasObjectType()
+                            && isset($args[0]->value->inferredType)
+                            && $args[0]->value->inferredType->hasObjectType()
                         ) {
-                            $return_type_candidate = clone $stmt->args[0]->value->inferredType;
-                        } elseif ($call_map_id === 'simplexmlelement::asxml' && !count($stmt->args)) {
+                            $return_type_candidate = clone $args[0]->value->inferredType;
+                        } elseif ($call_map_id === 'simplexmlelement::asxml' && !count($args)) {
                             $return_type_candidate = Type::parseString('string|false');
                         } else {
                             $return_type_candidate = CallMap::getReturnTypeFromCallMap($call_map_id);
@@ -603,7 +619,7 @@ class MethodCallChecker extends \Psalm\Checker\Statements\Expression\CallChecker
                     $return_type_candidate = $codebase->methods->getMethodReturnType(
                         $method_id,
                         $self_fq_class_name,
-                        $stmt->args
+                        $args
                     );
 
                     if (isset($stmt->inferredType)) {
@@ -656,7 +672,7 @@ class MethodCallChecker extends \Psalm\Checker\Statements\Expression\CallChecker
                         if ($method_storage->assertions) {
                             self::applyAssertionsToContext(
                                 $method_storage->assertions,
-                                $stmt->args,
+                                $args,
                                 $context,
                                 $statements_checker
                             );
@@ -672,7 +688,7 @@ class MethodCallChecker extends \Psalm\Checker\Statements\Expression\CallChecker
                     }
                 }
 
-                if (!$stmt->args && $var_id) {
+                if (!$args && $var_id) {
                     if ($config->memoize_method_calls) {
                         $method_var_id = $var_id . '->' . $method_name_lc . '()';
 
@@ -697,7 +713,7 @@ class MethodCallChecker extends \Psalm\Checker\Statements\Expression\CallChecker
                             $appearing_method_id,
                             $declaring_method_id,
                             $var_id,
-                            $stmt->args,
+                            $args,
                             $code_location,
                             $context,
                             $file_manipulations,
@@ -790,7 +806,7 @@ class MethodCallChecker extends \Psalm\Checker\Statements\Expression\CallChecker
         if ($method_id === null) {
             return self::checkMethodArgs(
                 $method_id,
-                $stmt->args,
+                $args,
                 $found_generic_params,
                 $context,
                 new CodeLocation($statements_checker->getSource(), $stmt),
diff --git a/tests/ClassTest.php b/tests/ClassTest.php
index b022471f4..f32cec03b 100644
--- a/tests/ClassTest.php
+++ b/tests/ClassTest.php
@@ -220,6 +220,25 @@ class ClassTest extends TestCase
                         }
                     }',
             ],
+            'typedMagicCall' => [
+                '<?php
+                    class B {
+                        public function __call(string $methodName, array $args) : string {
+                            return __METHOD__;
+                        }
+                    }
+                    class A {
+                        public function __call(string $methodName, array $args) : B {
+                            return new B;
+                        }
+                    }
+                    $a = (new A)->zugzug();
+                    $b = (new A)->bar()->baz();',
+                'assertions' => [
+                    '$a' => 'B',
+                    '$b' => 'string',
+                ],
+            ],
         ];
     }
 
diff --git a/tests/MagicMethodAnnotationTest.php b/tests/MagicMethodAnnotationTest.php
index 82d68e342..1afc817d5 100644
--- a/tests/MagicMethodAnnotationTest.php
+++ b/tests/MagicMethodAnnotationTest.php
@@ -114,7 +114,7 @@ class MagicMethodAnnotationTest extends TestCase
             'validSimpleAnnotations' => [
                 '<?php
                     class ParentClass {
-                        public function __call() {}
+                        public function __call(string $name, array $args) {}
                     }
 
                     /**
@@ -139,7 +139,7 @@ class MagicMethodAnnotationTest extends TestCase
                     $b = $child->setString(5);
                     $c = $child->getBool("hello");
                     $d = $child->getArray();
-                    $child->setArray(["boo"])
+                    $child->setArray(["boo"]);
                     $e = $child->getCallable();
                     $child->setMixed("hello");
                     $child->setMixed(4);
@@ -156,7 +156,7 @@ class MagicMethodAnnotationTest extends TestCase
             'validAnnotationWithDefault' => [
                 '<?php
                     class ParentClass {
-                        public function __call() {}
+                        public function __call(string $name, array $args) {}
                     }
 
                     /**
@@ -172,7 +172,7 @@ class MagicMethodAnnotationTest extends TestCase
             'validAnnotationWithVariadic' => [
                 '<?php
                     class ParentClass {
-                        public function __call() {}
+                        public function __call(string $name, array $args) {}
                     }
 
                     /**
@@ -187,7 +187,7 @@ class MagicMethodAnnotationTest extends TestCase
             'validUnionAnnotations' => [
                 '<?php
                     class ParentClass {
-                        public function __call() {}
+                        public function __call(string $name, array $args) {}
                     }
 
                     /**
@@ -211,7 +211,7 @@ class MagicMethodAnnotationTest extends TestCase
                     namespace Foo;
 
                     class ParentClass {
-                        public function __call() {}
+                        public function __call(string $name, array $args) {}
                     }
 
                     /**
@@ -255,7 +255,7 @@ class MagicMethodAnnotationTest extends TestCase
             'annotationWithBadDocblock' => [
                 '<?php
                     class ParentClass {
-                        public function __call() {}
+                        public function __call(string $name, array $args) {}
                     }
 
                     /**
@@ -267,7 +267,7 @@ class MagicMethodAnnotationTest extends TestCase
             'annotationWithByRefParam' => [
                 '<?php
                     class ParentClass {
-                        public function __call() {}
+                        public function __call(string $name, array $args) {}
                     }
 
                     /**
@@ -279,7 +279,7 @@ class MagicMethodAnnotationTest extends TestCase
             'annotationWithSealed' => [
                 '<?php
                     class ParentClass {
-                        public function __call() {}
+                        public function __call(string $name, array $args) {}
                     }
 
                     /**
@@ -296,7 +296,7 @@ class MagicMethodAnnotationTest extends TestCase
             'annotationInvalidArg' => [
                 '<?php
                     class ParentClass {
-                        public function __call() {}
+                        public function __call(string $name, array $args) {}
                     }
 
                     /**
@@ -312,7 +312,7 @@ class MagicMethodAnnotationTest extends TestCase
             'unionAnnotationInvalidArg' => [
                 '<?php
                     class ParentClass {
-                        public function __call() {}
+                        public function __call(string $name, array $args) {}
                     }
 
                     /**
@@ -328,7 +328,7 @@ class MagicMethodAnnotationTest extends TestCase
             'validAnnotationWithInvalidVariadicCall' => [
                 '<?php
                     class ParentClass {
-                        public function __call() {}
+                        public function __call(string $name, array $args) {}
                     }
 
                     /**
diff --git a/tests/MethodCallTest.php b/tests/MethodCallTest.php
index 200c66413..28227c790 100644
--- a/tests/MethodCallTest.php
+++ b/tests/MethodCallTest.php
@@ -85,7 +85,7 @@ class MethodCallTest extends TestCase
             'magicCall' => [
                 '<?php
                     class A {
-                        public function __call(string $method_name) {}
+                        public function __call(string $method_name, array $args) {}
                     }
 
                     $a = new A;
@@ -94,7 +94,7 @@ class MethodCallTest extends TestCase
             'canBeCalledOnMagic' => [
                 '<?php
                     class A {
-                      public function __call(string $method) {}
+                      public function __call(string $method, array $args) {}
                     }
 
                     class B {}