From e21ebbbaf22a880d9d66200e1562516c11aab749 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Tue, 16 Oct 2018 14:38:03 +0800 Subject: [PATCH] [CodeQuality] Add SimplifyForeachToCoalescingRector --- .../SimplifyForeachToCoalescingRector.php | 190 ++++++++++++++++++ .../Correct/correct.php.inc | 9 + .../Correct/correct2.php.inc | 3 + .../Correct/correct3.php.inc | 6 + .../SimplifyForeachToCoalescingRectorTest.php | 32 +++ .../Wrong/wrong.php.inc | 16 ++ .../Wrong/wrong2.php.inc | 9 + .../Wrong/wrong3.php.inc | 10 + .../config.yml | 2 + 9 files changed, 277 insertions(+) create mode 100644 packages/CodeQuality/src/Rector/Foreach_/SimplifyForeachToCoalescingRector.php create mode 100644 packages/CodeQuality/tests/Rector/Foreach_/SimplifyForeachToCoalescingRector/Correct/correct.php.inc create mode 100644 packages/CodeQuality/tests/Rector/Foreach_/SimplifyForeachToCoalescingRector/Correct/correct2.php.inc create mode 100644 packages/CodeQuality/tests/Rector/Foreach_/SimplifyForeachToCoalescingRector/Correct/correct3.php.inc create mode 100644 packages/CodeQuality/tests/Rector/Foreach_/SimplifyForeachToCoalescingRector/SimplifyForeachToCoalescingRectorTest.php create mode 100644 packages/CodeQuality/tests/Rector/Foreach_/SimplifyForeachToCoalescingRector/Wrong/wrong.php.inc create mode 100644 packages/CodeQuality/tests/Rector/Foreach_/SimplifyForeachToCoalescingRector/Wrong/wrong2.php.inc create mode 100644 packages/CodeQuality/tests/Rector/Foreach_/SimplifyForeachToCoalescingRector/Wrong/wrong3.php.inc create mode 100644 packages/CodeQuality/tests/Rector/Foreach_/SimplifyForeachToCoalescingRector/config.yml diff --git a/packages/CodeQuality/src/Rector/Foreach_/SimplifyForeachToCoalescingRector.php b/packages/CodeQuality/src/Rector/Foreach_/SimplifyForeachToCoalescingRector.php new file mode 100644 index 00000000000..80f0ee49344 --- /dev/null +++ b/packages/CodeQuality/src/Rector/Foreach_/SimplifyForeachToCoalescingRector.php @@ -0,0 +1,190 @@ +oldToNewFunctions as $oldFunction => $newFunction) { + if ($currentFunction === $oldFunction) { + return $newFunction; + } +} + +return null; +CODE_SAMPLE + , + <<<'CODE_SAMPLE' +return $this->oldToNewFunctions[$currentFunction] ?? null; +CODE_SAMPLE + ), + ]); + } + + /** + * @return string[] + */ + public function getNodeTypes(): array + { + return [Foreach_::class, Return_::class]; + } + + /** + * @param Foreach_|Return_ $node + */ + public function refactor(Node $node): ?Node + { + if ($node instanceof Return_) { + if ($node === $this->returnToBeRemoved) { + $this->returnToBeRemoved = null; + $this->removeNode = true; + } + + return null; + } + + if ($this->shouldSkip($node)) { + return null; + } + + return $this->processForeachNode($node); + } + + private function shouldSkip(Foreach_ $foreachNode): bool + { + if (! $foreachNode->keyVar) { + return true; + } + + if (count($foreachNode->stmts) !== 1) { + return true; + } + + $insideForeachStmt = $foreachNode->stmts[0]; + if (! $insideForeachStmt instanceof If_) { + return true; + } + + if (! $insideForeachStmt->cond instanceof Identical) { + return true; + } + + if (count($insideForeachStmt->stmts) !== 1) { + return true; + } + + return false; + } + + private function processForeachNode(Foreach_ $node): ?Node + { + /** @var If_ $insideForeachStmt */ + $insideForeachStmt = $node->stmts[0]; + + $returnOrAssignNode = $insideForeachStmt->stmts[0]; + + /** @var Return_|Assign|null $insideReturnOrAssignNode */ + $insideReturnOrAssignNode = $returnOrAssignNode instanceof Expression ? $returnOrAssignNode->expr : $returnOrAssignNode; + if ($insideReturnOrAssignNode === null) { + return null; + } + + // return $newValue; + // we don't return the node value + if (! $this->areNodesEqual($node->valueVar, $insideReturnOrAssignNode->expr)) { + return null; + } + + if ($insideReturnOrAssignNode instanceof Return_) { + return $this->processForeachNodeWithReturnInside($node, $insideReturnOrAssignNode); + } + + if ($insideReturnOrAssignNode instanceof Assign) { + return $this->processForeachNodeWithAssignInside($node, $insideReturnOrAssignNode); + } + } + + private function processForeachNodeWithReturnInside(Foreach_ $foreachNode, Return_ $returnNode): ?Node + { + if (! $this->areNodesEqual($foreachNode->valueVar, $returnNode->expr)) { + return null; + } + + /** @var If_ $ifNode */ + $ifNode = $foreachNode->stmts[0]; + + /** @var Identical $identicalNode */ + $identicalNode = $ifNode->cond; + + if ($this->areNodesEqual($identicalNode->left, $foreachNode->keyVar)) { + $checkedNode = $identicalNode->right; + } elseif ($this->areNodesEqual($identicalNode->right, $foreachNode->keyVar)) { + $checkedNode = $identicalNode->left; + } else { + return null; + } + + // is next node Return? + if ($foreachNode->getAttribute(Attribute::NEXT_NODE) instanceof Return_) { + $this->returnToBeRemoved = $foreachNode->getAttribute(Attribute::NEXT_NODE); + } + + $coalesceNode = new Coalesce(new ArrayDimFetch( + $foreachNode->expr, + $checkedNode + ), $this->returnToBeRemoved ? $this->returnToBeRemoved->expr : $checkedNode); + + if ($this->returnToBeRemoved) { + return new Return_($coalesceNode); + } + + return null; + } + + private function processForeachNodeWithAssignInside(Foreach_ $foreachNode, Assign $assignNode): ?Node + { + /** @var If_ $ifNode */ + $ifNode = $foreachNode->stmts[0]; + + /** @var Identical $identicalNode */ + $identicalNode = $ifNode->cond; + + if ($this->areNodesEqual($identicalNode->left, $foreachNode->keyVar)) { + $checkedNode = $assignNode->var; + } elseif ($this->areNodesEqual($identicalNode->right, $foreachNode->keyVar)) { + $checkedNode = $assignNode->var; + } else { + return null; + } + + $assignNode = new Assign($checkedNode, new Coalesce(new ArrayDimFetch( + $foreachNode->expr, + $foreachNode->valueVar + ), $checkedNode)); + + return new Expression($assignNode); + } +} diff --git a/packages/CodeQuality/tests/Rector/Foreach_/SimplifyForeachToCoalescingRector/Correct/correct.php.inc b/packages/CodeQuality/tests/Rector/Foreach_/SimplifyForeachToCoalescingRector/Correct/correct.php.inc new file mode 100644 index 00000000000..e78ef15fcbb --- /dev/null +++ b/packages/CodeQuality/tests/Rector/Foreach_/SimplifyForeachToCoalescingRector/Correct/correct.php.inc @@ -0,0 +1,9 @@ +oldToNewFunctions[$currentFunction] ?? 45; +foreach ($this->oldToNewFunctions as $oldFunction => $newFunction) { + if ($currentFunction === $oldFunction) { + return 15; + } +} +return 0; \ No newline at end of file diff --git a/packages/CodeQuality/tests/Rector/Foreach_/SimplifyForeachToCoalescingRector/Correct/correct2.php.inc b/packages/CodeQuality/tests/Rector/Foreach_/SimplifyForeachToCoalescingRector/Correct/correct2.php.inc new file mode 100644 index 00000000000..fdbe8a8ab50 --- /dev/null +++ b/packages/CodeQuality/tests/Rector/Foreach_/SimplifyForeachToCoalescingRector/Correct/correct2.php.inc @@ -0,0 +1,3 @@ +oldToNewFunctions[$currentFunction] ?? 45; \ No newline at end of file diff --git a/packages/CodeQuality/tests/Rector/Foreach_/SimplifyForeachToCoalescingRector/Correct/correct3.php.inc b/packages/CodeQuality/tests/Rector/Foreach_/SimplifyForeachToCoalescingRector/Correct/correct3.php.inc new file mode 100644 index 00000000000..4618f1b5577 --- /dev/null +++ b/packages/CodeQuality/tests/Rector/Foreach_/SimplifyForeachToCoalescingRector/Correct/correct3.php.inc @@ -0,0 +1,6 @@ +doTestFileMatchesExpectedContent($wrong, $fixed); + } + + public function provideWrongToFixedFiles(): Iterator + { + yield [__DIR__ . '/Wrong/wrong.php.inc', __DIR__ . '/Correct/correct.php.inc']; + yield [__DIR__ . '/Wrong/wrong2.php.inc', __DIR__ . '/Correct/correct2.php.inc']; + yield [__DIR__ . '/Wrong/wrong3.php.inc', __DIR__ . '/Correct/correct3.php.inc']; + } + + protected function provideConfig(): string + { + return __DIR__ . '/config.yml'; + } +} diff --git a/packages/CodeQuality/tests/Rector/Foreach_/SimplifyForeachToCoalescingRector/Wrong/wrong.php.inc b/packages/CodeQuality/tests/Rector/Foreach_/SimplifyForeachToCoalescingRector/Wrong/wrong.php.inc new file mode 100644 index 00000000000..e5f554fdb0c --- /dev/null +++ b/packages/CodeQuality/tests/Rector/Foreach_/SimplifyForeachToCoalescingRector/Wrong/wrong.php.inc @@ -0,0 +1,16 @@ +oldToNewFunctions as $oldFunction => $newFunction) { + if ($currentFunction === $oldFunction) { + return $newFunction; + } +} + +return 45; + +foreach ($this->oldToNewFunctions as $oldFunction => $newFunction) { + if ($currentFunction === $oldFunction) { + return 15; + } +} +return 0; diff --git a/packages/CodeQuality/tests/Rector/Foreach_/SimplifyForeachToCoalescingRector/Wrong/wrong2.php.inc b/packages/CodeQuality/tests/Rector/Foreach_/SimplifyForeachToCoalescingRector/Wrong/wrong2.php.inc new file mode 100644 index 00000000000..ec6acef22bc --- /dev/null +++ b/packages/CodeQuality/tests/Rector/Foreach_/SimplifyForeachToCoalescingRector/Wrong/wrong2.php.inc @@ -0,0 +1,9 @@ +oldToNewFunctions as $oldFunction => $newFunction) { + if ($oldFunction === $currentFunction) { + return $newFunction; + } +} + +return 45; diff --git a/packages/CodeQuality/tests/Rector/Foreach_/SimplifyForeachToCoalescingRector/Wrong/wrong3.php.inc b/packages/CodeQuality/tests/Rector/Foreach_/SimplifyForeachToCoalescingRector/Wrong/wrong3.php.inc new file mode 100644 index 00000000000..e4e98a9b4ea --- /dev/null +++ b/packages/CodeQuality/tests/Rector/Foreach_/SimplifyForeachToCoalescingRector/Wrong/wrong3.php.inc @@ -0,0 +1,10 @@ + $value) { + if ($key === $input) { + $newValue = $value; + } +} diff --git a/packages/CodeQuality/tests/Rector/Foreach_/SimplifyForeachToCoalescingRector/config.yml b/packages/CodeQuality/tests/Rector/Foreach_/SimplifyForeachToCoalescingRector/config.yml new file mode 100644 index 00000000000..6d1373b17ae --- /dev/null +++ b/packages/CodeQuality/tests/Rector/Foreach_/SimplifyForeachToCoalescingRector/config.yml @@ -0,0 +1,2 @@ +services: + Rector\CodeQuality\Rector\Foreach_\SimplifyForeachToCoalescingRector: ~