Skip to content

Commit

Permalink
Detect unused new on a separate line with possibly pure constructor
Browse files Browse the repository at this point in the history
  • Loading branch information
ondrejmirtes committed Apr 17, 2024
1 parent 6026869 commit 281a87d
Show file tree
Hide file tree
Showing 6 changed files with 241 additions and 0 deletions.
15 changes: 15 additions & 0 deletions conf/config.level4.neon
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,12 @@ conditionalTags:
phpstan.rules.rule: %featureToggles.paramOutType%
PHPStan\Rules\TooWideTypehints\TooWideMethodParameterOutTypeRule:
phpstan.rules.rule: %featureToggles.paramOutType%
PHPStan\Rules\DeadCode\CallToConstructorStatementWithoutImpurePointsRule:
phpstan.rules.rule: %featureToggles.pure%
PHPStan\Rules\DeadCode\PossiblyPureNewCollector:
phpstan.collector: %featureToggles.pure%
PHPStan\Rules\DeadCode\ConstructorWithoutImpurePointsCollector:
phpstan.collector: %featureToggles.pure%

parameters:
checkAdvancedIsset: true
Expand Down Expand Up @@ -79,6 +85,15 @@ services:
tags:
- phpstan.rules.rule

-
class: PHPStan\Rules\DeadCode\CallToConstructorStatementWithoutImpurePointsRule

-
class: PHPStan\Rules\DeadCode\ConstructorWithoutImpurePointsCollector

-
class: PHPStan\Rules\DeadCode\PossiblyPureNewCollector

-
class: PHPStan\Rules\DeadCode\UnusedPrivatePropertyRule
arguments:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\DeadCode;

use PhpParser\Node;
use PHPStan\Analyser\Scope;
use PHPStan\Node\CollectedDataNode;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleErrorBuilder;
use function array_key_exists;
use function sprintf;
use function strtolower;

/**
* @implements Rule<CollectedDataNode>
*/
class CallToConstructorStatementWithoutImpurePointsRule implements Rule
{

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

public function processNode(Node $node, Scope $scope): array
{
$classesWithConstructors = [];
foreach ($node->get(ConstructorWithoutImpurePointsCollector::class) as [$class]) {
$classesWithConstructors[strtolower($class)] = $class;
}

$errors = [];
foreach ($node->get(PossiblyPureNewCollector::class) as $filePath => $data) {
foreach ($data as [$class, $line]) {
$lowerClass = strtolower($class);
if (!array_key_exists($lowerClass, $classesWithConstructors)) {
continue;
}

$originalClassName = $classesWithConstructors[$lowerClass];
$errors[] = RuleErrorBuilder::message(sprintf(
'Call to new %s() on a separate line has no effect.',
$originalClassName,
))->file($filePath)
->line($line)
->identifier('new.resultUnused')
->build();
}
}

return $errors;
}

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

namespace PHPStan\Rules\DeadCode;

use PhpParser\Node;
use PHPStan\Analyser\Scope;
use PHPStan\Collectors\Collector;
use PHPStan\Node\MethodReturnStatementsNode;
use PHPStan\Reflection\ParametersAcceptorSelector;
use function count;
use function strtolower;

/**
* @implements Collector<MethodReturnStatementsNode, string>
*/
class ConstructorWithoutImpurePointsCollector implements Collector
{

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

public function processNode(Node $node, Scope $scope)
{
$method = $node->getMethodReflection();
if (strtolower($method->getName()) !== '__construct') {
return null;
}

if (!$method->isPure()->maybe()) {
return null;
}

if (count($node->getImpurePoints()) !== 0) {
return null;
}

if (count($node->getStatementResult()->getThrowPoints()) !== 0) {
return null;
}

$variant = ParametersAcceptorSelector::selectSingle($method->getVariants());
foreach ($variant->getParameters() as $parameter) {
if (!$parameter->passedByReference()->createsNewVariable()) {
continue;
}

return null;
}

if (count($method->getAsserts()->getAll()) !== 0) {
return null;
}

return $method->getDeclaringClass()->getName();
}

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

namespace PHPStan\Rules\DeadCode;

use PhpParser\Node;
use PhpParser\Node\Stmt\Expression;
use PHPStan\Analyser\Scope;
use PHPStan\Collectors\Collector;
use PHPStan\Reflection\ReflectionProvider;
use function strtolower;

/**
* @implements Collector<Expression, array{string, int}>
*/
class PossiblyPureNewCollector implements Collector
{

public function __construct(private ReflectionProvider $reflectionProvider)
{
}

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

public function processNode(Node $node, Scope $scope)
{
if (!$node->expr instanceof Node\Expr\New_) {
return null;
}

if (!$node->expr->class instanceof Node\Name) {
return null;
}

$className = $node->expr->class->toString();

if (!$this->reflectionProvider->hasClass($className)) {
return null;
}

$classReflection = $this->reflectionProvider->getClass($className);
if (!$classReflection->hasConstructor()) {
return null;
}

$constructor = $classReflection->getConstructor();
if (strtolower($constructor->getName()) !== '__construct') {
return null;
}

if (!$constructor->isPure()->maybe()) {
return null;
}

return [$constructor->getDeclaringClass()->getName(), $node->getStartLine()];
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\DeadCode;

use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;

/**
* @extends RuleTestCase<CallToConstructorStatementWithoutImpurePointsRule>
*/
class CallToConstructorStatementWithoutImpurePointsRuleTest extends RuleTestCase
{

protected function getRule(): Rule
{
return new CallToConstructorStatementWithoutImpurePointsRule();
}

public function testRule(): void
{
$this->analyse([__DIR__ . '/data/call-to-constructor-without-impure-points.php'], [
[
'Call to new CallToConstructorWithoutImpurePoints\Foo() on a separate line has no effect.',
15,
],
]);
}

protected function getCollectors(): array
{
return [
new PossiblyPureNewCollector($this->createReflectionProvider()),
new ConstructorWithoutImpurePointsCollector(),
];
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?php

namespace CallToConstructorWithoutImpurePoints;

class Foo
{

public function __construct()
{
}

}

function (): void {
new Foo();
};

0 comments on commit 281a87d

Please sign in to comment.