Merge pull request #678 from rectorphp/foreach-in-array-rector

Create ForeachToInArrayRector
This commit is contained in:
Gabriel Caruso 2018-10-23 16:04:22 -03:00 committed by GitHub
commit 958994e038
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 877 additions and 432 deletions

File diff suppressed because it is too large Load Diff

View File

@ -0,0 +1,184 @@
<?php declare(strict_types=1);
namespace Rector\CodeQuality\Rector\Foreach_;
use PhpParser\Node;
use PhpParser\Node\Expr\BinaryOp\Equal;
use PhpParser\Node\Expr\BinaryOp\Identical;
use PhpParser\Node\Expr\BooleanNot;
use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Expr\Variable;
use PhpParser\Node\Name;
use PhpParser\Node\Stmt\Foreach_;
use PhpParser\Node\Stmt\If_;
use PhpParser\Node\Stmt\Return_;
use Rector\Node\NodeFactory;
use Rector\NodeAnalyzer\ConstFetchAnalyzer;
use Rector\NodeTypeResolver\Node\Attribute;
use Rector\Rector\AbstractRector;
use Rector\RectorDefinition\CodeSample;
use Rector\RectorDefinition\RectorDefinition;
final class ForeachToInArrayRector extends AbstractRector
{
/**
* @var NodeFactory
*/
private $nodeFactory;
/**
* @var ConstFetchAnalyzer
*/
private $constFetchAnalyzer;
/**
* @var Return_
*/
private $returnNodeToRemove;
public function __construct(NodeFactory $nodeFactory, ConstFetchAnalyzer $constFetchAnalyzer)
{
$this->nodeFactory = $nodeFactory;
$this->constFetchAnalyzer = $constFetchAnalyzer;
}
public function getDefinition(): RectorDefinition
{
return new RectorDefinition(
'Simplify `foreach` loops into `in_array` when possible',
[
new CodeSample(
<<<'CODE_SAMPLE'
foreach ($items as $item) {
if ($item === 'something') {
return true;
}
}
return false;
CODE_SAMPLE
,
"in_array('something', \$items, true);"
),
]
);
}
/**
* @return string[]
*/
public function getNodeTypes(): array
{
return [Foreach_::class];
}
/**
* @param Foreach_ $foreachNode
*/
public function refactor(Node $foreachNode): ?Node
{
if (! $this->isAForeachCandidate($foreachNode)) {
return null;
}
$firstNodeInsideForeach = $foreachNode->stmts[0];
if (! $firstNodeInsideForeach instanceof If_) {
return null;
}
$ifCondition = $firstNodeInsideForeach->cond;
if (! $ifCondition instanceof Identical && ! $ifCondition instanceof Equal) {
return null;
}
$leftVariable = $ifCondition->left;
$rightVariable = $ifCondition->right;
if (! $leftVariable instanceof Variable && ! $rightVariable instanceof Variable) {
return null;
}
$condition = $this->normalizeYodaComparison($leftVariable, $rightVariable, $foreachNode);
if (! $this->isIfBodyABoolReturnNode($firstNodeInsideForeach)) {
return null;
}
$inArrayFunctionCall = $this->createInArrayFunction($condition, $ifCondition, $foreachNode);
$this->returnNodeToRemove = $foreachNode->getAttribute(Attribute::NEXT_NODE);
$this->removeNode($this->returnNodeToRemove);
/** @var Return_ $returnNode */
$returnNode = $firstNodeInsideForeach->stmts[0];
$negativeReturn = $this->constFetchAnalyzer->isFalse($returnNode->expr);
return new Return_($negativeReturn ? new BooleanNot($inArrayFunctionCall) : $inArrayFunctionCall);
}
/**
* @param mixed $leftValue
* @param mixed $rightValue
*
* @return mixed
*/
private function normalizeYodaComparison($leftValue, $rightValue, Foreach_ $foreachNode)
{
/** @var Variable $foreachVariable */
$foreachVariable = $foreachNode->valueVar;
if ($leftValue instanceof Variable) {
if ($leftValue->name === $foreachVariable->name) {
return $rightValue;
}
}
if ($rightValue->name === $foreachVariable->name) {
return $leftValue;
}
}
/**
* @param mixed $condition
* @param Identical|Equal $ifCondition
*/
private function createInArrayFunction($condition, $ifCondition, Foreach_ $foreachNode): FuncCall
{
$arguments = $this->nodeFactory->createArgs([$condition, $foreachNode->expr]);
if ($ifCondition instanceof Identical) {
$arguments[] = $this->nodeFactory->createArg($this->nodeFactory->createTrueConstant());
}
return new FuncCall(new Name('in_array'), $arguments);
}
private function isIfBodyABoolReturnNode(If_ $firstNodeInsideForeach): bool
{
$ifStatment = $firstNodeInsideForeach->stmts[0];
if (! $ifStatment instanceof Return_) {
return false;
}
return $this->constFetchAnalyzer->isBool($ifStatment->expr);
}
private function isAForeachCandidate(Foreach_ $foreachNode): bool
{
if (isset($foreachNode->keyVar)) {
return false;
}
$nextNode = $foreachNode->getAttribute(Attribute::NEXT_NODE);
if ($nextNode === null || ! $nextNode instanceof Return_) {
return false;
}
$returnExpression = $nextNode->expr;
return $returnExpression !== null && $this->constFetchAnalyzer->isBool($returnExpression);
}
}

