Skip to content

Commit fe15bef

Browse files
janedbaljanedbal
authored
Reflection: proper dead transitivity (#135)
1 parent 3e8c4f1 commit fe15bef

File tree

8 files changed

+101
-57
lines changed

8 files changed

+101
-57
lines changed

‎rules.neon‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@ services:
77
-
88
class: ShipMonk\PHPStan\DeadCode\Transformer\FileSystem
99

10+
-
11+
class: ShipMonk\PHPStan\DeadCode\Graph\UsageOriginDetector
12+
1013
-
1114
class: ShipMonk\PHPStan\DeadCode\Provider\VendorUsageProvider
1215
tags:

‎src/Collector/ConstantFetchCollector.php‎

Lines changed: 7 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,13 @@
1010
use PhpParser\Node\Name;
1111
use PHPStan\Analyser\Scope;
1212
use PHPStan\Collectors\Collector;
13-
use PHPStan\Reflection\MethodReflection;
1413
use PHPStan\Reflection\ReflectionProvider;
1514
use PHPStan\Type\Constant\ConstantStringType;
1615
use PHPStan\Type\Type;
1716
use PHPStan\Type\TypeUtils;
1817
use ShipMonk\PHPStan\DeadCode\Graph\ClassConstantRef;
1918
use ShipMonk\PHPStan\DeadCode\Graph\ClassConstantUsage;
20-
use ShipMonk\PHPStan\DeadCode\Graph\ClassMethodRef;
19+
use ShipMonk\PHPStan\DeadCode\Graph\UsageOriginDetector;
2120
use function array_map;
2221
use function count;
2322
use function current;
@@ -32,17 +31,21 @@ class ConstantFetchCollector implements Collector
3231

3332
use BufferedUsageCollector;
3433

34+
private UsageOriginDetector $usageOriginDetector;
35+
3536
private ReflectionProvider $reflectionProvider;
3637

3738
private bool $trackMixedAccess;
3839

3940
public function __construct(
41+
UsageOriginDetector $usageOriginDetector,
4042
ReflectionProvider $reflectionProvider,
4143
bool $trackMixedAccess
4244
)
4345
{
4446
$this->reflectionProvider = $reflectionProvider;
4547
$this->trackMixedAccess = $trackMixedAccess;
48+
$this->usageOriginDetector = $usageOriginDetector;
4649
}
4750

4851
public function getNodeType(): string
@@ -109,7 +112,7 @@ private function registerFunctionCall(FuncCall $node, Scope $scope): void
109112
}
110113

111114
$this->usageBuffer[] = new ClassConstantUsage(
112-
$this->getCaller($scope),
115+
$this->usageOriginDetector->detectOrigin($scope),
113116
new ClassConstantRef($className, $constantName, true),
114117
);
115118
}
@@ -137,7 +140,7 @@ private function registerFetch(ClassConstFetch $node, Scope $scope): void
137140

138141
foreach ($this->getDeclaringTypesWithConstant($ownerType, $constantName) as $className) {
139142
$this->usageBuffer[] = new ClassConstantUsage(
140-
$this->getCaller($scope),
143+
$this->usageOriginDetector->detectOrigin($scope),
141144
new ClassConstantRef($className, $constantName, $possibleDescendantFetch),
142145
);
143146
}
@@ -173,21 +176,4 @@ private function getDeclaringTypesWithConstant(
173176
return $result;
174177
}
175178

176-
private function getCaller(Scope $scope): ?ClassMethodRef
177-
{
178-
if (!$scope->isInClass()) {
179-
return null;
180-
}
181-
182-
if (!$scope->getFunction() instanceof MethodReflection) {
183-
return null;
184-
}
185-
186-
return new ClassMethodRef(
187-
$scope->getClassReflection()->getName(),
188-
$scope->getFunction()->getName(),
189-
false,
190-
);
191-
}
192-
193179
}

‎src/Collector/MethodCallCollector.php‎

Lines changed: 12 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,13 @@
1717
use PHPStan\Collectors\Collector;
1818
use PHPStan\Node\MethodCallableNode;
1919
use PHPStan\Node\StaticMethodCallableNode;
20-
use PHPStan\Reflection\MethodReflection;
2120
use PHPStan\TrinaryLogic;
2221
use PHPStan\Type\Type;
2322
use PHPStan\Type\TypeCombinator;
2423
use PHPStan\Type\TypeUtils;
2524
use ShipMonk\PHPStan\DeadCode\Graph\ClassMethodRef;
2625
use ShipMonk\PHPStan\DeadCode\Graph\ClassMethodUsage;
26+
use ShipMonk\PHPStan\DeadCode\Graph\UsageOriginDetector;
2727

