From e36a67feaf46f4afc0f551c3be01503c0f7ad03f Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Thu, 12 Oct 2023 08:34:30 +0700 Subject: [PATCH 01/10] [DeadCode] Add RemoveUselessReturnExprInConstruct --- .../docs/rector_rules_overview.md | 28 ++++- .../Fixture/return_expr_in_construct.php.inc | 37 ++++++ .../Fixture/skip_abstract_method.php.inc | 8 ++ .../Fixture/skip_not_construct.php.inc | 11 ++ .../Fixture/skip_return_no_expr.php.inc | 16 +++ ...RemoveUselessReturnExprInConstructTest.php | 28 +++++ .../config/configured_rule.php | 10 ++ .../RemoveUselessReturnExprInConstruct.php | 107 ++++++++++++++++++ 8 files changed, 243 insertions(+), 2 deletions(-) create mode 100644 rules-tests/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct/Fixture/return_expr_in_construct.php.inc create mode 100644 rules-tests/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct/Fixture/skip_abstract_method.php.inc create mode 100644 rules-tests/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct/Fixture/skip_not_construct.php.inc create mode 100644 rules-tests/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct/Fixture/skip_return_no_expr.php.inc create mode 100644 rules-tests/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct/RemoveUselessReturnExprInConstructTest.php create mode 100644 rules-tests/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct/config/configured_rule.php create mode 100644 rules/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct.php diff --git a/build/target-repository/docs/rector_rules_overview.md b/build/target-repository/docs/rector_rules_overview.md index 3070aa41987..0d8cf98125c 100644 --- a/build/target-repository/docs/rector_rules_overview.md +++ b/build/target-repository/docs/rector_rules_overview.md @@ -1,4 +1,4 @@ -# 354 Rules Overview +# 355 Rules Overview
@@ -10,7 +10,7 @@ - [CodingStyle](#codingstyle) (29) -- [DeadCode](#deadcode) (41) +- [DeadCode](#deadcode) (42) - [EarlyReturn](#earlyreturn) (9) @@ -2839,6 +2839,30 @@ Remove `@param` docblock with same type as parameter type
+### RemoveUselessReturnExprInConstruct + +Remove useless return Expr in `__construct()` + +- class: [`Rector\DeadCode\Rector\ClassMethod\RemoveUselessReturnExprInConstruct`](../rules/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct.php) + +```diff + class SomeClass + { + public function __construct() + { + if (rand(0, 1)) { + $this->init(); +- return true; ++ return; + } + + $this->execute(); + } + } +``` + +
+ ### RemoveUselessReturnTagRector Remove `@return` docblock with same type as defined in PHP diff --git a/rules-tests/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct/Fixture/return_expr_in_construct.php.inc b/rules-tests/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct/Fixture/return_expr_in_construct.php.inc new file mode 100644 index 00000000000..a723c7aea4e --- /dev/null +++ b/rules-tests/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct/Fixture/return_expr_in_construct.php.inc @@ -0,0 +1,37 @@ +init(); + return true; + } + + $this->execute(); + } +} + +?> +----- +init(); + return; + } + + $this->execute(); + } +} + +?> diff --git a/rules-tests/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct/Fixture/skip_abstract_method.php.inc b/rules-tests/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct/Fixture/skip_abstract_method.php.inc new file mode 100644 index 00000000000..2d6bed91a9e --- /dev/null +++ b/rules-tests/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct/Fixture/skip_abstract_method.php.inc @@ -0,0 +1,8 @@ +init(); + return; + } + + $this->execute(); + } +} diff --git a/rules-tests/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct/RemoveUselessReturnExprInConstructTest.php b/rules-tests/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct/RemoveUselessReturnExprInConstructTest.php new file mode 100644 index 00000000000..5426eb90db6 --- /dev/null +++ b/rules-tests/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct/RemoveUselessReturnExprInConstructTest.php @@ -0,0 +1,28 @@ +doTestFile($filePath); + } + + public static function provideData(): Iterator + { + return self::yieldFilesFromDirectory(__DIR__ . '/Fixture'); + } + + public function provideConfigFilePath(): string + { + return __DIR__ . '/config/configured_rule.php'; + } +} diff --git a/rules-tests/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct/config/configured_rule.php b/rules-tests/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct/config/configured_rule.php new file mode 100644 index 00000000000..2cd08ec4998 --- /dev/null +++ b/rules-tests/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct/config/configured_rule.php @@ -0,0 +1,10 @@ +rule(RemoveUselessReturnExprInConstruct::class); +}; diff --git a/rules/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct.php b/rules/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct.php new file mode 100644 index 00000000000..810eef7a096 --- /dev/null +++ b/rules/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct.php @@ -0,0 +1,107 @@ +init(); + return true; + } + + $this->execute(); + } +} +CODE_SAMPLE + , + <<<'CODE_SAMPLE' +class SomeClass +{ + public function __construct() + { + if (rand(0, 1)) { + $this->init(); + return; + } + + $this->execute(); + } +} +CODE_SAMPLE + ), + ] + ); + } + + /** + * @return array> + */ + public function getNodeTypes(): array + { + return [ClassMethod::class]; + } + + /** + * @param ClassMethod $node + */ + public function refactor(Node $node): ?Node + { + if ($node->stmts === null) { + return null; + } + + if (! $this->isName($node, MethodName::CONSTRUCT)) { + return null; + } + + $hasChanged = false; + $this->traverseNodesWithCallable($node->stmts, function (Node $subNode) use (&$hasChanged) { + if ($subNode instanceof Class_ || $subNode instanceof Function_ || $subNode instanceof Closure) { + return NodeTraverser::DONT_TRAVERSE_CURRENT_AND_CHILDREN; + } + + if ($subNode instanceof Return_ && $subNode->expr instanceof Expr) { + $subNode->expr = null; + return $subNode; + } + }); + + if ($hasChanged) { + return $node; + } + + return null; + } +} From d8b0b93f42ce01a5930e0c2cf292bf97e9a506b0 Mon Sep 17 00:00:00 2001 From: GitHub Action Date: Thu, 12 Oct 2023 01:37:02 +0000 Subject: [PATCH 02/10] [ci-review] Rector Rectify --- .../ClassMethod/RemoveUselessReturnExprInConstruct.php | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/rules/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct.php b/rules/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct.php index 810eef7a096..442c27889d5 100644 --- a/rules/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct.php +++ b/rules/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct.php @@ -12,11 +12,8 @@ use PhpParser\Node\Stmt\Function_; use PhpParser\Node\Stmt\Return_; use PhpParser\NodeTraverser; -use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfoFactory; -use Rector\Comments\NodeDocBlock\DocBlockUpdater; use Rector\Core\Rector\AbstractRector; use Rector\Core\ValueObject\MethodName; -use Rector\DeadCode\PhpDoc\TagRemover\ReturnTagRemover; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; @@ -87,11 +84,10 @@ public function refactor(Node $node): ?Node } $hasChanged = false; - $this->traverseNodesWithCallable($node->stmts, function (Node $subNode) use (&$hasChanged) { + $this->traverseNodesWithCallable($node->stmts, static function (Node $subNode) use (&$hasChanged) { if ($subNode instanceof Class_ || $subNode instanceof Function_ || $subNode instanceof Closure) { return NodeTraverser::DONT_TRAVERSE_CURRENT_AND_CHILDREN; } - if ($subNode instanceof Return_ && $subNode->expr instanceof Expr) { $subNode->expr = null; return $subNode; From f125a8ba7568530b65ff8dd45aba36e5916f4385 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Thu, 12 Oct 2023 08:37:28 +0700 Subject: [PATCH 03/10] skip inner class test --- .../Fixture/skip_inner_class.php.inc | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 rules-tests/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct/Fixture/skip_inner_class.php.inc diff --git a/rules-tests/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct/Fixture/skip_inner_class.php.inc b/rules-tests/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct/Fixture/skip_inner_class.php.inc new file mode 100644 index 00000000000..0570baaa06d --- /dev/null +++ b/rules-tests/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct/Fixture/skip_inner_class.php.inc @@ -0,0 +1,16 @@ + Date: Thu, 12 Oct 2023 01:38:22 +0000 Subject: [PATCH 04/10] [ci-review] Rector Rectify --- .../Rector/ClassMethod/RemoveUselessReturnExprInConstruct.php | 1 + 1 file changed, 1 insertion(+) diff --git a/rules/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct.php b/rules/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct.php index 442c27889d5..5d0ec611cc8 100644 --- a/rules/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct.php +++ b/rules/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct.php @@ -88,6 +88,7 @@ public function refactor(Node $node): ?Node if ($subNode instanceof Class_ || $subNode instanceof Function_ || $subNode instanceof Closure) { return NodeTraverser::DONT_TRAVERSE_CURRENT_AND_CHILDREN; } + if ($subNode instanceof Return_ && $subNode->expr instanceof Expr) { $subNode->expr = null; return $subNode; From d3476eb85a4997f6c203dc51c8c7825b77f355b7 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Thu, 12 Oct 2023 08:38:43 +0700 Subject: [PATCH 05/10] fix phpstan --- .../Rector/ClassMethod/RemoveUselessReturnExprInConstruct.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rules/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct.php b/rules/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct.php index 5d0ec611cc8..24471d1c051 100644 --- a/rules/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct.php +++ b/rules/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct.php @@ -90,6 +90,8 @@ public function refactor(Node $node): ?Node } if ($subNode instanceof Return_ && $subNode->expr instanceof Expr) { + $hasChanged = true; + $subNode->expr = null; return $subNode; } From c0386d6e35277f3c4022ea1f4343763ca1f34e92 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Thu, 12 Oct 2023 08:40:15 +0700 Subject: [PATCH 06/10] add Rector suffix --- build/target-repository/docs/rector_rules_overview.md | 4 ++-- .../Fixture/return_expr_in_construct.php.inc | 4 ++-- .../Fixture/skip_abstract_method.php.inc | 2 +- .../Fixture/skip_inner_class.php.inc | 2 +- .../Fixture/skip_not_construct.php.inc | 2 +- .../Fixture/skip_return_no_expr.php.inc | 2 +- .../RemoveUselessReturnExprInConstructRectorTest.php} | 4 ++-- .../config/configured_rule.php | 4 ++-- ...truct.php => RemoveUselessReturnExprInConstructRector.php} | 4 ++-- 9 files changed, 14 insertions(+), 14 deletions(-) rename rules-tests/DeadCode/Rector/ClassMethod/{RemoveUselessReturnExprInConstruct => RemoveUselessReturnExprInConstructRector}/Fixture/return_expr_in_construct.php.inc (88%) rename rules-tests/DeadCode/Rector/ClassMethod/{RemoveUselessReturnExprInConstruct => RemoveUselessReturnExprInConstructRector}/Fixture/skip_abstract_method.php.inc (79%) rename rules-tests/DeadCode/Rector/ClassMethod/{RemoveUselessReturnExprInConstruct => RemoveUselessReturnExprInConstructRector}/Fixture/skip_inner_class.php.inc (88%) rename rules-tests/DeadCode/Rector/ClassMethod/{RemoveUselessReturnExprInConstruct => RemoveUselessReturnExprInConstructRector}/Fixture/skip_not_construct.php.inc (81%) rename rules-tests/DeadCode/Rector/ClassMethod/{RemoveUselessReturnExprInConstruct => RemoveUselessReturnExprInConstructRector}/Fixture/skip_return_no_expr.php.inc (87%) rename rules-tests/DeadCode/Rector/ClassMethod/{RemoveUselessReturnExprInConstruct/RemoveUselessReturnExprInConstructTest.php => RemoveUselessReturnExprInConstructRector/RemoveUselessReturnExprInConstructRectorTest.php} (83%) rename rules-tests/DeadCode/Rector/ClassMethod/{RemoveUselessReturnExprInConstruct => RemoveUselessReturnExprInConstructRector}/config/configured_rule.php (67%) rename rules/DeadCode/Rector/ClassMethod/{RemoveUselessReturnExprInConstruct.php => RemoveUselessReturnExprInConstructRector.php} (94%) diff --git a/build/target-repository/docs/rector_rules_overview.md b/build/target-repository/docs/rector_rules_overview.md index 0d8cf98125c..7b3efb16531 100644 --- a/build/target-repository/docs/rector_rules_overview.md +++ b/build/target-repository/docs/rector_rules_overview.md @@ -2839,11 +2839,11 @@ Remove `@param` docblock with same type as parameter type
-### RemoveUselessReturnExprInConstruct +### RemoveUselessReturnExprInConstructRector Remove useless return Expr in `__construct()` -- class: [`Rector\DeadCode\Rector\ClassMethod\RemoveUselessReturnExprInConstruct`](../rules/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct.php) +- class: [`Rector\DeadCode\Rector\ClassMethod\RemoveUselessReturnExprInConstructRector`](../rules/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstructRector.php) ```diff class SomeClass diff --git a/rules-tests/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct/Fixture/return_expr_in_construct.php.inc b/rules-tests/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstructRector/Fixture/return_expr_in_construct.php.inc similarity index 88% rename from rules-tests/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct/Fixture/return_expr_in_construct.php.inc rename to rules-tests/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstructRector/Fixture/return_expr_in_construct.php.inc index a723c7aea4e..a60465e6526 100644 --- a/rules-tests/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct/Fixture/return_expr_in_construct.php.inc +++ b/rules-tests/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstructRector/Fixture/return_expr_in_construct.php.inc @@ -1,6 +1,6 @@ rule(RemoveUselessReturnExprInConstruct::class); + $rectorConfig->rule(RemoveUselessReturnExprInConstructRector::class); }; diff --git a/rules/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct.php b/rules/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstructRector.php similarity index 94% rename from rules/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct.php rename to rules/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstructRector.php index 24471d1c051..5338651b792 100644 --- a/rules/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstruct.php +++ b/rules/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstructRector.php @@ -18,9 +18,9 @@ use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; /** - * @see \Rector\Tests\DeadCode\Rector\ClassMethod\RemoveUselessReturnExprInConstruct\RemoveUselessReturnExprInConstructTest + * @see \Rector\Tests\DeadCode\Rector\ClassMethod\RemoveUselessReturnExprInConstructRector\RemoveUselessReturnExprInConstructRectorTest */ -final class RemoveUselessReturnExprInConstruct extends AbstractRector +final class RemoveUselessReturnExprInConstructRector extends AbstractRector { public function getRuleDefinition(): RuleDefinition { From f9c50992b4c4efb572ca94d6daf2d32a2bb3455c Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Thu, 12 Oct 2023 08:41:18 +0700 Subject: [PATCH 07/10] register --- config/set/dead-code.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/config/set/dead-code.php b/config/set/dead-code.php index 0d3c5279c06..8988a2ea573 100644 --- a/config/set/dead-code.php +++ b/config/set/dead-code.php @@ -16,6 +16,7 @@ use Rector\DeadCode\Rector\ClassMethod\RemoveUnusedPrivateMethodRector; use Rector\DeadCode\Rector\ClassMethod\RemoveUnusedPromotedPropertyRector; use Rector\DeadCode\Rector\ClassMethod\RemoveUselessParamTagRector; +use Rector\DeadCode\Rector\ClassMethod\RemoveUselessReturnExprInConstructRector; use Rector\DeadCode\Rector\ClassMethod\RemoveUselessReturnTagRector; use Rector\DeadCode\Rector\Concat\RemoveConcatAutocastRector; use Rector\DeadCode\Rector\ConstFetch\RemovePhpVersionIdCheckRector; @@ -91,5 +92,6 @@ RemoveAlwaysTrueIfConditionRector::class, RemoveDeadZeroAndOneOperationRector::class, RemovePhpVersionIdCheckRector::class, + RemoveUselessReturnExprInConstructRector::class, ]); }; From 1e79d54e4f429825e69d23acd8da0e9d9f355ac5 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Thu, 12 Oct 2023 16:51:51 +0700 Subject: [PATCH 08/10] add fixture for return dynamic --- .../return_dynamic_expr_in_construct.php.inc | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 rules-tests/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstructRector/Fixture/return_dynamic_expr_in_construct.php.inc diff --git a/rules-tests/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstructRector/Fixture/return_dynamic_expr_in_construct.php.inc b/rules-tests/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstructRector/Fixture/return_dynamic_expr_in_construct.php.inc new file mode 100644 index 00000000000..44ff691472c --- /dev/null +++ b/rules-tests/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstructRector/Fixture/return_dynamic_expr_in_construct.php.inc @@ -0,0 +1,38 @@ +execute(); + } +} + +?> +----- +execute(); + } +} + +?> From 9c2a8e567c095ec29c3631027a79d18926efd1b9 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Thu, 12 Oct 2023 16:53:54 +0700 Subject: [PATCH 09/10] update doc sample --- build/target-repository/docs/rector_rules_overview.md | 6 ++++++ .../RemoveUselessReturnExprInConstructRector.php | 9 +++++++++ 2 files changed, 15 insertions(+) diff --git a/build/target-repository/docs/rector_rules_overview.md b/build/target-repository/docs/rector_rules_overview.md index 7b3efb16531..3f6fa376392 100644 --- a/build/target-repository/docs/rector_rules_overview.md +++ b/build/target-repository/docs/rector_rules_overview.md @@ -2856,6 +2856,12 @@ Remove useless return Expr in `__construct()` + return; } + if (rand(2, 3)) { +- return parent::construct(); ++ parent::construct(); ++ return; + } + $this->execute(); } } diff --git a/rules/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstructRector.php b/rules/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstructRector.php index 5338651b792..c752f6b9b8c 100644 --- a/rules/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstructRector.php +++ b/rules/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstructRector.php @@ -38,6 +38,10 @@ public function __construct() return true; } + if (rand(2, 3)) { + return parent::construct(); + } + $this->execute(); } } @@ -53,6 +57,11 @@ public function __construct() return; } + if (rand(2, 3)) { + parent::construct(); + return; + } + $this->execute(); } } From c1f0717b7938eb9d33037ace005ee0d4fd3da685 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Thu, 12 Oct 2023 17:17:14 +0700 Subject: [PATCH 10/10] fix --- .../SimpleCallableNodeTraverser.php | 2 +- .../NodeVisitor/CallableNodeVisitor.php | 33 ++++++++++++++----- ...moveUselessReturnExprInConstructRector.php | 28 +++++++++++++--- src/Rector/AbstractRector.php | 2 +- 4 files changed, 50 insertions(+), 15 deletions(-) diff --git a/packages/PhpDocParser/NodeTraverser/SimpleCallableNodeTraverser.php b/packages/PhpDocParser/NodeTraverser/SimpleCallableNodeTraverser.php index 3fea0f1c4ae..c9779254fed 100644 --- a/packages/PhpDocParser/NodeTraverser/SimpleCallableNodeTraverser.php +++ b/packages/PhpDocParser/NodeTraverser/SimpleCallableNodeTraverser.php @@ -14,7 +14,7 @@ final class SimpleCallableNodeTraverser { /** - * @param callable(Node $node): (int|Node|null) $callable + * @param callable(Node): (int|Node|null|Node[]) $callable * @param Node|Node[]|null $node */ public function traverseNodesWithCallable(Node | array | null $node, callable $callable): void diff --git a/packages/PhpDocParser/NodeVisitor/CallableNodeVisitor.php b/packages/PhpDocParser/NodeVisitor/CallableNodeVisitor.php index 95dcd916dbe..54b91362507 100644 --- a/packages/PhpDocParser/NodeVisitor/CallableNodeVisitor.php +++ b/packages/PhpDocParser/NodeVisitor/CallableNodeVisitor.php @@ -14,14 +14,19 @@ final class CallableNodeVisitor extends NodeVisitorAbstract { /** - * @var callable(Node): (int|Node|null) + * @var callable(Node): (int|Node|null|Node[]) */ private $callable; private ?int $nodeIdToRemove = null; /** - * @param callable(Node $node): (int|Node|null) $callable + * @var array + */ + private array $nodesToReturn = []; + + /** + * @param callable(Node $node): (int|Node|null|Node[]) $callable */ public function __construct(callable $callable) { @@ -34,7 +39,7 @@ public function enterNode(Node $node): int|Node|null $callable = $this->callable; - /** @var int|Node|null $newNode */ + /** @var int|Node|null|Node[] $newNode */ $newNode = $callable($node); if ($newNode === NodeTraverser::REMOVE_NODE) { @@ -42,6 +47,13 @@ public function enterNode(Node $node): int|Node|null return $originalNode; } + if (is_array($newNode)) { + $nodeId = spl_object_id($node); + $this->nodesToReturn[$nodeId] = $newNode; + + return $node; + } + if ($originalNode instanceof Stmt && $newNode instanceof Expr) { return new Expression($newNode); } @@ -49,15 +61,20 @@ public function enterNode(Node $node): int|Node|null return $newNode; } - public function leaveNode(Node $node): int|Node + /** + * @return int|Node|Node[] + */ + public function leaveNode(Node $node): int|Node|array { - if ($this->nodeIdToRemove !== null - && $this->nodeIdToRemove === spl_object_id($node) - ) { + if ($this->nodeIdToRemove !== null && $this->nodeIdToRemove === spl_object_id($node)) { $this->nodeIdToRemove = null; return NodeTraverser::REMOVE_NODE; } - return $node; + if ($this->nodesToReturn === []) { + return $node; + } + + return $this->nodesToReturn[spl_object_id($node)] ?? $node; } } diff --git a/rules/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstructRector.php b/rules/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstructRector.php index c752f6b9b8c..8c127c26b61 100644 --- a/rules/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstructRector.php +++ b/rules/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstructRector.php @@ -9,9 +9,11 @@ use PhpParser\Node\Expr\Closure; use PhpParser\Node\Stmt\Class_; use PhpParser\Node\Stmt\ClassMethod; +use PhpParser\Node\Stmt\Expression; use PhpParser\Node\Stmt\Function_; use PhpParser\Node\Stmt\Return_; use PhpParser\NodeTraverser; +use Rector\Core\NodeAnalyzer\ExprAnalyzer; use Rector\Core\Rector\AbstractRector; use Rector\Core\ValueObject\MethodName; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; @@ -22,6 +24,11 @@ */ final class RemoveUselessReturnExprInConstructRector extends AbstractRector { + public function __construct( + private readonly ExprAnalyzer $exprAnalyzer + ) { + } + public function getRuleDefinition(): RuleDefinition { return new RuleDefinition( @@ -93,17 +100,28 @@ public function refactor(Node $node): ?Node } $hasChanged = false; - $this->traverseNodesWithCallable($node->stmts, static function (Node $subNode) use (&$hasChanged) { + $this->traverseNodesWithCallable($node->stmts, function (Node $subNode) use ( + &$hasChanged + ): int|null|array|Return_ { if ($subNode instanceof Class_ || $subNode instanceof Function_ || $subNode instanceof Closure) { return NodeTraverser::DONT_TRAVERSE_CURRENT_AND_CHILDREN; } - if ($subNode instanceof Return_ && $subNode->expr instanceof Expr) { - $hasChanged = true; + if (! $subNode instanceof Return_) { + return null; + } + + if (! $subNode->expr instanceof Expr) { + return null; + } - $subNode->expr = null; - return $subNode; + $hasChanged = true; + if ($this->exprAnalyzer->isDynamicExpr($subNode->expr)) { + return [new Expression($subNode->expr), new Return_()]; } + + $subNode->expr = null; + return $subNode; }); if ($hasChanged) { diff --git a/src/Rector/AbstractRector.php b/src/Rector/AbstractRector.php index 0b36a8954eb..d872f2b35b0 100644 --- a/src/Rector/AbstractRector.php +++ b/src/Rector/AbstractRector.php @@ -275,7 +275,7 @@ protected function getType(Node $node): Type /** * @param Node|Node[] $nodes - * @param callable(Node $node): (Node|null|int) $callable + * @param callable(Node): (int|Node|null|Node[]) $callable */ protected function traverseNodesWithCallable(Node | array $nodes, callable $callable): void {