From bb7995b83bdd42d4b55d993c52df08528f550590 Mon Sep 17 00:00:00 2001 From: TomasVotruba Date: Sat, 4 Jan 2020 15:30:53 +0100 Subject: [PATCH] preven variable name override --- .../ForRepeatedCountToOwnVariableRector.php | 47 +++++++++++++++---- .../already_existing_items_count.php.inc | 40 ++++++++++++++++ 2 files changed, 79 insertions(+), 8 deletions(-) create mode 100644 packages/CodeQuality/tests/Rector/For_/ForRepeatedCountToOwnVariableRector/Fixture/already_existing_items_count.php.inc diff --git a/packages/CodeQuality/src/Rector/For_/ForRepeatedCountToOwnVariableRector.php b/packages/CodeQuality/src/Rector/For_/ForRepeatedCountToOwnVariableRector.php index edae1a1b9c17..bc3521d0a5e2 100644 --- a/packages/CodeQuality/src/Rector/For_/ForRepeatedCountToOwnVariableRector.php +++ b/packages/CodeQuality/src/Rector/For_/ForRepeatedCountToOwnVariableRector.php @@ -9,6 +9,8 @@ use PhpParser\Node\Expr\FuncCall; use PhpParser\Node\Expr\Variable; use PhpParser\Node\Stmt\For_; +use PHPStan\Analyser\Scope; +use Rector\NodeTypeResolver\Node\AttributeKey; use Rector\Rector\AbstractRector; use Rector\RectorDefinition\CodeSample; use Rector\RectorDefinition\RectorDefinition; @@ -71,8 +73,13 @@ public function refactor(Node $node): ?Node { $countInCond = null; $countedValueName = null; + $for = $node; - $this->traverseNodesWithCallable($node->cond, function (Node $node) use (&$countInCond, &$countedValueName) { + $this->traverseNodesWithCallable($node->cond, function (Node $node) use ( + &$countInCond, + &$countedValueName, + $for + ) { if (! $node instanceof FuncCall) { return null; } @@ -83,12 +90,7 @@ public function refactor(Node $node): ?Node $countInCond = $node; $valueName = $this->getName($node->args[0]->value); - - if ($valueName === null) { - $countedValueName = self::DEFAULT_VARIABLE_COUNT_NAME; - } else { - $countedValueName = $valueName . 'Count'; - } + $countedValueName = $this->createCountedValueName($valueName, $for); return new Variable($countedValueName); }); @@ -98,9 +100,38 @@ public function refactor(Node $node): ?Node } $countAssign = new Assign(new Variable($countedValueName), $countInCond); - $this->addNodeBeforeNode($countAssign, $node); return $node; } + + private function createCountedValueName(?string $valueName, For_ $for): string + { + if ($valueName === null) { + $countedValueName = self::DEFAULT_VARIABLE_COUNT_NAME; + } else { + $countedValueName = $valueName . 'Count'; + } + + /** @var Scope|null $forScope */ + $forScope = $for->getAttribute(AttributeKey::SCOPE); + if ($forScope === null) { + return $countedValueName; + } + + // make sure variable name is unique + if (! $forScope->hasVariableType($countedValueName)->yes()) { + return $countedValueName; + } + + // we need to add number suffix until the variable is unique + $i = 2; + $countedValueNamePart = $countedValueName; + while ($forScope->hasVariableType($countedValueName)->yes()) { + $countedValueName = $countedValueNamePart . $i; + ++$i; + } + + return $countedValueName; + } } diff --git a/packages/CodeQuality/tests/Rector/For_/ForRepeatedCountToOwnVariableRector/Fixture/already_existing_items_count.php.inc b/packages/CodeQuality/tests/Rector/For_/ForRepeatedCountToOwnVariableRector/Fixture/already_existing_items_count.php.inc new file mode 100644 index 000000000000..ebfef4512d18 --- /dev/null +++ b/packages/CodeQuality/tests/Rector/For_/ForRepeatedCountToOwnVariableRector/Fixture/already_existing_items_count.php.inc @@ -0,0 +1,40 @@ +getItems() + 10); $i++) { + echo $items[$i]; + } + + return $itemsCount; + } +} + +?> +----- +getItems() + 10); + + for ($i = 5; $i <= $itemsCount2; $i++) { + echo $items[$i]; + } + + return $itemsCount; + } +} + +?>