Skip to content

Commit 56ec764

Browse files
committed
Add OrChainIdenticalComparisonToInArrayRule
1 parent 5ab9acc commit 56ec764

File tree

6 files changed

+276
-0
lines changed

6 files changed

+276
-0
lines changed
Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Build;
4+
5+
use PhpParser\Node;
6+
use PhpParser\Node\Arg;
7+
use PhpParser\Node\ArrayItem;
8+
use PhpParser\Node\Expr;
9+
use PhpParser\Node\Expr\Array_;
10+
use PhpParser\Node\Expr\BinaryOp\BooleanOr;
11+
use PhpParser\Node\Expr\BinaryOp\Identical;
12+
use PhpParser\Node\Expr\ClassConstFetch;
13+
use PhpParser\Node\Expr\ConstFetch;
14+
use PhpParser\Node\Expr\FuncCall;
15+
use PhpParser\Node\Name;
16+
use PhpParser\Node\Scalar;
17+
use PhpParser\Node\Stmt\If_;
18+
use PHPStan\Analyser\Scope;
19+
use PHPStan\DependencyInjection\AutowiredParameter;
20+
use PHPStan\File\FileHelper;
21+
use PHPStan\Fixable\PhpPrinter;
22+
use PHPStan\Rules\IdentifierRuleError;
23+
use PHPStan\Rules\Rule;
24+
use PHPStan\Rules\RuleErrorBuilder;
25+
use function array_map;
26+
use function array_merge;
27+
use function array_shift;
28+
use function count;
29+
use function dirname;
30+
use function str_starts_with;
31+
32+
/**
33+
* @implements Rule<If_>
34+
*/
35+
final class OrChainIdenticalComparisonToInArrayRule implements Rule
36+
{
37+
38+
public function __construct(
39+
#[AutowiredParameter]
40+
private PhpPrinter $printer,
41+
private FileHelper $fileHelper,
42+
private bool $skipTests = true,
43+
)
44+
{
45+
}
46+
47+
public function getNodeType(): string
48+
{
49+
return If_::class;
50+
}
51+
52+
public function processNode(Node $node, Scope $scope): array
53+
{
54+
$errors = $this->processConditionNode($node->cond, $scope);
55+
foreach ($node->elseifs as $elseifCondNode) {
56+
$errors = array_merge($errors, $this->processConditionNode($elseifCondNode->cond, $scope));
57+
}
58+
59+
return $errors;
60+
}
61+
62+
/**
63+
* @return list<IdentifierRuleError>
64+
*/
65+
public function processConditionNode(Expr $condNode, Scope $scope): array
66+
{
67+
$comparisons = $this->unpackOrChain($condNode);
68+
if (count($comparisons) < 2) {
69+
return [];
70+
}
71+
72+
$firstComparison = array_shift($comparisons);
73+
if (!$firstComparison instanceof Identical) {
74+
return [];
75+
}
76+
77+
$subjectAndValue = $this->getSubjectAndValue($firstComparison);
78+
if ($subjectAndValue === null) {
79+
return [];
80+
}
81+
82+
if ($this->skipTests && str_starts_with($this->fileHelper->normalizePath($scope->getFile()), $this->fileHelper->normalizePath(dirname(__DIR__, 3) . '/tests'))) {
83+
return [];
84+
}
85+
86+
$subjectNode = $subjectAndValue['subject'];
87+
$subjectStr = $this->printer->prettyPrintExpr($subjectNode);
88+
$values = [$subjectAndValue['value']];
89+
90+
foreach ($comparisons as $comparison) {
91+
if (!$comparison instanceof Identical) {
92+
return [];
93+
}
94+
95+
$currentSubjectAndValue = $this->getSubjectAndValue($comparison);
96+
if ($currentSubjectAndValue === null) {
97+
return [];
98+
}
99+
100+
if ($this->printer->prettyPrintExpr($currentSubjectAndValue['subject']) !== $subjectStr) {
101+
return [];
102+
}
103+
104+
$values[] = $currentSubjectAndValue['value'];
105+
}
106+
107+
$errorBuilder = RuleErrorBuilder::message('This chain of identical comparisons can be simplified using in_array().')
108+
->line($condNode->getStartLine())
109+
->fixNode($condNode, static fn (Expr $node) => self::createInArrayCall($subjectNode, $values))
110+
->identifier('or.chainIdenticalComparison');
111+
112+
return [$errorBuilder->build()];
113+
}
114+
115+
/**
116+
* @return list<Expr>
117+
*/
118+
private function unpackOrChain(Expr $node): array
119+
{
120+
if ($node instanceof BooleanOr) {
121+
return [...$this->unpackOrChain($node->left), ...$this->unpackOrChain($node->right)];
122+
}
123+
124+
return [$node];
125+
}
126+
127+
/**
128+
* @phpstan-assert-if-true Scalar|ClassConstFetch|ConstFetch $node
129+
*/
130+
private static function isSubjectNode(Expr $node): bool
131+
{
132+
return $node instanceof Scalar || $node instanceof ClassConstFetch || $node instanceof ConstFetch;
133+
}
134+
135+
/**
136+
* @return array{subject: Expr, value: Scalar|ClassConstFetch|ConstFetch}|null
137+
*/
138+
private function getSubjectAndValue(Identical $comparison): ?array
139+
{
140+
if (self::isSubjectNode($comparison->left) && !self::isSubjectNode($comparison->left)) {
141+
return ['subject' => $comparison->right, 'value' => $comparison->left];
142+
}
143+
144+
if (!self::isSubjectNode($comparison->left) && self::isSubjectNode($comparison->right)) {
145+
return ['subject' => $comparison->left, 'value' => $comparison->right];
146+
}
147+
148+
return null;
149+
}
150+
151+
/**
152+
* @param list<Scalar|ClassConstFetch|ConstFetch> $values
153+
*/
154+
private static function createInArrayCall(Expr $subjectNode, array $values): FuncCall
155+
{
156+
return new FuncCall(new Name('\in_array'), [
157+
new Arg($subjectNode),
158+
new Arg(new Array_(array_map(static fn ($value)=> new ArrayItem($value), $values))),
159+
new Arg(new ConstFetch(new Name('true'))),
160+
]);
161+
}
162+
163+
}

