Skip to content

Fix phpstan/phpstan#14308: Offset 0 should not be optional on non empty list #5236

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

Merged
VincentLanglet merged 10 commits into from
Mar 17, 2026
Merged

Fix phpstan/phpstan#14308: Offset 0 should not be optional on non empty list #5236

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
731b503
Fix false positive "Offset 0 might not exist" on non-empty-list
phpstan-bot Mar 17, 2026
7a66ada
Extract hasOffsetValueType result to variable to avoid duplicate comp…
phpstan-bot Mar 17, 2026
fdb4d05
Improve
VincentLanglet Mar 17, 2026
8dfe8e6
Update tests
VincentLanglet Mar 17, 2026
34cda84
Fix tests
VincentLanglet Mar 17, 2026
cca316e
Add test
VincentLanglet Mar 17, 2026
81f9466
Improve
VincentLanglet Mar 17, 2026
99fcf00
Address review: reorder instanceof checks before method calls in Type…
phpstan-bot Mar 17, 2026
b5efc6a
Add TypeCombinator test for list with all optional keys intersected w…
phpstan-bot Mar 17, 2026
a091de0
Add TypeCombinator test for list with all optional keys intersected w…
phpstan-bot Mar 17, 2026
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
8 changes: 7 additions & 1 deletion src/Rules/Arrays/NonexistentOffsetInArrayDimFetchCheck.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,13 @@ public function check(
return [];
}

