Skip to content

Commit

Permalink
Implement const-fetch over mixed in unified way
Browse files Browse the repository at this point in the history
  • Loading branch information
janedbal committed Nov 14, 2024
1 parent 52737ed commit 01b4273
Show file tree
Hide file tree
Showing 7 changed files with 188 additions and 67 deletions.
44 changes: 35 additions & 9 deletions src/Collector/ConstantFetchCollector.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
use PHPStan\Reflection\MethodReflection;
use PHPStan\Reflection\ReflectionProvider;
use PHPStan\Type\Constant\ConstantStringType;
use PHPStan\Type\Type;
use PHPStan\Type\TypeCombinator;
use PHPStan\Type\TypeUtils;
use ShipMonk\PHPStan\DeadCode\Crate\ClassConstantFetch;
use ShipMonk\PHPStan\DeadCode\Crate\ClassConstantRef;
use ShipMonk\PHPStan\DeadCode\Crate\ClassMethodRef;
Expand Down Expand Up @@ -140,15 +143,8 @@ private function registerFetch(ClassConstFetch $node, Scope $scope): void
? array_map(static fn (ConstantStringType $string): string => $string->getValue(), $scope->getType($node->name)->getConstantStrings())
: [$node->name->toString()];

foreach ($ownerType->getObjectClassReflections() as $classReflection) {
foreach ($constantNames as $constantName) {
if ($classReflection->hasConstant($constantName)) {
$className = $classReflection->getConstant($constantName)->getDeclaringClass()->getName();

} else { // call of unknown const (might be present on children)
$className = $classReflection->getName(); // TODO untested yet
}

foreach ($constantNames as $constantName) {
foreach ($this->getDeclaringTypesWithConstant($ownerType, $constantName) as $className) {
$this->accessBuffer[] = new ClassConstantFetch(
$this->getCaller($scope),
new ClassConstantRef($className, $constantName),
Expand All @@ -158,6 +154,36 @@ private function registerFetch(ClassConstFetch $node, Scope $scope): void
}
}

/**
* @return list<class-string<object>|null>
*/
private function getDeclaringTypesWithConstant(
Type $type,
string $constantName
): array
{
$typeNoNull = TypeCombinator::removeNull($type); // TODO needed ?
$typeNormalized = TypeUtils::toBenevolentUnion($typeNoNull); // extract possible calls even from Class|int
$classReflections = $typeNormalized->getObjectTypeOrClassStringObjectType()->getObjectClassReflections();

$result = [];

foreach ($classReflections as $classReflection) {
if ($classReflection->hasConstant($constantName)) {
$result[] = $classReflection->getConstant($constantName)->getDeclaringClass()->getName();

} else { // unknown constant fetch (might be present on children)
$result[] = $classReflection->getName(); // TODO untested yet ?
}
}

if ($result === []) { // TODO trackFetchesOnMixed
$result[] = null; // call over unknown type
}

return $result;
}

private function getCaller(Scope $scope): ?ClassMethodRef
{
if (!$scope->isInClass()) {
Expand Down
8 changes: 8 additions & 0 deletions src/Crate/ClassConstantFetch.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,14 @@ public function __construct(
$this->fetch = $fetch;
}

/**
* @return ClassMemberRef::TYPE_CONSTANT
*/
public function getMemberType(): int
{
return ClassMemberRef::TYPE_CONSTANT;
}

public function getMemberUse(): ClassConstantRef
{
return $this->fetch;
Expand Down
7 changes: 6 additions & 1 deletion src/Crate/ClassMemberUse.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
/**
* @immutable
*/
abstract class ClassMemberUse
abstract class ClassMemberUse // TODO rename to ClassMemberUsage ?
{

/**
Expand Down Expand Up @@ -41,6 +41,11 @@ public function getCaller(): ?ClassMethodRef
return $this->caller;
}

/**
* @return ClassMemberRef::TYPE_*
*/
abstract public function getMemberType(): int;

abstract public function getMemberUse(): ClassMemberRef;

public function serialize(): string
Expand Down
7 changes: 5 additions & 2 deletions src/Crate/ClassMethodCall.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,12 @@ public function __construct(
$this->callee = $callee;
}

public function getCallee(): ClassMethodRef
/**
* @return ClassMemberRef::TYPE_METHOD
*/
public function getMemberType(): int
{
return $this->callee;
return ClassMemberRef::TYPE_METHOD;
}

public function getMemberUse(): ClassMethodRef
Expand Down
109 changes: 60 additions & 49 deletions src/Rule/DeadMethodRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@
use ShipMonk\PHPStan\DeadCode\Hierarchy\ClassHierarchy;
use function array_key_exists;
use function array_keys;
use function array_map;
use function array_merge;
use function array_merge_recursive;
use function array_slice;
use function count;
use function array_sum;
use function in_array;
use function sprintf;
use function strpos;
Expand Down Expand Up @@ -90,14 +90,18 @@ class DeadMethodRule implements Rule, DiagnoseExtension // TODO rename to DeadCo
private bool $trackCallsOnMixed;

/**
* @var array<string, array{string, int, string, string, ClassMemberRef::TYPE_*}> memberKey => [file, line, typeName, memberName, memberType]
* memberKey => [file, line, typeName, memberName, memberType]
*
* @var array<string, array{string, int, string, string, ClassMemberRef::TYPE_*}>
*/
private array $blackMembers = [];

/**
* @var array<string, list<ClassMethodCall>> methodName => ClassMethodCall[]
* memberType => [memberName => ClassMemberUse[]]
*
* @var array<ClassMemberRef::TYPE_*, array<string, list<ClassMemberUse>>>
*/
private array $mixedCalls = [];
private array $mixedMemberUses = [];

/**
* @var array<string, list<string>> callerKey => memberUseKey[]
Expand Down Expand Up @@ -141,21 +145,21 @@ public function processNode(
$constFetchData = $node->get(ConstantFetchCollector::class);
$entrypointData = $node->get(EntrypointCollector::class);

/** @var array<string, list<list<string>>> $callData */
$callData = array_merge_recursive($methodCallData, $entrypointData);
unset($methodCallData, $entrypointData);
/** @var array<string, list<list<string>>> $memberUseData */
$memberUseData = array_merge_recursive($methodCallData, $entrypointData, $constFetchData);
unset($methodCallData, $entrypointData, $constFetchData);

foreach ($callData as $callsPerFile) {
foreach ($callsPerFile as $callStrings) {
foreach ($callStrings as $callString) {
$call = ClassMethodCall::deserialize($callString);
foreach ($memberUseData as $usesPerFile) {
foreach ($usesPerFile as $useStrings) {
foreach ($useStrings as $useString) {
$memberUse = ClassMemberUse::deserialize($useString);

if ($call->getCallee()->className === null) {
$this->mixedCalls[$call->getCallee()->memberName][] = $call;
if ($memberUse->getMemberUse()->className === null) {
$this->mixedMemberUses[$memberUse->getMemberType()][$memberUse->getMemberUse()->memberName][] = $memberUse;
continue;
}

$memberUses[] = $call;
$memberUses[] = $memberUse;
}
}
}
Expand Down Expand Up @@ -189,32 +193,29 @@ public function processNode(
$this->fillTraitConstantUsages($typeName, $this->getTraitUsages($typeName), $this->getTypeConstants($typeName));
$this->fillClassHierarchy($typeName, $ancestorNames);

foreach ($methods as $memberName => $methodData) {
$definition = ClassMethodRef::buildKey($typeName, $memberName);
$this->blackMembers[$definition] = [$file, $methodData['line'], $typeName, $memberName, ClassMemberRef::TYPE_METHOD];

if (isset($this->mixedCalls[$memberName])) {
foreach ($this->mixedCalls[$memberName] as $originalCall) {
$memberUses[] = new ClassMethodCall(
$originalCall->getCaller(),
new ClassMethodRef($typeName, $memberName),
$originalCall->isPossibleDescendantUse(),
);
}
foreach ($methods as $methodName => $methodData) {
$definition = ClassMethodRef::buildKey($typeName, $methodName);
$this->blackMembers[$definition] = [$file, $methodData['line'], $typeName, $methodName, ClassMemberRef::TYPE_METHOD];

foreach ($this->mixedMemberUses[ClassMemberRef::TYPE_METHOD][$methodName] ?? [] as $originalCall) {
$memberUses[] = new ClassMethodCall(
$originalCall->getCaller(),
new ClassMethodRef($typeName, $methodName),
$originalCall->isPossibleDescendantUse(),
);
}
}

foreach ($constants as $constantName => $constantData) {
$definition = ClassConstantRef::buildKey($typeName, $constantName);
$this->blackMembers[$definition] = [$file, $constantData['line'], $typeName, $constantName, ClassMemberRef::TYPE_CONSTANT];
}
}

foreach ($constFetchData as $file => $data) {
foreach ($data as $constantData) {
foreach ($constantData as $constantKey) {
$fetch = ClassConstantFetch::deserialize($constantKey);
$memberUses[] = $fetch;
foreach ($this->mixedMemberUses[ClassMemberRef::TYPE_CONSTANT][$constantName] ?? [] as $originalFetch) {
$memberUses[] = new ClassConstantFetch(
$originalFetch->getCaller(),
new ClassConstantRef($typeName, $constantName),
$originalFetch->isPossibleDescendantUse(),
);
}
}
}
Expand Down Expand Up @@ -629,40 +630,50 @@ private function isNeverReportedAsDead(string $typeName, string $memberName, int

public function print(Output $output): void
{
if ($this->mixedCalls === [] || !$output->isDebug() || !$this->trackCallsOnMixed) {
if ($this->mixedMemberUses === [] || !$output->isDebug() || !$this->trackCallsOnMixed) {
return;
}

$totalCount = array_sum(array_map('count', $this->mixedMemberUses));
$maxExamplesToShow = 20;
$output->writeLineFormatted(sprintf('<fg=red>Found %d methods called over unknown type</>:', count($this->mixedCalls)));
$examplesShown = 0;
$output->writeLineFormatted(sprintf('<fg=red>Found %d usages over unknown type</>:', $totalCount));

foreach (array_slice($this->mixedCalls, 0, $maxExamplesToShow) as $methodName => $calls) {
$output->writeFormatted(sprintf(' • <fg=white>%s</>', $methodName));
foreach ($this->mixedMemberUses as $memberType => $memberUses) {
foreach ($memberUses as $memberName => $uses) {
$examplesShown++;
$memberTypeString = $memberType === ClassMemberRef::TYPE_METHOD ? 'method' : 'constant';
$output->writeFormatted(sprintf(' • <fg=white>%s</> %s', $memberName, $memberTypeString));

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

if ($exampleCaller !== null) {
$output->writeFormatted(sprintf(', for example in <fg=white>%s</>', $exampleCaller));
}
if ($exampleCaller !== null) {
$output->writeFormatted(sprintf(', for example in <fg=white>%s</>', $exampleCaller));
}

$output->writeLineFormatted('');

$output->writeLineFormatted('');
if ($examplesShown >= $maxExamplesToShow) {
break 2;
}
}
}

if (count($this->mixedCalls) > $maxExamplesToShow) {
$output->writeLineFormatted(sprintf('... and %d more', count($this->mixedCalls) - $maxExamplesToShow));
if ($totalCount > $maxExamplesToShow) {
$output->writeLineFormatted(sprintf('... and %d more', $totalCount - $maxExamplesToShow));
}

$output->writeLineFormatted('');
$output->writeLineFormatted('Thus, any method named the same is considered used, no matter its declaring class!');
$output->writeLineFormatted('Thus, any member named the same is considered used, no matter its declaring class!');
$output->writeLineFormatted('');
}

/**
* @param list<ClassMethodCall> $calls
* @param list<ClassMemberUse> $uses
*/
private function getExampleCaller(array $calls): ?string
private function getExampleCaller(array $uses): ?string
{
foreach ($calls as $call) {
foreach ($uses as $call) {
if ($call->getCaller() !== null) {
return $call->getCaller()->toHumanString();
}
Expand Down
13 changes: 7 additions & 6 deletions tests/Rule/DeadMethodRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -147,13 +147,13 @@ static function (string $message) use (&$actualOutput): void {

$ec = ''; // hack editorconfig checker to ignore wrong indentation
$expectedOutput = <<<"OUTPUT"
<fg=red>Found 4 methods called over unknown type</>:
$ec • <fg=white>getter1</>, for example in <fg=white>DeadMixed1\Tester::__construct</>
$ec • <fg=white>getter2</>, for example in <fg=white>DeadMixed1\Tester::__construct</>
$ec • <fg=white>getter3</>, for example in <fg=white>DeadMixed1\Tester::__construct</>
$ec • <fg=white>staticMethod</>, for example in <fg=white>DeadMixed1\Tester::__construct</>
<fg=red>Found 4 usages over unknown type</>:
$ec • <fg=white>getter1</> method, for example in <fg=white>DeadMixed1\Tester::__construct</>
$ec • <fg=white>getter2</> method, for example in <fg=white>DeadMixed1\Tester::__construct</>
$ec • <fg=white>getter3</> method, for example in <fg=white>DeadMixed1\Tester::__construct</>
$ec • <fg=white>staticMethod</> method, for example in <fg=white>DeadMixed1\Tester::__construct</>
Thus, any method named the same is considered used, no matter its declaring class!
Thus, any member named the same is considered used, no matter its declaring class!
OUTPUT;
Expand Down Expand Up @@ -311,6 +311,7 @@ public static function provideFiles(): iterable
yield 'const-function' => [__DIR__ . '/data/DeadMethodRule/constants/constant-function.php'];
yield 'const-dynamic' => [__DIR__ . '/data/DeadMethodRule/constants/dynamic.php'];
yield 'const-expr' => [__DIR__ . '/data/DeadMethodRule/constants/expr.php'];
yield 'const-mixed' => [__DIR__ . '/data/DeadMethodRule/constants/mixed.php'];
yield 'const-override' => [__DIR__ . '/data/DeadMethodRule/constants/override.php'];
yield 'const-static' => [__DIR__ . '/data/DeadMethodRule/constants/static.php'];
yield 'const-traits-1' => [__DIR__ . '/data/DeadMethodRule/constants/traits-1.php'];
Expand Down
67 changes: 67 additions & 0 deletions tests/Rule/data/DeadMethodRule/constants/mixed.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
<?php declare(strict_types = 1);

namespace DeadConstMixed;


class Clazz {

const CONST1 = 1;
const CONST2 = 1;
const CONST3 = 1;
const CONST4 = 1; // error: Unused DeadConstMixed\Clazz::CONST4
const CONST5 = 1;
const CONST6 = 1; // error: Unused DeadConstMixed\Clazz::CONST6

const SOME_CONST = 1; // error: Unused DeadConstMixed\Clazz::SOME_CONST

}

interface IFace {

const CONST1 = 1;
const CONST2 = 1;
const CONST3 = 1;
const CONST4 = 1;
const CONST5 = 1; // error: Unused DeadConstMixed\IFace::CONST5

}

class Implementor implements IFace {

const CONST1 = 1;
const CONST2 = 1;
const CONST3 = 1;
const CONST4 = 1;
const CONST5 = 1; // error: Unused DeadConstMixed\Implementor::CONST5
const CONST6 = 1;

}

class Tester
{
function __construct($mixed, string $notClass, object $object, IFace $iface, int|Clazz $maybeClass)
{
echo $mixed::CONST1; // may be valid

if (!$mixed instanceof Clazz) {
echo $mixed::CONST2; // ideally, should mark only IFace, not Clazz (not implemented)
}

echo $object::CONST3;
echo $iface::CONST4;
echo $maybeClass::CONST5; // mark only Clazz, not IFace

$this->testMethodExists();
}

function testMethodExists(Iface $iface)
{
if (method_exists($iface, 'someMethod')) {
echo $iface::SOME_CONST; // does not not mark Clazz
}

echo $iface::CONST6; // not defined on Iface, but should mark used on its implementations but not on unrelated Clazz
}
}

new Tester();

0 comments on commit 01b4273

Please sign in to comment.