From d9aa188b7df5e37f4ecae980f46652dc41311816 Mon Sep 17 00:00:00 2001 From: Jan Nedbal Date: Tue, 31 Dec 2024 11:52:46 +0100 Subject: [PATCH] ClassHierarchy: fix missing parts of vendor hierarchy (#136) --- src/Collector/ClassDefinitionCollector.php | 51 +++++++------------ src/Rule/DeadCodeRule.php | 3 -- tests/Rule/DeadCodeRuleTest.php | 3 +- .../Rule/data/methods/hierarchy-in-vendor.php | 18 +++++++ tests/Rule/data/providers/reflection.php | 2 + 5 files changed, 41 insertions(+), 36 deletions(-) create mode 100644 tests/Rule/data/methods/hierarchy-in-vendor.php diff --git a/src/Collector/ClassDefinitionCollector.php b/src/Collector/ClassDefinitionCollector.php index 065f5fd..f6da3d7 100644 --- a/src/Collector/ClassDefinitionCollector.php +++ b/src/Collector/ClassDefinitionCollector.php @@ -4,7 +4,6 @@ use LogicException; use PhpParser\Node; -use PhpParser\Node\Name; use PhpParser\Node\Stmt\Class_; use PhpParser\Node\Stmt\ClassLike; use PhpParser\Node\Stmt\Enum_; @@ -14,6 +13,8 @@ use PhpParser\Node\Stmt\TraitUseAdaptation\Precedence; use PHPStan\Analyser\Scope; use PHPStan\Collectors\Collector; +use PHPStan\Reflection\ClassReflection; +use PHPStan\Reflection\ReflectionProvider; use ShipMonk\PHPStan\DeadCode\Enum\ClassLikeKind; use ShipMonk\PHPStan\DeadCode\Enum\Visibility; use function array_fill_keys; @@ -33,6 +34,13 @@ class ClassDefinitionCollector implements Collector { + private ReflectionProvider $reflectionProvider; + + public function __construct(ReflectionProvider $reflectionProvider) + { + $this->reflectionProvider = $reflectionProvider; + } + public function getNodeType(): string { return ClassLike::class; @@ -61,6 +69,7 @@ public function processNode( $kind = $this->getKind($node); $typeName = $node->namespacedName->toString(); + $reflection = $this->reflectionProvider->getClass($typeName); $methods = []; @@ -87,54 +96,32 @@ public function processNode( 'name' => $typeName, 'methods' => $methods, 'constants' => $constants, - 'parents' => $this->getParents($node), + 'parents' => $this->getParents($reflection), 'traits' => $this->getTraits($node), - 'interfaces' => $this->getInterfaces($node), + 'interfaces' => $this->getInterfaces($reflection), ]; } /** * @return array */ - private function getParents(ClassLike $node): array + private function getParents(ClassReflection $reflection): array { - if ($node instanceof Class_) { - if ($node->extends === null) { - return []; - } + $parents = []; - return [$node->extends->toString() => null]; + foreach ($reflection->getParentClassesNames() as $parent) { + $parents[$parent] = null; } - if ($node instanceof Interface_) { - return array_fill_keys( - array_map( - static fn(Name $name) => $name->toString(), - $node->extends, - ), - null, - ); - } - - return []; + return $parents; } /** * @return array */ - private function getInterfaces(ClassLike $node): array + private function getInterfaces(ClassReflection $reflection): array { - if ($node instanceof Class_ || $node instanceof Enum_) { - return array_fill_keys( - array_map( - static fn(Name $name) => $name->toString(), - $node->implements, - ), - null, - ); - } - - return []; + return array_fill_keys(array_map(static fn (ClassReflection $reflection) => $reflection->getName(), $reflection->getInterfaces()), null); } /** diff --git a/src/Rule/DeadCodeRule.php b/src/Rule/DeadCodeRule.php index 6d27a57..81b8ac2 100644 --- a/src/Rule/DeadCodeRule.php +++ b/src/Rule/DeadCodeRule.php @@ -344,8 +344,6 @@ private function fillClassHierarchy(string $typeName, array $ancestorNames): voi { foreach ($ancestorNames as $ancestorName) { $this->classHierarchy->registerClassPair($ancestorName, $typeName); - - $this->fillClassHierarchy($typeName, $this->getAncestorNames($ancestorName)); } } @@ -554,7 +552,6 @@ private function getAncestorNames(string $typeName): array { return array_merge( array_keys($this->typeDefinitions[$typeName]['parents'] ?? []), - array_keys($this->typeDefinitions[$typeName]['traits'] ?? []), array_keys($this->typeDefinitions[$typeName]['interfaces'] ?? []), ); } diff --git a/tests/Rule/DeadCodeRuleTest.php b/tests/Rule/DeadCodeRuleTest.php index 358086c..5a9aab9 100644 --- a/tests/Rule/DeadCodeRuleTest.php +++ b/tests/Rule/DeadCodeRuleTest.php @@ -76,7 +76,7 @@ protected function getCollectors(): array $reflectionProvider, $this->getMemberUsageProviders(), ), - new ClassDefinitionCollector(), + new ClassDefinitionCollector(self::createReflectionProvider()), new MethodCallCollector($this->createUsageOriginDetector(), $this->trackMixedAccess), new ConstantFetchCollector($this->createUsageOriginDetector(), $reflectionProvider, $this->trackMixedAccess), ]; @@ -272,6 +272,7 @@ public static function provideFiles(): iterable yield 'method-mixed' => [__DIR__ . '/data/methods/mixed/tracked.php']; yield 'method-new-in-initializers' => [__DIR__ . '/data/methods/new-in-initializers.php']; yield 'method-first-class-callable' => [__DIR__ . '/data/methods/first-class-callable.php']; + yield 'method-hierarchy-in-vendor' => [__DIR__ . '/data/methods/hierarchy-in-vendor.php']; yield 'method-overwriting-1' => [__DIR__ . '/data/methods/overwriting-methods-1.php']; yield 'method-overwriting-2' => [__DIR__ . '/data/methods/overwriting-methods-2.php']; yield 'method-overwriting-3' => [__DIR__ . '/data/methods/overwriting-methods-3.php']; diff --git a/tests/Rule/data/methods/hierarchy-in-vendor.php b/tests/Rule/data/methods/hierarchy-in-vendor.php new file mode 100644 index 0000000..5a53dca --- /dev/null +++ b/tests/Rule/data/methods/hierarchy-in-vendor.php @@ -0,0 +1,18 @@ + $class + */ +function test(string $class): void { + $class::someMethod(); +} diff --git a/tests/Rule/data/providers/reflection.php b/tests/Rule/data/providers/reflection.php index a9209e1..847a19b 100644 --- a/tests/Rule/data/providers/reflection.php +++ b/tests/Rule/data/providers/reflection.php @@ -2,6 +2,8 @@ namespace Reflection; +use PHPUnit\Framework\TestCase; +use ShipMonk\PHPStan\DeadCode\Rule\RuleTestCase; interface MyParent {