Call graph (#99)

Author: janedbal

README.md

@@ -89,6 +89,32 @@ class ApiOutputEntrypointProvider extends SimpleMethodEntrypointProvider
 }
 ```
 
+## Dead cycles & transitively dead methods
+- This library automatically detects dead cycles and transitively dead methods (methods that are only called from dead methods)
+- By default, it reports only the first dead method in the subtree and the rest as a tip:
+
+```
+ ------ ------------------------------------------------------------------------
+  Line   src/App/Facade/UserFacade.php
+ ------ ------------------------------------------------------------------------
+  26     Unused App\Facade\UserFacade::updateUserAddress
+         🪪  shipmonk.deadMethod
+         💡 Thus App\Entity\User::updateAddress is transitively also unused
+         💡 Thus App\Entity\Address::setPostalCode is transitively also unused
+         💡 Thus App\Entity\Address::setCountry is transitively also unused
+         💡 Thus App\Entity\Address::setStreet is transitively also unused
+         💡 Thus App\Entity\Address::setZip is transitively also unused
+ ------ ------------------------------------------------------------------------
+```
+
+- If you want to report all dead methods individually, you can enable it in your `phpstan.neon.dist`:
+
+```neon
+parameters:
+    shipmonkDeadCode:
+        reportTransitivelyDeadMethodAsSeparateError: true
+```
+
 ## Comparison with tomasvotruba/unused-public
 - You can see [detailed comparison PR](https://github.com/shipmonk-rnd/dead-code-detector/pull/53)
 - Basically, their analysis is less precise and less flexible. Mainly:
@@ -104,11 +130,8 @@ class ApiOutputEntrypointProvider extends SimpleMethodEntrypointProvider
 - Only method calls are detected so far
   - Including **constructors**, static methods, trait methods, interface methods, first class callables, clone, etc.
   - Any calls on mixed types are not detected, e.g. `$unknownClass->method()`
-  - Anonymous classes are ignored ([PHPStan limitation](https://github.com/phpstan/phpstan/issues/8410))
-  - Does not check most magic methods (`__get`, `__set` etc)
-  - Call-graph not implemented so far
-    - No transitive check is performed (dead method called only from dead method)
-    - No dead cycles are detected (e.g. dead method calling itself)
+  - Methods of anonymous classes are never reported as dead ([PHPStan limitation](https://github.com/phpstan/phpstan/issues/8410))
+  - Most magic methods (e.g. `__get`, `__set` etc) are never reported as dead
 
 ## Contributing
 - Check your code by `composer check`

rules.neon

@@ -64,10 +64,13 @@ services:
         class: ShipMonk\PHPStan\DeadCode\Rule\DeadMethodRule
         tags:
             - phpstan.rules.rule
+        arguments:
+            reportTransitivelyDeadMethodAsSeparateError: %shipmonkDeadCode.reportTransitivelyDeadMethodAsSeparateError%
 
 
 parameters:
     shipmonkDeadCode:
+        reportTransitivelyDeadMethodAsSeparateError: false
         entrypoints:
             vendor:
                 enabled: true
@@ -84,6 +87,7 @@ parameters:
 
 parametersSchema:
     shipmonkDeadCode: structure([
+        reportTransitivelyDeadMethodAsSeparateError: bool()
         entrypoints: structure([
             vendor: structure([
                 enabled: bool()

src/Collector/ClassDefinitionCollector.php

@@ -7,7 +7,6 @@
 use PhpParser\Node\Name;
 use PhpParser\Node\Stmt\Class_;
 use PhpParser\Node\Stmt\ClassLike;
-use PhpParser\Node\Stmt\ClassMethod;
 use PhpParser\Node\Stmt\Enum_;
 use PhpParser\Node\Stmt\Interface_;
 use PhpParser\Node\Stmt\Trait_;
@@ -16,15 +15,15 @@
 use PHPStan\Analyser\Scope;
 use PHPStan\Collectors\Collector;
 use ShipMonk\PHPStan\DeadCode\Crate\Kind;
+use ShipMonk\PHPStan\DeadCode\Crate\Visibility;
 use function array_fill_keys;
 use function array_map;
-use function strpos;
 
 /**
  * @implements Collector<ClassLike, array{
  *       kind: string,
  *       name: string,
- *       methods: array<string, array{line: int, abstract: bool}>,
+ *       methods: array<string, array{line: int, abstract: bool, visibility: int-mask-of<Visibility::*>}>,
  *       parents: array<string, null>,
  *       traits: array<string, array{excluded?: list<string>, aliases?: array<string, string>}>,
  *       interfaces: array<string, null>,
@@ -43,7 +42,7 @@ public function getNodeType(): string
      * @return array{
      *      kind: string,
      *      name: string,
-     *      methods: array<string, array{line: int, abstract: bool}>,
+     *      methods: array<string, array{line: int, abstract: bool, visibility: int-mask-of<Visibility::*>}>,
      *      parents: array<string, null>,
      *      traits: array<string, array{excluded?: list<string>, aliases?: array<string, string>}>,
      *      interfaces: array<string, null>,
@@ -64,13 +63,10 @@ public function processNode(
         $methods = [];
 
         foreach ($node->getMethods() as $method) {
-            if ($this->isUnsupportedMethod($node, $method)) {
-                continue;
-            }
-
             $methods[$method->name->toString()] = [
                 'line' => $method->getStartLine(),
                 'abstract' => $method->isAbstract() || $node instanceof Interface_,
+                'visibility' => $method->flags & (Visibility::PUBLIC | Visibility::PROTECTED | Visibility::PRIVATE),
             ];
         }
 
@@ -163,35 +159,6 @@ private function getTraits(ClassLike $node): array
         return $traits;
     }
 
-    private function isUnsupportedMethod(ClassLike $class, ClassMethod $method): bool
-    {
-        $methodName = $method->name->toString();
-
-        if ($methodName === '__destruct') {
-            return true;
-        }
-
-        if (
-            strpos($methodName, '__') === 0
-            && $methodName !== '__construct'
-            && $methodName !== '__clone'
-        ) {
-            return true; // magic methods like __toString, __get, __set etc
-        }
-
-        if ($methodName === '__construct' && $method->isPrivate()) { // e.g. classes with "denied" instantiation
-            return true;
-        }
-
-        // abstract methods in traits make sense (not dead) only when called within the trait itself, but that is hard to detect for now, so lets ignore them completely
-        // the difference from interface methods (or abstract methods) is that those methods can be called over the interface, but you cannot call method over trait
-        if ($class instanceof Trait_ && $method->isAbstract()) {
-            return true;
-        }
-
-        return false;
-    }
-
     private function getKind(ClassLike $node): string
     {
         if ($node instanceof Class_) {

src/Collector/EntrypointCollector.php

@@ -7,6 +7,7 @@
 use PHPStan\Collectors\Collector;
 use PHPStan\Node\InClassNode;
 use ShipMonk\PHPStan\DeadCode\Crate\Call;
+use ShipMonk\PHPStan\DeadCode\Crate\Method;
 use ShipMonk\PHPStan\DeadCode\Provider\MethodEntrypointProvider;
 
 /**
@@ -48,7 +49,12 @@ public function processNode(
 
         foreach ($this->entrypointProviders as $entrypointProvider) {
             foreach ($entrypointProvider->getEntrypoints($node->getClassReflection()) as $entrypointMethod) {
-                $entrypoints[] = (new Call($entrypointMethod->getDeclaringClass()->getName(), $entrypointMethod->getName(), false))->toString();
+                $call = new Call(
+                    null,
+                    new Method($entrypointMethod->getDeclaringClass()->getName(), $entrypointMethod->getName()),
+                    false,
+                );
+                $entrypoints[] = $call->toString();
             }
         }
 

src/Collector/MethodCallCollector.php

@@ -19,9 +19,11 @@
 use PHPStan\Node\MethodCallableNode;
 use PHPStan\Node\StaticMethodCallableNode;
 use PHPStan\Reflection\ClassReflection;
+use PHPStan\Reflection\MethodReflection;
 use PHPStan\Type\Type;
 use PHPStan\Type\TypeCombinator;
 use ShipMonk\PHPStan\DeadCode\Crate\Call;
+use ShipMonk\PHPStan\DeadCode\Crate\Method;
 use function array_map;
 
 /**
@@ -126,7 +128,11 @@ private function registerMethodCall(
                 }
 
                 $className = $classWithMethod->getMethod($methodName, $scope)->getDeclaringClass()->getName();
-                $this->callsBuffer[] = new Call($className, $methodName, $possibleDescendantCall);
+                $this->callsBuffer[] = new Call(
+                    $this->getCaller($scope),
+                    new Method($className, $methodName),
+                    $possibleDescendantCall,
+                );
             }
         }
     }
@@ -154,7 +160,11 @@ private function registerStaticCall(
                 }
 
                 $className = $classReflection->getMethod($methodName, $scope)->getDeclaringClass()->getName();
-                $this->callsBuffer[] = new Call($className, $methodName, $possibleDescendantCall);
+                $this->callsBuffer[] = new Call(
+                    $this->getCaller($scope),
+                    new Method($className, $methodName),
+                    $possibleDescendantCall,
+                );
             }
         }
     }
@@ -177,7 +187,11 @@ private function registerArrayCallable(
 
                     foreach ($this->getReflectionsWithMethod($caller, $methodName) as $classWithMethod) {
                         $className = $classWithMethod->getMethod($methodName, $scope)->getDeclaringClass()->getName();
-                        $this->callsBuffer[] = new Call($className, $methodName, $possibleDescendantCall);
+                        $this->callsBuffer[] = new Call(
+                            $this->getCaller($scope),
+                            new Method($className, $methodName),
+                            $possibleDescendantCall,
+                        );
                     }
                 }
             }
@@ -186,7 +200,11 @@ private function registerArrayCallable(
 
     private function registerAttribute(Attribute $node, Scope $scope): void
     {
-        $this->callsBuffer[] = new Call($scope->resolveName($node->name), '__construct', false);
+        $this->callsBuffer[] = new Call(
+            null,
+            new Method($scope->resolveName($node->name), '__construct'),
+            false,
+        );
     }
 
     private function registerClone(Clone_ $node, Scope $scope): void
@@ -196,7 +214,11 @@ private function registerClone(Clone_ $node, Scope $scope): void
 
         foreach ($this->getReflectionsWithMethod($callerType, $methodName) as $classWithMethod) {
             $className = $classWithMethod->getMethod($methodName, $scope)->getDeclaringClass()->getName();
-            $this->callsBuffer[] = new Call($className, $methodName, true);
+            $this->callsBuffer[] = new Call(
+                $this->getCaller($scope),
+                new Method($className, $methodName),
+                true,
+            );
         }
     }
 
@@ -239,4 +261,17 @@ private function getReflectionsWithMethod(Type $type, string $methodName): itera
         }
     }
 
+    private function getCaller(Scope $scope): ?Method
+    {
+        if (!$scope->isInClass()) {
+            return null;
+        }
+
+        if (!$scope->getFunction() instanceof MethodReflection) {
+            return null;
+        }
+
+        return new Method($scope->getClassReflection()->getName(), $scope->getFunction()->getName());
+    }
+
 }

src/Crate/Call.php

@@ -7,43 +7,81 @@
 use function explode;
 
 /**
- * @readonly
+ * @immutable
  */
 class Call
 {
 
-    public string $className;
+    public ?Method $caller;
 
-    public string $methodName;
+    public Method $callee;
 
     public bool $possibleDescendantCall;
 
     public function __construct(
-        string $className,
-        string $methodName,
+        ?Method $caller,
+        Method $callee,
         bool $possibleDescendantCall
     )
     {
-        $this->className = $className;
-        $this->methodName = $methodName;
+        $this->caller = $caller;
+        $this->callee = $callee;
         $this->possibleDescendantCall = $possibleDescendantCall;
     }
 
     public function toString(): string
     {
-        return "{$this->className}::{$this->methodName}::" . ($this->possibleDescendantCall ? '1' : '');
+        $callerRef = $this->caller === null ? '' : "{$this->caller->className}::{$this->caller->methodName}";
+        $calleeRef = "{$this->callee->className}::{$this->callee->methodName}";
+
+        return "{$callerRef}->$calleeRef;" . ($this->possibleDescendantCall ? '1' : '');
     }
 
-    public static function fromString(string $methodKey): self
+    public static function fromString(string $callKey): self
     {
-        $exploded = explode('::', $methodKey);
+        $split1 = explode(';', $callKey);
+
+        if (count($split1) !== 2) {
+            throw new LogicException("Invalid method key: $callKey");
+        }
+
+        [$edgeKey, $possibleDescendantCall] = $split1;
+
+        $split2 = explode('->', $edgeKey);
+
+        if (count($split2) !== 2) {
+            throw new LogicException("Invalid method key: $callKey");
+        }
+
+        [$callerKey, $calleeKey] = $split2;
+
+        $calleeSplit = explode('::', $calleeKey);
+
+        if (count($calleeSplit) !== 2) {
+            throw new LogicException("Invalid method key: $callKey");
+        }
+
+        [$calleeClassName, $calleeMethodName] = $calleeSplit;
+        $callee = new Method($calleeClassName, $calleeMethodName);
+
+        if ($callerKey === '') {
+            $caller = null;
+        } else {
+            $callerSplit = explode('::', $callerKey);
+
+            if (count($callerSplit) !== 2) {
+                throw new LogicException("Invalid method key: $callKey");
+            }
 
-        if (count($exploded) !== 3) {
-            throw new LogicException("Invalid method key: $methodKey");
+            [$callerClassName, $callerMethodName] = $callerSplit;
+            $caller = new Method($callerClassName, $callerMethodName);
         }
 
-        [$className, $methodName, $possibleDescendantCall] = $exploded;
-        return new self($className, $methodName, $possibleDescendantCall === '1');
+        return new self(
+            $caller,
+            $callee,
+            $possibleDescendantCall === '1',
+        );
     }
 
 }

src/Crate/Method.php

@@ -0,0 +1,29 @@
+<?php declare(strict_types = 1);
+
+namespace ShipMonk\PHPStan\DeadCode\Crate;
+
+/**
+ * @immutable
+ */
+class Method
+{
+
+    public string $className;
+
+    public string $methodName;
+
+    public function __construct(
+        string $className,
+        string $methodName
+    )
+    {
+        $this->className = $className;
+        $this->methodName = $methodName;
+    }
+
+    public function toString(): string
+    {
+        return $this->className . '::' . $this->methodName;
+    }
+
+}