mirror_php/rector
1
0
Fork 0
mirror of https://github.com/rectorphp/rector.git synced 2025-01-17 21:38:22 +01:00
Code Issues Projects Releases Wiki Activity

Added more logic for if statement splitting

Browse Source
This commit is contained in:
Jeroen Smit 2020-01-15 21:13:29 +01:00
parent ed868e61fc
commit 4d986dd23c
11 changed files with 227 additions and 125 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
docs/AllRectorsOverview.md
View File

@ -7148,7 +7148,7 @@ Finalize every class constant that is used only locally
- class: `Rector\SOLID\Rector\If_\RemoveAlwaysElseRector`
Remove if for last else, if previous values were throw
Split if statement, when if condition always break execution flow
```diff
class SomeClass

54
packages/SOLID/src/Rector/If_/RemoveAlwaysElseRector.php
View File

@ -5,30 +5,25 @@ declare(strict_types=1);
namespace Rector\SOLID\Rector\If_;
use PhpParser\Node;
use PhpParser\Node\Expr\Exit_;
use PhpParser\Node\Stmt\Continue_;
use PhpParser\Node\Stmt\ElseIf_;
use PhpParser\Node\Stmt\Expression;
use PhpParser\Node\Stmt\If_;
use Rector\PhpParser\Node\Manipulator\IfManipulator;
use PhpParser\Node\Stmt\Return_;
use PhpParser\Node\Stmt\Throw_;
use Rector\Rector\AbstractRector;
use Rector\RectorDefinition\CodeSample;
use Rector\RectorDefinition\RectorDefinition;
/**
* @see \Rector\SOLID\Tests\Rector\If_\RemoveAlwaysElseRector\RemoveAlwaysElseRectorTest
* @see \Rector\SOLID\Tests\Rector\If_\RemoveAlwaysElse\SplitIfsRectorTest
*/
final class RemoveAlwaysElseRector 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 throw', [
return new RectorDefinition('Split if statement, when if condition always break execution flow', [
new CodeSample(
<<<'PHP'
class SomeClass
@ -75,16 +70,37 @@ PHP
*/
public function refactor(Node $node): ?Node
{
if (! $this->ifManipulator->isEarlyElse($node)) {
if ($this->lastStatementBreaksFlow($node)) {
return null;
}
foreach ($node->else->stmts as $elseStmt) {
$this->addNodeAfterNode($elseStmt, $node);
if ($node->elseifs !== []) {
$newNode = new If_($node->cond);
$newNode->stmts = $node->stmts;
$this->addNodeBeforeNode($newNode, $node);
/** @var ElseIf_ $firstElseIf */
$firstElseIf = array_shift($node->elseifs);
$node->cond = $firstElseIf->cond;
$node->stmts = $firstElseIf->stmts;
return $node;
}
$node->else = null;
if ($node->else !== null) {
foreach ($node->else->stmts as $stmt) {
$this->addNodeAfterNode($stmt, $node);
}
$node->else = null;
return $node;
}
return $node;
return null;
}
protected function lastStatementBreaksFlow(Node $node): bool
{
$lastStmt = end($node->stmts);
return ! ($lastStmt instanceof Return_
|| $lastStmt instanceof Throw_
|| $lastStmt instanceof Continue_
|| ($lastStmt instanceof Expression && $lastStmt->expr instanceof Exit_));
}
}

44
packages/SOLID/tests/Rector/If_/RemoveAlwaysElseRector/Fixture/elseif.php.inc Normal file
View File

@ -0,0 +1,44 @@
<?php
namespace Rector\SOLID\Tests\Rector\If_\RemoveAlwaysElse\ElseIf_;
class SomeClass
{
public function run()
{
if ($cond1) {
return 'foo';
} elseif ($cond2) {
bar();
} elseif ($cond3) {
baz();
} else {
foo();
}
}
}
?>
-----
<?php
namespace Rector\SOLID\Tests\Rector\If_\RemoveAlwaysElse\ElseIf_;
class SomeClass
{
public function run()
{
if ($cond1) {
return 'foo';
}
if ($cond2) {
bar();
} elseif ($cond3) {
baz();
} else {
foo();
}
}
}
?>

17
packages/SOLID/tests/Rector/If_/RemoveAlwaysElseRector/Fixture/empty_if.php.inc Normal file
View File

@ -0,0 +1,17 @@
<?php
namespace Rector\SOLID\Tests\Rector\If_\RemoveAlwaysElse\EmptyIf;
class SomeClass {
public function run()
{
if ($cond1) {
} else {
foo();
return 'bar';
}
}
}
?>

34
packages/SOLID/tests/Rector/If_/RemoveAlwaysElseRector/Fixture/exit.php.inc Normal file
View File

@ -0,0 +1,34 @@
<?php
namespace Rector\SOLID\Tests\Rector\If_\RemoveAlwaysElse\Exit_;
class SomeClass {
public function run()
{
if ($cond1) {
exit('bye');
} else {
foo();
return 'bar';
}
}
}
?>
-----
<?php
namespace Rector\SOLID\Tests\Rector\If_\RemoveAlwaysElse\Exit_;
class SomeClass {
public function run()
{
if ($cond1) {
exit('bye');
}
foo();
return 'bar';
}
}
?>

16
packages/SOLID/tests/Rector/If_/RemoveAlwaysElseRector/Fixture/no_break.php.inc Normal file
View File

@ -0,0 +1,16 @@
<?php
namespace Rector\SOLID\Tests\Rector\If_\RemoveAlwaysElse\NoBreak;
class SomeClass {
public function run()
{
if ($cond1) {
foo();
} else {
return 'bar';
}
}
}
?>

35
packages/SOLID/tests/Rector/If_/RemoveAlwaysElseRector/Fixture/process_empty_return_last.php.inc
View File

@ -4,11 +4,24 @@ namespace Rector\SOLID\Tests\Rector\If_\RemoveAlwaysElse\Fixture;
class ProcessEmptyReturnLast
{
public function runAgain($value)
public function firstRun($value)
{
if ($value) {
return 5;
} elseif ($value- 1) {
} elseif ($value - 1) {
$value = 55;
return 10;
} else {
return;
}
}
public function secondRun($value)
{
if ($value) {
return 5;
}
if ($value - 1) {
$value = 55;
return 10;
} else {
@ -25,11 +38,25 @@ namespace Rector\SOLID\Tests\Rector\If_\RemoveAlwaysElse\Fixture;
class ProcessEmptyReturnLast
{
public function runAgain($value)
public function firstRun($value)
{
if ($value) {
return 5;
} elseif ($value- 1) {
}
if ($value - 1) {
$value = 55;
return 10;
} else {
return;
}
}
public function secondRun($value)
{
if ($value) {
return 5;
}
if ($value - 1) {
$value = 55;
return 10;
}

15
packages/SOLID/tests/Rector/If_/RemoveAlwaysElseRector/Fixture/simple_if.php.inc Normal file
View File

@ -0,0 +1,15 @@
<?php
namespace Rector\SOLID\Tests\Rector\If_\RemoveAlwaysElse\SimpleIf;
class SomeClass
{
public function run()
{
if ($cond1) {
return 'foo';
}
}
}
?>

11
packages/SOLID/tests/Rector/If_/RemoveAlwaysElseRector/Fixture/skip_not_only_else.php.inc
View File

@ -4,17 +4,6 @@ namespace Rector\SOLID\Tests\Rector\If_\RemoveAlwaysElse\Fixture;
class SkipNotOnlyElse
{
public function run($value)
{
if ($value) {
return 5;
} elseif ($value- 1) {
$value = 55;
return;
} else {
return 10;
}
}
public function runAgainAndAgain($value)
{

34
packages/SOLID/tests/Rector/If_/RemoveAlwaysElseRector/Fixture/throw.php.inc Normal file
View File

@ -0,0 +1,34 @@
<?php
namespace Rector\SOLID\Tests\Rector\If_\RemoveAlwaysElse\Throw_;
class SomeClass {
public function run()
{
if ($cond1) {
throw new \Exception('should not happen');
} else {
foo();
return 'bar';
}
}
}
?>
-----
<?php
namespace Rector\SOLID\Tests\Rector\If_\RemoveAlwaysElse\Throw_;
class SomeClass {
public function run()
{
if ($cond1) {
throw new \Exception('should not happen');
}
foo();
return 'bar';
}
}
?>

90
src/PhpParser/Node/Manipulator/IfManipulator.php
View File

@ -11,30 +11,13 @@ use PhpParser\Node\Expr\BinaryOp\Identical;
use PhpParser\Node\Expr\BinaryOp\NotIdentical;
use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Expr\Variable;
use PhpParser\Node\Stmt\Continue_;
use PhpParser\Node\Stmt\Do_;
use PhpParser\Node\Stmt\Else_;
use PhpParser\Node\Stmt\If_;
use PhpParser\Node\Stmt\Return_;
use PhpParser\Node\Stmt\Throw_;
use PhpParser\Node\Stmt\While_;
use PhpParser\NodeTraverser;
use Rector\PhpParser\Node\Resolver\NameResolver;
use Rector\PhpParser\NodeTraverser\CallableNodeTraverser;
use Rector\PhpParser\Printer\BetterStandardPrinter;
final class IfManipulator
{
/**
* @var string[]
*/
private const ALLOWED_BREAKING_NODE_TYPES = [Return_::class, Throw_::class, Continue_::class];
/**
* @var string[]
*/
private const SCOPE_CHANGING_NODE_TYPES = [Do_::class, While_::class, If_::class, Else_::class];
/**
* @var BetterStandardPrinter
*/
@ -45,11 +28,6 @@ final class IfManipulator
*/
private $constFetchManipulator;
/**
* @var CallableNodeTraverser
*/
private $callableNodeTraverser;
/**
* @var StmtsManipulator
*/
@ -63,13 +41,11 @@ final class IfManipulator
public function __construct(
BetterStandardPrinter $betterStandardPrinter,
ConstFetchManipulator $constFetchManipulator,
CallableNodeTraverser $callableNodeTraverser,
StmtsManipulator $stmtsManipulator,
NameResolver $nameResolver
) {
$this->betterStandardPrinter = $betterStandardPrinter;
$this->constFetchManipulator = $constFetchManipulator;
$this->callableNodeTraverser = $callableNodeTraverser;
$this->stmtsManipulator = $stmtsManipulator;
$this->nameResolver = $nameResolver;
}
@ -150,21 +126,6 @@ final class IfManipulator
return $this->hasOnlyStmtOfType($if, If_::class);
}
public function isEarlyElse(If_ $if): bool
{
if (! $this->isAlwaysAllowedType((array) $if->stmts, self::ALLOWED_BREAKING_NODE_TYPES)) {
return false;
}
foreach ($if->elseifs as $elseif) {
if (! $this->isAlwaysAllowedType((array) $elseif->stmts, self::ALLOWED_BREAKING_NODE_TYPES)) {
return false;
}
}
return $if->else !== null;
}
public function hasOnlyStmtOfType(If_ $if, string $desiredType): bool
{
if (count($if->stmts) !== 1) {
@ -283,57 +244,6 @@ final class IfManipulator
return null;
}
/**
* @param Node[] $stmts
* @param string[] $allowedTypes
*/
private function isAlwaysAllowedType(array $stmts, array $allowedTypes): bool
{
$isAlwaysReturnValue = false;
$this->callableNodeTraverser->traverseNodesWithCallable($stmts, function (Node $node) use (
&$isAlwaysReturnValue,
$allowedTypes
) {
if ($this->isScopeChangingNode($node)) {
$isAlwaysReturnValue = false;
return NodeTraverser::STOP_TRAVERSAL;
}
foreach ($allowedTypes as $allowedType) {
if (is_a($node, $allowedType, true)) {
if ($allowedType === Return_::class) {
if ($node->expr === null) {
$isAlwaysReturnValue = false;
return NodeTraverser::STOP_TRAVERSAL;
}
}
$isAlwaysReturnValue = true;
}
}
return null;
});
return $isAlwaysReturnValue;
}
private function isScopeChangingNode(Node $node): bool
{
foreach (self::SCOPE_CHANGING_NODE_TYPES as $scopeChangingNode) {
if (! is_a($node, $scopeChangingNode, true)) {
continue;
}
return true;
}
return false;
}
private function isIfWithoutElseAndElseIfs(If_ $if): bool
{
if ($if->else !== null) {