From 3df70bdd234efe4c6f49607df5378fb150b111e7 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Mon, 15 May 2023 14:57:00 +0200 Subject: [PATCH] Remove NEXT_NODE from RemoveDuplicatedIfReturnRector (#3858) Co-authored-by: GitHub Action --- .../bool_var_if_cond_return_true.php.inc | 31 -------------- .../Fixture/skip_param_reference.php.inc | 39 ++++++++++++++++++ .../RemoveDuplicatedIfReturnRector.php | 41 ++++--------------- 3 files changed, 47 insertions(+), 64 deletions(-) delete mode 100644 rules-tests/DeadCode/Rector/FunctionLike/RemoveDuplicatedIfReturnRector/Fixture/bool_var_if_cond_return_true.php.inc create mode 100644 rules-tests/DeadCode/Rector/FunctionLike/RemoveDuplicatedIfReturnRector/Fixture/skip_param_reference.php.inc diff --git a/rules-tests/DeadCode/Rector/FunctionLike/RemoveDuplicatedIfReturnRector/Fixture/bool_var_if_cond_return_true.php.inc b/rules-tests/DeadCode/Rector/FunctionLike/RemoveDuplicatedIfReturnRector/Fixture/bool_var_if_cond_return_true.php.inc deleted file mode 100644 index df86b0ad32f..00000000000 --- a/rules-tests/DeadCode/Rector/FunctionLike/RemoveDuplicatedIfReturnRector/Fixture/bool_var_if_cond_return_true.php.inc +++ /dev/null @@ -1,31 +0,0 @@ - ------ - diff --git a/rules-tests/DeadCode/Rector/FunctionLike/RemoveDuplicatedIfReturnRector/Fixture/skip_param_reference.php.inc b/rules-tests/DeadCode/Rector/FunctionLike/RemoveDuplicatedIfReturnRector/Fixture/skip_param_reference.php.inc new file mode 100644 index 00000000000..96f1902731f --- /dev/null +++ b/rules-tests/DeadCode/Rector/FunctionLike/RemoveDuplicatedIfReturnRector/Fixture/skip_param_reference.php.inc @@ -0,0 +1,39 @@ +simpleCallableNodeTraverser->traverseNodesWithCallable($stmt, function (Node $node) use ( + &$isParamUsed, + ): ?int { + if ($isParamUsed) { + return NodeTraverser::STOP_TRAVERSAL; + } + + // skip nested anonymous class + if ($node instanceof Class_ || $node instanceof Function_) { + $isParamUsed = true; + + return NodeTraverser::DONT_TRAVERSE_CURRENT_AND_CHILDREN; + } + + return null; + }); + } +} diff --git a/rules/DeadCode/Rector/FunctionLike/RemoveDuplicatedIfReturnRector.php b/rules/DeadCode/Rector/FunctionLike/RemoveDuplicatedIfReturnRector.php index d460bd25dda..edb6f94fb4d 100644 --- a/rules/DeadCode/Rector/FunctionLike/RemoveDuplicatedIfReturnRector.php +++ b/rules/DeadCode/Rector/FunctionLike/RemoveDuplicatedIfReturnRector.php @@ -5,7 +5,6 @@ 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; @@ -19,7 +18,6 @@ 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; @@ -99,11 +97,12 @@ public function refactor(Node $node): ?Node return null; } - $hasRemovedNode = false; + $hasChanged = false; foreach ($ifWithOnlyReturnsByHash as $ifWithOnlyReturns) { $isBool = $this->isBoolVarIfCondReturnTrueNextReturnBoolVar($ifWithOnlyReturns); - if (! $isBool && count($ifWithOnlyReturns) < 2) { + + if (count($ifWithOnlyReturns) < 2) { continue; } @@ -114,11 +113,11 @@ public function refactor(Node $node): ?Node foreach ($ifWithOnlyReturns as $ifWithOnlyReturn) { $this->removeNode($ifWithOnlyReturn); - $hasRemovedNode = true; + $hasChanged = true; } } - if ($hasRemovedNode) { + if ($hasChanged) { return $node; } @@ -134,42 +133,17 @@ private function isBoolVarIfCondReturnTrueNextReturnBoolVar(array $ifWithOnlyRet 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->isBoolean()->yes()) { - return false; - } - - $nextNode = $ifWithOnlyReturns[0]->getAttribute(AttributeKey::NEXT_NODE); - if (! $nextNode instanceof Return_) { - return false; - } - - $expr = $nextNode->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 $type->isBoolean()->yes(); } /** - * @return If_[][] + * @return array */ private function collectDuplicatedIfWithOnlyReturnByHash(FunctionLike $functionLike): array { @@ -183,6 +157,7 @@ private function collectDuplicatedIfWithOnlyReturnByHash(FunctionLike $functionL $modifiedVariableNames, $this->modifiedVariableNamesCollector->collectModifiedVariableNames($stmt) ); + continue; }