diff --git a/rules/dead-code/src/NodeManipulator/CountManipulator.php b/rules/dead-code/src/NodeManipulator/CountManipulator.php new file mode 100644 index 000000000000..ecf908e8561c --- /dev/null +++ b/rules/dead-code/src/NodeManipulator/CountManipulator.php @@ -0,0 +1,120 @@ +betterStandardPrinter = $betterStandardPrinter; + $this->nodeNameResolver = $nodeNameResolver; + } + + public function isCounterHigherThanOne(Node $node, Expr $expr): bool + { + // e.g. count($values) > 0 + if ($node instanceof Greater) { + return $this->processGreater($node, $expr); + } + + // e.g. count($values) >= 1 + if ($node instanceof GreaterOrEqual) { + return $this->processGreaterOrEqual($node, $expr); + } + + // e.g. 0 < count($values) + if ($node instanceof Smaller) { + return $this->processSmaller($node, $expr); + } + + // e.g. 1 <= count($values) + if ($node instanceof SmallerOrEqual) { + return $this->processSmallerOrEqual($node, $expr); + } + + return false; + } + + private function isNumber(Node $node, int $value): bool + { + if (! $node instanceof LNumber) { + return false; + } + + return $node->value === $value; + } + + private function processGreater(Greater $greater, Expr $expr): bool + { + if (! $this->isNumber($greater->right, 0)) { + return false; + } + + return $this->isCountWithExpression($greater->left, $expr); + } + + private function processSmaller(Smaller $smaller, Expr $expr): bool + { + if (! $this->isNumber($smaller->left, 0)) { + return false; + } + + return $this->isCountWithExpression($smaller->right, $expr); + } + + private function processGreaterOrEqual(GreaterOrEqual $greaterOrEqual, Expr $expr) + { + if (! $this->isNumber($greaterOrEqual->right, 1)) { + return false; + } + + return $this->isCountWithExpression($greaterOrEqual->left, $expr); + } + + private function processSmallerOrEqual(SmallerOrEqual $smallerOrEqual, Expr $expr): bool + { + if (! $this->isNumber($smallerOrEqual->left, 1)) { + return false; + } + + return $this->isCountWithExpression($smallerOrEqual->right, $expr); + } + + private function isCountWithExpression(Node $node, Expr $expr): bool + { + if (! $node instanceof FuncCall) { + return false; + } + + if (! $this->nodeNameResolver->isName($node, 'count')) { + return false; + } + + $countedExpr = $node->args[0]->value; + + return $this->betterStandardPrinter->areNodesWithoutCommentsEqual($countedExpr, $expr); + } +} diff --git a/rules/dead-code/src/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector.php b/rules/dead-code/src/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector.php index 5c6bc31df18a..8b81ee6df9a1 100644 --- a/rules/dead-code/src/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector.php +++ b/rules/dead-code/src/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector.php @@ -5,17 +5,14 @@ namespace Rector\DeadCode\Rector\If_; use PhpParser\Node; -use PhpParser\Node\Expr; -use PhpParser\Node\Expr\Array_; -use PhpParser\Node\Expr\BinaryOp; -use PhpParser\Node\Expr\BinaryOp\NotEqual; -use PhpParser\Node\Expr\BinaryOp\NotIdentical; use PhpParser\Node\Stmt\Foreach_; use PhpParser\Node\Stmt\If_; use Rector\Core\PhpParser\Node\Manipulator\IfManipulator; use Rector\Core\Rector\AbstractRector; use Rector\Core\RectorDefinition\CodeSample; use Rector\Core\RectorDefinition\RectorDefinition; +use Rector\DeadCode\NodeManipulator\CountManipulator; +use Rector\DeadCode\UselessIfCondBeforeForeachDetector; /** * @see \Rector\DeadCode\Tests\Rector\If_\RemoveUnusedNonEmptyArrayBeforeForeachRector\RemoveUnusedNonEmptyArrayBeforeForeachRectorTest @@ -27,9 +24,24 @@ final class RemoveUnusedNonEmptyArrayBeforeForeachRector extends AbstractRector */ private $ifManipulator; - public function __construct(IfManipulator $ifManipulator) - { + /** + * @var UselessIfCondBeforeForeachDetector + */ + private $uselessIfCondBeforeForeachDetector; + + /** + * @var CountManipulator + */ + private $countManipulator; + + public function __construct( + IfManipulator $ifManipulator, + UselessIfCondBeforeForeachDetector $uselessIfCondBeforeForeachDetector, + CountManipulator $countManipulator + ) { $this->ifManipulator = $ifManipulator; + $this->uselessIfCondBeforeForeachDetector = $uselessIfCondBeforeForeachDetector; + $this->countManipulator = $countManipulator; } public function getDefinition(): RectorDefinition @@ -81,55 +93,31 @@ public function getNodeTypes(): array */ public function refactor(Node $node): ?Node { - if (! $this->ifManipulator->isIfWithOnlyForeach($node)) { - return null; - } - - /** @var Foreach_ $foreach */ - $foreach = $node->stmts[0]; - $foreachExpr = $foreach->expr; - - if (! $node->cond instanceof NotIdentical && ! $node->cond instanceof NotEqual) { - return null; - } - - /** @var NotIdentical|NotEqual $notIdentical */ - $notIdentical = $node->cond; - - if (! $this->isMatching($notIdentical, $foreachExpr)) { + if (! $this->isUselessBeforeForeachCheck($node)) { return null; } - return $foreach; + return $node->stmts[0]; } - private function isEmptyArray(Expr $expr): bool + private function isUselessBeforeForeachCheck(If_ $if): bool { - if (! $expr instanceof Array_) { + if (! $this->ifManipulator->isIfWithOnlyForeach($if)) { return false; } - return $expr->items === []; - } + /** @var Foreach_ $foreach */ + $foreach = $if->stmts[0]; + $foreachExpr = $foreach->expr; - private function isEmptyArrayAndForeachedVariable(Expr $leftExpr, Expr $rightExpr, Expr $foreachExpr): bool - { - if (! $this->isEmptyArray($leftExpr)) { - return false; + if ($this->uselessIfCondBeforeForeachDetector->isMatchingNotIdenticalEmptyArray($if, $foreachExpr)) { + return true; } - return $this->areNodesWithoutCommentsEqual($foreachExpr, $rightExpr); - } - - /** - * @param NotIdentical|NotEqual $binaryOp - */ - private function isMatching(BinaryOp $binaryOp, Expr $foreachExpr): bool - { - if ($this->isEmptyArrayAndForeachedVariable($binaryOp->left, $binaryOp->right, $foreachExpr)) { + if ($this->uselessIfCondBeforeForeachDetector->isMatchingNotEmpty($if, $foreachExpr)) { return true; } - return $this->isEmptyArrayAndForeachedVariable($binaryOp->right, $binaryOp->left, $foreachExpr); + return $this->countManipulator->isCounterHigherThanOne($if->cond, $foreachExpr); } } diff --git a/rules/dead-code/src/UselessIfCondBeforeForeachDetector.php b/rules/dead-code/src/UselessIfCondBeforeForeachDetector.php new file mode 100644 index 000000000000..aebf1ad61cb7 --- /dev/null +++ b/rules/dead-code/src/UselessIfCondBeforeForeachDetector.php @@ -0,0 +1,98 @@ +betterStandardPrinter = $betterStandardPrinter; + } + + /** + * Matches: + * !empty($values) + */ + public function isMatchingNotEmpty(If_ $if, Expr $foreachExpr): bool + { + $cond = $if->cond; + if (! $cond instanceof BooleanNot) { + return false; + } + + if (! $cond->expr instanceof Empty_) { + return false; + } + + /** @var Empty_ $empty */ + $empty = $cond->expr; + + return $this->betterStandardPrinter->areNodesWithoutCommentsEqual($empty->expr, $foreachExpr); + } + + /** + * Matches: + * $values !== [] + * $values != [] + * [] !== $values + * [] != $values + */ + public function isMatchingNotIdenticalEmptyArray(If_ $if, Expr $foreachExpr): bool + { + if (! $if->cond instanceof NotIdentical && ! $if->cond instanceof NotEqual) { + return false; + } + + /** @var NotIdentical|NotEqual $notIdentical */ + $notIdentical = $if->cond; + + return $this->isMatchingNotBinaryOp($notIdentical, $foreachExpr); + } + + /** + * @param NotIdentical|NotEqual $binaryOp + */ + private function isMatchingNotBinaryOp(BinaryOp $binaryOp, Expr $foreachExpr): bool + { + if ($this->isEmptyArrayAndForeachedVariable($binaryOp->left, $binaryOp->right, $foreachExpr)) { + return true; + } + + return $this->isEmptyArrayAndForeachedVariable($binaryOp->right, $binaryOp->left, $foreachExpr); + } + + private function isEmptyArrayAndForeachedVariable(Expr $leftExpr, Expr $rightExpr, Expr $foreachExpr): bool + { + if (! $this->isEmptyArray($leftExpr)) { + return false; + } + + return $this->betterStandardPrinter->areNodesWithoutCommentsEqual($foreachExpr, $rightExpr); + } + + private function isEmptyArray(Expr $expr): bool + { + if (! $expr instanceof Array_) { + return false; + } + + return $expr->items === []; + } +} diff --git a/rules/dead-code/tests/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector/Fixture/keep_if_else.php.inc b/rules/dead-code/tests/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector/Fixture/keep_if_else.php.inc new file mode 100644 index 000000000000..5653ca82a7d0 --- /dev/null +++ b/rules/dead-code/tests/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector/Fixture/keep_if_else.php.inc @@ -0,0 +1,18 @@ += 1) { + foreach ($values as $value) { + echo $value; + } + } + } +} + +?> +----- + diff --git a/rules/dead-code/tests/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector/Fixture/more_or_equal_than_one_other_side.php.inc b/rules/dead-code/tests/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector/Fixture/more_or_equal_than_one_other_side.php.inc new file mode 100644 index 000000000000..e199748a79ef --- /dev/null +++ b/rules/dead-code/tests/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector/Fixture/more_or_equal_than_one_other_side.php.inc @@ -0,0 +1,35 @@ + +----- + diff --git a/rules/dead-code/tests/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector/Fixture/more_than_zero.php.inc b/rules/dead-code/tests/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector/Fixture/more_than_zero.php.inc new file mode 100644 index 000000000000..f83ae4c07a7d --- /dev/null +++ b/rules/dead-code/tests/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector/Fixture/more_than_zero.php.inc @@ -0,0 +1,35 @@ + 0) { + foreach ($values as $value) { + echo $value; + } + } + } +} + +?> +----- + diff --git a/rules/dead-code/tests/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector/Fixture/more_than_zero_other_side.php.inc b/rules/dead-code/tests/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector/Fixture/more_than_zero_other_side.php.inc new file mode 100644 index 000000000000..c9c036819559 --- /dev/null +++ b/rules/dead-code/tests/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector/Fixture/more_than_zero_other_side.php.inc @@ -0,0 +1,35 @@ + +----- + diff --git a/rules/dead-code/tests/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector/Fixture/not_empty.php.inc b/rules/dead-code/tests/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector/Fixture/not_empty.php.inc new file mode 100644 index 000000000000..a68caacfdb58 --- /dev/null +++ b/rules/dead-code/tests/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector/Fixture/not_empty.php.inc @@ -0,0 +1,35 @@ + +----- + diff --git a/rules/symfony/src/ValueObject/ServiceMap/ServiceMap.php b/rules/symfony/src/ValueObject/ServiceMap/ServiceMap.php index 169b8f5c7a1b..0d87cb241bcd 100644 --- a/rules/symfony/src/ValueObject/ServiceMap/ServiceMap.php +++ b/rules/symfony/src/ValueObject/ServiceMap/ServiceMap.php @@ -47,11 +47,9 @@ public function getServiceType(string $id): ?Type $interfaces = class_implements($class); - if (count($interfaces) > 0) { - foreach ($interfaces as $interface) { - // return first interface - return new ObjectType($interface); - } + foreach ($interfaces as $interface) { + // return first interface + return new ObjectType($interface); } return new ObjectType($class);