Skip to content

Commit 0b3f484

Browse files
committed
[DeadCode] Add RemoveDeadIfBlocksRector
Removes ifs, elseifs, and elses with no body. Combines them with others when appropriate. ```diff class RemoveDeadIfBlock { public function run($condition) { - if ($value) { - } elseif ($differentValue) { - if ($differentValue) { - } else { - } - if ($differentValue) { echo 'different'; - } elseif ($value) { - } else { } - if ($differentValue) { - } elseif ($value) { + if (!$differentValue && $value) { echo 'value'; } else { } return $differentValue; } } ``` We avoid merging `if`` into following conditions when the `if` itself has a comment, because we can't tell if the comment will still be correct for the negated condition. At first, I tried adding logic to RemoveDeadIfForeachForRector to remove elseifs and elses. That worked, but as I added code it was clunky to have so much unrelated `if` logic in `for` and `foreach` rector. So I moved it to a brand new rector with a simpler implementation. I borrowed significant code from RemoveAlwaysTrueIfConditionRector as well. This can serve as support for smarter rectors like "RemoveAlwaysTrueIf" and my WIP "RemoveAlwaysFalseIf". Properly modifying `if`s requires a lot of extra logic that complicates the implementation of those rectors. In the future, they can do something simpler like removing the body statements but leaving the if structure in place. Then this rector can perform the restructuring of the if.
1 parent 6602925 commit 0b3f484

12 files changed

+487
-0
lines changed
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
<?php
2+
3+
namespace Rector\Tests\DeadCode\Rector\If_\RemoveDeadIfBlocksRector\Fixture;
4+
5+
class RemoveDeadIfBlock
6+
{
7+
public function run($condition)
8+
{
9+
if ($value) {
10+
} elseif ($differentValue) {
11+
} else {
12+
}
13+
14+
if ($differentValue) {
15+
echo 'different';
16+
} elseif ($value) {
17+
} else {
18+
}
19+
20+
if ($differentValue) {
21+
} elseif ($value) {
22+
echo 'value';
23+
} else {
24+
}
25+
26+
return $differentValue;
27+
}
28+
}
29+
30+
?>
31+
-----
32+
<?php
33+
34+
namespace Rector\Tests\DeadCode\Rector\If_\RemoveDeadIfBlocksRector\Fixture;
35+
36+
class RemoveDeadIfBlock
37+
{
38+
public function run($condition)
39+
{
40+
if ($differentValue) {
41+
echo 'different';
42+
}
43+
44+
if (!$differentValue && $value) {
45+
echo 'value';
46+
}
47+
48+
return $differentValue;
49+
}
50+
}
51+
52+
?>
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
<?php
2+
3+
namespace Rector\Tests\DeadCode\Rector\If_\RemoveDeadIfBlocksRector\Fixture;
4+
5+
class IfElseNoStmt
6+
{
7+
public function run($condition)
8+
{
9+
if ($condition) {
10+
echo "something";
11+
} else {
12+
}
13+
}
14+
}
15+
16+
?>
17+
-----
18+
<?php
19+
20+
namespace Rector\Tests\DeadCode\Rector\If_\RemoveDeadIfBlocksRector\Fixture;
21+
22+
class IfElseNoStmt
23+
{
24+
public function run($condition)
25+
{
26+
if ($condition) {
27+
echo "something";
28+
}
29+
}
30+
}
31+
32+
?>
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
<?php
2+
3+
namespace Rector\Tests\DeadCode\Rector\If_\RemoveDeadIfBlocksRector\Fixture;
4+
5+
class IfElseNoStmt
6+
{
7+
public function run($condition)
8+
{
9+
if ($condition) {
10+
echo "something";
11+
} elseif (!$condition) {
12+
} elseif (true) {
13+
echo "global";
14+
}
15+
}
16+
}
17+
18+
?>
19+
-----
20+
<?php
21+
22+
namespace Rector\Tests\DeadCode\Rector\If_\RemoveDeadIfBlocksRector\Fixture;
23+
24+
class IfElseNoStmt
25+
{
26+
public function run($condition)
27+
{
28+
if ($condition) {
29+
echo "something";
30+
} elseif (true) {
31+
echo "global";
32+
}
33+
}
34+
}
35+
36+
?>
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
<?php
2+
3+
namespace Rector\Tests\DeadCode\Rector\If_\RemoveDeadIfBlocksRector\Fixture;
4+
5+
class IfNoStmtWithElse
6+
{
7+
public function run($condition)
8+
{
9+
if ($condition) {
10+
} else {
11+
echo "something";
12+
}
13+
}
14+
}
15+
16+
?>
17+
-----
18+
<?php
19+
20+
namespace Rector\Tests\DeadCode\Rector\If_\RemoveDeadIfBlocksRector\Fixture;
21+
22+
class IfNoStmtWithElse
23+
{
24+
public function run($condition)
25+
{
26+
if (!$condition) {
27+
echo "something";
28+
}
29+
}
30+
}
31+
32+
?>
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<?php
2+
3+
namespace Rector\Tests\DeadCode\Rector\If_\RemoveDeadIfBlocksRector\Fixture;
4+
5+
class IfNoStmtWithElseNoStmt
6+
{
7+
public function run($condition)
8+
{
9+
if ($condition) {
10+
} else {
11+
}
12+
}
13+
}
14+
15+
?>
16+
-----
17+
<?php
18+
19+
namespace Rector\Tests\DeadCode\Rector\If_\RemoveDeadIfBlocksRector\Fixture;
20+
21+
class IfNoStmtWithElseNoStmt
22+
{
23+
public function run($condition)
24+
{
25+
}
26+
}
27+
28+
?>
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
<?php
2+
3+
namespace Rector\Tests\DeadCode\Rector\If_\RemoveDeadIfBlocksRector\Fixture;
4+
5+
class SideEffectChecks
6+
{
7+
public function run()
8+
{
9+
if (10) {
10+
}
11+
12+
if (true) {
13+
}
14+
15+
if (5 + 10) {
16+
}
17+
}
18+
}
19+
20+
?>
21+
-----
22+
<?php
23+
24+
namespace Rector\Tests\DeadCode\Rector\If_\RemoveDeadIfBlocksRector\Fixture;
25+
26+
class SideEffectChecks
27+
{
28+
public function run()
29+
{
30+
}
31+
}
32+
33+
?>
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<?php
2+
3+
namespace Rector\Tests\DeadCode\Rector\If_\RemoveDeadIfBlocksRector\Fixture;
4+
5+
class MergeComments
6+
{
7+
public function run($b)
8+
{
9+
// toplevel
10+
if ($b) {
11+
} elseif (!$b) {
12+
// first innner
13+
f();
14+
// second inner
15+
}
16+
}
17+
}
18+
19+
?>
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<?php
2+
3+
namespace Rector\Tests\DeadCode\Rector\If_\RemoveDeadIfBlocksRector\Fixture;
4+
5+
class SkipUsedInNextStmt
6+
{
7+
public function run($resultLen, $result, $decimalSep)
8+
{
9+
for ($i = $resultLen - 1; $i >= 0 && $result[$i] === '0'; $i--) {
10+
;
11+
}
12+
13+
if ($i >= 0 && $result[$i] === $decimalSep) {
14+
$i--;
15+
}
16+
17+
$result = substr($result, 0, $i + 1);
18+
}
19+
}
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<?php
2+
3+
namespace Rector\Tests\DeadCode\Rector\If_\RemoveDeadIfBlocksRector\Fixture;
4+
5+
class SkipUsedInNextStmt2
6+
{
7+
public function run($resultLen, $result, $decimalSep)
8+
{
9+
foreach ($result as $i => $value) {
10+
;
11+
}
12+
13+
if ($i >= 0 && $result[$i] === $decimalSep) {
14+
$i--;
15+
}
16+
17+
$result = substr($result, 0, $i + 1);
18+
}
19+
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\Tests\DeadCode\Rector\If_\RemoveDeadIfBlocksRector;
6+
7+
use Iterator;
8+
use PHPUnit\Framework\Attributes\DataProvider;
9+
use Rector\Testing\PHPUnit\AbstractRectorTestCase;
10+
11+
final class RemoveDeadIfBlocksRectorTest extends AbstractRectorTestCase
12+
{
13+
#[DataProvider('provideData')]
14+
public function test(string $filePath): void
15+
{
16+
$this->doTestFile($filePath);
17+
}
18+
19+
public static function provideData(): Iterator
20+
{
21+
return self::yieldFilesFromDirectory(__DIR__ . '/Fixture');
22+
}
23+
24+
public function provideConfigFilePath(): string
25+
{
26+
return __DIR__ . '/config/configured_rule.php';
27+
}
28+
}

0 commit comments

Comments
 (0)