Skip to content

Commit

Permalink
Fix detection of excluded mixed usages
Browse files Browse the repository at this point in the history
  • Loading branch information
janedbal committed Jan 24, 2025
1 parent 1662b60 commit a9de4d2
Show file tree
Hide file tree
Showing 6 changed files with 84 additions and 44 deletions.
17 changes: 17 additions & 0 deletions src/Graph/ClassConstantUsage.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace ShipMonk\PHPStan\DeadCode\Graph;

use LogicException;
use ShipMonk\PHPStan\DeadCode\Enum\MemberType;

/**
Expand Down Expand Up @@ -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(),
),
);
}

}
5 changes: 5 additions & 0 deletions src/Graph/ClassMemberUsage.php
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
17 changes: 17 additions & 0 deletions src/Graph/ClassMethodUsage.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace ShipMonk\PHPStan\DeadCode\Graph;

use LogicException;
use ShipMonk\PHPStan\DeadCode\Enum\MemberType;

/**
Expand Down Expand Up @@ -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(),
),
);
}

}
8 changes: 8 additions & 0 deletions src/Graph/CollectedUsage.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
79 changes: 36 additions & 43 deletions src/Rule/DeadCodeRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -102,11 +100,11 @@ class DeadCodeRule implements Rule, DiagnoseExtension
private array $blackMembers = [];

/**
* memberType => [memberName => ClassMemberUse[]]
* memberType => [memberName => CollectedUsage[]]
*
* @var array<MemberType::*, array<string, list<ClassMemberUsage>>>
* @var array<MemberType::*, array<string, list<CollectedUsage>>>
*/
private array $mixedMemberUses = [];
private array $mixedMemberUsages = [];

/**
* @var array<string, list<string>> callerKey => memberUseKey[]
Expand Down Expand Up @@ -145,11 +143,8 @@ public function processNode(
return [];
}

/** @var list<ClassMemberUsage> $memberUsages */
$memberUsages = [];

/** @var list<CollectedUsage> $excludedMemberUsages */
$excludedMemberUsages = [];
/** @var list<CollectedUsage> $knownCollectedUsages */
$knownCollectedUsages = [];

$methodDeclarationData = $node->get(ClassDefinitionCollector::class);
$methodCallData = $node->get(MethodCallCollector::class);
Expand All @@ -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;
}
}
}
Expand Down Expand Up @@ -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);
}
}

Expand All @@ -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<string> $whiteMemberKeys */
$whiteMemberKeys = [];
/** @var list<CollectedUsage> $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) {
Expand Down Expand Up @@ -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('<fg=red>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(' • <fg=white>%s</> %s', $memberName, $memberTypeString));

$exampleCaller = $this->getExampleCaller($uses);
$exampleCaller = $this->getExampleCaller($usages);

if ($exampleCaller !== null) {
$output->writeFormatted(sprintf(', for example in <fg=white>%s</>', $exampleCaller));
Expand All @@ -687,13 +680,13 @@ public function print(Output $output): void
}

/**
* @param list<ClassMemberUsage> $uses
* @param list<CollectedUsage> $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();
}
}

Expand Down
2 changes: 1 addition & 1 deletion tests/Rule/data/excluders/mixed/code.php
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<?php

class Some {
public function mixed() { // error: Unused Some::mixed
public function mixed() { // error: Unused Some::mixed (all usages excluded by mixed excluder)
return 1;
}
}
Expand Down

0 comments on commit a9de4d2

Please sign in to comment.