Skip to content

Commit bd2aa50

Browse files
TomasVotrubaTomasVotruba
authored
[CodeQuality] Extract shared services to de-duplicate explicit bool compare and repeated compare rules (#8506)
1 parent ad71353 commit bd2aa50

7 files changed

Lines changed: 160 additions & 118 deletions

‎rules/CodeQuality/NodeAnalyzer/ExplicitBoolConditionResolver.php‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\CodeQuality\NodeAnalyzer;
6+
7+
use PhpParser\Node\Expr;
8+
use PhpParser\Node\Expr\BooleanNot;
9+
use PhpParser\Node\Expr\Cast\Bool_;
10+
use PhpParser\Node\Expr\Ternary;
11+
use PhpParser\Node\Stmt\ElseIf_;
12+
use PhpParser\Node\Stmt\If_;
13+
use PHPStan\Type\MixedType;
14+
use Rector\CodeQuality\ValueObject\ExplicitBoolCondition;
15+
use Rector\NodeTypeResolver\NodeTypeResolver;
16+
17+
final readonly class ExplicitBoolConditionResolver
18+
{
19+
public function __construct(
20+
private NodeTypeResolver $nodeTypeResolver
21+
) {
22+
}
23+
24+
public function resolve(If_|ElseIf_|Ternary $node): ?ExplicitBoolCondition
25+
{
26+
// skip short ternary
27+
if ($node instanceof Ternary && ! $node->if instanceof Expr) {
28+
return null;
29+
}
30+
31+
if ($node->cond instanceof BooleanNot) {
32+
$conditionNode = $node->cond->expr;
33+
$isNegated = true;
34+
} else {
35+
$conditionNode = $node->cond;
36+
$isNegated = false;
37+
}
38+
39+
if ($conditionNode instanceof Bool_) {
40+
return null;
41+
}
42+
43+
$conditionStaticType = $this->nodeTypeResolver->getNativeType($conditionNode);
44+
if ($conditionStaticType instanceof MixedType || $conditionStaticType->isBoolean()->yes()) {
45+
return null;
46+
}
47+
48+
return new ExplicitBoolCondition($conditionNode, $isNegated);
49+
}
50+
}

‎rules/CodeQuality/NodeFactory/InArrayFromRepeatedCompareFactory.php‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\CodeQuality\NodeFactory;
6+
7+
use PhpParser\Node\Arg;
8+
use Rector\CodeQuality\ValueObject\ComparedExprAndValueExpr;
9+
use Rector\PhpParser\Comparing\NodeComparator;
10+
use Rector\PhpParser\Node\NodeFactory;
11+
12+
final readonly class InArrayFromRepeatedCompareFactory
13+
{
14+
public function __construct(
15+
private NodeComparator $nodeComparator,
16+
private NodeFactory $nodeFactory
17+
) {
18+
}
19+
20+
/**
21+
* Builds the "$value, [...]" args of an in_array() call from a repeated compare chain,
22+
* once all compared expressions are confirmed equal. Returns null when the chain is too
23+
* short or the compared expressions differ.
24+
*
25+
* @param ComparedExprAndValueExpr[] $comparedExprAndValueExprs
26+
* @return Arg[]|null
27+
*/
28+
public function createInArrayArgs(array $comparedExprAndValueExprs): ?array
29+
{
30+
if (count($comparedExprAndValueExprs) < 3) {
31+
return null;
32+
}
33+
34+
$valueExprs = [];
35+
foreach ($comparedExprAndValueExprs as $comparedExprAndValueExpr) {
36+
$valueExprs[] = $comparedExprAndValueExpr->getValueExpr();
37+
}
38+
39+
/** @var ComparedExprAndValueExpr $firstComparedExprAndValue */
40+
$firstComparedExprAndValue = array_pop($comparedExprAndValueExprs);
41+
42+
// all compared expr must be equal
43+
foreach ($comparedExprAndValueExprs as $comparedExprAndValueExpr) {
44+
if (! $this->nodeComparator->areNodesEqual(
45+
$firstComparedExprAndValue->getComparedExpr(),
46+
$comparedExprAndValueExpr->getComparedExpr()
47+
)) {
48+
return null;
49+
}
50+
}
51+
52+
$array = $this->nodeFactory->createArray($valueExprs);
53+
54+
return $this->nodeFactory->createArgs([$firstComparedExprAndValue->getComparedExpr(), $array]);
55+
}
56+
}

‎rules/CodeQuality/Rector/BooleanAnd/RepeatedAndNotEqualToNotInArrayRector.php‎

Lines changed: 4 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use PhpParser\Node\Expr\ConstFetch;
1515
use PhpParser\Node\Expr\FuncCall;
1616
use PhpParser\Node\Name;
17+
use Rector\CodeQuality\NodeFactory\InArrayFromRepeatedCompareFactory;
1718
use Rector\CodeQuality\ValueObject\ComparedExprAndValueExpr;
1819
use Rector\PhpParser\Node\BetterNodeFinder;
1920
use Rector\Rector\AbstractRector;
@@ -27,6 +28,7 @@ final class RepeatedAndNotEqualToNotInArrayRector extends AbstractRector
2728
{
2829
public function __construct(
2930
private readonly BetterNodeFinder $betterNodeFinder,
31+
private readonly InArrayFromRepeatedCompareFactory $inArrayFromRepeatedCompareFactory,
3032
) {
3133
}
3234

@@ -80,30 +82,11 @@ public function refactor(Node $node): ?BooleanNot
8082
return null;
8183
}
8284

83-
if (count($comparedExprAndValueExprs) < 3) {
85+
$args = $this->inArrayFromRepeatedCompareFactory->createInArrayArgs($comparedExprAndValueExprs);
86+
if ($args === null) {
8487
return null;
8588
}
8689

87-
// ensure all compared expr are the same
88-
$valueExprs = $this->resolveValueExprs($comparedExprAndValueExprs);
89-
90-
/** @var ComparedExprAndValueExpr $firstComparedExprAndValue */
91-
$firstComparedExprAndValue = array_pop($comparedExprAndValueExprs);
92-
93-
// all compared expr must be equal
94-
foreach ($comparedExprAndValueExprs as $comparedExprAndValueExpr) {
95-
if (! $this->nodeComparator->areNodesEqual(
96-
$firstComparedExprAndValue->getComparedExpr(),
97-
$comparedExprAndValueExpr->getComparedExpr()
98-
)) {
99-
return null;
100-
}
101-
}
102-
103-
$array = $this->nodeFactory->createArray($valueExprs);
104-
105-
$args = $this->nodeFactory->createArgs([$firstComparedExprAndValue->getComparedExpr(), $array]);
106-
10790
if ($this->isStrictComparison($node)) {
10891
$args[] = new Arg(new ConstFetch(new Name('true')));
10992
}
@@ -127,21 +110,6 @@ private function matchComparedExprAndValueExpr(NotIdentical|NotEqual $expr): Com
127110
return new ComparedExprAndValueExpr($expr->left, $expr->right);
128111
}
129112

130-
/**
131-
* @param ComparedExprAndValueExpr[] $comparedExprAndValueExprs
132-
* @return Expr[]
133-
*/
134-
private function resolveValueExprs(array $comparedExprAndValueExprs): array
135-
{
136-
$valueExprs = [];
137-
138-
foreach ($comparedExprAndValueExprs as $comparedExprAndValueExpr) {
139-
$valueExprs[] = $comparedExprAndValueExpr->getValueExpr();
140-
}
141-
142-
return $valueExprs;
143-
}
144-
145113
/**
146114
* @return null|ComparedExprAndValueExpr[]
147115
*/

‎rules/CodeQuality/Rector/BooleanOr/RepeatedOrEqualToInArrayRector.php‎

Lines changed: 4 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
use PhpParser\Node\Expr\ConstFetch;
1414
use PhpParser\Node\Expr\FuncCall;
1515
use PhpParser\Node\Name;
16+
use Rector\CodeQuality\NodeFactory\InArrayFromRepeatedCompareFactory;
1617
use Rector\CodeQuality\ValueObject\ComparedExprAndValueExpr;
1718
use Rector\PhpParser\Node\BetterNodeFinder;
1819
use Rector\Rector\AbstractRector;
@@ -26,6 +27,7 @@ final class RepeatedOrEqualToInArrayRector extends AbstractRector
2627
{
2728
public function __construct(
2829
private readonly BetterNodeFinder $betterNodeFinder,
30+
private readonly InArrayFromRepeatedCompareFactory $inArrayFromRepeatedCompareFactory,
2931
) {
3032
}
3133

@@ -79,30 +81,11 @@ public function refactor(Node $node): ?FuncCall
7981
return null;
8082
}
8183

82-
if (count($comparedExprAndValueExprs) < 3) {
84+
$args = $this->inArrayFromRepeatedCompareFactory->createInArrayArgs($comparedExprAndValueExprs);
85+
if ($args === null) {
8386
return null;
8487
}
8588

86-
// ensure all compared expr are the same
87-
$valueExprs = $this->resolveValueExprs($comparedExprAndValueExprs);
88-
89-
/** @var ComparedExprAndValueExpr $firstComparedExprAndValue */
90-
$firstComparedExprAndValue = array_pop($comparedExprAndValueExprs);
91-
92-
// all compared expr must be equal
93-
foreach ($comparedExprAndValueExprs as $comparedExprAndValueExpr) {
94-
if (! $this->nodeComparator->areNodesEqual(
95-
$firstComparedExprAndValue->getComparedExpr(),
96-
$comparedExprAndValueExpr->getComparedExpr()
97-
)) {
98-
return null;
99-
}
100-
}
101-
102-
$array = $this->nodeFactory->createArray($valueExprs);
103-
104-
$args = $this->nodeFactory->createArgs([$firstComparedExprAndValue->getComparedExpr(), $array]);
105-
10689
$identicals = $this->betterNodeFinder->findInstanceOf($node, Identical::class);
10790
$equals = $this->betterNodeFinder->findInstanceOf($node, Equal::class);
10891

@@ -133,21 +116,6 @@ private function matchComparedExprAndValueExpr(Identical|Equal $expr): ComparedE
133116
return new ComparedExprAndValueExpr($expr->left, $expr->right);
134117
}
135118

136-
/**
137-
* @param ComparedExprAndValueExpr[] $comparedExprAndValueExprs
138-
* @return Expr[]
139-
*/
140-
private function resolveValueExprs(array $comparedExprAndValueExprs): array
141-
{
142-
$valueExprs = [];
143-
144-
foreach ($comparedExprAndValueExprs as $comparedExprAndValueExpr) {
145-
$valueExprs[] = $comparedExprAndValueExpr->getValueExpr();
146-
}
147-
148-
return $valueExprs;
149-
}
150-
151119
/**
152120
* @return null|ComparedExprAndValueExpr[]
153121
*/

‎rules/CodeQuality/Rector/If_/ArrayExplicitBoolCompareRector.php‎

Lines changed: 8 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,12 @@
99
use PhpParser\Node\Expr\Array_;
1010
use PhpParser\Node\Expr\BinaryOp\Identical;
1111
use PhpParser\Node\Expr\BinaryOp\NotIdentical;
12-
use PhpParser\Node\Expr\BooleanNot;
13-
use PhpParser\Node\Expr\Cast\Bool_;
1412
use PhpParser\Node\Expr\Ternary;
1513
use PhpParser\Node\Expr\Variable;
1614
use PhpParser\Node\Stmt\ElseIf_;
1715
use PhpParser\Node\Stmt\If_;
18-
use PHPStan\Type\MixedType;
16+
use Rector\CodeQuality\NodeAnalyzer\ExplicitBoolConditionResolver;
17+
use Rector\CodeQuality\ValueObject\ExplicitBoolCondition;
1918
use Rector\NodeTypeResolver\TypeAnalyzer\ArrayTypeAnalyzer;
2019
use Rector\Rector\AbstractRector;
2120
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
@@ -28,6 +27,7 @@ final class ArrayExplicitBoolCompareRector extends AbstractRector
2827
{
2928
public function __construct(
3029
private readonly ArrayTypeAnalyzer $arrayTypeAnalyzer,
30+
private readonly ExplicitBoolConditionResolver $explicitBoolConditionResolver,
3131
) {
3232
}
3333

@@ -75,33 +75,17 @@ public function getNodeTypes(): array
7575
*/
7676
public function refactor(Node $node): ?Node
7777
{
78-
// skip short ternary
79-
if ($node instanceof Ternary && ! $node->if instanceof Expr) {
78+
$explicitBoolCondition = $this->explicitBoolConditionResolver->resolve($node);
79+
if (! $explicitBoolCondition instanceof ExplicitBoolCondition) {
8080
return null;
8181
}
8282

83-
if ($node->cond instanceof BooleanNot) {
84-
$conditionNode = $node->cond->expr;
85-
$isNegated = true;
86-
} else {
87-
$conditionNode = $node->cond;
88-
$isNegated = false;
89-
}
90-
91-
if ($conditionNode instanceof Bool_) {
92-
return null;
93-
}
94-
95-
$conditionStaticType = $this->nodeTypeResolver->getNativeType($conditionNode);
96-
if ($conditionStaticType instanceof MixedType || $conditionStaticType->isBoolean()->yes()) {
97-
return null;
98-
}
99-
100-
if (! $this->arrayTypeAnalyzer->isArrayType($conditionNode)) {
83+
$expr = $explicitBoolCondition->getConditionNode();
84+
if (! $this->arrayTypeAnalyzer->isArrayType($expr)) {
10185
return null;
10286
}
10387

104-
$binaryOp = $this->resolveArray($isNegated, $conditionNode);
88+
$binaryOp = $this->resolveArray($explicitBoolCondition->isNegated(), $expr);
10589
if (! $binaryOp instanceof Expr) {
10690
return null;
10791
}

‎rules/CodeQuality/Rector/If_/ObjectExplicitBoolCompareRector.php‎

Lines changed: 12 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,14 @@
77
use PhpParser\Node;
88
use PhpParser\Node\Expr;
99
use PhpParser\Node\Expr\BooleanNot;
10-
use PhpParser\Node\Expr\Cast\Bool_;
1110
use PhpParser\Node\Expr\Instanceof_;
1211
use PhpParser\Node\Expr\Ternary;
1312
use PhpParser\Node\Name\FullyQualified;
1413
use PhpParser\Node\Stmt\ElseIf_;
1514
use PhpParser\Node\Stmt\If_;
16-
use PHPStan\Type\MixedType;
1715
use PHPStan\Type\ObjectType;
16+
use Rector\CodeQuality\NodeAnalyzer\ExplicitBoolConditionResolver;
17+
use Rector\CodeQuality\ValueObject\ExplicitBoolCondition;
1818
use Rector\Rector\AbstractRector;
1919
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
2020
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
@@ -24,6 +24,11 @@
2424
*/
2525
final class ObjectExplicitBoolCompareRector extends AbstractRector
2626
{
27+
public function __construct(
28+
private readonly ExplicitBoolConditionResolver $explicitBoolConditionResolver,
29+
) {
30+
}
31+
2732
public function getRuleDefinition(): RuleDefinition
2833
{
2934
return new RuleDefinition('Make nullable object if conditions more explicit', [
@@ -68,34 +73,19 @@ public function getNodeTypes(): array
6873
*/
6974
public function refactor(Node $node): ?Node
7075
{
71-
// skip short ternary
72-
if ($node instanceof Ternary && ! $node->if instanceof Expr) {
76+
$explicitBoolCondition = $this->explicitBoolConditionResolver->resolve($node);
77+
if (! $explicitBoolCondition instanceof ExplicitBoolCondition) {
7378
return null;
7479
}
7580

76-
if ($node->cond instanceof BooleanNot) {
77-
$conditionNode = $node->cond->expr;
78-
$isNegated = true;
79-
} else {
80-
$conditionNode = $node->cond;
81-
$isNegated = false;
82-
}
83-
84-
if ($conditionNode instanceof Bool_) {
85-
return null;
86-
}
87-
88-
$conditionStaticType = $this->nodeTypeResolver->getNativeType($conditionNode);
89-
if ($conditionStaticType instanceof MixedType || $conditionStaticType->isBoolean()->yes()) {
90-
return null;
91-
}
81+
$expr = $explicitBoolCondition->getConditionNode();
9282

93-
$objectType = $this->nodeTypeResolver->matchNullableTypeOfSpecificType($conditionNode, ObjectType::class);
83+
$objectType = $this->nodeTypeResolver->matchNullableTypeOfSpecificType($expr, ObjectType::class);
9484
if (! $objectType instanceof ObjectType) {
9585
return null;
9686
}
9787

98-
$node->cond = $this->resolveNullable($isNegated, $conditionNode, $objectType);
88+
$node->cond = $this->resolveNullable($explicitBoolCondition->isNegated(), $expr, $objectType);
9989

10090
return $node;
10191
}

0 commit comments

Comments
 (0)