diff --git a/rules-tests/CodeQuality/Rector/If_/ArrayExplicitBoolCompareRector/ArrayExplicitBoolCompareRectorTest.php b/rules-tests/CodeQuality/Rector/If_/ArrayExplicitBoolCompareRector/ArrayExplicitBoolCompareRectorTest.php new file mode 100644 index 00000000000..21e8d88aefd --- /dev/null +++ b/rules-tests/CodeQuality/Rector/If_/ArrayExplicitBoolCompareRector/ArrayExplicitBoolCompareRectorTest.php @@ -0,0 +1,28 @@ +doTestFile($filePath); + } + + public static function provideData(): Iterator + { + return self::yieldFilesFromDirectory(__DIR__ . '/Fixture'); + } + + public function provideConfigFilePath(): string + { + return __DIR__ . '/config/configured_rule.php'; + } +} diff --git a/rules-tests/CodeQuality/Rector/If_/ExplicitBoolCompareRector/Fixture/array_compare.php.inc b/rules-tests/CodeQuality/Rector/If_/ArrayExplicitBoolCompareRector/Fixture/array_compare.php.inc similarity index 72% rename from rules-tests/CodeQuality/Rector/If_/ExplicitBoolCompareRector/Fixture/array_compare.php.inc rename to rules-tests/CodeQuality/Rector/If_/ArrayExplicitBoolCompareRector/Fixture/array_compare.php.inc index 74fd9e6964c..25267c46f69 100644 --- a/rules-tests/CodeQuality/Rector/If_/ExplicitBoolCompareRector/Fixture/array_compare.php.inc +++ b/rules-tests/CodeQuality/Rector/If_/ArrayExplicitBoolCompareRector/Fixture/array_compare.php.inc @@ -1,6 +1,6 @@ withRules([ArrayExplicitBoolCompareRector::class]); diff --git a/rules-tests/CodeQuality/Rector/If_/ExplicitBoolCompareRector/Fixture/nullable.php.inc b/rules-tests/CodeQuality/Rector/If_/ObjectExplicitBoolCompareRector/Fixture/nullable.php.inc similarity index 82% rename from rules-tests/CodeQuality/Rector/If_/ExplicitBoolCompareRector/Fixture/nullable.php.inc rename to rules-tests/CodeQuality/Rector/If_/ObjectExplicitBoolCompareRector/Fixture/nullable.php.inc index 1ae41552cb9..ac46399e48d 100644 --- a/rules-tests/CodeQuality/Rector/If_/ExplicitBoolCompareRector/Fixture/nullable.php.inc +++ b/rules-tests/CodeQuality/Rector/If_/ObjectExplicitBoolCompareRector/Fixture/nullable.php.inc @@ -1,6 +1,6 @@ doTestFile($filePath); + } + + public static function provideData(): Iterator + { + return self::yieldFilesFromDirectory(__DIR__ . '/Fixture'); + } + + public function provideConfigFilePath(): string + { + return __DIR__ . '/config/configured_rule.php'; + } +} diff --git a/rules-tests/CodeQuality/Rector/If_/ObjectExplicitBoolCompareRector/config/configured_rule.php b/rules-tests/CodeQuality/Rector/If_/ObjectExplicitBoolCompareRector/config/configured_rule.php new file mode 100644 index 00000000000..3aee19c3d14 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/If_/ObjectExplicitBoolCompareRector/config/configured_rule.php @@ -0,0 +1,9 @@ +withRules([ObjectExplicitBoolCompareRector::class]); diff --git a/rules/CodeQuality/Rector/If_/ArrayExplicitBoolCompareRector.php b/rules/CodeQuality/Rector/If_/ArrayExplicitBoolCompareRector.php new file mode 100644 index 00000000000..3062a7d72c6 --- /dev/null +++ b/rules/CodeQuality/Rector/If_/ArrayExplicitBoolCompareRector.php @@ -0,0 +1,129 @@ +> + */ + public function getNodeTypes(): array + { + return [If_::class, ElseIf_::class, Ternary::class]; + } + + /** + * @param If_|ElseIf_|Ternary $node + */ + public function refactor(Node $node): ?Node + { + // skip short ternary + if ($node instanceof Ternary && ! $node->if instanceof Expr) { + return null; + } + + if ($node->cond instanceof BooleanNot) { + $conditionNode = $node->cond->expr; + $isNegated = true; + } else { + $conditionNode = $node->cond; + $isNegated = false; + } + + if ($conditionNode instanceof Bool_) { + return null; + } + + $conditionStaticType = $this->nodeTypeResolver->getNativeType($conditionNode); + if ($conditionStaticType instanceof MixedType || $conditionStaticType->isBoolean()->yes()) { + return null; + } + + if (! $this->arrayTypeAnalyzer->isArrayType($conditionNode)) { + return null; + } + + $binaryOp = $this->resolveArray($isNegated, $conditionNode); + if (! $binaryOp instanceof Expr) { + return null; + } + + $node->cond = $binaryOp; + + return $node; + } + + private function resolveArray(bool $isNegated, Expr $expr): Identical|NotIdentical|null + { + if (! $expr instanceof Variable) { + return null; + } + + $array = new Array_([]); + + // compare === [] + if ($isNegated) { + return new Identical($expr, $array); + } + + return new NotIdentical($expr, $array); + } +} diff --git a/rules/CodeQuality/Rector/If_/ExplicitBoolCompareRector.php b/rules/CodeQuality/Rector/If_/ExplicitBoolCompareRector.php index ec55868cd87..58b33b1933d 100644 --- a/rules/CodeQuality/Rector/If_/ExplicitBoolCompareRector.php +++ b/rules/CodeQuality/Rector/If_/ExplicitBoolCompareRector.php @@ -6,7 +6,6 @@ use PhpParser\Node; use PhpParser\Node\Expr; -use PhpParser\Node\Expr\Array_; use PhpParser\Node\Expr\Assign; use PhpParser\Node\Expr\BinaryOp; use PhpParser\Node\Expr\BinaryOp\BooleanAnd; @@ -17,10 +16,7 @@ use PhpParser\Node\Expr\BooleanNot; use PhpParser\Node\Expr\Cast\Bool_; use PhpParser\Node\Expr\FuncCall; -use PhpParser\Node\Expr\Instanceof_; use PhpParser\Node\Expr\Ternary; -use PhpParser\Node\Expr\Variable; -use PhpParser\Node\Name\FullyQualified; use PhpParser\Node\Scalar\Float_; use PhpParser\Node\Scalar\Int_; use PhpParser\Node\Scalar\String_; @@ -116,12 +112,25 @@ public function refactor(Node $node): null|array|Node return null; } + // handled by ArrayExplicitBoolCompareRector + if ($this->arrayTypeAnalyzer->isArrayType($conditionNode)) { + return null; + } + + // handled by ObjectExplicitBoolCompareRector + if ($this->nodeTypeResolver->matchNullableTypeOfSpecificType( + $conditionNode, + ObjectType::class + ) instanceof ObjectType) { + return null; + } + $binaryOp = $this->resolveNewConditionNode($conditionNode, $isNegated); if (! $binaryOp instanceof Expr) { return null; } - if ($node instanceof If_ && $node->cond instanceof Assign && $binaryOp instanceof BinaryOp && $binaryOp->left instanceof NotIdentical && $binaryOp->right instanceof NotIdentical) { + if ($node instanceof If_ && $node->cond instanceof Assign && $binaryOp->left instanceof NotIdentical && $binaryOp->right instanceof NotIdentical) { $expression = new Expression($node->cond); $binaryOp->left->left = $node->cond->var; $binaryOp->right->left = $node->cond->var; @@ -136,16 +145,12 @@ public function refactor(Node $node): null|array|Node return $node; } - private function resolveNewConditionNode(Expr $expr, bool $isNegated): BinaryOp|Instanceof_|BooleanNot|null + private function resolveNewConditionNode(Expr $expr, bool $isNegated): ?BinaryOp { if ($expr instanceof FuncCall && $this->isName($expr, 'count')) { return $this->resolveCount($isNegated, $expr); } - if ($this->arrayTypeAnalyzer->isArrayType($expr)) { - return $this->resolveArray($isNegated, $expr); - } - if ($this->stringTypeAnalyzer->isStringOrUnionStringOnlyType($expr)) { return $this->resolveString($isNegated, $expr); } @@ -159,11 +164,6 @@ private function resolveNewConditionNode(Expr $expr, bool $isNegated): BinaryOp| return $this->resolveFloat($isNegated, $expr); } - $objectType = $this->nodeTypeResolver->matchNullableTypeOfSpecificType($expr, ObjectType::class); - if ($objectType instanceof ObjectType) { - return $this->resolveNullable($isNegated, $expr, $objectType); - } - return null; } @@ -189,25 +189,6 @@ private function resolveCount(bool $isNegated, FuncCall $funcCall): Identical|Gr return new Greater($funcCall, $int); } - /** - * @return Identical|NotIdentical|null - */ - private function resolveArray(bool $isNegated, Expr $expr): ?BinaryOp - { - if (! $expr instanceof Variable) { - return null; - } - - $array = new Array_([]); - - // compare === [] - if ($isNegated) { - return new Identical($expr, $array); - } - - return new NotIdentical($expr, $array); - } - private function resolveString(bool $isNegated, Expr $expr): Identical|NotIdentical|BooleanAnd|BooleanOr { $emptyString = new String_(''); @@ -275,12 +256,4 @@ private function resolveFloat(bool $isNegated, Expr $expr): Identical|NotIdentic return new NotIdentical($expr, $float); } - - private function resolveNullable(bool $isNegated, Expr $expr, ObjectType $objectType): BooleanNot|Instanceof_ - { - $fullyQualified = new FullyQualified($objectType->getClassName()); - $instanceof = new Instanceof_($expr, $fullyQualified); - - return $isNegated ? new BooleanNot($instanceof) : $instanceof; - } } diff --git a/rules/CodeQuality/Rector/If_/ObjectExplicitBoolCompareRector.php b/rules/CodeQuality/Rector/If_/ObjectExplicitBoolCompareRector.php new file mode 100644 index 00000000000..c0b79587697 --- /dev/null +++ b/rules/CodeQuality/Rector/If_/ObjectExplicitBoolCompareRector.php @@ -0,0 +1,110 @@ +> + */ + public function getNodeTypes(): array + { + return [If_::class, ElseIf_::class, Ternary::class]; + } + + /** + * @param If_|ElseIf_|Ternary $node + */ + public function refactor(Node $node): ?Node + { + // skip short ternary + if ($node instanceof Ternary && ! $node->if instanceof Expr) { + return null; + } + + if ($node->cond instanceof BooleanNot) { + $conditionNode = $node->cond->expr; + $isNegated = true; + } else { + $conditionNode = $node->cond; + $isNegated = false; + } + + if ($conditionNode instanceof Bool_) { + return null; + } + + $conditionStaticType = $this->nodeTypeResolver->getNativeType($conditionNode); + if ($conditionStaticType instanceof MixedType || $conditionStaticType->isBoolean()->yes()) { + return null; + } + + $objectType = $this->nodeTypeResolver->matchNullableTypeOfSpecificType($conditionNode, ObjectType::class); + if (! $objectType instanceof ObjectType) { + return null; + } + + $node->cond = $this->resolveNullable($isNegated, $conditionNode, $objectType); + + return $node; + } + + private function resolveNullable(bool $isNegated, Expr $expr, ObjectType $objectType): BooleanNot|Instanceof_ + { + $fullyQualified = new FullyQualified($objectType->getClassName()); + $instanceof = new Instanceof_($expr, $fullyQualified); + + return $isNegated ? new BooleanNot($instanceof) : $instanceof; + } +} diff --git a/src/Config/Level/CodeQualityLevel.php b/src/Config/Level/CodeQualityLevel.php index dc6c447f637..9402e5481c0 100644 --- a/src/Config/Level/CodeQualityLevel.php +++ b/src/Config/Level/CodeQualityLevel.php @@ -56,10 +56,12 @@ use Rector\CodeQuality\Rector\Identical\SimplifyBoolIdenticalTrueRector; use Rector\CodeQuality\Rector\Identical\SimplifyConditionsRector; use Rector\CodeQuality\Rector\Identical\StrlenZeroToIdenticalEmptyStringRector; +use Rector\CodeQuality\Rector\If_\ArrayExplicitBoolCompareRector; use Rector\CodeQuality\Rector\If_\CombineIfRector; use Rector\CodeQuality\Rector\If_\CompleteMissingIfElseBracketRector; use Rector\CodeQuality\Rector\If_\ConsecutiveNullCompareReturnsToNullCoalesceQueueRector; use Rector\CodeQuality\Rector\If_\ExplicitBoolCompareRector; +use Rector\CodeQuality\Rector\If_\ObjectExplicitBoolCompareRector; use Rector\CodeQuality\Rector\If_\ShortenElseIfRector; use Rector\CodeQuality\Rector\If_\SimplifyIfElseToTernaryRector; use Rector\CodeQuality\Rector\If_\SimplifyIfNotNullReturnRector; @@ -139,6 +141,8 @@ final class CodeQualityLevel SimplifyIfElseToTernaryRector::class, TernaryImplodeToImplodeRector::class, ConsecutiveNullCompareReturnsToNullCoalesceQueueRector::class, + ArrayExplicitBoolCompareRector::class, + ObjectExplicitBoolCompareRector::class, ExplicitBoolCompareRector::class, CombineIfRector::class, UseIdenticalOverEqualWithSameTypeRector::class, diff --git a/tests/Issues/ReplaceStmtToExpr/config/configured_rule.php b/tests/Issues/ReplaceStmtToExpr/config/configured_rule.php index d48b1793e39..d7bb687f586 100644 --- a/tests/Issues/ReplaceStmtToExpr/config/configured_rule.php +++ b/tests/Issues/ReplaceStmtToExpr/config/configured_rule.php @@ -3,14 +3,14 @@ declare(strict_types=1); use Rector\CodeQuality\Rector\Identical\FlipTypeControlToUseExclusiveTypeRector; -use Rector\CodeQuality\Rector\If_\ExplicitBoolCompareRector; +use Rector\CodeQuality\Rector\If_\ObjectExplicitBoolCompareRector; use Rector\Config\RectorConfig; use Rector\EarlyReturn\Rector\Return_\ReturnBinaryOrToEarlyReturnRector; return RectorConfig::configure() ->withRules( [ - ExplicitBoolCompareRector::class, + ObjectExplicitBoolCompareRector::class, FlipTypeControlToUseExclusiveTypeRector::class, ReturnBinaryOrToEarlyReturnRector::class, ]