build/phpstan.neon

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@ rules:
131131
- PHPStan\Build\OverrideAttributeThirdPartyMethodRule
132132
- PHPStan\Build\SkipTestsWithRequiresPhpAttributeRule
133133
- PHPStan\Build\MemoizationPropertyRule
134+
- PHPStan\Build\OrChainIdenticalComparisonToInArrayRule
134135

135136
services:
136137
-

src/Fixable/PhpPrinter.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,11 @@
55
use Override;
66
use PhpParser\Node;
77
use PhpParser\PrettyPrinter\Standard;
8+
use PHPStan\DependencyInjection\AutowiredService;
89
use function count;
910
use function rtrim;
1011

12+
#[AutowiredService]
1113
final class PhpPrinter extends Standard
1214
{
1315

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Build;
4+
5+
use PHPStan\File\FileHelper;
6+
use PHPStan\Fixable\PhpPrinter;
7+
use PHPStan\Rules\Rule;
8+
use PHPStan\Testing\RuleTestCase;
9+
10+
/**
11+
* @extends RuleTestCase<OrChainIdenticalComparisonToInArrayRule>
12+
*/
13+
final class OrChainIdenticalComparisonToInArrayRuleTest extends RuleTestCase
14+
{
15+
16+
protected function getRule(): Rule
17+
{
18+
return new OrChainIdenticalComparisonToInArrayRule(new PhpPrinter(), self::getContainer()->getByType(FileHelper::class), false);
19+
}
20+
21+
public function testRule(): void
22+
{
23+
$this->analyse([__DIR__ . '/data/or-chain-identical-comparison.php'], [
24+
[
25+
'This chain of identical comparisons can be simplified using in_array().',
26+
7,
27+
],
28+
[
29+
'This chain of identical comparisons can be simplified using in_array().',
30+
11,
31+
],
32+
[
33+
'This chain of identical comparisons can be simplified using in_array().',
34+
15,
35+
],
36+
[
37+
'This chain of identical comparisons can be simplified using in_array().',
38+
17,
39+
],
40+
]);
41+
}
42+
43+
public function testFix(): void
44+
{
45+
$this->fix(__DIR__ . '/data/or-chain-identical-comparison.php', __DIR__ . '/data/or-chain-identical-comparison.php.fixed');
46+
}
47+
48+
}
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<?php
2+
3+
if ($var === 'foo') {
4+
echo 'ok';
5+
}
6+
7+
if ($var === 'foo' || $var === 'bar') {
8+
echo 'ok';
9+
}
10+
11+
if ($var === 'foo' || $var === 'bar' || $var === 'buz') {
12+
echo 'ok';
13+
}
14+
15+
if ($var === 'foo' || $var === 'bar' || $var === 'buz') {
16+
echo 'ok';
17+
} elseif ($var === 'foofoo' || $var === 'barbar' || $var === 'buzbuz') {
18+
echo 'ok';
19+
}
20+
21+
if ($var === 'foo' || $var === 'bar' || $var2 === 'buz') {
22+
echo 'no';
23+
}
24+
25+
if ($var === 'foo' || $var2 === 'bar' || $var === 'buz') {
26+
echo 'no';
27+
}
28+
29+
if ($var === 'foo' || $var2 === 'bar' || $var2 === 'buz') {
30+
echo 'no';
31+
}
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<?php
2+
3+
if ($var === 'foo') {
4+
echo 'ok';
5+
}
6+
7+
if (\in_array($var, ['foo', 'bar'], true)) {
8+
echo 'ok';
9+
}
10+
11+
if (\in_array($var, ['foo', 'bar', 'buz'], true)) {
12+
echo 'ok';
13+
}
14+
15+
if (\in_array($var, ['foo', 'bar', 'buz'], true)) {
16+
echo 'ok';
17+
} elseif (\in_array($var, ['foofoo', 'barbar', 'buzbuz'], true)) {
18+
echo 'ok';
19+
}
20+
21+
if ($var === 'foo' || $var === 'bar' || $var2 === 'buz') {
22+
echo 'no';
23+
}
24+
25+
if ($var === 'foo' || $var2 === 'bar' || $var === 'buz') {
26+
echo 'no';
27+
}
28+
29+
if ($var === 'foo' || $var2 === 'bar' || $var2 === 'buz') {
30+
echo 'no';
31+
}

0 commit comments

Comments
 (0)