From bc1d17e730058704c6ea574af6eb64b0ced28620 Mon Sep 17 00:00:00 2001
From: Jan Nedbal <janedbal@gmail.com>
Date: Tue, 25 Mar 2025 13:55:04 +0100
Subject: [PATCH] Improve descendant detection for const fetches

---
 src/Collector/ConstantFetchCollector.php      | 23 ++++++++-----------
 src/Rule/DeadCodeRule.php                     | 17 +++++++++-----
 tests/Rule/DeadCodeRuleTest.php               |  5 +++-
 tests/Rule/data/constants/descendant-1.php    | 18 +++++++++++++++
 tests/Rule/data/constants/descendant-2.php    | 19 +++++++++++++++
 tests/Rule/data/constants/descendant-3.php    | 20 ++++++++++++++++
 .../{static.php => descendant-4.php}          |  2 +-
 tests/Rule/data/constants/traits-14.php       | 10 +++++---
 8 files changed, 90 insertions(+), 24 deletions(-)
 create mode 100644 tests/Rule/data/constants/descendant-1.php
 create mode 100644 tests/Rule/data/constants/descendant-2.php
 create mode 100644 tests/Rule/data/constants/descendant-3.php
 rename tests/Rule/data/constants/{static.php => descendant-4.php} (87%)

diff --git a/src/Collector/ConstantFetchCollector.php b/src/Collector/ConstantFetchCollector.php
index c14d00ea..73be547f 100644
--- a/src/Collector/ConstantFetchCollector.php
+++ b/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;
diff --git a/src/Rule/DeadCodeRule.php b/src/Rule/DeadCodeRule.php
index db27ea63..aac78de8 100644
--- a/src/Rule/DeadCodeRule.php
+++ b/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;
diff --git a/tests/Rule/DeadCodeRuleTest.php b/tests/Rule/DeadCodeRuleTest.php
index 2a01022a..f1a85418 100644
--- a/tests/Rule/DeadCodeRuleTest.php
+++ b/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'];
diff --git a/tests/Rule/data/constants/descendant-1.php b/tests/Rule/data/constants/descendant-1.php
new file mode 100644
index 00000000..2cbc30e0
--- /dev/null
+++ b/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
diff --git a/tests/Rule/data/constants/descendant-2.php b/tests/Rule/data/constants/descendant-2.php
new file mode 100644
index 00000000..97914aed
--- /dev/null
+++ b/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;
+}
+
diff --git a/tests/Rule/data/constants/descendant-3.php b/tests/Rule/data/constants/descendant-3.php
new file mode 100644
index 00000000..76501765
--- /dev/null
+++ b/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
diff --git a/tests/Rule/data/constants/static.php b/tests/Rule/data/constants/descendant-4.php
similarity index 87%
rename from tests/Rule/data/constants/static.php
rename to tests/Rule/data/constants/descendant-4.php
index 292ccb43..d3a889b0 100644
--- a/tests/Rule/data/constants/static.php
+++ b/tests/Rule/data/constants/descendant-4.php
@@ -1,6 +1,6 @@
 <?php declare(strict_types = 1);
 
-namespace DeadConstStatic;
+namespace DeadConstDescendant4;
 
 
 class P {
diff --git a/tests/Rule/data/constants/traits-14.php b/tests/Rule/data/constants/traits-14.php
index 53303b13..36b8ceab 100644
--- a/tests/Rule/data/constants/traits-14.php
+++ b/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