2828
/**
2929
* @implements Collector<Node, list<string>>
@@ -33,10 +33,16 @@ class MethodCallCollector implements Collector
3333

3434
use BufferedUsageCollector;
3535

36+
private UsageOriginDetector $usageOriginDetector;
37+
3638
private bool $trackMixedAccess;
3739

38-
public function __construct(bool $trackMixedAccess)
40+
public function __construct(
41+
UsageOriginDetector $usageOriginDetector,
42+
bool $trackMixedAccess
43+
)
3944
{
45+
$this->usageOriginDetector = $usageOriginDetector;
4046
$this->trackMixedAccess = $trackMixedAccess;
4147
}
4248

@@ -114,7 +120,7 @@ private function registerMethodCall(
114120
foreach ($methodNames as $methodName) {
115121
foreach ($this->getDeclaringTypesWithMethod($scope, $callerType, $methodName, TrinaryLogic::createNo()) as $className) {
116122
$this->usageBuffer[] = new ClassMethodUsage(
117-
$this->getCaller($scope),
123+
$this->usageOriginDetector->detectOrigin($scope),
118124
new ClassMethodRef($className, $methodName, $possibleDescendantCall),
119125
);
120126
}
@@ -140,7 +146,7 @@ private function registerStaticCall(
140146
foreach ($methodNames as $methodName) {
141147
foreach ($this->getDeclaringTypesWithMethod($scope, $callerType, $methodName, TrinaryLogic::createYes()) as $className) {
142148
$this->usageBuffer[] = new ClassMethodUsage(
143-
$this->getCaller($scope),
149+
$this->usageOriginDetector->detectOrigin($scope),
144150
new ClassMethodRef($className, $methodName, $possibleDescendantCall),
145151
);
146152
}
@@ -165,7 +171,7 @@ private function registerArrayCallable(
165171

166172
foreach ($this->getDeclaringTypesWithMethod($scope, $caller, $methodName, TrinaryLogic::createMaybe()) as $className) {
167173
$this->usageBuffer[] = new ClassMethodUsage(
168-
$this->getCaller($scope),
174+
$this->usageOriginDetector->detectOrigin($scope),
169175
new ClassMethodRef($className, $methodName, $possibleDescendantCall),
170176
);
171177
}
@@ -189,7 +195,7 @@ private function registerClone(Clone_ $node, Scope $scope): void
189195

190196
foreach ($this->getDeclaringTypesWithMethod($scope, $callerType, $methodName, TrinaryLogic::createNo()) as $className) {
191197
$this->usageBuffer[] = new ClassMethodUsage(
192-
$this->getCaller($scope),
198+
$this->usageOriginDetector->detectOrigin($scope),
193199
new ClassMethodRef($className, $methodName, true),
194200
);
195201
}
@@ -255,21 +261,4 @@ private function getDeclaringTypesWithMethod(
255261
return $result;
256262
}
257263

258-
private function getCaller(Scope $scope): ?ClassMethodRef
259-
{
260-
if (!$scope->isInClass()) {
261-
return null;
262-
}
263-
264-
if (!$scope->getFunction() instanceof MethodReflection) {
265-
return null;
266-
}
267-
268-
return new ClassMethodRef(
269-
$scope->getClassReflection()->getName(),
270-
$scope->getFunction()->getName(),
271-
false,
272-
);
273-
}
274-
275264
}

‎src/Graph/ClassMemberUsage.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ abstract class ClassMemberUsage
1717

1818
/**
1919
* Origin method of the usage, "where it was called from"
20+
* This is required for proper transitive dead code detection.
21+
*
22+
* @see UsageOriginDetector for typical usage
2023
*/
2124
private ?ClassMethodRef $origin;
2225

