Skip to content

Commit

Permalink
[PHP 7.2] Add error suppres support to each() rule (#5844)
Browse files Browse the repository at this point in the history
  • Loading branch information
@TomasVotruba
TomasVotruba committed Apr 24, 2024
1 parent d5069f0 commit 8281a06
Show file tree
Hide file tree
Showing 6 changed files with 113 additions and 51 deletions.
31 changes: 31 additions & 0 deletions rules-tests/Php72/Rector/While_/WhileEachToForeachRector/Fixture/include_supress.php.inc
View file
@@ -0,0 +1,31 @@
<?php

namespace Rector\Tests\Php72\Rector\While_\WhileEachToForeachRector\Fixture;

final class IncludeSupress
{
public function run(array $items)
{
while (list(, $item) = @each($items)) {
// some code
}
}
}

?>
-----
<?php

namespace Rector\Tests\Php72\Rector\While_\WhileEachToForeachRector\Fixture;

final class IncludeSupress
{
public function run(array $items)
{
foreach ($items as $item) {
// some code
}
}
}

?>
2 changes: 1 addition & 1 deletion rules/DeadCode/NodeAnalyzer/IsClassMethodUsedAnalyzer.php
View file
Expand Up @@ -4,13 +4,13 @@

namespace Rector\DeadCode\NodeAnalyzer;

use PhpParser\Node\Expr\NullsafeMethodCall;
use PhpParser\Node;
use PhpParser\Node\Arg;
use PhpParser\Node\Expr\Array_;
use PhpParser\Node\Expr\ArrayItem;
use PhpParser\Node\Expr\CallLike;
use PhpParser\Node\Expr\MethodCall;
use PhpParser\Node\Expr\NullsafeMethodCall;
use PhpParser\Node\Expr\StaticCall;
use PhpParser\Node\Name;
use PhpParser\Node\Stmt\Class_;
Expand Down
44 changes: 19 additions & 25 deletions rules/Php72/Rector/Assign/ListEachRector.php
View file
Expand Up @@ -7,12 +7,11 @@
use PhpParser\Node;
use PhpParser\Node\Expr\ArrayItem;
use PhpParser\Node\Expr\Assign;
use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Expr\List_;
use PhpParser\Node\Stmt;
use PhpParser\Node\Stmt\Expression;
use Rector\Exception\ShouldNotHappenException;
use Rector\NodeManipulator\AssignManipulator;
use Rector\Php72\ValueObject\ListAndEach;
use Rector\Rector\AbstractRector;
use Rector\ValueObject\PhpVersionFeature;
use Rector\VersionBonding\Contract\MinPhpVersionInterface;
Expand Down Expand Up @@ -74,31 +73,32 @@ public function refactor(Node $node): null|Expression|array
return null;
}

