Skip to content

Commit bc02099

Browse files
janedbaljanedbal
committed
ForbidUnsafeArrayKeyRule
1 parent 7088eff commit bc02099

File tree

8 files changed

+256
-2
lines changed

8 files changed

+256
-2
lines changed

‎README.md

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,9 @@ parameters:
8686
forbidReturnValueInYieldingMethod:
8787
enabled: true
8888
reportRegardlessOfReturnType: true
89+
forbidUnsafeArrayKey:
90+
enabled: true
91+
reportMixed: true
8992
forbidVariableTypeOverwriting:
9093
enabled: true
9194
forbidUnsetClassField:
@@ -610,6 +613,29 @@ parameters:
610613
reportRegardlessOfReturnType: true # optional stricter mode, defaults to false
611614
```
612615

616+
617+
### forbidUnsafeArrayKey
618+
- Denies non-int non-string array keys
619+
- PHP casts `null`, `float` and `bool` to some nearest int/string
620+
- You should rather do that intentionally and explicitly
621+
- Those types are the main difference to default PHPStan behaviour which allows using them as array keys
622+
- You can exclude reporting `mixed` keys via configuration
623+
624+
```php
625+
$taxRates = [ // denied, float key gets casted to int (causing $taxRates to contain one item)
626+
1.15 => 'reduced',
627+
1.21 => 'standard',
628+
];
629+
```
630+
631+
```neon
632+
parameters:
633+
shipmonkRules:
634+
forbidUnsafeArrayKey:
635+
reportMixed: false # defaults to true
636+
```
637+
638+
613639
### forbidVariableTypeOverwriting
614640
- Restricts variable assignment to those that does not change its type
615641
- Array append `$array[] = 1;` not yet supported

‎rules.neon

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,9 @@ parameters:
6565
forbidReturnValueInYieldingMethod:
6666
enabled: true
6767
reportRegardlessOfReturnType: true
68+
forbidUnsafeArrayKey:
69+
enabled: true
70+
reportMixed: true
6871
forbidVariableTypeOverwriting:
6972
enabled: true
7073
forbidUnsetClassField:
@@ -177,6 +180,10 @@ parametersSchema:
177180
enabled: bool()
178181
reportRegardlessOfReturnType: bool()
179182
])
183+
forbidUnsafeArrayKey: structure([
184+
enabled: bool()
185+
reportMixed: bool()
186+
])
180187
forbidVariableTypeOverwriting: structure([
181188
enabled: bool()
182189
])
@@ -259,6 +266,8 @@ conditionalTags:
259266
phpstan.rules.rule: %shipmonkRules.forbidProtectedEnumMethod.enabled%
260267
ShipMonk\PHPStan\Rule\ForbidReturnValueInYieldingMethodRule:
261268
phpstan.rules.rule: %shipmonkRules.forbidReturnValueInYieldingMethod.enabled%
269+
ShipMonk\PHPStan\Rule\ForbidUnsafeArrayKeyRule:
270+
phpstan.rules.rule: %shipmonkRules.forbidUnsafeArrayKey.enabled%
262271
ShipMonk\PHPStan\Rule\ForbidVariableTypeOverwritingRule:
263272
phpstan.rules.rule: %shipmonkRules.forbidVariableTypeOverwriting.enabled%
264273
ShipMonk\PHPStan\Rule\ForbidUnsetClassFieldRule:
@@ -369,6 +378,10 @@ services:
369378
class: ShipMonk\PHPStan\Rule\ForbidReturnValueInYieldingMethodRule
370379
arguments:
371380
reportRegardlessOfReturnType: %shipmonkRules.forbidReturnValueInYieldingMethod.reportRegardlessOfReturnType%
381+
-
382+
class: ShipMonk\PHPStan\Rule\ForbidUnsafeArrayKeyRule
383+
arguments:
384+
reportMixed: %shipmonkRules.forbidUnsafeArrayKey.reportMixed%
372385
-
373386
class: ShipMonk\PHPStan\Rule\ForbidVariableTypeOverwritingRule
374387
-

‎src/Rule/ForbidCheckedExceptionInCallableRule.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,7 @@ private function whitelistAllowedCallables(CallLike $node, Scope $scope): void
469469
$parameters = $parametersAcceptor->getParameters();
470470

471471
foreach ($args as $index => $arg) {
472-
$parameterIndex = $this->getParameterIndex($arg, $index, $parameters);
472+
$parameterIndex = $this->getParameterIndex($arg, $index, $parameters) ?? -1;
473473
$parameter = $parameters[$parameterIndex] ?? null;
474474
$argHash = spl_object_hash($arg->value);
475475

‎src/Rule/ForbidUnsafeArrayKeyRule.php

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace ShipMonk\PHPStan\Rule;
4+
5+
use PhpParser\Node;
6+
use PhpParser\Node\Expr\ArrayDimFetch;
7+
use PhpParser\Node\Expr\ArrayItem;
8+
use PHPStan\Analyser\Scope;
9+
use PHPStan\Rules\IdentifierRuleError;
10+
use PHPStan\Rules\Rule;
11+
use PHPStan\Rules\RuleErrorBuilder;
12+
use PHPStan\Type\IntegerType;
13+
use PHPStan\Type\MixedType;
14+
use PHPStan\Type\StringType;
15+
use PHPStan\Type\Type;
16+
use PHPStan\Type\TypeCombinator;
17+
use PHPStan\Type\VerbosityLevel;
18+
19+
/**
20+
* @implements Rule<Node>
21+
*/
22+
class ForbidUnsafeArrayKeyRule implements Rule
23+
{
24+
25+
private bool $reportMixed;
26+
27+
public function __construct(bool $reportMixed)
28+
{
29+
$this->reportMixed = $reportMixed;
30+
}
31+
32+
public function getNodeType(): string
33+
{
34+
return Node::class;
35+
}
36+
37+
/**
38+
* @return list<IdentifierRuleError>
39+
*/
40+
public function processNode(
41+
Node $node,
42+
Scope $scope
43+
): array
44+
{
45+
if ($node instanceof ArrayItem && $node->key !== null) {
46+
$keyType = $scope->getType($node->key);
47+
48+
if (!$this->isArrayKey($keyType)) {
49+
return [
50+
RuleErrorBuilder::message('Array key must be integer or string, but ' . $keyType->describe(VerbosityLevel::precise()) . ' given.')
51+
->identifier('shipmonk.unsafeArrayKey')
52+
->build(),
53+
];
54+
}
55+
}
56+
57+
if (
58+
$node instanceof ArrayDimFetch
59+
&& $node->dim !== null
60+
&& !$scope->getType($node->var)->isArray()->no()
61+
) {
62+
$dimType = $scope->getType($node->dim);
63+
64+
if (!$this->isArrayKey($dimType)) {
65+
return [
66+
RuleErrorBuilder::message('Array key must be integer or string, but ' . $dimType->describe(VerbosityLevel::precise()) . ' given.')
67+
->identifier('shipmonk.unsafeArrayKey')
68+
->build(),
69+
];
70+
}
71+
}
72+
73+
return [];
74+
}
75+
76+
private function isArrayKey(Type $type): bool
77+
{
78+
if (!$this->reportMixed && $type instanceof MixedType) {
79+
return true;
80+
}
81+
82+
return $this->containsOnlyTypes($type, [new StringType(), new IntegerType()]);
83+
}
84+
85+
/**
86+
* @param list<Type> $allowedTypes
87+
*/
88+
private function containsOnlyTypes(
89+
Type $checkedType,
90+
array $allowedTypes
91+
): bool
92+
{
93+
$allowedType = TypeCombinator::union(...$allowedTypes);
94+
return $allowedType->isSuperTypeOf($checkedType)->yes();
95+
}
96+
97+
}

‎tests/Rule/ForbidUnsafeArrayKeyRuleTest.php

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Rule;
4+
5+
use LogicException;
6+
use PHPStan\Rules\Rule;
7+
use ShipMonk\PHPStan\Rule\ForbidUnsafeArrayKeyRule;
8+
use ShipMonk\PHPStan\RuleTestCase;
9+
10+
/**
11+
* @extends RuleTestCase<ForbidUnsafeArrayKeyRule>
12+
*/
13+
class ForbidUnsafeArrayKeyRuleTest extends RuleTestCase
14+
{
15+
16+
private ?bool $checkMixed = null;
17+
18+
protected function getRule(): Rule
19+
{
20+
if ($this->checkMixed === null) {
21+
throw new LogicException('Property checkMixed must be set');
22+
}
23+
24+
return new ForbidUnsafeArrayKeyRule(
25+
$this->checkMixed,
26+
);
27+
}
28+
29+
public function testMixedDisabled(): void
30+
{
31+
$this->checkMixed = false;
32+
$this->analyseFile(__DIR__ . '/data/ForbidUnsafeArrayKeyRule/mixed-disabled.php');
33+
}
34+
35+
public function testMixedEnabled(): void
36+
{
37+
$this->checkMixed = true;
38+
$this->analyseFile(__DIR__ . '/data/ForbidUnsafeArrayKeyRule/mixed-enabled.php');
39+
}
40+
41+
}

‎tests/Rule/data/ForbidUnsafeArrayKeyRule/mixed-disabled.php

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace ForbidUnsafeArrayKeyRule\MixedDisabled;
4+
5+
6+
class ArrayKey
7+
{
8+
9+
/**
10+
* @param array-key $arrayKey
11+
*/
12+
public function test(
13+
int $int,
14+
string $string,
15+
float $float,
16+
$arrayKey,
17+
int|string $intOrString,
18+
int|float $intOrFloat,
19+
array $array,
20+
object $object,
21+
\WeakMap $weakMap,
22+
mixed $explicitMixed,
23+
$implicitMixed
24+
) {
25+
$array[$array] = ''; // error: Array key must be integer or string, but array given.
26+
$array[$int] = '';
27+
$array[$string] = '';
28+
$array[$float] = ''; // error: Array key must be integer or string, but float given.
29+
$array[$intOrFloat] = ''; // error: Array key must be integer or string, but float|int given.
30+
$array[$intOrString] = '';
31+
$array[$explicitMixed] = '';
32+
$array[$implicitMixed] = '';
33+
$array[$arrayKey] = '';
34+
$weakMap[$object] = '';
35+
}
36+
}
37+
38+

‎tests/Rule/data/ForbidUnsafeArrayKeyRule/mixed-enabled.php

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace ForbidUnsafeArrayKeyRule\MixedEnabled;
4+
5+
6+
class ArrayKey
7+
{
8+
9+
/**
10+
* @param array-key $arrayKey
11+
*/
12+
public function test(
13+
int $int,
14+
string $string,
15+
float $float,
16+
$arrayKey,
17+
int|string $intOrString,
18+
int|float $intOrFloat,
19+
array $array,
20+
object $object,
21+
\WeakMap $weakMap,
22+
mixed $explicitMixed,
23+
$implicitMixed
24+
) {
25+
$array[$array] = ''; // error: Array key must be integer or string, but array given.
26+
$array[$int] = '';
27+
$array[$string] = '';
28+
$array[$float] = ''; // error: Array key must be integer or string, but float given.
29+
$array[$intOrFloat] = ''; // error: Array key must be integer or string, but float|int given.
30+
$array[$intOrString] = '';
31+
$array[$explicitMixed] = ''; // error: Array key must be integer or string, but mixed given.
32+
$array[$implicitMixed] = ''; // error: Array key must be integer or string, but mixed given.
33+
$array[$arrayKey] = '';
34+
$weakMap[$object] = '';
35+
}
36+
}
37+
38+

‎tests/RuleTestCase.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,8 @@ private function autofix(string $file, array $analyserErrors): void
111111
$errorsByLines = [];
112112

113113
foreach ($analyserErrors as $analyserError) {
114-
$errorsByLines[$analyserError->getLine()] = $analyserError;
114+
$line = $analyserError->getLine() ?? -1;
115+
$errorsByLines[$line] = $analyserError;
115116
}
116117

117118
$fileLines = $this->getFileLines($file);

0 commit comments

Comments
 (0)