Skip to content

Commit 9b6cc48

Browse files
committed
Fix array_key_first/array_key_last offset check for non-variable expressions
- The special-case logic in NonexistentOffsetInArrayDimFetchRule that suppresses false positives for $arr[array_key_first($arr)] only matched Variable AST nodes - Changed expression comparison to use ExprPrinter so it works with property fetches, static property fetches, and any other expression type - Also fixed the same limitation in the array_rand and count()-1 special cases - New regression test in tests/PHPStan/Rules/Arrays/data/bug-14390.php
1 parent 72a52e0 commit 9b6cc48

3 files changed

Lines changed: 71 additions & 12 deletions

File tree

src/Rules/Arrays/NonexistentOffsetInArrayDimFetchRule.php

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
use PHPStan\DependencyInjection\AutowiredParameter;
1111
use PHPStan\DependencyInjection\RegisteredRule;
1212
use PHPStan\Internal\SprintfHelper;
13+
use PHPStan\Node\Printer\ExprPrinter;
1314
use PHPStan\Rules\Rule;
1415
use PHPStan\Rules\RuleErrorBuilder;
1516
use PHPStan\Rules\RuleLevelHelper;
@@ -33,6 +34,7 @@ final class NonexistentOffsetInArrayDimFetchRule implements Rule
3334
public function __construct(
3435
private RuleLevelHelper $ruleLevelHelper,
3536
private NonexistentOffsetInArrayDimFetchCheck $nonexistentOffsetInArrayDimFetchCheck,
37+
private ExprPrinter $exprPrinter,
3638
#[AutowiredParameter]
3739
private bool $reportMaybes,
3840
)
@@ -120,10 +122,7 @@ public function processNode(Node $node, Scope $scope): array
120122
$arrayArg = $node->dim->getArgs()[0]->value;
121123
$arrayType = $scope->getType($arrayArg);
122124
if (
123-
$arrayArg instanceof Node\Expr\Variable
124-
&& $node->var instanceof Node\Expr\Variable
125-
&& is_string($arrayArg->name)
126-
&& $arrayArg->name === $node->var->name
125+
$this->exprPrinter->printExpr($arrayArg) === $this->exprPrinter->printExpr($node->var)
127126
&& $arrayType->isArray()->yes()
128127
&& $arrayType->isIterableAtLeastOnce()->yes()
129128
) {
@@ -146,10 +145,7 @@ public function processNode(Node $node, Scope $scope): array
146145
$arrayType = $scope->getType($arrayArg);
147146

148147
if (
149-
$arrayArg instanceof Node\Expr\Variable
150-
&& $node->var instanceof Node\Expr\Variable
151-
&& is_string($arrayArg->name)
152-
&& $arrayArg->name === $node->var->name
148+
$this->exprPrinter->printExpr($arrayArg) === $this->exprPrinter->printExpr($node->var)
153149
&& $arrayType->isArray()->yes()
154150
&& $arrayType->isIterableAtLeastOnce()->yes()
155151
&& ($numArg === null || $one->isSuperTypeOf($scope->getType($numArg))->yes())
@@ -170,10 +166,7 @@ public function processNode(Node $node, Scope $scope): array
170166
$arrayArg = $node->dim->left->getArgs()[0]->value;
171167
$arrayType = $scope->getType($arrayArg);
172168
if (
173-
$arrayArg instanceof Node\Expr\Variable
174-
&& $node->var instanceof Node\Expr\Variable
175-
&& is_string($arrayArg->name)
176-
&& $arrayArg->name === $node->var->name
169+
$this->exprPrinter->printExpr($arrayArg) === $this->exprPrinter->printExpr($node->var)
177170
&& $arrayType->isList()->yes()
178171
&& $arrayType->isIterableAtLeastOnce()->yes()
179172
) {

tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
namespace PHPStan\Rules\Arrays;
44

5+
use PHPStan\Node\Printer\ExprPrinter;
6+
use PHPStan\Node\Printer\Printer;
57
use PHPStan\Rules\Rule;
68
use PHPStan\Rules\RuleLevelHelper;
79
use PHPStan\Testing\RuleTestCase;
@@ -44,6 +46,7 @@ protected function getRule(): Rule
4446
reportPossiblyNonexistentGeneralArrayOffset: $this->reportPossiblyNonexistentGeneralArrayOffset,
4547
reportPossiblyNonexistentConstantArrayOffset: $this->reportPossiblyNonexistentConstantArrayOffset,
4648
),
49+
new ExprPrinter(new Printer()),
4750
true,
4851
);
4952
}
@@ -1277,4 +1280,12 @@ public function testBug14308(): void
12771280
$this->analyse([__DIR__ . '/data/bug-14308.php'], []);
12781281
}
12791282

1283+
#[RequiresPhp('>= 8.2')]
1284+
public function testBug14390(): void
1285+
{
1286+
$this->reportPossiblyNonexistentGeneralArrayOffset = true;
1287+
1288+
$this->analyse([__DIR__ . '/data/bug-14390.php'], []);
1289+
}
1290+
12801291
}
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
<?php // lint >= 8.2
2+
3+
declare(strict_types = 1);
4+
5+
namespace Bug14390;
6+
7+
readonly class Sample
8+
{
9+
/**
10+
* @param array<string, string> $fields
11+
*/
12+
public function __construct(
13+
public array $fields = [],
14+
) {
15+
}
16+
}
17+
18+
class Foo
19+
{
20+
public function bar(
21+
Sample $sample,
22+
): void {
23+
if ($sample->fields !== []) {
24+
echo $sample->fields[array_key_first($sample->fields)];
25+
}
26+
}
27+
28+
/**
29+
* @param array<string, string> $fields
30+
*/
31+
public function zoo(
32+
array $fields,
33+
): void {
34+
if ($fields !== []) {
35+
echo $fields[array_key_first($fields)];
36+
}
37+
}
38+
39+
public function withKey(
40+
Sample $sample,
41+
): void {
42+
if ($sample->fields !== []) {
43+
$key = array_key_first($sample->fields);
44+
echo $sample->fields[$key];
45+
}
46+
}
47+
48+
public function arrayKeyLast(
49+
Sample $sample,
50+
): void {
51+
if ($sample->fields !== []) {
52+
echo $sample->fields[array_key_last($sample->fields)];
53+
}
54+
}
55+
}

0 commit comments

Comments
 (0)