Improve descendant detection for const fetches (#176)

Author: janedbal

src/Collector/ConstantFetchCollector.php

@@ -131,7 +131,7 @@ private function registerFetch(ClassConstFetch $node, Scope $scope): void
     {
         if ($node->class instanceof Expr) {
             $ownerType = $scope->getType($node->class);
-            $possibleDescendantFetch = true;
+            $possibleDescendantFetch = null;
         } else {
             $ownerType = $scope->resolveTypeByName($node->class);
             $possibleDescendantFetch = $node->class->toString() === 'static';
@@ -146,11 +146,11 @@ private function registerFetch(ClassConstFetch $node, Scope $scope): void
                 continue; // reserved for class name fetching
             }
 
-            foreach ($this->getDeclaringTypesWithConstant($ownerType, $constantName) as $className) {
+            foreach ($this->getDeclaringTypesWithConstant($ownerType, $constantName, $possibleDescendantFetch) as $constantRef) {
                 $this->registerUsage(
                     new ClassConstantUsage(
                         UsageOrigin::createRegular($node, $scope),
-                        new ClassConstantRef($className, $constantName, $possibleDescendantFetch),
+                        $constantRef,
                     ),
                     $node,
                     $scope,
@@ -160,29 +160,26 @@ private function registerFetch(ClassConstFetch $node, Scope $scope): void
     }
 
     /**
-     * @return list<class-string<object>|null>
+     * @return list<ClassConstantRef>
      */
     private function getDeclaringTypesWithConstant(
         Type $type,
-        string $constantName
+        string $constantName,
+        ?bool $isPossibleDescendant
     ): array
     {
-        $typeNormalized = TypeUtils::toBenevolentUnion($type); // extract possible calls even from Class|int
+        $typeNormalized = TypeUtils::toBenevolentUnion($type); // extract possible fetches even from Class|int
         $classReflections = $typeNormalized->getObjectTypeOrClassStringObjectType()->getObjectClassReflections();
 
         $result = [];
 
         foreach ($classReflections as $classReflection) {
-            if ($classReflection->hasConstant($constantName)) {
-                $result[] = $classReflection->getConstant($constantName)->getDeclaringClass()->getName();
-
-            } else { // unknown constant fetch (might be present on children)
-                $result[] = $classReflection->getName();
-            }
+            $possibleDescendant = $isPossibleDescendant ?? !$classReflection->isFinal();
+            $result[] = new ClassConstantRef($classReflection->getName(), $constantName, $possibleDescendant);
         }
 
         if ($result === []) {
-            $result[] = null; // call over unknown type
+            $result[] = new ClassConstantRef(null, $constantName, true); // call over unknown type
         }
 
         return $result;

src/Rule/DeadCodeRule.php

@@ -427,15 +427,16 @@ private function getAlternativeMemberKeys(ClassMemberRef $member): array
     }
 
     private function findDefinerMemberKey(
-        ClassMemberRef $origin,
+        ClassMemberRef $memberRef,
         string $className,
         bool $includeParentLookup = true
     ): ?string
     {
-        $memberName = $origin->getMemberName();
-        $memberKey = $origin::buildKey($className, $memberName);
+        $memberName = $memberRef->getMemberName();
+        $memberKey = $memberRef::buildKey($className, $memberName);
+        $memberType = $memberRef->getMemberType();
 
-        if ($this->hasMember($className, $memberName, $origin->getMemberType())) {
+        if ($this->hasMember($className, $memberName, $memberRef->getMemberType())) {
             return $memberKey;
         }
 
@@ -447,9 +448,13 @@ private function findDefinerMemberKey(
         }
 
         if ($includeParentLookup) {
+            $parentNames = $memberType === MemberType::CONSTANT
+                ? $this->getAncestorNames($className) // constants can be declared in interfaces
+                : $this->getParentNames($className);
+
             // search for definition in parents (and its traits)
-            foreach ($this->getParentNames($className) as $parentName) {
-                $found = $this->findDefinerMemberKey($origin, $parentName, false);
+            foreach ($parentNames as $parentName) {
+                $found = $this->findDefinerMemberKey($memberRef, $parentName, false);
 
                 if ($found !== null) {
                     return $found;

tests/Rule/DeadCodeRuleTest.php

@@ -547,12 +547,15 @@ public static function provideFiles(): iterable
         // constants
         yield 'const-basic' => [__DIR__ . '/data/constants/basic.php'];
         yield 'const-function' => [__DIR__ . '/data/constants/constant-function.php'];
+        yield 'const-descendant-1' => [__DIR__ . '/data/constants/descendant-1.php'];
+        yield 'const-descendant-2' => [__DIR__ . '/data/constants/descendant-2.php'];
+        yield 'const-descendant-3' => [__DIR__ . '/data/constants/descendant-3.php'];
+        yield 'const-descendant-4' => [__DIR__ . '/data/constants/descendant-4.php'];
         yield 'const-dynamic' => [__DIR__ . '/data/constants/dynamic.php'];
         yield 'const-expr' => [__DIR__ . '/data/constants/expr.php'];
         yield 'const-magic' => [__DIR__ . '/data/constants/magic.php'];
         yield 'const-mixed' => [__DIR__ . '/data/constants/mixed/tracked.php'];
         yield 'const-override' => [__DIR__ . '/data/constants/override.php'];
-        yield 'const-static' => [__DIR__ . '/data/constants/static.php'];
         yield 'const-traits-1' => [__DIR__ . '/data/constants/traits-1.php'];
         yield 'const-traits-2' => [__DIR__ . '/data/constants/traits-2.php'];
         yield 'const-traits-3' => [__DIR__ . '/data/constants/traits-3.php'];

tests/Rule/data/constants/descendant-1.php

@@ -0,0 +1,18 @@
+<?php declare(strict_types = 1);
+
+namespace DeadConstDescendant1;
+
+
+class ParentClass
+{
+    const CONSTANT = 1;
+}
+
+class ChildClass extends ParentClass
+{
+    const CONSTANT = 1; // error: Unused DeadConstDescendant1\ChildClass::CONSTANT
+}
+
+echo (new ParentClass())::CONSTANT;
+
+// test is similar to tests/Rule/data/methods/parent-call-5.php

tests/Rule/data/constants/descendant-2.php

@@ -0,0 +1,19 @@
+<?php declare(strict_types = 1);
+
+namespace DeadConstDescendant2;
+
+
+class ParentClass
+{
+    const CONSTANT = 1;
+}
+
+class ChildClass extends ParentClass
+{
+    const CONSTANT = 1;
+}
+
+function test(ParentClass $class) {
+    echo $class::CONSTANT;
+}
+

tests/Rule/data/constants/descendant-3.php

@@ -0,0 +1,20 @@
+<?php declare(strict_types = 1);
+
+namespace DeadConstDescendant3;
+
+
+class P {
+
+    const CONSTANT = 1;
+
+    public function test() {
+        echo self::CONSTANT;
+    }
+}
+
+class C extends P {
+    const CONSTANT = 2; // error: Unused DeadConstDescendant3\C::CONSTANT
+}
+
+$c = new C();
+$c->test(); // prints 1

tests/Rule/data/constants/static.php renamed to tests/Rule/data/constants/descendant-4.php

@@ -1,6 +1,6 @@
 <?php declare(strict_types = 1);
 
-namespace DeadConstStatic;
+namespace DeadConstDescendant4;
 
 
 class P {

tests/Rule/data/constants/traits-14.php

@@ -3,16 +3,20 @@
 namespace DeadTraitConst14;
 
 trait SomeTrait {
-    const FOO = 1; // error: Unused DeadTraitConst14\SomeTrait::FOO
+    const FOO = 1;
 }
 
 class ParentClass {
-    const FOO = 1;
+    const FOO = 1; // error: Unused DeadTraitConst14\ParentClass::FOO
 }
 
 class User extends ParentClass {
     use SomeTrait;
 }
 
-echo User::FOO; // verify by var_dump((new ReflectionClass('User'))->getReflectionConstants());
+echo User::FOO;
+
+// this test does not fully comply with result of var_dump((new ReflectionClass('User'))->getReflectionConstants());
+// this behaviour is kept for simplicity as it has equal behaviour with methods (see methods/traits-14.php)
+// also, overridden constants are ensured to have the same value