From a9de4d2e2bb653d75c62e3daa0d842fe145cd84b Mon Sep 17 00:00:00 2001 From: Jan Nedbal Date: Fri, 24 Jan 2025 15:27:23 +0100 Subject: [PATCH] Fix detection of excluded mixed usages --- src/Graph/ClassConstantUsage.php | 17 +++++ src/Graph/ClassMemberUsage.php | 5 ++ src/Graph/ClassMethodUsage.php | 17 +++++ src/Graph/CollectedUsage.php | 8 +++ src/Rule/DeadCodeRule.php | 79 +++++++++++------------- tests/Rule/data/excluders/mixed/code.php | 2 +- 6 files changed, 84 insertions(+), 44 deletions(-) diff --git a/src/Graph/ClassConstantUsage.php b/src/Graph/ClassConstantUsage.php index 69cd56a..79db9bb 100644 --- a/src/Graph/ClassConstantUsage.php +++ b/src/Graph/ClassConstantUsage.php @@ -2,6 +2,7 @@ namespace ShipMonk\PHPStan\DeadCode\Graph; +use LogicException; use ShipMonk\PHPStan\DeadCode\Enum\MemberType; /** @@ -38,4 +39,20 @@ public function getMemberRef(): ClassConstantRef return $this->fetch; } + public function concretizeMixedUsage(string $className): self + { + if ($this->fetch->getClassName() !== null) { + throw new LogicException('Usage is not mixed, thus it cannot be concretized'); + } + + return new self( + $this->getOrigin(), + new ClassConstantRef( + $className, + $this->fetch->getMemberName(), + $this->fetch->isPossibleDescendant(), + ), + ); + } + } diff --git a/src/Graph/ClassMemberUsage.php b/src/Graph/ClassMemberUsage.php index 7fd9e1c..ac34b0b 100644 --- a/src/Graph/ClassMemberUsage.php +++ b/src/Graph/ClassMemberUsage.php @@ -44,6 +44,11 @@ abstract public function getMemberType(): int; abstract public function getMemberRef(): ClassMemberRef; + /** + * @return static + */ + abstract public function concretizeMixedUsage(string $className): self; + public function toHumanString(): string { $origin = $this->origin !== null ? $this->origin->toHumanString() : 'unknown'; diff --git a/src/Graph/ClassMethodUsage.php b/src/Graph/ClassMethodUsage.php index 7bee2a3..6630bcb 100644 --- a/src/Graph/ClassMethodUsage.php +++ b/src/Graph/ClassMethodUsage.php @@ -2,6 +2,7 @@ namespace ShipMonk\PHPStan\DeadCode\Graph; +use LogicException; use ShipMonk\PHPStan\DeadCode\Enum\MemberType; /** @@ -39,4 +40,20 @@ public function getMemberRef(): ClassMethodRef return $this->callee; } + public function concretizeMixedUsage(string $className): self + { + if ($this->callee->getClassName() !== null) { + throw new LogicException('Usage is not mixed, thus it cannot be concretized'); + } + + return new self( + $this->getOrigin(), + new ClassMethodRef( + $className, + $this->callee->getMemberName(), + $this->callee->isPossibleDescendant(), + ), + ); + } + } diff --git a/src/Graph/CollectedUsage.php b/src/Graph/CollectedUsage.php index b372c81..9a7cfa4 100644 --- a/src/Graph/CollectedUsage.php +++ b/src/Graph/CollectedUsage.php @@ -44,6 +44,14 @@ public function getExcludedBy(): string return $this->excludedBy; } + public function concretizeMixedUsage(string $className): self + { + return new self( + $this->usage->concretizeMixedUsage($className), + $this->excludedBy, + ); + } + public function serialize(): string { $origin = $this->usage->getOrigin(); diff --git a/src/Rule/DeadCodeRule.php b/src/Rule/DeadCodeRule.php index 95ae86d..d92f4b5 100644 --- a/src/Rule/DeadCodeRule.php +++ b/src/Rule/DeadCodeRule.php @@ -21,11 +21,9 @@ use ShipMonk\PHPStan\DeadCode\Enum\Visibility; use ShipMonk\PHPStan\DeadCode\Error\BlackMember; use ShipMonk\PHPStan\DeadCode\Graph\ClassConstantRef; -use ShipMonk\PHPStan\DeadCode\Graph\ClassConstantUsage; use ShipMonk\PHPStan\DeadCode\Graph\ClassMemberRef; use ShipMonk\PHPStan\DeadCode\Graph\ClassMemberUsage; use ShipMonk\PHPStan\DeadCode\Graph\ClassMethodRef; -use ShipMonk\PHPStan\DeadCode\Graph\ClassMethodUsage; use ShipMonk\PHPStan\DeadCode\Graph\CollectedUsage; use ShipMonk\PHPStan\DeadCode\Hierarchy\ClassHierarchy; use function array_key_exists; @@ -102,11 +100,11 @@ class DeadCodeRule implements Rule, DiagnoseExtension private array $blackMembers = []; /** - * memberType => [memberName => ClassMemberUse[]] + * memberType => [memberName => CollectedUsage[]] * - * @var array>> + * @var array>> */ - private array $mixedMemberUses = []; + private array $mixedMemberUsages = []; /** * @var array> callerKey => memberUseKey[] @@ -145,11 +143,8 @@ public function processNode( return []; } - /** @var list $memberUsages */ - $memberUsages = []; - - /** @var list $excludedMemberUsages */ - $excludedMemberUsages = []; + /** @var list $knownCollectedUsages */ + $knownCollectedUsages = []; $methodDeclarationData = $node->get(ClassDefinitionCollector::class); $methodCallData = $node->get(MethodCallCollector::class); @@ -164,19 +159,14 @@ public function processNode( foreach ($usesPerFile as $useStrings) { foreach ($useStrings as $useString) { $collectedUsage = CollectedUsage::deserialize($useString); - $memberUse = $collectedUsage->getUsage(); - - if ($collectedUsage->isExcluded()) { - $excludedMemberUsages[] = $collectedUsage; - continue; - } + $memberUsage = $collectedUsage->getUsage(); - if ($memberUse->getMemberRef()->getClassName() === null) { - $this->mixedMemberUses[$memberUse->getMemberType()][$memberUse->getMemberRef()->getMemberName()][] = $memberUse; + if ($memberUsage->getMemberRef()->getClassName() === null) { + $this->mixedMemberUsages[$memberUsage->getMemberType()][$memberUsage->getMemberRef()->getMemberName()][] = $collectedUsage; continue; } - $memberUsages[] = $memberUse; + $knownCollectedUsages[] = $collectedUsage; } } } @@ -216,11 +206,8 @@ public function processNode( $this->blackMembers[$methodKey] = new BlackMember($methodRef, $file, $methodData['line']); - foreach ($this->mixedMemberUses[MemberType::METHOD][$methodName] ?? [] as $originalCall) { - $memberUsages[] = new ClassMethodUsage( - $originalCall->getOrigin(), - new ClassMethodRef($typeName, $methodName, $originalCall->getMemberRef()->isPossibleDescendant()), - ); + foreach ($this->mixedMemberUsages[MemberType::METHOD][$methodName] ?? [] as $mixedUsage) { + $knownCollectedUsages[] = $mixedUsage->concretizeMixedUsage($typeName); } } @@ -230,22 +217,28 @@ public function processNode( $this->blackMembers[$constantKey] = new BlackMember($constantRef, $file, $constantData['line']); - foreach ($this->mixedMemberUses[MemberType::CONSTANT][$constantName] ?? [] as $originalFetch) { - $memberUsages[] = new ClassConstantUsage( - $originalFetch->getOrigin(), - new ClassConstantRef($typeName, $constantName, $originalFetch->getMemberRef()->isPossibleDescendant()), - ); + foreach ($this->mixedMemberUsages[MemberType::CONSTANT][$constantName] ?? [] as $mixedUsage) { + $knownCollectedUsages[] = $mixedUsage->concretizeMixedUsage($typeName); } } } + /** @var list $whiteMemberKeys */ $whiteMemberKeys = []; + /** @var list $excludedMemberUsages */ + $excludedMemberUsages = []; + + foreach ($knownCollectedUsages as $collectedUsage) { + if ($collectedUsage->isExcluded()) { + $excludedMemberUsages[] = $collectedUsage; + continue; + } - foreach ($memberUsages as $memberUse) { - $isWhite = $this->isConsideredWhite($memberUse); + $memberUsage = $collectedUsage->getUsage(); + $isWhite = $this->isConsideredWhite($memberUsage); - $alternativeMemberKeys = $this->getAlternativeMemberKeys($memberUse->getMemberRef()); - $alternativeOriginKeys = $memberUse->getOrigin() !== null ? $this->getAlternativeMemberKeys($memberUse->getOrigin()) : []; + $alternativeMemberKeys = $this->getAlternativeMemberKeys($memberUsage->getMemberRef()); + $alternativeOriginKeys = $memberUsage->getOrigin() !== null ? $this->getAlternativeMemberKeys($memberUsage->getOrigin()) : []; foreach ($alternativeMemberKeys as $alternativeMemberKey) { foreach ($alternativeOriginKeys as $alternativeOriginKey) { @@ -648,22 +641,22 @@ private function isNeverReportedAsDead(BlackMember $blackMember): bool public function print(Output $output): void { - if ($this->mixedMemberUses === [] || !$output->isDebug() || !$this->trackMixedAccess) { + if ($this->mixedMemberUsages === [] || !$output->isDebug() || !$this->trackMixedAccess) { return; } - $totalCount = array_sum(array_map('count', $this->mixedMemberUses)); + $totalCount = array_sum(array_map('count', $this->mixedMemberUsages)); $maxExamplesToShow = 20; $examplesShown = 0; $output->writeLineFormatted(sprintf('Found %d usages over unknown type:', $totalCount)); - foreach ($this->mixedMemberUses as $memberType => $memberUses) { - foreach ($memberUses as $memberName => $uses) { + 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($uses); + $exampleCaller = $this->getExampleCaller($usages); if ($exampleCaller !== null) { $output->writeFormatted(sprintf(', for example in %s', $exampleCaller)); @@ -687,13 +680,13 @@ public function print(Output $output): void } /** - * @param list $uses + * @param list $usages */ - private function getExampleCaller(array $uses): ?string + private function getExampleCaller(array $usages): ?string { - foreach ($uses as $call) { - if ($call->getOrigin() !== null) { - return $call->getOrigin()->toHumanString(); + foreach ($usages as $usage) { + if ($usage->getUsage()->getOrigin() !== null) { + return $usage->getUsage()->getOrigin()->toHumanString(); } } diff --git a/tests/Rule/data/excluders/mixed/code.php b/tests/Rule/data/excluders/mixed/code.php index f164ebd..90d6c3d 100644 --- a/tests/Rule/data/excluders/mixed/code.php +++ b/tests/Rule/data/excluders/mixed/code.php @@ -1,7 +1,7 @@