Fix #276 - fix false positive in magic call

src/Psalm/Checker/CommentChecker.php

@@ -3,7 +3,6 @@ namespace Psalm\Checker;
 
 use Psalm\Aliases;
 use Psalm\ClassLikeDocblockComment;
-use Psalm\Context;
 use Psalm\Exception\DocblockParseException;
 use Psalm\Exception\TypeParseTreeException;
 use Psalm\FunctionDocblockComment;

src/Psalm/Checker/MethodChecker.php

@@ -447,6 +447,91 @@ class MethodChecker extends FunctionLikeChecker
         return null;
     }
 
+    /**
+     * @param  string           $method_id
+     * @param  string|null      $calling_context
+     * @param  StatementsSource $source
+     * @param  CodeLocation     $code_location
+     * @param  array            $suppressed_issues
+     *
+     * @return bool
+     */
+    public static function isMethodVisible(
+        $method_id,
+        $calling_context,
+        StatementsSource $source
+    ) {
+        $project_checker = $source->getFileChecker()->project_checker;
+
+        $declaring_method_id = self::getDeclaringMethodId($project_checker, $method_id);
+
+        if (!$declaring_method_id) {
+            $method_name = explode('::', $method_id)[1];
+
+            if ($method_name === '__construct') {
+                return true;
+            }
+
+            throw new \UnexpectedValueException('$declaring_method_id not expected to be null here');
+        }
+
+        $appearing_method_id = self::getAppearingMethodId($project_checker, $method_id);
+
+        $appearing_method_class = null;
+
+        if ($appearing_method_id) {
+            list($appearing_method_class) = explode('::', $appearing_method_id);
+
+            // if the calling class is the same, we know the method exists, so it must be visible
+            if ($appearing_method_class === $calling_context) {
+                return true;
+            }
+        }
+
+        list($declaring_method_class) = explode('::', $declaring_method_id);
+
+        if ($source->getSource() instanceof TraitChecker && $declaring_method_class === $source->getFQCLN()) {
+            return true;
+        }
+
+        $storage = self::getStorage($project_checker, $declaring_method_id);
+
+        if (!$storage) {
+            throw new \UnexpectedValueException('$storage should not be null for ' . $declaring_method_id);
+        }
+
+        switch ($storage->visibility) {
+            case ClassLikeChecker::VISIBILITY_PUBLIC:
+                return true;
+
+            case ClassLikeChecker::VISIBILITY_PRIVATE:
+                if (!$calling_context || $appearing_method_class !== $calling_context) {
+                    return false;
+                }
+
+                return true;
+
+            case ClassLikeChecker::VISIBILITY_PROTECTED:
+                if (!$calling_context) {
+                    return false;
+                }
+
+                if ($appearing_method_class
+                    && ClassChecker::classExtends($project_checker, $appearing_method_class, $calling_context)
+                ) {
+                    return true;
+                }
+
+                if ($appearing_method_class
+                    && !ClassChecker::classExtends($project_checker, $calling_context, $appearing_method_class)
+                ) {
+                    return false;
+                }
+        }
+
+        return true;
+    }
+
     /**
      * @param  string           $method_id
      * @param  string|null      $calling_context

src/Psalm/Checker/ProjectChecker.php

@@ -1126,7 +1126,7 @@ class ProjectChecker
                     )) {
                         // fall through
                     }
-                } else {
+                } elseif (!isset($classlike_storage->declaring_method_ids['__call'])) {
                     if (IssueBuffer::accepts(
                         new UnusedMethod(
                             'Method ' . $method_id . ' is never used',

src/Psalm/Checker/Statements/Expression/CallChecker.php

@@ -852,17 +852,25 @@ class CallChecker
                     return;
                 }
 
+                $method_id = $fq_class_name . '::' . strtolower($stmt->name);
+
                 if (MethodChecker::methodExists(
                     $project_checker,
                     $fq_class_name . '::__call'
                 )
                 ) {
-                    $return_type = Type::getMixed();
-                    continue;
+                    if (!MethodChecker::methodExists($project_checker, $method_id)
+                        || !MethodChecker::isMethodVisible(
+                            $method_id,
+                            $context->self,
+                            $statements_checker->getSource()
+                        )
+                    ) {
+                        $return_type = Type::getMixed();
+                        continue;
+                    }
                 }
 
-                $method_id = $fq_class_name . '::' . strtolower($stmt->name);
-
                 if ($var_id === '$this' &&
                     $context->self &&
                     $fq_class_name !== $context->self &&

src/Psalm/Checker/TraitChecker.php

@@ -2,7 +2,6 @@
 namespace Psalm\Checker;
 
 use PhpParser;
-use Psalm\StatementsSource;
 use Psalm\TraitSource;
 
 class TraitChecker extends ClassLikeChecker

tests/UnusedCodeTest.php

@@ -136,6 +136,32 @@ class UnusedCodeTest extends TestCase
                         return $a || $b;
                     }',
             ],
+            'magicCall' => [
+                '<?php
+                    class A {
+                        /** @var string */
+                        private $value = "default";
+
+                        /** @param string[] $args */
+                        public function __call(string $name, array $args) {
+                            if (count($args) == 1) {
+                                $this->modify($name, $args[0]);
+                            }
+                        }
+
+                        private function modify(string $name, string $value) : void {
+                            call_user_func(array($this, "modify_" . $name), $value);
+                        }
+
+                        public function modifyFoo(string $value) : void {
+                            $this->value = $value;
+                        }
+                    }
+
+                    $m = new A();
+                    $m->foo("value");
+                    $m->modifyFoo("value2");',
+            ],
         ];
     }