From bc5462b8aeba5369c7476b20b9e0847964bb65ec Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Sat, 20 Nov 2021 20:25:08 +0700 Subject: [PATCH] [DeadCode][EarlyReturn] Handle RemoveUnusedVariableAssignRector + RemoveAlwaysElseRector (#1277) * [DeadCode][EarlyReturn] Handle RemoveUnusedVariableAssignRector + RemoveAlwaysElseRector * update fixture * eol * Fixed :tada: * comment * Trigger notification * check in ExprUsedInNextNodeAnalyzer to handle error in parallel test * comment * clean up * phpstan * [ci-review] Rector Rectify * [ci-review] Rector Rectify * fix * move comment * [ci-review] Rector Rectify * Trigger notification * try 3 process * try 3 process Co-authored-by: GitHub Action --- .github/workflows/tests.yaml | 4 +- .../ExprUsedInNextNodeAnalyzer.php | 21 ++++++++- .../Rector/If_/RemoveAlwaysElseRector.php | 4 ++ .../Fixture/fixture.php.inc | 46 +++++++++++++++++++ .../RemoveUnusedVariableAlwaysElseTest.php | 33 +++++++++++++ .../config/configured_rule.php | 13 ++++++ 6 files changed, 118 insertions(+), 3 deletions(-) create mode 100644 tests/Issues/RemoveUnusedVariableAlwaysElse/Fixture/fixture.php.inc create mode 100644 tests/Issues/RemoveUnusedVariableAlwaysElse/RemoveUnusedVariableAlwaysElseTest.php create mode 100644 tests/Issues/RemoveUnusedVariableAlwaysElse/config/configured_rule.php diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 2eaed75fe8c..8ce3515e858 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -33,9 +33,9 @@ jobs: - uses: "ramsey/composer-install@v1" - # with --runner=WrapperRunner, it is faster, only ideal with 4 processes (p4) + # with --runner=WrapperRunner, it is faster, only ideal with 3 processes (p3) # @see https://github.com/rectorphp/rector-src/pull/551#issuecomment-889990905 - - run: vendor/bin/paratest -p5 --runner=WrapperRunner ${{ matrix.path }} --colors + - run: vendor/bin/paratest -p3 --runner=WrapperRunner ${{ matrix.path }} --colors if: ${{ matrix.path == 'rules-tests' }} - run: vendor/bin/phpunit ${{ matrix.path }} diff --git a/rules/DeadCode/NodeAnalyzer/ExprUsedInNextNodeAnalyzer.php b/rules/DeadCode/NodeAnalyzer/ExprUsedInNextNodeAnalyzer.php index 3fb43995f93..3c873b6246d 100644 --- a/rules/DeadCode/NodeAnalyzer/ExprUsedInNextNodeAnalyzer.php +++ b/rules/DeadCode/NodeAnalyzer/ExprUsedInNextNodeAnalyzer.php @@ -8,8 +8,10 @@ use PhpParser\Node\Arg; use PhpParser\Node\Expr; use PhpParser\Node\Name; +use PhpParser\Node\Stmt\If_; use PHPStan\Analyser\Scope; use Rector\Core\PhpParser\Node\BetterNodeFinder; +use Rector\EarlyReturn\Rector\If_\RemoveAlwaysElseRector; use Rector\NodeTypeResolver\Node\AttributeKey; final class ExprUsedInNextNodeAnalyzer @@ -39,8 +41,25 @@ function (Node $node) use ($expr, $isCheckNameScope): bool { } } - return $this->exprUsedInNodeAnalyzer->isUsed($node, $expr); + if (! $node instanceof If_) { + return $this->exprUsedInNodeAnalyzer->isUsed($node, $expr); + } + + /** + * handle when used along with RemoveAlwaysElseRector + */ + if (! $this->hasIfChangedByRemoveAlwaysElseRector($node)) { + return $this->exprUsedInNodeAnalyzer->isUsed($node, $expr); + } + + return true; } ); } + + private function hasIfChangedByRemoveAlwaysElseRector(If_ $if): bool + { + $createdByRule = $if->getAttribute(AttributeKey::CREATED_BY_RULE); + return $createdByRule === RemoveAlwaysElseRector::class; + } } diff --git a/rules/EarlyReturn/Rector/If_/RemoveAlwaysElseRector.php b/rules/EarlyReturn/Rector/If_/RemoveAlwaysElseRector.php index a5683d6de07..bc578e3e745 100644 --- a/rules/EarlyReturn/Rector/If_/RemoveAlwaysElseRector.php +++ b/rules/EarlyReturn/Rector/If_/RemoveAlwaysElseRector.php @@ -14,6 +14,7 @@ use PhpParser\Node\Stmt\Return_; use PhpParser\Node\Stmt\Throw_; use Rector\Core\Rector\AbstractRector; +use Rector\NodeTypeResolver\Node\AttributeKey; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; @@ -81,6 +82,7 @@ public function refactor(Node $node): ?Node $originalNode = clone $node; $if = new If_($node->cond); $if->stmts = $node->stmts; + $node->setAttribute(AttributeKey::CREATED_BY_RULE, self::class); $this->nodesToAddCollector->addNodeBeforeNode($if, $node); $this->mirrorComments($if, $node); @@ -107,6 +109,8 @@ public function refactor(Node $node): ?Node if ($node->else !== null) { $this->nodesToAddCollector->addNodesAfterNode($node->else->stmts, $node); $node->else = null; + + $node->setAttribute(AttributeKey::CREATED_BY_RULE, self::class); return $node; } diff --git a/tests/Issues/RemoveUnusedVariableAlwaysElse/Fixture/fixture.php.inc b/tests/Issues/RemoveUnusedVariableAlwaysElse/Fixture/fixture.php.inc new file mode 100644 index 00000000000..fde5145f025 --- /dev/null +++ b/tests/Issues/RemoveUnusedVariableAlwaysElse/Fixture/fixture.php.inc @@ -0,0 +1,46 @@ +get('path', null); + $url = $request->get('url', null); + if (null !== $path) { + return response()->file($path); + } elseif (null !== $url) { + return response()->getFile($url); + } + throw new \Exception('Error'); + } +} + +?> +----- +get('path', null); + $url = $request->get('url', null); + if (null !== $path) { + return response()->file($path); + } + if (null !== $url) { + return response()->getFile($url); + } + throw new \Exception('Error'); + } +} + +?> diff --git a/tests/Issues/RemoveUnusedVariableAlwaysElse/RemoveUnusedVariableAlwaysElseTest.php b/tests/Issues/RemoveUnusedVariableAlwaysElse/RemoveUnusedVariableAlwaysElseTest.php new file mode 100644 index 00000000000..0cde6c6800e --- /dev/null +++ b/tests/Issues/RemoveUnusedVariableAlwaysElse/RemoveUnusedVariableAlwaysElseTest.php @@ -0,0 +1,33 @@ +doTestFileInfo($fileInfo); + } + + /** + * @return Iterator + */ + public function provideData(): Iterator + { + return $this->yieldFilesFromDirectory(__DIR__ . '/Fixture'); + } + + public function provideConfigFilePath(): string + { + return __DIR__ . '/config/configured_rule.php'; + } +} diff --git a/tests/Issues/RemoveUnusedVariableAlwaysElse/config/configured_rule.php b/tests/Issues/RemoveUnusedVariableAlwaysElse/config/configured_rule.php new file mode 100644 index 00000000000..aced355be19 --- /dev/null +++ b/tests/Issues/RemoveUnusedVariableAlwaysElse/config/configured_rule.php @@ -0,0 +1,13 @@ +services(); + $services->set(RemoveUnusedVariableAssignRector::class); + $services->set(RemoveAlwaysElseRector::class); +};