Skip to content

Commit 633eb73

Browse files
committed
[DeadCode] Skip private method removal in classes using reflection
Classes that instantiate a *Reflection* object can use private methods dynamically (by name, or by reading their own source/body), which static analysis cannot detect. Real-world false positive: spiral/framework ReflectionFileTest::deadend(), kept via withSkip(). Keep all private methods in such classes to stay on the safe side.
1 parent 0df45b4 commit 633eb73

3 files changed

Lines changed: 52 additions & 48 deletions

File tree

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
<?php
2+
3+
namespace Rector\Tests\DeadCode\Rector\ClassMethod\RemoveUnusedPrivateMethodRector\Fixture;
4+
5+
// real-world false positive (spiral/framework ReflectionFileTest::deadend):
6+
// the method body is read back via reflection over the current file and its
7+
// function invocations are asserted, so it is used only at runtime - invisible
8+
// to static analysis. The class instantiates a *Reflection* object, so the rule
9+
// keeps all private methods to stay on the safe side.
10+
final class KeepUsedViaSelfReflection
11+
{
12+
public function testInvocations(): void
13+
{
14+
$reflection = new \ReflectionClass(self::class);
15+
foreach ($reflection->getMethods() as $method) {
16+
// inspect own methods dynamically
17+
}
18+
}
19+
20+
private function deadend(): void
21+
{
22+
$a = $b = null;
23+
test_function_a($this, $a + $b);
24+
test_function_b("string", 123);
25+
}
26+
}

rules-tests/DeadCode/Rector/ClassMethod/RemoveUnusedPrivateMethodRector/Fixture/used_via_self_reflection.php.inc

Lines changed: 0 additions & 48 deletions
This file was deleted.

rules/DeadCode/Rector/ClassMethod/RemoveUnusedPrivateMethodRector.php

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@
66

77
use PhpParser\Node;
88
use PhpParser\Node\Expr\MethodCall;
9+
use PhpParser\Node\Expr\New_;
910
use PhpParser\Node\Expr\Variable;
11+
use PhpParser\Node\Name;
1012
use PhpParser\Node\Stmt\Class_;
1113
use PhpParser\Node\Stmt\ClassMethod;
1214
use PHPStan\Reflection\ClassReflection;
@@ -89,6 +91,12 @@ public function refactor(Node $node): ?Node
8991
return null;
9092
}
9193

94+
// reflection-heavy classes can use private methods dynamically (by name,
95+
// or by reading their own source/body), which is invisible to static analysis
96+
if ($this->usesReflection($node)) {
97+
return null;
98+
}
99+
92100
$dataProviderMethodNames = $this->resolveDataProviderMethodNames($node);
93101

94102
foreach ($node->stmts as $classStmtKey => $classStmt) {
@@ -153,6 +161,24 @@ private function shouldSkip(ClassMethod $classMethod, ClassReflection $classRefl
153161
return $classReflection->hasMethod(MethodName::CALL);
154162
}
155163

164+
private function usesReflection(Class_ $class): bool
165+
{
166+
return (bool) $this->betterNodeFinder->findFirst(
167+
$class->stmts,
168+
static function (Node $subNode): bool {
169+
if (! $subNode instanceof New_) {
170+
return false;
171+
}
172+
173+
if (! $subNode->class instanceof Name) {
174+
return false;
175+
}
176+
177+
return str_contains(strtolower($subNode->class->toString()), 'reflection');
178+
}
179+
);
180+
}
181+
156182
private function hasDynamicMethodCallOnFetchThis(ClassMethod $classMethod): bool
157183
{
158184
return (bool) $this->betterNodeFinder->findFirst(

0 commit comments

Comments
 (0)