Skip to content

Detect more possibly-impossible offset #4164

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: 2.1.x
Choose a base branch
from
Open

Detect more possibly-impossible offset #4164

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
dae7c97
Add non regression test
VincentLanglet Jul 26, 2025
71f3e51
Rework
VincentLanglet Jul 26, 2025
2165958
Fix
VincentLanglet Jul 26, 2025
0a8a1e2
Fix tests
VincentLanglet Aug 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions src/Rules/Arrays/NonexistentOffsetInArrayDimFetchCheck.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
use PHPStan\Type\ErrorType;
use PHPStan\Type\Type;
use PHPStan\Type\TypeUtils;
use PHPStan\Type\UnionType;
use PHPStan\Type\VerbosityLevel;
use function count;
use function sprintf;
Expand Down Expand Up @@ -76,6 +75,7 @@ public function check(
} else {
$flattenedTypes = TypeUtils::flattenTypes($type);
}

foreach ($flattenedTypes as $innerType) {
if (
$this->reportPossiblyNonexistentGeneralArrayOffset
Expand All @@ -94,15 +94,15 @@ public function check(
$report = true;
break;
}
if ($dimType instanceof UnionType) {
if ($innerType->hasOffsetValueType($dimType)->no()) {
$report = true;
break;
}
continue;
if ($dimType instanceof BenevolentUnionType) {
$flattenedInnerTypes = [$dimType];
} else {
$flattenedInnerTypes = TypeUtils::flattenTypes($dimType);
}
foreach (TypeUtils::flattenTypes($dimType) as $innerDimType) {
if ($innerType->hasOffsetValueType($innerDimType)->no()) {
foreach ($flattenedInnerTypes as $innerDimType) {
if (
$innerType->hasOffsetValueType($innerDimType)->no()
) {
$report = true;
break 2;
}
Expand Down
15 changes: 15 additions & 0 deletions tests/PHPStan/Levels/data/stringOffsetAccess-7.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
[
{
"message": "Offset int|object might not exist on 'foo'.",
"line": 19,
"ignorable": true
},
{
"message": "Offset 'foo' might not exist on array|string.",
"line": 27,
Expand All @@ -9,11 +14,21 @@
"line": 31,
"ignorable": true
},
{
"message": "Offset int|object might not exist on array|string.",
"line": 35,
"ignorable": true
},
{
"message": "Possibly invalid array key type int|object.",
"line": 35,
"ignorable": true
},
{
"message": "Offset int|object might not exist on array{baz: 21}|array{foo: 17, bar: 19}.",
"line": 55,
"ignorable": true
},
{
"message": "Possibly invalid array key type int|object.",
"line": 55,
Expand Down
26 changes: 26 additions & 0 deletions tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,10 @@ public function testRule(): void
'Cannot access offset \'a\' on array{a: 1, b: 1}|(Closure(): void).',
258,
],
[
'Offset int|null might not exist on array<int, string>.',
309,
],
[
'Offset null does not exist on array<int, string>.',
310,
Expand All @@ -127,6 +131,10 @@ public function testRule(): void
'Offset int does not exist on array<string, string>.',
312,
],
[
'Offset int|null might not exist on array<string, string>.',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well this is totally wrong, null gets cast to '' in an array key and that can exist on array<string, string>.. Not sure where to do the fix.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You misunderstood the error message.

You can see that phpstan already report on line 312 (the error above)

'Offset int does not exist on array<string, string>.',

So here we're reporting the same message with "might not exist" certainty because

  • int does not exists
  • null can exist

314,
],
[
'Offset \'baz\' might not exist on array{bar: 1, baz?: 2}.',
344,
Expand Down Expand Up @@ -185,6 +193,10 @@ public function testStrings(): void
'Offset 12.34 does not exist on \'foo\'.',
13,
],
[
'Offset int|object might not exist on \'foo\'.',
16,
],
[
'Offset \'foo\' might not exist on array|string.',
24,
Expand All @@ -193,6 +205,10 @@ public function testStrings(): void
'Offset 12.34 might not exist on array|string.',
28,
],
[
'Offset int|object might not exist on array|string.',
32,
],
]);
}

Expand Down Expand Up @@ -926,4 +942,14 @@ public function testBug3747(): void
$this->analyse([__DIR__ . '/data/bug-3747.php'], []);
}

public function testBug1061(): void
{
$this->analyse([__DIR__ . '/data/bug-1061.php'], [
[
"Offset 'one'|'two' might not exist on array{two: 1, three: 2}.",
14,
],
]);
}

}
15 changes: 15 additions & 0 deletions tests/PHPStan/Rules/Arrays/data/bug-1061.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<?php declare(strict_types = 1);

namespace Bug1061;

class A {
const KEYS = ["one", "two"];
const ARR = [
"two" => 1,
"three" => 2
];
}

foreach (A::KEYS as $key) {
echo A::ARR[$key];
}
Loading