‎src/Graph/UsageOriginDetector.php‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace ShipMonk\PHPStan\DeadCode\Graph;
4+
5+
use PHPStan\Analyser\Scope;
6+
use PHPStan\Reflection\MethodReflection;
7+
8+
class UsageOriginDetector
9+
{
10+
11+
/**
12+
* Most of the time, this is the only implementation you need for ClassMemberUsage constructor
13+
*/
14+
public function detectOrigin(Scope $scope): ?ClassMethodRef
15+
{
16+
if (!$scope->isInClass()) {
17+
return null;
18+
}
19+
20+
if (!$scope->getFunction() instanceof MethodReflection) {
21+
return null;
22+
}
23+
24+
return new ClassMethodRef(
25+
$scope->getClassReflection()->getName(),
26+
$scope->getFunction()->getName(),
27+
false,
28+
);
29+
}
30+
31+
}

‎src/Provider/ReflectionUsageProvider.php‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,17 +18,24 @@
1818
use ShipMonk\PHPStan\DeadCode\Graph\ClassMemberUsage;
1919
use ShipMonk\PHPStan\DeadCode\Graph\ClassMethodRef;
2020
use ShipMonk\PHPStan\DeadCode\Graph\ClassMethodUsage;
21+
use ShipMonk\PHPStan\DeadCode\Graph\UsageOriginDetector;
2122
use function array_key_first;
2223
use function count;
2324
use function in_array;
2425

