octoleo/rector
1
1
Fork 0
mirror of https://github.com/rectorphp/rector.git synced 2024-06-29 06:03:30 +00:00
Code Issues Packages Projects Releases Wiki Activity

[DeadCode] Add RemoveDuplicatedIfReturnRector

Browse Source
This commit is contained in:
TomasVotruba 2020-02-27 23:35:25 +01:00
parent ff147216b7
commit e29f8fafab
10 changed files with 369 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
config/set/dead-code/dead-code.yaml
View File

@ -34,3 +34,4 @@ services:
Rector\DeadCode\Rector\TryCatch\RemoveDeadTryCatchRector: null
Rector\DeadCode\Rector\ClassConst\RemoveUnusedClassConstantRector: null
Rector\DeadCode\Rector\Assign\RemoveUnusedVariableAssignRector: null
Rector\DeadCode\Rector\FunctionLike\RemoveDuplicatedIfReturnRector: null

29
docs/AllRectorsOverview.md
View File

@ -1,4 +1,4 @@
# All 463 Rectors Overview
# All 464 Rectors Overview
- [Projects](#projects)
- [General](#general)
@ -2791,6 +2791,33 @@ Remove duplicated key in defined arrays.
<br>
### `RemoveDuplicatedIfReturnRector`
- class: [`Rector\DeadCode\Rector\FunctionLike\RemoveDuplicatedIfReturnRector`](/../master/rules/dead-code/src/Rector/FunctionLike/RemoveDuplicatedIfReturnRector.php)
- [test fixtures](/../master/rules/dead-code/tests/Rector/FunctionLike/RemoveDuplicatedIfReturnRector/Fixture)
Remove duplicated if stmt with return in function/method body
```diff
class SomeClass
{
public function run($value)
{
if ($value) {
return true;
}
$value2 = 100;
-
- if ($value) {
- return true;
- }
}
}
```
<br>
### `RemoveDuplicatedInstanceOfRector`
- class: [`Rector\DeadCode\Rector\Instanceof_\RemoveDuplicatedInstanceOfRector`](/../master/rules/dead-code/src/Rector/Instanceof_/RemoveDuplicatedInstanceOfRector.php)

200
rules/dead-code/src/Rector/FunctionLike/RemoveDuplicatedIfReturnRector.php Normal file
View File

@ -0,0 +1,200 @@
<?php
declare(strict_types=1);
namespace Rector\DeadCode\Rector\FunctionLike;
use PhpParser\Node;
use PhpParser\Node\Expr\Assign;
use PhpParser\Node\Expr\Variable;
use PhpParser\Node\FunctionLike;
use PhpParser\Node\Stmt\If_;
use PhpParser\NodeTraverser;
use Rector\Core\PhpParser\Node\Manipulator\IfManipulator;
use Rector\Core\Rector\AbstractRector;
use Rector\Core\RectorDefinition\CodeSample;
use Rector\Core\RectorDefinition\RectorDefinition;
/**
* @see https://github.com/rectorphp/rector/issues/2945
*
* @see \Rector\DeadCode\Tests\Rector\FunctionLike\RemoveDuplicatedIfReturnRector\RemoveDuplicatedIfReturnRectorTest
*/
final class RemoveDuplicatedIfReturnRector extends AbstractRector
{
/**
* @var IfManipulator
*/
private $ifManipulator;
public function __construct(IfManipulator $ifManipulator)
{
$this->ifManipulator = $ifManipulator;
}
public function getDefinition(): RectorDefinition
{
return new RectorDefinition('Remove duplicated if stmt with return in function/method body', [
new CodeSample(
<<<'PHP'
class SomeClass
{
public function run($value)
{
if ($value) {
return true;
}
$value2 = 100;
if ($value) {
return true;
}
}
}
PHP
,
<<<'PHP'
class SomeClass
{
public function run($value)
{
if ($value) {
return true;
}
$value2 = 100;
}
}
PHP
),
]);
}
/**
* @return string[]
*/
public function getNodeTypes(): array
{
return [FunctionLike::class];
}
/**
* @param FunctionLike $node
*/
public function refactor(Node $node): ?Node
{
$ifWithOnlyReturnsByHash = $this->collectDuplicatedIfWithOnlyReturnByHash($node);
if ($ifWithOnlyReturnsByHash === []) {
return null;
}
foreach ($ifWithOnlyReturnsByHash as $stmts) {
// keep first one
array_shift($stmts);
foreach ($stmts as $stmt) {
$this->removeNode($stmt);
}
}
return $node;
}
/**
* @return If_[][]
*/
private function collectDuplicatedIfWithOnlyReturnByHash(FunctionLike $functionLike): array
{
$ifWithOnlyReturnsByHash = [];
$modifiedVariableNames = [];
foreach ((array) $functionLike->getStmts() as $stmt) {
if (! $this->ifManipulator->isIfWithOnlyReturn($stmt)) {
// variable modification
$modifiedVariableNames += $this->collectModifiedVariableNames($stmt);
continue;
}
if ($this->containsVariableNames($stmt, $modifiedVariableNames)) {
continue;
}
/** @var If_ $stmt */
$hash = $this->printWithoutComments($stmt);
$ifWithOnlyReturnsByHash[$hash][] = $stmt;
}
return $this->filterOutSingleItemStmts($ifWithOnlyReturnsByHash);
}
/**
* @param If_[][] $ifWithOnlyReturnsByHash
* @return If_[][]
*/
private function filterOutSingleItemStmts(array $ifWithOnlyReturnsByHash): array
{
return array_filter($ifWithOnlyReturnsByHash, function (array $stmts) {
return count($stmts) >= 2;
});
}
/**
* @return string[]
*/
private function collectModifiedVariableNames(Node $node): array
{
$modifiedVariableNames = [];
$this->traverseNodesWithCallable($node, function (Node $node) use (&$modifiedVariableNames) {
if (! $node instanceof Assign) {
return null;
}
if (! $node->var instanceof Variable) {
return null;
}
$variableName = $this->getName($node->var);
if ($variableName === null) {
return null;
}
$modifiedVariableNames[] = $variableName;
});
return $modifiedVariableNames;
}
/**
* @param string[] $modifiedVariableNames
*/
private function containsVariableNames(Node $node, array $modifiedVariableNames): bool
{
if ($modifiedVariableNames === []) {
return false;
}
$containsVariableNames = false;
$this->traverseNodesWithCallable($node, function (Node $node) use (
$modifiedVariableNames,
&$containsVariableNames
) {
if (! $node instanceof Variable) {
return null;
}
if (! $this->isNames($node, $modifiedVariableNames)) {
return null;
}
$containsVariableNames = true;
return NodeTraverser::STOP_TRAVERSAL;
});
return $containsVariableNames;
}
}

39
rules/dead-code/tests/Rector/FunctionLike/RemoveDuplicatedIfReturnRector/Fixture/fixture.php.inc Normal file
View File

@ -0,0 +1,39 @@
<?php
namespace Rector\DeadCode\Tests\Rector\FunctionLike\RemoveDuplicatedIfReturnRector\Fixture;
class SomeClass
{
public function run($value)
{
if ($value) {
return true;
}
$value2 = 100;
if ($value) {
return true;
}
}
}
?>
-----
<?php
namespace Rector\DeadCode\Tests\Rector\FunctionLike\RemoveDuplicatedIfReturnRector\Fixture;
class SomeClass
{
public function run($value)
{
if ($value) {
return true;
}
$value2 = 100;
}
}
?>

19
rules/dead-code/tests/Rector/FunctionLike/RemoveDuplicatedIfReturnRector/Fixture/skip_modified_value.php.inc Normal file
View File

@ -0,0 +1,19 @@
<?php
namespace Rector\DeadCode\Tests\Rector\FunctionLike\RemoveDuplicatedIfReturnRector\Fixture;
class SkipModifiedValue
{
public function run()
{
$value = random_int(1, 5);
if ($value) {
return true;
}
$value = random_int(1, 5);
if ($value) {
return true;
}
}
}

19
rules/dead-code/tests/Rector/FunctionLike/RemoveDuplicatedIfReturnRector/Fixture/skip_this.php.inc Normal file
View File

@ -0,0 +1,19 @@
<?php
namespace Rector\DeadCode\Tests\Rector\FunctionLike\RemoveDuplicatedIfReturnRector\Fixture;
class SkipThis
{
public function run($value)
{
if ($value) {
return true;
}
$value2 = 100;
if ($value2) {
return true;
}
}
}

19
rules/dead-code/tests/Rector/FunctionLike/RemoveDuplicatedIfReturnRector/Fixture/skip_with_differnt_value.php.inc Normal file
View File

@ -0,0 +1,19 @@
<?php
namespace Rector\DeadCode\Tests\Rector\FunctionLike\RemoveDuplicatedIfReturnRector\Fixture;
class SkipWithDifferentValue
{
public function run($value)
{
// is yml/yaml file
if (Strings::match($possibleFileNodeAsString, '#\.(yml|yaml)(\'|")$#')) {
return true;
}
// is probably a file variable
if (Strings::match($possibleFileNodeAsString, '#\File$#')) {
return true;
}
}
}

30
rules/dead-code/tests/Rector/FunctionLike/RemoveDuplicatedIfReturnRector/RemoveDuplicatedIfReturnRectorTest.php Normal file
View File

@ -0,0 +1,30 @@
<?php
declare(strict_types=1);
namespace Rector\DeadCode\Tests\Rector\FunctionLike\RemoveDuplicatedIfReturnRector;
use Iterator;
use Rector\Core\Testing\PHPUnit\AbstractRectorTestCase;
use Rector\DeadCode\Rector\FunctionLike\RemoveDuplicatedIfReturnRector;
final class RemoveDuplicatedIfReturnRectorTest extends AbstractRectorTestCase
{
/**
* @dataProvider provideData()
*/
public function test(string $file): void
{
$this->doTestFile($file);
}
public function provideData(): Iterator
{
return $this->yieldFilesFromDirectory(__DIR__ . '/Fixture');
}
protected function getRectorClass(): string
{
return RemoveDuplicatedIfReturnRector::class;
}
}

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

@ -263,6 +263,19 @@ final class IfManipulator
return $ifs;
}
public function isIfWithOnlyReturn(Node $node): bool
{
if (! $node instanceof If_) {
return false;
}
if (! $this->isIfWithoutElseAndElseIfs($node)) {
return false;
}
return $this->hasOnlyStmtOfType($node, Return_::class);
}
private function matchComparedAndReturnedNode(NotIdentical $notIdentical, Return_ $returnNode): ?Expr
{
if ($this->betterStandardPrinter->areNodesEqual(

2
src/PhpParser/Printer/BetterStandardPrinter.php
View File

@ -376,7 +376,7 @@ final class BetterStandardPrinter extends Standard
$printerNode = Strings::replace($printerNode, '#\/*\*(.*?)\*\/#');
// remove # ...
$printerNode = Strings::replace($printerNode, '#\#(.*?)$#m');
$printerNode = Strings::replace($printerNode, '#^(\s+)?\#(.*?)$#m');
// remove // ...
return Strings::replace($printerNode, '#\/\/(.*?)$#m');