diff --git a/rules-tests/CodeQuality/Rector/Foreach_/ForeachItemsAssignToEmptyArrayToAssignRector/Fixture/fixture.php.inc b/rules-tests/CodeQuality/Rector/Foreach_/ForeachItemsAssignToEmptyArrayToAssignRector/Fixture/fixture.php.inc deleted file mode 100644 index 7cb9a0f0558..00000000000 --- a/rules-tests/CodeQuality/Rector/Foreach_/ForeachItemsAssignToEmptyArrayToAssignRector/Fixture/fixture.php.inc +++ /dev/null @@ -1,31 +0,0 @@ - ------ - diff --git a/rules-tests/CodeQuality/Rector/Foreach_/ForeachItemsAssignToEmptyArrayToAssignRector/Fixture/skip_append_non_empty_array.php.inc b/rules-tests/CodeQuality/Rector/Foreach_/ForeachItemsAssignToEmptyArrayToAssignRector/Fixture/skip_append_non_empty_array.php.inc deleted file mode 100644 index 12f71e60ee9..00000000000 --- a/rules-tests/CodeQuality/Rector/Foreach_/ForeachItemsAssignToEmptyArrayToAssignRector/Fixture/skip_append_non_empty_array.php.inc +++ /dev/null @@ -1,21 +0,0 @@ -getItems() as $item) { - if ($item->hasGroup()) { - $groupList[] = ['id' => $item->getGroupId()]; - } else { - $list[] = ['id' => $item->getId()]; - } - } - - foreach ($groupList as $group) { - $list[] = $group; - } - - return $list; - } -} diff --git a/rules-tests/CodeQuality/Rector/Foreach_/ForeachItemsAssignToEmptyArrayToAssignRector/Fixture/skip_deep_append_foreach.php.inc b/rules-tests/CodeQuality/Rector/Foreach_/ForeachItemsAssignToEmptyArrayToAssignRector/Fixture/skip_deep_append_foreach.php.inc deleted file mode 100644 index f7fb00e9ff2..00000000000 --- a/rules-tests/CodeQuality/Rector/Foreach_/ForeachItemsAssignToEmptyArrayToAssignRector/Fixture/skip_deep_append_foreach.php.inc +++ /dev/null @@ -1,22 +0,0 @@ -files = []; - } - - private function resolveDependentFiles( - SomeDependency $someDependency, - Node $node, - MutatingScope $mutatingScope - ): void { - foreach ($someDependency->resolve($node, $mutatingScope) as $file) { - $this->files[] = $file; - } - } -} diff --git a/rules-tests/CodeQuality/Rector/Foreach_/ForeachItemsAssignToEmptyArrayToAssignRector/Fixture/skip_foreach_reused_assign.php.inc b/rules-tests/CodeQuality/Rector/Foreach_/ForeachItemsAssignToEmptyArrayToAssignRector/Fixture/skip_foreach_reused_assign.php.inc deleted file mode 100644 index ece6dd155ad..00000000000 --- a/rules-tests/CodeQuality/Rector/Foreach_/ForeachItemsAssignToEmptyArrayToAssignRector/Fixture/skip_foreach_reused_assign.php.inc +++ /dev/null @@ -1,25 +0,0 @@ - 1]; - - public function run() - { - $currentAlphabet = []; - - // Includes - foreach (\array_keys($this->includes) as $alphabet) { - foreach ($this->resolveAlphabetCharacters($alphabet) as $char) { - $currentAlphabet[] = $char; - } - } - } - - private function resolveAlphabetCharacters($alphabet): array - { - return ['a', 'b', 'c']; - } -} diff --git a/rules-tests/CodeQuality/Rector/Foreach_/ForeachItemsAssignToEmptyArrayToAssignRector/Fixture/skip_foreach_this_if_class_implements_iterator_aggregate.php.inc b/rules-tests/CodeQuality/Rector/Foreach_/ForeachItemsAssignToEmptyArrayToAssignRector/Fixture/skip_foreach_this_if_class_implements_iterator_aggregate.php.inc deleted file mode 100644 index 8f28111ccb5..00000000000 --- a/rules-tests/CodeQuality/Rector/Foreach_/ForeachItemsAssignToEmptyArrayToAssignRector/Fixture/skip_foreach_this_if_class_implements_iterator_aggregate.php.inc +++ /dev/null @@ -1,28 +0,0 @@ -items = [new \stdClass()]; - } - - public function getIterator(): \ArrayIterator - { - return new \ArrayIterator($this->items); - } - - public function getItems(): array - { - $items = []; - foreach($this as $item) { - $items[] = $item; - } - return $items; - } -} -?> diff --git a/rules-tests/CodeQuality/Rector/Foreach_/ForeachItemsAssignToEmptyArrayToAssignRector/Fixture/skip_foreach_when_iterated_item_is_iterator_aggregate.php.inc b/rules-tests/CodeQuality/Rector/Foreach_/ForeachItemsAssignToEmptyArrayToAssignRector/Fixture/skip_foreach_when_iterated_item_is_iterator_aggregate.php.inc deleted file mode 100644 index 5b0d9d01fd3..00000000000 --- a/rules-tests/CodeQuality/Rector/Foreach_/ForeachItemsAssignToEmptyArrayToAssignRector/Fixture/skip_foreach_when_iterated_item_is_iterator_aggregate.php.inc +++ /dev/null @@ -1,19 +0,0 @@ - diff --git a/rules-tests/CodeQuality/Rector/Foreach_/ForeachItemsAssignToEmptyArrayToAssignRector/Fixture/skip_mixed_type.php.inc b/rules-tests/CodeQuality/Rector/Foreach_/ForeachItemsAssignToEmptyArrayToAssignRector/Fixture/skip_mixed_type.php.inc deleted file mode 100644 index d12521a9bb4..00000000000 --- a/rules-tests/CodeQuality/Rector/Foreach_/ForeachItemsAssignToEmptyArrayToAssignRector/Fixture/skip_mixed_type.php.inc +++ /dev/null @@ -1,14 +0,0 @@ -rolesProviders as $rolesProvider) { - foreach ($rolesProvider->getRoles() as $role) { - $roles[] = $role; - } - } - - return $roles; - } -} diff --git a/rules-tests/CodeQuality/Rector/Foreach_/ForeachItemsAssignToEmptyArrayToAssignRector/Fixture/skip_non_empty_array.php.inc b/rules-tests/CodeQuality/Rector/Foreach_/ForeachItemsAssignToEmptyArrayToAssignRector/Fixture/skip_non_empty_array.php.inc deleted file mode 100644 index b16c478951e..00000000000 --- a/rules-tests/CodeQuality/Rector/Foreach_/ForeachItemsAssignToEmptyArrayToAssignRector/Fixture/skip_non_empty_array.php.inc +++ /dev/null @@ -1,14 +0,0 @@ - $value) { - foreach (self::assertIterable($fn($value, $key)) as $mapKey => $mapValue) { - $result[$mapKey] = $mapValue; - } - } - - return $result; - } - - public static function assertIterable($traversable) - { - if (is_iterable($traversable)) { - return $traversable; - } - - throw new \UnexpectedValueException('Traversable expected'); - } -} diff --git a/rules-tests/CodeQuality/Rector/Foreach_/ForeachItemsAssignToEmptyArrayToAssignRector/Fixture/skip_traversable.php.inc b/rules-tests/CodeQuality/Rector/Foreach_/ForeachItemsAssignToEmptyArrayToAssignRector/Fixture/skip_traversable.php.inc deleted file mode 100644 index 909d7f1ddb9..00000000000 --- a/rules-tests/CodeQuality/Rector/Foreach_/ForeachItemsAssignToEmptyArrayToAssignRector/Fixture/skip_traversable.php.inc +++ /dev/null @@ -1,19 +0,0 @@ -getTraversable() as $item) { - $items[] = $item; - } - } - - private function getTraversable(): iterable - { - yield 123; - } -} diff --git a/rules-tests/CodeQuality/Rector/Foreach_/ForeachItemsAssignToEmptyArrayToAssignRector/Fixture/skip_while.php.inc b/rules-tests/CodeQuality/Rector/Foreach_/ForeachItemsAssignToEmptyArrayToAssignRector/Fixture/skip_while.php.inc deleted file mode 100644 index 9dccb9e61b8..00000000000 --- a/rules-tests/CodeQuality/Rector/Foreach_/ForeachItemsAssignToEmptyArrayToAssignRector/Fixture/skip_while.php.inc +++ /dev/null @@ -1,16 +0,0 @@ -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/CodeQuality/Rector/Foreach_/ForeachItemsAssignToEmptyArrayToAssignRector/Source/IteratorAggregateImplementation.php b/rules-tests/CodeQuality/Rector/Foreach_/ForeachItemsAssignToEmptyArrayToAssignRector/Source/IteratorAggregateImplementation.php deleted file mode 100644 index f1c290c436d..00000000000 --- a/rules-tests/CodeQuality/Rector/Foreach_/ForeachItemsAssignToEmptyArrayToAssignRector/Source/IteratorAggregateImplementation.php +++ /dev/null @@ -1,23 +0,0 @@ -items = [new \stdClass()]; - } - - public function getIterator(): \ArrayIterator - { - return new \ArrayIterator($this->items); - } -} diff --git a/rules-tests/CodeQuality/Rector/Foreach_/ForeachItemsAssignToEmptyArrayToAssignRector/Source/RolesProvider.php b/rules-tests/CodeQuality/Rector/Foreach_/ForeachItemsAssignToEmptyArrayToAssignRector/Source/RolesProvider.php deleted file mode 100644 index 63ebfc54c94..00000000000 --- a/rules-tests/CodeQuality/Rector/Foreach_/ForeachItemsAssignToEmptyArrayToAssignRector/Source/RolesProvider.php +++ /dev/null @@ -1,16 +0,0 @@ -withRules([ForeachItemsAssignToEmptyArrayToAssignRector::class]); diff --git a/rules/CodeQuality/NodeAnalyzer/ForeachAnalyzer.php b/rules/CodeQuality/NodeAnalyzer/ForeachAnalyzer.php deleted file mode 100644 index 993a41d9913..00000000000 --- a/rules/CodeQuality/NodeAnalyzer/ForeachAnalyzer.php +++ /dev/null @@ -1,56 +0,0 @@ - = $value; - * } - */ - public function matchAssignItemsOnlyForeachArrayVariable(Foreach_ $foreach): ?Expr - { - if (count($foreach->stmts) !== 1) { - return null; - } - - $onlyStatement = $foreach->stmts[0]; - if ($onlyStatement instanceof Expression) { - $onlyStatement = $onlyStatement->expr; - } - - if (! $onlyStatement instanceof Assign) { - return null; - } - - if (! $onlyStatement->var instanceof ArrayDimFetch) { - return null; - } - - if ($onlyStatement->var->dim instanceof Expr) { - return null; - } - - if (! $this->nodeComparator->areNodesEqual($foreach->valueVar, $onlyStatement->expr)) { - return null; - } - - return $onlyStatement->var->var; - } -} diff --git a/rules/CodeQuality/Rector/Foreach_/ForeachItemsAssignToEmptyArrayToAssignRector.php b/rules/CodeQuality/Rector/Foreach_/ForeachItemsAssignToEmptyArrayToAssignRector.php index debfef05f43..85224baebab 100644 --- a/rules/CodeQuality/Rector/Foreach_/ForeachItemsAssignToEmptyArrayToAssignRector.php +++ b/rules/CodeQuality/Rector/Foreach_/ForeachItemsAssignToEmptyArrayToAssignRector.php @@ -5,35 +5,18 @@ namespace Rector\CodeQuality\Rector\Foreach_; use PhpParser\Node; -use PhpParser\Node\Expr; -use PhpParser\Node\Expr\Array_; -use PhpParser\Node\Expr\ArrayDimFetch; -use PhpParser\Node\Expr\Assign; -use PhpParser\Node\Expr\Variable; -use PhpParser\Node\Stmt; -use PhpParser\Node\Stmt\Expression; -use PhpParser\Node\Stmt\Foreach_; -use PhpParser\NodeVisitor; -use Rector\CodeQuality\NodeAnalyzer\ForeachAnalyzer; -use Rector\NodeAnalyzer\ExprAnalyzer; +use Rector\Configuration\Deprecation\Contract\DeprecatedInterface; +use Rector\Exception\ShouldNotHappenException; use Rector\PhpParser\Enum\NodeGroup; -use Rector\PhpParser\Node\Value\ValueResolver; use Rector\Rector\AbstractRector; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; /** - * @see \Rector\Tests\CodeQuality\Rector\Foreach_\ForeachItemsAssignToEmptyArrayToAssignRector\ForeachItemsAssignToEmptyArrayToAssignRectorTest + * @deprecated This rule is deprecated, as the foreach assign and direct array assign are not equivalent - the foreach re-indexes keys from 0, while the direct assign keeps the original keys. This can create risky situations, e.g. with sparse arrays. */ -final class ForeachItemsAssignToEmptyArrayToAssignRector extends AbstractRector +final class ForeachItemsAssignToEmptyArrayToAssignRector extends AbstractRector implements DeprecatedInterface { - public function __construct( - private readonly ForeachAnalyzer $foreachAnalyzer, - private readonly ValueResolver $valueResolver, - private readonly ExprAnalyzer $exprAnalyzer - ) { - } - public function getRuleDefinition(): RuleDefinition { return new RuleDefinition( @@ -78,134 +61,11 @@ public function getNodeTypes(): array return NodeGroup::STMTS_AWARE; } - /** - * @param StmtsAware $node - */ public function refactor(Node $node): ?Node { - if ($node->stmts === null) { - return null; - } - - $emptyArrayVariables = []; - - foreach ($node->stmts as $key => $stmt) { - $variableName = $this->matchEmptyArrayVariableAssign($stmt); - if (is_string($variableName)) { - $emptyArrayVariables[] = $variableName; - } - - if (! $stmt instanceof Foreach_) { - if ($this->isAppend($stmt, $emptyArrayVariables)) { - return null; - } - - continue; - } - - if ($this->shouldSkip($stmt, $emptyArrayVariables)) { - if ($this->isAppend($stmt, $emptyArrayVariables)) { - return null; - } - - continue; - } - - $assignVariable = $this->foreachAnalyzer->matchAssignItemsOnlyForeachArrayVariable($stmt); - if (! $assignVariable instanceof Expr) { - continue; - } - - $directAssign = new Assign($assignVariable, $stmt->expr); - $node->stmts[$key] = new Expression($directAssign); - - return $node; - } - - return null; - } - - /** - * @param string[] $emptyArrayVariables - */ - private function isAppend(Stmt $stmt, array $emptyArrayVariables): bool - { - $isAppend = false; - - $this->traverseNodesWithCallable( - $stmt, - function (Node $subNode) use ($emptyArrayVariables, &$isAppend): ?int { - if ($subNode instanceof Assign && $subNode->var instanceof ArrayDimFetch) { - $isAppend = $this->isNames($subNode->var->var, $emptyArrayVariables); - - if ($isAppend) { - return NodeVisitor::STOP_TRAVERSAL; - } - } - - if ($subNode instanceof Assign && - $subNode->var instanceof Variable && - $this->isNames($subNode->var, $emptyArrayVariables) && - ! $this->valueResolver->isValue($subNode->expr, [])) { - $isAppend = true; - return NodeVisitor::STOP_TRAVERSAL; - } - - return null; - } - ); - - return $isAppend; - } - - /** - * @param string[] $emptyArrayVariables - */ - private function shouldSkip(Foreach_ $foreach, array $emptyArrayVariables): bool - { - $assignVariableExpr = $this->foreachAnalyzer->matchAssignItemsOnlyForeachArrayVariable($foreach); - if (! $assignVariableExpr instanceof Expr) { - return true; - } - - $foreachedExprType = $this->nodeTypeResolver->getNativeType($foreach->expr); - - // only arrays, not traversable/iterable - if (! $foreachedExprType->isArray()->yes()) { - return true; - } - - return ! $this->isNames($assignVariableExpr, $emptyArrayVariables); - } - - private function matchEmptyArrayVariableAssign(Stmt $stmt): ?string - { - if (! $stmt instanceof Expression) { - return null; - } - - if (! $stmt->expr instanceof Assign) { - return null; - } - - $assign = $stmt->expr; - if (! $assign->var instanceof Variable) { - return null; - } - - if (! $assign->expr instanceof Array_) { - return null; - } - - // must be assign of empty array - if (! $this->valueResolver->isValue($assign->expr, [])) { - return null; - } - - if ($this->exprAnalyzer->isDynamicArray($assign->expr)) { - return null; - } - - return $this->getName($assign->var); + throw new ShouldNotHappenException(sprintf( + '"%s" rule is deprecated, as the foreach re-indexes keys from 0 while direct assign keeps original keys, which can create risky situations', + self::class + )); } } diff --git a/src/Config/Level/CodeQualityLevel.php b/src/Config/Level/CodeQualityLevel.php index 47c61128a2a..69f118a6056 100644 --- a/src/Config/Level/CodeQualityLevel.php +++ b/src/Config/Level/CodeQualityLevel.php @@ -31,7 +31,6 @@ use Rector\CodeQuality\Rector\Expression\InlineIfToExplicitIfRector; use Rector\CodeQuality\Rector\Expression\TernaryFalseExpressionToIfRector; use Rector\CodeQuality\Rector\For_\ForRepeatedCountToOwnVariableRector; -use Rector\CodeQuality\Rector\Foreach_\ForeachItemsAssignToEmptyArrayToAssignRector; use Rector\CodeQuality\Rector\Foreach_\ForeachToInArrayRector; use Rector\CodeQuality\Rector\Foreach_\SimplifyForeachToCoalescingRector; use Rector\CodeQuality\Rector\FuncCall\ArrayMergeOfNonArraysToSimpleArrayRector; @@ -152,7 +151,6 @@ final class CodeQualityLevel AbsolutizeRequireAndIncludePathRector::class, ChangeArrayPushToArrayAssignRector::class, ForRepeatedCountToOwnVariableRector::class, - ForeachItemsAssignToEmptyArrayToAssignRector::class, CommonNotEqualRector::class, SetTypeToCastRector::class, LogicalToBooleanRector::class,