diff --git a/rules/CodeQuality/NodeAnalyzer/ExplicitBoolConditionResolver.php b/rules/CodeQuality/NodeAnalyzer/ExplicitBoolConditionResolver.php new file mode 100644 index 00000000000..d093f992dcc --- /dev/null +++ b/rules/CodeQuality/NodeAnalyzer/ExplicitBoolConditionResolver.php @@ -0,0 +1,50 @@ +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; + } + + return new ExplicitBoolCondition($conditionNode, $isNegated); + } +} diff --git a/rules/CodeQuality/NodeFactory/InArrayFromRepeatedCompareFactory.php b/rules/CodeQuality/NodeFactory/InArrayFromRepeatedCompareFactory.php new file mode 100644 index 00000000000..75d99c556d8 --- /dev/null +++ b/rules/CodeQuality/NodeFactory/InArrayFromRepeatedCompareFactory.php @@ -0,0 +1,56 @@ +getValueExpr(); + } + + /** @var ComparedExprAndValueExpr $firstComparedExprAndValue */ + $firstComparedExprAndValue = array_pop($comparedExprAndValueExprs); + + // all compared expr must be equal + foreach ($comparedExprAndValueExprs as $comparedExprAndValueExpr) { + if (! $this->nodeComparator->areNodesEqual( + $firstComparedExprAndValue->getComparedExpr(), + $comparedExprAndValueExpr->getComparedExpr() + )) { + return null; + } + } + + $array = $this->nodeFactory->createArray($valueExprs); + + return $this->nodeFactory->createArgs([$firstComparedExprAndValue->getComparedExpr(), $array]); + } +} diff --git a/rules/CodeQuality/Rector/BooleanAnd/RepeatedAndNotEqualToNotInArrayRector.php b/rules/CodeQuality/Rector/BooleanAnd/RepeatedAndNotEqualToNotInArrayRector.php index 6078257de27..10b0f66fccf 100644 --- a/rules/CodeQuality/Rector/BooleanAnd/RepeatedAndNotEqualToNotInArrayRector.php +++ b/rules/CodeQuality/Rector/BooleanAnd/RepeatedAndNotEqualToNotInArrayRector.php @@ -14,6 +14,7 @@ use PhpParser\Node\Expr\ConstFetch; use PhpParser\Node\Expr\FuncCall; use PhpParser\Node\Name; +use Rector\CodeQuality\NodeFactory\InArrayFromRepeatedCompareFactory; use Rector\CodeQuality\ValueObject\ComparedExprAndValueExpr; use Rector\PhpParser\Node\BetterNodeFinder; use Rector\Rector\AbstractRector; @@ -27,6 +28,7 @@ final class RepeatedAndNotEqualToNotInArrayRector extends AbstractRector { public function __construct( private readonly BetterNodeFinder $betterNodeFinder, + private readonly InArrayFromRepeatedCompareFactory $inArrayFromRepeatedCompareFactory, ) { } @@ -80,30 +82,11 @@ public function refactor(Node $node): ?BooleanNot return null; } - if (count($comparedExprAndValueExprs) < 3) { + $args = $this->inArrayFromRepeatedCompareFactory->createInArrayArgs($comparedExprAndValueExprs); + if ($args === null) { return null; } - // ensure all compared expr are the same - $valueExprs = $this->resolveValueExprs($comparedExprAndValueExprs); - - /** @var ComparedExprAndValueExpr $firstComparedExprAndValue */ - $firstComparedExprAndValue = array_pop($comparedExprAndValueExprs); - - // all compared expr must be equal - foreach ($comparedExprAndValueExprs as $comparedExprAndValueExpr) { - if (! $this->nodeComparator->areNodesEqual( - $firstComparedExprAndValue->getComparedExpr(), - $comparedExprAndValueExpr->getComparedExpr() - )) { - return null; - } - } - - $array = $this->nodeFactory->createArray($valueExprs); - - $args = $this->nodeFactory->createArgs([$firstComparedExprAndValue->getComparedExpr(), $array]); - if ($this->isStrictComparison($node)) { $args[] = new Arg(new ConstFetch(new Name('true'))); } @@ -127,21 +110,6 @@ private function matchComparedExprAndValueExpr(NotIdentical|NotEqual $expr): Com return new ComparedExprAndValueExpr($expr->left, $expr->right); } - /** - * @param ComparedExprAndValueExpr[] $comparedExprAndValueExprs - * @return Expr[] - */ - private function resolveValueExprs(array $comparedExprAndValueExprs): array - { - $valueExprs = []; - - foreach ($comparedExprAndValueExprs as $comparedExprAndValueExpr) { - $valueExprs[] = $comparedExprAndValueExpr->getValueExpr(); - } - - return $valueExprs; - } - /** * @return null|ComparedExprAndValueExpr[] */ diff --git a/rules/CodeQuality/Rector/BooleanOr/RepeatedOrEqualToInArrayRector.php b/rules/CodeQuality/Rector/BooleanOr/RepeatedOrEqualToInArrayRector.php index f6f9372a11b..d875451b9fb 100644 --- a/rules/CodeQuality/Rector/BooleanOr/RepeatedOrEqualToInArrayRector.php +++ b/rules/CodeQuality/Rector/BooleanOr/RepeatedOrEqualToInArrayRector.php @@ -13,6 +13,7 @@ use PhpParser\Node\Expr\ConstFetch; use PhpParser\Node\Expr\FuncCall; use PhpParser\Node\Name; +use Rector\CodeQuality\NodeFactory\InArrayFromRepeatedCompareFactory; use Rector\CodeQuality\ValueObject\ComparedExprAndValueExpr; use Rector\PhpParser\Node\BetterNodeFinder; use Rector\Rector\AbstractRector; @@ -26,6 +27,7 @@ final class RepeatedOrEqualToInArrayRector extends AbstractRector { public function __construct( private readonly BetterNodeFinder $betterNodeFinder, + private readonly InArrayFromRepeatedCompareFactory $inArrayFromRepeatedCompareFactory, ) { } @@ -79,30 +81,11 @@ public function refactor(Node $node): ?FuncCall return null; } - if (count($comparedExprAndValueExprs) < 3) { + $args = $this->inArrayFromRepeatedCompareFactory->createInArrayArgs($comparedExprAndValueExprs); + if ($args === null) { return null; } - // ensure all compared expr are the same - $valueExprs = $this->resolveValueExprs($comparedExprAndValueExprs); - - /** @var ComparedExprAndValueExpr $firstComparedExprAndValue */ - $firstComparedExprAndValue = array_pop($comparedExprAndValueExprs); - - // all compared expr must be equal - foreach ($comparedExprAndValueExprs as $comparedExprAndValueExpr) { - if (! $this->nodeComparator->areNodesEqual( - $firstComparedExprAndValue->getComparedExpr(), - $comparedExprAndValueExpr->getComparedExpr() - )) { - return null; - } - } - - $array = $this->nodeFactory->createArray($valueExprs); - - $args = $this->nodeFactory->createArgs([$firstComparedExprAndValue->getComparedExpr(), $array]); - $identicals = $this->betterNodeFinder->findInstanceOf($node, Identical::class); $equals = $this->betterNodeFinder->findInstanceOf($node, Equal::class); @@ -133,21 +116,6 @@ private function matchComparedExprAndValueExpr(Identical|Equal $expr): ComparedE return new ComparedExprAndValueExpr($expr->left, $expr->right); } - /** - * @param ComparedExprAndValueExpr[] $comparedExprAndValueExprs - * @return Expr[] - */ - private function resolveValueExprs(array $comparedExprAndValueExprs): array - { - $valueExprs = []; - - foreach ($comparedExprAndValueExprs as $comparedExprAndValueExpr) { - $valueExprs[] = $comparedExprAndValueExpr->getValueExpr(); - } - - return $valueExprs; - } - /** * @return null|ComparedExprAndValueExpr[] */ diff --git a/rules/CodeQuality/Rector/If_/ArrayExplicitBoolCompareRector.php b/rules/CodeQuality/Rector/If_/ArrayExplicitBoolCompareRector.php index 3062a7d72c6..647d7e751f2 100644 --- a/rules/CodeQuality/Rector/If_/ArrayExplicitBoolCompareRector.php +++ b/rules/CodeQuality/Rector/If_/ArrayExplicitBoolCompareRector.php @@ -9,13 +9,12 @@ use PhpParser\Node\Expr\Array_; use PhpParser\Node\Expr\BinaryOp\Identical; use PhpParser\Node\Expr\BinaryOp\NotIdentical; -use PhpParser\Node\Expr\BooleanNot; -use PhpParser\Node\Expr\Cast\Bool_; use PhpParser\Node\Expr\Ternary; use PhpParser\Node\Expr\Variable; use PhpParser\Node\Stmt\ElseIf_; use PhpParser\Node\Stmt\If_; -use PHPStan\Type\MixedType; +use Rector\CodeQuality\NodeAnalyzer\ExplicitBoolConditionResolver; +use Rector\CodeQuality\ValueObject\ExplicitBoolCondition; use Rector\NodeTypeResolver\TypeAnalyzer\ArrayTypeAnalyzer; use Rector\Rector\AbstractRector; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; @@ -28,6 +27,7 @@ final class ArrayExplicitBoolCompareRector extends AbstractRector { public function __construct( private readonly ArrayTypeAnalyzer $arrayTypeAnalyzer, + private readonly ExplicitBoolConditionResolver $explicitBoolConditionResolver, ) { } @@ -75,33 +75,17 @@ public function getNodeTypes(): array */ public function refactor(Node $node): ?Node { - // skip short ternary - if ($node instanceof Ternary && ! $node->if instanceof Expr) { + $explicitBoolCondition = $this->explicitBoolConditionResolver->resolve($node); + if (! $explicitBoolCondition instanceof ExplicitBoolCondition) { 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)) { + $expr = $explicitBoolCondition->getConditionNode(); + if (! $this->arrayTypeAnalyzer->isArrayType($expr)) { return null; } - $binaryOp = $this->resolveArray($isNegated, $conditionNode); + $binaryOp = $this->resolveArray($explicitBoolCondition->isNegated(), $expr); if (! $binaryOp instanceof Expr) { return null; } diff --git a/rules/CodeQuality/Rector/If_/ObjectExplicitBoolCompareRector.php b/rules/CodeQuality/Rector/If_/ObjectExplicitBoolCompareRector.php index c0b79587697..d5917d34bf7 100644 --- a/rules/CodeQuality/Rector/If_/ObjectExplicitBoolCompareRector.php +++ b/rules/CodeQuality/Rector/If_/ObjectExplicitBoolCompareRector.php @@ -7,14 +7,14 @@ use PhpParser\Node; use PhpParser\Node\Expr; use PhpParser\Node\Expr\BooleanNot; -use PhpParser\Node\Expr\Cast\Bool_; use PhpParser\Node\Expr\Instanceof_; use PhpParser\Node\Expr\Ternary; use PhpParser\Node\Name\FullyQualified; use PhpParser\Node\Stmt\ElseIf_; use PhpParser\Node\Stmt\If_; -use PHPStan\Type\MixedType; use PHPStan\Type\ObjectType; +use Rector\CodeQuality\NodeAnalyzer\ExplicitBoolConditionResolver; +use Rector\CodeQuality\ValueObject\ExplicitBoolCondition; use Rector\Rector\AbstractRector; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; @@ -24,6 +24,11 @@ */ final class ObjectExplicitBoolCompareRector extends AbstractRector { + public function __construct( + private readonly ExplicitBoolConditionResolver $explicitBoolConditionResolver, + ) { + } + public function getRuleDefinition(): RuleDefinition { return new RuleDefinition('Make nullable object if conditions more explicit', [ @@ -68,34 +73,19 @@ public function getNodeTypes(): array */ public function refactor(Node $node): ?Node { - // skip short ternary - if ($node instanceof Ternary && ! $node->if instanceof Expr) { + $explicitBoolCondition = $this->explicitBoolConditionResolver->resolve($node); + if (! $explicitBoolCondition instanceof ExplicitBoolCondition) { 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; - } + $expr = $explicitBoolCondition->getConditionNode(); - $objectType = $this->nodeTypeResolver->matchNullableTypeOfSpecificType($conditionNode, ObjectType::class); + $objectType = $this->nodeTypeResolver->matchNullableTypeOfSpecificType($expr, ObjectType::class); if (! $objectType instanceof ObjectType) { return null; } - $node->cond = $this->resolveNullable($isNegated, $conditionNode, $objectType); + $node->cond = $this->resolveNullable($explicitBoolCondition->isNegated(), $expr, $objectType); return $node; } diff --git a/rules/CodeQuality/ValueObject/ExplicitBoolCondition.php b/rules/CodeQuality/ValueObject/ExplicitBoolCondition.php new file mode 100644 index 00000000000..a4450e25654 --- /dev/null +++ b/rules/CodeQuality/ValueObject/ExplicitBoolCondition.php @@ -0,0 +1,26 @@ +expr; + } + + public function isNegated(): bool + { + return $this->isNegated; + } +}