Skip to content

Commit

Permalink
Detect unused static method call on a separate line with possibly pur…
Browse files Browse the repository at this point in the history
…e method
  • Loading branch information
staabm authored Apr 18, 2024
1 parent fbd7273 commit df8e1f3
Show file tree
Hide file tree
Showing 5 changed files with 306 additions and 0 deletions.
10 changes: 10 additions & 0 deletions conf/config.level4.neon
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ conditionalTags:
phpstan.collector: %featureToggles.pure%
PHPStan\Rules\DeadCode\MethodWithoutImpurePointsCollector:
phpstan.collector: %featureToggles.pure%
PHPStan\Rules\DeadCode\CallToStaticMethodStatementWithoutImpurePointsRule:
phpstan.rules.rule: %featureToggles.pure%
PHPStan\Rules\DeadCode\PossiblyPureStaticCallCollector:
phpstan.collector: %featureToggles.pure%

parameters:
checkAdvancedIsset: true
Expand Down Expand Up @@ -124,6 +128,12 @@ services:
-
class: PHPStan\Rules\DeadCode\PossiblyPureMethodCallCollector

-
class: PHPStan\Rules\DeadCode\CallToStaticMethodStatementWithoutImpurePointsRule

-
class: PHPStan\Rules\DeadCode\PossiblyPureStaticCallCollector

-
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 CallToStaticMethodStatementWithoutImpurePointsRule 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]) {
$lowerClassName = strtolower($className);

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

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

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

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

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

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

return $errors;
}

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

/**
* @implements Collector<Node\Stmt\Expression, array{class-string, string, int}>
*/
class PossiblyPureStaticCallCollector 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\StaticCall) {
return null;
}
if (!$node->expr->name instanceof Node\Identifier) {
return null;
}

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

$methodName = $node->expr->name->toString();
$calledOnType = $scope->resolveTypeByName($node->expr->class);
$methodReflection = $scope->getMethodReflection($calledOnType, $methodName);

if ($methodReflection === null) {
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,69 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\DeadCode;

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

/**
* @extends RuleTestCase<CallToStaticMethodStatementWithoutImpurePointsRule>
*/
class CallToStaticMethodStatementWithoutImpurePointsRuleTest extends RuleTestCase
{

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

public function testRule(): void
{
$this->analyse([__DIR__ . '/data/call-to-static-method-without-impure-points.php'], [
[
'Call to CallToStaticMethodWithoutImpurePoints\X::myFunc() on a separate line has no effect.',
6,
],
[
'Call to CallToStaticMethodWithoutImpurePoints\X::myFunc() on a separate line has no effect.',
7,
],
[
'Call to CallToStaticMethodWithoutImpurePoints\X::myFunc() on a separate line has no effect.',
16,
],
[
'Call to CallToStaticMethodWithoutImpurePoints\y::myFunc() on a separate line has no effect.',
18,
],
[
'Call to CallToStaticMethodWithoutImpurePoints\y::myFunc() on a separate line has no effect.',
20,
],
[
'Call to CallToStaticMethodWithoutImpurePoints\SubSubY::mySubSubFunc() on a separate line has no effect.',
21,
],
[
'Call to CallToStaticMethodWithoutImpurePoints\y::myFunc() on a separate line has no effect.',
48,
],
[
'Call to CallToStaticMethodWithoutImpurePoints\y::myFunc() on a separate line has no effect.',
53,
],
[
'Call to CallToStaticMethodWithoutImpurePoints\y::myFunc() on a separate line has no effect.',
58,
],
]);
}

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

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

namespace CallToStaticMethodWithoutImpurePoints;

function (): void {
X::myFunc();
X::myFUNC();
X::throwingFUNC();
X::throwingFunc();
X::funcWithRef();
X::impureFunc();
X::callingImpureFunc();

$a = X::myFunc();

x::myFunc(); // case-insensitive class name

subY::myFunc();

SubSubY::myFunc();
SubSubY::mySubSubFunc();
SubSubY::mySubSubCallSelfFunc();
SubSubY::mySubSubCallParentFunc();
SubSubY::mySubSubCallStaticFunc();
SubSubY::mySubSubCallSelfImpureFunc();
SubSubY::mySubSubCallParentImpureFunc();
SubSubY::mySubSubCallStaticImpureFunc();
};

class y
{
static function myFunc()
{
}

static function myImpureFunc()
{
echo '1';
}
}

class subY extends y {
}

class SubSubY extends subY {
static function mySubSubCallStaticFunc()
{
static::myFunc();
}

static function mySubSubCallSelfFunc()
{
self::myFunc();
}

static function mySubSubCallParentFunc()
{
parent::myFunc();
}
static function mySubSubCallStaticImpureFunc()
{
static::myImpureFunc();
}

static function mySubSubCallSelfImpureFunc()
{
self::myImpureFunc();
}

static function mySubSubCallParentImpureFunc()
{
parent::myImpureFunc();
}

static function mySubSubFunc()
{
}
}

class X {
static function myFunc()
{
}

static function throwingFunc()
{
throw new \Exception();
}

static function funcWithRef(&$a)
{
}

/** @phpstan-impure */
static function impureFunc()
{
}

static function callingImpureFunc()
{
self::impureFunc();
}
}

0 comments on commit df8e1f3

Please sign in to comment.