Fix disabling dead methods detection (#257)

Author: janedbal

rules.neon

@@ -128,7 +128,6 @@ services:
         tags:
             - phpstan.collector
         arguments:
-            detectDeadMethods: %shipmonkDeadCode.detect.deadMethods%
             detectDeadConstants: %shipmonkDeadCode.detect.deadConstants%
             detectDeadEnumCases: %shipmonkDeadCode.detect.deadEnumCases%
 
@@ -146,6 +145,7 @@ services:
             - phpstan.rules.rule
             - phpstan.diagnoseExtension
         arguments:
+            detectDeadMethods: %shipmonkDeadCode.detect.deadMethods%
             reportTransitivelyDeadMethodAsSeparateError: %shipmonkDeadCode.reportTransitivelyDeadMethodAsSeparateError%
 
     -

src/Collector/ClassDefinitionCollector.php

@@ -39,21 +39,17 @@ class ClassDefinitionCollector implements Collector
 
     private ReflectionProvider $reflectionProvider;
 
-    private bool $detectDeadMethods;
-
     private bool $detectDeadConstants;
 
     private bool $detectDeadEnumCases;
 
     public function __construct(
         ReflectionProvider $reflectionProvider,
-        bool $detectDeadMethods,
         bool $detectDeadConstants,
         bool $detectDeadEnumCases
     )
     {
         $this->reflectionProvider = $reflectionProvider;
-        $this->detectDeadMethods = $detectDeadMethods;
         $this->detectDeadConstants = $detectDeadConstants;
         $this->detectDeadEnumCases = $detectDeadEnumCases;
     }
@@ -93,15 +89,13 @@ public function processNode(
         $constants = [];
         $cases = [];
 
-        if ($this->detectDeadMethods) {
-            foreach ($node->getMethods() as $method) {
-                $methods[$method->name->toString()] = [
-                    'line' => $method->name->getStartLine(),
-                    'params' => count($method->params),
-                    'abstract' => $method->isAbstract() || $node instanceof Interface_,
-                    'visibility' => $method->flags & (Visibility::PUBLIC | Visibility::PROTECTED | Visibility::PRIVATE),
-                ];
-            }
+        foreach ($node->getMethods() as $method) {
+            $methods[$method->name->toString()] = [
+                'line' => $method->name->getStartLine(),
+                'params' => count($method->params),
+                'abstract' => $method->isAbstract() || $node instanceof Interface_,
+                'visibility' => $method->flags & (Visibility::PUBLIC | Visibility::PROTECTED | Visibility::PRIVATE),
+            ];
         }
 
         if ($this->detectDeadConstants) {

src/Rule/DeadCodeRule.php

@@ -75,6 +75,8 @@ class DeadCodeRule implements Rule, DiagnoseExtension
 
     private ClassHierarchy $classHierarchy;
 
+    private bool $detectDeadMethods;
+
     /**
      * typename => data
      *
@@ -130,12 +132,14 @@ class DeadCodeRule implements Rule, DiagnoseExtension
     public function __construct(
         DebugUsagePrinter $debugUsagePrinter,
         ClassHierarchy $classHierarchy,
+        bool $detectDeadMethods,
         bool $reportTransitivelyDeadMethodAsSeparateError,
         BackwardCompatibilityChecker $checker
     )
     {
         $this->debugUsagePrinter = $debugUsagePrinter;
         $this->classHierarchy = $classHierarchy;
+        $this->detectDeadMethods = $detectDeadMethods;
         $this->reportTransitivelyDeadAsSeparateError = $reportTransitivelyDeadMethodAsSeparateError;
 
         $checker->check();
@@ -311,6 +315,10 @@ public function processNode(
 
                 unset($this->blackMembers[$blackMemberKey]);
             }
+
+            if (!$this->detectDeadMethods && $blackMember->getMember() instanceof ClassMethodRef) {
+                unset($this->blackMembers[$blackMemberKey]);
+            }
         }
 
         foreach ($excludedMemberUsages as $excludedMemberUsage) {

tests/Rule/DeadCodeRuleTest.php

@@ -117,6 +117,7 @@ protected function getRule(): DeadCodeRule
                     self::createReflectionProvider(),
                 ),
                 new ClassHierarchy(),
+                $this->detectDeadMethods,
                 !$this->emitErrorsInGroups,
                 new BackwardCompatibilityChecker([], null),
             );
@@ -134,7 +135,7 @@ protected function getCollectors(): array
 
         return [
             new ProvidedUsagesCollector($reflectionProvider, $this->getMemberUsageProviders(), $this->getMemberUsageExcluders()),
-            new ClassDefinitionCollector($reflectionProvider, $this->detectDeadMethods, $this->detectDeadConstants, $this->detectDeadEnumCases),
+            new ClassDefinitionCollector($reflectionProvider, $this->detectDeadConstants, $this->detectDeadEnumCases),
             new MethodCallCollector($this->getMemberUsageExcluders()),
             new ConstantFetchCollector($reflectionProvider, $this->getMemberUsageExcluders()),
         ];
@@ -490,7 +491,7 @@ public function testMethodDetectionCanBeDisabled(): void
         $this->detectDeadMethods = false;
         $this->analyse([__DIR__ . '/data/other/member-types.php'], [
             ['Unused MemberTypes\Clazz::CONSTANT', 7],
-            ['Unused MemberTypes\MyEnum::EnumCase', 13],
+            ['Unused MemberTypes\MyEnum::EnumCase', 25],
         ]);
     }
 
@@ -502,8 +503,8 @@ public function testConstantDetectionCanBeDisabled(): void
 
         $this->detectDeadConstants = false;
         $this->analyse([__DIR__ . '/data/other/member-types.php'], [
-            ['Unused MemberTypes\MyEnum::EnumCase', 13],
-            ['Unused MemberTypes\Clazz::method', 8],
+            ['Unused MemberTypes\MyEnum::EnumCase', 25],
+            ['Unused MemberTypes\Clazz::method', 10],
         ]);
     }
 
@@ -516,7 +517,7 @@ public function testEnumCaseDetectionCanBeDisabled(): void
         $this->detectDeadEnumCases = false;
         $this->analyse([__DIR__ . '/data/other/member-types.php'], [
             ['Unused MemberTypes\Clazz::CONSTANT', 7],
-            ['Unused MemberTypes\Clazz::method', 8],
+            ['Unused MemberTypes\Clazz::method', 10],
         ]);
     }
 

tests/Rule/data/other/member-types.php

@@ -5,10 +5,25 @@
 class Clazz
 {
     public const CONSTANT = 1; // error: Unused MemberTypes\Clazz::CONSTANT
+    public const CONSTANT_USED = 1;
+
     public function method(): void {} // error: Unused MemberTypes\Clazz::method
+
+    public function methodUsed(): void {
+        $this->transitivity();
+    }
+
+    private function transitivity(): void
+    {
+        echo MyEnum::EnumCaseUsed->name;
+        echo self::CONSTANT_USED;
+    }
 }
 
 enum MyEnum
 {
     case EnumCase; // error: Unused MemberTypes\MyEnum::EnumCase
+    case EnumCaseUsed;
 }
+
+(new Clazz())->methodUsed();