Skip to content

Commit 87c524a

Browse files
zonuexezonuexe
authored
Simplify by replacing chained $var === const with in_array() 
1 parent bf703ef commit 87c524a

12 files changed

+294
-15
lines changed

‎build/PHPStan/Build/OrChainIdenticalComparisonToInArrayRule.php

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
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\Node\Printer\ExprPrinter;
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+
private ExprPrinter $printer,
40+
private FileHelper $fileHelper,
41+
private bool $skipTests = true,
42+
)
43+
{
44+
}
45+
46+
public function getNodeType(): string
47+
{
48+
return If_::class;
49+
}
50+
51+
public function processNode(Node $node, Scope $scope): array
52+
{
53+
$errors = $this->processConditionNode($node->cond, $scope);
54+
foreach ($node->elseifs as $elseifCondNode) {
55+
$errors = array_merge($errors, $this->processConditionNode($elseifCondNode->cond, $scope));
56+
}
57+
58+
return $errors;
59+
}
60+
61+
/**
62+
* @return list<IdentifierRuleError>
63+
*/
64+
public function processConditionNode(Expr $condNode, Scope $scope): array
65+
{
66+
$comparisons = $this->unpackOrChain($condNode);
67+
if (count($comparisons) < 2) {
68+
return [];
69+
}
70+
71+
$firstComparison = array_shift($comparisons);
72+
if (!$firstComparison instanceof Identical) {
73+
return [];
74+
}
75+
76+
$subjectAndValue = $this->getSubjectAndValue($firstComparison);
77+
if ($subjectAndValue === null) {
78+
return [];
79+
}
80+
81+
if ($this->skipTests && str_starts_with($this->fileHelper->normalizePath($scope->getFile()), $this->fileHelper->normalizePath(dirname(__DIR__, 3) . '/tests'))) {
82+
return [];
83+
}
84+
85+
$subjectNode = $subjectAndValue['subject'];
86+
$subjectStr = $this->printer->printExpr($subjectNode);
87+
$values = [$subjectAndValue['value']];
88+
89+
foreach ($comparisons as $comparison) {
90+
if (!$comparison instanceof Identical) {
91+
return [];
92+
}
93+
94+
$currentSubjectAndValue = $this->getSubjectAndValue($comparison);
95+
if ($currentSubjectAndValue === null) {
96+
return [];
97+
}
98+
99+
if ($this->printer->printExpr($currentSubjectAndValue['subject']) !== $subjectStr) {
100+
return [];
101+
}
102+
103+
$values[] = $currentSubjectAndValue['value'];
104+
}
105+
106+
$errorBuilder = RuleErrorBuilder::message('This chain of identical comparisons can be simplified using in_array().')
107+
->line($condNode->getStartLine())
108+
->fixNode($condNode, static fn (Expr $node) => self::createInArrayCall($subjectNode, $values))
109+
->identifier('or.chainIdenticalComparison');
110+
111+
return [$errorBuilder->build()];
112+
}
113+
114+
/**
115+
* @return list<Expr>
116+
*/
117+
private function unpackOrChain(Expr $node): array
118+
{
119+
if ($node instanceof BooleanOr) {
120+
return [...$this->unpackOrChain($node->left), ...$this->unpackOrChain($node->right)];
121+
}
122+
123+
return [$node];
124+
}
125+
126+
/**
127+
* @phpstan-assert-if-true Scalar|ClassConstFetch|ConstFetch $node
128+
*/
129+
private static function isSubjectNode(Expr $node): bool
130+
{
131+
return $node instanceof Scalar || $node instanceof ClassConstFetch || $node instanceof ConstFetch;
132+
}
133+
134+
/**
135+
* @return array{subject: Expr, value: Scalar|ClassConstFetch|ConstFetch}|null
136+
*/
137+
private function getSubjectAndValue(Identical $comparison): ?array
138+
{
139+
if (self::isSubjectNode($comparison->left) && !self::isSubjectNode($comparison->left)) {
140+
return ['subject' => $comparison->right, 'value' => $comparison->left];
141+
}
142+
143+
if (!self::isSubjectNode($comparison->left) && self::isSubjectNode($comparison->right)) {
144+
return ['subject' => $comparison->left, 'value' => $comparison->right];
145+
}
146+
147+
return null;
148+
}
149+
150+
/**
151+
* @param list<Scalar|ClassConstFetch|ConstFetch> $values
152+
*/
153+
private static function createInArrayCall(Expr $subjectNode, array $values): FuncCall
154+
{
155+
return new FuncCall(new Name('\in_array'), [
156+
new Arg($subjectNode),
157+
new Arg(new Array_(array_map(static fn ($value) => new ArrayItem($value), $values))),
158+
new Arg(new ConstFetch(new Name('true'))),
159+
]);
160+
}
161+
162+
}

‎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/PhpPrinterIndentationDetectorVisitor.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
use PhpParser\NodeVisitor;
99
use PhpParser\NodeVisitorAbstract;
1010
use function count;
11+
use function in_array;
1112
use function is_array;
1213
use function preg_match;
1314
use function preg_match_all;
@@ -47,7 +48,7 @@ public function enterNode(Node $node): ?int
4748
$text = $this->origTokens->getTokenCode($node->getStartTokenPos(), $firstStmt->getStartTokenPos(), 0);
4849

