From 888d7098d0b34497d62b601671f5769ce62038fe Mon Sep 17 00:00:00 2001 From: Jan Nedbal Date: Tue, 11 Mar 2025 14:07:33 +0100 Subject: [PATCH 01/15] Allow debugging of usages --- phpcs.xml.dist | 3 +- rules.neon | 7 + src/Collector/ConstantFetchCollector.php | 4 +- src/Collector/MethodCallCollector.php | 11 +- src/Collector/ProvidedUsagesCollector.php | 22 +- src/Enum/NeverReportedReason.php | 12 + src/Graph/ClassConstantUsage.php | 4 +- src/Graph/ClassMemberUsage.php | 25 +- src/Graph/ClassMethodUsage.php | 4 +- src/Graph/CollectedUsage.php | 14 +- src/Graph/UsageOrigin.php | 119 ++++++++++ src/Graph/UsageOriginDetector.php | 18 +- src/Provider/DoctrineUsageProvider.php | 6 +- src/Provider/PhpUnitUsageProvider.php | 9 +- .../ReflectionBasedMemberUsageProvider.php | 5 +- src/Provider/ReflectionUsageProvider.php | 34 ++- src/Provider/SymfonyUsageProvider.php | 28 +-- src/Rule/DeadCodeRule.php | 224 +++++++++++++++++- tests/AllServicesInConfigTest.php | 4 + tests/Graph/SerializationTest.php | 18 +- tests/Rule/DeadCodeRuleTest.php | 16 +- 21 files changed, 450 insertions(+), 137 deletions(-) create mode 100644 src/Enum/NeverReportedReason.php create mode 100644 src/Graph/UsageOrigin.php diff --git a/phpcs.xml.dist b/phpcs.xml.dist index 124aafd5..58b3cc6e 100644 --- a/phpcs.xml.dist +++ b/phpcs.xml.dist @@ -30,7 +30,7 @@ - + @@ -196,7 +196,6 @@ - diff --git a/rules.neon b/rules.neon index e85ef4a2..965df133 100644 --- a/rules.neon +++ b/rules.neon @@ -106,6 +106,8 @@ services: arguments: reportTransitivelyDeadMethodAsSeparateError: %shipmonkDeadCode.reportTransitivelyDeadMethodAsSeparateError% trackMixedAccess: %shipmonkDeadCode.trackMixedAccess% + debugMembers: %shipmonkDeadCode.debug.usagesOf% + editorUrl: %editorUrl% - class: ShipMonk\PHPStan\DeadCode\Compatibility\BackwardCompatibilityChecker @@ -137,6 +139,8 @@ parameters: tests: enabled: false devPaths: null + debug: + usagesOf: [] parametersSchema: shipmonkDeadCode: structure([ @@ -172,4 +176,7 @@ parametersSchema: devPaths: schema(listOf(string()), nullable()) ]) ]) + debug: structure([ + usagesOf: listOf(string()) + ]) ]) diff --git a/src/Collector/ConstantFetchCollector.php b/src/Collector/ConstantFetchCollector.php index 07d9c11e..2670d17b 100644 --- a/src/Collector/ConstantFetchCollector.php +++ b/src/Collector/ConstantFetchCollector.php @@ -125,7 +125,7 @@ private function registerFunctionCall(FuncCall $node, Scope $scope): void $this->registerUsage( new ClassConstantUsage( - $this->usageOriginDetector->detectOrigin($scope), + $this->usageOriginDetector->detectOrigin($node, $scope), new ClassConstantRef($className, $constantName, true), ), $node, @@ -157,7 +157,7 @@ private function registerFetch(ClassConstFetch $node, Scope $scope): void foreach ($this->getDeclaringTypesWithConstant($ownerType, $constantName) as $className) { $this->registerUsage( new ClassConstantUsage( - $this->usageOriginDetector->detectOrigin($scope), + $this->usageOriginDetector->detectOrigin($node, $scope), new ClassConstantRef($className, $constantName, $possibleDescendantFetch), ), $node, diff --git a/src/Collector/MethodCallCollector.php b/src/Collector/MethodCallCollector.php index a6b8e6b4..a1063e09 100644 --- a/src/Collector/MethodCallCollector.php +++ b/src/Collector/MethodCallCollector.php @@ -25,6 +25,7 @@ use ShipMonk\PHPStan\DeadCode\Graph\ClassMethodRef; use ShipMonk\PHPStan\DeadCode\Graph\ClassMethodUsage; use ShipMonk\PHPStan\DeadCode\Graph\CollectedUsage; +use ShipMonk\PHPStan\DeadCode\Graph\UsageOrigin; use ShipMonk\PHPStan\DeadCode\Graph\UsageOriginDetector; /** @@ -133,7 +134,7 @@ private function registerMethodCall( foreach ($this->getDeclaringTypesWithMethod($methodName, $callerType, TrinaryLogic::createNo(), $possibleDescendantCall) as $methodRef) { $this->registerUsage( new ClassMethodUsage( - $this->usageOriginDetector->detectOrigin($scope), + $this->usageOriginDetector->detectOrigin($methodCall, $scope), $methodRef, ), $methodCall, @@ -163,7 +164,7 @@ private function registerStaticCall( foreach ($this->getDeclaringTypesWithMethod($methodName, $callerType, TrinaryLogic::createYes(), $possibleDescendantCall) as $methodRef) { $this->registerUsage( new ClassMethodUsage( - $this->usageOriginDetector->detectOrigin($scope), + $this->usageOriginDetector->detectOrigin($staticCall, $scope), $methodRef, ), $staticCall, @@ -189,7 +190,7 @@ private function registerArrayCallable( foreach ($this->getDeclaringTypesWithMethod($methodName, $caller, TrinaryLogic::createMaybe()) as $methodRef) { $this->registerUsage( new ClassMethodUsage( - $this->usageOriginDetector->detectOrigin($scope), + $this->usageOriginDetector->detectOrigin($array, $scope), $methodRef, ), $array, @@ -205,7 +206,7 @@ private function registerAttribute(Attribute $node, Scope $scope): void { $this->registerUsage( new ClassMethodUsage( - null, + UsageOrigin::fromScope($node, $scope), new ClassMethodRef($scope->resolveName($node->name), '__construct', false), ), $node, @@ -221,7 +222,7 @@ private function registerClone(Clone_ $node, Scope $scope): void foreach ($this->getDeclaringTypesWithMethod($methodName, $callerType, TrinaryLogic::createNo()) as $methodRef) { $this->registerUsage( new ClassMethodUsage( - $this->usageOriginDetector->detectOrigin($scope), + $this->usageOriginDetector->detectOrigin($node, $scope), $methodRef, ), $node, diff --git a/src/Collector/ProvidedUsagesCollector.php b/src/Collector/ProvidedUsagesCollector.php index 6df74e3b..fb26a570 100644 --- a/src/Collector/ProvidedUsagesCollector.php +++ b/src/Collector/ProvidedUsagesCollector.php @@ -86,12 +86,13 @@ private function validateUsage( $memberRef = $usage->getMemberRef(); $memberRefClass = $memberRef->getClassName(); - $originRef = $usage->getOrigin(); - $originRefClass = $originRef === null ? null : $originRef->getClassName(); + $origin = $usage->getOrigin(); + $originClass = $origin->getClassName(); + $originMethod = $origin->getMethodName(); $context = sprintf( - "It was emitted as %s by %s for node '%s' in '%s' on line %s", - $usage->toHumanString(), + "It emitted usage of %s by %s for node '%s' in '%s' on line %s", + $usage->getMemberRef()->toHumanString(), get_class($provider), get_class($node), $scope->getFile(), @@ -102,16 +103,13 @@ private function validateUsage( throw new LogicException("Class '$memberRefClass' does not exist. $context"); } - if ( - $originRef !== null - && $originRefClass !== null - ) { - if (!$this->reflectionProvider->hasClass($originRefClass)) { - throw new LogicException("Class '{$originRefClass}' does not exist. $context"); + if ($originClass !== null) { + if (!$this->reflectionProvider->hasClass($originClass)) { + throw new LogicException("Class '{$originClass}' does not exist. $context"); } - if (!$this->reflectionProvider->getClass($originRefClass)->hasMethod($originRef->getMemberName())) { - throw new LogicException("Method '{$originRef->getMemberName()}' does not exist in class '$originRefClass'. $context"); + if ($originMethod !== null && !$this->reflectionProvider->getClass($originClass)->hasMethod($originMethod)) { + throw new LogicException("Method '{$originMethod}' does not exist in class '$originClass'. $context"); } } } diff --git a/src/Enum/NeverReportedReason.php b/src/Enum/NeverReportedReason.php new file mode 100644 index 00000000..4cd3414f --- /dev/null +++ b/src/Enum/NeverReportedReason.php @@ -0,0 +1,12 @@ +isPossibleDescendant()) { - throw new LogicException('Origin should always be exact place in codebase.'); - } - - if ($origin !== null && $origin->getClassName() === null) { - throw new LogicException('Origin should always be exact place in codebase, thus className should be known.'); - } - $this->origin = $origin; } - public function getOrigin(): ?ClassMethodRef + public function getOrigin(): UsageOrigin { return $this->origin; } @@ -49,12 +38,4 @@ abstract public function getMemberRef(): ClassMemberRef; */ abstract public function concretizeMixedUsage(string $className): self; - public function toHumanString(): string - { - $origin = $this->origin !== null ? $this->origin->toHumanString() : 'unknown'; - $callee = $this->getMemberRef()->toHumanString(); - - return "$origin -> $callee"; - } - } diff --git a/src/Graph/ClassMethodUsage.php b/src/Graph/ClassMethodUsage.php index 6630bcb6..e900c019 100644 --- a/src/Graph/ClassMethodUsage.php +++ b/src/Graph/ClassMethodUsage.php @@ -14,11 +14,11 @@ final class ClassMethodUsage extends ClassMemberUsage private ClassMethodRef $callee; /** - * @param ClassMethodRef|null $origin The method where the call occurs + * @param UsageOrigin $origin The method where the call occurs * @param ClassMethodRef $callee The method being called */ public function __construct( - ?ClassMethodRef $origin, + UsageOrigin $origin, ClassMethodRef $callee ) { diff --git a/src/Graph/CollectedUsage.php b/src/Graph/CollectedUsage.php index 9a7cfa46..780d9bee 100644 --- a/src/Graph/CollectedUsage.php +++ b/src/Graph/CollectedUsage.php @@ -60,12 +60,12 @@ public function serialize(): string $data = [ 'e' => $this->excludedBy, 't' => $this->usage->getMemberType(), - 'o' => $origin === null - ? null - : [ + 'o' => [ 'c' => $origin->getClassName(), - 'm' => $origin->getMemberName(), - 'd' => $origin->isPossibleDescendant(), + 'm' => $origin->getMethodName(), + 'f' => $origin->getFile(), + 'l' => $origin->getLine(), + 'r' => $origin->getReason(), ], 'm' => [ 'c' => $memberRef->getClassName(), @@ -84,14 +84,14 @@ public function serialize(): string public static function deserialize(string $data): self { try { - /** @var array{e: string|null, t: MemberType::*, o: array{c: string|null, m: string, d: bool}|null, m: array{c: string|null, m: string, d: bool}} $result */ + /** @var array{e: string|null, t: MemberType::*, o: array{c: string|null, m: string|null, f: string|null, l: int|null, r: string|null}, m: array{c: string|null, m: string, d: bool}} $result */ $result = json_decode($data, true, 3, JSON_THROW_ON_ERROR); } catch (JsonException $e) { throw new LogicException('Deserialization failure: ' . $e->getMessage(), 0, $e); } $memberType = $result['t']; - $origin = $result['o'] === null ? null : new ClassMethodRef($result['o']['c'], $result['o']['m'], $result['o']['d']); + $origin = new UsageOrigin($result['o']['c'], $result['o']['m'], $result['o']['f'], $result['o']['l'], $result['o']['r']); $exclusionReason = $result['e']; $usage = $memberType === MemberType::CONSTANT diff --git a/src/Graph/UsageOrigin.php b/src/Graph/UsageOrigin.php new file mode 100644 index 00000000..7d7aa191 --- /dev/null +++ b/src/Graph/UsageOrigin.php @@ -0,0 +1,119 @@ +className = $className; + $this->methodName = $methodName; + $this->fileName = $fileName; + $this->line = $line; + $this->reason = $reason; + } + + /** + * @param class-string $providerClass + * @param ?string $reason More detailed identification why provider emitted this usage + */ + public static function fromProvider(string $providerClass, ?string $reason): self + { + return new self( + null, + null, + null, + null, + $providerClass . ' - ' . (string) $reason, // TODO better approach? + ); + } + + public static function fromScope(Node $node, Scope $scope): self + { + if (!$scope->isInClass() || !$scope->getFunction() instanceof MethodReflection) { + return new self( + null, + null, + $scope->getFile(), + $node->getStartLine(), + ); + } + + return new self( + $scope->getClassReflection()->getName(), + $scope->getFunction()->getName(), + $scope->getFile(), + $node->getStartLine(), + ); + } + + public function getClassName(): ?string + { + return $this->className; + } + + public function getMethodName(): ?string + { + return $this->methodName; + } + + public function getFile(): ?string + { + return $this->fileName; + } + + public function getLine(): ?int + { + return $this->line; + } + + public function getReason(): ?string + { + return $this->reason; + } + + public function hasClassMethodRef(): bool + { + return $this->className !== null && $this->methodName !== null; + } + + public function toClassMethodRef(): ClassMethodRef + { + if ($this->className === null || $this->methodName === null) { + throw new LogicException('Usage origin does not have class method ref'); + } + + return new ClassMethodRef( + $this->className, + $this->methodName, + false, + ); + } + +} diff --git a/src/Graph/UsageOriginDetector.php b/src/Graph/UsageOriginDetector.php index 03911259..5873854f 100644 --- a/src/Graph/UsageOriginDetector.php +++ b/src/Graph/UsageOriginDetector.php @@ -2,8 +2,8 @@ namespace ShipMonk\PHPStan\DeadCode\Graph; +use PhpParser\Node; use PHPStan\Analyser\Scope; -use PHPStan\Reflection\MethodReflection; class UsageOriginDetector { @@ -11,21 +11,9 @@ class UsageOriginDetector /** * Most of the time, this is the only implementation you need for ClassMemberUsage constructor */ - public function detectOrigin(Scope $scope): ?ClassMethodRef + public function detectOrigin(Node $node, Scope $scope): UsageOrigin { - if (!$scope->isInClass()) { - return null; - } - - if (!$scope->getFunction() instanceof MethodReflection) { - return null; - } - - return new ClassMethodRef( - $scope->getClassReflection()->getName(), - $scope->getFunction()->getName(), - false, - ); + return UsageOrigin::fromScope($node, $scope); } } diff --git a/src/Provider/DoctrineUsageProvider.php b/src/Provider/DoctrineUsageProvider.php index e50c9cf7..542bc97f 100644 --- a/src/Provider/DoctrineUsageProvider.php +++ b/src/Provider/DoctrineUsageProvider.php @@ -13,6 +13,7 @@ use PHPStan\Reflection\MethodReflection; use ShipMonk\PHPStan\DeadCode\Graph\ClassMethodRef; use ShipMonk\PHPStan\DeadCode\Graph\ClassMethodUsage; +use ShipMonk\PHPStan\DeadCode\Graph\UsageOrigin; class DoctrineUsageProvider implements MemberUsageProvider { @@ -100,12 +101,13 @@ private function getUsagesOfEventSubscriber(Return_ $node, Scope $scope): array $className = $scope->getClassReflection()->getName(); $usages = []; + $usageOrigin = UsageOrigin::fromScope($node, $scope); foreach ($scope->getType($node->expr)->getConstantArrays() as $rootArray) { foreach ($rootArray->getValuesArray()->getValueTypes() as $eventConfig) { foreach ($eventConfig->getConstantStrings() as $subscriberMethodString) { $usages[] = new ClassMethodUsage( - null, + $usageOrigin, new ClassMethodRef( $className, $subscriberMethodString->getValue(), @@ -199,7 +201,7 @@ private function isDoctrineInstalled(): bool private function createMethodUsage(ExtendedMethodReflection $methodReflection): ClassMethodUsage { return new ClassMethodUsage( - null, + UsageOrigin::fromProvider(self::class, null), new ClassMethodRef( $methodReflection->getDeclaringClass()->getName(), $methodReflection->getName(), diff --git a/src/Provider/PhpUnitUsageProvider.php b/src/Provider/PhpUnitUsageProvider.php index 2911bee6..dc6f65f7 100644 --- a/src/Provider/PhpUnitUsageProvider.php +++ b/src/Provider/PhpUnitUsageProvider.php @@ -14,6 +14,7 @@ use PHPUnit\Framework\TestCase; use ShipMonk\PHPStan\DeadCode\Graph\ClassMethodRef; use ShipMonk\PHPStan\DeadCode\Graph\ClassMethodUsage; +use ShipMonk\PHPStan\DeadCode\Graph\UsageOrigin; use function array_merge; use function is_string; use function strpos; @@ -56,12 +57,12 @@ public function getUsages(Node $node, Scope $scope): array foreach ($dataProviders as $dataProvider) { if ($classReflection->hasNativeMethod($dataProvider)) { - $usages[] = $this->createUsage($classReflection->getNativeMethod($dataProvider)); + $usages[] = $this->createUsage($classReflection->getNativeMethod($dataProvider), 'data provider method'); } } if ($this->isTestCaseMethod($method)) { - $usages[] = $this->createUsage($classReflection->getNativeMethod($method->getName())); + $usages[] = $this->createUsage($classReflection->getNativeMethod($method->getName()), 'test method'); } } @@ -141,10 +142,10 @@ private function hasAnnotation(ReflectionMethod $method, string $string): bool return strpos($method->getDocComment(), $string) !== false; } - private function createUsage(ExtendedMethodReflection $getNativeMethod): ClassMethodUsage + private function createUsage(ExtendedMethodReflection $getNativeMethod, string $reason): ClassMethodUsage { return new ClassMethodUsage( - null, + UsageOrigin::fromProvider(self::class, $reason), new ClassMethodRef( $getNativeMethod->getDeclaringClass()->getName(), $getNativeMethod->getName(), diff --git a/src/Provider/ReflectionBasedMemberUsageProvider.php b/src/Provider/ReflectionBasedMemberUsageProvider.php index c7626c21..e2ad1d20 100644 --- a/src/Provider/ReflectionBasedMemberUsageProvider.php +++ b/src/Provider/ReflectionBasedMemberUsageProvider.php @@ -13,6 +13,7 @@ use ShipMonk\PHPStan\DeadCode\Graph\ClassMemberUsage; use ShipMonk\PHPStan\DeadCode\Graph\ClassMethodRef; use ShipMonk\PHPStan\DeadCode\Graph\ClassMethodUsage; +use ShipMonk\PHPStan\DeadCode\Graph\UsageOrigin; use function array_merge; abstract class ReflectionBasedMemberUsageProvider implements MemberUsageProvider @@ -92,7 +93,7 @@ private function getConstantUsages(ClassReflection $classReflection): array private function createConstantUsage(ReflectionClassConstant $constantReflection): ClassConstantUsage { return new ClassConstantUsage( - null, + UsageOrigin::fromProvider(self::class, null), new ClassConstantRef( $constantReflection->getDeclaringClass()->getName(), $constantReflection->getName(), @@ -104,7 +105,7 @@ private function createConstantUsage(ReflectionClassConstant $constantReflection private function createMethodUsage(ReflectionMethod $methodReflection): ClassMethodUsage { return new ClassMethodUsage( - null, + UsageOrigin::fromProvider(self::class, null), new ClassMethodRef( $methodReflection->getDeclaringClass()->getName(), $methodReflection->getName(), diff --git a/src/Provider/ReflectionUsageProvider.php b/src/Provider/ReflectionUsageProvider.php index 3f279aa7..b69ba03c 100644 --- a/src/Provider/ReflectionUsageProvider.php +++ b/src/Provider/ReflectionUsageProvider.php @@ -73,11 +73,11 @@ private function processMethodCall(MethodCall $node, Scope $scope): array foreach ($genericType->getObjectClassReflections() as $genericReflection) { $usedConstants = [ ...$usedConstants, - ...$this->extractConstantsUsedByReflection($methodName, $genericReflection, $node->getArgs(), $scope), + ...$this->extractConstantsUsedByReflection($methodName, $genericReflection, $node->getArgs(), $node, $scope), ]; $usedMethods = [ ...$usedMethods, - ...$this->extractMethodsUsedByReflection($methodName, $genericReflection, $node->getArgs(), $scope), + ...$this->extractMethodsUsedByReflection($methodName, $genericReflection, $node->getArgs(), $node, $scope), ]; } } @@ -98,6 +98,7 @@ private function extractConstantsUsedByReflection( string $methodName, ClassReflection $genericReflection, array $args, + Node $node, Scope $scope ): array { @@ -105,7 +106,7 @@ private function extractConstantsUsedByReflection( if ($methodName === 'getConstants' || $methodName === 'getReflectionConstants') { foreach ($genericReflection->getNativeReflection()->getReflectionConstants() as $reflectionConstant) { - $usedConstants[] = $this->createConstantUsage($scope, $reflectionConstant->getDeclaringClass()->getName(), $reflectionConstant->getName()); + $usedConstants[] = $this->createConstantUsage($node, $scope, $reflectionConstant->getDeclaringClass()->getName(), $reflectionConstant->getName()); } } @@ -113,7 +114,7 @@ private function extractConstantsUsedByReflection( $firstArg = $args[array_key_first($args)]; // @phpstan-ignore offsetAccess.notFound foreach ($scope->getType($firstArg->value)->getConstantStrings() as $constantString) { - $usedConstants[] = $this->createConstantUsage($scope, $genericReflection->getName(), $constantString->getValue()); + $usedConstants[] = $this->createConstantUsage($node, $scope, $genericReflection->getName(), $constantString->getValue()); } } @@ -128,6 +129,7 @@ private function extractMethodsUsedByReflection( string $methodName, ClassReflection $genericReflection, array $args, + Node $node, Scope $scope ): array { @@ -135,7 +137,7 @@ private function extractMethodsUsedByReflection( if ($methodName === 'getMethods') { foreach ($genericReflection->getNativeReflection()->getMethods() as $reflectionMethod) { - $usedMethods[] = $this->createMethodUsage($scope, $reflectionMethod->getDeclaringClass()->getName(), $reflectionMethod->getName()); + $usedMethods[] = $this->createMethodUsage($node, $scope, $reflectionMethod->getDeclaringClass()->getName(), $reflectionMethod->getName()); } } @@ -143,7 +145,7 @@ private function extractMethodsUsedByReflection( $firstArg = $args[array_key_first($args)]; // @phpstan-ignore offsetAccess.notFound foreach ($scope->getType($firstArg->value)->getConstantStrings() as $constantString) { - $usedMethods[] = $this->createMethodUsage($scope, $genericReflection->getName(), $constantString->getValue()); + $usedMethods[] = $this->createMethodUsage($node, $scope, $genericReflection->getName(), $constantString->getValue()); } } @@ -151,7 +153,7 @@ private function extractMethodsUsedByReflection( $constructor = $genericReflection->getNativeReflection()->getConstructor(); if ($constructor !== null) { - $usedMethods[] = $this->createMethodUsage($scope, $constructor->getDeclaringClass()->getName(), '__construct'); + $usedMethods[] = $this->createMethodUsage($node, $scope, $constructor->getDeclaringClass()->getName(), '__construct'); } } @@ -181,10 +183,15 @@ private function getMethodNames(CallLike $call, Scope $scope): array return [$call->name->toString()]; } - private function createConstantUsage(Scope $scope, string $className, string $constantName): ClassConstantUsage + private function createConstantUsage( + Node $node, + Scope $scope, + string $className, + string $constantName + ): ClassConstantUsage { return new ClassConstantUsage( - $this->usageOriginDetector->detectOrigin($scope), + $this->usageOriginDetector->detectOrigin($node, $scope), new ClassConstantRef( $className, $constantName, @@ -193,10 +200,15 @@ private function createConstantUsage(Scope $scope, string $className, string $co ); } - private function createMethodUsage(Scope $scope, string $className, string $methodName): ClassMethodUsage + private function createMethodUsage( + Node $node, + Scope $scope, + string $className, + string $methodName + ): ClassMethodUsage { return new ClassMethodUsage( - $this->usageOriginDetector->detectOrigin($scope), + $this->usageOriginDetector->detectOrigin($node, $scope), new ClassMethodRef( $className, $methodName, diff --git a/src/Provider/SymfonyUsageProvider.php b/src/Provider/SymfonyUsageProvider.php index 1b6902e6..543e928b 100644 --- a/src/Provider/SymfonyUsageProvider.php +++ b/src/Provider/SymfonyUsageProvider.php @@ -27,7 +27,7 @@ use ShipMonk\PHPStan\DeadCode\Graph\ClassConstantUsage; use ShipMonk\PHPStan\DeadCode\Graph\ClassMethodRef; use ShipMonk\PHPStan\DeadCode\Graph\ClassMethodUsage; -use ShipMonk\PHPStan\DeadCode\Graph\UsageOriginDetector; +use ShipMonk\PHPStan\DeadCode\Graph\UsageOrigin; use SimpleXMLElement; use SplFileInfo; use UnexpectedValueException; @@ -64,16 +64,12 @@ class SymfonyUsageProvider implements MemberUsageProvider */ private array $dicConstants = []; - private UsageOriginDetector $usageOriginDetector; - public function __construct( Container $container, - UsageOriginDetector $usageOriginDetector, ?bool $enabled, ?string $configDir ) { - $this->usageOriginDetector = $usageOriginDetector; $this->enabled = $enabled ?? $this->isSymfonyInstalled(); $resolvedConfigDir = $configDir ?? $this->autodetectConfigDir(); $containerXmlPath = $this->getContainerXmlPath($container); @@ -148,6 +144,7 @@ private function getUsagesOfEventSubscriber(Return_ $node, Scope $scope): array $className = $scope->getClassReflection()->getName(); $usages = []; + $usageOrigin = UsageOrigin::fromScope($node, $scope); // phpcs:disable Squiz.PHP.CommentedOutCode.Found foreach ($scope->getType($node->expr)->getConstantArrays() as $rootArray) { @@ -155,7 +152,7 @@ private function getUsagesOfEventSubscriber(Return_ $node, Scope $scope): array // ['eventName' => 'methodName'] foreach ($eventConfig->getConstantStrings() as $subscriberMethodString) { $usages[] = new ClassMethodUsage( - null, + $usageOrigin, new ClassMethodRef( $className, $subscriberMethodString->getValue(), @@ -168,7 +165,7 @@ private function getUsagesOfEventSubscriber(Return_ $node, Scope $scope): array foreach ($eventConfig->getConstantArrays() as $subscriberMethodArray) { foreach ($subscriberMethodArray->getFirstIterableValueType()->getConstantStrings() as $subscriberMethodString) { $usages[] = new ClassMethodUsage( - null, + $usageOrigin, new ClassMethodRef( $className, $subscriberMethodString->getValue(), @@ -183,7 +180,7 @@ private function getUsagesOfEventSubscriber(Return_ $node, Scope $scope): array foreach ($subscriberMethodArray->getIterableValueType()->getConstantArrays() as $innerArray) { foreach ($innerArray->getFirstIterableValueType()->getConstantStrings() as $subscriberMethodString) { $usages[] = new ClassMethodUsage( - null, + $usageOrigin, new ClassMethodRef( $className, $subscriberMethodString->getValue(), @@ -214,7 +211,7 @@ private function getMethodUsagesFromReflection(InClassNode $node): array foreach ($nativeReflection->getMethods() as $method) { if (isset($this->dicCalls[$className][$method->getName()])) { - $usages[] = $this->createUsage($classReflection->getNativeMethod($method->getName())); + $usages[] = $this->createUsage($classReflection->getNativeMethod($method->getName()), 'called via DIC'); } if ($method->getDeclaringClass()->getName() !== $nativeReflection->getName()) { @@ -222,7 +219,7 @@ private function getMethodUsagesFromReflection(InClassNode $node): array } if ($this->shouldMarkAsUsed($method)) { - $usages[] = $this->createUsage($classReflection->getNativeMethod($method->getName())); + $usages[] = $this->createUsage($classReflection->getNativeMethod($method->getName()), null); } } @@ -235,6 +232,7 @@ private function getMethodUsagesFromReflection(InClassNode $node): array private function getMethodUsagesFromAttributeReflection(InClassMethodNode $node, Scope $scope): array { $usages = []; + $usageOrigin = UsageOrigin::fromScope($node, $scope); foreach ($node->getMethodReflection()->getParameters() as $parameter) { foreach ($parameter->getAttributes() as $attributeReflection) { @@ -259,7 +257,7 @@ private function getMethodUsagesFromAttributeReflection(InClassMethodNode $node, foreach ($classNames as $className) { $usages[] = new ClassMethodUsage( - $this->usageOriginDetector->detectOrigin($scope), + $usageOrigin, new ClassMethodRef( $className->getValue(), $defaultIndexMethod[0]->getValue(), @@ -283,7 +281,7 @@ private function getMethodUsagesFromAttributeReflection(InClassMethodNode $node, foreach ($classNames as $className) { $usages[] = new ClassMethodUsage( - $this->usageOriginDetector->detectOrigin($scope), + UsageOrigin::fromScope($node, $scope), new ClassMethodRef( $className->getValue(), $defaultIndexMethod[0]->getValue(), @@ -481,10 +479,10 @@ private function isSymfonyInstalled(): bool || InstalledVersions::isInstalled('symfony/dependency-injection'); } - private function createUsage(ExtendedMethodReflection $methodReflection): ClassMethodUsage + private function createUsage(ExtendedMethodReflection $methodReflection, ?string $reason): ClassMethodUsage { return new ClassMethodUsage( - null, + UsageOrigin::fromProvider(self::class, $reason), new ClassMethodRef( $methodReflection->getDeclaringClass()->getName(), $methodReflection->getName(), @@ -570,7 +568,7 @@ private function getConstantUsages(ClassReflection $classReflection): array } $usages[] = new ClassConstantUsage( - null, + UsageOrigin::fromProvider(self::class, 'used in DIC'), new ClassConstantRef( $classReflection->getName(), $constantName, diff --git a/src/Rule/DeadCodeRule.php b/src/Rule/DeadCodeRule.php index 31648930..c5d3053c 100644 --- a/src/Rule/DeadCodeRule.php +++ b/src/Rule/DeadCodeRule.php @@ -7,7 +7,9 @@ use PHPStan\Analyser\Scope; use PHPStan\Command\Output; use PHPStan\Diagnose\DiagnoseExtension; +use PHPStan\File\RelativePathHelper; use PHPStan\Node\CollectedDataNode; +use PHPStan\Reflection\ReflectionProvider; use PHPStan\Rules\IdentifierRuleError; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; @@ -18,6 +20,7 @@ use ShipMonk\PHPStan\DeadCode\Compatibility\BackwardCompatibilityChecker; use ShipMonk\PHPStan\DeadCode\Enum\ClassLikeKind; use ShipMonk\PHPStan\DeadCode\Enum\MemberType; +use ShipMonk\PHPStan\DeadCode\Enum\NeverReportedReason; use ShipMonk\PHPStan\DeadCode\Enum\Visibility; use ShipMonk\PHPStan\DeadCode\Error\BlackMember; use ShipMonk\PHPStan\DeadCode\Graph\ClassConstantRef; @@ -25,6 +28,7 @@ use ShipMonk\PHPStan\DeadCode\Graph\ClassMemberUsage; use ShipMonk\PHPStan\DeadCode\Graph\ClassMethodRef; use ShipMonk\PHPStan\DeadCode\Graph\CollectedUsage; +use ShipMonk\PHPStan\DeadCode\Graph\UsageOrigin; use ShipMonk\PHPStan\DeadCode\Hierarchy\ClassHierarchy; use function array_key_exists; use function array_keys; @@ -34,9 +38,12 @@ use function array_slice; use function array_sum; use function array_values; +use function count; +use function explode; use function in_array; use function ksort; use function sprintf; +use function str_replace; use function strpos; /** @@ -66,8 +73,14 @@ class DeadCodeRule implements Rule, DiagnoseExtension '__debugInfo' => null, ]; + private RelativePathHelper $relativePathHelper; + + private ReflectionProvider $reflectionProvider; + private ClassHierarchy $classHierarchy; + private ?string $editorUrl; + /** * typename => data * @@ -93,6 +106,13 @@ class DeadCodeRule implements Rule, DiagnoseExtension private bool $trackMixedAccess; + /** + * memberKey => usage info + * + * @var array, eliminationPath?: list, neverReported?: string}> + */ + private array $debugMembers; + /** * memberKey => DeadMember * @@ -112,16 +132,27 @@ class DeadCodeRule implements Rule, DiagnoseExtension */ private array $usageGraph = []; + /** + * @param list $debugMembers + */ public function __construct( + RelativePathHelper $relativePathHelper, + ReflectionProvider $reflectionProvider, ClassHierarchy $classHierarchy, + ?string $editorUrl, bool $reportTransitivelyDeadMethodAsSeparateError, bool $trackMixedAccess, + array $debugMembers, BackwardCompatibilityChecker $checker ) { + $this->relativePathHelper = $relativePathHelper; + $this->reflectionProvider = $reflectionProvider; $this->classHierarchy = $classHierarchy; + $this->editorUrl = $editorUrl; $this->reportTransitivelyDeadAsSeparateError = $reportTransitivelyDeadMethodAsSeparateError; $this->trackMixedAccess = $trackMixedAccess; + $this->debugMembers = $this->buildDebugMemberKeys($debugMembers); $checker->check(); } @@ -162,6 +193,8 @@ public function processNode( $collectedUsage = CollectedUsage::deserialize($useString); $memberUsage = $collectedUsage->getUsage(); + $this->recordDebugUsage($collectedUsage); + if ($memberUsage->getMemberRef()->getClassName() === null) { $this->mixedMemberUsages[$memberUsage->getMemberType()][$memberUsage->getMemberRef()->getMemberName()][] = $collectedUsage; continue; @@ -239,7 +272,9 @@ public function processNode( $isWhite = $this->isConsideredWhite($memberUsage); $alternativeMemberKeys = $this->getAlternativeMemberKeys($memberUsage->getMemberRef()); - $alternativeOriginKeys = $memberUsage->getOrigin() !== null ? $this->getAlternativeMemberKeys($memberUsage->getOrigin()) : []; + $alternativeOriginKeys = $memberUsage->getOrigin()->hasClassMethodRef() + ? $this->getAlternativeMemberKeys($memberUsage->getOrigin()->toClassMethodRef()) + : []; foreach ($alternativeMemberKeys as $alternativeMemberKey) { foreach ($alternativeOriginKeys as $alternativeOriginKey) { @@ -257,7 +292,11 @@ public function processNode( } foreach ($this->blackMembers as $blackMemberKey => $blackMember) { - if ($this->isNeverReportedAsDead($blackMember)) { + $neverReportedReason = $this->isNeverReportedAsDead($blackMember); + + if ($neverReportedReason !== null) { + $this->markDebugMemberAsNeverReported($blackMember, $neverReportedReason); + unset($this->blackMembers[$blackMemberKey]); } } @@ -454,7 +493,11 @@ private function markTransitivesWhite(string $callerKey, array $visitedKeys = [] $visitedKeys = $visitedKeys === [] ? [$callerKey => null] : $visitedKeys; $calleeKeys = $this->usageGraph[$callerKey] ?? []; - unset($this->blackMembers[$callerKey]); + if (isset($this->blackMembers[$callerKey])) { // TODO debug why not always present + $this->markDebugMemberAsWhite($this->blackMembers[$callerKey], array_keys($visitedKeys)); + + unset($this->blackMembers[$callerKey]); + } foreach ($calleeKeys as $calleeKey) { if (array_key_exists($calleeKey, $visitedKeys)) { @@ -673,15 +716,15 @@ private function getTraitUsages(string $typeName): array private function isConsideredWhite(ClassMemberUsage $memberUsage): bool { - return $memberUsage->getOrigin() === null + return $memberUsage->getOrigin()->getClassName() === null || $this->isAnonymousClass($memberUsage->getOrigin()->getClassName()) - || (array_key_exists($memberUsage->getOrigin()->getMemberName(), self::UNSUPPORTED_MAGIC_METHODS)); + || (array_key_exists((string) $memberUsage->getOrigin()->getMethodName(), self::UNSUPPORTED_MAGIC_METHODS)); } - private function isNeverReportedAsDead(BlackMember $blackMember): bool + private function isNeverReportedAsDead(BlackMember $blackMember): ?string { if (!$blackMember->getMember() instanceof ClassMethodRef) { - return false; + return null; } $typeName = $blackMember->getMember()->getClassName(); @@ -699,22 +742,28 @@ private function isNeverReportedAsDead(BlackMember $blackMember): bool if ($kind === ClassLikeKind::TRAIT && $abstract) { // abstract methods in traits make sense (not dead) only when called within the trait itself, but that is hard to detect for now, so lets ignore them completely // the difference from interface methods (or abstract methods) is that those methods can be called over the interface, but you cannot call method over trait - return true; + return NeverReportedReason::ABSTRACT_TRAIT_METHOD; } if ($memberName === '__construct' && ($visibility & Visibility::PRIVATE) !== 0 && $params === 0) { // private constructors with zero parameters are often used to deny instantiation - return true; + return NeverReportedReason::PRIVATE_CONSTRUCTOR_NO_PARAMS; } if (array_key_exists($memberName, self::UNSUPPORTED_MAGIC_METHODS)) { - return true; + return NeverReportedReason::UNSUPPORTED_MAGIC_METHOD; } - return false; + return null; } public function print(Output $output): void + { + $this->printMixedMemberUsages($output); + $this->printDebugMemberUsages($output); + } + + private function printMixedMemberUsages(Output $output): void { if ($this->mixedMemberUsages === [] || !$output->isDebug() || !$this->trackMixedAccess) { return; @@ -760,12 +809,161 @@ public function print(Output $output): void private function getExampleCaller(array $usages): ?string { foreach ($usages as $usage) { - if ($usage->getUsage()->getOrigin() !== null) { - return $usage->getUsage()->getOrigin()->toHumanString(); + $origin = $usage->getUsage()->getOrigin(); + + if ($origin->getFile() !== null) { + return $this->getOriginReference($origin); } } return null; } + private function printDebugMemberUsages(Output $output): void + { + if ($this->debugMembers === [] || !$output->isDebug()) { + return; + } + + $output->writeLineFormatted("\nUsage debugging information:"); + + foreach ($this->debugMembers as $memberKey => $debugMember) { + $output->writeLineFormatted(sprintf("\n%s", $memberKey)); + + if (isset($debugMember['eliminationPath'])) { + $output->writeLineFormatted("|\n| Elimination path:"); + + foreach ($debugMember['eliminationPath'] as $index => $eliminationPath) { + $entrypoint = $index === 0 ? '(entrypoint)' : ''; + $output->writeLineFormatted(sprintf('| -> %s %s', $eliminationPath, $entrypoint)); + } + } + + if (isset($debugMember['neverReported'])) { + $output->writeLineFormatted(sprintf("|\n| Is never reported as dead: %s", $debugMember['neverReported'])); + } + + if (isset($debugMember['usages'])) { + $output->writeLineFormatted(sprintf("|\n| Found %d usages:", count($debugMember['usages']))); + + foreach ($debugMember['usages'] as $collectedUsage) { + $origin = $collectedUsage->getUsage()->getOrigin(); + $output->writeFormatted(sprintf('| • %s', $this->getOriginReference($origin))); + + if ($collectedUsage->isExcluded()) { + $output->writeFormatted(sprintf('| Excluded by %s', $collectedUsage->getExcludedBy())); + } + + $output->writeLineFormatted(''); + } + } + + $output->writeLineFormatted(''); + } + } + + private function getOriginReference(UsageOrigin $origin): string + { + $file = $origin->getFile(); + $line = $origin->getLine(); + + if ($file !== null && $line !== null) { + $relativeFile = $this->relativePathHelper->getRelativePath($file); + + if ($this->editorUrl === null) { + return sprintf( + '%s:%s', + $relativeFile, + $line, + ); + } + + return sprintf( + '%s:%s', + str_replace(['%file%', '%relFile%', '%line%'], [$file, $relativeFile, (string) $line], $this->editorUrl), + $relativeFile, + $line, + ); + } + + if ($origin->getReason() !== null) { + return $origin->getReason(); + } + + throw new LogicException('Unknown state of usage origin'); + } + + /** + * @param list $debugMembers + * @return array, eliminationPath?: list, neverReported?: string}> + */ + private function buildDebugMemberKeys(array $debugMembers): array + { + $result = []; + + foreach ($debugMembers as $debugMember) { + if (strpos($debugMember, '::') === false) { + throw new LogicException("Invalid debug member format: $debugMember"); + } + + [$class, $memberName] = explode('::', $debugMember); // @phpstan-ignore offsetAccess.notFound + + if (!$this->reflectionProvider->hasClass($class)) { + throw new LogicException("Class $class does not exist"); + } + + $classReflection = $this->reflectionProvider->getClass($class); + + if ($classReflection->hasMethod($memberName)) { + $key = ClassMethodRef::buildKey($class, $memberName); + + } elseif ($classReflection->hasConstant($memberName)) { + $key = ClassConstantRef::buildKey($class, $memberName); + + } else { + throw new LogicException("Member $memberName does not exist in $class"); + } + + $result[$key] = []; + } + + return $result; + } + + private function recordDebugUsage(CollectedUsage $collectedUsage): void + { + $memberKey = $collectedUsage->getUsage()->getMemberRef()->toKey(); + + if (!isset($this->debugMembers[$memberKey])) { + return; + } + + $this->debugMembers[$memberKey]['usages'][] = $collectedUsage; + } + + /** + * @param list $usagePath + */ + private function markDebugMemberAsWhite(BlackMember $blackMember, array $usagePath): void + { + $memberKey = $blackMember->getMember()->toKey(); + + if (!isset($this->debugMembers[$memberKey])) { + return; + } + + $this->debugMembers[$memberKey]['eliminationPath'] = $usagePath; + } + + private function markDebugMemberAsNeverReported(BlackMember $blackMember, string $reason): void + { + $memberKey = $blackMember->getMember()->toKey(); + + if (!isset($this->debugMembers[$memberKey])) { + return; + } + + $this->debugMembers[$memberKey]['neverReported'] = $reason; + } + } diff --git a/tests/AllServicesInConfigTest.php b/tests/AllServicesInConfigTest.php index 8d1b4a33..f9c9bb95 100644 --- a/tests/AllServicesInConfigTest.php +++ b/tests/AllServicesInConfigTest.php @@ -10,6 +10,7 @@ use ShipMonk\PHPStan\DeadCode\Collector\BufferedUsageCollector; use ShipMonk\PHPStan\DeadCode\Enum\ClassLikeKind; use ShipMonk\PHPStan\DeadCode\Enum\MemberType; +use ShipMonk\PHPStan\DeadCode\Enum\NeverReportedReason; use ShipMonk\PHPStan\DeadCode\Enum\Visibility; use ShipMonk\PHPStan\DeadCode\Error\BlackMember; use ShipMonk\PHPStan\DeadCode\Graph\ClassConstantRef; @@ -19,6 +20,7 @@ use ShipMonk\PHPStan\DeadCode\Graph\ClassMethodRef; use ShipMonk\PHPStan\DeadCode\Graph\ClassMethodUsage; use ShipMonk\PHPStan\DeadCode\Graph\CollectedUsage; +use ShipMonk\PHPStan\DeadCode\Graph\UsageOrigin; use ShipMonk\PHPStan\DeadCode\Provider\MemberUsageProvider; use ShipMonk\PHPStan\DeadCode\Provider\ReflectionBasedMemberUsageProvider; use ShipMonk\PHPStan\DeadCode\Transformer\RemoveClassMemberVisitor; @@ -54,6 +56,7 @@ public function test(): void $iterator = new RecursiveIteratorIterator(new RecursiveDirectoryIterator($directory)); $missingClassNames = []; $excluded = [ + UsageOrigin::class, ClassMethodUsage::class, ClassMethodRef::class, ClassConstantRef::class, @@ -70,6 +73,7 @@ public function test(): void RemoveDeadCodeTransformer::class, RemoveClassMemberVisitor::class, MemberType::class, + NeverReportedReason::class, ]; /** @var DirectoryIterator $file */ diff --git a/tests/Graph/SerializationTest.php b/tests/Graph/SerializationTest.php index 63bdbca3..96679538 100644 --- a/tests/Graph/SerializationTest.php +++ b/tests/Graph/SerializationTest.php @@ -12,10 +12,8 @@ class SerializationTest extends TestCase */ public function testSerialization(CollectedUsage $expected, string $serialized): void { - $unserialized = CollectedUsage::deserialize($serialized); - self::assertSame($serialized, $expected->serialize()); - self::assertEquals($expected, $unserialized); + self::assertEquals($expected, CollectedUsage::deserialize($serialized)); } /** @@ -23,25 +21,15 @@ public function testSerialization(CollectedUsage $expected, string $serialized): */ public static function provideData(): iterable { - yield [ - new CollectedUsage( - new ClassMethodUsage( - null, - new ClassMethodRef('Some', 'method', false), - ), - null, - ), - '{"e":null,"t":1,"o":null,"m":{"c":"Some","m":"method","d":false}}', - ]; yield [ new CollectedUsage( new ClassConstantUsage( - new ClassMethodRef('Clazz', 'method', false), + new UsageOrigin('Clazz', 'method', '/app/index.php', 7, null), new ClassConstantRef(null, 'CONSTANT', true), ), 'excluder', ), - '{"e":"excluder","t":2,"o":{"c":"Clazz","m":"method","d":false},"m":{"c":null,"m":"CONSTANT","d":true}}', + '{"e":"excluder","t":2,"o":{"c":"Clazz","m":"method","f":"\/app\/index.php","l":7,"r":null},"m":{"c":null,"m":"CONSTANT","d":true}}', ]; } diff --git a/tests/Rule/DeadCodeRuleTest.php b/tests/Rule/DeadCodeRuleTest.php index db7ecca7..09f92056 100644 --- a/tests/Rule/DeadCodeRuleTest.php +++ b/tests/Rule/DeadCodeRuleTest.php @@ -11,6 +11,7 @@ use PHPStan\Command\AnalysisResult; use PHPStan\Command\Output; use PHPStan\DependencyInjection\Container; +use PHPStan\File\SimpleRelativePathHelper; use PHPStan\PhpDocParser\Lexer\Lexer; use PHPStan\PhpDocParser\Parser\PhpDocParser; use PHPStan\Reflection\ReflectionProvider; @@ -60,9 +61,13 @@ protected function getRule(): DeadCodeRule { if ($this->rule === null) { $this->rule = new DeadCodeRule( + new SimpleRelativePathHelper(__DIR__), // @phpstan-ignore phpstanApi.constructor + self::createReflectionProvider(), new ClassHierarchy(), + null, !$this->emitErrorsInGroups, true, + [], new BackwardCompatibilityChecker([]), ); } @@ -79,7 +84,7 @@ protected function getCollectors(): array return [ new ProvidedUsagesCollector($reflectionProvider, $this->getMemberUsageProviders(), $this->getMemberUsageExcluders()), - new ClassDefinitionCollector(self::createReflectionProvider()), + new ClassDefinitionCollector($reflectionProvider), new MethodCallCollector($this->createUsageOriginDetector(), $this->trackMixedAccess, $this->getMemberUsageExcluders()), new ConstantFetchCollector($this->createUsageOriginDetector(), $reflectionProvider, $this->trackMixedAccess, $this->getMemberUsageExcluders()), ]; @@ -159,10 +164,10 @@ static function (string $message) use (&$actualOutput): void { $ec = ''; // hack editorconfig checker to ignore wrong indentation $expectedOutput = <<<"OUTPUT" Found 4 usages over unknown type: - $ec • getter1 method, for example in DeadMixed1\Tester::__construct - $ec • getter2 method, for example in DeadMixed1\Tester::__construct - $ec • getter3 method, for example in DeadMixed1\Tester::__construct - $ec • staticMethod method, for example in DeadMixed1\Tester::__construct + $ec • getter1 method, for example in data/methods/mixed/tracked.php:46 + $ec • getter2 method, for example in data/methods/mixed/tracked.php:49 + $ec • getter3 method, for example in data/methods/mixed/tracked.php:52 + $ec • staticMethod method, for example in data/methods/mixed/tracked.php:57 Thus, any member named the same is considered used, no matter its declaring class! @@ -481,7 +486,6 @@ public function shouldMarkMethodAsUsed(ReflectionMethod $method): bool ), new SymfonyUsageProvider( $this->createContainerMockWithSymfonyConfig(), - new UsageOriginDetector(), true, __DIR__ . '/data/providers/symfony/', ), From c89e0942899c7261aa59918b852b601237ac3b0f Mon Sep 17 00:00:00 2001 From: Jan Nedbal Date: Tue, 11 Mar 2025 14:30:31 +0100 Subject: [PATCH 02/15] Move debug printing logic to separate class --- rules.neon | 10 +- src/Debug/DebugUsagePrinter.php | 268 ++++++++++++++++++++++++++++++++ src/Rule/DeadCodeRule.php | 252 ++---------------------------- tests/Rule/DeadCodeRuleTest.php | 13 +- 4 files changed, 292 insertions(+), 251 deletions(-) create mode 100644 src/Debug/DebugUsagePrinter.php diff --git a/rules.neon b/rules.neon index 965df133..8fb96875 100644 --- a/rules.neon +++ b/rules.neon @@ -10,6 +10,13 @@ services: - class: ShipMonk\PHPStan\DeadCode\Graph\UsageOriginDetector + - + class: ShipMonk\PHPStan\DeadCode\Debug\DebugUsagePrinter + arguments: + trackMixedAccess: %shipmonkDeadCode.trackMixedAccess% + debugMembers: %shipmonkDeadCode.debug.usagesOf% + editorUrl: %editorUrl% + - class: ShipMonk\PHPStan\DeadCode\Provider\VendorUsageProvider tags: @@ -105,9 +112,6 @@ services: - phpstan.diagnoseExtension arguments: reportTransitivelyDeadMethodAsSeparateError: %shipmonkDeadCode.reportTransitivelyDeadMethodAsSeparateError% - trackMixedAccess: %shipmonkDeadCode.trackMixedAccess% - debugMembers: %shipmonkDeadCode.debug.usagesOf% - editorUrl: %editorUrl% - class: ShipMonk\PHPStan\DeadCode\Compatibility\BackwardCompatibilityChecker diff --git a/src/Debug/DebugUsagePrinter.php b/src/Debug/DebugUsagePrinter.php new file mode 100644 index 00000000..cdf247c2 --- /dev/null +++ b/src/Debug/DebugUsagePrinter.php @@ -0,0 +1,268 @@ + usage info + * + * @var array, eliminationPath?: list, neverReported?: string}> + */ + private array $debugMembers; + + /** + * @param list $debugMembers + */ + public function __construct( + RelativePathHelper $relativePathHelper, + ReflectionProvider $reflectionProvider, + ?string $editorUrl, + bool $trackMixedAccess, + array $debugMembers + ) + { + $this->relativePathHelper = $relativePathHelper; + $this->reflectionProvider = $reflectionProvider; + $this->editorUrl = $editorUrl; + $this->trackMixedAccess = $trackMixedAccess; + $this->debugMembers = $this->buildDebugMemberKeys($debugMembers); + } + + /** + * @param array>> $mixedMemberUsages + */ + public function printMixedMemberUsages(Output $output, array $mixedMemberUsages): void + { + if ($mixedMemberUsages === [] || !$output->isDebug() || !$this->trackMixedAccess) { + return; + } + + $totalCount = array_sum(array_map('count', $mixedMemberUsages)); + $maxExamplesToShow = 20; + $examplesShown = 0; + $output->writeLineFormatted(sprintf('Found %d usages over unknown type:', $totalCount)); + + foreach ($mixedMemberUsages as $memberType => $collectedUsages) { + foreach ($collectedUsages as $memberName => $usages) { + $examplesShown++; + $memberTypeString = $memberType === MemberType::METHOD ? 'method' : 'constant'; + $output->writeFormatted(sprintf(' • %s %s', $memberName, $memberTypeString)); + + $exampleCaller = $this->getExampleCaller($usages); + + if ($exampleCaller !== null) { + $output->writeFormatted(sprintf(', for example in %s', $exampleCaller)); + } + + $output->writeLineFormatted(''); + + if ($examplesShown >= $maxExamplesToShow) { + break 2; + } + } + } + + if ($totalCount > $maxExamplesToShow) { + $output->writeLineFormatted(sprintf('... and %d more', $totalCount - $maxExamplesToShow)); + } + + $output->writeLineFormatted(''); + $output->writeLineFormatted('Thus, any member named the same is considered used, no matter its declaring class!'); + $output->writeLineFormatted(''); + } + + /** + * @param list $usages + */ + private function getExampleCaller(array $usages): ?string + { + foreach ($usages as $usage) { + $origin = $usage->getUsage()->getOrigin(); + + if ($origin->getFile() !== null) { + return $this->getOriginReference($origin); + } + } + + return null; + } + + public function printDebugMemberUsages(Output $output): void + { + if ($this->debugMembers === [] || !$output->isDebug()) { + return; + } + + $output->writeLineFormatted("\nUsage debugging information:"); + + foreach ($this->debugMembers as $memberKey => $debugMember) { + $output->writeLineFormatted(sprintf("\n%s", $memberKey)); + + if (isset($debugMember['eliminationPath'])) { + $output->writeLineFormatted("|\n| Elimination path:"); + + foreach ($debugMember['eliminationPath'] as $index => $eliminationPath) { + $entrypoint = $index === 0 ? '(entrypoint)' : ''; + $output->writeLineFormatted(sprintf('| -> %s %s', $eliminationPath, $entrypoint)); + } + } + + if (isset($debugMember['neverReported'])) { + $output->writeLineFormatted(sprintf("|\n| Is never reported as dead: %s", $debugMember['neverReported'])); + } + + if (isset($debugMember['usages'])) { + $output->writeLineFormatted(sprintf("|\n| Found %d usages:", count($debugMember['usages']))); + + foreach ($debugMember['usages'] as $collectedUsage) { + $origin = $collectedUsage->getUsage()->getOrigin(); + $output->writeFormatted(sprintf('| • %s', $this->getOriginReference($origin))); + + if ($collectedUsage->isExcluded()) { + $output->writeFormatted(sprintf(' - Excluded by %s', $collectedUsage->getExcludedBy())); + } + + $output->writeLineFormatted(''); + } + } + + $output->writeLineFormatted(''); + } + } + + private function getOriginReference(UsageOrigin $origin): string + { + $file = $origin->getFile(); + $line = $origin->getLine(); + + if ($file !== null && $line !== null) { + $relativeFile = $this->relativePathHelper->getRelativePath($file); + + if ($this->editorUrl === null) { + return sprintf( + '%s:%s', + $relativeFile, + $line, + ); + } + + return sprintf( + '%s:%s', + str_replace(['%file%', '%relFile%', '%line%'], [$file, $relativeFile, (string) $line], $this->editorUrl), + $relativeFile, + $line, + ); + } + + if ($origin->getReason() !== null) { + return $origin->getReason(); + } + + throw new LogicException('Unknown state of usage origin'); + } + + public function recordUsage(CollectedUsage $collectedUsage): void + { + $memberKey = $collectedUsage->getUsage()->getMemberRef()->toKey(); + + if (!isset($this->debugMembers[$memberKey])) { + return; + } + + $this->debugMembers[$memberKey]['usages'][] = $collectedUsage; + } + + /** + * @param list $usagePath + */ + public function markMemberAsWhite(BlackMember $blackMember, array $usagePath): void + { + $memberKey = $blackMember->getMember()->toKey(); + + if (!isset($this->debugMembers[$memberKey])) { + return; + } + + $this->debugMembers[$memberKey]['eliminationPath'] = $usagePath; + } + + public function markMemberAsNeverReported(BlackMember $blackMember, string $reason): void + { + $memberKey = $blackMember->getMember()->toKey(); + + if (!isset($this->debugMembers[$memberKey])) { + return; + } + + $this->debugMembers[$memberKey]['neverReported'] = $reason; + } + + /** + * @param list $debugMembers + * @return array, eliminationPath?: list, neverReported?: string}> + */ + private function buildDebugMemberKeys(array $debugMembers): array + { + $result = []; + + foreach ($debugMembers as $debugMember) { + if (strpos($debugMember, '::') === false) { + throw new LogicException("Invalid debug member format: $debugMember"); + } + + [$class, $memberName] = explode('::', $debugMember); // @phpstan-ignore offsetAccess.notFound + + if (!$this->reflectionProvider->hasClass($class)) { + throw new LogicException("Class $class does not exist"); + } + + $classReflection = $this->reflectionProvider->getClass($class); + + if ($classReflection->hasMethod($memberName)) { + $key = ClassMethodRef::buildKey($class, $memberName); + + } elseif ($classReflection->hasConstant($memberName)) { + $key = ClassConstantRef::buildKey($class, $memberName); + + } elseif ($classReflection->hasProperty($memberName)) { + throw new LogicException('Properties are not yet supported'); + + } else { + throw new LogicException("Member $memberName does not exist in $class"); + } + + $result[$key] = []; + } + + return $result; + } + +} diff --git a/src/Rule/DeadCodeRule.php b/src/Rule/DeadCodeRule.php index c5d3053c..fd9a5611 100644 --- a/src/Rule/DeadCodeRule.php +++ b/src/Rule/DeadCodeRule.php @@ -7,9 +7,7 @@ use PHPStan\Analyser\Scope; use PHPStan\Command\Output; use PHPStan\Diagnose\DiagnoseExtension; -use PHPStan\File\RelativePathHelper; use PHPStan\Node\CollectedDataNode; -use PHPStan\Reflection\ReflectionProvider; use PHPStan\Rules\IdentifierRuleError; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; @@ -18,6 +16,7 @@ use ShipMonk\PHPStan\DeadCode\Collector\MethodCallCollector; use ShipMonk\PHPStan\DeadCode\Collector\ProvidedUsagesCollector; use ShipMonk\PHPStan\DeadCode\Compatibility\BackwardCompatibilityChecker; +use ShipMonk\PHPStan\DeadCode\Debug\DebugUsagePrinter; use ShipMonk\PHPStan\DeadCode\Enum\ClassLikeKind; use ShipMonk\PHPStan\DeadCode\Enum\MemberType; use ShipMonk\PHPStan\DeadCode\Enum\NeverReportedReason; @@ -28,7 +27,6 @@ use ShipMonk\PHPStan\DeadCode\Graph\ClassMemberUsage; use ShipMonk\PHPStan\DeadCode\Graph\ClassMethodRef; use ShipMonk\PHPStan\DeadCode\Graph\CollectedUsage; -use ShipMonk\PHPStan\DeadCode\Graph\UsageOrigin; use ShipMonk\PHPStan\DeadCode\Hierarchy\ClassHierarchy; use function array_key_exists; use function array_keys; @@ -36,14 +34,9 @@ use function array_merge; use function array_merge_recursive; use function array_slice; -use function array_sum; use function array_values; -use function count; -use function explode; use function in_array; use function ksort; -use function sprintf; -use function str_replace; use function strpos; /** @@ -73,14 +66,10 @@ class DeadCodeRule implements Rule, DiagnoseExtension '__debugInfo' => null, ]; - private RelativePathHelper $relativePathHelper; - - private ReflectionProvider $reflectionProvider; + private DebugUsagePrinter $debugUsagePrinter; private ClassHierarchy $classHierarchy; - private ?string $editorUrl; - /** * typename => data * @@ -104,15 +93,6 @@ class DeadCodeRule implements Rule, DiagnoseExtension private bool $reportTransitivelyDeadAsSeparateError; - private bool $trackMixedAccess; - - /** - * memberKey => usage info - * - * @var array, eliminationPath?: list, neverReported?: string}> - */ - private array $debugMembers; - /** * memberKey => DeadMember * @@ -132,27 +112,16 @@ class DeadCodeRule implements Rule, DiagnoseExtension */ private array $usageGraph = []; - /** - * @param list $debugMembers - */ public function __construct( - RelativePathHelper $relativePathHelper, - ReflectionProvider $reflectionProvider, + DebugUsagePrinter $debugUsagePrinter, ClassHierarchy $classHierarchy, - ?string $editorUrl, bool $reportTransitivelyDeadMethodAsSeparateError, - bool $trackMixedAccess, - array $debugMembers, BackwardCompatibilityChecker $checker ) { - $this->relativePathHelper = $relativePathHelper; - $this->reflectionProvider = $reflectionProvider; + $this->debugUsagePrinter = $debugUsagePrinter; $this->classHierarchy = $classHierarchy; - $this->editorUrl = $editorUrl; $this->reportTransitivelyDeadAsSeparateError = $reportTransitivelyDeadMethodAsSeparateError; - $this->trackMixedAccess = $trackMixedAccess; - $this->debugMembers = $this->buildDebugMemberKeys($debugMembers); $checker->check(); } @@ -193,7 +162,7 @@ public function processNode( $collectedUsage = CollectedUsage::deserialize($useString); $memberUsage = $collectedUsage->getUsage(); - $this->recordDebugUsage($collectedUsage); + $this->debugUsagePrinter->recordUsage($collectedUsage); if ($memberUsage->getMemberRef()->getClassName() === null) { $this->mixedMemberUsages[$memberUsage->getMemberType()][$memberUsage->getMemberRef()->getMemberName()][] = $collectedUsage; @@ -295,7 +264,7 @@ public function processNode( $neverReportedReason = $this->isNeverReportedAsDead($blackMember); if ($neverReportedReason !== null) { - $this->markDebugMemberAsNeverReported($blackMember, $neverReportedReason); + $this->debugUsagePrinter->markMemberAsNeverReported($blackMember, $neverReportedReason); unset($this->blackMembers[$blackMemberKey]); } @@ -494,7 +463,7 @@ private function markTransitivesWhite(string $callerKey, array $visitedKeys = [] $calleeKeys = $this->usageGraph[$callerKey] ?? []; if (isset($this->blackMembers[$callerKey])) { // TODO debug why not always present - $this->markDebugMemberAsWhite($this->blackMembers[$callerKey], array_keys($visitedKeys)); + $this->debugUsagePrinter->markMemberAsWhite($this->blackMembers[$callerKey], array_keys($visitedKeys)); unset($this->blackMembers[$callerKey]); } @@ -759,211 +728,8 @@ private function isNeverReportedAsDead(BlackMember $blackMember): ?string public function print(Output $output): void { - $this->printMixedMemberUsages($output); - $this->printDebugMemberUsages($output); - } - - private function printMixedMemberUsages(Output $output): void - { - if ($this->mixedMemberUsages === [] || !$output->isDebug() || !$this->trackMixedAccess) { - return; - } - - $totalCount = array_sum(array_map('count', $this->mixedMemberUsages)); - $maxExamplesToShow = 20; - $examplesShown = 0; - $output->writeLineFormatted(sprintf('Found %d usages over unknown type:', $totalCount)); - - foreach ($this->mixedMemberUsages as $memberType => $collectedUsages) { - foreach ($collectedUsages as $memberName => $usages) { - $examplesShown++; - $memberTypeString = $memberType === MemberType::METHOD ? 'method' : 'constant'; - $output->writeFormatted(sprintf(' • %s %s', $memberName, $memberTypeString)); - - $exampleCaller = $this->getExampleCaller($usages); - - if ($exampleCaller !== null) { - $output->writeFormatted(sprintf(', for example in %s', $exampleCaller)); - } - - $output->writeLineFormatted(''); - - if ($examplesShown >= $maxExamplesToShow) { - break 2; - } - } - } - - if ($totalCount > $maxExamplesToShow) { - $output->writeLineFormatted(sprintf('... and %d more', $totalCount - $maxExamplesToShow)); - } - - $output->writeLineFormatted(''); - $output->writeLineFormatted('Thus, any member named the same is considered used, no matter its declaring class!'); - $output->writeLineFormatted(''); - } - - /** - * @param list $usages - */ - private function getExampleCaller(array $usages): ?string - { - foreach ($usages as $usage) { - $origin = $usage->getUsage()->getOrigin(); - - if ($origin->getFile() !== null) { - return $this->getOriginReference($origin); - } - } - - return null; - } - - private function printDebugMemberUsages(Output $output): void - { - if ($this->debugMembers === [] || !$output->isDebug()) { - return; - } - - $output->writeLineFormatted("\nUsage debugging information:"); - - foreach ($this->debugMembers as $memberKey => $debugMember) { - $output->writeLineFormatted(sprintf("\n%s", $memberKey)); - - if (isset($debugMember['eliminationPath'])) { - $output->writeLineFormatted("|\n| Elimination path:"); - - foreach ($debugMember['eliminationPath'] as $index => $eliminationPath) { - $entrypoint = $index === 0 ? '(entrypoint)' : ''; - $output->writeLineFormatted(sprintf('| -> %s %s', $eliminationPath, $entrypoint)); - } - } - - if (isset($debugMember['neverReported'])) { - $output->writeLineFormatted(sprintf("|\n| Is never reported as dead: %s", $debugMember['neverReported'])); - } - - if (isset($debugMember['usages'])) { - $output->writeLineFormatted(sprintf("|\n| Found %d usages:", count($debugMember['usages']))); - - foreach ($debugMember['usages'] as $collectedUsage) { - $origin = $collectedUsage->getUsage()->getOrigin(); - $output->writeFormatted(sprintf('| • %s', $this->getOriginReference($origin))); - - if ($collectedUsage->isExcluded()) { - $output->writeFormatted(sprintf('| Excluded by %s', $collectedUsage->getExcludedBy())); - } - - $output->writeLineFormatted(''); - } - } - - $output->writeLineFormatted(''); - } - } - - private function getOriginReference(UsageOrigin $origin): string - { - $file = $origin->getFile(); - $line = $origin->getLine(); - - if ($file !== null && $line !== null) { - $relativeFile = $this->relativePathHelper->getRelativePath($file); - - if ($this->editorUrl === null) { - return sprintf( - '%s:%s', - $relativeFile, - $line, - ); - } - - return sprintf( - '%s:%s', - str_replace(['%file%', '%relFile%', '%line%'], [$file, $relativeFile, (string) $line], $this->editorUrl), - $relativeFile, - $line, - ); - } - - if ($origin->getReason() !== null) { - return $origin->getReason(); - } - - throw new LogicException('Unknown state of usage origin'); - } - - /** - * @param list $debugMembers - * @return array, eliminationPath?: list, neverReported?: string}> - */ - private function buildDebugMemberKeys(array $debugMembers): array - { - $result = []; - - foreach ($debugMembers as $debugMember) { - if (strpos($debugMember, '::') === false) { - throw new LogicException("Invalid debug member format: $debugMember"); - } - - [$class, $memberName] = explode('::', $debugMember); // @phpstan-ignore offsetAccess.notFound - - if (!$this->reflectionProvider->hasClass($class)) { - throw new LogicException("Class $class does not exist"); - } - - $classReflection = $this->reflectionProvider->getClass($class); - - if ($classReflection->hasMethod($memberName)) { - $key = ClassMethodRef::buildKey($class, $memberName); - - } elseif ($classReflection->hasConstant($memberName)) { - $key = ClassConstantRef::buildKey($class, $memberName); - - } else { - throw new LogicException("Member $memberName does not exist in $class"); - } - - $result[$key] = []; - } - - return $result; - } - - private function recordDebugUsage(CollectedUsage $collectedUsage): void - { - $memberKey = $collectedUsage->getUsage()->getMemberRef()->toKey(); - - if (!isset($this->debugMembers[$memberKey])) { - return; - } - - $this->debugMembers[$memberKey]['usages'][] = $collectedUsage; - } - - /** - * @param list $usagePath - */ - private function markDebugMemberAsWhite(BlackMember $blackMember, array $usagePath): void - { - $memberKey = $blackMember->getMember()->toKey(); - - if (!isset($this->debugMembers[$memberKey])) { - return; - } - - $this->debugMembers[$memberKey]['eliminationPath'] = $usagePath; - } - - private function markDebugMemberAsNeverReported(BlackMember $blackMember, string $reason): void - { - $memberKey = $blackMember->getMember()->toKey(); - - if (!isset($this->debugMembers[$memberKey])) { - return; - } - - $this->debugMembers[$memberKey]['neverReported'] = $reason; + $this->debugUsagePrinter->printMixedMemberUsages($output, $this->mixedMemberUsages); + $this->debugUsagePrinter->printDebugMemberUsages($output); } } diff --git a/tests/Rule/DeadCodeRuleTest.php b/tests/Rule/DeadCodeRuleTest.php index 09f92056..90841062 100644 --- a/tests/Rule/DeadCodeRuleTest.php +++ b/tests/Rule/DeadCodeRuleTest.php @@ -21,6 +21,7 @@ use ShipMonk\PHPStan\DeadCode\Collector\MethodCallCollector; use ShipMonk\PHPStan\DeadCode\Collector\ProvidedUsagesCollector; use ShipMonk\PHPStan\DeadCode\Compatibility\BackwardCompatibilityChecker; +use ShipMonk\PHPStan\DeadCode\Debug\DebugUsagePrinter; use ShipMonk\PHPStan\DeadCode\Excluder\MemberUsageExcluder; use ShipMonk\PHPStan\DeadCode\Excluder\TestsUsageExcluder; use ShipMonk\PHPStan\DeadCode\Formatter\RemoveDeadCodeFormatter; @@ -61,13 +62,15 @@ protected function getRule(): DeadCodeRule { if ($this->rule === null) { $this->rule = new DeadCodeRule( - new SimpleRelativePathHelper(__DIR__), // @phpstan-ignore phpstanApi.constructor - self::createReflectionProvider(), + new DebugUsagePrinter( + new SimpleRelativePathHelper(__DIR__), // @phpstan-ignore phpstanApi.constructor + self::createReflectionProvider(), + null, + true, + [], + ), new ClassHierarchy(), - null, !$this->emitErrorsInGroups, - true, - [], new BackwardCompatibilityChecker([]), ); } From f0360fab2eb91c0c6ae61ec7595597f14d800de0 Mon Sep 17 00:00:00 2001 From: Jan Nedbal Date: Tue, 11 Mar 2025 20:08:25 +0100 Subject: [PATCH 03/15] Elimination path to contain exact usage --- src/Debug/DebugUsagePrinter.php | 69 +++++++++++++++++++++++---------- src/Rule/DeadCodeRule.php | 29 +++++++------- 2 files changed, 65 insertions(+), 33 deletions(-) diff --git a/src/Debug/DebugUsagePrinter.php b/src/Debug/DebugUsagePrinter.php index cdf247c2..4ced4061 100644 --- a/src/Debug/DebugUsagePrinter.php +++ b/src/Debug/DebugUsagePrinter.php @@ -9,6 +9,7 @@ use ShipMonk\PHPStan\DeadCode\Enum\MemberType; use ShipMonk\PHPStan\DeadCode\Error\BlackMember; use ShipMonk\PHPStan\DeadCode\Graph\ClassConstantRef; +use ShipMonk\PHPStan\DeadCode\Graph\ClassMemberUsage; use ShipMonk\PHPStan\DeadCode\Graph\ClassMethodRef; use ShipMonk\PHPStan\DeadCode\Graph\CollectedUsage; use ShipMonk\PHPStan\DeadCode\Graph\UsageOrigin; @@ -16,7 +17,12 @@ use function array_sum; use function count; use function explode; +use function ltrim; +use function next; +use function preg_replace; +use function reset; use function sprintf; +use function str_repeat; use function str_replace; use function strpos; @@ -34,7 +40,7 @@ class DebugUsagePrinter /** * memberKey => usage info * - * @var array, eliminationPath?: list, neverReported?: string}> + * @var array, eliminationPath?: array>, neverReported?: string}> */ private array $debugMembers; @@ -124,21 +130,28 @@ public function printDebugMemberUsages(Output $output): void $output->writeLineFormatted("\nUsage debugging information:"); foreach ($this->debugMembers as $memberKey => $debugMember) { - $output->writeLineFormatted(sprintf("\n%s", $memberKey)); + $output->writeLineFormatted(sprintf("\n%s", $this->prettyMemberKey($memberKey))); if (isset($debugMember['eliminationPath'])) { - $output->writeLineFormatted("|\n| Elimination path:"); + $output->writeLineFormatted("|\n| Elimination path:"); + $depth = 0; + + foreach ($debugMember['eliminationPath'] as $fragmentKey => $fragmentUsages) { + $indent = $depth === 0 ? 'entrypoint ' : ' ' . str_repeat(' ', $depth) . 'calls '; + $nextFragmentUsages = next($debugMember['eliminationPath']); + $nextFragmentFirstUsage = $nextFragmentUsages !== false ? reset($nextFragmentUsages) : null; + $nextFragmentFirstUsageOrigin = $nextFragmentFirstUsage instanceof ClassMemberUsage ? $nextFragmentFirstUsage->getOrigin() : null; + + if ($nextFragmentFirstUsageOrigin === null) { + $output->writeLineFormatted(sprintf('| %s%s', $indent, $this->prettyMemberKey($fragmentKey))); + } else { + $output->writeLineFormatted(sprintf('| %s%s (%s)', $indent, $this->prettyMemberKey($fragmentKey), $this->getOriginReference($nextFragmentFirstUsageOrigin))); + } - foreach ($debugMember['eliminationPath'] as $index => $eliminationPath) { - $entrypoint = $index === 0 ? '(entrypoint)' : ''; - $output->writeLineFormatted(sprintf('| -> %s %s', $eliminationPath, $entrypoint)); + $depth++; } } - if (isset($debugMember['neverReported'])) { - $output->writeLineFormatted(sprintf("|\n| Is never reported as dead: %s", $debugMember['neverReported'])); - } - if (isset($debugMember['usages'])) { $output->writeLineFormatted(sprintf("|\n| Found %d usages:", count($debugMember['usages']))); @@ -152,12 +165,27 @@ public function printDebugMemberUsages(Output $output): void $output->writeLineFormatted(''); } + } elseif (isset($debugMember['neverReported'])) { + $output->writeLineFormatted(sprintf("|\n| Is never reported as dead: %s", $debugMember['neverReported'])); + } else { + $output->writeLineFormatted("|\n| No usages found"); } $output->writeLineFormatted(''); } } + private function prettyMemberKey(string $memberKey): string + { + $replaced = preg_replace('/^(m|c)\//', '', $memberKey); + + if ($replaced === null) { + throw new LogicException('Failed to pretty member key ' . $memberKey); + } + + return $replaced; + } + private function getOriginReference(UsageOrigin $origin): string { $file = $origin->getFile(); @@ -201,9 +229,9 @@ public function recordUsage(CollectedUsage $collectedUsage): void } /** - * @param list $usagePath + * @param array> $eliminationPath */ - public function markMemberAsWhite(BlackMember $blackMember, array $usagePath): void + public function markMemberAsWhite(BlackMember $blackMember, array $eliminationPath): void { $memberKey = $blackMember->getMember()->toKey(); @@ -211,7 +239,7 @@ public function markMemberAsWhite(BlackMember $blackMember, array $usagePath): v return; } - $this->debugMembers[$memberKey]['eliminationPath'] = $usagePath; + $this->debugMembers[$memberKey]['eliminationPath'] = $eliminationPath; } public function markMemberAsNeverReported(BlackMember $blackMember, string $reason): void @@ -227,7 +255,7 @@ public function markMemberAsNeverReported(BlackMember $blackMember, string $reas /** * @param list $debugMembers - * @return array, eliminationPath?: list, neverReported?: string}> + * @return array, eliminationPath?: array>, neverReported?: string}> */ private function buildDebugMemberKeys(array $debugMembers): array { @@ -239,24 +267,25 @@ private function buildDebugMemberKeys(array $debugMembers): array } [$class, $memberName] = explode('::', $debugMember); // @phpstan-ignore offsetAccess.notFound + $normalizedClass = ltrim($class, '\\'); - if (!$this->reflectionProvider->hasClass($class)) { - throw new LogicException("Class $class does not exist"); + if (!$this->reflectionProvider->hasClass($normalizedClass)) { + throw new LogicException("Class $normalizedClass does not exist"); } - $classReflection = $this->reflectionProvider->getClass($class); + $classReflection = $this->reflectionProvider->getClass($normalizedClass); if ($classReflection->hasMethod($memberName)) { - $key = ClassMethodRef::buildKey($class, $memberName); + $key = ClassMethodRef::buildKey($normalizedClass, $memberName); } elseif ($classReflection->hasConstant($memberName)) { - $key = ClassConstantRef::buildKey($class, $memberName); + $key = ClassConstantRef::buildKey($normalizedClass, $memberName); } elseif ($classReflection->hasProperty($memberName)) { throw new LogicException('Properties are not yet supported'); } else { - throw new LogicException("Member $memberName does not exist in $class"); + throw new LogicException("Member $memberName does not exist in $normalizedClass"); } $result[$key] = []; diff --git a/src/Rule/DeadCodeRule.php b/src/Rule/DeadCodeRule.php index fd9a5611..bf1635c4 100644 --- a/src/Rule/DeadCodeRule.php +++ b/src/Rule/DeadCodeRule.php @@ -34,6 +34,7 @@ use function array_merge; use function array_merge_recursive; use function array_slice; +use function array_unique; use function array_values; use function in_array; use function ksort; @@ -108,7 +109,7 @@ class DeadCodeRule implements Rule, DiagnoseExtension private array $mixedMemberUsages = []; /** - * @var array> callerKey => memberUseKey[] + * @var array>> callerKey => array */ private array $usageGraph = []; @@ -247,7 +248,7 @@ public function processNode( foreach ($alternativeMemberKeys as $alternativeMemberKey) { foreach ($alternativeOriginKeys as $alternativeOriginKey) { - $this->usageGraph[$alternativeOriginKey][] = $alternativeMemberKey; + $this->usageGraph[$alternativeOriginKey][$alternativeMemberKey][] = $memberUsage; } if ($isWhite) { @@ -415,6 +416,8 @@ private function getAlternativeMemberKeys(ClassMemberRef $member): array } } + $result = array_values(array_unique($result)); + $this->memberAlternativesCache[$cacheKey] = $result; return $result; @@ -455,21 +458,21 @@ private function findDefinerMemberKey( } /** - * @param array $visitedKeys + * @param array> $visited */ - private function markTransitivesWhite(string $callerKey, array $visitedKeys = []): void + private function markTransitivesWhite(string $callerKey, array $visited = []): void { - $visitedKeys = $visitedKeys === [] ? [$callerKey => null] : $visitedKeys; - $calleeKeys = $this->usageGraph[$callerKey] ?? []; + $visited = $visited === [] ? [$callerKey => []] : $visited; // TODO [] init? + $callees = $this->usageGraph[$callerKey] ?? []; if (isset($this->blackMembers[$callerKey])) { // TODO debug why not always present - $this->debugUsagePrinter->markMemberAsWhite($this->blackMembers[$callerKey], array_keys($visitedKeys)); + $this->debugUsagePrinter->markMemberAsWhite($this->blackMembers[$callerKey], $visited); unset($this->blackMembers[$callerKey]); } - foreach ($calleeKeys as $calleeKey) { - if (array_key_exists($calleeKey, $visitedKeys)) { + foreach ($callees as $calleeKey => $usages) { + if (array_key_exists($calleeKey, $visited)) { continue; } @@ -477,7 +480,7 @@ private function markTransitivesWhite(string $callerKey, array $visitedKeys = [] continue; } - $this->markTransitivesWhite($calleeKey, array_merge($visitedKeys, [$calleeKey => null])); + $this->markTransitivesWhite($calleeKey, array_merge($visited, [$calleeKey => $usages])); } } @@ -488,11 +491,11 @@ private function markTransitivesWhite(string $callerKey, array $visitedKeys = [] private function getTransitiveDeadCalls(string $callerKey, array $visitedKeys = []): array { $visitedKeys = $visitedKeys === [] ? [$callerKey => null] : $visitedKeys; - $calleeKeys = $this->usageGraph[$callerKey] ?? []; + $callees = $this->usageGraph[$callerKey] ?? []; $result = []; - foreach ($calleeKeys as $calleeKey) { + foreach ($callees as $calleeKey => $_) { if (array_key_exists($calleeKey, $visitedKeys)) { continue; } @@ -528,7 +531,7 @@ private function groupDeadMembers(): array continue; } - foreach ($callees as $callee) { + foreach ($callees as $callee => $_) { if (array_key_exists($callee, $this->blackMembers)) { $deadMethodsWithCaller[$callee] = true; } From c285783f5c733c1f4f5479e7c59c39cf95fc0e78 Mon Sep 17 00:00:00 2001 From: Jan Nedbal Date: Wed, 12 Mar 2025 14:17:44 +0100 Subject: [PATCH 04/15] Add tests, improve output --- src/Collector/MethodCallCollector.php | 2 +- src/Debug/DebugUsagePrinter.php | 42 ++++-- src/Graph/CollectedUsage.php | 7 +- src/Graph/UsageOrigin.php | 46 +++++-- src/Graph/UsageOriginDetector.php | 2 +- src/Provider/DoctrineUsageProvider.php | 4 +- src/Provider/PhpUnitUsageProvider.php | 2 +- .../ReflectionBasedMemberUsageProvider.php | 4 +- src/Provider/SymfonyUsageProvider.php | 10 +- src/Rule/DeadCodeRule.php | 3 + tests/Graph/SerializationTest.php | 4 +- tests/Rule/DeadCodeRuleTest.php | 122 +++++++++++++++++- tests/Rule/data/debug/cycle.php | 19 +++ tests/Rule/data/debug/entrypoint.php | 21 +++ tests/Rule/data/debug/global.php | 10 ++ tests/Rule/data/debug/never.php | 9 ++ tests/Rule/data/debug/virtual.php | 13 ++ tests/Rule/data/debug/zero.php | 10 ++ 18 files changed, 292 insertions(+), 38 deletions(-) create mode 100644 tests/Rule/data/debug/cycle.php create mode 100644 tests/Rule/data/debug/entrypoint.php create mode 100644 tests/Rule/data/debug/global.php create mode 100644 tests/Rule/data/debug/never.php create mode 100644 tests/Rule/data/debug/virtual.php create mode 100644 tests/Rule/data/debug/zero.php diff --git a/src/Collector/MethodCallCollector.php b/src/Collector/MethodCallCollector.php index a1063e09..d88fa03e 100644 --- a/src/Collector/MethodCallCollector.php +++ b/src/Collector/MethodCallCollector.php @@ -206,7 +206,7 @@ private function registerAttribute(Attribute $node, Scope $scope): void { $this->registerUsage( new ClassMethodUsage( - UsageOrigin::fromScope($node, $scope), + UsageOrigin::createRegular($node, $scope), new ClassMethodRef($scope->resolveName($node->name), '__construct', false), ), $node, diff --git a/src/Debug/DebugUsagePrinter.php b/src/Debug/DebugUsagePrinter.php index 4ced4061..c09a029e 100644 --- a/src/Debug/DebugUsagePrinter.php +++ b/src/Debug/DebugUsagePrinter.php @@ -137,19 +137,22 @@ public function printDebugMemberUsages(Output $output): void $depth = 0; foreach ($debugMember['eliminationPath'] as $fragmentKey => $fragmentUsages) { - $indent = $depth === 0 ? 'entrypoint ' : ' ' . str_repeat(' ', $depth) . 'calls '; + $indent = $depth === 0 ? 'entrypoint ' : ' ' . str_repeat(' ', $depth) . 'calls '; $nextFragmentUsages = next($debugMember['eliminationPath']); $nextFragmentFirstUsage = $nextFragmentUsages !== false ? reset($nextFragmentUsages) : null; $nextFragmentFirstUsageOrigin = $nextFragmentFirstUsage instanceof ClassMemberUsage ? $nextFragmentFirstUsage->getOrigin() : null; - if ($nextFragmentFirstUsageOrigin === null) { - $output->writeLineFormatted(sprintf('| %s%s', $indent, $this->prettyMemberKey($fragmentKey))); - } else { - $output->writeLineFormatted(sprintf('| %s%s (%s)', $indent, $this->prettyMemberKey($fragmentKey), $this->getOriginReference($nextFragmentFirstUsageOrigin))); - } + $pathFragment = $nextFragmentFirstUsageOrigin === null + ? $this->prettyMemberKey($fragmentKey) + : $this->getOriginLink($nextFragmentFirstUsageOrigin, $this->prettyMemberKey($fragmentKey)); + + $output->writeLineFormatted(sprintf('| %s%s', $indent, $pathFragment)); $depth++; } + } elseif (isset($debugMember['usages'])) { + $output->writeLineFormatted("|\n| Elimination path:"); + $output->writeLineFormatted('| Not found, all usages originate in unused code'); } if (isset($debugMember['usages'])) { @@ -210,13 +213,36 @@ private function getOriginReference(UsageOrigin $origin): string ); } - if ($origin->getReason() !== null) { - return $origin->getReason(); + if ($origin->getProvider() !== null) { + $note = $origin->getNote() !== null ? " ({$origin->getNote()})" : ''; + return 'virtual usage from ' . $origin->getProvider() . $note; } throw new LogicException('Unknown state of usage origin'); } + private function getOriginLink(UsageOrigin $origin, string $title): string + { + $file = $origin->getFile(); + $line = $origin->getLine(); + + if ($line !== null) { + $title = sprintf('%s:%s', $title, $line); + } + + if ($this->editorUrl !== null && $file !== null && $line !== null) { + $relativeFile = $this->relativePathHelper->getRelativePath($file); + + return sprintf( + '%s', + str_replace(['%file%', '%relFile%', '%line%'], [$file, $relativeFile, (string) $line], $this->editorUrl), + $title, + ); + } + + return $title; + } + public function recordUsage(CollectedUsage $collectedUsage): void { $memberKey = $collectedUsage->getUsage()->getMemberRef()->toKey(); diff --git a/src/Graph/CollectedUsage.php b/src/Graph/CollectedUsage.php index 780d9bee..68e58b18 100644 --- a/src/Graph/CollectedUsage.php +++ b/src/Graph/CollectedUsage.php @@ -65,7 +65,8 @@ public function serialize(): string 'm' => $origin->getMethodName(), 'f' => $origin->getFile(), 'l' => $origin->getLine(), - 'r' => $origin->getReason(), + 'p' => $origin->getProvider(), + 'n' => $origin->getNote(), ], 'm' => [ 'c' => $memberRef->getClassName(), @@ -84,14 +85,14 @@ public function serialize(): string public static function deserialize(string $data): self { try { - /** @var array{e: string|null, t: MemberType::*, o: array{c: string|null, m: string|null, f: string|null, l: int|null, r: string|null}, m: array{c: string|null, m: string, d: bool}} $result */ + /** @var array{e: string|null, t: MemberType::*, o: array{c: string|null, m: string|null, f: string|null, l: int|null, p: string|null, n: string|null}, m: array{c: string|null, m: string, d: bool}} $result */ $result = json_decode($data, true, 3, JSON_THROW_ON_ERROR); } catch (JsonException $e) { throw new LogicException('Deserialization failure: ' . $e->getMessage(), 0, $e); } $memberType = $result['t']; - $origin = new UsageOrigin($result['o']['c'], $result['o']['m'], $result['o']['f'], $result['o']['l'], $result['o']['r']); + $origin = new UsageOrigin($result['o']['c'], $result['o']['m'], $result['o']['f'], $result['o']['l'], $result['o']['p'], $result['o']['n']); $exclusionReason = $result['e']; $usage = $memberType === MemberType::CONSTANT diff --git a/src/Graph/UsageOrigin.php b/src/Graph/UsageOrigin.php index 7d7aa191..a20b8fe3 100644 --- a/src/Graph/UsageOrigin.php +++ b/src/Graph/UsageOrigin.php @@ -7,6 +7,7 @@ use PHPStan\Analyser\Scope; use PHPStan\Reflection\MethodReflection; use ShipMonk\PHPStan\DeadCode\Provider\MemberUsageProvider; +use function get_class; /** * @immutable @@ -22,39 +23,51 @@ final class UsageOrigin private ?int $line; - private ?string $reason; + private ?string $provider; - public function __construct( // TODO private? + private ?string $note; + + /** + * @internal Please use static constructors instead. + */ + public function __construct( ?string $className, ?string $methodName, - ?string $fileName, + ?string $fileName, // TODO try reusing collector file to reduce memory usage ?int $line, - ?string $reason = null + ?string $provider, + ?string $note ) { $this->className = $className; $this->methodName = $methodName; $this->fileName = $fileName; $this->line = $line; - $this->reason = $reason; + $this->provider = $provider; + $this->note = $note; } /** - * @param class-string $providerClass - * @param ?string $reason More detailed identification why provider emitted this usage + * Creates virtual usage origin with no reference to any place in code + * + * @param ?string $note More detailed identification why provider emitted this virtual usage */ - public static function fromProvider(string $providerClass, ?string $reason): self + public static function createVirtual(MemberUsageProvider $provider, ?string $note = null): self { return new self( null, null, null, null, - $providerClass . ' - ' . (string) $reason, // TODO better approach? + get_class($provider), + $note, ); } - public static function fromScope(Node $node, Scope $scope): self + /** + * Creates usage origin with reference to file:line + */ + public static function createRegular(Node $node, Scope $scope): self { if (!$scope->isInClass() || !$scope->getFunction() instanceof MethodReflection) { return new self( @@ -62,6 +75,8 @@ public static function fromScope(Node $node, Scope $scope): self null, $scope->getFile(), $node->getStartLine(), + null, + null, ); } @@ -70,6 +85,8 @@ public static function fromScope(Node $node, Scope $scope): self $scope->getFunction()->getName(), $scope->getFile(), $node->getStartLine(), + null, + null, ); } @@ -93,9 +110,14 @@ public function getLine(): ?int return $this->line; } - public function getReason(): ?string + public function getProvider(): ?string + { + return $this->provider; + } + + public function getNote(): ?string { - return $this->reason; + return $this->note; } public function hasClassMethodRef(): bool diff --git a/src/Graph/UsageOriginDetector.php b/src/Graph/UsageOriginDetector.php index 5873854f..8f68eea3 100644 --- a/src/Graph/UsageOriginDetector.php +++ b/src/Graph/UsageOriginDetector.php @@ -13,7 +13,7 @@ class UsageOriginDetector */ public function detectOrigin(Node $node, Scope $scope): UsageOrigin { - return UsageOrigin::fromScope($node, $scope); + return UsageOrigin::createRegular($node, $scope); } } diff --git a/src/Provider/DoctrineUsageProvider.php b/src/Provider/DoctrineUsageProvider.php index 542bc97f..04f874f1 100644 --- a/src/Provider/DoctrineUsageProvider.php +++ b/src/Provider/DoctrineUsageProvider.php @@ -101,7 +101,7 @@ private function getUsagesOfEventSubscriber(Return_ $node, Scope $scope): array $className = $scope->getClassReflection()->getName(); $usages = []; - $usageOrigin = UsageOrigin::fromScope($node, $scope); + $usageOrigin = UsageOrigin::createRegular($node, $scope); foreach ($scope->getType($node->expr)->getConstantArrays() as $rootArray) { foreach ($rootArray->getValuesArray()->getValueTypes() as $eventConfig) { @@ -201,7 +201,7 @@ private function isDoctrineInstalled(): bool private function createMethodUsage(ExtendedMethodReflection $methodReflection): ClassMethodUsage { return new ClassMethodUsage( - UsageOrigin::fromProvider(self::class, null), + UsageOrigin::createVirtual($this), new ClassMethodRef( $methodReflection->getDeclaringClass()->getName(), $methodReflection->getName(), diff --git a/src/Provider/PhpUnitUsageProvider.php b/src/Provider/PhpUnitUsageProvider.php index dc6f65f7..eb02b8ac 100644 --- a/src/Provider/PhpUnitUsageProvider.php +++ b/src/Provider/PhpUnitUsageProvider.php @@ -145,7 +145,7 @@ private function hasAnnotation(ReflectionMethod $method, string $string): bool private function createUsage(ExtendedMethodReflection $getNativeMethod, string $reason): ClassMethodUsage { return new ClassMethodUsage( - UsageOrigin::fromProvider(self::class, $reason), + UsageOrigin::createVirtual($this, $reason), new ClassMethodRef( $getNativeMethod->getDeclaringClass()->getName(), $getNativeMethod->getName(), diff --git a/src/Provider/ReflectionBasedMemberUsageProvider.php b/src/Provider/ReflectionBasedMemberUsageProvider.php index e2ad1d20..54c13a1f 100644 --- a/src/Provider/ReflectionBasedMemberUsageProvider.php +++ b/src/Provider/ReflectionBasedMemberUsageProvider.php @@ -93,7 +93,7 @@ private function getConstantUsages(ClassReflection $classReflection): array private function createConstantUsage(ReflectionClassConstant $constantReflection): ClassConstantUsage { return new ClassConstantUsage( - UsageOrigin::fromProvider(self::class, null), + UsageOrigin::createVirtual($this), new ClassConstantRef( $constantReflection->getDeclaringClass()->getName(), $constantReflection->getName(), @@ -105,7 +105,7 @@ private function createConstantUsage(ReflectionClassConstant $constantReflection private function createMethodUsage(ReflectionMethod $methodReflection): ClassMethodUsage { return new ClassMethodUsage( - UsageOrigin::fromProvider(self::class, null), + UsageOrigin::createVirtual($this), new ClassMethodRef( $methodReflection->getDeclaringClass()->getName(), $methodReflection->getName(), diff --git a/src/Provider/SymfonyUsageProvider.php b/src/Provider/SymfonyUsageProvider.php index 543e928b..4297dd03 100644 --- a/src/Provider/SymfonyUsageProvider.php +++ b/src/Provider/SymfonyUsageProvider.php @@ -144,7 +144,7 @@ private function getUsagesOfEventSubscriber(Return_ $node, Scope $scope): array $className = $scope->getClassReflection()->getName(); $usages = []; - $usageOrigin = UsageOrigin::fromScope($node, $scope); + $usageOrigin = UsageOrigin::createRegular($node, $scope); // phpcs:disable Squiz.PHP.CommentedOutCode.Found foreach ($scope->getType($node->expr)->getConstantArrays() as $rootArray) { @@ -232,7 +232,7 @@ private function getMethodUsagesFromReflection(InClassNode $node): array private function getMethodUsagesFromAttributeReflection(InClassMethodNode $node, Scope $scope): array { $usages = []; - $usageOrigin = UsageOrigin::fromScope($node, $scope); + $usageOrigin = UsageOrigin::createRegular($node, $scope); foreach ($node->getMethodReflection()->getParameters() as $parameter) { foreach ($parameter->getAttributes() as $attributeReflection) { @@ -281,7 +281,7 @@ private function getMethodUsagesFromAttributeReflection(InClassMethodNode $node, foreach ($classNames as $className) { $usages[] = new ClassMethodUsage( - UsageOrigin::fromScope($node, $scope), + UsageOrigin::createRegular($node, $scope), new ClassMethodRef( $className->getValue(), $defaultIndexMethod[0]->getValue(), @@ -482,7 +482,7 @@ private function isSymfonyInstalled(): bool private function createUsage(ExtendedMethodReflection $methodReflection, ?string $reason): ClassMethodUsage { return new ClassMethodUsage( - UsageOrigin::fromProvider(self::class, $reason), + UsageOrigin::createVirtual($this, $reason), new ClassMethodRef( $methodReflection->getDeclaringClass()->getName(), $methodReflection->getName(), @@ -568,7 +568,7 @@ private function getConstantUsages(ClassReflection $classReflection): array } $usages[] = new ClassConstantUsage( - UsageOrigin::fromProvider(self::class, 'used in DIC'), + UsageOrigin::createVirtual($this, 'used in DIC'), new ClassConstantRef( $classReflection->getName(), $constantName, diff --git a/src/Rule/DeadCodeRule.php b/src/Rule/DeadCodeRule.php index bf1635c4..c3e27dde 100644 --- a/src/Rule/DeadCodeRule.php +++ b/src/Rule/DeadCodeRule.php @@ -693,6 +693,9 @@ private function isConsideredWhite(ClassMemberUsage $memberUsage): bool || (array_key_exists((string) $memberUsage->getOrigin()->getMethodName(), self::UNSUPPORTED_MAGIC_METHODS)); } + /** + * @return NeverReportedReason::*|null + */ private function isNeverReportedAsDead(BlackMember $blackMember): ?string { if (!$blackMember->getMember() instanceof ClassMethodRef) { diff --git a/tests/Graph/SerializationTest.php b/tests/Graph/SerializationTest.php index 96679538..b8fe9a1b 100644 --- a/tests/Graph/SerializationTest.php +++ b/tests/Graph/SerializationTest.php @@ -24,12 +24,12 @@ public static function provideData(): iterable yield [ new CollectedUsage( new ClassConstantUsage( - new UsageOrigin('Clazz', 'method', '/app/index.php', 7, null), + new UsageOrigin('Clazz', 'method', '/app/index.php', 7, null, null), new ClassConstantRef(null, 'CONSTANT', true), ), 'excluder', ), - '{"e":"excluder","t":2,"o":{"c":"Clazz","m":"method","f":"\/app\/index.php","l":7,"r":null},"m":{"c":null,"m":"CONSTANT","d":true}}', + '{"e":"excluder","t":2,"o":{"c":"Clazz","m":"method","f":"\/app\/index.php","l":7,"p":null,"n":null},"m":{"c":null,"m":"CONSTANT","d":true}}', ]; } diff --git a/tests/Rule/DeadCodeRuleTest.php b/tests/Rule/DeadCodeRuleTest.php index 90841062..8b614521 100644 --- a/tests/Rule/DeadCodeRuleTest.php +++ b/tests/Rule/DeadCodeRuleTest.php @@ -4,6 +4,7 @@ use Composer\InstalledVersions; use Composer\Semver\VersionParser; +use LogicException; use PhpParser\Node; use PHPStan\Analyser\Error; use PHPStan\Analyser\Scope; @@ -40,6 +41,7 @@ use ShipMonk\PHPStan\DeadCode\Transformer\FileSystem; use function file_get_contents; use function is_array; +use function preg_replace; use function str_replace; use function strpos; use const PHP_VERSION_ID; @@ -50,6 +52,11 @@ class DeadCodeRuleTest extends RuleTestCase { + /** + * @var list + */ + private array $debugMembers = []; + private bool $trackMixedAccess = true; private bool $emitErrorsInGroups = true; @@ -67,7 +74,7 @@ protected function getRule(): DeadCodeRule self::createReflectionProvider(), null, true, - [], + $this->debugMembers, ), new ClassHierarchy(), !$this->emitErrorsInGroups, @@ -180,6 +187,104 @@ static function (string $message) use (&$actualOutput): void { self::assertSame($expectedOutput, $actualOutput); } + public function testDebugUsage(): void + { + $this->debugMembers = [ + 'DebugNever\Foo::__get', + 'DebugVirtual\FooTest::testFoo', + 'DebugGlobal\Foo::__construct', + 'DebugCycle\Foo::__construct', + 'DebugRegular\Another::call', + 'DebugZero\Foo::__construct', + ]; + $this->analyseFiles([ + __DIR__ . '/data/debug/entrypoint.php', + __DIR__ . '/data/debug/cycle.php', + __DIR__ . '/data/debug/global.php', + __DIR__ . '/data/debug/never.php', + __DIR__ . '/data/debug/virtual.php', + __DIR__ . '/data/debug/zero.php', + ]); + $rule = $this->getRule(); + + $actualOutput = ''; + $output = $this->createMock(Output::class); + $output->expects(self::once()) + ->method('isDebug') + ->willReturn(true); + $output->expects(self::atLeastOnce()) + ->method('writeFormatted') + ->willReturnCallback( + static function (string $message) use (&$actualOutput): void { + $actualOutput .= $message; + }, + ); + $output->expects(self::atLeastOnce()) + ->method('writeLineFormatted') + ->willReturnCallback( + static function (string $message) use (&$actualOutput): void { + $actualOutput .= $message . "\n"; + }, + ); + + $rule->print($output); + + $expectedOutput = <<<'OUTPUT' + + Usage debugging information: + + DebugNever\Foo::__get + | + | Is never reported as dead: unsupported magic method + + + DebugVirtual\FooTest::testFoo + | + | Elimination path: + | entrypoint DebugVirtual\FooTest::testFoo + | + | Found 1 usages: + | • virtual usage from ShipMonk\PHPStan\DeadCode\Provider\PhpUnitUsageProvider (test method) + + + DebugGlobal\Foo::__construct + | + | Elimination path: + | entrypoint DebugGlobal\Foo::__construct + | + | Found 1 usages: + | • data/debug/global.php:10 + + + DebugCycle\Foo::__construct + | + | Elimination path: + | Not found, all usages originate in unused code + | + | Found 1 usages: + | • data/debug/cycle.php:17 + + + DebugRegular\Another::call + | + | Elimination path: + | entrypoint DebugRegular\FooController::dummyAction:12 + | calls DebugRegular\Another::call + | + | Found 1 usages: + | • data/debug/entrypoint.php:12 + + + DebugZero\Foo::__construct + | + | No usages found + + + OUTPUT; + + self::assertSame($expectedOutput, $this->trimColors($actualOutput)); + } + /** * @dataProvider provideAutoRemoveFiles */ @@ -611,4 +716,19 @@ private static function requiresPackage(string $package, string $constraint): bo return InstalledVersions::satisfies(new VersionParser(), $package, $constraint); } + private function trimColors(string $output): string + { + $replaced = preg_replace( + '/(.*?)<\/>/', + '$1', + $output, + ); + + if ($replaced === null) { + throw new LogicException('Failed to trim colors'); + } + + return $replaced; + } + } diff --git a/tests/Rule/data/debug/cycle.php b/tests/Rule/data/debug/cycle.php new file mode 100644 index 00000000..f3197425 --- /dev/null +++ b/tests/Rule/data/debug/cycle.php @@ -0,0 +1,19 @@ +call(); + } +} + +class Another +{ + public function call(): void + { + } +} diff --git a/tests/Rule/data/debug/global.php b/tests/Rule/data/debug/global.php new file mode 100644 index 00000000..1d3ff505 --- /dev/null +++ b/tests/Rule/data/debug/global.php @@ -0,0 +1,10 @@ + Date: Wed, 12 Mar 2025 16:25:19 +0100 Subject: [PATCH 05/15] More tests; use resolved mixed usages for debug --- src/Debug/DebugUsagePrinter.php | 35 +++++---- src/Rule/DeadCodeRule.php | 4 +- tests/Rule/DeadCodeRuleTest.php | 72 ++++++++++++++----- tests/Rule/data/debug/ctor.php | 13 ++++ tests/Rule/data/debug/exclude.php | 11 +++ tests/Rule/data/debug/global.php | 4 +- tests/Rule/data/debug/mixed.php | 14 ++++ .../debug/{entrypoint.php => regular.php} | 1 + 8 files changed, 121 insertions(+), 33 deletions(-) create mode 100644 tests/Rule/data/debug/ctor.php create mode 100644 tests/Rule/data/debug/exclude.php create mode 100644 tests/Rule/data/debug/mixed.php rename tests/Rule/data/debug/{entrypoint.php => regular.php} (92%) diff --git a/src/Debug/DebugUsagePrinter.php b/src/Debug/DebugUsagePrinter.php index c09a029e..e55138ea 100644 --- a/src/Debug/DebugUsagePrinter.php +++ b/src/Debug/DebugUsagePrinter.php @@ -74,7 +74,8 @@ public function printMixedMemberUsages(Output $output, array $mixedMemberUsages) $totalCount = array_sum(array_map('count', $mixedMemberUsages)); $maxExamplesToShow = 20; $examplesShown = 0; - $output->writeLineFormatted(sprintf('Found %d usages over unknown type:', $totalCount)); + $plural = $totalCount > 1 ? 's' : ''; + $output->writeLineFormatted(sprintf('Found %d usage%s over unknown type:', $totalCount, $plural)); foreach ($mixedMemberUsages as $memberType => $collectedUsages) { foreach ($collectedUsages as $memberName => $usages) { @@ -136,34 +137,40 @@ public function printDebugMemberUsages(Output $output): void $output->writeLineFormatted("|\n| Elimination path:"); $depth = 0; - foreach ($debugMember['eliminationPath'] as $fragmentKey => $fragmentUsages) { - $indent = $depth === 0 ? 'entrypoint ' : ' ' . str_repeat(' ', $depth) . 'calls '; - $nextFragmentUsages = next($debugMember['eliminationPath']); - $nextFragmentFirstUsage = $nextFragmentUsages !== false ? reset($nextFragmentUsages) : null; - $nextFragmentFirstUsageOrigin = $nextFragmentFirstUsage instanceof ClassMemberUsage ? $nextFragmentFirstUsage->getOrigin() : null; + if (count($debugMember['eliminationPath']) === 1) { + $output->writeLineFormatted('| direct usage'); - $pathFragment = $nextFragmentFirstUsageOrigin === null - ? $this->prettyMemberKey($fragmentKey) - : $this->getOriginLink($nextFragmentFirstUsageOrigin, $this->prettyMemberKey($fragmentKey)); + } else { + foreach ($debugMember['eliminationPath'] as $fragmentKey => $fragmentUsages) { + $indent = $depth === 0 ? 'entrypoint ' : ' ' . str_repeat(' ', $depth) . 'calls '; + $nextFragmentUsages = next($debugMember['eliminationPath']); + $nextFragmentFirstUsage = $nextFragmentUsages !== false ? reset($nextFragmentUsages) : null; + $nextFragmentFirstUsageOrigin = $nextFragmentFirstUsage instanceof ClassMemberUsage ? $nextFragmentFirstUsage->getOrigin() : null; - $output->writeLineFormatted(sprintf('| %s%s', $indent, $pathFragment)); + $pathFragment = $nextFragmentFirstUsageOrigin === null + ? $this->prettyMemberKey($fragmentKey) + : $this->getOriginLink($nextFragmentFirstUsageOrigin, $this->prettyMemberKey($fragmentKey)); - $depth++; + $output->writeLineFormatted(sprintf('| %s%s', $indent, $pathFragment)); + + $depth++; + } } } elseif (isset($debugMember['usages'])) { $output->writeLineFormatted("|\n| Elimination path:"); - $output->writeLineFormatted('| Not found, all usages originate in unused code'); + $output->writeLineFormatted('| not found, all usages originate in unused code'); } if (isset($debugMember['usages'])) { - $output->writeLineFormatted(sprintf("|\n| Found %d usages:", count($debugMember['usages']))); + $plural = count($debugMember['usages']) > 1 ? 's' : ''; + $output->writeLineFormatted(sprintf("|\n| Found %d usage%s:", count($debugMember['usages']), $plural)); foreach ($debugMember['usages'] as $collectedUsage) { $origin = $collectedUsage->getUsage()->getOrigin(); $output->writeFormatted(sprintf('| • %s', $this->getOriginReference($origin))); if ($collectedUsage->isExcluded()) { - $output->writeFormatted(sprintf(' - Excluded by %s', $collectedUsage->getExcludedBy())); + $output->writeFormatted(sprintf(' - excluded by %s excluder', $collectedUsage->getExcludedBy())); } $output->writeLineFormatted(''); diff --git a/src/Rule/DeadCodeRule.php b/src/Rule/DeadCodeRule.php index c3e27dde..6d8beae1 100644 --- a/src/Rule/DeadCodeRule.php +++ b/src/Rule/DeadCodeRule.php @@ -163,8 +163,6 @@ public function processNode( $collectedUsage = CollectedUsage::deserialize($useString); $memberUsage = $collectedUsage->getUsage(); - $this->debugUsagePrinter->recordUsage($collectedUsage); - if ($memberUsage->getMemberRef()->getClassName() === null) { $this->mixedMemberUsages[$memberUsage->getMemberType()][$memberUsage->getMemberRef()->getMemberName()][] = $collectedUsage; continue; @@ -233,6 +231,8 @@ public function processNode( $excludedMemberUsages = []; foreach ($knownCollectedUsages as $collectedUsage) { + $this->debugUsagePrinter->recordUsage($collectedUsage); + if ($collectedUsage->isExcluded()) { $excludedMemberUsages[] = $collectedUsage; continue; diff --git a/tests/Rule/DeadCodeRuleTest.php b/tests/Rule/DeadCodeRuleTest.php index 8b614521..26cbca73 100644 --- a/tests/Rule/DeadCodeRuleTest.php +++ b/tests/Rule/DeadCodeRuleTest.php @@ -190,18 +190,24 @@ static function (string $message) use (&$actualOutput): void { public function testDebugUsage(): void { $this->debugMembers = [ + 'DebugCtor\Foo::__construct', + 'DebugExclude\Foo::mixedExcluder', 'DebugNever\Foo::__get', 'DebugVirtual\FooTest::testFoo', - 'DebugGlobal\Foo::__construct', + 'DebugGlobal\Foo::chain', + 'DebugMixed\Foo::any', 'DebugCycle\Foo::__construct', 'DebugRegular\Another::call', 'DebugZero\Foo::__construct', ]; $this->analyseFiles([ - __DIR__ . '/data/debug/entrypoint.php', + __DIR__ . '/data/debug/ctor.php', + __DIR__ . '/data/debug/exclude.php', __DIR__ . '/data/debug/cycle.php', __DIR__ . '/data/debug/global.php', + __DIR__ . '/data/debug/mixed.php', __DIR__ . '/data/debug/never.php', + __DIR__ . '/data/debug/regular.php', __DIR__ . '/data/debug/virtual.php', __DIR__ . '/data/debug/zero.php', ]); @@ -209,7 +215,7 @@ public function testDebugUsage(): void $actualOutput = ''; $output = $this->createMock(Output::class); - $output->expects(self::once()) + $output->expects(self::atLeastOnce()) ->method('isDebug') ->willReturn(true); $output->expects(self::atLeastOnce()) @@ -229,10 +235,34 @@ static function (string $message) use (&$actualOutput): void { $rule->print($output); - $expectedOutput = <<<'OUTPUT' + $ec = ''; + $expectedOutput = <<<"OUTPUT" + Found 1 usage over unknown type: + $ec • any method, for example in data/debug/mixed.php:13 + + Thus, any member named the same is considered used, no matter its declaring class! + Usage debugging information: + DebugCtor\Foo::__construct + | + | Elimination path: + | not found, all usages originate in unused code + | + | Found 1 usage: + | • data/debug/ctor.php:9 + + + DebugExclude\Foo::mixedExcluder + | + | Elimination path: + | not found, all usages originate in unused code + | + | Found 1 usage: + | • data/debug/exclude.php:10 - excluded by mixed excluder + + DebugNever\Foo::__get | | Is never reported as dead: unsupported magic method @@ -241,27 +271,36 @@ static function (string $message) use (&$actualOutput): void { DebugVirtual\FooTest::testFoo | | Elimination path: - | entrypoint DebugVirtual\FooTest::testFoo + | direct usage | - | Found 1 usages: + | Found 1 usage: | • virtual usage from ShipMonk\PHPStan\DeadCode\Provider\PhpUnitUsageProvider (test method) - DebugGlobal\Foo::__construct + DebugGlobal\Foo::chain + | + | Elimination path: + | direct usage + | + | Found 1 usage: + | • data/debug/global.php:12 + + + DebugMixed\Foo::any | | Elimination path: - | entrypoint DebugGlobal\Foo::__construct + | direct usage | - | Found 1 usages: - | • data/debug/global.php:10 + | Found 1 usage: + | • data/debug/mixed.php:13 DebugCycle\Foo::__construct | | Elimination path: - | Not found, all usages originate in unused code + | not found, all usages originate in unused code | - | Found 1 usages: + | Found 1 usage: | • data/debug/cycle.php:17 @@ -271,8 +310,9 @@ static function (string $message) use (&$actualOutput): void { | entrypoint DebugRegular\FooController::dummyAction:12 | calls DebugRegular\Another::call | - | Found 1 usages: - | • data/debug/entrypoint.php:12 + | Found 2 usages: + | • data/debug/regular.php:12 + | • data/debug/regular.php:13 DebugZero\Foo::__construct @@ -282,7 +322,7 @@ static function (string $message) use (&$actualOutput): void { OUTPUT; - self::assertSame($expectedOutput, $this->trimColors($actualOutput)); + self::assertSame($expectedOutput, $this->trimFgColors($actualOutput)); } /** @@ -716,7 +756,7 @@ private static function requiresPackage(string $package, string $constraint): bo return InstalledVersions::satisfies(new VersionParser(), $package, $constraint); } - private function trimColors(string $output): string + private function trimFgColors(string $output): string { $replaced = preg_replace( '/(.*?)<\/>/', diff --git a/tests/Rule/data/debug/ctor.php b/tests/Rule/data/debug/ctor.php new file mode 100644 index 00000000..a5ab7d90 --- /dev/null +++ b/tests/Rule/data/debug/ctor.php @@ -0,0 +1,13 @@ +chain(); diff --git a/tests/Rule/data/debug/mixed.php b/tests/Rule/data/debug/mixed.php new file mode 100644 index 00000000..d859ba4b --- /dev/null +++ b/tests/Rule/data/debug/mixed.php @@ -0,0 +1,14 @@ +any(); +} diff --git a/tests/Rule/data/debug/entrypoint.php b/tests/Rule/data/debug/regular.php similarity index 92% rename from tests/Rule/data/debug/entrypoint.php rename to tests/Rule/data/debug/regular.php index 1a96232f..a3ec5fdb 100644 --- a/tests/Rule/data/debug/entrypoint.php +++ b/tests/Rule/data/debug/regular.php @@ -10,6 +10,7 @@ class FooController { public function dummyAction(Another $another): void { $another->call(); + $another->call(); } } From 4c1f934b0f587c4391192bb7be66efb62c2eb2e9 Mon Sep 17 00:00:00 2001 From: Jan Nedbal Date: Wed, 12 Mar 2025 16:39:44 +0100 Subject: [PATCH 06/15] Readme: add debug info --- README.md | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/README.md b/README.md index ac6605ab..f671cc77 100644 --- a/README.md +++ b/README.md @@ -350,6 +350,33 @@ class IgnoreDeadInterfaceUsageProvider extends ReflectionBasedMemberUsageProvide } ``` +## Debugging: +- If you want to see how dead code detector evaluated usages of certain method, you do the following: + +```neon +parameters: + shipmonkDeadCode: + debug: + usagesOf: + - App\User\Entity\Address::__construct +``` + +Then, run PHPStan with `-vvv` CLI option and you will see the output like this: + +```txt +App\Facade\UserFacade::registerUser +| +| Elimination path: +| entrypoint App\User\RegisterUserController::__invoke:36 +| calls App\User\UserFacade::registerUser:142 +| calls App\User\Entity\Address::__construct +| +| Found 2 usages: +| • src/User/UserFacade.php:142 +| • tests/User/Entity/AddressTest.php:64 - excluded by tests excluder +``` + +If you set up `editorUrl` [parameter](https://phpstan.org/user-guide/output-format#opening-file-in-an-editor), you can click on the usages to open it in your IDE. ## Future scope: - Dead class property detection From d4877086efbe702210b1e5c8701f459ec22d2b91 Mon Sep 17 00:00:00 2001 From: Jan Nedbal Date: Wed, 12 Mar 2025 16:57:51 +0100 Subject: [PATCH 07/15] Drop UsageOriginDetector --- README.md | 8 ++++---- rules.neon | 3 --- src/Collector/ConstantFetchCollector.php | 10 +++------- src/Collector/MethodCallCollector.php | 13 ++++--------- src/Graph/UsageOriginDetector.php | 19 ------------------- src/Provider/ReflectionUsageProvider.php | 10 +++------- tests/Rule/DeadCodeRuleTest.php | 18 ++---------------- 7 files changed, 16 insertions(+), 65 deletions(-) delete mode 100644 src/Graph/UsageOriginDetector.php diff --git a/README.md b/README.md index f671cc77..a9f9f36a 100644 --- a/README.md +++ b/README.md @@ -157,7 +157,7 @@ use PHPStan\Analyser\Scope; use ReflectionMethod; use ShipMonk\PHPStan\DeadCode\Graph\ClassMethodRef; use ShipMonk\PHPStan\DeadCode\Graph\ClassMethodUsage; -use ShipMonk\PHPStan\DeadCode\Graph\UsageOriginDetector; +use ShipMonk\PHPStan\DeadCode\Graph\UsageOrigin; use ShipMonk\PHPStan\DeadCode\Provider\MemberUsageProvider; use Symfony\Component\Serializer\SerializerInterface; @@ -185,13 +185,13 @@ class DeserializationUsageProvider implements MemberUsageProvider $secondArgument = $node->getArgs()[1]->value; $serializedClass = $scope->getType($secondArgument)->getConstantStrings()[0]; - // record the method it was called from (needed for proper transitive dead code elimination) - $originRef = $this->originDetector->detectOrigin($scope); + // record the place it was called from (needed for proper transitive dead code elimination) + $usageOrigin = UsageOrigin::createRegular($node, $scope); // record the hidden constructor call $constructorRef = new ClassMethodRef($serializedClass->getValue(), '__construct', false); - return [new ClassMethodUsage($originRef, $constructorRef)]; + return [new ClassMethodUsage($usageOrigin, $constructorRef)]; } return []; diff --git a/rules.neon b/rules.neon index 8fb96875..44f9173c 100644 --- a/rules.neon +++ b/rules.neon @@ -7,9 +7,6 @@ services: - class: ShipMonk\PHPStan\DeadCode\Transformer\FileSystem - - - class: ShipMonk\PHPStan\DeadCode\Graph\UsageOriginDetector - - class: ShipMonk\PHPStan\DeadCode\Debug\DebugUsagePrinter arguments: diff --git a/src/Collector/ConstantFetchCollector.php b/src/Collector/ConstantFetchCollector.php index 2670d17b..98a21061 100644 --- a/src/Collector/ConstantFetchCollector.php +++ b/src/Collector/ConstantFetchCollector.php @@ -18,7 +18,7 @@ use ShipMonk\PHPStan\DeadCode\Graph\ClassConstantRef; use ShipMonk\PHPStan\DeadCode\Graph\ClassConstantUsage; use ShipMonk\PHPStan\DeadCode\Graph\CollectedUsage; -use ShipMonk\PHPStan\DeadCode\Graph\UsageOriginDetector; +use ShipMonk\PHPStan\DeadCode\Graph\UsageOrigin; use function array_map; use function count; use function current; @@ -33,8 +33,6 @@ class ConstantFetchCollector implements Collector use BufferedUsageCollector; - private UsageOriginDetector $usageOriginDetector; - private ReflectionProvider $reflectionProvider; private bool $trackMixedAccess; @@ -48,7 +46,6 @@ class ConstantFetchCollector implements Collector * @param list $memberUsageExcluders */ public function __construct( - UsageOriginDetector $usageOriginDetector, ReflectionProvider $reflectionProvider, bool $trackMixedAccess, array $memberUsageExcluders @@ -56,7 +53,6 @@ public function __construct( { $this->reflectionProvider = $reflectionProvider; $this->trackMixedAccess = $trackMixedAccess; - $this->usageOriginDetector = $usageOriginDetector; $this->memberUsageExcluders = $memberUsageExcluders; } @@ -125,7 +121,7 @@ private function registerFunctionCall(FuncCall $node, Scope $scope): void $this->registerUsage( new ClassConstantUsage( - $this->usageOriginDetector->detectOrigin($node, $scope), + UsageOrigin::createRegular($node, $scope), new ClassConstantRef($className, $constantName, true), ), $node, @@ -157,7 +153,7 @@ private function registerFetch(ClassConstFetch $node, Scope $scope): void foreach ($this->getDeclaringTypesWithConstant($ownerType, $constantName) as $className) { $this->registerUsage( new ClassConstantUsage( - $this->usageOriginDetector->detectOrigin($node, $scope), + UsageOrigin::createRegular($node, $scope), new ClassConstantRef($className, $constantName, $possibleDescendantFetch), ), $node, diff --git a/src/Collector/MethodCallCollector.php b/src/Collector/MethodCallCollector.php index d88fa03e..bc1f4c20 100644 --- a/src/Collector/MethodCallCollector.php +++ b/src/Collector/MethodCallCollector.php @@ -26,7 +26,6 @@ use ShipMonk\PHPStan\DeadCode\Graph\ClassMethodUsage; use ShipMonk\PHPStan\DeadCode\Graph\CollectedUsage; use ShipMonk\PHPStan\DeadCode\Graph\UsageOrigin; -use ShipMonk\PHPStan\DeadCode\Graph\UsageOriginDetector; /** * @implements Collector> @@ -36,8 +35,6 @@ class MethodCallCollector implements Collector use BufferedUsageCollector; - private UsageOriginDetector $usageOriginDetector; - private bool $trackMixedAccess; /** @@ -49,12 +46,10 @@ class MethodCallCollector implements Collector * @param list $memberUsageExcluders */ public function __construct( - UsageOriginDetector $usageOriginDetector, bool $trackMixedAccess, array $memberUsageExcluders ) { - $this->usageOriginDetector = $usageOriginDetector; $this->trackMixedAccess = $trackMixedAccess; $this->memberUsageExcluders = $memberUsageExcluders; } @@ -134,7 +129,7 @@ private function registerMethodCall( foreach ($this->getDeclaringTypesWithMethod($methodName, $callerType, TrinaryLogic::createNo(), $possibleDescendantCall) as $methodRef) { $this->registerUsage( new ClassMethodUsage( - $this->usageOriginDetector->detectOrigin($methodCall, $scope), + UsageOrigin::createRegular($methodCall, $scope), $methodRef, ), $methodCall, @@ -164,7 +159,7 @@ private function registerStaticCall( foreach ($this->getDeclaringTypesWithMethod($methodName, $callerType, TrinaryLogic::createYes(), $possibleDescendantCall) as $methodRef) { $this->registerUsage( new ClassMethodUsage( - $this->usageOriginDetector->detectOrigin($staticCall, $scope), + UsageOrigin::createRegular($staticCall, $scope), $methodRef, ), $staticCall, @@ -190,7 +185,7 @@ private function registerArrayCallable( foreach ($this->getDeclaringTypesWithMethod($methodName, $caller, TrinaryLogic::createMaybe()) as $methodRef) { $this->registerUsage( new ClassMethodUsage( - $this->usageOriginDetector->detectOrigin($array, $scope), + UsageOrigin::createRegular($array, $scope), $methodRef, ), $array, @@ -222,7 +217,7 @@ private function registerClone(Clone_ $node, Scope $scope): void foreach ($this->getDeclaringTypesWithMethod($methodName, $callerType, TrinaryLogic::createNo()) as $methodRef) { $this->registerUsage( new ClassMethodUsage( - $this->usageOriginDetector->detectOrigin($node, $scope), + UsageOrigin::createRegular($node, $scope), $methodRef, ), $node, diff --git a/src/Graph/UsageOriginDetector.php b/src/Graph/UsageOriginDetector.php deleted file mode 100644 index 8f68eea3..00000000 --- a/src/Graph/UsageOriginDetector.php +++ /dev/null @@ -1,19 +0,0 @@ -usageOriginDetector = $usageOriginDetector; $this->enabled = $enabled; } @@ -191,7 +187,7 @@ private function createConstantUsage( ): ClassConstantUsage { return new ClassConstantUsage( - $this->usageOriginDetector->detectOrigin($node, $scope), + UsageOrigin::createRegular($node, $scope), new ClassConstantRef( $className, $constantName, @@ -208,7 +204,7 @@ private function createMethodUsage( ): ClassMethodUsage { return new ClassMethodUsage( - $this->usageOriginDetector->detectOrigin($node, $scope), + UsageOrigin::createRegular($node, $scope), new ClassMethodRef( $className, $methodName, diff --git a/tests/Rule/DeadCodeRuleTest.php b/tests/Rule/DeadCodeRuleTest.php index 26cbca73..165c7eff 100644 --- a/tests/Rule/DeadCodeRuleTest.php +++ b/tests/Rule/DeadCodeRuleTest.php @@ -27,7 +27,6 @@ use ShipMonk\PHPStan\DeadCode\Excluder\TestsUsageExcluder; use ShipMonk\PHPStan\DeadCode\Formatter\RemoveDeadCodeFormatter; use ShipMonk\PHPStan\DeadCode\Graph\ClassMemberUsage; -use ShipMonk\PHPStan\DeadCode\Graph\UsageOriginDetector; use ShipMonk\PHPStan\DeadCode\Hierarchy\ClassHierarchy; use ShipMonk\PHPStan\DeadCode\Provider\DoctrineUsageProvider; use ShipMonk\PHPStan\DeadCode\Provider\MemberUsageProvider; @@ -95,8 +94,8 @@ protected function getCollectors(): array return [ new ProvidedUsagesCollector($reflectionProvider, $this->getMemberUsageProviders(), $this->getMemberUsageExcluders()), new ClassDefinitionCollector($reflectionProvider), - new MethodCallCollector($this->createUsageOriginDetector(), $this->trackMixedAccess, $this->getMemberUsageExcluders()), - new ConstantFetchCollector($this->createUsageOriginDetector(), $reflectionProvider, $this->trackMixedAccess, $this->getMemberUsageExcluders()), + new MethodCallCollector($this->trackMixedAccess, $this->getMemberUsageExcluders()), + new ConstantFetchCollector($reflectionProvider, $this->trackMixedAccess, $this->getMemberUsageExcluders()), ]; } @@ -601,7 +600,6 @@ private function getMemberUsageProviders(): array { return [ new ReflectionUsageProvider( - $this->createUsageOriginDetector(), true, ), new class extends ReflectionBasedMemberUsageProvider @@ -684,18 +682,6 @@ static function (string $type): array { return $mock; } - private function createUsageOriginDetector(): UsageOriginDetector - { - /** @var UsageOriginDetector|null $detector */ - static $detector = null; - - if ($detector === null) { - $detector = new UsageOriginDetector(); - } - - return $detector; - } - public function gatherAnalyserErrors(array $files): array { if (!$this->unwrapGroupedErrors) { From 6d414577b08718b3bc6a6710a4247ae07523e813 Mon Sep 17 00:00:00 2001 From: Jan Nedbal Date: Wed, 12 Mar 2025 17:06:13 +0100 Subject: [PATCH 08/15] Readme: fix typo --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index a9f9f36a..2c986837 100644 --- a/README.md +++ b/README.md @@ -364,7 +364,7 @@ parameters: Then, run PHPStan with `-vvv` CLI option and you will see the output like this: ```txt -App\Facade\UserFacade::registerUser +App\User\Entity\Address::__construct | | Elimination path: | entrypoint App\User\RegisterUserController::__invoke:36 From 6ce017be5af2b5a3ab588115fa4cf6b29b4198aa Mon Sep 17 00:00:00 2001 From: Jan Nedbal Date: Thu, 13 Mar 2025 17:05:28 +0100 Subject: [PATCH 09/15] DebugPrinter: better error messages --- src/Debug/DebugUsagePrinter.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Debug/DebugUsagePrinter.php b/src/Debug/DebugUsagePrinter.php index e55138ea..129c8908 100644 --- a/src/Debug/DebugUsagePrinter.php +++ b/src/Debug/DebugUsagePrinter.php @@ -296,14 +296,14 @@ private function buildDebugMemberKeys(array $debugMembers): array foreach ($debugMembers as $debugMember) { if (strpos($debugMember, '::') === false) { - throw new LogicException("Invalid debug member format: $debugMember"); + throw new LogicException("Invalid debug member format: '$debugMember', expected 'ClassName::memberName'"); } [$class, $memberName] = explode('::', $debugMember); // @phpstan-ignore offsetAccess.notFound $normalizedClass = ltrim($class, '\\'); if (!$this->reflectionProvider->hasClass($normalizedClass)) { - throw new LogicException("Class $normalizedClass does not exist"); + throw new LogicException("Class '$normalizedClass' does not exist"); } $classReflection = $this->reflectionProvider->getClass($normalizedClass); @@ -315,10 +315,10 @@ private function buildDebugMemberKeys(array $debugMembers): array $key = ClassConstantRef::buildKey($normalizedClass, $memberName); } elseif ($classReflection->hasProperty($memberName)) { - throw new LogicException('Properties are not yet supported'); + throw new LogicException("Cannot debug '$debugMember', properties are not supported yet"); } else { - throw new LogicException("Member $memberName does not exist in $normalizedClass"); + throw new LogicException("Member '$memberName' does not exist in '$normalizedClass'"); } $result[$key] = []; From c1d478a4c0df90d0bcb89e261b72272ef42243d8 Mon Sep 17 00:00:00 2001 From: Jan Nedbal Date: Fri, 14 Mar 2025 09:32:36 +0100 Subject: [PATCH 10/15] Fix alternative usage debug, improve tests and edgecases --- src/Debug/DebugUsagePrinter.php | 90 ++++++++++++++++++++++----- src/Rule/DeadCodeRule.php | 8 ++- tests/Rule/DeadCodeRuleTest.php | 48 ++++++++++++++ tests/Rule/data/debug/alternative.php | 13 ++++ tests/Rule/data/debug/foreign.php | 5 ++ 5 files changed, 146 insertions(+), 18 deletions(-) create mode 100644 tests/Rule/data/debug/alternative.php create mode 100644 tests/Rule/data/debug/foreign.php diff --git a/src/Debug/DebugUsagePrinter.php b/src/Debug/DebugUsagePrinter.php index 129c8908..65f448a4 100644 --- a/src/Debug/DebugUsagePrinter.php +++ b/src/Debug/DebugUsagePrinter.php @@ -5,7 +5,9 @@ use LogicException; use PHPStan\Command\Output; use PHPStan\File\RelativePathHelper; +use PHPStan\Reflection\ClassReflection; use PHPStan\Reflection\ReflectionProvider; +use ReflectionException; use ShipMonk\PHPStan\DeadCode\Enum\MemberType; use ShipMonk\PHPStan\DeadCode\Error\BlackMember; use ShipMonk\PHPStan\DeadCode\Graph\ClassConstantRef; @@ -15,6 +17,7 @@ use ShipMonk\PHPStan\DeadCode\Graph\UsageOrigin; use function array_map; use function array_sum; +use function array_unique; use function count; use function explode; use function ltrim; @@ -40,7 +43,7 @@ class DebugUsagePrinter /** * memberKey => usage info * - * @var array, eliminationPath?: array>, neverReported?: string}> + * @var array, eliminationPath?: array>, neverReported?: string}> */ private array $debugMembers; @@ -122,7 +125,10 @@ private function getExampleCaller(array $usages): ?string return null; } - public function printDebugMemberUsages(Output $output): void + /** + * @param array $analysedClasses + */ + public function printDebugMemberUsages(Output $output, array $analysedClasses): void { if ($this->debugMembers === [] || !$output->isDebug()) { return; @@ -131,6 +137,8 @@ public function printDebugMemberUsages(Output $output): void $output->writeLineFormatted("\nUsage debugging information:"); foreach ($this->debugMembers as $memberKey => $debugMember) { + $typeName = $debugMember['typename']; + $output->writeLineFormatted(sprintf("\n%s", $this->prettyMemberKey($memberKey))); if (isset($debugMember['eliminationPath'])) { @@ -158,7 +166,12 @@ public function printDebugMemberUsages(Output $output): void } } elseif (isset($debugMember['usages'])) { $output->writeLineFormatted("|\n| Elimination path:"); - $output->writeLineFormatted('| not found, all usages originate in unused code'); + + if (!isset($analysedClasses[$typeName])) { + $output->writeLineFormatted("| '$typeName' is not defined within analysed files"); + } else { + $output->writeLineFormatted('| not found, all usages originate in unused code'); + } } if (isset($debugMember['usages'])) { @@ -250,15 +263,23 @@ private function getOriginLink(UsageOrigin $origin, string $title): string return $title; } - public function recordUsage(CollectedUsage $collectedUsage): void + /** + * @param list $alternativeKeys + */ + public function recordUsage(CollectedUsage $collectedUsage, array $alternativeKeys = []): void { - $memberKey = $collectedUsage->getUsage()->getMemberRef()->toKey(); + $memberKeys = array_unique([ + $collectedUsage->getUsage()->getMemberRef()->toKey(), + ...$alternativeKeys, + ]); + + foreach ($memberKeys as $memberKey) { + if (!isset($this->debugMembers[$memberKey])) { + continue; + } - if (!isset($this->debugMembers[$memberKey])) { - return; + $this->debugMembers[$memberKey]['usages'][] = $collectedUsage; } - - $this->debugMembers[$memberKey]['usages'][] = $collectedUsage; } /** @@ -288,7 +309,7 @@ public function markMemberAsNeverReported(BlackMember $blackMember, string $reas /** * @param list $debugMembers - * @return array, eliminationPath?: array>, neverReported?: string}> + * @return array, eliminationPath?: array>, neverReported?: string}> */ private function buildDebugMemberKeys(array $debugMembers): array { @@ -308,23 +329,62 @@ private function buildDebugMemberKeys(array $debugMembers): array $classReflection = $this->reflectionProvider->getClass($normalizedClass); - if ($classReflection->hasMethod($memberName)) { + if ($this->hasOwnMethod($classReflection, $memberName)) { $key = ClassMethodRef::buildKey($normalizedClass, $memberName); - } elseif ($classReflection->hasConstant($memberName)) { + } elseif ($this->hasOwnConstant($classReflection, $memberName)) { $key = ClassConstantRef::buildKey($normalizedClass, $memberName); - } elseif ($classReflection->hasProperty($memberName)) { + } elseif ($this->hasOwnProperty($classReflection, $memberName)) { throw new LogicException("Cannot debug '$debugMember', properties are not supported yet"); } else { - throw new LogicException("Member '$memberName' does not exist in '$normalizedClass'"); + throw new LogicException("Member '$memberName' does not exist directly in '$normalizedClass'"); } - $result[$key] = []; + $result[$key] = [ + 'typename' => $normalizedClass, + ]; } return $result; } + private function hasOwnMethod(ClassReflection $classReflection, string $methodName): bool + { + if (!$classReflection->hasMethod($methodName)) { + return false; + } + + try { + return $classReflection->getNativeReflection()->getMethod($methodName)->getBetterReflection()->getDeclaringClass()->getName() === $classReflection->getName(); + } catch (ReflectionException $e) { + return false; + } + } + + private function hasOwnConstant(ClassReflection $classReflection, string $constantName): bool + { + $constantReflection = $classReflection->getNativeReflection()->getReflectionConstant($constantName); + + if ($constantReflection === false) { + return false; + } + + return $constantReflection->getBetterReflection()->getDeclaringClass()->getName() === $classReflection->getName(); + } + + private function hasOwnProperty(ClassReflection $classReflection, string $propertyName): bool + { + if (!$classReflection->hasProperty($propertyName)) { + return false; + } + + try { + return $classReflection->getNativeReflection()->getProperty($propertyName)->getBetterReflection()->getDeclaringClass()->getName() === $classReflection->getName(); + } catch (ReflectionException $e) { + return false; + } + } + } diff --git a/src/Rule/DeadCodeRule.php b/src/Rule/DeadCodeRule.php index 6d8beae1..50560889 100644 --- a/src/Rule/DeadCodeRule.php +++ b/src/Rule/DeadCodeRule.php @@ -231,8 +231,6 @@ public function processNode( $excludedMemberUsages = []; foreach ($knownCollectedUsages as $collectedUsage) { - $this->debugUsagePrinter->recordUsage($collectedUsage); - if ($collectedUsage->isExcluded()) { $excludedMemberUsages[] = $collectedUsage; continue; @@ -255,6 +253,8 @@ public function processNode( $whiteMemberKeys[] = $alternativeMemberKey; } } + + $this->debugUsagePrinter->recordUsage($collectedUsage, $alternativeMemberKeys); } foreach ($whiteMemberKeys as $whiteCalleeKey) { @@ -272,6 +272,8 @@ public function processNode( } foreach ($excludedMemberUsages as $excludedMemberUsage) { + $this->debugUsagePrinter->recordUsage($excludedMemberUsage); + $excludedBy = $excludedMemberUsage->getExcludedBy(); $excludedMemberRef = $excludedMemberUsage->getUsage()->getMemberRef(); @@ -735,7 +737,7 @@ private function isNeverReportedAsDead(BlackMember $blackMember): ?string public function print(Output $output): void { $this->debugUsagePrinter->printMixedMemberUsages($output, $this->mixedMemberUsages); - $this->debugUsagePrinter->printDebugMemberUsages($output); + $this->debugUsagePrinter->printDebugMemberUsages($output, $this->typeDefinitions); } } diff --git a/tests/Rule/DeadCodeRuleTest.php b/tests/Rule/DeadCodeRuleTest.php index 165c7eff..d00b6bca 100644 --- a/tests/Rule/DeadCodeRuleTest.php +++ b/tests/Rule/DeadCodeRuleTest.php @@ -189,6 +189,8 @@ static function (string $message) use (&$actualOutput): void { public function testDebugUsage(): void { $this->debugMembers = [ + 'DateTime::format', + 'DebugAlternative\Foo::foo', 'DebugCtor\Foo::__construct', 'DebugExclude\Foo::mixedExcluder', 'DebugNever\Foo::__get', @@ -200,9 +202,11 @@ public function testDebugUsage(): void 'DebugZero\Foo::__construct', ]; $this->analyseFiles([ + __DIR__ . '/data/debug/alternative.php', __DIR__ . '/data/debug/ctor.php', __DIR__ . '/data/debug/exclude.php', __DIR__ . '/data/debug/cycle.php', + __DIR__ . '/data/debug/foreign.php', __DIR__ . '/data/debug/global.php', __DIR__ . '/data/debug/mixed.php', __DIR__ . '/data/debug/never.php', @@ -244,6 +248,24 @@ static function (string $message) use (&$actualOutput): void { Usage debugging information: + DateTime::format + | + | Elimination path: + | 'DateTime' is not defined within analysed files + | + | Found 1 usage: + | • data/debug/foreign.php:5 + + + DebugAlternative\Foo::foo + | + | Elimination path: + | direct usage + | + | Found 1 usage: + | • data/debug/alternative.php:13 + + DebugCtor\Foo::__construct | | Elimination path: @@ -324,6 +346,32 @@ static function (string $message) use (&$actualOutput): void { self::assertSame($expectedOutput, $this->trimFgColors($actualOutput)); } + /** + * @dataProvider provideDebugUsageInvalidArgs + */ + public function testDebugUsageInvalidArgs(string $member, string $error): void + { + $this->expectException(LogicException::class); + $this->expectExceptionMessage($error); + + $this->debugMembers = [$member]; + $this->analyseFiles([__DIR__ . '/data/debug/alternative.php']); + $this->getRule(); + } + + /** + * @return array + */ + public static function provideDebugUsageInvalidArgs(): array + { + return [ + 'method not owned' => ['DebugAlternative\Clazz::foo', "Member 'foo' does not exist directly in 'DebugAlternative\Clazz'"], + 'no method' => ['DebugAlternative\Clazz::xyz', "Member 'xyz' does not exist directly in 'DebugAlternative\Clazz'"], + 'no class' => ['InvalidClass::foo', "Class 'InvalidClass' does not exist"], + 'invalid format' => ['InvalidFormat', "Invalid debug member format: 'InvalidFormat', expected 'ClassName::memberName'"], + ]; + } + /** * @dataProvider provideAutoRemoveFiles */ diff --git a/tests/Rule/data/debug/alternative.php b/tests/Rule/data/debug/alternative.php new file mode 100644 index 00000000..e7672611 --- /dev/null +++ b/tests/Rule/data/debug/alternative.php @@ -0,0 +1,13 @@ +foo(); diff --git a/tests/Rule/data/debug/foreign.php b/tests/Rule/data/debug/foreign.php new file mode 100644 index 00000000..c029db4c --- /dev/null +++ b/tests/Rule/data/debug/foreign.php @@ -0,0 +1,5 @@ +format('Y-m-d H:i:s'); From d2217af19832f3e74a2b8899311f976300b2ac91 Mon Sep 17 00:00:00 2001 From: Jan Nedbal Date: Fri, 14 Mar 2025 10:30:24 +0100 Subject: [PATCH 11/15] Bypass PHPStan bug, allow debugging of constants --- rules.neon | 1 - src/Debug/DebugUsagePrinter.php | 13 +++++++------ tests/Rule/DeadCodeRuleTest.php | 7 ++++++- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/rules.neon b/rules.neon index 44f9173c..1af62305 100644 --- a/rules.neon +++ b/rules.neon @@ -11,7 +11,6 @@ services: class: ShipMonk\PHPStan\DeadCode\Debug\DebugUsagePrinter arguments: trackMixedAccess: %shipmonkDeadCode.trackMixedAccess% - debugMembers: %shipmonkDeadCode.debug.usagesOf% editorUrl: %editorUrl% - diff --git a/src/Debug/DebugUsagePrinter.php b/src/Debug/DebugUsagePrinter.php index 65f448a4..0cef2261 100644 --- a/src/Debug/DebugUsagePrinter.php +++ b/src/Debug/DebugUsagePrinter.php @@ -4,6 +4,7 @@ use LogicException; use PHPStan\Command\Output; +use PHPStan\DependencyInjection\Container; use PHPStan\File\RelativePathHelper; use PHPStan\Reflection\ClassReflection; use PHPStan\Reflection\ReflectionProvider; @@ -47,22 +48,22 @@ class DebugUsagePrinter */ private array $debugMembers; - /** - * @param list $debugMembers - */ public function __construct( + Container $container, RelativePathHelper $relativePathHelper, ReflectionProvider $reflectionProvider, ?string $editorUrl, - bool $trackMixedAccess, - array $debugMembers + bool $trackMixedAccess ) { $this->relativePathHelper = $relativePathHelper; $this->reflectionProvider = $reflectionProvider; $this->editorUrl = $editorUrl; $this->trackMixedAccess = $trackMixedAccess; - $this->debugMembers = $this->buildDebugMemberKeys($debugMembers); + $this->debugMembers = $this->buildDebugMemberKeys( + // @phpstan-ignore offsetAccess.nonOffsetAccessible, offsetAccess.nonOffsetAccessible, missingType.checkedException, argument.type + $container->getParameter('shipmonkDeadCode')['debug']['usagesOf'], // prevents https://github.com/phpstan/phpstan/issues/12740 + ); } /** diff --git a/tests/Rule/DeadCodeRuleTest.php b/tests/Rule/DeadCodeRuleTest.php index d00b6bca..ac3cb37a 100644 --- a/tests/Rule/DeadCodeRuleTest.php +++ b/tests/Rule/DeadCodeRuleTest.php @@ -66,14 +66,19 @@ class DeadCodeRuleTest extends RuleTestCase protected function getRule(): DeadCodeRule { + $container = $this->createMock(Container::class); + $container->expects(self::any()) + ->method('getParameter') + ->willReturn(['debug' => ['usagesOf' => $this->debugMembers]]); + if ($this->rule === null) { $this->rule = new DeadCodeRule( new DebugUsagePrinter( + $container, new SimpleRelativePathHelper(__DIR__), // @phpstan-ignore phpstanApi.constructor self::createReflectionProvider(), null, true, - $this->debugMembers, ), new ClassHierarchy(), !$this->emitErrorsInGroups, From 24cfa5fce262f791010773230f9eab73188b6ece Mon Sep 17 00:00:00 2001 From: Jan Nedbal Date: Fri, 14 Mar 2025 13:52:40 +0100 Subject: [PATCH 12/15] Improve debug output to always contain true entrypoint --- .ecrc | 5 + phpcs.xml.dist | 2 +- src/Debug/DebugUsagePrinter.php | 68 +++++++------ src/Graph/UsageOrigin.php | 2 +- src/Rule/DeadCodeRule.php | 21 ++-- tests/Rule/DeadCodeRuleTest.php | 113 ++------------------- tests/Rule/data/debug/expected_output.txt | 114 ++++++++++++++++++++++ tests/Rule/data/debug/global.php | 12 ++- tests/Rule/data/debug/unsupported.php | 14 +++ 9 files changed, 194 insertions(+), 157 deletions(-) create mode 100644 .ecrc create mode 100644 tests/Rule/data/debug/expected_output.txt create mode 100644 tests/Rule/data/debug/unsupported.php diff --git a/.ecrc b/.ecrc new file mode 100644 index 00000000..430e6751 --- /dev/null +++ b/.ecrc @@ -0,0 +1,5 @@ +{ + "Exclude": [ + "^tests/Rule/data/debug/expected_output.txt" + ] +} diff --git a/phpcs.xml.dist b/phpcs.xml.dist index 58b3cc6e..6cd93ebc 100644 --- a/phpcs.xml.dist +++ b/phpcs.xml.dist @@ -30,7 +30,7 @@ - + diff --git a/src/Debug/DebugUsagePrinter.php b/src/Debug/DebugUsagePrinter.php index 0cef2261..65699051 100644 --- a/src/Debug/DebugUsagePrinter.php +++ b/src/Debug/DebugUsagePrinter.php @@ -44,7 +44,7 @@ class DebugUsagePrinter /** * memberKey => usage info * - * @var array, eliminationPath?: array>, neverReported?: string}> + * @var array, eliminationPath?: array>, neverReported?: string}> */ private array $debugMembers; @@ -143,36 +143,35 @@ public function printDebugMemberUsages(Output $output, array $analysedClasses): $output->writeLineFormatted(sprintf("\n%s", $this->prettyMemberKey($memberKey))); if (isset($debugMember['eliminationPath'])) { - $output->writeLineFormatted("|\n| Elimination path:"); - $depth = 0; + $output->writeLineFormatted("|\n| Marked as alive at:"); + $depth = 1; - if (count($debugMember['eliminationPath']) === 1) { - $output->writeLineFormatted('| direct usage'); + foreach ($debugMember['eliminationPath'] as $fragmentKey => $fragmentUsages) { + if ($depth === 1) { + $entrypoint = $this->getOriginReference($fragmentUsages[0]->getOrigin(), false); + $output->writeLineFormatted(sprintf('| entry %s', $entrypoint)); + } - } else { - foreach ($debugMember['eliminationPath'] as $fragmentKey => $fragmentUsages) { - $indent = $depth === 0 ? 'entrypoint ' : ' ' . str_repeat(' ', $depth) . 'calls '; - $nextFragmentUsages = next($debugMember['eliminationPath']); - $nextFragmentFirstUsage = $nextFragmentUsages !== false ? reset($nextFragmentUsages) : null; - $nextFragmentFirstUsageOrigin = $nextFragmentFirstUsage instanceof ClassMemberUsage ? $nextFragmentFirstUsage->getOrigin() : null; + $indent = str_repeat(' ', $depth) . 'calls '; - $pathFragment = $nextFragmentFirstUsageOrigin === null - ? $this->prettyMemberKey($fragmentKey) - : $this->getOriginLink($nextFragmentFirstUsageOrigin, $this->prettyMemberKey($fragmentKey)); + $nextFragmentUsages = next($debugMember['eliminationPath']); + $nextFragmentFirstUsage = $nextFragmentUsages !== false ? reset($nextFragmentUsages) : null; + $nextFragmentFirstUsageOrigin = $nextFragmentFirstUsage instanceof ClassMemberUsage ? $nextFragmentFirstUsage->getOrigin() : null; - $output->writeLineFormatted(sprintf('| %s%s', $indent, $pathFragment)); + $pathFragment = $nextFragmentFirstUsageOrigin === null + ? $this->prettyMemberKey($fragmentKey) + : $this->getOriginLink($nextFragmentFirstUsageOrigin, $this->prettyMemberKey($fragmentKey)); - $depth++; - } - } - } elseif (isset($debugMember['usages'])) { - $output->writeLineFormatted("|\n| Elimination path:"); + $output->writeLineFormatted(sprintf('| %s%s', $indent, $pathFragment)); - if (!isset($analysedClasses[$typeName])) { - $output->writeLineFormatted("| '$typeName' is not defined within analysed files"); - } else { - $output->writeLineFormatted('| not found, all usages originate in unused code'); + $depth++; } + } elseif (!isset($analysedClasses[$typeName])) { + $output->writeLineFormatted("|\n| Not defined within analysed files!"); + + } elseif (isset($debugMember['usages'])) { + $output->writeLineFormatted("|\n| Dead because:"); + $output->writeLineFormatted('| all usages originate in unused code'); } if (isset($debugMember['usages'])) { @@ -210,7 +209,7 @@ private function prettyMemberKey(string $memberKey): string return $replaced; } - private function getOriginReference(UsageOrigin $origin): string + private function getOriginReference(UsageOrigin $origin, bool $preferFileLine = true): string { $file = $origin->getFile(); $line = $origin->getLine(); @@ -218,19 +217,18 @@ private function getOriginReference(UsageOrigin $origin): string if ($file !== null && $line !== null) { $relativeFile = $this->relativePathHelper->getRelativePath($file); + $title = $origin->getClassName() !== null && $origin->getMethodName() !== null && !$preferFileLine + ? sprintf('%s::%s:%d', $origin->getClassName(), $origin->getMethodName(), $line) + : sprintf('%s:%s', $relativeFile, $line); + if ($this->editorUrl === null) { - return sprintf( - '%s:%s', - $relativeFile, - $line, - ); + return $title; } return sprintf( - '%s:%s', + '%s', str_replace(['%file%', '%relFile%', '%line%'], [$file, $relativeFile, (string) $line], $this->editorUrl), - $relativeFile, - $line, + $title, ); } @@ -284,7 +282,7 @@ public function recordUsage(CollectedUsage $collectedUsage, array $alternativeKe } /** - * @param array> $eliminationPath + * @param array> $eliminationPath */ public function markMemberAsWhite(BlackMember $blackMember, array $eliminationPath): void { @@ -310,7 +308,7 @@ public function markMemberAsNeverReported(BlackMember $blackMember, string $reas /** * @param list $debugMembers - * @return array, eliminationPath?: array>, neverReported?: string}> + * @return array, eliminationPath?: array>, neverReported?: string}> */ private function buildDebugMemberKeys(array $debugMembers): array { diff --git a/src/Graph/UsageOrigin.php b/src/Graph/UsageOrigin.php index a20b8fe3..b06698d0 100644 --- a/src/Graph/UsageOrigin.php +++ b/src/Graph/UsageOrigin.php @@ -33,7 +33,7 @@ final class UsageOrigin public function __construct( ?string $className, ?string $methodName, - ?string $fileName, // TODO try reusing collector file to reduce memory usage + ?string $fileName, ?int $line, ?string $provider, ?string $note diff --git a/src/Rule/DeadCodeRule.php b/src/Rule/DeadCodeRule.php index 50560889..e940ec33 100644 --- a/src/Rule/DeadCodeRule.php +++ b/src/Rule/DeadCodeRule.php @@ -109,7 +109,7 @@ class DeadCodeRule implements Rule, DiagnoseExtension private array $mixedMemberUsages = []; /** - * @var array>> callerKey => array + * @var array>> callerKey => array */ private array $usageGraph = []; @@ -225,8 +225,8 @@ public function processNode( } } - /** @var list $whiteMemberKeys */ - $whiteMemberKeys = []; + /** @var array> $whiteMembers */ + $whiteMembers = []; /** @var list $excludedMemberUsages */ $excludedMemberUsages = []; @@ -250,15 +250,15 @@ public function processNode( } if ($isWhite) { - $whiteMemberKeys[] = $alternativeMemberKey; + $whiteMembers[$alternativeMemberKey][] = $collectedUsage->getUsage(); } } $this->debugUsagePrinter->recordUsage($collectedUsage, $alternativeMemberKeys); } - foreach ($whiteMemberKeys as $whiteCalleeKey) { - $this->markTransitivesWhite($whiteCalleeKey); + foreach ($whiteMembers as $whiteCalleeKey => $usages) { + $this->markTransitivesWhite($whiteCalleeKey, [$whiteCalleeKey => $usages]); } foreach ($this->blackMembers as $blackMemberKey => $blackMember) { @@ -460,14 +460,13 @@ private function findDefinerMemberKey( } /** - * @param array> $visited + * @param non-empty-array> $visited */ - private function markTransitivesWhite(string $callerKey, array $visited = []): void + private function markTransitivesWhite(string $callerKey, array $visited): void { - $visited = $visited === [] ? [$callerKey => []] : $visited; // TODO [] init? $callees = $this->usageGraph[$callerKey] ?? []; - if (isset($this->blackMembers[$callerKey])) { // TODO debug why not always present + if (isset($this->blackMembers[$callerKey])) { $this->debugUsagePrinter->markMemberAsWhite($this->blackMembers[$callerKey], $visited); unset($this->blackMembers[$callerKey]); @@ -690,7 +689,7 @@ private function getTraitUsages(string $typeName): array private function isConsideredWhite(ClassMemberUsage $memberUsage): bool { - return $memberUsage->getOrigin()->getClassName() === null + return $memberUsage->getOrigin()->getClassName() === null // out-of-class scope || $this->isAnonymousClass($memberUsage->getOrigin()->getClassName()) || (array_key_exists((string) $memberUsage->getOrigin()->getMethodName(), self::UNSUPPORTED_MAGIC_METHODS)); } diff --git a/tests/Rule/DeadCodeRuleTest.php b/tests/Rule/DeadCodeRuleTest.php index ac3cb37a..0d59d1bd 100644 --- a/tests/Rule/DeadCodeRuleTest.php +++ b/tests/Rule/DeadCodeRuleTest.php @@ -200,10 +200,11 @@ public function testDebugUsage(): void 'DebugExclude\Foo::mixedExcluder', 'DebugNever\Foo::__get', 'DebugVirtual\FooTest::testFoo', - 'DebugGlobal\Foo::chain', + 'DebugGlobal\Foo::chain2', 'DebugMixed\Foo::any', 'DebugCycle\Foo::__construct', 'DebugRegular\Another::call', + 'DebugUnsupported\Foo::notDead', 'DebugZero\Foo::__construct', ]; $this->analyseFiles([ @@ -216,6 +217,7 @@ public function testDebugUsage(): void __DIR__ . '/data/debug/mixed.php', __DIR__ . '/data/debug/never.php', __DIR__ . '/data/debug/regular.php', + __DIR__ . '/data/debug/unsupported.php', __DIR__ . '/data/debug/virtual.php', __DIR__ . '/data/debug/zero.php', ]); @@ -243,112 +245,9 @@ static function (string $message) use (&$actualOutput): void { $rule->print($output); - $ec = ''; - $expectedOutput = <<<"OUTPUT" - Found 1 usage over unknown type: - $ec • any method, for example in data/debug/mixed.php:13 - - Thus, any member named the same is considered used, no matter its declaring class! - - - Usage debugging information: - - DateTime::format - | - | Elimination path: - | 'DateTime' is not defined within analysed files - | - | Found 1 usage: - | • data/debug/foreign.php:5 - - - DebugAlternative\Foo::foo - | - | Elimination path: - | direct usage - | - | Found 1 usage: - | • data/debug/alternative.php:13 - - - DebugCtor\Foo::__construct - | - | Elimination path: - | not found, all usages originate in unused code - | - | Found 1 usage: - | • data/debug/ctor.php:9 - - - DebugExclude\Foo::mixedExcluder - | - | Elimination path: - | not found, all usages originate in unused code - | - | Found 1 usage: - | • data/debug/exclude.php:10 - excluded by mixed excluder - - - DebugNever\Foo::__get - | - | Is never reported as dead: unsupported magic method - - - DebugVirtual\FooTest::testFoo - | - | Elimination path: - | direct usage - | - | Found 1 usage: - | • virtual usage from ShipMonk\PHPStan\DeadCode\Provider\PhpUnitUsageProvider (test method) - - - DebugGlobal\Foo::chain - | - | Elimination path: - | direct usage - | - | Found 1 usage: - | • data/debug/global.php:12 - - - DebugMixed\Foo::any - | - | Elimination path: - | direct usage - | - | Found 1 usage: - | • data/debug/mixed.php:13 - - - DebugCycle\Foo::__construct - | - | Elimination path: - | not found, all usages originate in unused code - | - | Found 1 usage: - | • data/debug/cycle.php:17 - - - DebugRegular\Another::call - | - | Elimination path: - | entrypoint DebugRegular\FooController::dummyAction:12 - | calls DebugRegular\Another::call - | - | Found 2 usages: - | • data/debug/regular.php:12 - | • data/debug/regular.php:13 - - - DebugZero\Foo::__construct - | - | No usages found - - - OUTPUT; - - self::assertSame($expectedOutput, $this->trimFgColors($actualOutput)); + $expectedOutput = file_get_contents(__DIR__ . '/data/debug/expected_output.txt'); + self::assertNotFalse($expectedOutput); + self::assertSame($expectedOutput . "\n", $this->trimFgColors($actualOutput)); } /** diff --git a/tests/Rule/data/debug/expected_output.txt b/tests/Rule/data/debug/expected_output.txt new file mode 100644 index 00000000..501c2fd8 --- /dev/null +++ b/tests/Rule/data/debug/expected_output.txt @@ -0,0 +1,114 @@ +Found 1 usage over unknown type: + • any method, for example in data/debug/mixed.php:13 + +Thus, any member named the same is considered used, no matter its declaring class! + + +Usage debugging information: + +DateTime::format +| +| Not defined within analysed files! +| +| Found 1 usage: +| • data/debug/foreign.php:5 + + +DebugAlternative\Foo::foo +| +| Marked as alive at: +| entry data/debug/alternative.php:13 +| calls DebugAlternative\Foo::foo +| +| Found 1 usage: +| • data/debug/alternative.php:13 + + +DebugCtor\Foo::__construct +| +| Dead because: +| all usages originate in unused code +| +| Found 1 usage: +| • data/debug/ctor.php:9 + + +DebugExclude\Foo::mixedExcluder +| +| Dead because: +| all usages originate in unused code +| +| Found 1 usage: +| • data/debug/exclude.php:10 - excluded by mixed excluder + + +DebugNever\Foo::__get +| +| Is never reported as dead: unsupported magic method + + +DebugVirtual\FooTest::testFoo +| +| Marked as alive at: +| entry virtual usage from ShipMonk\PHPStan\DeadCode\Provider\PhpUnitUsageProvider (test method) +| calls DebugVirtual\FooTest::testFoo +| +| Found 1 usage: +| • virtual usage from ShipMonk\PHPStan\DeadCode\Provider\PhpUnitUsageProvider (test method) + + +DebugGlobal\Foo::chain2 +| +| Marked as alive at: +| entry data/debug/global.php:20 +| calls DebugGlobal\Foo::chain1:11 +| calls DebugGlobal\Foo::chain2 +| +| Found 1 usage: +| • data/debug/global.php:11 + + +DebugMixed\Foo::any +| +| Marked as alive at: +| entry data/debug/mixed.php:13 +| calls DebugMixed\Foo::any +| +| Found 1 usage: +| • data/debug/mixed.php:13 + + +DebugCycle\Foo::__construct +| +| Dead because: +| all usages originate in unused code +| +| Found 1 usage: +| • data/debug/cycle.php:17 + + +DebugRegular\Another::call +| +| Marked as alive at: +| entry virtual usage from ShipMonk\PHPStan\DeadCode\Provider\SymfonyUsageProvider +| calls DebugRegular\FooController::dummyAction:12 +| calls DebugRegular\Another::call +| +| Found 2 usages: +| • data/debug/regular.php:12 +| • data/debug/regular.php:13 + + +DebugUnsupported\Foo::notDead +| +| Marked as alive at: +| entry DebugUnsupported\Foo::__destruct:8 +| calls DebugUnsupported\Foo::notDead +| +| Found 1 usage: +| • data/debug/unsupported.php:8 + + +DebugZero\Foo::__construct +| +| No usages found diff --git a/tests/Rule/data/debug/global.php b/tests/Rule/data/debug/global.php index 9bef4435..1e0e8ea8 100644 --- a/tests/Rule/data/debug/global.php +++ b/tests/Rule/data/debug/global.php @@ -6,7 +6,15 @@ class Foo { public function __construct() {} - public function chain() {} + public function chain1() + { + $this->chain2(); + } + + public function chain2() + { + + } } -(new Foo())->chain(); +(new Foo())->chain1(); diff --git a/tests/Rule/data/debug/unsupported.php b/tests/Rule/data/debug/unsupported.php new file mode 100644 index 00000000..b170a8f4 --- /dev/null +++ b/tests/Rule/data/debug/unsupported.php @@ -0,0 +1,14 @@ +notDead(); + } + + private function notDead() + { + } +} From 2112ceb70cc8d605aa4f885b6163c419a1c81e49 Mon Sep 17 00:00:00 2001 From: Jan Nedbal Date: Fri, 14 Mar 2025 14:07:39 +0100 Subject: [PATCH 13/15] Minor test improvement --- tests/Rule/DeadCodeRuleTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/Rule/DeadCodeRuleTest.php b/tests/Rule/DeadCodeRuleTest.php index 0d59d1bd..62dccc09 100644 --- a/tests/Rule/DeadCodeRuleTest.php +++ b/tests/Rule/DeadCodeRuleTest.php @@ -270,6 +270,7 @@ public static function provideDebugUsageInvalidArgs(): array { return [ 'method not owned' => ['DebugAlternative\Clazz::foo', "Member 'foo' does not exist directly in 'DebugAlternative\Clazz'"], + 'method not declared' => ['DebugAlternative\Clazz::__construct', "Member '__construct' does not exist directly in 'DebugAlternative\Clazz'"], 'no method' => ['DebugAlternative\Clazz::xyz', "Member 'xyz' does not exist directly in 'DebugAlternative\Clazz'"], 'no class' => ['InvalidClass::foo', "Class 'InvalidClass' does not exist"], 'invalid format' => ['InvalidFormat', "Invalid debug member format: 'InvalidFormat', expected 'ClassName::memberName'"], From 50551184aec5f80928f6a27c6142bbf2c607f6ce Mon Sep 17 00:00:00 2001 From: Jan Nedbal Date: Fri, 14 Mar 2025 14:09:51 +0100 Subject: [PATCH 14/15] Adjust readme with new output --- README.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 2c986837..69287ee4 100644 --- a/README.md +++ b/README.md @@ -366,10 +366,11 @@ Then, run PHPStan with `-vvv` CLI option and you will see the output like this: ```txt App\User\Entity\Address::__construct | -| Elimination path: -| entrypoint App\User\RegisterUserController::__invoke:36 -| calls App\User\UserFacade::registerUser:142 -| calls App\User\Entity\Address::__construct +| Marked as alive by: +| entry virtual usage from ShipMonk\PHPStan\DeadCode\Provider\SymfonyUsageProvider +| calls App\User\RegisterUserController::__invoke:36 +| calls App\User\UserFacade::registerUser:142 +| calls App\User\Entity\Address::__construct | | Found 2 usages: | • src/User/UserFacade.php:142 From 13ef045664c549e9a993b1a731c548f16a4628f5 Mon Sep 17 00:00:00 2001 From: Jan Nedbal Date: Fri, 14 Mar 2025 14:16:32 +0100 Subject: [PATCH 15/15] Simplify markTransitivesWhite params --- src/Rule/DeadCodeRule.php | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/Rule/DeadCodeRule.php b/src/Rule/DeadCodeRule.php index e940ec33..7b21597b 100644 --- a/src/Rule/DeadCodeRule.php +++ b/src/Rule/DeadCodeRule.php @@ -29,6 +29,7 @@ use ShipMonk\PHPStan\DeadCode\Graph\CollectedUsage; use ShipMonk\PHPStan\DeadCode\Hierarchy\ClassHierarchy; use function array_key_exists; +use function array_key_last; use function array_keys; use function array_map; use function array_merge; @@ -258,7 +259,7 @@ public function processNode( } foreach ($whiteMembers as $whiteCalleeKey => $usages) { - $this->markTransitivesWhite($whiteCalleeKey, [$whiteCalleeKey => $usages]); + $this->markTransitivesWhite([$whiteCalleeKey => $usages]); } foreach ($this->blackMembers as $blackMemberKey => $blackMember) { @@ -460,20 +461,21 @@ private function findDefinerMemberKey( } /** - * @param non-empty-array> $visited + * @param non-empty-array> $stack callerKey => usages[] */ - private function markTransitivesWhite(string $callerKey, array $visited): void + private function markTransitivesWhite(array $stack): void { + $callerKey = array_key_last($stack); $callees = $this->usageGraph[$callerKey] ?? []; if (isset($this->blackMembers[$callerKey])) { - $this->debugUsagePrinter->markMemberAsWhite($this->blackMembers[$callerKey], $visited); + $this->debugUsagePrinter->markMemberAsWhite($this->blackMembers[$callerKey], $stack); unset($this->blackMembers[$callerKey]); } foreach ($callees as $calleeKey => $usages) { - if (array_key_exists($calleeKey, $visited)) { + if (array_key_exists($calleeKey, $stack)) { continue; } @@ -481,7 +483,7 @@ private function markTransitivesWhite(string $callerKey, array $visited): void continue; } - $this->markTransitivesWhite($calleeKey, array_merge($visited, [$calleeKey => $usages])); + $this->markTransitivesWhite(array_merge($stack, [$calleeKey => $usages])); } }