diff --git a/build/target-repository/docs/rector_rules_overview.md b/build/target-repository/docs/rector_rules_overview.md index 3070aa41987..3f6fa376392 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,36 @@ Remove `@param` docblock with same type as parameter type
+### RemoveUselessReturnExprInConstructRector + +Remove useless return Expr in `__construct()` + +- class: [`Rector\DeadCode\Rector\ClassMethod\RemoveUselessReturnExprInConstructRector`](../rules/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstructRector.php) + +```diff + class SomeClass + { + public function __construct() + { + if (rand(0, 1)) { + $this->init(); +- return true; ++ return; + } + + if (rand(2, 3)) { +- return parent::construct(); ++ parent::construct(); ++ return; + } + + $this->execute(); + } + } +``` + +
+ ### RemoveUselessReturnTagRector Remove `@return` docblock with same type as defined in PHP 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, ]); }; 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-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(); + } +} + +?> diff --git a/rules-tests/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstructRector/Fixture/return_expr_in_construct.php.inc b/rules-tests/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstructRector/Fixture/return_expr_in_construct.php.inc new file mode 100644 index 00000000000..a60465e6526 --- /dev/null +++ b/rules-tests/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstructRector/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/RemoveUselessReturnExprInConstructRector/Fixture/skip_abstract_method.php.inc b/rules-tests/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstructRector/Fixture/skip_abstract_method.php.inc new file mode 100644 index 00000000000..3b645207310 --- /dev/null +++ b/rules-tests/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstructRector/Fixture/skip_abstract_method.php.inc @@ -0,0 +1,8 @@ +init(); + return; + } + + $this->execute(); + } +} diff --git a/rules-tests/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstructRector/RemoveUselessReturnExprInConstructRectorTest.php b/rules-tests/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstructRector/RemoveUselessReturnExprInConstructRectorTest.php new file mode 100644 index 00000000000..1773e8229e0 --- /dev/null +++ b/rules-tests/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstructRector/RemoveUselessReturnExprInConstructRectorTest.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/RemoveUselessReturnExprInConstructRector/config/configured_rule.php b/rules-tests/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstructRector/config/configured_rule.php new file mode 100644 index 00000000000..296f8138cbf --- /dev/null +++ b/rules-tests/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstructRector/config/configured_rule.php @@ -0,0 +1,10 @@ +rule(RemoveUselessReturnExprInConstructRector::class); +}; diff --git a/rules/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstructRector.php b/rules/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstructRector.php new file mode 100644 index 00000000000..8c127c26b61 --- /dev/null +++ b/rules/DeadCode/Rector/ClassMethod/RemoveUselessReturnExprInConstructRector.php @@ -0,0 +1,133 @@ +init(); + return true; + } + + if (rand(2, 3)) { + return parent::construct(); + } + + $this->execute(); + } +} +CODE_SAMPLE + , + <<<'CODE_SAMPLE' +class SomeClass +{ + public function __construct() + { + if (rand(0, 1)) { + $this->init(); + return; + } + + if (rand(2, 3)) { + parent::construct(); + 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 + ): 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_) { + return null; + } + + if (! $subNode->expr instanceof Expr) { + return null; + } + + $hasChanged = true; + if ($this->exprAnalyzer->isDynamicExpr($subNode->expr)) { + return [new Expression($subNode->expr), new Return_()]; + } + + $subNode->expr = null; + return $subNode; + }); + + if ($hasChanged) { + return $node; + } + + return null; + } +} 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 {