[DeadCode] Add SimplifyIfElseWithSameContentRector

This commit is contained in:
Tomas Votruba 2019-10-06 01:54:34 +02:00
parent 9310c37058
commit f25f303dc8
10 changed files with 276 additions and 0 deletions

View File

@ -29,3 +29,4 @@ services:
Rector\DeadCode\Rector\Property\RemoveNullPropertyInitializationRector: ~
Rector\DeadCode\Rector\Stmt\RemoveUnreachableStatementRector: ~
Rector\DeadCode\Rector\If_\SimplifyIfElseWithSameContentRector: ~

View File

@ -0,0 +1,93 @@
<?php declare(strict_types=1);
namespace Rector\DeadCode\Rector\If_;
use PhpParser\Node;
use PhpParser\Node\Stmt\If_;
use Rector\Rector\AbstractRector;
use Rector\RectorDefinition\CodeSample;
use Rector\RectorDefinition\RectorDefinition;
/**
* @see \Rector\DeadCode\Tests\Rector\If_\SimplifyIfElseWithSameContentRector\SimplifyIfElseWithSameContentRectorTest
*/
final class SimplifyIfElseWithSameContentRector extends AbstractRector
{
public function getDefinition(): RectorDefinition
{
return new RectorDefinition('Remove if/else if they have same content', [
new CodeSample(
<<<'PHP'
class SomeClass
{
public function run()
{
if (true) {
return 1;
} else {
return 1;
}
}
}
PHP
,
<<<'PHP'
class SomeClass
{
public function run()
{
return 1;
}
}
PHP
),
]);
}
/**
* @return string[]
*/
public function getNodeTypes(): array
{
return [If_::class];
}
/**
* @param If_ $node
*/
public function refactor(Node $node): ?Node
{
if ($node->else === null) {
return null;
}
if (! $this->isIfWithConstantReturns($node)) {
return null;
}
foreach ($node->stmts as $stmt) {
$this->addNodeBeforeNode($stmt, $node);
}
$this->removeNode($node);
return $node;
}
private function isIfWithConstantReturns(If_ $if): bool
{
$possibleContents = [];
$possibleContents[] = $this->print($if->stmts);
foreach ($if->elseifs as $elseif) {
$possibleContents[] = $this->print($elseif->stmts);
}
$possibleContents[] = $this->print($if->else->stmts);
$uniqueContents = array_unique($possibleContents);
// only one content for all
return count($uniqueContents) === 1;
}
}

View File

@ -0,0 +1,31 @@
<?php
namespace Rector\DeadCode\Tests\Rector\If_\SimplifyIfElseWithSameContentRector\Fixture;
class SomeClass
{
public function run()
{
if (true) {
return 1;
} else {
return 1;
}
}
}
?>
-----
<?php
namespace Rector\DeadCode\Tests\Rector\If_\SimplifyIfElseWithSameContentRector\Fixture;
class SomeClass
{
public function run()
{
return 1;
}
}
?>

View File

@ -0,0 +1,15 @@
<?php
namespace Rector\DeadCode\Tests\Rector\If_\SimplifyIfElseWithSameContentRector\Fixture;
class SkipDifferentContent
{
public function run()
{
if (true) {
return 1;
} else {
return 2;
}
}
}

View File

@ -0,0 +1,15 @@
<?php
namespace Rector\DeadCode\Tests\Rector\If_\SimplifyIfElseWithSameContentRector\Fixture;
class SkipElseWithNoReturn
{
public function go()
{
if (true) {
return 1;
} else {
$value = 10;
}
}
}

View File

@ -0,0 +1,15 @@
<?php
namespace Rector\DeadCode\Tests\Rector\If_\SimplifyIfElseWithSameContentRector\Fixture;
class SkipMissingElse
{
public function go()
{
if (true) {
return 1;
} elseif (false) {
return 1;
}
}
}

View File

@ -0,0 +1,16 @@
<?php
namespace Rector\DeadCode\Tests\Rector\If_\SimplifyIfElseWithSameContentRector\Fixture;
class SkipMoreThanReturn
{
public function go()
{
if (true) {
return 1;
} else {
$also = 5;
return 1;
}
}
}

View File

@ -0,0 +1,51 @@
<?php
namespace Rector\DeadCode\Tests\Rector\If_\SimplifyIfElseWithSameContentRector\Fixture;
class WithElseIfs
{
public function run()
{
if (true) {
return 1;
} elseif (false) {
return 1;
} else {
return 1;
}
}
public function go()
{
if (true) {
return 1;
} elseif (false) {
return 1;
}
}
}
?>
-----
<?php
namespace Rector\DeadCode\Tests\Rector\If_\SimplifyIfElseWithSameContentRector\Fixture;
class WithElseIfs
{
public function run()
{
return 1;
}
public function go()
{
if (true) {
return 1;
} elseif (false) {
return 1;
}
}
}
?>

View File

@ -0,0 +1,33 @@
<?php declare(strict_types=1);
namespace Rector\DeadCode\Tests\Rector\If_\SimplifyIfElseWithSameContentRector;
use Iterator;
use Rector\DeadCode\Rector\If_\SimplifyIfElseWithSameContentRector;
use Rector\Testing\PHPUnit\AbstractRectorTestCase;
final class SimplifyIfElseWithSameContentRectorTest extends AbstractRectorTestCase
{
/**
* @dataProvider provideDataForTest()
*/
public function test(string $file): void
{
$this->doTestFile($file);
}
public function provideDataForTest(): Iterator
{
yield [__DIR__ . '/Fixture/fixture.php.inc'];
yield [__DIR__ . '/Fixture/with_else_ifs.php.inc'];
yield [__DIR__ . '/Fixture/skip_different_content.php.inc'];
yield [__DIR__ . '/Fixture/skip_missing_else.php.inc'];
yield [__DIR__ . '/Fixture/skip_else_with_no_return.php.inc'];
yield [__DIR__ . '/Fixture/skip_more_than_return.php.inc'];
}
protected function getRectorClass(): string
{
return SimplifyIfElseWithSameContentRector::class;
}
}

View File

@ -88,6 +88,12 @@ final class NodeAddingCommander implements CommanderInterface
return spl_object_hash($node);
}
// special case for "If_"
$parentNode = $node->getAttribute(AttributeKey::CURRENT_EXPRESSION);
if ($parentNode === null) {
return spl_object_hash($node);
}
/** @var Expression|null $foundNode */
$foundNode = $this->betterNodeFinder->findFirstAncestorInstanceOf($node, Expression::class);
if ($foundNode === null) {