Skip to content

Proper hierarchy call detection #48

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 26 additions & 8 deletions src/Collector/MethodCallCollector.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@
use PHPStan\Reflection\ReflectionProvider;
use PHPStan\Type\Type;
use PHPStan\Type\TypeCombinator;
use ShipMonk\PHPStan\DeadCode\Helper\DeadCodeHelper;
use ShipMonk\PHPStan\DeadCode\Crate\Call;
use function array_map;

/**
* @implements Collector<Node, list<string>>
Expand All @@ -33,7 +34,7 @@ class MethodCallCollector implements Collector
private ReflectionProvider $reflectionProvider;

/**
* @var list<string>
* @var list<Call>
*/
private array $callsBuffer = [];

Expand Down Expand Up @@ -82,7 +83,14 @@ public function processNode(
if (!$scope->isInClass() || $node instanceof ClassMethodsNode) { // @phpstan-ignore-line ignore BC promise
$data = $this->callsBuffer;
$this->callsBuffer = [];
return $data === [] ? null : $data; // collect data once per class to save memory & resultCache size

// collect data once per class to save memory & resultCache size
return $data === []
? null
: array_map(
static fn (Call $call): string => $call->toString(),
$data,
);
}

return null;
Expand All @@ -101,15 +109,18 @@ private function registerMethodCall(
if ($methodCall instanceof New_) {
if ($methodCall->class instanceof Expr) {
$callerType = $scope->getType($methodCall->class);
$possibleDescendantCall = true;

} elseif ($methodCall->class instanceof Name) {
$callerType = $scope->resolveTypeByName($methodCall->class);
$possibleDescendantCall = $methodCall->class->toString() === 'static';

} else {
return;
}
} else {
$callerType = $scope->getType($methodCall->var);
$possibleDescendantCall = true;
}

if ($methodName === null) {
Expand All @@ -118,7 +129,7 @@ private function registerMethodCall(

foreach ($this->getReflectionsWithMethod($callerType, $methodName) as $classWithMethod) {
$className = $classWithMethod->getMethod($methodName, $scope)->getDeclaringClass()->getName();
$this->callsBuffer[] = DeadCodeHelper::composeMethodKey($className, $methodName);
$this->callsBuffer[] = new Call($className, $methodName, $possibleDescendantCall);
}
}

Expand All @@ -136,8 +147,11 @@ private function registerStaticCall(
if ($staticCall->class instanceof Expr) {
$callerType = $scope->getType($staticCall->class);
$classReflections = $this->getReflectionsWithMethod($callerType, $methodName);
$possibleDescendantCall = true;

} else {
$className = $scope->resolveName($staticCall->class);
$possibleDescendantCall = $staticCall->class->toString() === 'static';

if ($this->reflectionProvider->hasClass($className)) {
$classReflections = [
Expand All @@ -150,7 +164,7 @@ private function registerStaticCall(

foreach ($classReflections as $classWithMethod) {
$className = $classWithMethod->getMethod($methodName, $scope)->getDeclaringClass()->getName();
$this->callsBuffer[] = DeadCodeHelper::composeMethodKey($className, $methodName);
$this->callsBuffer[] = new Call($className, $methodName, $possibleDescendantCall);
}
}

Expand All @@ -164,11 +178,15 @@ private function registerArrayCallable(
$callableTypeAndNames = $constantArray->findTypeAndMethodNames();

foreach ($callableTypeAndNames as $typeAndName) {
$caller = $typeAndName->getType();
$methodName = $typeAndName->getMethod();

foreach ($this->getReflectionsWithMethod($typeAndName->getType(), $methodName) as $classWithMethod) {
// currently always true, see https://github.com/phpstan/phpstan-src/pull/3372
$possibleDescendantCall = !$caller->isClassStringType()->yes();

foreach ($this->getReflectionsWithMethod($caller, $methodName) as $classWithMethod) {
$className = $classWithMethod->getMethod($methodName, $scope)->getDeclaringClass()->getName();
$this->callsBuffer[] = DeadCodeHelper::composeMethodKey($className, $methodName);
$this->callsBuffer[] = new Call($className, $methodName, $possibleDescendantCall);
}
}
}
Expand All @@ -177,7 +195,7 @@ private function registerArrayCallable(

private function registerAttribute(Attribute $node, Scope $scope): void
{
$this->callsBuffer[] = DeadCodeHelper::composeMethodKey($scope->resolveName($node->name), '__construct');
$this->callsBuffer[] = new Call($scope->resolveName($node->name), '__construct', false);
}

/**
Expand Down
80 changes: 65 additions & 15 deletions src/Collector/MethodDefinitionCollector.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,20 @@

namespace ShipMonk\PHPStan\DeadCode\Collector;

use Closure;
use PhpParser\Node;
use PHPStan\Analyser\Scope;
use PHPStan\BetterReflection\Reflection\ReflectionMethod as BetterReflectionMethod;
use PHPStan\Collectors\Collector;
use PHPStan\Node\InClassNode;
use ShipMonk\PHPStan\DeadCode\Helper\DeadCodeHelper;
use PHPStan\Reflection\ClassReflection;
use ReflectionException;
use ShipMonk\PHPStan\DeadCode\Crate\MethodDefinition;
use function array_map;
use function strpos;

/**
* @implements Collector<InClassNode, list<array{line: int, methodKey: string, overrides: array<string, string>, traitOrigin: ?string}>>
* @implements Collector<InClassNode, list<array{line: int, definition: string, overriddenDefinitions: list<string>, traitOriginDefinition: ?string}>>
*/
class MethodDefinitionCollector implements Collector
{
Expand All @@ -22,7 +27,7 @@ public function getNodeType(): string

/**
* @param InClassNode $node
* @return list<array{line: int, methodKey: string, overrides: array<string, string>, traitOrigin: ?string}>|null
* @return list<array{line: int, definition: string, overriddenDefinitions: list<string>, traitOriginDefinition: ?string}>|null
*/
public function processNode(
Node $node,
Expand All @@ -37,6 +42,27 @@ public function processNode(
return null; // https://github.com/phpstan/phpstan/issues/8410
}

// we need to collect even methods of traits that are always overridden
foreach ($reflection->getTraits(true) as $trait) {
foreach ($trait->getNativeReflection()->getMethods() as $traitMethod) {
$traitLine = $traitMethod->getStartLine();
$traitName = $trait->getName();
$traitMethodName = $traitMethod->getName();
$declaringTraitDefinition = $this->getDeclaringTraitDefinition($trait, $traitMethodName);

if ($traitLine === false) {
continue;
}

$result[] = [
'line' => $traitLine,
'definition' => (new MethodDefinition($traitName, $traitMethodName))->toString(),
'overriddenDefinitions' => [],
'traitOriginDefinition' => $declaringTraitDefinition !== null ? $declaringTraitDefinition->toString() : null,
];
}
}

foreach ($nativeReflection->getMethods() as $method) {
if ($method->isDestructor()) {
continue;
Expand Down Expand Up @@ -70,11 +96,11 @@ public function processNode(

$className = $method->getDeclaringClass()->getName();
$methodName = $method->getName();
$methodKey = DeadCodeHelper::composeMethodKey($className, $methodName);
$definition = new MethodDefinition($className, $methodName);

$declaringTraitMethodKey = DeadCodeHelper::getDeclaringTraitMethodKey($reflection, $methodName);
$declaringTraitDefinition = $this->getDeclaringTraitDefinition($reflection, $methodName);

$methodOverrides = [];
$overriddenDefinitions = [];

foreach ($reflection->getAncestors() as $ancestor) {
if ($ancestor === $reflection) {
Expand All @@ -85,23 +111,47 @@ public function processNode(
continue;
}

if ($ancestor->isTrait()) {
continue;
}

$ancestorMethodKey = DeadCodeHelper::composeMethodKey($ancestor->getName(), $methodName);
$methodOverrides[$ancestorMethodKey] = $methodKey;
$overriddenDefinitions[] = new MethodDefinition($ancestor->getName(), $methodName);
}

$result[] = [
'line' => $line,
'methodKey' => $methodKey,
'overrides' => $methodOverrides,
'traitOrigin' => $declaringTraitMethodKey,
'definition' => $definition->toString(),
'overriddenDefinitions' => array_map(static fn (MethodDefinition $definition) => $definition->toString(), $overriddenDefinitions),
'traitOriginDefinition' => $declaringTraitDefinition !== null ? $declaringTraitDefinition->toString() : null,
];
}

return $result !== [] ? $result : null;
}

private function getDeclaringTraitDefinition(
ClassReflection $classReflection,
string $methodName
): ?MethodDefinition
{
try {
$nativeReflectionMethod = $classReflection->getNativeReflection()->getMethod($methodName);
$betterReflectionMethod = $nativeReflectionMethod->getBetterReflection();
$realDeclaringClass = $betterReflectionMethod->getDeclaringClass();

// when trait method name is aliased, we need the original name
$realName = Closure::bind(function (): string {
return $this->name;
}, $betterReflectionMethod, BetterReflectionMethod::class)();

} catch (ReflectionException $e) {
return null;
}

if ($realDeclaringClass->isTrait()) {
return new MethodDefinition(
$realDeclaringClass->getName(),
$realName,
);
}

return null;
}

}
54 changes: 54 additions & 0 deletions src/Crate/Call.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
<?php declare(strict_types = 1);

namespace ShipMonk\PHPStan\DeadCode\Crate;

use LogicException;
use function count;
use function explode;

/**
* @readonly
*/
class Call
{

public string $className;

public string $methodName;

public bool $possibleDescendantCall;

public function __construct(
string $className,
string $methodName,
bool $possibleDescendantCall
)
{
$this->className = $className;
$this->methodName = $methodName;
$this->possibleDescendantCall = $possibleDescendantCall;
}

public function getDefinition(): MethodDefinition
{
return new MethodDefinition($this->className, $this->methodName);
}

public function toString(): string
{
return "{$this->className}::{$this->methodName}::" . ($this->possibleDescendantCall ? '1' : '');
}

public static function fromString(string $methodKey): self
{
$exploded = explode('::', $methodKey);

if (count($exploded) !== 3) {
throw new LogicException("Invalid method key: $methodKey");
}

[$className, $methodName, $possibleDescendantCall] = $exploded;
return new self($className, $methodName, $possibleDescendantCall === '1');
}

}
21 changes: 0 additions & 21 deletions src/Crate/ClassAndMethod.php

This file was deleted.

45 changes: 45 additions & 0 deletions src/Crate/MethodDefinition.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
<?php declare(strict_types = 1);

namespace ShipMonk\PHPStan\DeadCode\Crate;

use LogicException;
use function count;
use function explode;

/**
* @readonly
*/
class MethodDefinition
{

public string $className;

public string $methodName;

public function __construct(
string $className,
string $methodName
)
{
$this->className = $className;
$this->methodName = $methodName;
}

public static function fromString(string $methodKey): self
{
$exploded = explode('::', $methodKey);

if (count($exploded) !== 2) {
throw new LogicException("Invalid method key: $methodKey");
}

[$className, $methodName] = $exploded;
return new self($className, $methodName);
}

public function toString(): string
{
return "{$this->className}::{$this->methodName}";
}

}
Loading
Loading