2526
class ReflectionUsageProvider implements MemberUsageProvider
2627
{
2728

29+
private UsageOriginDetector $usageOriginDetector;
30+
2831
private bool $enabled;
2932

30-
public function __construct(bool $enabled)
33+
public function __construct(
34+
UsageOriginDetector $usageOriginDetector,
35+
bool $enabled
36+
)
3137
{
38+
$this->usageOriginDetector = $usageOriginDetector;
3239
$this->enabled = $enabled;
3340
}
3441

@@ -98,15 +105,15 @@ private function extractConstantsUsedByReflection(
98105

99106
if ($methodName === 'getConstants' || $methodName === 'getReflectionConstants') {
100107
foreach ($genericReflection->getNativeReflection()->getReflectionConstants() as $reflectionConstant) {
101-
$usedConstants[] = $this->createConstantUsage($reflectionConstant->getDeclaringClass()->getName(), $reflectionConstant->getName());
108+
$usedConstants[] = $this->createConstantUsage($scope, $reflectionConstant->getDeclaringClass()->getName(), $reflectionConstant->getName());
102109
}
103110
}
104111

105112
if (($methodName === 'getConstant' || $methodName === 'getReflectionConstant') && count($args) === 1) {
106113
$firstArg = $args[array_key_first($args)]; // @phpstan-ignore offsetAccess.notFound
107114

108115
foreach ($scope->getType($firstArg->value)->getConstantStrings() as $constantString) {
109-
$usedConstants[] = $this->createConstantUsage($genericReflection->getName(), $constantString->getValue());
116+
$usedConstants[] = $this->createConstantUsage($scope, $genericReflection->getName(), $constantString->getValue());
110117
}
111118
}
112119

@@ -128,23 +135,23 @@ private function extractMethodsUsedByReflection(
128135

129136
if ($methodName === 'getMethods') {
130137
foreach ($genericReflection->getNativeReflection()->getMethods() as $reflectionMethod) {
131-
$usedMethods[] = $this->createMethodUsage($reflectionMethod->getDeclaringClass()->getName(), $reflectionMethod->getName());
138+
$usedMethods[] = $this->createMethodUsage($scope, $reflectionMethod->getDeclaringClass()->getName(), $reflectionMethod->getName());
132139
}
133140
}
134141

135142
if ($methodName === 'getMethod' && count($args) === 1) {
136143
$firstArg = $args[array_key_first($args)]; // @phpstan-ignore offsetAccess.notFound
137144

138145
foreach ($scope->getType($firstArg->value)->getConstantStrings() as $constantString) {
139-
$usedMethods[] = $this->createMethodUsage($genericReflection->getName(), $constantString->getValue());
146+
$usedMethods[] = $this->createMethodUsage($scope, $genericReflection->getName(), $constantString->getValue());
140147
}
141148
}
142149

143150
if (in_array($methodName, ['getConstructor', 'newInstance', 'newInstanceArgs'], true)) {
144151
$constructor = $genericReflection->getNativeReflection()->getConstructor();
145152

146153
if ($constructor !== null) {
147-
$usedMethods[] = $this->createMethodUsage($constructor->getDeclaringClass()->getName(), '__construct');
154+
$usedMethods[] = $this->createMethodUsage($scope, $constructor->getDeclaringClass()->getName(), '__construct');
148155
}
149156
}
150157

@@ -174,10 +181,10 @@ private function getMethodNames(CallLike $call, Scope $scope): array
174181
return [$call->name->toString()];
175182
}
176183

177-
private function createConstantUsage(string $className, string $constantName): ClassConstantUsage
184+
private function createConstantUsage(Scope $scope, string $className, string $constantName): ClassConstantUsage
178185
{
179186
return new ClassConstantUsage(
180-
null,
187+
$this->usageOriginDetector->detectOrigin($scope),
181188
new ClassConstantRef(
182189
$className,
183190
$constantName,
@@ -186,10 +193,10 @@ private function createConstantUsage(string $className, string $constantName): C
186193
);
187194
}
188195

189-
private function createMethodUsage(string $className, string $methodName): ClassMethodUsage
196+
private function createMethodUsage(Scope $scope, string $className, string $methodName): ClassMethodUsage
190197
{
191198
return new ClassMethodUsage(
192-
null,
199+
$this->usageOriginDetector->detectOrigin($scope),
193200
new ClassMethodRef(
194201
$className,
195202
$methodName,

‎tests/Rule/DeadCodeRuleTest.php‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
use ShipMonk\PHPStan\DeadCode\Collector\ProvidedUsagesCollector;
2020
use ShipMonk\PHPStan\DeadCode\Compatibility\BackwardCompatibilityChecker;
2121
use ShipMonk\PHPStan\DeadCode\Formatter\RemoveDeadCodeFormatter;
22+
use ShipMonk\PHPStan\DeadCode\Graph\UsageOriginDetector;
2223
use ShipMonk\PHPStan\DeadCode\Hierarchy\ClassHierarchy;
2324
use ShipMonk\PHPStan\DeadCode\Provider\DoctrineUsageProvider;
2425
use ShipMonk\PHPStan\DeadCode\Provider\MemberUsageProvider;
@@ -76,8 +77,8 @@ protected function getCollectors(): array
7677
$this->getMemberUsageProviders(),
7778
),
7879
new ClassDefinitionCollector(),
79-
new MethodCallCollector($this->trackMixedAccess),
80-
new ConstantFetchCollector($reflectionProvider, $this->trackMixedAccess),
80+
new MethodCallCollector($this->createUsageOriginDetector(), $this->trackMixedAccess),
81+
new ConstantFetchCollector($this->createUsageOriginDetector(), $reflectionProvider, $this->trackMixedAccess),
8182
];
8283
}
8384

@@ -368,7 +369,10 @@ private function getTransformedFilePath(string $file): string
368369
private function getMemberUsageProviders(): array
369370
{
370371
return [
371-
new ReflectionUsageProvider(true),
372+
new ReflectionUsageProvider(
373+
$this->createUsageOriginDetector(),
374+
true,
375+
),
372376
new class extends ReflectionBasedMemberUsageProvider
373377
{
374378

@@ -421,6 +425,18 @@ static function (string $type): array {
421425
return $mock;
422426
}
423427

428+
private function createUsageOriginDetector(): UsageOriginDetector
429+
{
430+
/** @var UsageOriginDetector|null $detector */
431+
static $detector = null;
432+
433+
if ($detector === null) {
434+
$detector = new UsageOriginDetector();
435+
}
436+
437+
return $detector;
438+
}
439+
424440
public function gatherAnalyserErrors(array $files): array
425441
{
426442
if (!$this->unwrapGroupedErrors) {

‎tests/Rule/data/providers/reflection.php‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,15 @@ enum EnumHolder1 {
5757
public function used() {}
5858
}
5959

60+
enum TransitiveHolder {
61+
const TRANSITIVELY_DEAD = 1; // error: Unused Reflection\TransitiveHolder::TRANSITIVELY_DEAD
62+
63+
public function test() // error: Unused Reflection\TransitiveHolder::test
64+
{
65+
(new \ReflectionClass(self::class))->getConstant('TRANSITIVELY_DEAD');
66+
}
67+
}
68+
6069
$reflection1 = new \ReflectionClass(Holder1::class);
6170
$reflection1->getConstants();
6271
$reflection1->getMethod('used');

0 commit comments

Comments
 (0)