Skip to content

Detect dead constructors #39

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 4 commits into from
Jul 24, 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
5 changes: 3 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ includes:
## Configuration:
- All entrypoints of your code (controllers, consumers, commands, ...) need to be known to the detector to get proper results
- By default, all overridden methods which declaration originates inside vendor are considered entrypoints
- Also, there are some built-in providers for some magic calls that occur in `doctrine`, `symfony` and `phpunit`
- Also, there are some built-in providers for some magic calls that occur in `doctrine`, `symfony`, `phpstan` and `phpunit`
- For everything else, you can implement your own entrypoint provider, just tag it with `shipmonk.deadCode.entrypointProvider` and implement `ShipMonk\PHPStan\DeadCode\Provider\EntrypointProvider`

```neon
Expand All @@ -29,6 +29,8 @@ parameters:
entrypoints:
vendor:
enabled: true # enabled by default
phpstan:
enabled: true # enabled by default
symfony:
enabled: true
phpunit:
Expand Down Expand Up @@ -65,7 +67,6 @@ class MyEntrypointProvider implements EntrypointProvider
- Any calls on mixed types are not detected, e.g. `$unknownClass->method()`
- Expression method calls are not detected, e.g. `$this->$methodName()`
- Anonymous classes are ignored
- Does not check constructor calls
- Does not check magic methods
- No transitive check is performed (dead method called only from dead method)
- No dead cycles are detected (e.g. dead method calling itself)
Expand Down
1 change: 1 addition & 0 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
"ergebnis/composer-normalize": "^2.28",
"phpstan/phpstan-phpunit": "^1.1.1",
"phpstan/phpstan-strict-rules": "^1.2.3",
"phpstan/phpstan-symfony": "^1.4",
"phpunit/phpunit": "^9.5.20",
"shipmonk/composer-dependency-analyser": "^1.6",
"shipmonk/name-collision-detector": "^2.0.0",
Expand Down
74 changes: 73 additions & 1 deletion composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

24 changes: 22 additions & 2 deletions rules.neon
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
services:
-
class: ShipMonk\PHPStan\DeadCode\Reflection\ClassHierarchy

-
class: ShipMonk\PHPStan\DeadCode\Provider\VendorEntrypointProvider
tags:
Expand Down Expand Up @@ -27,20 +30,32 @@ services:
arguments:
enabled: %shipmonkDeadCode.entrypoints.doctrine.enabled%

-
class: ShipMonk\PHPStan\DeadCode\Provider\PhpStanEntrypointProvider
tags:
- shipmonk.deadCode.entrypointProvider
arguments:
enabled: %shipmonkDeadCode.entrypoints.phpstan.enabled%

-
class: ShipMonk\PHPStan\DeadCode\Collector\MethodCallCollector
tags:
- phpstan.collector

-
class: ShipMonk\PHPStan\DeadCode\Collector\ClassDefinitionCollector
tags:
- phpstan.collector

-
class: ShipMonk\PHPStan\DeadCode\Collector\MethodDefinitionCollector
arguments:
entrypointProviders: tagged(shipmonk.deadCode.entrypointProvider)
tags:
- phpstan.collector

-
class: ShipMonk\PHPStan\DeadCode\Rule\DeadMethodRule
arguments:
entrypointProviders: tagged(shipmonk.deadCode.entrypointProvider)
tags:
- phpstan.rules.rule

Expand All @@ -50,6 +65,8 @@ parameters:
entrypoints:
vendor:
enabled: true
phpstan:
enabled: true
phpunit:
enabled: false
symfony:
Expand All @@ -72,5 +89,8 @@ parametersSchema:
doctrine: structure([
enabled: bool()
])
phpstan: structure([
enabled: bool()
])
])
])
33 changes: 33 additions & 0 deletions src/Collector/ClassDefinitionCollector.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?php declare(strict_types = 1);

namespace ShipMonk\PHPStan\DeadCode\Collector;

use PhpParser\Node;
use PHPStan\Analyser\Scope;
use PHPStan\Collectors\Collector;
use PHPStan\Node\InClassNode;

/**
* @implements Collector<InClassNode, array{string}>
*/
class ClassDefinitionCollector implements Collector
{

public function getNodeType(): string
{
return InClassNode::class;
}

/**
* @param InClassNode $node
* @return array{string}
*/
public function processNode(
Node $node,
Scope $scope
): array
{
return [$node->getClassReflection()->getName()];
}

}
36 changes: 32 additions & 4 deletions src/Collector/MethodCallCollector.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@
namespace ShipMonk\PHPStan\DeadCode\Collector;

use PhpParser\Node;
use PhpParser\Node\Attribute;
use PhpParser\Node\Expr;
use PhpParser\Node\Expr\CallLike;
use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Expr\MethodCall;
use PhpParser\Node\Expr\New_;
use PhpParser\Node\Expr\NullsafeMethodCall;
use PhpParser\Node\Expr\StaticCall;
use PhpParser\Node\Identifier;
Expand Down Expand Up @@ -60,7 +62,7 @@ public function processNode(
$this->registerStaticCall($node->getOriginalNode(), $scope);
}

if ($node instanceof MethodCall || $node instanceof NullsafeMethodCall) {
if ($node instanceof MethodCall || $node instanceof NullsafeMethodCall || $node instanceof New_) {
$this->registerMethodCall($node, $scope);
}

Expand All @@ -72,6 +74,10 @@ public function processNode(
$this->registerFuncCall($node, $scope);
}

if ($node instanceof Attribute) {
$this->registerAttribute($node, $scope);
}

if (!$scope->isInClass() || $node instanceof ClassMethodsNode) { // @phpstan-ignore-line ignore BC promise
$data = $this->callsBuffer;
$this->callsBuffer = [];
Expand All @@ -82,15 +88,28 @@ public function processNode(
}

/**
* @param NullsafeMethodCall|MethodCall $methodCall
* @param NullsafeMethodCall|MethodCall|New_ $methodCall
*/
private function registerMethodCall(
CallLike $methodCall,
Scope $scope
): void
{
$methodName = $this->getMethodName($methodCall);
$callerType = $scope->getType($methodCall->var);

if ($methodCall instanceof New_) {
if ($methodCall->class instanceof Expr) {
$callerType = $scope->getType($methodCall->class);

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

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

if ($methodName === null) {
return;
Expand Down Expand Up @@ -165,11 +184,20 @@ private function registerFuncCall(
}
}

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

/**
* @param NullsafeMethodCall|MethodCall|StaticCall $call
* @param NullsafeMethodCall|MethodCall|StaticCall|New_ $call
*/
private function getMethodName(CallLike $call): ?string
{
if ($call instanceof New_) {
return '__construct';
}

if (!$call->name instanceof Identifier) {
return null;
}
Expand Down
36 changes: 2 additions & 34 deletions src/Collector/MethodDefinitionCollector.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@
use PHPStan\Analyser\Scope;
use PHPStan\Collectors\Collector;
use PHPStan\Node\InClassNode;
use ReflectionMethod;
use ShipMonk\PHPStan\DeadCode\Helper\DeadCodeHelper;
use ShipMonk\PHPStan\DeadCode\Provider\EntrypointProvider;
use function strpos;

/**
Expand All @@ -17,21 +15,6 @@
class MethodDefinitionCollector implements Collector
{

/**
* @var array<EntrypointProvider>
*/
private array $entrypointProviders;

/**
* @param array<EntrypointProvider> $entrypointProviders
*/
public function __construct(
array $entrypointProviders
)
{
$this->entrypointProviders = $entrypointProviders;
}

public function getNodeType(): string
{
return InClassNode::class;
Expand All @@ -55,11 +38,11 @@ public function processNode(
continue;
}

if ($method->isConstructor()) {
if (!$method->isConstructor() && strpos($method->getName(), '__') === 0) { // magic methods like __toString, __clone, __get, __set etc
continue;
}

if (strpos($method->getName(), '__') === 0) { // magic methods like __toString, __clone, __get, __set etc
if ($method->isConstructor() && $method->isPrivate()) { // e.g. classes used for storing static methods only
continue;
}

Expand All @@ -75,10 +58,6 @@ public function processNode(
continue;
}

if ($this->isEntrypoint($method)) {
continue;
}

$line = $method->getStartLine();

if ($line === false) {
Expand All @@ -92,15 +71,4 @@ public function processNode(
return $result !== [] ? $result : null;
}

private function isEntrypoint(ReflectionMethod $reflectionMethod): bool
{
foreach ($this->entrypointProviders as $entrypointProvider) {
if ($entrypointProvider->isEntrypoint($reflectionMethod)) {
return true;
}
}

return false;
}

}
Loading
Loading