if ($type->hasOffsetValueType($dimType)->no()) {
$hasOffsetValueType = $type->hasOffsetValueType($dimType);

if ($hasOffsetValueType->yes()) {
return [];
}

if ($hasOffsetValueType->no()) {
if ($type->isArray()->yes()) {
$validArrayDimType = TypeCombinator::intersect(AllowedArrayKeysTypes::getType(), $dimType);
if ($validArrayDimType instanceof NeverType) {
Expand Down
8 changes: 4 additions & 4 deletions src/Type/TypeCombinator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1349,9 +1349,9 @@

if (
$types[$i] instanceof ConstantArrayType
&& count($types[$i]->getKeyTypes()) === 1
&& $types[$i]->isOptionalKey(0)
&& $types[$j] instanceof NonEmptyArrayType
&& (count($types[$i]->getKeyTypes()) === 1 || $types[$i]->isList()->yes())
&& $types[$i]->isOptionalKey(0)
) {
$types[$i] = $types[$i]->makeOffsetRequired($types[$i]->getKeyTypes()[0]);
array_splice($types, $j--, 1);
Expand All @@ -1361,9 +1361,9 @@

if (
$types[$j] instanceof ConstantArrayType
&& count($types[$j]->getKeyTypes()) === 1
&& $types[$j]->isOptionalKey(0)
&& $types[$i] instanceof NonEmptyArrayType
&& (count($types[$j]->getKeyTypes()) === 1 || $types[$j]->isList()->yes())

Check warning on line 1365 in src/Type/TypeCombinator.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.4, ubuntu-latest) 

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator":

@@ @@
 					if (
 						$types[$j] instanceof ConstantArrayType
 						&& $types[$i] instanceof NonEmptyArrayType
-						&& (count($types[$j]->getKeyTypes()) === 1 || $types[$j]->isList()->yes())
+						&& (count($types[$j]->getKeyTypes()) === 1 || !$types[$j]->isList()->no())
 						&& $types[$j]->isOptionalKey(0)
 					) {
 						$types[$j] = $types[$j]->makeOffsetRequired($types[$j]->getKeyTypes()[0]);

Check warning on line 1365 in src/Type/TypeCombinator.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.3, ubuntu-latest) 

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator":

@@ @@
 					if (
 						$types[$j] instanceof ConstantArrayType
 						&& $types[$i] instanceof NonEmptyArrayType
-						&& (count($types[$j]->getKeyTypes()) === 1 || $types[$j]->isList()->yes())
+						&& (count($types[$j]->getKeyTypes()) === 1 || !$types[$j]->isList()->no())
 						&& $types[$j]->isOptionalKey(0)
 					) {
 						$types[$j] = $types[$j]->makeOffsetRequired($types[$j]->getKeyTypes()[0]);
&& $types[$j]->isOptionalKey(0)
) {
$types[$j] = $types[$j]->makeOffsetRequired($types[$j]->getKeyTypes()[0]);
array_splice($types, $i--, 1);
Expand Down
1 change: 1 addition & 0 deletions tests/PHPStan/Analyser/NodeScopeResolverTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,7 @@ private static function findTestFiles(): iterable
yield __DIR__ . '/../Rules/Properties/data/bug-14012.php';
yield __DIR__ . '/../Rules/Variables/data/bug-14124.php';
yield __DIR__ . '/../Rules/Variables/data/bug-14124b.php';
yield __DIR__ . '/../Rules/Arrays/data/bug-14308.php';
}

/**
Expand Down
2 changes: 1 addition & 1 deletion tests/PHPStan/Analyser/nsrt/array-shape-list-optional.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public function doFoo(
): void
{
assertType('list{0: string, 1: int, 2?: string, 3?: string}', $valid1);
assertType('non-empty-list{0?: string, 1?: int, 2?: string, 3?: string}', $valid2);
assertType('list{0: string, 1?: int, 2?: string, 3?: string}', $valid2);
assertType('non-empty-array{0?: string, 1?: int, 2?: string, 3?: string}', $valid3);
assertType('*NEVER*', $invalid1);
assertType('*NEVER*', $invalid2);
Expand Down
2 changes: 1 addition & 1 deletion tests/PHPStan/Analyser/nsrt/bug-14297.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ function (): void {
return;
}

assertType("non-empty-list{0?: 'a'|'b', 1?: 'b'}", $a);
assertType("array{0: 'a'|'b', 1?: 'b'}", $a);
assertType("int<1, 2>", count($a));

if (count($a) === 2) {
Expand Down
7 changes: 7 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 @@ -1256,4 +1256,11 @@ public function testBug13773(): void
]);
}

public function testBug14308(): void
{
$this->reportPossiblyNonexistentConstantArrayOffset = true;

$this->analyse([__DIR__ . '/data/bug-14308.php'], []);
}

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

namespace Bug14308;

use RuntimeException;
use function PHPStan\Testing\assertType;

function getUi(string $s1, string $s2, string $s3): string
{
$available = array_keys(array_filter([
'swagger' => $s1,
'redoc' => $s2,
'scalar' => $s3,
]));

if ([] === $available) {
throw new RuntimeException('No documentation UI is enabled.');
}

assertType("list{0: 'redoc'|'scalar'|'swagger', 1?: 'redoc'|'scalar', 2?: 'scalar'}", $available);

return $available[0];
}
64 changes: 64 additions & 0 deletions tests/PHPStan/Type/TypeCombinatorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3485,6 +3485,70 @@ public static function dataIntersect(): iterable
ConstantArrayType::class,
'array{string}',
],
[
[
new ConstantArrayType([
new ConstantIntegerType(0),
new ConstantIntegerType(1),
], [
new StringType(),
new StringType(),
], optionalKeys: [0, 1], isList: TrinaryLogic::createYes()),
new NonEmptyArrayType(),
],
ConstantArrayType::class,
'array{0: string, 1?: string}',
],
[
[
new ConstantArrayType([
new ConstantIntegerType(0),
new ConstantIntegerType(1),
new ConstantIntegerType(2),
new ConstantIntegerType(3),
], [
new StringType(),
new StringType(),
new StringType(),
new StringType(),
], nextAutoIndexes: [3], optionalKeys: [0, 1, 2, 3], isList: TrinaryLogic::createYes()),
new NonEmptyArrayType(),
],
ConstantArrayType::class,
'list{0: string, 1?: string, 2?: string, 3?: string}',
],
[
[
new ConstantArrayType([
new ConstantIntegerType(0),
new ConstantIntegerType(1),
], [
new StringType(),
new StringType(),
], optionalKeys: [0, 1]),
new NonEmptyArrayType(),
],
IntersectionType::class,
'non-empty-array{0?: string, 1?: string}',
],
[
[
new ConstantArrayType([
new ConstantIntegerType(0),
new ConstantIntegerType(1),
new ConstantIntegerType(2),
new ConstantIntegerType(3),
], [
new StringType(),
new StringType(),
new StringType(),
new StringType(),
], [3], [0, 1, 2, 3], TrinaryLogic::createYes()),
new NonEmptyArrayType(),
],
ConstantArrayType::class,
'list{0: string, 1?: string, 2?: string, 3?: string}',
],
[
[
new ConstantArrayType([], []),
Expand Down
26 changes: 18 additions & 8 deletions tests/PHPStan/Type/TypeToPhpDocNodeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,14 +404,6 @@ public static function dataToPhpDocNode(): iterable
'list{0?: string, 1?: string, 2?: string, 3?: string}',
];

yield [
new IntersectionType([
$listArrayWithAllOptionalKeys,
new NonEmptyArrayType(),
]),
'non-empty-list{0?: string, 1?: string, 2?: string, 3?: string}',
];
Comment on lines -407 to -413
Copy link
Contributor

@staabm staabm Mar 17, 2026

Choose a reason for hiding this comment

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

why the need to drop a test?

Copy link
Contributor

@VincentLanglet VincentLanglet Mar 17, 2026

Choose a reason for hiding this comment

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

This test was testing that the type is transformed into the description and the description is transformed into the type.

But it's not true anymore.

While

new IntersectionType([
				$listArrayWithAllOptionalKeys,
				new NonEmptyArrayType(),
			])

is transformed into 'list{0: string, 1?: string, 2?: string, 3?: string}', the opposite is not true.

This has to be a TypeCombinator test instead.

Copy link
Collaborator Author

@phpstan-bot phpstan-bot Mar 17, 2026

Choose a reason for hiding this comment

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

All review comments addressed and pushed. Here's a summary of the changes:

  1. TypeCombinator.php - Moved instanceof checks before method calls (isList(), isOptionalKey()) in both condition blocks for short-circuit performance. Also fixed a bug in the second block where $types[$i]->isList() should have been $types[$j]->isList() (since $types[$j] is the ConstantArrayType in that branch, not $types[$i]).

  2. TypeToPhpDocNodeTest.php - Moved the test to dataToPhpDocNodeWithoutCheckingEquals instead of deleting it. The test was removed because TypeCombinator::intersect() now simplifies the intersection into a plain ConstantArrayType, so the round-trip assertion ($type->equals($parsedType)) fails — parsing 'non-empty-list{0?: string, ...}' back produces list{0: string, ...}. The toPhpDocNode() output itself is still correct, so the test belongs in the "without checking equals" provider.

Copy link
Collaborator Author

@phpstan-bot phpstan-bot Mar 17, 2026

Choose a reason for hiding this comment

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

Already handled — all 11588 tests passed and the commit has been pushed.


$constantArrayWithAllOptionalKeys = new ConstantArrayType([
new ConstantIntegerType(0),
new ConstantIntegerType(1),
Expand Down Expand Up @@ -448,6 +440,24 @@ public function testToPhpDocNode(Type $type, string $expected): void

public static function dataToPhpDocNodeWithoutCheckingEquals(): iterable
{
yield [
new IntersectionType([
new ConstantArrayType([
new ConstantIntegerType(0),
new ConstantIntegerType(1),
new ConstantIntegerType(2),
new ConstantIntegerType(3),
], [
new StringType(),
new StringType(),
new StringType(),
new StringType(),
], [3], [0, 1, 2, 3], TrinaryLogic::createYes()),
new NonEmptyArrayType(),
]),
'non-empty-list{0?: string, 1?: string, 2?: string, 3?: string}',
];

yield [
new ConstantStringType("foo\nbar\nbaz"),
'(literal-string & lowercase-string & non-falsy-string)',
Expand Down
Loading