From 97131724a9eae20ef452ad827ebf4a11c1b4f0e0 Mon Sep 17 00:00:00 2001 From: Jan Nedbal Date: Fri, 20 Dec 2024 16:06:28 +0100 Subject: [PATCH] ClassMemberRefs: encapsulate properties (#123) --- src/Collector/ProvidedUsagesCollector.php | 16 +++++------ src/Error/BlackMember.php | 7 +++-- src/Graph/ClassConstantRef.php | 2 +- src/Graph/ClassMemberRef.php | 35 +++++++++++++---------- src/Graph/ClassMemberUsage.php | 4 +-- src/Graph/ClassMethodRef.php | 2 +- src/Rule/DeadCodeRule.php | 24 ++++++++-------- 7 files changed, 48 insertions(+), 42 deletions(-) diff --git a/src/Collector/ProvidedUsagesCollector.php b/src/Collector/ProvidedUsagesCollector.php index 03558b8..95e7870 100644 --- a/src/Collector/ProvidedUsagesCollector.php +++ b/src/Collector/ProvidedUsagesCollector.php @@ -74,10 +74,10 @@ private function validateUsage( ): void { $memberRef = $usage->getMemberRef(); - $memberRefClass = $memberRef->className; + $memberRefClass = $memberRef->getClassName(); $originRef = $usage->getOrigin(); - $originRefClass = $originRef->className ?? null; + $originRefClass = $originRef === null ? null : $originRef->getClassName(); $context = sprintf( "It was emitted as %s by %s for node '%s' in '%s' on line %s", @@ -93,12 +93,12 @@ private function validateUsage( throw new LogicException("Class '$memberRefClass' does not exist. $context"); } - if ($memberRef instanceof ClassMethodRef && !$this->reflectionProvider->getClass($memberRefClass)->hasMethod($memberRef->memberName)) { - throw new LogicException("Method '{$memberRef->memberName}' does not exist in class '$memberRefClass'. $context"); + if ($memberRef instanceof ClassMethodRef && !$this->reflectionProvider->getClass($memberRefClass)->hasMethod($memberRef->getMemberName())) { + throw new LogicException("Method '{$memberRef->getMemberName()}' does not exist in class '$memberRefClass'. $context"); } - if ($memberRef instanceof ClassConstantRef && !$this->reflectionProvider->getClass($memberRefClass)->hasConstant($memberRef->memberName)) { - throw new LogicException("Constant '{$memberRef->memberName}' does not exist in class '$memberRefClass'. $context"); + if ($memberRef instanceof ClassConstantRef && !$this->reflectionProvider->getClass($memberRefClass)->hasConstant($memberRef->getMemberName())) { + throw new LogicException("Constant '{$memberRef->getMemberName()}' does not exist in class '$memberRefClass'. $context"); } } @@ -110,8 +110,8 @@ private function validateUsage( throw new LogicException("Class '{$originRefClass}' does not exist. $context"); } - if (!$this->reflectionProvider->getClass($originRefClass)->hasMethod($originRef->memberName)) { - throw new LogicException("Method '{$originRef->memberName}' does not exist in class '$originRefClass'. $context"); + if (!$this->reflectionProvider->getClass($originRefClass)->hasMethod($originRef->getMemberName())) { + throw new LogicException("Method '{$originRef->getMemberName()}' does not exist in class '$originRefClass'. $context"); } } } diff --git a/src/Error/BlackMember.php b/src/Error/BlackMember.php index 0e5fa21..d6e82f0 100644 --- a/src/Error/BlackMember.php +++ b/src/Error/BlackMember.php @@ -3,6 +3,7 @@ namespace ShipMonk\PHPStan\DeadCode\Error; use LogicException; +use ShipMonk\PHPStan\DeadCode\Graph\ClassConstantRef; use ShipMonk\PHPStan\DeadCode\Graph\ClassMemberRef; use ShipMonk\PHPStan\DeadCode\Rule\DeadCodeRule; @@ -24,11 +25,11 @@ public function __construct( int $line ) { - if ($member->className === null) { + if ($member->getClassName() === null) { throw new LogicException('Class name must be known'); } - if ($member->possibleDescendant) { + if ($member->isPossibleDescendant()) { throw new LogicException('Using possible descendant does not make sense here'); } @@ -39,7 +40,7 @@ public function __construct( public function getErrorIdentifier(): string { - return $this->member->memberType === ClassMemberRef::TYPE_CONSTANT + return $this->member instanceof ClassConstantRef ? DeadCodeRule::IDENTIFIER_CONSTANT : DeadCodeRule::IDENTIFIER_METHOD; } diff --git a/src/Graph/ClassConstantRef.php b/src/Graph/ClassConstantRef.php index 20e89cf..007c9df 100644 --- a/src/Graph/ClassConstantRef.php +++ b/src/Graph/ClassConstantRef.php @@ -14,7 +14,7 @@ public function __construct( bool $possibleDescendant ) { - parent::__construct($className, $constantName, $possibleDescendant, ClassMemberRef::TYPE_CONSTANT); + parent::__construct($className, $constantName, $possibleDescendant); } public static function buildKey(string $typeName, string $memberName): string diff --git a/src/Graph/ClassMemberRef.php b/src/Graph/ClassMemberRef.php index b2d87fe..aa86ec4 100644 --- a/src/Graph/ClassMemberRef.php +++ b/src/Graph/ClassMemberRef.php @@ -11,33 +11,38 @@ abstract class ClassMemberRef public const TYPE_METHOD = 1; public const TYPE_CONSTANT = 2; - public const UNKNOWN_CLASS = '*'; + private const UNKNOWN_CLASS = '*'; - public ?string $className; + private ?string $className; - public string $memberName; + private string $memberName; - public bool $possibleDescendant; + private bool $possibleDescendant; - /** - * @var self::TYPE_* - */ - public int $memberType; - - /** - * @param self::TYPE_* $memberType - */ public function __construct( ?string $className, string $memberName, - bool $possibleDescendant, - int $memberType + bool $possibleDescendant ) { $this->className = $className; $this->memberName = $memberName; $this->possibleDescendant = $possibleDescendant; - $this->memberType = $memberType; + } + + public function getClassName(): ?string + { + return $this->className; + } + + public function getMemberName(): string + { + return $this->memberName; + } + + public function isPossibleDescendant(): bool + { + return $this->possibleDescendant; } public function toHumanString(): string diff --git a/src/Graph/ClassMemberUsage.php b/src/Graph/ClassMemberUsage.php index 4b47c64..fe32e85 100644 --- a/src/Graph/ClassMemberUsage.php +++ b/src/Graph/ClassMemberUsage.php @@ -19,11 +19,11 @@ abstract class ClassMemberUsage public function __construct(?ClassMethodRef $origin) { - if ($origin !== null && $origin->possibleDescendant) { + if ($origin !== null && $origin->isPossibleDescendant()) { throw new LogicException('Origin should always be exact place in codebase.'); } - if ($origin !== null && $origin->className === null) { + if ($origin !== null && $origin->getClassName() === null) { throw new LogicException('Origin should always be exact place in codebase, thus className should be known.'); } diff --git a/src/Graph/ClassMethodRef.php b/src/Graph/ClassMethodRef.php index 2abe0f9..3244f83 100644 --- a/src/Graph/ClassMethodRef.php +++ b/src/Graph/ClassMethodRef.php @@ -18,7 +18,7 @@ public function __construct( bool $possibleDescendant ) { - parent::__construct($className, $methodName, $possibleDescendant, ClassMemberRef::TYPE_METHOD); + parent::__construct($className, $methodName, $possibleDescendant); } public static function buildKey(string $typeName, string $memberName): string diff --git a/src/Rule/DeadCodeRule.php b/src/Rule/DeadCodeRule.php index dd17df2..f80d176 100644 --- a/src/Rule/DeadCodeRule.php +++ b/src/Rule/DeadCodeRule.php @@ -160,8 +160,8 @@ public function processNode( foreach ($useStrings as $useString) { $memberUse = ClassMemberUsage::deserialize($useString); - if ($memberUse->getMemberRef()->className === null) { - $this->mixedMemberUses[$memberUse->getMemberType()][$memberUse->getMemberRef()->memberName][] = $memberUse; + if ($memberUse->getMemberRef()->getClassName() === null) { + $this->mixedMemberUses[$memberUse->getMemberType()][$memberUse->getMemberRef()->getMemberName()][] = $memberUse; continue; } @@ -208,7 +208,7 @@ public function processNode( foreach ($this->mixedMemberUses[ClassMemberRef::TYPE_METHOD][$methodName] ?? [] as $originalCall) { $memberUses[] = new ClassMethodUsage( $originalCall->getOrigin(), - new ClassMethodRef($typeName, $methodName, $originalCall->getMemberRef()->possibleDescendant), + new ClassMethodRef($typeName, $methodName, $originalCall->getMemberRef()->isPossibleDescendant()), ); } } @@ -222,7 +222,7 @@ public function processNode( foreach ($this->mixedMemberUses[ClassMemberRef::TYPE_CONSTANT][$constantName] ?? [] as $originalFetch) { $memberUses[] = new ClassConstantUsage( $originalFetch->getOrigin(), - new ClassConstantRef($typeName, $constantName, $originalFetch->getMemberRef()->possibleDescendant), + new ClassConstantRef($typeName, $constantName, $originalFetch->getMemberRef()->isPossibleDescendant()), ); } } @@ -359,12 +359,12 @@ private function isAnonymousClass(?string $className): bool */ private function getAlternativeMemberKeys(ClassMemberRef $member): array { - if ($member->className === null) { + if ($member->getClassName() === null) { throw new LogicException('Those were eliminated above, should never happen'); } $memberKey = $member->toKey(); - $possibleDescendant = $member->possibleDescendant; + $possibleDescendant = $member->isPossibleDescendant(); $cacheKey = $memberKey . ';' . ($possibleDescendant ? '1' : '0'); if (isset($this->memberAlternativesCache[$cacheKey])) { @@ -374,8 +374,8 @@ private function getAlternativeMemberKeys(ClassMemberRef $member): array $result = [$memberKey]; if ($possibleDescendant) { - foreach ($this->classHierarchy->getClassDescendants($member->className) as $descendantName) { - $result[] = $member::buildKey($descendantName, $member->memberName); + foreach ($this->classHierarchy->getClassDescendants($member->getClassName()) as $descendantName) { + $result[] = $member::buildKey($descendantName, $member->getMemberName()); } } @@ -585,8 +585,8 @@ private function getTraitUsages(string $typeName): array private function isConsideredWhite(ClassMemberUsage $memberUsage): bool { return $memberUsage->getOrigin() === null - || $this->isAnonymousClass($memberUsage->getOrigin()->className) - || (array_key_exists($memberUsage->getOrigin()->memberName, self::UNSUPPORTED_MAGIC_METHODS)); + || $this->isAnonymousClass($memberUsage->getOrigin()->getClassName()) + || (array_key_exists($memberUsage->getOrigin()->getMemberName(), self::UNSUPPORTED_MAGIC_METHODS)); } private function isNeverReportedAsDead(BlackMember $blackMember): bool @@ -595,8 +595,8 @@ private function isNeverReportedAsDead(BlackMember $blackMember): bool return false; } - $typeName = $blackMember->member->className; - $memberName = $blackMember->member->memberName; + $typeName = $blackMember->member->getClassName(); + $memberName = $blackMember->member->getMemberName(); if ($typeName === null) { throw new LogicException('Ensured by BlackMember constructor');