Support dynamic method calls (#52)

Author: janedbal

README.md

@@ -50,7 +50,6 @@ class MyEntrypointProvider implements EntrypointProvider
 - Only method calls are detected
   - Including static methods, trait methods, interface methods, first class callables, etc.
   - Any calls on mixed types are not detected, e.g. `$unknownClass->method()`
-  - Expression method calls are not detected, e.g. `$this->$methodName()`
   - Anonymous classes are ignored
   - Does not check magic methods
   - No transitive check is performed (dead method called only from dead method)

src/Collector/MethodCallCollector.php

@@ -11,15 +11,13 @@
 use PhpParser\Node\Expr\New_;
 use PhpParser\Node\Expr\NullsafeMethodCall;
 use PhpParser\Node\Expr\StaticCall;
-use PhpParser\Node\Identifier;
 use PhpParser\Node\Name;
 use PHPStan\Analyser\Scope;
 use PHPStan\Collectors\Collector;
 use PHPStan\Node\ClassMethodsNode;
 use PHPStan\Node\MethodCallableNode;
 use PHPStan\Node\StaticMethodCallableNode;
 use PHPStan\Reflection\ClassReflection;
-use PHPStan\Reflection\ReflectionProvider;
 use PHPStan\Type\Type;
 use PHPStan\Type\TypeCombinator;
 use ShipMonk\PHPStan\DeadCode\Crate\Call;
@@ -31,18 +29,11 @@
 class MethodCallCollector implements Collector
 {
 
-    private ReflectionProvider $reflectionProvider;
-
     /**
      * @var list<Call>
      */
     private array $callsBuffer = [];
 
-    public function __construct(ReflectionProvider $reflectionProvider)
-    {
-        $this->reflectionProvider = $reflectionProvider;
-    }
-
     public function getNodeType(): string
     {
         return Node::class;
@@ -104,7 +95,7 @@ private function registerMethodCall(
         Scope $scope
     ): void
     {
-        $methodName = $this->getMethodName($methodCall);
+        $methodNames = $this->getMethodName($methodCall, $scope);
 
         if ($methodCall instanceof New_) {
             if ($methodCall->class instanceof Expr) {
@@ -123,13 +114,15 @@ private function registerMethodCall(
             $possibleDescendantCall = true;
         }
 
-        if ($methodName === null) {
-            return;
-        }
+        foreach ($methodNames as $methodName) {
+            foreach ($this->getReflectionsWithMethod($callerType, $methodName) as $classWithMethod) {
+                if (!$classWithMethod->hasMethod($methodName)) {
+                    continue;
+                }
 
-        foreach ($this->getReflectionsWithMethod($callerType, $methodName) as $classWithMethod) {
-            $className = $classWithMethod->getMethod($methodName, $scope)->getDeclaringClass()->getName();
-            $this->callsBuffer[] = new Call($className, $methodName, $possibleDescendantCall);
+                $className = $classWithMethod->getMethod($methodName, $scope)->getDeclaringClass()->getName();
+                $this->callsBuffer[] = new Call($className, $methodName, $possibleDescendantCall);
+            }
         }
     }
 
@@ -138,33 +131,26 @@ private function registerStaticCall(
         Scope $scope
     ): void
     {
-        $methodName = $this->getMethodName($staticCall);
-
-        if ($methodName === null) {
-            return;
-        }
+        $methodNames = $this->getMethodName($staticCall, $scope);
 
         if ($staticCall->class instanceof Expr) {
             $callerType = $scope->getType($staticCall->class);
-            $classReflections = $this->getReflectionsWithMethod($callerType, $methodName);
             $possibleDescendantCall = true;
 
         } else {
-            $className = $scope->resolveName($staticCall->class);
+            $callerType = $scope->resolveTypeByName($staticCall->class);
             $possibleDescendantCall = $staticCall->class->toString() === 'static';
-
-            if ($this->reflectionProvider->hasClass($className)) {
-                $classReflections = [
-                    $this->reflectionProvider->getClass($className),
-                ];
-            } else {
-                $classReflections = [];
-            }
         }
 
-        foreach ($classReflections as $classWithMethod) {
-            $className = $classWithMethod->getMethod($methodName, $scope)->getDeclaringClass()->getName();
-            $this->callsBuffer[] = new Call($className, $methodName, $possibleDescendantCall);
+        foreach ($methodNames as $methodName) {
+            foreach ($this->getReflectionsWithMethod($callerType, $methodName) as $classReflection) {
+                if (!$classReflection->hasMethod($methodName)) {
+                    continue;
+                }
+
+                $className = $classReflection->getMethod($methodName, $scope)->getDeclaringClass()->getName();
+                $this->callsBuffer[] = new Call($className, $methodName, $possibleDescendantCall);
+            }
         }
     }
 
@@ -200,18 +186,25 @@ private function registerAttribute(Attribute $node, Scope $scope): void
 
     /**
      * @param NullsafeMethodCall|MethodCall|StaticCall|New_ $call
+     * @return list<string>
      */
-    private function getMethodName(CallLike $call): ?string
+    private function getMethodName(CallLike $call, Scope $scope): array
     {
         if ($call instanceof New_) {
-            return '__construct';
+            return ['__construct'];
         }
 
-        if (!$call->name instanceof Identifier) {
-            return null;
+        if ($call->name instanceof Expr) {
+            $possibleMethodNames = [];
+
+            foreach ($scope->getType($call->name)->getConstantStrings() as $constantString) {
+                $possibleMethodNames[] = $constantString->getValue();
+            }
+
+            return $possibleMethodNames;
         }
 
-        return $call->name->toString();
+        return [$call->name->toString()];
     }
 
     /**

tests/Rule/DeadMethodRuleTest.php

@@ -55,7 +55,7 @@ protected function getCollectors(): array
         return [
             new ClassDefinitionCollector(),
             new MethodDefinitionCollector(),
-            new MethodCallCollector(self::getContainer()->getByType(ReflectionProvider::class)),
+            new MethodCallCollector(),
         ];
     }
 
@@ -105,8 +105,10 @@ public static function provideFiles(): iterable
         yield 'parent-call-3' => [__DIR__ . '/data/DeadMethodRule/parent-call-3.php'];
         yield 'parent-call-4' => [__DIR__ . '/data/DeadMethodRule/parent-call-4.php'];
         yield 'attribute' => [__DIR__ . '/data/DeadMethodRule/attribute.php'];
+        yield 'dynamic-method' => [__DIR__ . '/data/DeadMethodRule/dynamic-method.php'];
         yield 'call-on-class-string' => [__DIR__ . '/data/DeadMethodRule/class-string.php'];
         yield 'array-map-1' => [__DIR__ . '/data/DeadMethodRule/array-map-1.php'];
+        yield 'unknown-class' => [__DIR__ . '/data/DeadMethodRule/unknown-class.php'];
         yield 'provider-default' => [__DIR__ . '/data/DeadMethodRule/providers/default.php'];
         yield 'provider-symfony' => [__DIR__ . '/data/DeadMethodRule/providers/symfony.php', 80_000];
         yield 'provider-phpunit' => [__DIR__ . '/data/DeadMethodRule/providers/phpunit.php', 80_000];

tests/Rule/data/DeadMethodRule/dynamic-method.php

@@ -0,0 +1,26 @@
+<?php
+
+namespace DynamicMethod;
+
+class Test {
+
+    public static function a(): void {}
+    public static function b(): void {}
+
+    public function c(): void {}
+    public function d(): void {}
+}
+
+/**
+ * @param 'a'|'b' $method
+ */
+function test(string $method): void {
+    Test::$method();
+}
+
+/**
+ * @param 'c'|'d'|'e' $method
+ */
+function test2(Test $test, string $method): void {
+    $test->$method();
+}

tests/Rule/data/DeadMethodRule/unknown-class.php

@@ -0,0 +1,5 @@
+<?php
+
+namespace UnknownClass;
+
+Unknown::notFailing();