Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions rules/CodeQuality/NodeAnalyzer/ExplicitBoolConditionResolver.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
<?php

declare(strict_types=1);

namespace Rector\CodeQuality\NodeAnalyzer;

use PhpParser\Node\Expr;
use PhpParser\Node\Expr\BooleanNot;
use PhpParser\Node\Expr\Cast\Bool_;
use PhpParser\Node\Expr\Ternary;
use PhpParser\Node\Stmt\ElseIf_;
use PhpParser\Node\Stmt\If_;
use PHPStan\Type\MixedType;
use Rector\CodeQuality\ValueObject\ExplicitBoolCondition;
use Rector\NodeTypeResolver\NodeTypeResolver;

final readonly class ExplicitBoolConditionResolver
{
public function __construct(
private NodeTypeResolver $nodeTypeResolver
) {
}

public function resolve(If_|ElseIf_|Ternary $node): ?ExplicitBoolCondition
{
// 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;
}

return new ExplicitBoolCondition($conditionNode, $isNegated);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
<?php

declare(strict_types=1);

namespace Rector\CodeQuality\NodeFactory;

use PhpParser\Node\Arg;
use Rector\CodeQuality\ValueObject\ComparedExprAndValueExpr;
use Rector\PhpParser\Comparing\NodeComparator;
use Rector\PhpParser\Node\NodeFactory;

final readonly class InArrayFromRepeatedCompareFactory
{
public function __construct(
private NodeComparator $nodeComparator,
private NodeFactory $nodeFactory
) {
}

/**
* Builds the "$value, [...]" args of an in_array() call from a repeated compare chain,
* once all compared expressions are confirmed equal. Returns null when the chain is too
* short or the compared expressions differ.
*
* @param ComparedExprAndValueExpr[] $comparedExprAndValueExprs
* @return Arg[]|null
*/
public function createInArrayArgs(array $comparedExprAndValueExprs): ?array
{
if (count($comparedExprAndValueExprs) < 3) {
return null;
}

$valueExprs = [];
foreach ($comparedExprAndValueExprs as $comparedExprAndValueExpr) {
$valueExprs[] = $comparedExprAndValueExpr->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]);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -27,6 +28,7 @@ final class RepeatedAndNotEqualToNotInArrayRector extends AbstractRector
{
public function __construct(
private readonly BetterNodeFinder $betterNodeFinder,
private readonly InArrayFromRepeatedCompareFactory $inArrayFromRepeatedCompareFactory,
) {
}

Expand Down Expand Up @@ -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')));
}
Expand All @@ -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[]
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -26,6 +27,7 @@ final class RepeatedOrEqualToInArrayRector extends AbstractRector
{
public function __construct(
private readonly BetterNodeFinder $betterNodeFinder,
private readonly InArrayFromRepeatedCompareFactory $inArrayFromRepeatedCompareFactory,
) {
}

Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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[]
*/
Expand Down
32 changes: 8 additions & 24 deletions rules/CodeQuality/Rector/If_/ArrayExplicitBoolCompareRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -28,6 +27,7 @@ final class ArrayExplicitBoolCompareRector extends AbstractRector
{
public function __construct(
private readonly ArrayTypeAnalyzer $arrayTypeAnalyzer,
private readonly ExplicitBoolConditionResolver $explicitBoolConditionResolver,
) {
}

Expand Down Expand Up @@ -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;
}
Expand Down
34 changes: 12 additions & 22 deletions rules/CodeQuality/Rector/If_/ObjectExplicitBoolCompareRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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', [
Expand Down Expand Up @@ -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;
}
Expand Down
Loading
Loading