mirror of
https://github.com/rectorphp/rector.git
synced 2024-06-26 12:52:36 +00:00
[DeadCode] RemoveUnusedElseForReturnedValueRector
This commit is contained in:
parent
11b5bee518
commit
3e575b89e5
14
abz/UnderElse.php
Normal file
14
abz/UnderElse.php
Normal file
|
@ -0,0 +1,14 @@
|
|||
<?php declare(strict_types=1);
|
||||
|
||||
|
||||
final class UnderElse
|
||||
{
|
||||
public function run($value)
|
||||
{
|
||||
if ($value) {
|
||||
return 5;
|
||||
} else {
|
||||
return 10;
|
||||
}
|
||||
}
|
||||
}
|
|
@ -30,3 +30,4 @@ services:
|
|||
Rector\DeadCode\Rector\Stmt\RemoveUnreachableStatementRector: ~
|
||||
Rector\DeadCode\Rector\If_\SimplifyIfElseWithSameContentRector: ~
|
||||
Rector\DeadCode\Rector\Ternary\TernaryToBooleanOrFalseToBooleanAndRector: ~
|
||||
Rector\DeadCode\Rector\FunctionLike\RemoveUnusedElseForReturnedValueRector: ~
|
||||
|
|
|
@ -0,0 +1,97 @@
|
|||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace Rector\DeadCode\Rector\FunctionLike;
|
||||
|
||||
use PhpParser\Node;
|
||||
use PhpParser\Node\FunctionLike;
|
||||
use PhpParser\Node\Stmt\If_;
|
||||
use Rector\PhpParser\Node\Manipulator\IfManipulator;
|
||||
use Rector\Rector\AbstractRector;
|
||||
use Rector\RectorDefinition\CodeSample;
|
||||
use Rector\RectorDefinition\RectorDefinition;
|
||||
|
||||
/**
|
||||
* @see \Rector\DeadCode\Tests\Rector\FunctionLike\RemoveUnusedElseForReturnedValueRector\RemoveUnusedElseForReturnedValueRectorTest
|
||||
*/
|
||||
final class RemoveUnusedElseForReturnedValueRector extends AbstractRector
|
||||
{
|
||||
/**
|
||||
* @var IfManipulator
|
||||
*/
|
||||
private $ifManipulator;
|
||||
|
||||
public function __construct(IfManipulator $ifManipulator)
|
||||
{
|
||||
$this->ifManipulator = $ifManipulator;
|
||||
}
|
||||
|
||||
public function getDefinition(): RectorDefinition
|
||||
{
|
||||
return new RectorDefinition('Remove if for last else, if previous values were awlways returned', [
|
||||
new CodeSample(
|
||||
<<<'PHP'
|
||||
class SomeClass
|
||||
{
|
||||
public function run($value)
|
||||
{
|
||||
if ($value) {
|
||||
return 5;
|
||||
} else {
|
||||
return 10;
|
||||
}
|
||||
}
|
||||
}
|
||||
PHP
|
||||
,
|
||||
<<<'PHP'
|
||||
class SomeClass
|
||||
{
|
||||
public function run($value)
|
||||
{
|
||||
if ($value) {
|
||||
return 5;
|
||||
}
|
||||
|
||||
return 10;
|
||||
}
|
||||
}
|
||||
PHP
|
||||
|
||||
),
|
||||
]);
|
||||
}
|
||||
|
||||
/**
|
||||
* @return string[]
|
||||
*/
|
||||
public function getNodeTypes(): array
|
||||
{
|
||||
return [FunctionLike::class];
|
||||
}
|
||||
|
||||
/**
|
||||
* @param FunctionLike $node
|
||||
*/
|
||||
public function refactor(Node $node): ?Node
|
||||
{
|
||||
$this->traverseNodesWithCallable((array) $node->getStmts(), function (Node $node) {
|
||||
if (! $node instanceof If_) {
|
||||
return null;
|
||||
}
|
||||
|
||||
if (! $this->ifManipulator->isWithElseAlwaysReturnValue($node)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
foreach ($node->else->stmts as $elseStmt) {
|
||||
$this->addNodeAfterNode($elseStmt, $node);
|
||||
}
|
||||
|
||||
$node->else = null;
|
||||
});
|
||||
|
||||
return $node;
|
||||
}
|
||||
}
|
|
@ -0,0 +1,34 @@
|
|||
<?php
|
||||
|
||||
namespace Rector\DeadCode\Tests\Rector\FunctionLike\RemoveUnusedElseForReturnedValueRector\Fixture;
|
||||
|
||||
class SomeClass
|
||||
{
|
||||
public function run($value)
|
||||
{
|
||||
if ($value) {
|
||||
return 5;
|
||||
} else {
|
||||
return 10;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
?>
|
||||
-----
|
||||
<?php
|
||||
|
||||
namespace Rector\DeadCode\Tests\Rector\FunctionLike\RemoveUnusedElseForReturnedValueRector\Fixture;
|
||||
|
||||
class SomeClass
|
||||
{
|
||||
public function run($value)
|
||||
{
|
||||
if ($value) {
|
||||
return 5;
|
||||
}
|
||||
return 10;
|
||||
}
|
||||
}
|
||||
|
||||
?>
|
|
@ -0,0 +1,19 @@
|
|||
<?php
|
||||
|
||||
namespace Rector\DeadCode\Tests\Rector\FunctionLike\RemoveUnusedElseForReturnedValueRector\Fixture;
|
||||
|
||||
use PhpParser\Node\Expr\BinaryOp\Equal;
|
||||
|
||||
class SkipNonFirstLevel
|
||||
{
|
||||
public function test($node)
|
||||
{
|
||||
if ($node->cond instanceof Equal) {
|
||||
if ($node->stmts[0]->expr === null) {
|
||||
return null;
|
||||
}
|
||||
} else {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
}
|
|
@ -0,0 +1,39 @@
|
|||
<?php
|
||||
|
||||
namespace Rector\DeadCode\Tests\Rector\FunctionLike\RemoveUnusedElseForReturnedValueRector\Fixture;
|
||||
|
||||
class SkipNotOnlyElse
|
||||
{
|
||||
public function run($value)
|
||||
{
|
||||
if ($value) {
|
||||
return 5;
|
||||
} elseif ($value- 1) {
|
||||
$value = 55;
|
||||
return;
|
||||
} else {
|
||||
return 10;
|
||||
}
|
||||
}
|
||||
|
||||
public function runAgain($value)
|
||||
{
|
||||
if ($value) {
|
||||
return 5;
|
||||
} elseif ($value- 1) {
|
||||
$value = 55;
|
||||
return 10;
|
||||
} else {
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
public function runAgainAndAgain($value)
|
||||
{
|
||||
if ($value) {
|
||||
5 + 10;
|
||||
} else {
|
||||
return $value;
|
||||
}
|
||||
}
|
||||
}
|
|
@ -0,0 +1,30 @@
|
|||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace Rector\DeadCode\Tests\Rector\FunctionLike\RemoveUnusedElseForReturnedValueRector;
|
||||
|
||||
use Iterator;
|
||||
use Rector\DeadCode\Rector\FunctionLike\RemoveUnusedElseForReturnedValueRector;
|
||||
use Rector\Testing\PHPUnit\AbstractRectorTestCase;
|
||||
|
||||
final class RemoveUnusedElseForReturnedValueRectorTest extends AbstractRectorTestCase
|
||||
{
|
||||
/**
|
||||
* @dataProvider provideDataForTest()
|
||||
*/
|
||||
public function test(string $file): void
|
||||
{
|
||||
$this->doTestFile($file);
|
||||
}
|
||||
|
||||
public function provideDataForTest(): Iterator
|
||||
{
|
||||
return $this->yieldFilesFromDirectory(__DIR__ . '/Fixture');
|
||||
}
|
||||
|
||||
protected function getRectorClass(): string
|
||||
{
|
||||
return RemoveUnusedElseForReturnedValueRector::class;
|
||||
}
|
||||
}
|
|
@ -4,11 +4,17 @@ declare(strict_types=1);
|
|||
|
||||
namespace Rector\PhpParser\Node\Manipulator;
|
||||
|
||||
use PhpParser\Node;
|
||||
use PhpParser\Node\Expr;
|
||||
use PhpParser\Node\Expr\BinaryOp\Identical;
|
||||
use PhpParser\Node\Expr\BinaryOp\NotIdentical;
|
||||
use PhpParser\Node\Stmt\Do_;
|
||||
use PhpParser\Node\Stmt\Else_;
|
||||
use PhpParser\Node\Stmt\If_;
|
||||
use PhpParser\Node\Stmt\Return_;
|
||||
use PhpParser\Node\Stmt\While_;
|
||||
use PhpParser\NodeTraverser;
|
||||
use Rector\PhpParser\NodeTraverser\CallableNodeTraverser;
|
||||
use Rector\PhpParser\Printer\BetterStandardPrinter;
|
||||
|
||||
final class IfManipulator
|
||||
|
@ -23,12 +29,19 @@ final class IfManipulator
|
|||
*/
|
||||
private $constFetchManipulator;
|
||||
|
||||
/**
|
||||
* @var CallableNodeTraverser
|
||||
*/
|
||||
private $callableNodeTraverser;
|
||||
|
||||
public function __construct(
|
||||
BetterStandardPrinter $betterStandardPrinter,
|
||||
ConstFetchManipulator $constFetchManipulator
|
||||
ConstFetchManipulator $constFetchManipulator,
|
||||
CallableNodeTraverser $callableNodeTraverser
|
||||
) {
|
||||
$this->betterStandardPrinter = $betterStandardPrinter;
|
||||
$this->constFetchManipulator = $constFetchManipulator;
|
||||
$this->callableNodeTraverser = $callableNodeTraverser;
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -98,6 +111,25 @@ final class IfManipulator
|
|||
return null;
|
||||
}
|
||||
|
||||
public function isWithElseAlwaysReturnValue(If_ $if): bool
|
||||
{
|
||||
if (! $this->isAlwaysReturnValue((array) $if->stmts)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
foreach ($if->elseifs as $elseif) {
|
||||
if (! $this->isAlwaysReturnValue((array) $elseif->stmts)) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
if ($if->else === null) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return $this->isAlwaysReturnValue((array) $if->else->stmts);
|
||||
}
|
||||
|
||||
private function matchComparedAndReturnedNode(NotIdentical $notIdentical, Return_ $returnNode): ?Expr
|
||||
{
|
||||
if ($this->betterStandardPrinter->areNodesEqual($notIdentical->left, $returnNode->expr)) {
|
||||
|
@ -114,4 +146,46 @@ final class IfManipulator
|
|||
|
||||
return null;
|
||||
}
|
||||
|
||||
private function isAlwaysReturnValue(array $stmts): bool
|
||||
{
|
||||
$isAlwaysReturnValue = false;
|
||||
|
||||
$this->callableNodeTraverser->traverseNodesWithCallable($stmts, function (Node $node) use (
|
||||
&$isAlwaysReturnValue
|
||||
) {
|
||||
if ($this->isScopeChangingNode($node)) {
|
||||
$isAlwaysReturnValue = false;
|
||||
|
||||
return NodeTraverser::STOP_TRAVERSAL;
|
||||
}
|
||||
|
||||
if ($node instanceof Return_) {
|
||||
if ($node->expr === null) {
|
||||
$isAlwaysReturnValue = false;
|
||||
|
||||
return NodeTraverser::STOP_TRAVERSAL;
|
||||
}
|
||||
|
||||
$isAlwaysReturnValue = true;
|
||||
}
|
||||
});
|
||||
|
||||
return $isAlwaysReturnValue;
|
||||
}
|
||||
|
||||
private function isScopeChangingNode(Node $node): bool
|
||||
{
|
||||
$scopeChangingNodes = [Do_::class, While_::class, If_::class, Else_::class];
|
||||
|
||||
foreach ($scopeChangingNodes as $scopeChangingNode) {
|
||||
if (! is_a($node, $scopeChangingNode, true)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue
Block a user