Reflection: proper dead transitivity (#135)

shipmonk-rnd · Dec 30, 2024 · fe15bef · fe15bef
1 parent 3e8c4f1
commit fe15bef
Show file tree

Hide file tree

Showing 8 changed files with 101 additions and 57 deletions.
diff --git a/rules.neon b/rules.neon
@@ -7,6 +7,9 @@ services:
     -
         class: ShipMonk\PHPStan\DeadCode\Transformer\FileSystem
 
+    -
+        class: ShipMonk\PHPStan\DeadCode\Graph\UsageOriginDetector
+
     -
         class: ShipMonk\PHPStan\DeadCode\Provider\VendorUsageProvider
         tags:

diff --git a/src/Collector/ConstantFetchCollector.php b/src/Collector/ConstantFetchCollector.php
@@ -10,14 +10,13 @@
 use PhpParser\Node\Name;
 use PHPStan\Analyser\Scope;
 use PHPStan\Collectors\Collector;
-use PHPStan\Reflection\MethodReflection;
 use PHPStan\Reflection\ReflectionProvider;
 use PHPStan\Type\Constant\ConstantStringType;
 use PHPStan\Type\Type;
 use PHPStan\Type\TypeUtils;
 use ShipMonk\PHPStan\DeadCode\Graph\ClassConstantRef;
 use ShipMonk\PHPStan\DeadCode\Graph\ClassConstantUsage;
-use ShipMonk\PHPStan\DeadCode\Graph\ClassMethodRef;
+use ShipMonk\PHPStan\DeadCode\Graph\UsageOriginDetector;
 use function array_map;
 use function count;
 use function current;
@@ -32,17 +31,21 @@ class ConstantFetchCollector implements Collector
 
     use BufferedUsageCollector;
 
+    private UsageOriginDetector $usageOriginDetector;
+
     private ReflectionProvider $reflectionProvider;
 
     private bool $trackMixedAccess;
 
     public function __construct(
+        UsageOriginDetector $usageOriginDetector,
         ReflectionProvider $reflectionProvider,
         bool $trackMixedAccess
     )
     {
         $this->reflectionProvider = $reflectionProvider;
         $this->trackMixedAccess = $trackMixedAccess;
+        $this->usageOriginDetector = $usageOriginDetector;
     }
 
     public function getNodeType(): string
@@ -109,7 +112,7 @@ private function registerFunctionCall(FuncCall $node, Scope $scope): void
                 }
 
                 $this->usageBuffer[] = new ClassConstantUsage(
-                    $this->getCaller($scope),
+                    $this->usageOriginDetector->detectOrigin($scope),
                     new ClassConstantRef($className, $constantName, true),
                 );
             }
@@ -137,7 +140,7 @@ private function registerFetch(ClassConstFetch $node, Scope $scope): void
 
             foreach ($this->getDeclaringTypesWithConstant($ownerType, $constantName) as $className) {
                 $this->usageBuffer[] = new ClassConstantUsage(
-                    $this->getCaller($scope),
+                    $this->usageOriginDetector->detectOrigin($scope),
                     new ClassConstantRef($className, $constantName, $possibleDescendantFetch),
                 );
             }
@@ -173,21 +176,4 @@ private function getDeclaringTypesWithConstant(
         return $result;
     }
 
-    private function getCaller(Scope $scope): ?ClassMethodRef
-    {
-        if (!$scope->isInClass()) {
-            return null;
-        }
-
-        if (!$scope->getFunction() instanceof MethodReflection) {
-            return null;
-        }
-
-        return new ClassMethodRef(
-            $scope->getClassReflection()->getName(),
-            $scope->getFunction()->getName(),
-            false,
-        );
-    }
-
 }
diff --git a/src/Collector/MethodCallCollector.php b/src/Collector/MethodCallCollector.php
@@ -17,13 +17,13 @@
 use PHPStan\Collectors\Collector;
 use PHPStan\Node\MethodCallableNode;
 use PHPStan\Node\StaticMethodCallableNode;
-use PHPStan\Reflection\MethodReflection;
 use PHPStan\TrinaryLogic;
 use PHPStan\Type\Type;
 use PHPStan\Type\TypeCombinator;
 use PHPStan\Type\TypeUtils;
 use ShipMonk\PHPStan\DeadCode\Graph\ClassMethodRef;
 use ShipMonk\PHPStan\DeadCode\Graph\ClassMethodUsage;
