From 72ee6833705c8c373b3240075e08eb566fccac6c Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Thu, 9 Jul 2026 10:31:11 +0200 Subject: [PATCH 1/3] [CodeQuality] Split ExplicitBoolCompareRector into array and object variants --- .../ArrayExplicitBoolCompareRectorTest.php | 28 ++++ .../Fixture/array_compare.php.inc | 4 +- .../Fixture/skip_not_variable_type.php.inc | 2 +- .../config/configured_rule.php | 9 ++ .../Fixture/nullable.php.inc | 4 +- .../nullable_instance_to_instanceof.php.inc | 4 +- .../Fixture/skip_docblock_nullable.php.inc | 2 +- .../Fixture/with_assign.php.inc | 4 +- .../ObjectExplicitBoolCompareRectorTest.php | 28 ++++ .../config/configured_rule.php | 9 ++ .../If_/ArrayExplicitBoolCompareRector.php | 129 ++++++++++++++++++ .../Rector/If_/ExplicitBoolCompareRector.php | 47 +------ .../If_/ObjectExplicitBoolCompareRector.php | 110 +++++++++++++++ src/Config/Level/CodeQualityLevel.php | 4 + 14 files changed, 329 insertions(+), 55 deletions(-) create mode 100644 rules-tests/CodeQuality/Rector/If_/ArrayExplicitBoolCompareRector/ArrayExplicitBoolCompareRectorTest.php rename rules-tests/CodeQuality/Rector/If_/{ExplicitBoolCompareRector => ArrayExplicitBoolCompareRector}/Fixture/array_compare.php.inc (72%) rename rules-tests/CodeQuality/Rector/If_/{ExplicitBoolCompareRector => ArrayExplicitBoolCompareRector}/Fixture/skip_not_variable_type.php.inc (62%) create mode 100644 rules-tests/CodeQuality/Rector/If_/ArrayExplicitBoolCompareRector/config/configured_rule.php rename rules-tests/CodeQuality/Rector/If_/{ExplicitBoolCompareRector => ObjectExplicitBoolCompareRector}/Fixture/nullable.php.inc (82%) rename rules-tests/CodeQuality/Rector/If_/{ExplicitBoolCompareRector => ObjectExplicitBoolCompareRector}/Fixture/nullable_instance_to_instanceof.php.inc (78%) rename rules-tests/CodeQuality/Rector/If_/{ExplicitBoolCompareRector => ObjectExplicitBoolCompareRector}/Fixture/skip_docblock_nullable.php.inc (69%) rename rules-tests/CodeQuality/Rector/If_/{ExplicitBoolCompareRector => ObjectExplicitBoolCompareRector}/Fixture/with_assign.php.inc (76%) create mode 100644 rules-tests/CodeQuality/Rector/If_/ObjectExplicitBoolCompareRector/ObjectExplicitBoolCompareRectorTest.php create mode 100644 rules-tests/CodeQuality/Rector/If_/ObjectExplicitBoolCompareRector/config/configured_rule.php create mode 100644 rules/CodeQuality/Rector/If_/ArrayExplicitBoolCompareRector.php create mode 100644 rules/CodeQuality/Rector/If_/ObjectExplicitBoolCompareRector.php 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..5da841f5fe6 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_; @@ -29,8 +25,6 @@ use PhpParser\Node\Stmt\Expression; use PhpParser\Node\Stmt\If_; use PHPStan\Type\MixedType; -use PHPStan\Type\ObjectType; -use Rector\NodeTypeResolver\TypeAnalyzer\ArrayTypeAnalyzer; use Rector\NodeTypeResolver\TypeAnalyzer\StringTypeAnalyzer; use Rector\PhpParser\Node\Value\ValueResolver; use Rector\Rector\AbstractRector; @@ -44,7 +38,6 @@ final class ExplicitBoolCompareRector extends AbstractRector { public function __construct( private readonly StringTypeAnalyzer $stringTypeAnalyzer, - private readonly ArrayTypeAnalyzer $arrayTypeAnalyzer, private readonly ValueResolver $valueResolver, ) { } @@ -121,7 +114,7 @@ public function refactor(Node $node): null|array|Node 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 +129,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 +148,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 +173,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 +240,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..9af9144cdaa 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; @@ -140,6 +142,8 @@ final class CodeQualityLevel TernaryImplodeToImplodeRector::class, ConsecutiveNullCompareReturnsToNullCoalesceQueueRector::class, ExplicitBoolCompareRector::class, + ArrayExplicitBoolCompareRector::class, + ObjectExplicitBoolCompareRector::class, CombineIfRector::class, UseIdenticalOverEqualWithSameTypeRector::class, SimplifyBoolIdenticalTrueRector::class, From c2dc8a60577c31ea0af0e51186c352b2dde5ed7f Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Thu, 9 Jul 2026 10:33:42 +0200 Subject: [PATCH 2/3] Skip array and object cases in ExplicitBoolCompareRector, run new rules first --- .../Rector/If_/ExplicitBoolCompareRector.php | 16 ++++++++++++++++ src/Config/Level/CodeQualityLevel.php | 2 +- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/rules/CodeQuality/Rector/If_/ExplicitBoolCompareRector.php b/rules/CodeQuality/Rector/If_/ExplicitBoolCompareRector.php index 5da841f5fe6..58b33b1933d 100644 --- a/rules/CodeQuality/Rector/If_/ExplicitBoolCompareRector.php +++ b/rules/CodeQuality/Rector/If_/ExplicitBoolCompareRector.php @@ -25,6 +25,8 @@ use PhpParser\Node\Stmt\Expression; use PhpParser\Node\Stmt\If_; use PHPStan\Type\MixedType; +use PHPStan\Type\ObjectType; +use Rector\NodeTypeResolver\TypeAnalyzer\ArrayTypeAnalyzer; use Rector\NodeTypeResolver\TypeAnalyzer\StringTypeAnalyzer; use Rector\PhpParser\Node\Value\ValueResolver; use Rector\Rector\AbstractRector; @@ -38,6 +40,7 @@ final class ExplicitBoolCompareRector extends AbstractRector { public function __construct( private readonly StringTypeAnalyzer $stringTypeAnalyzer, + private readonly ArrayTypeAnalyzer $arrayTypeAnalyzer, private readonly ValueResolver $valueResolver, ) { } @@ -109,6 +112,19 @@ 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; diff --git a/src/Config/Level/CodeQualityLevel.php b/src/Config/Level/CodeQualityLevel.php index 9af9144cdaa..9402e5481c0 100644 --- a/src/Config/Level/CodeQualityLevel.php +++ b/src/Config/Level/CodeQualityLevel.php @@ -141,9 +141,9 @@ final class CodeQualityLevel SimplifyIfElseToTernaryRector::class, TernaryImplodeToImplodeRector::class, ConsecutiveNullCompareReturnsToNullCoalesceQueueRector::class, - ExplicitBoolCompareRector::class, ArrayExplicitBoolCompareRector::class, ObjectExplicitBoolCompareRector::class, + ExplicitBoolCompareRector::class, CombineIfRector::class, UseIdenticalOverEqualWithSameTypeRector::class, SimplifyBoolIdenticalTrueRector::class, From e254fdccb9053272572af5ad72af0b07643ffb63 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Thu, 9 Jul 2026 10:42:37 +0200 Subject: [PATCH 3/3] Use ObjectExplicitBoolCompareRector in ReplaceStmtToExpr issue test config --- tests/Issues/ReplaceStmtToExpr/config/configured_rule.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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, ]