Skip to content

Commit

Permalink
ClassMemberRefs: encapsulate properties (#123)
Browse files Browse the repository at this point in the history
  • Loading branch information
janedbal authored Dec 20, 2024
1 parent 593829a commit 9713172
Show file tree
Hide file tree
Showing 7 changed files with 48 additions and 42 deletions.
16 changes: 8 additions & 8 deletions src/Collector/ProvidedUsagesCollector.php
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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");
}
}

Expand All @@ -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");
}
}
}
Expand Down
7 changes: 4 additions & 3 deletions src/Error/BlackMember.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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');
}

Expand All @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion src/Graph/ClassConstantRef.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
35 changes: 20 additions & 15 deletions src/Graph/ClassMemberRef.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/Graph/ClassMemberUsage.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.');
}

Expand Down
2 changes: 1 addition & 1 deletion src/Graph/ClassMethodRef.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
24 changes: 12 additions & 12 deletions src/Rule/DeadCodeRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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()),
);
}
}
Expand All @@ -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()),
);
}
}
Expand Down Expand Up @@ -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])) {
Expand All @@ -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());
}
}

Expand Down Expand Up @@ -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
Expand All @@ -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');
Expand Down

0 comments on commit 9713172

Please sign in to comment.