+use ShipMonk\PHPStan\DeadCode\Graph\UsageOriginDetector;
 
 /**
  * @implements Collector<Node, list<string>>
@@ -33,10 +33,16 @@ class MethodCallCollector implements Collector
 
     use BufferedUsageCollector;
 
+    private UsageOriginDetector $usageOriginDetector;
+
     private bool $trackMixedAccess;
 
-    public function __construct(bool $trackMixedAccess)
+    public function __construct(
+        UsageOriginDetector $usageOriginDetector,
+        bool $trackMixedAccess
+    )
     {
+        $this->usageOriginDetector = $usageOriginDetector;
         $this->trackMixedAccess = $trackMixedAccess;
     }
 
@@ -114,7 +120,7 @@ private function registerMethodCall(
         foreach ($methodNames as $methodName) {
             foreach ($this->getDeclaringTypesWithMethod($scope, $callerType, $methodName, TrinaryLogic::createNo()) as $className) {
                 $this->usageBuffer[] = new ClassMethodUsage(
-                    $this->getCaller($scope),
+                    $this->usageOriginDetector->detectOrigin($scope),
                     new ClassMethodRef($className, $methodName, $possibleDescendantCall),
                 );
             }
@@ -140,7 +146,7 @@ private function registerStaticCall(
         foreach ($methodNames as $methodName) {
             foreach ($this->getDeclaringTypesWithMethod($scope, $callerType, $methodName, TrinaryLogic::createYes()) as $className) {
                 $this->usageBuffer[] = new ClassMethodUsage(
-                    $this->getCaller($scope),
+                    $this->usageOriginDetector->detectOrigin($scope),
                     new ClassMethodRef($className, $methodName, $possibleDescendantCall),
                 );
             }
@@ -165,7 +171,7 @@ private function registerArrayCallable(
 
                     foreach ($this->getDeclaringTypesWithMethod($scope, $caller, $methodName, TrinaryLogic::createMaybe()) as $className) {
                         $this->usageBuffer[] = new ClassMethodUsage(
-                            $this->getCaller($scope),
+                            $this->usageOriginDetector->detectOrigin($scope),
                             new ClassMethodRef($className, $methodName, $possibleDescendantCall),
                         );
                     }
@@ -189,7 +195,7 @@ private function registerClone(Clone_ $node, Scope $scope): void
 
         foreach ($this->getDeclaringTypesWithMethod($scope, $callerType, $methodName, TrinaryLogic::createNo()) as $className) {
             $this->usageBuffer[] = new ClassMethodUsage(
-                $this->getCaller($scope),
+                $this->usageOriginDetector->detectOrigin($scope),
                 new ClassMethodRef($className, $methodName, true),
             );
         }
@@ -255,21 +261,4 @@ private function getDeclaringTypesWithMethod(
         return $result;
     }
 
-    private function getCaller(Scope $scope): ?ClassMethodRef
-    {
-        if (!$scope->isInClass()) {
-            return null;
-        }
-
-        if (!$scope->getFunction() instanceof MethodReflection) {
-            return null;
-        }
-
-        return new ClassMethodRef(
-            $scope->getClassReflection()->getName(),
-            $scope->getFunction()->getName(),
-            false,
-        );
-    }
-
 }
diff --git a/src/Graph/ClassMemberUsage.php b/src/Graph/ClassMemberUsage.php
@@ -17,6 +17,9 @@ abstract class ClassMemberUsage
 
     /**
      * Origin method of the usage, "where it was called from"
+     * This is required for proper transitive dead code detection.
+     *
+     * @see UsageOriginDetector for typical usage
      */
     private ?ClassMethodRef $origin;
 

