Skip to content

Commit

Permalink
[DeadCode] Handle bool var if cond return true next return bool var o…
Browse files Browse the repository at this point in the history
…n RemoveDuplicatedIfReturnRector (#1136)

Co-authored-by: GitHub Action <action@github.com>
  • Loading branch information
samsonasik and actions-user committed Nov 3, 2021
1 parent c86a6ea commit 37ade3f
Show file tree
Hide file tree
Showing 4 changed files with 119 additions and 12 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?php

namespace Rector\Tests\DeadCode\Rector\FunctionLike\RemoveDuplicatedIfReturnRector\Fixture;

class BoolVarIfCondReturnTrueNextReturnBoolVar
{
public function run(bool $value)
{
if ($value) {
return true;
}

return $value;
}
}

?>
-----
<?php

namespace Rector\Tests\DeadCode\Rector\FunctionLike\RemoveDuplicatedIfReturnRector\Fixture;

class BoolVarIfCondReturnTrueNextReturnBoolVar
{
public function run(bool $value)
{
return $value;
}
}

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

namespace Rector\Tests\DeadCode\Rector\FunctionLike\RemoveDuplicatedIfReturnRector\Fixture;

class SkipNegationIfCondReturnReturnTrueNextReturnCond
{
public function run(bool $value)
{
if (! $value) {
return true;
}

return $value;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<?php

namespace Rector\Tests\DeadCode\Rector\FunctionLike\RemoveDuplicatedIfReturnRector\Fixture;

class SkipNonBoolIfCond
{
public function run(stdClass $value)
{
if ($value) {
return true;
}

return $value;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,22 @@
namespace Rector\DeadCode\Rector\FunctionLike;

use PhpParser\Node;
use PhpParser\Node\Expr;
use PhpParser\Node\Expr\PropertyFetch;
use PhpParser\Node\Expr\StaticPropertyFetch;
use PhpParser\Node\Expr\Variable;
use PhpParser\Node\FunctionLike;
use PhpParser\Node\Stmt;
use PhpParser\Node\Stmt\If_;
use PhpParser\Node\Stmt\Return_;
use PhpParser\NodeTraverser;
use PHPStan\Type\BooleanType;
use Rector\Core\NodeAnalyzer\PropertyFetchAnalyzer;
use Rector\Core\NodeManipulator\IfManipulator;
use Rector\Core\PhpParser\Comparing\NodeComparator;
use Rector\Core\Rector\AbstractRector;
use Rector\DeadCode\NodeCollector\ModifiedVariableNamesCollector;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;

Expand Down Expand Up @@ -96,8 +101,15 @@ public function refactor(Node $node): ?Node
}

foreach ($ifWithOnlyReturnsByHash as $ifWithOnlyReturns) {
// keep first one
array_shift($ifWithOnlyReturns);
$isBool = $this->isBoolVarIfCondReturnTrueNextReturnBoolVar($ifWithOnlyReturns);
if (! $isBool && count($ifWithOnlyReturns) < 2) {
continue;
}

if (! $isBool) {
// keep first one
array_shift($ifWithOnlyReturns);
}

foreach ($ifWithOnlyReturns as $ifWithOnlyReturn) {
$this->removeNode($ifWithOnlyReturn);
Expand All @@ -107,6 +119,49 @@ public function refactor(Node $node): ?Node
return $node;
}

/**
* @param If_[] $ifWithOnlyReturns
*/
private function isBoolVarIfCondReturnTrueNextReturnBoolVar(array $ifWithOnlyReturns): bool
{
if (count($ifWithOnlyReturns) > 1) {
return false;
}

/** @var Expr $cond */
$cond = $ifWithOnlyReturns[0]->cond;
if (! in_array($cond::class, [Variable::class, PropertyFetch::class, StaticPropertyFetch::class], true)) {
return false;
}

$type = $this->nodeTypeResolver->getType($cond);
if (! $type instanceof BooleanType) {
return false;
}

$next = $ifWithOnlyReturns[0]->getAttribute(AttributeKey::NEXT_NODE);
if (! $next instanceof Return_) {
return false;
}

$expr = $next->expr;
if (! $expr instanceof Expr) {
return false;
}

if (! $this->nodeComparator->areNodesEqual($expr, $cond)) {
return false;
}

/** @var Return_ $returnStmt */
$returnStmt = $ifWithOnlyReturns[0]->stmts[0];
if (! $returnStmt->expr instanceof Expr) {
return false;
}

return $this->valueResolver->isValue($returnStmt->expr, true);
}

/**
* @return If_[][]
*/
Expand Down Expand Up @@ -142,7 +197,7 @@ private function collectDuplicatedIfWithOnlyReturnByHash(FunctionLike $functionL
$ifWithOnlyReturnsByHash[$hash][] = $stmt;
}

return $this->filterOutSingleItemStmts($ifWithOnlyReturnsByHash);
return $ifWithOnlyReturnsByHash;
}

/**
Expand Down Expand Up @@ -174,13 +229,4 @@ private function containsVariableNames(Stmt $stmt, array $modifiedVariableNames)

return $containsVariableNames;
}

/**
* @param array<string, If_[]> $ifWithOnlyReturnsByHash
* @return array<string, If_[]>
*/
private function filterOutSingleItemStmts(array $ifWithOnlyReturnsByHash): array
{
return array_filter($ifWithOnlyReturnsByHash, fn (array $stmts): bool => count($stmts) >= 2);
}
}

0 comments on commit 37ade3f

Please sign in to comment.