improve CodeQuality

This commit is contained in:
Tomas Votruba 2018-11-07 02:37:39 +01:00
parent 981708f998
commit d166ff0151
9 changed files with 55 additions and 12 deletions

View File

@ -78,7 +78,7 @@ CODE_SAMPLE
*/
public function refactor(Node $node): ?Node
{
if (! $this->shouldSkipForeach($node)) {
if ($this->shouldSkipForeach($node)) {
return null;
}
@ -124,6 +124,7 @@ CODE_SAMPLE
return null;
}
// cannot be "return true;" + "return true;"
if ($this->areNodesEqual($returnNode, $returnNodeToRemove)) {
return null;
}
@ -142,25 +143,29 @@ CODE_SAMPLE
private function shouldSkipForeach(Foreach_ $foreachNode): bool
{
if (isset($foreachNode->keyVar)) {
return false;
return true;
}
if (count($foreachNode->stmts) > 1) {
return false;
return true;
}
$nextNode = $foreachNode->getAttribute(Attribute::NEXT_NODE);
if ($nextNode === null || ! $nextNode instanceof Return_) {
return false;
return true;
}
$returnExpression = $nextNode->expr;
if ($returnExpression !== null && $this->isBool($returnExpression)) {
if ($returnExpression === null) {
return true;
}
return $foreachNode->stmts[0] instanceof If_;
if (! $this->isBool($returnExpression)) {
return true;
}
return ! $foreachNode->stmts[0] instanceof If_;
}
private function isIfBodyABoolReturnNode(If_ $firstNodeInsideForeach): bool

View File

@ -4,6 +4,7 @@ namespace Rector\CodeQuality\Rector\Identical;
use PhpParser\Node;
use PhpParser\Node\Expr\BinaryOp;
use PhpParser\Node\Expr\BinaryOp\BooleanOr;
use PhpParser\Node\Expr\BinaryOp\Identical;
use PhpParser\Node\Expr\BinaryOp\NotIdentical;
use PhpParser\Node\Expr\BooleanNot;
@ -93,6 +94,10 @@ final class SimplifyConditionsRector extends AbstractRector
*/
private function shouldSkip(BinaryOp $binaryOpNode): bool
{
if ($binaryOpNode instanceof BooleanOr) {
return true;
}
if ($binaryOpNode->left instanceof BinaryOp) {
return true;
}

View File

@ -2,6 +2,7 @@
namespace Rector\CodeQuality\Rector\If_;
use Nette\Utils\Strings;
use PhpParser\Node;
use PhpParser\Node\Expr;
use PhpParser\Node\Expr\BinaryOp\Identical;
@ -61,16 +62,18 @@ CODE_SAMPLE
$newReturnNode = $this->processReturnTrue($node, $nextNode);
} elseif ($this->isFalse($ifInnerNode->expr)) {
$newReturnNode = $this->processReturnFalse($node, $nextNode);
} else {
return null;
}
if (isset($newReturnNode) && $newReturnNode !== null) {
$this->keepComments($node, $newReturnNode);
$this->removeNode($nextNode);
return $newReturnNode;
if ($newReturnNode === null) {
return null;
}
return null;
$this->keepComments($node, $newReturnNode);
$this->removeNode($nextNode);
return $newReturnNode;
}
private function shouldSkip(If_ $ifNode): bool
@ -100,6 +103,12 @@ CODE_SAMPLE
if (! $nextNode instanceof Return_) {
return true;
}
// negate + negate → skip for now
if ($this->isFalse($ifInnerNode->expr) && Strings::contains($this->print($ifNode->cond), '!=')) {
return true;
}
return ! $this->isBool($nextNode->expr);
}

View File

@ -3,3 +3,7 @@
if ($a === '-' && ! $start && ! ($i < $l && $s[$i] === ']')) {
echo 'maybe';
}
if (!($orderItem instanceof OrderTransport || $orderItem instanceof OrderPayment)) {
$itemsWithoutTransportAndPayment[] = $orderItem;
}

View File

@ -3,3 +3,7 @@
if ($a === '-' && ! $start && ! ($i < $l && $s[$i] === ']')) {
echo 'maybe';
}
if (!($orderItem instanceof OrderTransport || $orderItem instanceof OrderPayment)) {
$itemsWithoutTransportAndPayment[] = $orderItem;
}

View File

@ -0,0 +1,7 @@
<?php
if ($dateTimePattern->getTimeType() !== null && $dateTimePattern->getTimeType() !== $timeType) {
return false;
}
return true;

View File

@ -28,6 +28,7 @@ final class SimplifyIfReturnBoolRectorTest extends AbstractRectorTestCase
yield [__DIR__ . '/Wrong/wrong6.php.inc', __DIR__ . '/Correct/correct6.php.inc'];
yield [__DIR__ . '/Wrong/wrong7.php.inc', __DIR__ . '/Correct/correct7.php.inc'];
yield [__DIR__ . '/Wrong/wrong8.php.inc', __DIR__ . '/Correct/correct8.php.inc'];
yield [__DIR__ . '/Wrong/wrong9.php.inc', __DIR__ . '/Correct/correct9.php.inc'];
}
protected function provideConfig(): string

View File

@ -0,0 +1,7 @@
<?php
if ($dateTimePattern->getTimeType() !== null && $dateTimePattern->getTimeType() !== $timeType) {
return false;
}
return true;

View File

@ -74,6 +74,7 @@ parameters:
# subtype
- '#Property PhpParser\\Node\\Param::\$type \(PhpParser\\Node\\Name|PhpParser\\Node\\NullableType\|string\|null\) does not accept PhpParser\\Node\\Identifier|PhpParser\\Node\\Name\|PhpParser\\Node\\NullableType#' # 3
- '#Parameter \#2 \$ifCondition of method Rector\\CodeQuality\\Rector\\Foreach_\\ForeachToInArrayRector::createInArrayFunction() expects PhpParser\\Node\\Expr\\BinaryOp\\Equal|PhpParser\\Node\\Expr\\BinaryOp\\Identical, PhpParser\\Node\\Expr\\BinaryOp given#'
# intentionally incorrect - part of the test
- '#Parameter \#2 \$codeSamples of class Rector\\RectorDefinition\\RectorDefinition constructor expects array<Rector\\Contract\\RectorDefinition\\CodeSampleInterface>, array<int, stdClass> given#' # 1