Skip to content
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
<?php

namespace Rector\Tests\DeadCode\Rector\ClassMethod\RemoveUnusedPrivateMethodRector\Fixture;

// real-world false positive (spiral/framework ReflectionFileTest::deadend):
// the method body is read back via new ReflectionFile(__FILE__) and its
// function invocations are asserted - so it is "used" only at runtime via
// self-reflection, which static analysis cannot see.
final class UsedViaSelfReflection
{
public function testInvocations(): void
{
$reflection = new ReflectionFile(__FILE__);
foreach ($reflection->getInvocations() as $invocation) {
// asserts test_function_a / test_function_b are found
}
}

private function deadend(): void
{
$a = $b = null;
test_function_a($this, $a + $b);
test_function_b("string", 123);
}
}

?>
-----
<?php

namespace Rector\Tests\DeadCode\Rector\ClassMethod\RemoveUnusedPrivateMethodRector\Fixture;

// real-world false positive (spiral/framework ReflectionFileTest::deadend):
// the method body is read back via new ReflectionFile(__FILE__) and its
// function invocations are asserted - so it is "used" only at runtime via
// self-reflection, which static analysis cannot see.
final class UsedViaSelfReflection
{
public function testInvocations(): void
{
$reflection = new ReflectionFile(__FILE__);
foreach ($reflection->getInvocations() as $invocation) {
// asserts test_function_a / test_function_b are found
}
}
}

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

namespace Rector\Tests\DeadCode\Rector\If_\RemoveAlwaysTrueIfConditionRector\Fixture;

class SkipClassExistsAfterEval
{
public function compile(string $class, string $compiled): object
{
if (! \class_exists($class)) {
// the class is defined dynamically here, PHPStan does not see it
eval('?>' . $compiled);
}

return new $class();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<?php

namespace Rector\Tests\DeadCode\Rector\If_\RemoveAlwaysTrueIfConditionRector\Fixture;

class SkipFunctionExistsEnum
{
/**
* @param array<string> $enums
*/
public function expand(array $enums): array
{
$cases = [];
foreach ($enums as $enum) {
if (is_string($enum) && function_exists('enum_exists') && enum_exists($enum)) {
foreach ($enum::cases() as $case) {
$cases[] = $case;
}
} else {
$cases[] = $enum;
}
}

return $cases;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?php

namespace Rector\Tests\DeadCode\Rector\If_\RemoveAlwaysTrueIfConditionRector\Fixture;

class SkipInterfaceExistsInLoop
{
/**
* @param string[] $dependencies
*/
public function check(array $dependencies): void
{
foreach ($dependencies as $dep) {
if (! \class_exists($dep, true) && ! \interface_exists($dep, true)) {
continue;
}

echo $dep;
}
}
}
36 changes: 35 additions & 1 deletion rules/DeadCode/Rector/If_/RemoveAlwaysTrueIfConditionRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use PhpParser\Node\Expr\ArrayDimFetch;
use PhpParser\Node\Expr\Assign;
use PhpParser\Node\Expr\BinaryOp\BooleanAnd;
use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Expr\MethodCall;
use PhpParser\Node\Expr\PropertyFetch;
use PhpParser\Node\Expr\StaticCall;
Expand All @@ -34,6 +35,25 @@
*/
final class RemoveAlwaysTrueIfConditionRector extends AbstractRector
{
/**
* Functions that depend on runtime state (autoloading, eval(), loaded extensions).
* PHPStan may narrow their result to a constant true, but it can change at runtime,
* so the condition must not be treated as always true.
*
* @var string[]
*/
private const array RUNTIME_STATE_FUNCTIONS = [
'class_exists',
'interface_exists',
'trait_exists',
'enum_exists',
'function_exists',
'method_exists',
'property_exists',
'defined',
'extension_loaded',
];

public function __construct(
private readonly ExprAnalyzer $exprAnalyzer,
private readonly BetterNodeFinder $betterNodeFinder,
Expand Down Expand Up @@ -170,7 +190,7 @@ private function shouldSkipFromVariable(Expr $expr): bool

private function shouldSkipExpr(Expr $expr): bool
{
return (bool) $this->betterNodeFinder->findInstancesOf(
$hasNonStaticNode = (bool) $this->betterNodeFinder->findInstancesOf(
$expr,
[
PropertyFetch::class,
Expand All @@ -180,6 +200,20 @@ private function shouldSkipExpr(Expr $expr): bool
StaticCall::class,
]
);

if ($hasNonStaticNode) {
return true;
}

/** @var FuncCall[] $funcCalls */
$funcCalls = $this->betterNodeFinder->findInstancesOf($expr, [FuncCall::class]);
foreach ($funcCalls as $funcCall) {
if ($this->isNames($funcCall, self::RUNTIME_STATE_FUNCTIONS)) {
return true;
}
}

return false;
}

private function refactorIfWithBooleanAnd(If_ $if): ?If_
Expand Down
Loading