Skip to content

Commit 6c18d79

Browse files
robotomarvinclaude
andcommitted
Keep optional keys of unsealed shapes in falsey isset() narrowing
Removing HasOffsetType('k') from an unsealed shape like array{k?: string, ...} drops the explicit key, which re-admits it through the unsealed extras with their (wider) value type - the "narrowed" type array{...} is in fact wider. Merged back with the truthy branch it collapsed the subject to plain array, e.g. after `$a['k'] ?? ''`. Skip the optional-key removal when any union member is not sealed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent cb3b7e9 commit 6c18d79

7 files changed

Lines changed: 193 additions & 4 deletions

File tree

‎src/Analyser/ExprHandler/Helper/DefaultNarrowingHelper.php‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -729,6 +729,7 @@ public function createIssetSingleSubjectNonTrueTypes(
729729
$typesToRemove = [];
730730
$hasOptionalNonNullableOffset = false;
731731
$hasPossiblyNullOffsetValue = false;
732+
$hasUnsealedOptionalOffset = false;
732733
foreach ($constantArrays as $constantArray) {
733734
$hasOffset = $constantArray->hasOffsetValueType($dimType);
734735
if ($hasOffset->no()) {
@@ -744,13 +745,21 @@ public function createIssetSingleSubjectNonTrueTypes(
744745
continue;
745746
}
746747

748+
// Removing the key from an unsealed shape re-admits it through
749+
// the unsealed extras, widening its value type instead.
750+
if ($constantArray->isUnsealed()->yes()) {
751+
$hasUnsealedOptionalOffset = true;
752+
continue;
753+
}
754+
747755
$hasOptionalNonNullableOffset = true;
748756
}
749757

750758
// !isset() on an optional key with a non-nullable value means the
751759
// key is absent - but only when no member can hold null at that
752-
// offset (the removal distributes over every union member).
753-
if ($hasOptionalNonNullableOffset && !$hasPossiblyNullOffsetValue) {
760+
// offset and no member is unsealed (the removal distributes over
761+
// every union member).
762+
if ($hasOptionalNonNullableOffset && !$hasPossiblyNullOffsetValue && !$hasUnsealedOptionalOffset) {
754763
$typesToRemove[] = new HasOffsetType($dimType);
755764
}
756765

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Analyser;
4+
5+
use PHPStan\Testing\TypeInferenceTestCase;
6+
use PHPUnit\Framework\Attributes\DataProvider;
7+
8+
/**
9+
* Runs without bleedingEdge.neon, where a sealed PHPDoc shape carries no
10+
* sealedness information and reports isUnsealed() as maybe.
11+
*/
12+
class Bug15428Test extends TypeInferenceTestCase
13+
{
14+
15+
public static function dataFileAsserts(): iterable
16+
{
17+
yield from self::gatherAssertTypes(__DIR__ . '/data/bug-15428.php');
18+
}
19+
20+
/**
21+
* @param mixed ...$args
22+
*/
23+
#[DataProvider('dataFileAsserts')]
24+
public function testFileAsserts(
25+
string $assertType,
26+
string $file,
27+
...$args,
28+
): void
29+
{
30+
$this->assertFileAsserts($assertType, $file, ...$args);
31+
}
32+
33+
}
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug15428Data;
4+
5+
use function PHPStan\Testing\assertType;
6+
7+
/**
8+
* @param array{a?: string, ...} $c
9+
*/
10+
function unsealed(array $c): void
11+
{
12+
$x = $c['a'] ?? '';
13+
assertType('array{a?: string, ...}', $c);
14+
}
15+
16+
/**
17+
* @param array{a?: string} $c
18+
*/
19+
function sealed(array $c): void
20+
{
21+
if (!isset($c['a'])) {
22+
assertType('array{}', $c);
23+
}
24+
}
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug15428;
4+
5+
use function PHPStan\Testing\assertType;
6+
7+
/**
8+
* @param array{a?: string, ...} $c
9+
*/
10+
function repro(array &$c): void
11+
{
12+
$x = $c['a'] ?? ''; // this causes $c to lose it's type
13+
assertType('array{a?: string, ...}', $c);
14+
$c['a'] = '1';
15+
}
16+
17+
/**
18+
* @param array{a?: string, ...} $c
19+
*/
20+
function ifIsset(array $c): void
21+
{
22+
if (!isset($c['a'])) {
23+
assertType('array{a?: string, ...}', $c);
24+
}
25+
assertType('array{a?: string, ...}', $c);
26+
}
27+
28+
/**
29+
* @param array{a?: string, ...<string, int>} $c
30+
*/
31+
function typedUnsealed(array $c): void
32+
{
33+
if (!isset($c['a'])) {
34+
assertType('array{a?: string, ...<string, int>}', $c);
35+
}
36+
assertType('array{a?: string, ...<string, int>}', $c);
37+
}
38+
39+
/**
40+
* @param array{a?: string} $c
41+
*/
42+
function sealed(array $c): void
43+
{
44+
if (!isset($c['a'])) {
45+
assertType('array{}', $c);
46+
}
47+
assertType('array{}|array{a: string}', $c);
48+
}
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\Variables;
4+
5+
use PHPStan\Rules\Comparison\ConstantConditionInTraitHelper;
6+
use PHPStan\Rules\IssetCheck;
7+
use PHPStan\Rules\Properties\PropertyDescriptor;
8+
use PHPStan\Rules\Rule;
9+
use PHPStan\Testing\RuleTestCase;
10+
11+
/**
12+
* Runs without bleedingEdge.neon, where a sealed PHPDoc shape carries no
13+
* sealedness information and reports isUnsealed() as maybe.
14+
*
15+
* @extends RuleTestCase<IssetRule>
16+
*/
17+
class IssetRuleWithoutBleedingEdgeTest extends RuleTestCase
18+
{
19+
20+
protected function getRule(): Rule
21+
{
22+
return new IssetRule(
23+
new IssetCheck(
24+
new PropertyDescriptor(),
25+
true,
26+
true,
27+
),
28+
self::getContainer()->getByType(ConstantConditionInTraitHelper::class),
29+
);
30+
}
31+
32+
public function testBug15428(): void
33+
{
34+
$this->analyse([__DIR__ . '/data/bug-15428.php'], [
35+
[
36+
'Offset \'a\' on array{} in isset() does not exist.',
37+
15,
38+
],
39+
]);
40+
}
41+
42+
public static function getAdditionalConfigFiles(): array
43+
{
44+
return [];
45+
}
46+
47+
}
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug15428Rule;
4+
5+
/**
6+
* @param array{a?: string, ...} $c
7+
* @param array{a?: string} $d
8+
*/
9+
function repro(array $c, array $d): void
10+
{
11+
if (!isset($c['a'])) {
12+
isset($c['a']);
13+
}
14+
if (!isset($d['a'])) {
15+
isset($d['a']);
16+
}
17+
}

‎turbo-ext/src/DefaultNarrowingHelper.cpp‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1586,6 +1586,7 @@ class DefaultNarrowingHelper
15861586
zv::Arr typesToRemove = zv::Arr::empty();
15871587
bool hasOptionalNonNullableOffset = false;
15881588
bool hasPossiblyNullOffsetValue = false;
1589+
bool hasUnsealedOptionalOffset = false;
15891590
if (Z_TYPE_P(constantArrays.raw()) == IS_ARRAY) {
15901591
for (zv::ArrayEntry entry : zv::ArrRef(constantArrays.raw())) {
15911592
zval *constantArray = entry.value().deref().raw();
@@ -1607,14 +1608,24 @@ class DefaultNarrowingHelper
16071608
continue;
16081609
}
16091610

1611+
// Removing the key from an unsealed shape re-admits it through
1612+
// the unsealed extras, widening its value type instead.
1613+
zend_long isUnsealed = pt_type_op_trinary(Z_OBJ_P(constantArray), PT_OP_IS_UNSEALED, 0, NULL);
1614+
if (UNEXPECTED(isUnsealed < 0)) return zv::Val();
1615+
if (isUnsealed == PT_TRI_YES) {
1616+
hasUnsealedOptionalOffset = true;
1617+
continue;
1618+
}
1619+
16101620
hasOptionalNonNullableOffset = true;
16111621
}
16121622
}
16131623

16141624
// !isset() on an optional key with a non-nullable value means the
16151625
// key is absent - but only when no member can hold null at that
1616-
// offset (the removal distributes over every union member).
1617-
if (hasOptionalNonNullableOffset && !hasPossiblyNullOffsetValue) {
1626+
// offset and no member is unsealed (the removal distributes over
1627+
// every union member).
1628+
if (hasOptionalNonNullableOffset && !hasPossiblyNullOffsetValue && !hasUnsealedOptionalOffset) {
16181629
zval hasOffsetZv;
16191630
if (UNEXPECTED(!pt_has_offset_type_new(&hasOffsetZv, dimType.raw()))) return zv::Val();
16201631
typesToRemove.push(zv::Val::adopt(hasOffsetZv));

0 commit comments

Comments
 (0)