$assign = $node->expr;
if ($this->shouldSkipAssign($assign)) {
$listAndEach = $this->assignManipulator->matchListAndEach($node->expr);
if (! $listAndEach instanceof ListAndEach) {
return null;
}

/** @var List_ $listNode */
$listNode = $assign->var;
if ($this->shouldSkipAssign($listAndEach)) {
return null;
}

/** @var FuncCall $eachFuncCall */
$eachFuncCall = $assign->expr;
$list = $listAndEach->getList();
$eachFuncCall = $listAndEach->getEachFuncCall();

// only key: list($key, ) = each($values);
if ($listNode->items[0] instanceof ArrayItem && ! $listNode->items[1] instanceof ArrayItem) {
if ($list->items[0] instanceof ArrayItem && ! $list->items[1] instanceof ArrayItem) {
$keyFuncCall = $this->nodeFactory->createFuncCall('key', $eachFuncCall->args);
$keyFuncCallAssign = new Assign($listNode->items[0]->value, $keyFuncCall);
$keyFuncCallAssign = new Assign($list->items[0]->value, $keyFuncCall);

return new Expression($keyFuncCallAssign);
}

// only value: list(, $value) = each($values);
if ($listNode->items[1] instanceof ArrayItem && ! $listNode->items[0] instanceof ArrayItem) {
if ($list->items[1] instanceof ArrayItem && ! $list->items[0] instanceof ArrayItem) {
$nextFuncCall = $this->nodeFactory->createFuncCall('next', $eachFuncCall->args);
$currentFuncCall = $this->nodeFactory->createFuncCall('current', $eachFuncCall->args);

$secondArrayItem = $listNode->items[1];
$secondArrayItem = $list->items[1];
$currentAssign = new Assign($secondArrayItem->value, $currentFuncCall);

return [new Expression($currentAssign), new Expression($nextFuncCall)];
Expand All @@ -107,7 +107,7 @@ public function refactor(Node $node): null|Expression|array
// both: list($key, $value) = each($values);
$currentFuncCall = $this->nodeFactory->createFuncCall('current', $eachFuncCall->args);

$secondArrayItem = $listNode->items[1];
$secondArrayItem = $list->items[1];
if (! $secondArrayItem instanceof ArrayItem) {
throw new ShouldNotHappenException();
}
Expand All @@ -117,7 +117,7 @@ public function refactor(Node $node): null|Expression|array
$nextFuncCall = $this->nodeFactory->createFuncCall('next', $eachFuncCall->args);
$keyFuncCall = $this->nodeFactory->createFuncCall('key', $eachFuncCall->args);

$firstArrayItem = $listNode->items[0];
$firstArrayItem = $list->items[0];
if (! $firstArrayItem instanceof ArrayItem) {
throw new ShouldNotHappenException();
}
Expand All @@ -127,24 +127,18 @@ public function refactor(Node $node): null|Expression|array
return [new Expression($keyAssign), new Expression($currentAssign), new Expression($nextFuncCall)];
}

private function shouldSkipAssign(Assign $assign): bool
private function shouldSkipAssign(ListAndEach $listAndEach): bool
{
if (! $this->assignManipulator->isListToEachAssign($assign)) {
return true;
}

/** @var List_ $listNode */
$listNode = $assign->var;

if (count($listNode->items) !== 2) {
$list = $listAndEach->getList();
if (count($list->items) !== 2) {
return true;
}

// empty list → cannot handle
if ($listNode->items[0] instanceof ArrayItem) {
if ($list->items[0] instanceof ArrayItem) {
return false;
}

return ! $listNode->items[1] instanceof ArrayItem;
return ! $list->items[1] instanceof ArrayItem;
}
}
29 changes: 10 additions & 19 deletions rules/Php72/Rector/While_/WhileEachToForeachRector.php
View file
Expand Up @@ -7,11 +7,10 @@
use PhpParser\Node;
use PhpParser\Node\Expr\ArrayItem;
use PhpParser\Node\Expr\Assign;
use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Expr\List_;
use PhpParser\Node\Stmt\Foreach_;
use PhpParser\Node\Stmt\While_;
use Rector\NodeManipulator\AssignManipulator;
use Rector\Php72\ValueObject\ListAndEach;
use Rector\Rector\AbstractRector;
use Rector\ValueObject\PhpVersionFeature;
use Rector\VersionBonding\Contract\MinPhpVersionInterface;
Expand Down Expand Up @@ -87,41 +86,33 @@ public function refactor(Node $node): ?Node
return null;
}

/** @var Assign $assignNode */
$assignNode = $node->cond;
if (! $this->assignManipulator->isListToEachAssign($assignNode)) {
$listAndEach = $this->assignManipulator->matchListAndEach($node->cond);
if (! $listAndEach instanceof ListAndEach) {
return null;
}

/** @var FuncCall $eachFuncCall */
$eachFuncCall = $assignNode->expr;

if ($eachFuncCall->isFirstClassCallable()) {
return null;
}

/** @var List_ $listNode */
$listNode = $assignNode->var;
$eachFuncCall = $listAndEach->getEachFuncCall();

$list = $listAndEach->getList();
if (! isset($eachFuncCall->getArgs()[0])) {
return null;
}

$firstArg = $eachFuncCall->getArgs()[0];

$foreachedExpr = count($listNode->items) === 1 ? $this->nodeFactory->createFuncCall(
$foreachedExpr = count($list->items) === 1 ? $this->nodeFactory->createFuncCall(
'array_keys',
[$firstArg]
) : $firstArg->value;

$arrayItem = array_pop($listNode->items);
$arrayItem = array_pop($list->items);

$isTrailingCommaLast = false;

if (! $arrayItem instanceof ArrayItem) {
$foreachedExpr = $this->nodeFactory->createFuncCall('array_keys', [$eachFuncCall->args[0]]);
/** @var ArrayItem $arrayItem */
$arrayItem = current($listNode->items);
$arrayItem = current($list->items);
$isTrailingCommaLast = true;
}

Expand All @@ -132,9 +123,9 @@ public function refactor(Node $node): ?Node
$this->mirrorComments($foreach, $node);

// is key included? add it to foreach
if ($listNode->items !== []) {
if ($list->items !== []) {
/** @var ArrayItem|null $keyItem */
$keyItem = array_pop($listNode->items);
$keyItem = array_pop($list->items);

if ($keyItem instanceof ArrayItem && ! $isTrailingCommaLast) {
$foreach->keyVar = $keyItem->value;
Expand Down
27 changes: 27 additions & 0 deletions rules/Php72/ValueObject/ListAndEach.php
View file
@@ -0,0 +1,27 @@
<?php

declare(strict_types=1);

namespace Rector\Php72\ValueObject;

use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Expr\List_;

final readonly class ListAndEach
{
public function __construct(
private List_ $list,
private FuncCall $eachFuncCall,
) {
}

public function getList(): List_
{
return $this->list;
}

public function getEachFuncCall(): FuncCall
{
return $this->eachFuncCall;
}
}
31 changes: 25 additions & 6 deletions src/NodeManipulator/AssignManipulator.php
View file
Expand Up @@ -6,6 +6,7 @@

use PhpParser\Node;
use PhpParser\Node\Expr\Assign;
use PhpParser\Node\Expr\ErrorSuppress;
use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Expr\List_;
use PhpParser\Node\Expr\PropertyFetch;
Expand All @@ -14,6 +15,7 @@
use Rector\NodeAnalyzer\PropertyFetchAnalyzer;
use Rector\NodeNameResolver\NodeNameResolver;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Rector\Php72\ValueObject\ListAndEach;
use Rector\PhpParser\Node\BetterNodeFinder;

final readonly class AssignManipulator
Expand All @@ -27,19 +29,36 @@ public function __construct(

/**
* Matches:
* each() = [1, 2];
* list([1, 2]) = each($items)
*/
public function isListToEachAssign(Assign $assign): bool
public function matchListAndEach(Assign $assign): ?ListAndEach
{
if (! $assign->expr instanceof FuncCall) {
return false;
// could be behind error suppress
if ($assign->expr instanceof ErrorSuppress) {
$errorSuppress = $assign->expr;
$bareExpr = $errorSuppress->expr;
} else {
$bareExpr = $assign->expr;
}

if (! $bareExpr instanceof FuncCall) {
return null;
}

if (! $assign->var instanceof List_) {
return false;
return null;
}

if (! $this->nodeNameResolver->isName($bareExpr, 'each')) {
return null;
}

// no placeholders
if ($bareExpr->isFirstClassCallable()) {
return null;
}

return $this->nodeNameResolver->isName($assign->expr, 'each');
return new ListAndEach($assign->var, $bareExpr);
}

public function isLeftPartOfAssign(Node $node): bool
Expand Down

0 comments on commit 8281a06

Please sign in to comment.