Skip to content

Commit

Permalink
[DeadCode] Handle always terminated TryCatch on RemoveUnreachableStat…
Browse files Browse the repository at this point in the history
…ementRector (#2545)

* [DeadCode] Handle always terminated TryCatch on RemoveUnreachableStatementRector

* implemented 🎉
  • Loading branch information
samsonasik committed Jun 21, 2022
1 parent a16bc1b commit 6c713a7
Show file tree
Hide file tree
Showing 5 changed files with 140 additions and 38 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<?php

namespace Rector\Tests\DeadCode\Rector\Stmt\RemoveUnreachableStatementRector\Fixture;

use Exception;

class AlwaysTerminatedTryCatch
{
public function run()
{
try {
return something();
} catch (Exception $e) {
return null;
}

echo 'never executed';
}
}

?>
-----
<?php

namespace Rector\Tests\DeadCode\Rector\Stmt\RemoveUnreachableStatementRector\Fixture;

use Exception;

class AlwaysTerminatedTryCatch
{
public function run()
{
try {
return something();
} catch (Exception $e) {
return null;
}
}
}

?>

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<?php

namespace Rector\Tests\DeadCode\Rector\Stmt\RemoveUnreachableStatementRector\Fixture;

class TryFinallyOnlyCommentStmt
{
public function setMultiple($values, $ttl = null): bool
{
try {
return true;
} finally {
// only comment
}

return false;
}
}

?>
-----
<?php

namespace Rector\Tests\DeadCode\Rector\Stmt\RemoveUnreachableStatementRector\Fixture;

class TryFinallyOnlyCommentStmt
{
public function setMultiple($values, $ttl = null): bool
{
try {
return true;
} finally {
// only comment
}
}
}

?>
27 changes: 6 additions & 21 deletions rules/DeadCode/Rector/Stmt/RemoveUnreachableStatementRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
use PhpParser\Node\Stmt\Break_;
use PhpParser\Node\Stmt\Continue_;
use PhpParser\Node\Stmt\Expression;
use PhpParser\Node\Stmt\Finally_;
use PhpParser\Node\Stmt\Goto_;
use PhpParser\Node\Stmt\InlineHTML;
use PhpParser\Node\Stmt\Label;
Expand All @@ -19,8 +18,8 @@
use PhpParser\Node\Stmt\Throw_;
use PhpParser\Node\Stmt\TryCatch;
use Rector\Core\Contract\PhpParser\Node\StmtsAwareInterface;
use Rector\Core\NodeAnalyzer\TryCatchAnalyzer;
use Rector\Core\Rector\AbstractRector;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;

Expand All @@ -31,6 +30,10 @@
*/
final class RemoveUnreachableStatementRector extends AbstractRector
{
public function __construct(private readonly TryCatchAnalyzer $tryCatchAnalyzer)
{
}

public function getRuleDefinition(): RuleDefinition
{
return new RuleDefinition('Remove unreachable statements', [
Expand Down Expand Up @@ -142,24 +145,6 @@ private function shouldRemove(Stmt $previousStmt, Stmt $currentStmt): bool
return false;
}

$isUnreachable = $currentStmt->getAttribute(AttributeKey::IS_UNREACHABLE);
if ($isUnreachable !== true) {
return false;
}

if (! $previousStmt->finally instanceof Finally_) {
return false;
}

return $this->cleanNop($previousStmt->finally->stmts) !== [];
}

/**
* @param Stmt[] $stmts
* @return Stmt[]
*/
private function cleanNop(array $stmts): array
{
return array_filter($stmts, fn (Stmt $stmt): bool => ! $stmt instanceof Nop);
return $this->tryCatchAnalyzer->isAlwaysTerminated($previousStmt);
}
}
56 changes: 56 additions & 0 deletions src/NodeAnalyzer/TryCatchAnalyzer.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
<?php

declare(strict_types=1);

namespace Rector\Core\NodeAnalyzer;

use PhpParser\Node;
use PhpParser\Node\Expr\Exit_;
use PhpParser\Node\Stmt;
use PhpParser\Node\Stmt\Expression;
use PhpParser\Node\Stmt\Finally_;
use PhpParser\Node\Stmt\Return_;
use PhpParser\Node\Stmt\Throw_;
use PhpParser\Node\Stmt\TryCatch;

final class TryCatchAnalyzer
{
/**
* @var array<class-string<Node>>
*/
private const TERMINATED_NODES = [Return_::class, Throw_::class];

public function isAlwaysTerminated(TryCatch $tryCatch): bool
{
if ($tryCatch->finally instanceof Finally_ && $this->isTerminated($tryCatch->finally->stmts)) {
return true;
}

foreach ($tryCatch->catches as $catch) {
if (! $this->isTerminated($catch->stmts)) {
return false;
}
}

return $this->isTerminated($tryCatch->stmts);
}

/**
* @param Stmt[] $stmts
*/
private function isTerminated(array $stmts): bool
{
if ($stmts === []) {
return false;
}

$lastKey = array_key_last($stmts);
$lastNode = $stmts[$lastKey];

if ($lastNode instanceof Expression) {
return $lastNode->expr instanceof Exit_;
}

return in_array($lastNode::class, self::TERMINATED_NODES, true);
}
}

0 comments on commit 6c713a7

Please sign in to comment.