Skip to content

Commit

Permalink
Bugfix for SimplifyForeachToArrayFilterRector (#5859)
Browse files Browse the repository at this point in the history
  • Loading branch information
fonata authored Mar 15, 2021
1 parent c1c6c9a commit b84f76e
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 8 deletions.
2 changes: 1 addition & 1 deletion packages/Testing/PHPUnit/AbstractRectorTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ protected function doTestFileInfo(SmartFileInfo $fixtureFileInfo, array $extraFi

$inputFileInfo = $inputFileInfoAndExpectedFileInfo->getInputFileInfo();

// needed for PHPStan, because the analyzed file is just create in /temp
// needed for PHPStan, because the analyzed file is just created in /temp
/** @var NodeScopeResolver $nodeScopeResolver */
$nodeScopeResolver = $this->getService(NodeScopeResolver::class);
$nodeScopeResolver->setAnalysedFiles([$inputFileInfo->getRealPath()]);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?php

namespace Rector\Tests\CodeQuality\Rector\Foreach_\SimplifyForeachToArrayFilterRector\Fixture;

$responseData = [];
foreach ($responseData as $key => $value) {
if (is_string($value)) {
$responseData[$key] = $value;
}
}

?>
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use PhpParser\Node\Expr\ArrayDimFetch;
use PhpParser\Node\Expr\Assign;
use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Expr\Variable;
use PhpParser\Node\Name;
use PhpParser\Node\Scalar\String_;
use PhpParser\Node\Stmt\Expression;
Expand Down Expand Up @@ -73,11 +74,7 @@ public function refactor(Node $node): ?Node
return null;
}

if (count($funcCallNode->args) !== 1) {
return null;
}

if (! $this->nodeComparator->areNodesEqual($funcCallNode->args[0], $node->valueVar)) {
if (! $this->isSimpleCall($funcCallNode, $node)) {
return null;
}

Expand All @@ -90,7 +87,8 @@ public function refactor(Node $node): ?Node
return null;
}

if (! $onlyNodeInIf->var instanceof ArrayDimFetch) {
$arrayDimFetch = $onlyNodeInIf->var;
if (! $arrayDimFetch instanceof ArrayDimFetch) {
return null;
}

Expand All @@ -103,7 +101,11 @@ public function refactor(Node $node): ?Node
return null;
}

return $this->createAssignNode($node, $name, $onlyNodeInIf->var);
if (! $this->forLoopFillsAnotherArray($node, $arrayDimFetch)) {
return null;
}

return $this->createAssignNode($node, $name, $arrayDimFetch);
}

private function shouldSkip(Foreach_ $foreach): bool
Expand Down Expand Up @@ -139,4 +141,27 @@ private function createAssignNode(Foreach_ $foreach, string $name, ArrayDimFetch

return new Assign($arrayDimFetch->var, $arrayFilterFuncCall);
}

private function forLoopFillsAnotherArray(Foreach_ $node, ArrayDimFetch $arrayDimFetch): bool
{
$loopVar = $node->expr;
if (! $loopVar instanceof Variable) {
return false;
}
$varThatIsModified = $arrayDimFetch->var;
if (! $varThatIsModified instanceof Variable) {
return false;
}

return $loopVar->name !== $varThatIsModified->name;
}

private function isSimpleCall(FuncCall $funcCallNode, Foreach_ $foreach): bool
{
if (count($funcCallNode->args) !== 1) {
return false;
}

return $this->nodeComparator->areNodesEqual($funcCallNode->args[0], $foreach->valueVar);
}
}

0 comments on commit b84f76e

Please sign in to comment.