From 5e5681d9edbb438b5782b45fde6e88650aa3843f Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Tue, 4 Apr 2023 00:15:55 +0700 Subject: [PATCH] [NodeRemover] Use return null after $this->removeNode() (#3558) * [NodeRemover] Use return null after $this->removeNode() * return node if it try to change next node before remove * update warning message --- .../Rector/Assign/RemoveUnusedVariableAssignRector.php | 2 +- .../Rector/ClassMethod/RemoveEmptyClassMethodRector.php | 2 +- .../Rector/ClassMethod/RemoveUnusedPrivateMethodRector.php | 2 +- rules/DeadCode/Rector/For_/RemoveDeadLoopRector.php | 2 +- .../Rector/MethodCall/RemoveEmptyMethodCallRector.php | 2 +- rules/DeadCode/Rector/TryCatch/RemoveDeadTryCatchRector.php | 2 +- .../Rector/Break_/BreakNotInLoopOrSwitchToReturnRector.php | 2 +- rules/Php73/Rector/FuncCall/ArrayKeyFirstLastRector.php | 1 + src/Rector/AbstractRector.php | 6 +++--- 9 files changed, 11 insertions(+), 10 deletions(-) diff --git a/rules/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector.php b/rules/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector.php index c794e903584..12fed8d6e84 100644 --- a/rules/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector.php +++ b/rules/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector.php @@ -109,7 +109,7 @@ public function refactor(Node $node): ?Node } $this->removeNode($node); - return $node; + return null; } private function cleanCastedExpr(Expr $expr): Expr diff --git a/rules/DeadCode/Rector/ClassMethod/RemoveEmptyClassMethodRector.php b/rules/DeadCode/Rector/ClassMethod/RemoveEmptyClassMethodRector.php index cfc1aee39cd..f8ff56de13b 100644 --- a/rules/DeadCode/Rector/ClassMethod/RemoveEmptyClassMethodRector.php +++ b/rules/DeadCode/Rector/ClassMethod/RemoveEmptyClassMethodRector.php @@ -93,7 +93,7 @@ public function refactor(Node $node): ?Node $this->removeNode($node); - return $node; + return null; } private function shouldSkipNonFinalNonPrivateClassMethod(Class_ $class, ClassMethod $classMethod): bool diff --git a/rules/DeadCode/Rector/ClassMethod/RemoveUnusedPrivateMethodRector.php b/rules/DeadCode/Rector/ClassMethod/RemoveUnusedPrivateMethodRector.php index 6a1c8fcfffb..9455cf18fb9 100644 --- a/rules/DeadCode/Rector/ClassMethod/RemoveUnusedPrivateMethodRector.php +++ b/rules/DeadCode/Rector/ClassMethod/RemoveUnusedPrivateMethodRector.php @@ -87,7 +87,7 @@ public function refactor(Node $node): ?Node $this->removeNode($node); - return $node; + return null; } private function shouldSkip(ClassMethod $classMethod): bool diff --git a/rules/DeadCode/Rector/For_/RemoveDeadLoopRector.php b/rules/DeadCode/Rector/For_/RemoveDeadLoopRector.php index e1c7f764290..c369d9d3f0e 100644 --- a/rules/DeadCode/Rector/For_/RemoveDeadLoopRector.php +++ b/rules/DeadCode/Rector/For_/RemoveDeadLoopRector.php @@ -66,6 +66,6 @@ public function refactor(Node $node): ?Node } $this->removeNode($node); - return $node; + return null; } } diff --git a/rules/DeadCode/Rector/MethodCall/RemoveEmptyMethodCallRector.php b/rules/DeadCode/Rector/MethodCall/RemoveEmptyMethodCallRector.php index cf1345633f1..e1843c99528 100644 --- a/rules/DeadCode/Rector/MethodCall/RemoveEmptyMethodCallRector.php +++ b/rules/DeadCode/Rector/MethodCall/RemoveEmptyMethodCallRector.php @@ -127,7 +127,7 @@ public function refactor(Node $node): ?Node $this->removeNode($node); - return $node; + return null; } private function resolveClassLike(MethodCall $methodCall): ?ClassLike diff --git a/rules/DeadCode/Rector/TryCatch/RemoveDeadTryCatchRector.php b/rules/DeadCode/Rector/TryCatch/RemoveDeadTryCatchRector.php index 15f36d24ef8..34917bfece8 100644 --- a/rules/DeadCode/Rector/TryCatch/RemoveDeadTryCatchRector.php +++ b/rules/DeadCode/Rector/TryCatch/RemoveDeadTryCatchRector.php @@ -73,7 +73,7 @@ public function refactor(Node $node): array|null|TryCatch if ($this->isEmpty($node->stmts)) { $this->removeNode($node); - return $node; + return null; } if (count($node->catches) !== 1) { diff --git a/rules/Php70/Rector/Break_/BreakNotInLoopOrSwitchToReturnRector.php b/rules/Php70/Rector/Break_/BreakNotInLoopOrSwitchToReturnRector.php index c348a8db3c7..2bb88aad214 100644 --- a/rules/Php70/Rector/Break_/BreakNotInLoopOrSwitchToReturnRector.php +++ b/rules/Php70/Rector/Break_/BreakNotInLoopOrSwitchToReturnRector.php @@ -97,6 +97,6 @@ public function refactor(Node $node): ?Node $this->removeNode($node); - return $node; + return null; } } diff --git a/rules/Php73/Rector/FuncCall/ArrayKeyFirstLastRector.php b/rules/Php73/Rector/FuncCall/ArrayKeyFirstLastRector.php index 9c58d6fccbf..9ee95c72378 100644 --- a/rules/Php73/Rector/FuncCall/ArrayKeyFirstLastRector.php +++ b/rules/Php73/Rector/FuncCall/ArrayKeyFirstLastRector.php @@ -130,6 +130,7 @@ public function refactor(Node $node): ?Node $this->removeNode($node); + // change next node before remove, so it needs to return the Node return $node; } diff --git a/src/Rector/AbstractRector.php b/src/Rector/AbstractRector.php index 5a93bcfe9bf..db909d52cc5 100644 --- a/src/Rector/AbstractRector.php +++ b/src/Rector/AbstractRector.php @@ -52,14 +52,14 @@ abstract class AbstractRector extends NodeVisitorAbstract implements PhpRectorIn private const EMPTY_NODE_ARRAY_MESSAGE = <<refactor()" returns non-empty array for Nodes. -A) Return null for no change: +A) Direct return null for no change: return null; B) Remove the Node: \$this->removeNode(\$node); - return \$node; + return null; CODE_SAMPLE; protected NodeNameResolver $nodeNameResolver; @@ -210,7 +210,7 @@ final public function enterNode(Node $node) $refactoredNode = $this->refactor($node); - // nothing to change → continue + // nothing to change or just removed via removeNode() → continue if ($refactoredNode === null) { return null; }