4950
$c = preg_match_all('~\n([\\x09\\x20]*)~', $text, $matches, PREG_SET_ORDER);
50-
if ($c === 0 || $c === false) {
51+
if (in_array($c, [0, false], true)) {
5152
return null;
5253
}
5354

‎src/Reflection/InitializerExprTypeResolver.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -864,7 +864,7 @@ public function getDivType(Expr $left, Expr $right, callable $getTypeCallback):
864864

865865
$rightScalarValues = $rightType->toNumber()->getConstantScalarValues();
866866
foreach ($rightScalarValues as $scalarValue) {
867-
if ($scalarValue === 0 || $scalarValue === 0.0) {
867+
if (in_array($scalarValue, [0, 0.0], true)) {
868868
return new ErrorType();
869869
}
870870
}
@@ -938,7 +938,7 @@ public function getModType(Expr $left, Expr $right, callable $getTypeCallback):
938938
$rightScalarValues = $rightType->toNumber()->getConstantScalarValues();
939939
foreach ($rightScalarValues as $scalarValue) {
940940

941-
if ($scalarValue === 0 || $scalarValue === 0.0) {
941+
if (in_array($scalarValue, [0, 0.0], true)) {
942942
return new ErrorType();
943943
}
944944
}

‎src/Rules/Keywords/ContinueBreakInLoopRule.php

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
use PHPStan\Rules\Rule;
1111
use PHPStan\Rules\RuleErrorBuilder;
1212
use function array_reverse;
13+
use function in_array;
1314
use function sprintf;
1415

1516
/**
@@ -52,13 +53,13 @@ public function processNode(Node $node, Scope $scope): array
5253
->build(),
5354
];
5455
}
55-
if (
56-
$parentStmtType === Stmt\For_::class
57-
|| $parentStmtType === Stmt\Foreach_::class
58-
|| $parentStmtType === Stmt\Do_::class
59-
|| $parentStmtType === Stmt\While_::class
60-
|| $parentStmtType === Stmt\Switch_::class
61-
) {
56+
if (in_array($parentStmtType, [
57+
Stmt\For_::class,
58+
Stmt\Foreach_::class,
59+
Stmt\Do_::class,
60+
Stmt\While_::class,
61+
Stmt\Switch_::class,
62+
], true)) {
6263
$value--;
6364
}
6465
if ($value === 0) {

‎src/Rules/UnusedFunctionParametersCheck.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use function array_combine;
1313
use function array_map;
1414
use function array_merge;
15+
use function in_array;
1516
use function is_array;
1617
use function is_string;
1718
use function sprintf;
@@ -78,7 +79,7 @@ private function getUsedVariables(Scope $scope, $node): array
7879
if ($node instanceof Node) {
7980
if ($node instanceof Node\Expr\FuncCall && $node->name instanceof Node\Name) {
8081
$functionName = $this->reflectionProvider->resolveFunctionName($node->name, $scope);
81-
if ($functionName === 'func_get_args' || $functionName === 'get_defined_vars') {
82+
if (in_array($functionName, ['func_get_args', 'get_defined_vars'], true)) {
8283
return $scope->getDefinedVariables();
8384
}
8485
}

‎src/Type/Php/DsMapDynamicMethodThrowTypeExtension.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
use PHPStan\Type\Type;
1111
use PHPStan\Type\VoidType;
1212
use function count;
13+
use function in_array;
1314

1415
#[AutowiredService]
1516
final class DsMapDynamicMethodThrowTypeExtension implements DynamicMethodThrowTypeExtension
@@ -18,7 +19,7 @@ final class DsMapDynamicMethodThrowTypeExtension implements DynamicMethodThrowTy
1819
public function isMethodSupported(MethodReflection $methodReflection): bool
1920
{
2021
return $methodReflection->getDeclaringClass()->getName() === 'Ds\Map'
21-
&& ($methodReflection->getName() === 'get' || $methodReflection->getName() === 'remove');
22+
&& in_array($methodReflection->getName(), ['get', 'remove'], true);
2223
}
2324

2425
public function getThrowTypeFromMethodCall(MethodReflection $methodReflection, MethodCall $methodCall, Scope $scope): ?Type

‎src/Type/Php/StrWordCountFunctionDynamicReturnTypeExtension.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use PHPStan\Type\Type;
1717
use PHPStan\Type\UnionType;
1818
use function count;
19+
use function in_array;
1920

2021
#[AutowiredService]
2122
final class StrWordCountFunctionDynamicReturnTypeExtension implements DynamicFunctionReturnTypeExtension
@@ -35,14 +36,14 @@ public function getTypeFromFunctionCall(
3536
$argsCount = count($functionCall->getArgs());
3637
if ($argsCount === 1) {
3738
return new IntegerType();
38-
} elseif ($argsCount === 2 || $argsCount === 3) {
39+
} elseif (in_array($argsCount, [2, 3], true)) {
3940
$formatType = $scope->getType($functionCall->getArgs()[1]->value);
4041
if ($formatType instanceof ConstantIntegerType) {
4142
$val = $formatType->getValue();
4243
if ($val === 0) {
4344
// return word count
4445
return new IntegerType();
45-
} elseif ($val === 1 || $val === 2) {
46+
} elseif (in_array($val, [1, 2], true)) {
4647
// return [word] or [offset => word]
4748
return new ArrayType(new IntegerType(), new StringType());
4849
}

‎src/Type/Regex/RegexGroupParser.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -711,7 +711,7 @@ private function getLiteralValue(TreeNode $node, ?array &$onlyLiterals, bool $ap
711711
}
712712
}
713713

714-
if ($token === 'anchor' || $token === 'match_point_reset') {
714+
if (in_array($token, ['anchor', 'match_point_reset'], true)) {
715715
return '';
716716
}
717717

‎tests/PHPStan/Build/OrChainIdenticalComparisonToInArrayRuleTest.php

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

0 commit comments

Comments
 (0)