From 7842149bd06d709d9b7b54699a03915ba8181df2 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sun, 12 Oct 2025 09:56:24 +0200 Subject: [PATCH 1/7] [dead-code] Add RemoveEmptyArrayConditionReturnRector --- .../Fixture/binary_op.php.inc | 0 .../RemoveDeadStmtRector/Fixture/cast.php.inc | 0 .../Fixture/class_constant_fetch.php.inc | 4 +- .../Fixture/clone.php.inc | 4 +- .../Fixture/closure.php.inc | 4 +- .../Fixture/constant_fetch.php.inc | 0 .../Fixture/instanceOf.php.inc | 4 +- .../Fixture/isset.php.inc | 4 +- .../Fixture/scalar.php.inc | 0 .../Fixture/static_property_fetch.php.inc | 4 +- .../Fixture/unairy.php.inc | 4 +- .../Fixture/variable.php.inc | 4 +- .../array_dim_fetch.php.inc | 0 .../property_fetch.php.inc | 0 .../Fixture/empty.php.inc | 20 --- .../comment_unwrap_keep.php.inc | 23 --- .../Fixture/allow_equal.php.inc | 31 ++++ .../Fixture/skip_different_return.php.inc | 15 ++ .../Fixture/some_class.php.inc | 31 ++++ ...oveEmptyArrayConditionReturnRectorTest.php | 28 ++++ .../config/configured_rule.php | 9 ++ .../RemoveEmptyArrayConditionReturnRector.php | 141 ++++++++++++++++++ .../Stmt/RemoveUnreachableStatementRector.php | 5 + 23 files changed, 276 insertions(+), 59 deletions(-) rename rules-tests/DeadCode/Rector/{Stmt => Expression}/RemoveDeadStmtRector/Fixture/binary_op.php.inc (100%) rename rules-tests/DeadCode/Rector/{Stmt => Expression}/RemoveDeadStmtRector/Fixture/cast.php.inc (100%) rename rules-tests/DeadCode/Rector/{Stmt => Expression}/RemoveDeadStmtRector/Fixture/class_constant_fetch.php.inc (59%) rename rules-tests/DeadCode/Rector/{Stmt => Expression}/RemoveDeadStmtRector/Fixture/clone.php.inc (56%) rename rules-tests/DeadCode/Rector/{Stmt => Expression}/RemoveDeadStmtRector/Fixture/closure.php.inc (55%) rename rules-tests/DeadCode/Rector/{Stmt => Expression}/RemoveDeadStmtRector/Fixture/constant_fetch.php.inc (100%) rename rules-tests/DeadCode/Rector/{Stmt => Expression}/RemoveDeadStmtRector/Fixture/instanceOf.php.inc (67%) rename rules-tests/DeadCode/Rector/{Stmt => Expression}/RemoveDeadStmtRector/Fixture/isset.php.inc (70%) rename rules-tests/DeadCode/Rector/{Stmt => Expression}/RemoveDeadStmtRector/Fixture/scalar.php.inc (100%) rename rules-tests/DeadCode/Rector/{Stmt => Expression}/RemoveDeadStmtRector/Fixture/static_property_fetch.php.inc (64%) rename rules-tests/DeadCode/Rector/{Stmt => Expression}/RemoveDeadStmtRector/Fixture/unairy.php.inc (63%) rename rules-tests/DeadCode/Rector/{Stmt => Expression}/RemoveDeadStmtRector/Fixture/variable.php.inc (57%) rename rules-tests/DeadCode/Rector/{Stmt => Expression}/RemoveDeadStmtRector/FixtureRemovedComments/array_dim_fetch.php.inc (100%) rename rules-tests/DeadCode/Rector/{Stmt => Expression}/RemoveDeadStmtRector/FixtureRemovedComments/property_fetch.php.inc (100%) delete mode 100644 rules-tests/DeadCode/Rector/Stmt/RemoveDeadStmtRector/Fixture/empty.php.inc delete mode 100644 rules-tests/DeadCode/Rector/Stmt/RemoveDeadStmtRector/FixtureRemovedComments/comment_unwrap_keep.php.inc create mode 100644 rules-tests/DeadCode/Rector/Stmt/RemoveEmptyArrayConditionReturnRector/Fixture/allow_equal.php.inc create mode 100644 rules-tests/DeadCode/Rector/Stmt/RemoveEmptyArrayConditionReturnRector/Fixture/skip_different_return.php.inc create mode 100644 rules-tests/DeadCode/Rector/Stmt/RemoveEmptyArrayConditionReturnRector/Fixture/some_class.php.inc create mode 100644 rules-tests/DeadCode/Rector/Stmt/RemoveEmptyArrayConditionReturnRector/RemoveEmptyArrayConditionReturnRectorTest.php create mode 100644 rules-tests/DeadCode/Rector/Stmt/RemoveEmptyArrayConditionReturnRector/config/configured_rule.php create mode 100644 rules/DeadCode/Rector/Stmt/RemoveEmptyArrayConditionReturnRector.php diff --git a/rules-tests/DeadCode/Rector/Stmt/RemoveDeadStmtRector/Fixture/binary_op.php.inc b/rules-tests/DeadCode/Rector/Expression/RemoveDeadStmtRector/Fixture/binary_op.php.inc similarity index 100% rename from rules-tests/DeadCode/Rector/Stmt/RemoveDeadStmtRector/Fixture/binary_op.php.inc rename to rules-tests/DeadCode/Rector/Expression/RemoveDeadStmtRector/Fixture/binary_op.php.inc diff --git a/rules-tests/DeadCode/Rector/Stmt/RemoveDeadStmtRector/Fixture/cast.php.inc b/rules-tests/DeadCode/Rector/Expression/RemoveDeadStmtRector/Fixture/cast.php.inc similarity index 100% rename from rules-tests/DeadCode/Rector/Stmt/RemoveDeadStmtRector/Fixture/cast.php.inc rename to rules-tests/DeadCode/Rector/Expression/RemoveDeadStmtRector/Fixture/cast.php.inc diff --git a/rules-tests/DeadCode/Rector/Stmt/RemoveDeadStmtRector/Fixture/class_constant_fetch.php.inc b/rules-tests/DeadCode/Rector/Expression/RemoveDeadStmtRector/Fixture/class_constant_fetch.php.inc similarity index 59% rename from rules-tests/DeadCode/Rector/Stmt/RemoveDeadStmtRector/Fixture/class_constant_fetch.php.inc rename to rules-tests/DeadCode/Rector/Expression/RemoveDeadStmtRector/Fixture/class_constant_fetch.php.inc index ab267e944bb..42b76d2aac6 100644 --- a/rules-tests/DeadCode/Rector/Stmt/RemoveDeadStmtRector/Fixture/class_constant_fetch.php.inc +++ b/rules-tests/DeadCode/Rector/Expression/RemoveDeadStmtRector/Fixture/class_constant_fetch.php.inc @@ -1,6 +1,6 @@ ------ - diff --git a/rules-tests/DeadCode/Rector/Stmt/RemoveDeadStmtRector/FixtureRemovedComments/comment_unwrap_keep.php.inc b/rules-tests/DeadCode/Rector/Stmt/RemoveDeadStmtRector/FixtureRemovedComments/comment_unwrap_keep.php.inc deleted file mode 100644 index 1b5a15d4c11..00000000000 --- a/rules-tests/DeadCode/Rector/Stmt/RemoveDeadStmtRector/FixtureRemovedComments/comment_unwrap_keep.php.inc +++ /dev/null @@ -1,23 +0,0 @@ - ------ - diff --git a/rules-tests/DeadCode/Rector/Stmt/RemoveEmptyArrayConditionReturnRector/Fixture/allow_equal.php.inc b/rules-tests/DeadCode/Rector/Stmt/RemoveEmptyArrayConditionReturnRector/Fixture/allow_equal.php.inc new file mode 100644 index 00000000000..caee4f7abc9 --- /dev/null +++ b/rules-tests/DeadCode/Rector/Stmt/RemoveEmptyArrayConditionReturnRector/Fixture/allow_equal.php.inc @@ -0,0 +1,31 @@ + +----- + diff --git a/rules-tests/DeadCode/Rector/Stmt/RemoveEmptyArrayConditionReturnRector/Fixture/skip_different_return.php.inc b/rules-tests/DeadCode/Rector/Stmt/RemoveEmptyArrayConditionReturnRector/Fixture/skip_different_return.php.inc new file mode 100644 index 00000000000..284c55f04f1 --- /dev/null +++ b/rules-tests/DeadCode/Rector/Stmt/RemoveEmptyArrayConditionReturnRector/Fixture/skip_different_return.php.inc @@ -0,0 +1,15 @@ + +----- + diff --git a/rules-tests/DeadCode/Rector/Stmt/RemoveEmptyArrayConditionReturnRector/RemoveEmptyArrayConditionReturnRectorTest.php b/rules-tests/DeadCode/Rector/Stmt/RemoveEmptyArrayConditionReturnRector/RemoveEmptyArrayConditionReturnRectorTest.php new file mode 100644 index 00000000000..01155076546 --- /dev/null +++ b/rules-tests/DeadCode/Rector/Stmt/RemoveEmptyArrayConditionReturnRector/RemoveEmptyArrayConditionReturnRectorTest.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/Stmt/RemoveEmptyArrayConditionReturnRector/config/configured_rule.php b/rules-tests/DeadCode/Rector/Stmt/RemoveEmptyArrayConditionReturnRector/config/configured_rule.php new file mode 100644 index 00000000000..563009ece30 --- /dev/null +++ b/rules-tests/DeadCode/Rector/Stmt/RemoveEmptyArrayConditionReturnRector/config/configured_rule.php @@ -0,0 +1,9 @@ +withRules([RemoveEmptyArrayConditionReturnRector::class]); diff --git a/rules/DeadCode/Rector/Stmt/RemoveEmptyArrayConditionReturnRector.php b/rules/DeadCode/Rector/Stmt/RemoveEmptyArrayConditionReturnRector.php new file mode 100644 index 00000000000..b6d1dd9be4a --- /dev/null +++ b/rules/DeadCode/Rector/Stmt/RemoveEmptyArrayConditionReturnRector.php @@ -0,0 +1,141 @@ +> + */ + public function getNodeTypes(): array + { + return [StmtsAwareInterface::class]; + } + + /** + * @param StmtsAwareInterface $node + */ + public function refactor(Node $node): ?Node + { + if ($node->stmts === null) { + return null; + } + + foreach ($node->stmts as $key => $stmt) { + if (! $stmt instanceof Node\Stmt\If_) { + continue; + } + + if (count($stmt->stmts) !== 1) { + continue; + } + + $soleIfStmt = $stmt->stmts[0]; + if (! $soleIfStmt instanceof Return_) { + continue; + } + + if (! $stmt->cond instanceof Identical && ! $stmt->cond instanceof Equal) { + continue; + } + + $identicalOrEqual = $stmt->cond; + $return = $soleIfStmt; + + if (! $this->isEmptyArray($identicalOrEqual->right)) { + continue; + } + + $comparedVariable = $identicalOrEqual->left; + + if (! $this->isEmptyArray($return->expr)) { + continue; + } + + // next stmt must be return of the same var + $nextStmt = $node->stmts[$key + 1] ?? null; + if (! $nextStmt instanceof Return_) { + continue; + } + + if (! $nextStmt->expr instanceof Expr) { + continue; + } + + if (! $this->nodeComparator->areNodesEqual($nextStmt->expr, $comparedVariable)) { + continue; + } + + // remove next stmt + unset($node->stmts[$key + 1]); + + // replace if with return + $node->stmts[$key] = $nextStmt; + + return $node; + } + + return null; + } + + private function isEmptyArray(?Expr $expr): bool + { + if (! $expr instanceof Array_) { + return false; + } + + return $expr->items === []; + } +} diff --git a/rules/DeadCode/Rector/Stmt/RemoveUnreachableStatementRector.php b/rules/DeadCode/Rector/Stmt/RemoveUnreachableStatementRector.php index 839c6234047..1ce39353c00 100644 --- a/rules/DeadCode/Rector/Stmt/RemoveUnreachableStatementRector.php +++ b/rules/DeadCode/Rector/Stmt/RemoveUnreachableStatementRector.php @@ -68,6 +68,11 @@ public function refactor(Node $node): ?Node return null; } + // at least 2 items are needed + if (count($node->stmts) < 2) { + return null; + } + $originalStmts = $node->stmts; $cleanedStmts = $this->processCleanUpUnreachableStmts($node, $node->stmts); From 36adf6fef1de4e401bd3321775be44f98c6e1016 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sun, 12 Oct 2025 10:34:40 +0200 Subject: [PATCH 2/7] register rule --- .../Fixture/allow_any_type.php.inc | 31 +++++++++++++++++++ .../RemoveEmptyArrayConditionReturnRector.php | 3 +- src/Config/Level/DeadCodeLevel.php | 2 ++ src/Testing/PHPUnit/AbstractLazyTestCase.php | 1 + 4 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 rules-tests/DeadCode/Rector/Stmt/RemoveEmptyArrayConditionReturnRector/Fixture/allow_any_type.php.inc diff --git a/rules-tests/DeadCode/Rector/Stmt/RemoveEmptyArrayConditionReturnRector/Fixture/allow_any_type.php.inc b/rules-tests/DeadCode/Rector/Stmt/RemoveEmptyArrayConditionReturnRector/Fixture/allow_any_type.php.inc new file mode 100644 index 00000000000..991b06402d1 --- /dev/null +++ b/rules-tests/DeadCode/Rector/Stmt/RemoveEmptyArrayConditionReturnRector/Fixture/allow_any_type.php.inc @@ -0,0 +1,31 @@ + +----- + diff --git a/rules/DeadCode/Rector/Stmt/RemoveEmptyArrayConditionReturnRector.php b/rules/DeadCode/Rector/Stmt/RemoveEmptyArrayConditionReturnRector.php index b6d1dd9be4a..b8d90c81b9e 100644 --- a/rules/DeadCode/Rector/Stmt/RemoveEmptyArrayConditionReturnRector.php +++ b/rules/DeadCode/Rector/Stmt/RemoveEmptyArrayConditionReturnRector.php @@ -9,6 +9,7 @@ use PhpParser\Node\Expr\Array_; use PhpParser\Node\Expr\BinaryOp\Equal; use PhpParser\Node\Expr\BinaryOp\Identical; +use PhpParser\Node\Stmt\If_; use PhpParser\Node\Stmt\Return_; use Rector\Contract\PhpParser\Node\StmtsAwareInterface; use Rector\Rector\AbstractRector; @@ -74,7 +75,7 @@ public function refactor(Node $node): ?Node } foreach ($node->stmts as $key => $stmt) { - if (! $stmt instanceof Node\Stmt\If_) { + if (! $stmt instanceof If_) { continue; } diff --git a/src/Config/Level/DeadCodeLevel.php b/src/Config/Level/DeadCodeLevel.php index 294cd360aa0..4d5d047843e 100644 --- a/src/Config/Level/DeadCodeLevel.php +++ b/src/Config/Level/DeadCodeLevel.php @@ -53,6 +53,7 @@ use Rector\DeadCode\Rector\PropertyProperty\RemoveNullPropertyInitializationRector; use Rector\DeadCode\Rector\Return_\RemoveDeadConditionAboveReturnRector; use Rector\DeadCode\Rector\StaticCall\RemoveParentCallWithoutParentRector; +use Rector\DeadCode\Rector\Stmt\RemoveEmptyArrayConditionReturnRector; use Rector\DeadCode\Rector\Stmt\RemoveUnreachableStatementRector; use Rector\DeadCode\Rector\Switch_\RemoveDuplicatedCaseInSwitchRector; use Rector\DeadCode\Rector\Ternary\TernaryToBooleanOrFalseToBooleanAndRector; @@ -124,6 +125,7 @@ final class DeadCodeLevel RemoveDeadCatchRector::class, RemoveDeadTryCatchRector::class, RemoveDeadIfForeachForRector::class, + RemoveEmptyArrayConditionReturnRector::class, RemoveDeadStmtRector::class, UnwrapFutureCompatibleIfPhpVersionRector::class, RemoveParentCallWithoutParentRector::class, diff --git a/src/Testing/PHPUnit/AbstractLazyTestCase.php b/src/Testing/PHPUnit/AbstractLazyTestCase.php index f4236153043..3fc41dd2e3a 100644 --- a/src/Testing/PHPUnit/AbstractLazyTestCase.php +++ b/src/Testing/PHPUnit/AbstractLazyTestCase.php @@ -70,6 +70,7 @@ private function includePreloadFilesAndScoperAutoload(): void if (! class_exists(Version::class, true) || (int) Version::id() < 12) { require_once __DIR__ . '/../../../preload.php'; } + // test case in rector split package } elseif (file_exists(__DIR__ . '/../../../../../../vendor')) { require_once __DIR__ . '/../../../preload-split-package.php'; From 85304227543c8a26a8095570440f5cc6fcfddb94 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sun, 12 Oct 2025 10:38:44 +0200 Subject: [PATCH 3/7] update missed test fixtures --- .../Fixture/binary_op.php.inc | 2 +- .../Fixture/isset.php.inc | 4 ++-- .../Fixture/static_property_fetch.php.inc | 16 --------------- .../array_dim_fetch.php.inc | 3 ++- .../property_fetch.php.inc | 20 ------------------- 5 files changed, 5 insertions(+), 40 deletions(-) diff --git a/rules-tests/DeadCode/Rector/Expression/RemoveDeadStmtRector/Fixture/binary_op.php.inc b/rules-tests/DeadCode/Rector/Expression/RemoveDeadStmtRector/Fixture/binary_op.php.inc index cdf5141bfa4..49ce128a63c 100644 --- a/rules-tests/DeadCode/Rector/Expression/RemoveDeadStmtRector/Fixture/binary_op.php.inc +++ b/rules-tests/DeadCode/Rector/Expression/RemoveDeadStmtRector/Fixture/binary_op.php.inc @@ -64,7 +64,7 @@ function wrapToPreventPhpStanCallingMethods () { foo(); foo(); - foo2(); foo3(); + foo2(); } ?> diff --git a/rules-tests/DeadCode/Rector/Expression/RemoveDeadStmtRector/Fixture/isset.php.inc b/rules-tests/DeadCode/Rector/Expression/RemoveDeadStmtRector/Fixture/isset.php.inc index 8167be95abd..ec5a1277a41 100644 --- a/rules-tests/DeadCode/Rector/Expression/RemoveDeadStmtRector/Fixture/isset.php.inc +++ b/rules-tests/DeadCode/Rector/Expression/RemoveDeadStmtRector/Fixture/isset.php.inc @@ -21,10 +21,10 @@ function wrapToPreventPhpStanCallingMethods ($object) { $a = isset($a); $b = isset($b, $c, $d); - - func1(); func2(); func3(); func4(); + + func1(); } ?> diff --git a/rules-tests/DeadCode/Rector/Expression/RemoveDeadStmtRector/Fixture/static_property_fetch.php.inc b/rules-tests/DeadCode/Rector/Expression/RemoveDeadStmtRector/Fixture/static_property_fetch.php.inc index eed80217d90..c1ab9f96e65 100644 --- a/rules-tests/DeadCode/Rector/Expression/RemoveDeadStmtRector/Fixture/static_property_fetch.php.inc +++ b/rules-tests/DeadCode/Rector/Expression/RemoveDeadStmtRector/Fixture/static_property_fetch.php.inc @@ -13,19 +13,3 @@ function wrapToPreventPhpStanCallingMethods () bar()::${baz()}::${foo()}; } ?> ------ - diff --git a/rules-tests/DeadCode/Rector/Expression/RemoveDeadStmtRector/FixtureRemovedComments/array_dim_fetch.php.inc b/rules-tests/DeadCode/Rector/Expression/RemoveDeadStmtRector/FixtureRemovedComments/array_dim_fetch.php.inc index 9ad5840b1d7..d756d9867e6 100644 --- a/rules-tests/DeadCode/Rector/Expression/RemoveDeadStmtRector/FixtureRemovedComments/array_dim_fetch.php.inc +++ b/rules-tests/DeadCode/Rector/Expression/RemoveDeadStmtRector/FixtureRemovedComments/array_dim_fetch.php.inc @@ -32,9 +32,10 @@ function wrapToPreventPhpStanCallingMethods () //comment methodCall(); + //comment + methodCall2(); //comment methodCall1(); - methodCall2(); } ?> diff --git a/rules-tests/DeadCode/Rector/Expression/RemoveDeadStmtRector/FixtureRemovedComments/property_fetch.php.inc b/rules-tests/DeadCode/Rector/Expression/RemoveDeadStmtRector/FixtureRemovedComments/property_fetch.php.inc index 2ee32638804..270d7c4de9d 100644 --- a/rules-tests/DeadCode/Rector/Expression/RemoveDeadStmtRector/FixtureRemovedComments/property_fetch.php.inc +++ b/rules-tests/DeadCode/Rector/Expression/RemoveDeadStmtRector/FixtureRemovedComments/property_fetch.php.inc @@ -18,23 +18,3 @@ function wrapToPreventPhpStanCallingMethods () ${methodCall1()}->{methodCall2()}; } ?> ------ -prop = 1; - - //comment - methodCall(); - - //comment - methodCall(); - - //comment - methodCall1(); - methodCall2(); -} -?> From 62fac422b6920c114bad6f14b9d917be4ce81f50 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sun, 12 Oct 2025 10:44:48 +0200 Subject: [PATCH 4/7] rename to RemoveConditionExactReturnRector --- .../Fixture/allow_any_type.php.inc | 4 ++-- .../Fixture/allow_equal.php.inc | 4 ++-- .../Fixture/skip_different_return.php.inc | 2 +- .../Fixture/some_class.php.inc | 4 ++-- .../RemoveEmptyArrayConditionReturnRectorTest.php | 4 ++-- .../config/configured_rule.php | 9 +++++++++ .../config/configured_rule.php | 9 --------- ...rnRector.php => RemoveConditionExactReturnRector.php} | 2 +- src/Config/Level/DeadCodeLevel.php | 4 ++-- 9 files changed, 21 insertions(+), 21 deletions(-) rename rules-tests/DeadCode/Rector/Stmt/{RemoveEmptyArrayConditionReturnRector => RemoveConditionExactReturnRector}/Fixture/allow_any_type.php.inc (61%) rename rules-tests/DeadCode/Rector/Stmt/{RemoveEmptyArrayConditionReturnRector => RemoveConditionExactReturnRector}/Fixture/allow_equal.php.inc (62%) rename rules-tests/DeadCode/Rector/Stmt/{RemoveEmptyArrayConditionReturnRector => RemoveConditionExactReturnRector}/Fixture/skip_different_return.php.inc (66%) rename rules-tests/DeadCode/Rector/Stmt/{RemoveEmptyArrayConditionReturnRector => RemoveConditionExactReturnRector}/Fixture/some_class.php.inc (61%) rename rules-tests/DeadCode/Rector/Stmt/{RemoveEmptyArrayConditionReturnRector => RemoveConditionExactReturnRector}/RemoveEmptyArrayConditionReturnRectorTest.php (76%) create mode 100644 rules-tests/DeadCode/Rector/Stmt/RemoveConditionExactReturnRector/config/configured_rule.php delete mode 100644 rules-tests/DeadCode/Rector/Stmt/RemoveEmptyArrayConditionReturnRector/config/configured_rule.php rename rules/DeadCode/Rector/Stmt/{RemoveEmptyArrayConditionReturnRector.php => RemoveConditionExactReturnRector.php} (97%) diff --git a/rules-tests/DeadCode/Rector/Stmt/RemoveEmptyArrayConditionReturnRector/Fixture/allow_any_type.php.inc b/rules-tests/DeadCode/Rector/Stmt/RemoveConditionExactReturnRector/Fixture/allow_any_type.php.inc similarity index 61% rename from rules-tests/DeadCode/Rector/Stmt/RemoveEmptyArrayConditionReturnRector/Fixture/allow_any_type.php.inc rename to rules-tests/DeadCode/Rector/Stmt/RemoveConditionExactReturnRector/Fixture/allow_any_type.php.inc index 991b06402d1..addcf8cdd51 100644 --- a/rules-tests/DeadCode/Rector/Stmt/RemoveEmptyArrayConditionReturnRector/Fixture/allow_any_type.php.inc +++ b/rules-tests/DeadCode/Rector/Stmt/RemoveConditionExactReturnRector/Fixture/allow_any_type.php.inc @@ -1,6 +1,6 @@ withRules([RemoveConditionExactReturnRector::class]); diff --git a/rules-tests/DeadCode/Rector/Stmt/RemoveEmptyArrayConditionReturnRector/config/configured_rule.php b/rules-tests/DeadCode/Rector/Stmt/RemoveEmptyArrayConditionReturnRector/config/configured_rule.php deleted file mode 100644 index 563009ece30..00000000000 --- a/rules-tests/DeadCode/Rector/Stmt/RemoveEmptyArrayConditionReturnRector/config/configured_rule.php +++ /dev/null @@ -1,9 +0,0 @@ -withRules([RemoveEmptyArrayConditionReturnRector::class]); diff --git a/rules/DeadCode/Rector/Stmt/RemoveEmptyArrayConditionReturnRector.php b/rules/DeadCode/Rector/Stmt/RemoveConditionExactReturnRector.php similarity index 97% rename from rules/DeadCode/Rector/Stmt/RemoveEmptyArrayConditionReturnRector.php rename to rules/DeadCode/Rector/Stmt/RemoveConditionExactReturnRector.php index b8d90c81b9e..5ab8d569608 100644 --- a/rules/DeadCode/Rector/Stmt/RemoveEmptyArrayConditionReturnRector.php +++ b/rules/DeadCode/Rector/Stmt/RemoveConditionExactReturnRector.php @@ -19,7 +19,7 @@ /** * @see \Rector\Tests\DeadCode\Rector\Stmt\RemoveEmptyArrayConditionReturnRector\RemoveEmptyArrayConditionReturnRectorTest */ -final class RemoveEmptyArrayConditionReturnRector extends AbstractRector +final class RemoveConditionExactReturnRector extends AbstractRector { public function getRuleDefinition(): RuleDefinition { diff --git a/src/Config/Level/DeadCodeLevel.php b/src/Config/Level/DeadCodeLevel.php index 4d5d047843e..71c5f42f0ae 100644 --- a/src/Config/Level/DeadCodeLevel.php +++ b/src/Config/Level/DeadCodeLevel.php @@ -53,7 +53,7 @@ use Rector\DeadCode\Rector\PropertyProperty\RemoveNullPropertyInitializationRector; use Rector\DeadCode\Rector\Return_\RemoveDeadConditionAboveReturnRector; use Rector\DeadCode\Rector\StaticCall\RemoveParentCallWithoutParentRector; -use Rector\DeadCode\Rector\Stmt\RemoveEmptyArrayConditionReturnRector; +use Rector\DeadCode\Rector\Stmt\RemoveConditionExactReturnRector; use Rector\DeadCode\Rector\Stmt\RemoveUnreachableStatementRector; use Rector\DeadCode\Rector\Switch_\RemoveDuplicatedCaseInSwitchRector; use Rector\DeadCode\Rector\Ternary\TernaryToBooleanOrFalseToBooleanAndRector; @@ -125,7 +125,7 @@ final class DeadCodeLevel RemoveDeadCatchRector::class, RemoveDeadTryCatchRector::class, RemoveDeadIfForeachForRector::class, - RemoveEmptyArrayConditionReturnRector::class, + RemoveConditionExactReturnRector::class, RemoveDeadStmtRector::class, UnwrapFutureCompatibleIfPhpVersionRector::class, RemoveParentCallWithoutParentRector::class, From e592082f721da4505b1c49ff03caa39dfbb3387e Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sun, 12 Oct 2025 10:45:39 +0200 Subject: [PATCH 5/7] add any expr fixture --- .../Fixture/any_expr_compare.php.inc | 31 +++++++++++++++++++ ... RemoveConditionExactReturnRectorTest.php} | 0 .../Stmt/RemoveConditionExactReturnRector.php | 2 +- 3 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 rules-tests/DeadCode/Rector/Stmt/RemoveConditionExactReturnRector/Fixture/any_expr_compare.php.inc rename rules-tests/DeadCode/Rector/Stmt/RemoveConditionExactReturnRector/{RemoveEmptyArrayConditionReturnRectorTest.php => RemoveConditionExactReturnRectorTest.php} (100%) diff --git a/rules-tests/DeadCode/Rector/Stmt/RemoveConditionExactReturnRector/Fixture/any_expr_compare.php.inc b/rules-tests/DeadCode/Rector/Stmt/RemoveConditionExactReturnRector/Fixture/any_expr_compare.php.inc new file mode 100644 index 00000000000..0483501edb8 --- /dev/null +++ b/rules-tests/DeadCode/Rector/Stmt/RemoveConditionExactReturnRector/Fixture/any_expr_compare.php.inc @@ -0,0 +1,31 @@ + +----- + diff --git a/rules-tests/DeadCode/Rector/Stmt/RemoveConditionExactReturnRector/RemoveEmptyArrayConditionReturnRectorTest.php b/rules-tests/DeadCode/Rector/Stmt/RemoveConditionExactReturnRector/RemoveConditionExactReturnRectorTest.php similarity index 100% rename from rules-tests/DeadCode/Rector/Stmt/RemoveConditionExactReturnRector/RemoveEmptyArrayConditionReturnRectorTest.php rename to rules-tests/DeadCode/Rector/Stmt/RemoveConditionExactReturnRector/RemoveConditionExactReturnRectorTest.php diff --git a/rules/DeadCode/Rector/Stmt/RemoveConditionExactReturnRector.php b/rules/DeadCode/Rector/Stmt/RemoveConditionExactReturnRector.php index 5ab8d569608..6831bb0e50b 100644 --- a/rules/DeadCode/Rector/Stmt/RemoveConditionExactReturnRector.php +++ b/rules/DeadCode/Rector/Stmt/RemoveConditionExactReturnRector.php @@ -17,7 +17,7 @@ use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; /** - * @see \Rector\Tests\DeadCode\Rector\Stmt\RemoveEmptyArrayConditionReturnRector\RemoveEmptyArrayConditionReturnRectorTest + * @see \Rector\Tests\DeadCode\Rector\Stmt\RemoveConditionExactReturnRector\RemoveConditionExactReturnRectorTest */ final class RemoveConditionExactReturnRector extends AbstractRector { From 33fe354e14f91978bddf3ee4b496cd0b64e41a95 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sun, 12 Oct 2025 10:47:01 +0200 Subject: [PATCH 6/7] allow any expr --- .../Stmt/RemoveConditionExactReturnRector.php | 34 +++++++------------ 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/rules/DeadCode/Rector/Stmt/RemoveConditionExactReturnRector.php b/rules/DeadCode/Rector/Stmt/RemoveConditionExactReturnRector.php index 6831bb0e50b..92bb046d2d1 100644 --- a/rules/DeadCode/Rector/Stmt/RemoveConditionExactReturnRector.php +++ b/rules/DeadCode/Rector/Stmt/RemoveConditionExactReturnRector.php @@ -6,7 +6,6 @@ use PhpParser\Node; use PhpParser\Node\Expr; -use PhpParser\Node\Expr\Array_; use PhpParser\Node\Expr\BinaryOp\Equal; use PhpParser\Node\Expr\BinaryOp\Identical; use PhpParser\Node\Stmt\If_; @@ -23,9 +22,11 @@ final class RemoveConditionExactReturnRector extends AbstractRector { public function getRuleDefinition(): RuleDefinition { - return new RuleDefinition('Remove unused empty array condition and return value directly', [ - new CodeSample( - <<<'CODE_SAMPLE' + return new RuleDefinition( + 'Remove if with condition and return with same expr, followed by compared expr return', + [ + new CodeSample( + <<<'CODE_SAMPLE' final class SomeClass { public function __construct(array $items) @@ -39,8 +40,8 @@ public function __construct(array $items) } CODE_SAMPLE - , - <<<'CODE_SAMPLE' + , + <<<'CODE_SAMPLE' final class SomeClass { public function __construct(array $items) @@ -53,8 +54,10 @@ public function __construct(array $items) } } CODE_SAMPLE - ), - ]); + ), + + ] + ); } /** @@ -95,16 +98,12 @@ public function refactor(Node $node): ?Node $identicalOrEqual = $stmt->cond; $return = $soleIfStmt; - if (! $this->isEmptyArray($identicalOrEqual->right)) { + if (! $this->nodeComparator->areNodesEqual($identicalOrEqual->right, $return->expr)) { continue; } $comparedVariable = $identicalOrEqual->left; - if (! $this->isEmptyArray($return->expr)) { - continue; - } - // next stmt must be return of the same var $nextStmt = $node->stmts[$key + 1] ?? null; if (! $nextStmt instanceof Return_) { @@ -130,13 +129,4 @@ public function refactor(Node $node): ?Node return null; } - - private function isEmptyArray(?Expr $expr): bool - { - if (! $expr instanceof Array_) { - return false; - } - - return $expr->items === []; - } } From 7f198159addb85025a6411e6d9ccfd9980cb68b0 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sun, 12 Oct 2025 10:52:28 +0200 Subject: [PATCH 7/7] skip method/static calls --- .../Fixture/skip_side_effect_call.php.inc | 20 ++++++++++ .../Stmt/RemoveConditionExactReturnRector.php | 37 ++++++++++++++----- 2 files changed, 48 insertions(+), 9 deletions(-) create mode 100644 rules-tests/DeadCode/Rector/Stmt/RemoveConditionExactReturnRector/Fixture/skip_side_effect_call.php.inc diff --git a/rules-tests/DeadCode/Rector/Stmt/RemoveConditionExactReturnRector/Fixture/skip_side_effect_call.php.inc b/rules-tests/DeadCode/Rector/Stmt/RemoveConditionExactReturnRector/Fixture/skip_side_effect_call.php.inc new file mode 100644 index 00000000000..008ac98f1fc --- /dev/null +++ b/rules-tests/DeadCode/Rector/Stmt/RemoveConditionExactReturnRector/Fixture/skip_side_effect_call.php.inc @@ -0,0 +1,20 @@ +isMissing(100)) { + return $this->isMissing(100); + } + + return $result; + } + + private function isMissing(int $int): int + { + return mt_rand(0, 1) * $int; + } +} diff --git a/rules/DeadCode/Rector/Stmt/RemoveConditionExactReturnRector.php b/rules/DeadCode/Rector/Stmt/RemoveConditionExactReturnRector.php index 92bb046d2d1..6e1dece3316 100644 --- a/rules/DeadCode/Rector/Stmt/RemoveConditionExactReturnRector.php +++ b/rules/DeadCode/Rector/Stmt/RemoveConditionExactReturnRector.php @@ -8,6 +8,8 @@ use PhpParser\Node\Expr; use PhpParser\Node\Expr\BinaryOp\Equal; use PhpParser\Node\Expr\BinaryOp\Identical; +use PhpParser\Node\Expr\MethodCall; +use PhpParser\Node\Expr\StaticCall; use PhpParser\Node\Stmt\If_; use PhpParser\Node\Stmt\Return_; use Rector\Contract\PhpParser\Node\StmtsAwareInterface; @@ -82,29 +84,32 @@ public function refactor(Node $node): ?Node continue; } - if (count($stmt->stmts) !== 1) { + $soleIfReturn = $this->matchSoleIfReturn($stmt); + if (! $soleIfReturn instanceof Return_) { continue; } - $soleIfStmt = $stmt->stmts[0]; - if (! $soleIfStmt instanceof Return_) { + if (! $stmt->cond instanceof Identical && ! $stmt->cond instanceof Equal) { continue; } - if (! $stmt->cond instanceof Identical && ! $stmt->cond instanceof Equal) { + $identicalOrEqual = $stmt->cond; + // skip obvious complexity + if ($identicalOrEqual->right instanceof MethodCall) { continue; } - $identicalOrEqual = $stmt->cond; - $return = $soleIfStmt; + if ($identicalOrEqual->right instanceof StaticCall) { + continue; + } - if (! $this->nodeComparator->areNodesEqual($identicalOrEqual->right, $return->expr)) { + if (! $this->nodeComparator->areNodesEqual($identicalOrEqual->right, $soleIfReturn->expr)) { continue; } $comparedVariable = $identicalOrEqual->left; - // next stmt must be return of the same var + // next if must be return of the same var $nextStmt = $node->stmts[$key + 1] ?? null; if (! $nextStmt instanceof Return_) { continue; @@ -118,7 +123,7 @@ public function refactor(Node $node): ?Node continue; } - // remove next stmt + // remove next if unset($node->stmts[$key + 1]); // replace if with return @@ -129,4 +134,18 @@ public function refactor(Node $node): ?Node return null; } + + private function matchSoleIfReturn(If_ $if): ?Return_ + { + if (count($if->stmts) !== 1) { + return null; + } + + $soleIfStmt = $if->stmts[0]; + if (! $soleIfStmt instanceof Return_) { + return null; + } + + return $soleIfStmt; + } }