View File

@ -0,0 +1,71 @@
<?php declare(strict_types=1);
final class MyClass
{
public function foreachToInArray() : bool
{
return in_array('something', $items);
}
public function foreachToInArrayYoda() : bool
{
return in_array('something', $items);
}
public function foreachToInArrayStrict() : bool
{
return in_array('something', $items, true);
}
public function invertedForeachToInArrayStrict() : bool
{
return !in_array('something', $items, true);
}
public function foreachToInArrayWithToVariables() : bool
{
return in_array($something, $items, true);
}
public function foreachWithoutReturnFalse()
{
foreach ($items as $item) {
if ($item === 'something') {
return true;
}
}
}
public function foreachReturnString()
{
foreach ($items as $item) {
if ($item === 'something') {
return true;
}
}
return 'false';
}
public function foreachWithSomethingElseAfterIt()
{
foreach ($items as $item) {
if ($item === 'something') {
return true;
}
}
$foo = 'bar';
}
public function foreachWithElseNullable()
{
foreach ($items as $item) {
if ('string') {
return true;
}
}
return;
}
}

View File

@ -0,0 +1,30 @@
<?php declare(strict_types=1);
namespace Rector\CodeQuality\Tests\Rector\Foreach_\ForeachToInArrayRector;
use Iterator;
use Rector\Testing\PHPUnit\AbstractRectorTestCase;
/**
* @covers \Rector\CodeQuality\Rector\Foreach_\ForeachToInArrayRector
*/
final class ForeachToInArrayRectorTest extends AbstractRectorTestCase
{
/**
* @dataProvider provideWrongToFixedFiles()
*/
public function test(string $wrong, string $fixed): void
{
$this->doTestFileMatchesExpectedContent($wrong, $fixed);
}
public function provideWrongToFixedFiles(): Iterator
{
yield [__DIR__ . '/Wrong/wrong.php.inc', __DIR__ . '/Correct/correct.php.inc'];
}
protected function provideConfig(): string
{
return __DIR__ . '/config.yml';
}
}

View File

@ -0,0 +1,101 @@
<?php declare(strict_types=1);
final class MyClass
{
public function foreachToInArray() : bool
{
foreach ($items as $item) {
if ($item == 'something') {
return true;
}
}
return false;
}
public function foreachToInArrayYoda() : bool
{
foreach ($items as $item) {
if ('something' == $item) {
return true;
}
}
return false;
}
public function foreachToInArrayStrict() : bool
{
foreach ($items as $item) {
if ($item === 'something') {
return true;
}
}
return false;
}
public function invertedForeachToInArrayStrict() : bool
{
foreach ($items as $item) {
if ($item === 'something') {
return false;
}
}
return true;
}
public function foreachToInArrayWithToVariables() : bool
{
foreach ($items as $item) {
if ($something === $item) {
return true;
}
}
return false;
}
public function foreachWithoutReturnFalse()
{
foreach ($items as $item) {
if ($item === 'something') {
return true;
}
}
}
public function foreachReturnString()
{
foreach ($items as $item) {
if ($item === 'something') {
return true;
}
}
return 'false';
}
public function foreachWithSomethingElseAfterIt()
{
foreach ($items as $item) {
if ($item === 'something') {
return true;
}
}
$foo = 'bar';
}
public function foreachWithElseNullable()
{
foreach ($items as $item) {
if ('string') {
return true;
}
}
return;
}
}

View File

@ -0,0 +1,2 @@
services:
Rector\CodeQuality\Rector\Foreach_\ForeachToInArrayRector: ~

View File

@ -63,6 +63,14 @@ final class NodeFactory
return BuilderHelpers::normalizeValue(null);
}
/**
* Creates "true"
*/
public function createTrueConstant(): ConstFetch
{
return BuilderHelpers::normalizeValue(true);
}
/**
* Creates "\SomeClass::CONSTANT"
*/

View File

@ -36,6 +36,7 @@ final class ConstFetchAnalyzer
if (! $node instanceof ConstFetch) {
return false;
}
return $node->name->toLowerString() === $name;
}
}

View File

@ -157,7 +157,7 @@ CODE_SAMPLE
return 'true';
}
if ($resolvedValue === false) {
if (! $resolvedValue) {
return 'false';
}

View File

@ -128,12 +128,6 @@ CODE_SAMPLE
{
$nodeValue = $this->constExprEvaluator->evaluateDirectly($argNode->value);
foreach ($values as $value) {
if ($nodeValue === $value) {
return true;
}
}
return false;
return in_array($nodeValue, $values, true);
}
}

View File

@ -170,13 +170,8 @@ final class NamespaceReplacerRector extends AbstractRector
$fullyQualifiedNode = $parentNode->class;
$newClassName = $fullyQualifiedNode->toString();
foreach (array_keys($this->oldToNewNamespaces) as $oldNamespace) {
if ($newClassName === $oldNamespace) {
return true;
}
}
return false;
return array_key_exists($newClassName, $this->oldToNewNamespaces);
}
private function isPartialNamespace(Name $nameNode): bool