Skip to content

Commit

Permalink
Detect unused method call on a separate line with possibly pure method
Browse files Browse the repository at this point in the history
  • Loading branch information
staabm committed Apr 18, 2024
1 parent 7045457 commit 47f837b
Show file tree
Hide file tree
Showing 7 changed files with 395 additions and 2 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/static-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,9 @@ jobs:
uses: actions/cache@v4
with:
path: ./tmp
key: "result-cache-v12-${{ matrix.php-version }}-${{ github.run_id }}"
key: "result-cache-v13-${{ matrix.php-version }}-${{ github.run_id }}"
restore-keys: |
result-cache-v12-${{ matrix.php-version }}-
result-cache-v13-${{ matrix.php-version }}-
- name: "PHPStan with result cache"
run: |
Expand Down
15 changes: 15 additions & 0 deletions conf/config.level4.neon
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,12 @@ conditionalTags:
phpstan.collector: %featureToggles.pure%
PHPStan\Rules\DeadCode\FunctionWithoutImpurePointsCollector:
phpstan.collector: %featureToggles.pure%
PHPStan\Rules\DeadCode\CallToMethodStatementWithoutImpurePointsRule:
phpstan.rules.rule: %featureToggles.pure%
PHPStan\Rules\DeadCode\PossiblyPureMethodCallCollector:
phpstan.collector: %featureToggles.pure%
PHPStan\Rules\DeadCode\MethodWithoutImpurePointsCollector:
phpstan.collector: %featureToggles.pure%

parameters:
checkAdvancedIsset: true
Expand Down Expand Up @@ -109,6 +115,15 @@ services:
-
class: PHPStan\Rules\DeadCode\PossiblyPureFuncCallCollector

-
class: PHPStan\Rules\DeadCode\CallToMethodStatementWithoutImpurePointsRule

-
class: PHPStan\Rules\DeadCode\MethodWithoutImpurePointsCollector

-
class: PHPStan\Rules\DeadCode\PossiblyPureMethodCallCollector

-
class: PHPStan\Rules\DeadCode\UnusedPrivatePropertyRule
arguments:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
<?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 CallToMethodStatementWithoutImpurePointsRule implements Rule
{

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

public function processNode(Node $node, Scope $scope): array
{
$methods = [];
foreach ($node->get(MethodWithoutImpurePointsCollector::class) as $collected) {
foreach ($collected as [$className, $methodName, $classDisplayName]) {
$className = strtolower($className);

if (!array_key_exists($className, $methods)) {
$methods[$className] = [];
}
$methods[$className][strtolower($methodName)] = $classDisplayName . '::' . $methodName;
}
}

$errors = [];
foreach ($node->get(PossiblyPureMethodCallCollector::class) as $filePath => $data) {
foreach ($data as [$className, $method, $line]) {
$className = strtolower($className);

if (!array_key_exists($className, $methods)) {
continue;
}

$lowerMethod = strtolower($method);
if (!array_key_exists($lowerMethod, $methods[$className])) {
continue;
}

$originalMethodName = $methods[$className][$lowerMethod];

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

return $errors;
}

}
57 changes: 57 additions & 0 deletions src/Rules/DeadCode/MethodWithoutImpurePointsCollector.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
<?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;

/**
* @implements Collector<MethodReturnStatementsNode, array{class-string, string, string}>
*/
class MethodWithoutImpurePointsCollector implements Collector
{

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

public function processNode(Node $node, Scope $scope)
{
$method = $node->getMethodReflection();
if (!$method->isPure()->maybe()) {
return null;
}
if (!$method->hasSideEffects()->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(), $method->getName(), $method->getDeclaringClass()->getDisplayName()];
}

}
68 changes: 68 additions & 0 deletions src/Rules/DeadCode/PossiblyPureMethodCallCollector.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
<?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 function count;
use function strtolower;

/**
* @implements Collector<Node\Stmt\Expression, array{class-string, string, int}>
*/
class PossiblyPureMethodCallCollector implements Collector
{

public function __construct()
{
}

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

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

$methodName = $node->expr->name->toString();
if (strtolower($methodName) === '__construct') {
return null;
}

$calledOnType = $scope->getType($node->expr->var);
$methodReflection = $scope->getMethodReflection($calledOnType, $methodName);
if ($methodReflection === null) {
return null;
}
if (!$methodReflection->isFinal()->yes()) {
if (!$methodReflection->getDeclaringClass()->isFinal()) {
$typeClassReflections = $calledOnType->getObjectClassReflections();
if (count($typeClassReflections) !== 1) {
return null;
}

if (!$typeClassReflections[0]->isFinal()) {
return null;
}
}
}
if (!$methodReflection->isPure()->maybe()) {
return null;
}
if (!$methodReflection->hasSideEffects()->maybe()) {
return null;
}

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

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

namespace PHPStan\Rules\DeadCode;

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

/**
* @extends RuleTestCase<CallToMethodStatementWithoutImpurePointsRule>
*/
class CallToMethodStatementWithoutImpurePointsRuleTest extends RuleTestCase
{

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

public function testRule(): void
{
$this->analyse([__DIR__ . '/data/call-to-method-without-impure-points.php'], [
[
'Call to method CallToMethodWithoutImpurePoints\finalX::myFunc() on a separate line has no effect.',
7,
],
[
'Call to method CallToMethodWithoutImpurePoints\finalX::myFunc() on a separate line has no effect.',
8,
],
[
'Call to method CallToMethodWithoutImpurePoints\foo::finalFunc() on a separate line has no effect.',
30,
],
[
'Call to method CallToMethodWithoutImpurePoints\y::myFinalBaseFunc() on a separate line has no effect.',
36,
],
[
'Call to method CallToMethodWithoutImpurePoints\y::myFunc() on a separate line has no effect.',
39,
],
[
'Call to method CallToMethodWithoutImpurePoints\finalSubSubY::mySubSubFunc() on a separate line has no effect.',
40,
],
[
'Call to method CallToMethodWithoutImpurePoints\y::myFinalBaseFunc() on a separate line has no effect.',
41,
],
[
'Call to method CallToMethodWithoutImpurePoints\AbstractFoo::myFunc() on a separate line has no effect.',
119,
],
]);
}

protected function getCollectors(): array
{
return [
new PossiblyPureMethodCallCollector(),
new MethodWithoutImpurePointsCollector(),
];
}

}

0 comments on commit 47f837b

Please sign in to comment.