Skip to content

Commit 2154685

Browse files
staabmstaabm
authored
Fix false positives on non-existing array offsets
1 parent 82251a3 commit 2154685

5 files changed

+277
-2
lines changed

Diff for: ‎src/Analyser/TypeSpecifier.php

+76-2
Original file line numberDiff line numberDiff line change
@@ -664,11 +664,85 @@ public function specifyTypesInCondition(
664664
if (!$scope instanceof MutatingScope) {
665665
throw new ShouldNotHappenException();
666666
}
667+
667668
if ($context->null()) {
668-
return $this->specifyTypesInCondition($scope->exitFirstLevelStatements(), $expr->expr, $context)->setRootExpr($expr);
669+
$specifiedTypes = $this->specifyTypesInCondition($scope->exitFirstLevelStatements(), $expr->expr, $context)->setRootExpr($expr);
670+
671+
// infer $arr[$key] after $key = array_key_first/last($arr)
672+
if (
673+
$expr->expr instanceof FuncCall
674+
&& $expr->expr->name instanceof Name
675+
&& in_array($expr->expr->name->toLowerString(), ['array_key_first', 'array_key_last'], true)
676+
&& count($expr->expr->getArgs()) >= 1
677+
) {
678+
$arrayArg = $expr->expr->getArgs()[0]->value;
679+
$arrayType = $scope->getType($arrayArg);
680+
if (
681+
$arrayType->isArray()->yes()
682+
&& $arrayType->isIterableAtLeastOnce()->yes()
683+
) {
684+
$dimFetch = new ArrayDimFetch($arrayArg, $expr->var);
685+
$iterableValueType = $expr->expr->name->toLowerString() === 'array_key_first'
686+
? $arrayType->getFirstIterableValueType()
687+
: $arrayType->getLastIterableValueType();
688+
689+
return $specifiedTypes->unionWith(
690+
$this->create($dimFetch, $iterableValueType, TypeSpecifierContext::createTrue(), $scope),
691+
);
692+
}
693+
}
694+
695+
// infer $list[$count] after $count = count($list) - 1
696+
if (
697+
$expr->expr instanceof Expr\BinaryOp\Minus
698+
&& $expr->expr->left instanceof FuncCall
699+
&& $expr->expr->left->name instanceof Name
700+
&& in_array($expr->expr->left->name->toLowerString(), ['count', 'sizeof'], true)
701+
&& count($expr->expr->left->getArgs()) >= 1
702+
&& $expr->expr->right instanceof Node\Scalar\Int_
703+
&& $expr->expr->right->value === 1
704+
) {
705+
$arrayArg = $expr->expr->left->getArgs()[0]->value;
706+
$arrayType = $scope->getType($arrayArg);
707+
if (
708+
$arrayType->isList()->yes()
709+
&& $arrayType->isIterableAtLeastOnce()->yes()
710+
) {
711+
$dimFetch = new ArrayDimFetch($arrayArg, $expr->var);
712+
713+
return $specifiedTypes->unionWith(
714+
$this->create($dimFetch, $arrayType->getLastIterableValueType(), TypeSpecifierContext::createTrue(), $scope),
715+
);
716+
}
717+
}
718+
719+
return $specifiedTypes;
669720
}
670721

671-
return $this->specifyTypesInCondition($scope->exitFirstLevelStatements(), $expr->var, $context)->setRootExpr($expr);
722+
$specifiedTypes = $this->specifyTypesInCondition($scope->exitFirstLevelStatements(), $expr->var, $context)->setRootExpr($expr);
723+
724+
if ($context->true()) {
725+
// infer $arr[$key] after $key = array_search($needle, $arr)
726+
if (
727+
$expr->expr instanceof FuncCall
728+
&& $expr->expr->name instanceof Name
729+
&& $expr->expr->name->toLowerString() === 'array_search'
730+
&& count($expr->expr->getArgs()) >= 2
731+
) {
732+
$arrayArg = $expr->expr->getArgs()[1]->value;
733+
$arrayType = $scope->getType($arrayArg);
734+
735+
if ($arrayType->isArray()->yes()) {
736+
$dimFetch = new ArrayDimFetch($arrayArg, $expr->var);
737+
$iterableValueType = $arrayType->getIterableValueType();
738+
739+
return $specifiedTypes->unionWith(
740+
$this->create($dimFetch, $iterableValueType, TypeSpecifierContext::createTrue(), $scope),
741+
);
742+
}
743+
}
744+
}
745+
return $specifiedTypes;
672746
} elseif (
673747
$expr instanceof Expr\Isset_
674748
&& count($expr->vars) > 0

Diff for: ‎tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php

+44
Original file line numberDiff line numberDiff line change
@@ -785,4 +785,48 @@ public function testBug12122(): void
785785
$this->analyse([__DIR__ . '/data/bug-12122.php'], []);
786786
}
787787