diff --git a/src/Graph/UsageOriginDetector.php b/src/Graph/UsageOriginDetector.php
@@ -0,0 +1,31 @@
+<?php declare(strict_types = 1);
+
+namespace ShipMonk\PHPStan\DeadCode\Graph;
+
+use PHPStan\Analyser\Scope;
+use PHPStan\Reflection\MethodReflection;
+
+class UsageOriginDetector
+{
+
+    /**
+     * Most of the time, this is the only implementation you need for ClassMemberUsage constructor
+     */
+    public function detectOrigin(Scope $scope): ?ClassMethodRef
+    {
+        if (!$scope->isInClass()) {
+            return null;
+        }
+
+        if (!$scope->getFunction() instanceof MethodReflection) {
+            return null;
+        }
+
+        return new ClassMethodRef(
+            $scope->getClassReflection()->getName(),
+            $scope->getFunction()->getName(),
+            false,
+        );
+    }
+
+}
diff --git a/src/Provider/ReflectionUsageProvider.php b/src/Provider/ReflectionUsageProvider.php
@@ -18,17 +18,24 @@
 use ShipMonk\PHPStan\DeadCode\Graph\ClassMemberUsage;
 use ShipMonk\PHPStan\DeadCode\Graph\ClassMethodRef;
 use ShipMonk\PHPStan\DeadCode\Graph\ClassMethodUsage;
