From 85b49bff4470389bfcc2b16f88064ebab840218c Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Fri, 4 Aug 2023 19:06:37 +0200 Subject: [PATCH 1/2] [CodeQuality] Make variable name simpler, as used just once --- .../already_existing_items_count.php.inc | 4 +-- .../Fixture/fallback_for_complex.php.inc | 4 +-- .../Fixture/fixture.php.inc | 4 +-- .../Fixture/method_call_count.php.inc | 4 +-- .../ForRepeatedCountToOwnVariableRector.php | 33 +++++++------------ .../config/configured_rule.php | 2 +- 6 files changed, 21 insertions(+), 30 deletions(-) diff --git a/rules-tests/CodeQuality/Rector/For_/ForRepeatedCountToOwnVariableRector/Fixture/already_existing_items_count.php.inc b/rules-tests/CodeQuality/Rector/For_/ForRepeatedCountToOwnVariableRector/Fixture/already_existing_items_count.php.inc index 764aeab543e..1679797fbc2 100644 --- a/rules-tests/CodeQuality/Rector/For_/ForRepeatedCountToOwnVariableRector/Fixture/already_existing_items_count.php.inc +++ b/rules-tests/CodeQuality/Rector/For_/ForRepeatedCountToOwnVariableRector/Fixture/already_existing_items_count.php.inc @@ -27,9 +27,9 @@ class AlreadyExistingItemsCount public function run($items, \stdClass $someObject) { $itemsCount = 500000; - $itemsCount2 = count($someObject->getItems() + 10); + $counter = count($someObject->getItems() + 10); - for ($i = 5; $i <= $itemsCount2; $i++) { + for ($i = 5; $i <= $counter; $i++) { echo $items[$i]; } diff --git a/rules-tests/CodeQuality/Rector/For_/ForRepeatedCountToOwnVariableRector/Fixture/fallback_for_complex.php.inc b/rules-tests/CodeQuality/Rector/For_/ForRepeatedCountToOwnVariableRector/Fixture/fallback_for_complex.php.inc index cb94dd48927..2093a7ea9a6 100644 --- a/rules-tests/CodeQuality/Rector/For_/ForRepeatedCountToOwnVariableRector/Fixture/fallback_for_complex.php.inc +++ b/rules-tests/CodeQuality/Rector/For_/ForRepeatedCountToOwnVariableRector/Fixture/fallback_for_complex.php.inc @@ -22,8 +22,8 @@ class FallbackForComplex { public function run($items, \stdClass $someObject) { - $itemsCount = count($someObject->getItems() + 10); - for ($i = 5; $i <= $itemsCount; $i++) { + $counter = count($someObject->getItems() + 10); + for ($i = 5; $i <= $counter; $i++) { echo $items[$i]; } } diff --git a/rules-tests/CodeQuality/Rector/For_/ForRepeatedCountToOwnVariableRector/Fixture/fixture.php.inc b/rules-tests/CodeQuality/Rector/For_/ForRepeatedCountToOwnVariableRector/Fixture/fixture.php.inc index 880020a440f..9868c392aed 100644 --- a/rules-tests/CodeQuality/Rector/For_/ForRepeatedCountToOwnVariableRector/Fixture/fixture.php.inc +++ b/rules-tests/CodeQuality/Rector/For_/ForRepeatedCountToOwnVariableRector/Fixture/fixture.php.inc @@ -22,8 +22,8 @@ class Fixture { public function run($items) { - $itemsCount = count($items); - for ($i = 5; $i <= $itemsCount; $i++) { + $counter = count($items); + for ($i = 5; $i <= $counter; $i++) { echo $items[$i]; } } diff --git a/rules-tests/CodeQuality/Rector/For_/ForRepeatedCountToOwnVariableRector/Fixture/method_call_count.php.inc b/rules-tests/CodeQuality/Rector/For_/ForRepeatedCountToOwnVariableRector/Fixture/method_call_count.php.inc index 0b491836d6f..59f928b4d11 100644 --- a/rules-tests/CodeQuality/Rector/For_/ForRepeatedCountToOwnVariableRector/Fixture/method_call_count.php.inc +++ b/rules-tests/CodeQuality/Rector/For_/ForRepeatedCountToOwnVariableRector/Fixture/method_call_count.php.inc @@ -22,8 +22,8 @@ class MethodCallCount { public function run($items, \stdClass $someObject) { - $getItemsCount = count($someObject->getItems()); - for ($i = 5; $i <= $getItemsCount; $i++) { + $counter = count($someObject->getItems()); + for ($i = 5; $i <= $counter; $i++) { echo $items[$i]; } } diff --git a/rules/CodeQuality/Rector/For_/ForRepeatedCountToOwnVariableRector.php b/rules/CodeQuality/Rector/For_/ForRepeatedCountToOwnVariableRector.php index 36ab1b80eb4..500dcab0f63 100644 --- a/rules/CodeQuality/Rector/For_/ForRepeatedCountToOwnVariableRector.php +++ b/rules/CodeQuality/Rector/For_/ForRepeatedCountToOwnVariableRector.php @@ -14,21 +14,19 @@ use PhpParser\Node\Stmt; use PhpParser\Node\Stmt\Expression; use PhpParser\Node\Stmt\For_; -use PHPStan\Analyser\Scope; -use Rector\Core\Rector\AbstractScopeAwareRector; -use Rector\Naming\Naming\VariableNaming; +use Rector\Core\Rector\AbstractRector; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; /** * @see \Rector\Tests\CodeQuality\Rector\For_\ForRepeatedCountToOwnVariableRector\ForRepeatedCountToOwnVariableRectorTest */ -final class ForRepeatedCountToOwnVariableRector extends AbstractScopeAwareRector +final class ForRepeatedCountToOwnVariableRector extends AbstractRector { - public function __construct( - private readonly VariableNaming $variableNaming - ) { - } + /** + * @var string + */ + private const COUNTER_NAME = 'counter'; public function getRuleDefinition(): RuleDefinition { @@ -77,10 +75,10 @@ public function getNodeTypes(): array * @param For_ $node * @return Stmt[]|null */ - public function refactorWithScope(Node $node, Scope $scope): ?array + public function refactor(Node $node): ?array { - $variableName = null; $countInCond = null; + $counterVariable = new Variable(self::COUNTER_NAME); foreach ($node->cond as $condExpr) { if (! $condExpr instanceof Smaller && ! $condExpr instanceof SmallerOrEqual) { @@ -96,23 +94,16 @@ public function refactorWithScope(Node $node, Scope $scope): ?array continue; } - $variableName = $this->variableNaming->resolveFromFuncCallFirstArgumentWithSuffix( - $funcCall, - 'Count', - 'itemsCount', - $scope - ); - $countInCond = $condExpr->right; - - $condExpr->right = new Variable($variableName); + $condExpr->right = $counterVariable; } - if (! is_string($variableName) || ! $countInCond instanceof Expr) { + if (! $countInCond instanceof Expr) { return null; } - $countAssign = new Assign(new Variable($variableName), $countInCond); + $countAssign = new Assign($counterVariable, $countInCond); + return [new Expression($countAssign), $node]; } } diff --git a/tests/Issues/IssueTraitUseKey/config/configured_rule.php b/tests/Issues/IssueTraitUseKey/config/configured_rule.php index 4d3b5fbe87c..b72984fdbf5 100644 --- a/tests/Issues/IssueTraitUseKey/config/configured_rule.php +++ b/tests/Issues/IssueTraitUseKey/config/configured_rule.php @@ -2,8 +2,8 @@ declare(strict_types=1); -use Rector\Config\RectorConfig; use Rector\CodingStyle\Rector\Use_\SeparateMultiUseImportsRector; +use Rector\Config\RectorConfig; use Rector\Php80\Rector\Class_\ClassPropertyAssignToConstructorPromotionRector; use Rector\Privatization\Rector\Class_\FinalizeClassesWithoutChildrenRector; From 2b734e6994b844fafd6b492d1fae8b42fc895420 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Fri, 4 Aug 2023 19:07:31 +0200 Subject: [PATCH 2/2] cleanup variable naming --- rules/Naming/Naming/VariableNaming.php | 39 ------------------- .../Fixture/crash_indentation2.php.inc | 4 +- 2 files changed, 2 insertions(+), 41 deletions(-) diff --git a/rules/Naming/Naming/VariableNaming.php b/rules/Naming/Naming/VariableNaming.php index 89189bacd9c..3b061a20ab1 100644 --- a/rules/Naming/Naming/VariableNaming.php +++ b/rules/Naming/Naming/VariableNaming.php @@ -88,16 +88,6 @@ public function createCountedValueName(string $valueName, ?Scope $scope): string return $valueName; } - public function resolveFromFuncCallFirstArgumentWithSuffix( - FuncCall $funcCall, - string $suffix, - string $fallbackName, - ?Scope $scope - ): string { - $bareName = $this->resolveBareFuncCallArgumentName($funcCall, $fallbackName, $suffix); - return $this->createCountedValueName($bareName, $scope); - } - private function resolveFromNodeAndType(Node $node, Type $type): ?string { $variableName = $this->resolveBareFromNode($node); @@ -188,33 +178,4 @@ private function unwrapNode(Node $node): ?Node return $node; } - - private function resolveBareFuncCallArgumentName( - FuncCall $funcCall, - string $fallbackName, - string $suffix - ): string { - if ($funcCall->isFirstClassCallable()) { - return ''; - } - - if (! isset($funcCall->getArgs()[0])) { - return ''; - } - - $argumentValue = $funcCall->getArgs()[0] -->value; - - if ($argumentValue instanceof MethodCall || $argumentValue instanceof StaticCall) { - $name = $this->nodeNameResolver->getName($argumentValue->name); - } else { - $name = $this->nodeNameResolver->getName($argumentValue); - } - - if ($name === null) { - return $fallbackName; - } - - return $name . $suffix; - } } diff --git a/tests/Issues/IndentationCrash/Fixture/crash_indentation2.php.inc b/tests/Issues/IndentationCrash/Fixture/crash_indentation2.php.inc index 0459c96824b..9e0650430f5 100644 --- a/tests/Issues/IndentationCrash/Fixture/crash_indentation2.php.inc +++ b/tests/Issues/IndentationCrash/Fixture/crash_indentation2.php.inc @@ -15,9 +15,9 @@ for ($i = 0; $i < sizeof($psychiatry); $i++) { namespace Rector\Core\Tests\Issues\IndentationCrash\Fixture; $psychiatry = [1, 2, 3]; -$psychiatryCount = count($psychiatry); +$counter = count($psychiatry); -for ($i = 0; $i < $psychiatryCount; $i++) { +for ($i = 0; $i < $counter; $i++) { $psychiatry[$i]["sufficient_cancellation_notice"] = mt_rand(1, 3) ? 1 : 0; }