788+
public function testArrayDimFetchAfterArrayKeyFirstOrLast(): void
789+
{
790+
$this->reportPossiblyNonexistentGeneralArrayOffset = true;
791+
792+
$this->analyse([__DIR__ . '/data/array-dim-after-array-key-first-or-last.php'], [
793+
[
794+
'Offset null does not exist on array{}.',
795+
19,
796+
],
797+
]);
798+
}
799+
800+
public function testArrayDimFetchAfterCount(): void
801+
{
802+
$this->reportPossiblyNonexistentGeneralArrayOffset = true;
803+
804+
$this->analyse([__DIR__ . '/data/array-dim-after-count.php'], [
805+
[
806+
'Offset int<0, max> might not exist on list<string>.',
807+
26,
808+
],
809+
[
810+
'Offset int<-1, max> might not exist on array<string>.',
811+
35,
812+
],
813+
[
814+
'Offset int<0, max> might not exist on non-empty-array<string>.',
815+
42,
816+
],
817+
]);
818+
}
819+
820+
public function testArrayDimFetchAfterArraySearch(): void
821+
{
822+
$this->reportPossiblyNonexistentGeneralArrayOffset = true;
823+
824+
$this->analyse([__DIR__ . '/data/array-dim-after-array-search.php'], [
825+
[
826+
'Offset int|string might not exist on array.',
827+
20,
828+
],
829+
]);
830+
}
831+
788832
}

Diff for: ‎tests/PHPStan/Rules/Arrays/data/array-dim-after-array-key-first-or-last.php

+75
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
<?php // lint >= 8.0
2+
3+
declare(strict_types = 1);
4+
5+
namespace ArrayDimAfterArrayKeyFirstOrLast;
6+
7+
class HelloWorld
8+
{
9+
/**
10+
* @param list<string> $hellos
11+
*/
12+
public function last(array $hellos): string
13+
{
14+
if ($hellos !== []) {
15+
$last = array_key_last($hellos);
16+
return $hellos[$last];
17+
} else {
18+
$last = array_key_last($hellos);
19+
return $hellos[$last];
20+
}
21+
}
22+
23+
/**
24+
* @param array<string> $hellos
25+
*/
26+
public function lastOnArray(array $hellos): string
27+
{
28+
if ($hellos !== []) {
29+
$last = array_key_last($hellos);
30+
return $hellos[$last];
31+
}
32+
33+
return 'nothing';
34+
}
35+
36+
/**
37+
* @param list<string> $hellos
38+
*/
39+
public function first(array $hellos): string
40+
{
41+
if ($hellos !== []) {
42+
$first = array_key_first($hellos);
43+
return $hellos[$first];
44+
}
45+
46+
return 'nothing';
47+
}
48+
49+
/**
50+
* @param array<string> $hellos
51+
*/
52+
public function firstOnArray(array $hellos): string
53+
{
54+
if ($hellos !== []) {
55+
$first = array_key_first($hellos);
56+
return $hellos[$first];
57+
}
58+
59+
return 'nothing';
60+
}
61+
62+
/**
63+
* @param array{first: int, middle: float, last: bool} $hellos
64+
*/
65+
public function shape(array $hellos): int|bool
66+
{
67+
$first = array_key_first($hellos);
68+
$last = array_key_last($hellos);
69+
70+
if (rand(0,1)) {
71+
return $hellos[$first];
72+
}
73+
return $hellos[$last];
74+
}
75+
}

Diff for: ‎tests/PHPStan/Rules/Arrays/data/array-dim-after-array-search.php

+37
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
<?php // lint >= 8.0
2+
3+
declare(strict_types = 1);
4+
5+
namespace ArrayDimAfterArraySeach;
6+
7+
class HelloWorld
8+
{
9+
public function doFoo(array $arr, string $needle): string
10+
{
11+
if (($key = array_search($needle, $arr, true)) !== false) {
12+
echo $arr[$key];
13+
}
14+
}
15+
16+
public function doBar(array $arr, string $needle): string
17+
{
18+
$key = array_search($needle, $arr, true);
19+
if ($key !== false) {
20+
echo $arr[$key];
21+
}
22+
}
23+
24+
public function doFooBar(array $arr, string $needle): string
25+
{
26+
if (($key = array_search($needle, $arr, false)) !== false) {
27+
echo $arr[$key];
28+
}
29+
}
30+
31+
public function doBaz(array $arr, string $needle): string
32+
{
33+
if (($key = array_search($needle, $arr)) !== false) {
34+
echo $arr[$key];
35+
}
36+
}
37+
}

Diff for: ‎tests/PHPStan/Rules/Arrays/data/array-dim-after-count.php

+45
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace ArrayDimFetchOnCount;
4+
5+
class HelloWorld
6+
{
7+
/**
8+
* @param list<string> $hellos
9+
*/
10+
public function works(array $hellos): string
11+
{
12+
if ($hellos === []) {
13+
return 'nothing';
14+
}
15+
16+
$count = count($hellos) - 1;
17+
return $hellos[$count];
18+
}
19+
20+
/**
21+
* @param list<string> $hellos
22+
*/
23+
public function offByOne(array $hellos): string
24+
{
25+
$count = count($hellos);
26+
return $hellos[$count];
27+
}
28+
29+
/**
30+
* @param array<string> $hellos
31+
*/
32+
public function maybeInvalid(array $hellos): string
33+
{
34+
$count = count($hellos) - 1;
35+
echo $hellos[$count];
36+
37+
if ($hellos === []) {
38+
return 'nothing';
39+
}
40+
41+
$count = count($hellos) - 1;
42+
return $hellos[$count];
43+
}
44+
45+
}

0 commit comments

Comments
 (0)