+use ShipMonk\PHPStan\DeadCode\Graph\UsageOriginDetector;
 use function array_key_first;
 use function count;
 use function in_array;
 
 class ReflectionUsageProvider implements MemberUsageProvider
 {
 
+    private UsageOriginDetector $usageOriginDetector;
+
     private bool $enabled;
 
-    public function __construct(bool $enabled)
+    public function __construct(
+        UsageOriginDetector $usageOriginDetector,
+        bool $enabled
+    )
     {
+        $this->usageOriginDetector = $usageOriginDetector;
         $this->enabled = $enabled;
     }
 
@@ -98,15 +105,15 @@ private function extractConstantsUsedByReflection(
 
         if ($methodName === 'getConstants' || $methodName === 'getReflectionConstants') {
             foreach ($genericReflection->getNativeReflection()->getReflectionConstants() as $reflectionConstant) {
-                $usedConstants[] = $this->createConstantUsage($reflectionConstant->getDeclaringClass()->getName(), $reflectionConstant->getName());
+                $usedConstants[] = $this->createConstantUsage($scope, $reflectionConstant->getDeclaringClass()->getName(), $reflectionConstant->getName());
             }
         }
 
         if (($methodName === 'getConstant' || $methodName === 'getReflectionConstant') && count($args) === 1) {
             $firstArg = $args[array_key_first($args)]; // @phpstan-ignore offsetAccess.notFound
 
             foreach ($scope->getType($firstArg->value)->getConstantStrings() as $constantString) {
-                $usedConstants[] = $this->createConstantUsage($genericReflection->getName(), $constantString->getValue());
+                $usedConstants[] = $this->createConstantUsage($scope, $genericReflection->getName(), $constantString->getValue());
             }
         }
 
@@ -128,23 +135,23 @@ private function extractMethodsUsedByReflection(
 
         if ($methodName === 'getMethods') {
             foreach ($genericReflection->getNativeReflection()->getMethods() as $reflectionMethod) {
-                $usedMethods[] = $this->createMethodUsage($reflectionMethod->getDeclaringClass()->getName(), $reflectionMethod->getName());
+                $usedMethods[] = $this->createMethodUsage($scope, $reflectionMethod->getDeclaringClass()->getName(), $reflectionMethod->getName());
             }
         }
 
         if ($methodName === 'getMethod' && count($args) === 1) {
             $firstArg = $args[array_key_first($args)]; // @phpstan-ignore offsetAccess.notFound
 
             foreach ($scope->getType($firstArg->value)->getConstantStrings() as $constantString) {
-                $usedMethods[] = $this->createMethodUsage($genericReflection->getName(), $constantString->getValue());
+                $usedMethods[] = $this->createMethodUsage($scope, $genericReflection->getName(), $constantString->getValue());
             }
         }
 
         if (in_array($methodName, ['getConstructor', 'newInstance', 'newInstanceArgs'], true)) {
             $constructor = $genericReflection->getNativeReflection()->getConstructor();
 
             if ($constructor !== null) {
-                $usedMethods[] = $this->createMethodUsage($constructor->getDeclaringClass()->getName(), '__construct');
+                $usedMethods[] = $this->createMethodUsage($scope, $constructor->getDeclaringClass()->getName(), '__construct');
             }
         }
 
@@ -174,10 +181,10 @@ private function getMethodNames(CallLike $call, Scope $scope): array
         return [$call->name->toString()];
     }
 
-    private function createConstantUsage(string $className, string $constantName): ClassConstantUsage
+    private function createConstantUsage(Scope $scope, string $className, string $constantName): ClassConstantUsage
     {
         return new ClassConstantUsage(
-            null,
+            $this->usageOriginDetector->detectOrigin($scope),
             new ClassConstantRef(
                 $className,
                 $constantName,
@@ -186,10 +193,10 @@ private function createConstantUsage(string $className, string $constantName): C
         );
     }
 
-    private function createMethodUsage(string $className, string $methodName): ClassMethodUsage
+    private function createMethodUsage(Scope $scope, string $className, string $methodName): ClassMethodUsage
     {
         return new ClassMethodUsage(
-            null,
+            $this->usageOriginDetector->detectOrigin($scope),
             new ClassMethodRef(
                 $className,
                 $methodName,

diff --git a/tests/Rule/DeadCodeRuleTest.php b/tests/Rule/DeadCodeRuleTest.php
@@ -19,6 +19,7 @@
 use ShipMonk\PHPStan\DeadCode\Collector\ProvidedUsagesCollector;
 use ShipMonk\PHPStan\DeadCode\Compatibility\BackwardCompatibilityChecker;
 use ShipMonk\PHPStan\DeadCode\Formatter\RemoveDeadCodeFormatter;
+use ShipMonk\PHPStan\DeadCode\Graph\UsageOriginDetector;
 use ShipMonk\PHPStan\DeadCode\Hierarchy\ClassHierarchy;
 use ShipMonk\PHPStan\DeadCode\Provider\DoctrineUsageProvider;
 use ShipMonk\PHPStan\DeadCode\Provider\MemberUsageProvider;
@@ -76,8 +77,8 @@ protected function getCollectors(): array
                 $this->getMemberUsageProviders(),
             ),
             new ClassDefinitionCollector(),
-            new MethodCallCollector($this->trackMixedAccess),
-            new ConstantFetchCollector($reflectionProvider, $this->trackMixedAccess),
+            new MethodCallCollector($this->createUsageOriginDetector(), $this->trackMixedAccess),
+            new ConstantFetchCollector($this->createUsageOriginDetector(), $reflectionProvider, $this->trackMixedAccess),
         ];
     }
 
@@ -368,7 +369,10 @@ private function getTransformedFilePath(string $file): string
     private function getMemberUsageProviders(): array
     {
         return [
-            new ReflectionUsageProvider(true),
+            new ReflectionUsageProvider(
+                $this->createUsageOriginDetector(),
+                true,
+            ),
             new class extends ReflectionBasedMemberUsageProvider
             {
 
@@ -421,6 +425,18 @@ static function (string $type): array {
         return $mock;
     }
 
+    private function createUsageOriginDetector(): UsageOriginDetector
+    {
+        /** @var UsageOriginDetector|null $detector */
+        static $detector = null;
+
+        if ($detector === null) {
+            $detector = new UsageOriginDetector();
+        }
+
+        return $detector;
+    }
+
     public function gatherAnalyserErrors(array $files): array
     {
         if (!$this->unwrapGroupedErrors) {

diff --git a/tests/Rule/data/providers/reflection.php b/tests/Rule/data/providers/reflection.php
@@ -57,6 +57,15 @@ enum EnumHolder1 {
     public function used() {}
 }
 
+enum TransitiveHolder {
+    const TRANSITIVELY_DEAD = 1; // error: Unused Reflection\TransitiveHolder::TRANSITIVELY_DEAD
+
+    public function test() // error: Unused Reflection\TransitiveHolder::test
+    {
+        (new \ReflectionClass(self::class))->getConstant('TRANSITIVELY_DEAD');
+    }
+}
+
 $reflection1 = new \ReflectionClass(Holder1::class);
 $reflection1->getConstants();
 $reflection1->getMethod('used');
-Original file line number
+Diff line change
@@ Expand Up / @@ -7,6 +7,9 @@ services: @@
         -
             class: ShipMonk\PHPStan\DeadCode\Transformer\FileSystem
+        -
+            class: ShipMonk\PHPStan\DeadCode\Graph\UsageOriginDetector
         -
             class: ShipMonk\PHPStan\DeadCode\Provider\VendorUsageProvider
             tags:
@@ Expand Down @@