Move most reflection work to collectors (#47)

shipmonk-rnd · Aug 29, 2024 · 89a38bd · 89a38bd
1 parent 712aa81
commit 89a38bd
Show file tree

Hide file tree

Showing 6 changed files with 114 additions and 110 deletions.
diff --git a/src/Collector/ClassDefinitionCollector.php b/src/Collector/ClassDefinitionCollector.php
@@ -8,7 +8,7 @@
 use PHPStan\Node\InClassNode;
 
 /**
- * @implements Collector<InClassNode, array{string}>
+ * @implements Collector<InClassNode, array<string, string>>
  */
 class ClassDefinitionCollector implements Collector
 {
@@ -20,14 +20,25 @@ public function getNodeType(): string
 
     /**
      * @param InClassNode $node
-     * @return array{string}
+     * @return array<string, string>
      */
     public function processNode(
         Node $node,
         Scope $scope
     ): array
     {
-        return [$node->getClassReflection()->getName()];
+        $pairs = [];
+        $origin = $node->getClassReflection();
+
+        foreach ($origin->getAncestors() as $ancestor) {
+            if ($ancestor->isTrait() || $ancestor === $origin) {
+                continue;
+            }
+
+            $pairs[$ancestor->getName()] = $origin->getName();
+        }
+
+        return $pairs;
     }
 
 }
diff --git a/src/Collector/MethodDefinitionCollector.php b/src/Collector/MethodDefinitionCollector.php
@@ -10,7 +10,7 @@
 use function strpos;
 
 /**
- * @implements Collector<InClassNode, list<array{string, int}>>
+ * @implements Collector<InClassNode, list<array{line: int, methodKey: string, overrides: array<string, string>, traitOrigin: ?string}>>
  */
 class MethodDefinitionCollector implements Collector
 {
@@ -22,7 +22,7 @@ public function getNodeType(): string
 
     /**
      * @param InClassNode $node
-     * @return list<array{string, int}>|null
+     * @return list<array{line: int, methodKey: string, overrides: array<string, string>, traitOrigin: ?string}>|null
      */
     public function processNode(
         Node $node,
@@ -33,6 +33,10 @@ public function processNode(
         $nativeReflection = $reflection->getNativeReflection();
         $result = [];
 
+        if ($reflection->isAnonymous()) {
+            return null; // https://github.com/phpstan/phpstan/issues/8410
+        }
+
         foreach ($nativeReflection->getMethods() as $method) {
             if ($method->isDestructor()) {
                 continue;
@@ -64,8 +68,37 @@ public function processNode(
                 continue;
             }
 
-            $methodKey = DeadCodeHelper::composeMethodKey($method->getDeclaringClass()->getName(), $method->getName());
-            $result[] = [$methodKey, $line];
+            $className = $method->getDeclaringClass()->getName();
+            $methodName = $method->getName();
+            $methodKey = DeadCodeHelper::composeMethodKey($className, $methodName);
+
+            $declaringTraitMethodKey = DeadCodeHelper::getDeclaringTraitMethodKey($reflection, $methodName);
+
+            $methodOverrides = [];
+
+            foreach ($reflection->getAncestors() as $ancestor) {
+                if ($ancestor === $reflection) {
+                    continue;
+                }
+
+                if (!$ancestor->hasMethod($methodName)) {
+                    continue;
+                }
+
+                if ($ancestor->isTrait()) {
+                    continue;
+                }
+
+                $ancestorMethodKey = DeadCodeHelper::composeMethodKey($ancestor->getName(), $methodName);
+                $methodOverrides[$ancestorMethodKey] = $methodKey;
+            }
+
+            $result[] = [
+                'line' => $line,
+                'methodKey' => $methodKey,
+                'overrides' => $methodOverrides,
+                'traitOrigin' => $declaringTraitMethodKey,
+            ];
         }
 
         return $result !== [] ? $result : null;

diff --git a/src/Reflection/ClassHierarchy.php b/src/Reflection/ClassHierarchy.php
@@ -2,7 +2,6 @@
 
 namespace ShipMonk\PHPStan\DeadCode\Reflection;
 
-use PHPStan\Reflection\ClassReflection;
 use function array_keys;
 
 class ClassHierarchy
@@ -16,32 +15,48 @@ class ClassHierarchy
     private array $classDescendants = [];
 
     /**
-     * parentMethodKey => childrenMethodKey[] that can mark parent as used
+     * parentMethodKey => childrenMethodKey[]
      *
      * @var array<string, list<string>>
      */
     private array $methodDescendants = [];
 
+    /**
+     * childrenMethodKey => parentMethodKey[]
+     *
+     * @var array<string, list<string>>
+     */
+    private array $methodAncestors = [];
+
     /**
      * traitMethodKey => traitUserMethodKey[]
      *
      * @var array<string, list<string>>
      */
     private array $methodTraitUsages = [];
 
-    public function registerClassPair(ClassReflection $ancestor, ClassReflection $descendant): void
+    /**
+     * traitUserMethodKey => declaringTraitMethodKey
+     *
+     * @var array<string, string>
+     */
+    private array $declaringTraits = [];
+
+    public function registerClassPair(string $ancestorName, string $descendantName): void
     {
-        $this->classDescendants[$ancestor->getName()][$descendant->getName()] = true;
+        $this->classDescendants[$ancestorName][$descendantName] = true;
     }
 
     public function registerMethodPair(string $ancestorMethodKey, string $descendantMethodKey): void
     {
         $this->methodDescendants[$ancestorMethodKey][] = $descendantMethodKey;
+        $this->methodAncestors[$descendantMethodKey][] = $ancestorMethodKey;
     }
 
-    public function registerMethodTraitUsage(string $declaringTraitMethodKey, string $methodKey): void
+    public function registerMethodTraitUsage(string $declaringTraitMethodKey, string $traitUsageMethodKey): void
     {
-        $this->methodTraitUsages[$declaringTraitMethodKey][] = $methodKey;
+        $this->methodTraitUsages[$declaringTraitMethodKey][] = $traitUsageMethodKey;
+        $this->declaringTraits[$traitUsageMethodKey] = $declaringTraitMethodKey;
     }
 
     /**
@@ -62,6 +77,14 @@ public function getMethodDescendants(string $methodKey): array
         return $this->methodDescendants[$methodKey] ?? [];
     }
 
+    /**
+     * @return list<string>
+     */
+    public function getMethodAncestors(string $methodKey): array
+    {
+        return $this->methodAncestors[$methodKey] ?? [];
+    }
+
     /**
      * @return list<string>
      */
@@ -70,4 +93,9 @@ public function getMethodTraitUsages(string $traitMethodKey): array
         return $this->methodTraitUsages[$traitMethodKey] ?? [];
     }
 
+    public function getDeclaringTraitMethodKey(string $methodKey): ?string
+    {
+        return $this->declaringTraits[$methodKey] ?? null;
+    }
+
 }
diff --git a/src/Rule/DeadMethodRule.php b/src/Rule/DeadMethodRule.php
@@ -78,23 +78,32 @@ public function processNode(
         $declaredMethods = [];
 
         foreach ($classDeclarationData as $file => $classesInFile) {
-            foreach ($classesInFile as $classes) {
-                foreach ($classes as $declaredClassName) {
-                    $this->registerClassToHierarchy($declaredClassName);
+            foreach ($classesInFile as $classPairs) {
+                foreach ($classPairs as $ancestor => $descendant) {
+                    $this->classHierarchy->registerClassPair($ancestor, $descendant);
                 }
             }
         }
 
+        unset($classDeclarationData);
+
         foreach ($methodDeclarationData as $file => $methodsInFile) {
             foreach ($methodsInFile as $declared) {
-                foreach ($declared as [$declaredMethodKey, $line]) {
-                    if ($this->isAnonymousClass($declaredMethodKey)) {
-                        continue;
+                foreach ($declared as [
+                    'line' => $line,
+                    'methodKey' => $methodKey,
+                    'overrides' => $methodOverrides,
+                    'traitOrigin' => $declaringTraitMethodKey,
+                ]) {
+                    $declaredMethods[$methodKey] = [$file, $line];
+
+                    if ($declaringTraitMethodKey !== null) {
+                        $this->classHierarchy->registerMethodTraitUsage($declaringTraitMethodKey, $methodKey);
                     }
 
-                    $declaredMethods[$declaredMethodKey] = [$file, $line];
-
-                    $this->fillHierarchy($declaredMethodKey);
+                    foreach ($methodOverrides as $ancestorMethodKey => $descendantMethodKey) {
+                        $this->classHierarchy->registerMethodPair($ancestorMethodKey, $descendantMethodKey);
+                    }
                 }
             }
         }
@@ -139,79 +148,16 @@ private function isAnonymousClass(string $methodKey): bool
      */
     private function getMethodsToMarkAsUsed(string $methodKey): array
     {
-        $classAndMethod = DeadCodeHelper::splitMethodKey($methodKey);
-
-        if (!$this->reflectionProvider->hasClass($classAndMethod->className)) {
-            return []; // e.g. attributes
-        }
-
-        $reflection = $this->reflectionProvider->getClass($classAndMethod->className);
-        $traitMethodKey = DeadCodeHelper::getDeclaringTraitMethodKey($reflection, $classAndMethod->methodName);
-
-        $result = array_merge(
-            $this->getDescendantsToMarkAsUsed($methodKey),
-            $this->getTraitUsersToMarkAsUsed($traitMethodKey),
+        $traitMethodKey = $this->classHierarchy->getDeclaringTraitMethodKey($methodKey);
+
+        return array_merge(
+            [$methodKey],
+            $this->classHierarchy->getMethodDescendants($methodKey),
+            $this->classHierarchy->getMethodAncestors($methodKey),
+            $traitMethodKey !== null
+                ? $this->classHierarchy->getMethodTraitUsages($traitMethodKey)
+                : [],
         );
-
-        foreach ($reflection->getAncestors() as $ancestor) {
-            if (!$ancestor->hasMethod($classAndMethod->methodName)) {
-                continue;
-            }
-
-            $ancestorMethodKey = DeadCodeHelper::composeMethodKey($ancestor->getName(), $classAndMethod->methodName);
-            $result[] = $ancestorMethodKey;
-        }
-
-        return $result;
-    }
-
-    /**
-     * @return list<string>
-     */
-    private function getTraitUsersToMarkAsUsed(?string $traitMethodKey): array
-    {
-        if ($traitMethodKey === null) {
-            return [];
-        }
-
-        return $this->classHierarchy->getMethodTraitUsages($traitMethodKey);
-    }
-
-    /**
-     * @return list<string>
-     */
-    private function getDescendantsToMarkAsUsed(string $methodKey): array
-    {
-        return $this->classHierarchy->getMethodDescendants($methodKey);
-    }
-
-    private function fillHierarchy(string $methodKey): void
-    {
-        $classAndMethod = DeadCodeHelper::splitMethodKey($methodKey);
-        $reflection = $this->reflectionProvider->getClass($classAndMethod->className);
-
-        $declaringTraitMethodKey = DeadCodeHelper::getDeclaringTraitMethodKey($reflection, $classAndMethod->methodName);
-
-        if ($declaringTraitMethodKey !== null) {
-            $this->classHierarchy->registerMethodTraitUsage($declaringTraitMethodKey, $methodKey);
-        }
-
-        foreach ($reflection->getAncestors() as $ancestor) {
-            if ($ancestor === $reflection) {
-                continue;
-            }
-
-            if (!$ancestor->hasMethod($classAndMethod->methodName)) {
-                continue;
-            }
-
-            if ($ancestor->isTrait()) {
-                continue;
-            }
-
-            $ancestorMethodKey = DeadCodeHelper::composeMethodKey($ancestor->getName(), $classAndMethod->methodName);
-            $this->classHierarchy->registerMethodPair($ancestorMethodKey, $methodKey);
-        }
     }
 
     private function raiseError(
@@ -264,19 +210,4 @@ private function isEntryPoint(string $methodKey): bool
         return false;
     }
 
-    private function registerClassToHierarchy(string $className): void
-    {
-        if ($this->reflectionProvider->hasClass($className)) {
-            $origin = $this->reflectionProvider->getClass($className);
-
-            foreach ($origin->getAncestors() as $ancestor) {
-                if ($ancestor->isTrait() || $ancestor === $origin) {
-                    continue;
-                }
-
-                $this->classHierarchy->registerClassPair($ancestor, $origin);
-            }
-        }
-    }
-
 }
diff --git a/tests/Rule/data/DeadMethodRule/attribute.php b/tests/Rule/data/DeadMethodRule/attribute.php
@@ -12,6 +12,7 @@ public function __construct(string $name)
 }
 
 #[Attr("arg")]
+#[Unknown("arg")]
 class AttrUser
 {
 

diff --git a/tests/Rule/data/DeadMethodRule/indirect-interface.php b/tests/Rule/data/DeadMethodRule/indirect-interface.php
@@ -4,7 +4,7 @@
 
 interface FooInterface
 {
-    public function foo(): void; // error: Unused DeadIndirect\FooInterface::foo
+    public function foo(): void;
 }
 
 abstract class FooAbstract
-Original file line number
+Diff line change
@@ Expand Up / @@ -12,6 +12,7 @@ public function __construct(string $name) @@
     }
     #[Attr("arg")]
+    #[Unknown("arg")]
     class AttrUser
     {
@@ Expand Down @@