Skip to content

Commit 7604335

Browse files
ondrejmirtesclaude
andcommitted
Track $arr[$key] existence across array_key_first/last + !== null
When `$key = array_key_first/last($arr)` runs against an array that might be empty, `$arr[$key]` was only registered in scope if the array was already known non-empty at assignment time. Once the user narrowed `$key` separately with `if ($key !== null)`, AssignHandler's deep-write path couldn't see the dim-fetch and fell back to a lossy `setOffsetValueType`, generalising untouched keys to optional. Attach a conditional holder at assignment time so the dim-fetch lands in scope once `$key` is narrowed, matching what `isset($arr[$key])` already does. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 9009697 commit 7604335

3 files changed

Lines changed: 88 additions & 1 deletion

File tree

src/Analyser/TypeSpecifier.php

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -847,6 +847,31 @@ public function specifyTypesInCondition(
847847
$specifiedTypes = $specifiedTypes->unionWith(
848848
$this->create($dimFetch, $arrayType->getIterableValueType(), TypeSpecifierContext::createTrue(), $scope),
849849
);
850+
} elseif ($expr->var instanceof Expr\Variable && is_string($expr->var->name)) {
851+
// The array might be empty here, so we cannot register
852+
// $arr[$key] unconditionally. Attach a conditional holder
853+
// that fires once the user narrows $key to non-null
854+
// (e.g. `if ($key !== null)`), giving the deep-write
855+
// path the same shape information `isset($arr[$key])`
856+
// would have provided.
857+
$keyType = $scope->getType($expr->expr);
858+
$nonNullKeyType = TypeCombinator::removeNull($keyType);
859+
if (!$nonNullKeyType instanceof NeverType && !$keyType->isNull()->yes()) {
860+
$dimFetch = new ArrayDimFetch($arrayArg, $expr->var);
861+
$dimFetchString = $this->exprPrinter->printExpr($dimFetch);
862+
$keyExprString = $this->exprPrinter->printExpr($expr->var);
863+
864+
$holder = new ConditionalExpressionHolder(
865+
[$keyExprString => ExpressionTypeHolder::createYes($expr->var, $nonNullKeyType)],
866+
ExpressionTypeHolder::createYes($dimFetch, $arrayType->getIterableValueType()),
867+
);
868+
869+
$specifiedTypes = $specifiedTypes->unionWith(
870+
(new SpecifiedTypes([], []))->setNewConditionalExpressionHolders([
871+
$dimFetchString => [$holder->getKey() => $holder],
872+
]),
873+
);
874+
}
850875
}
851876
}
852877
}

src/Parser/RichParser.php

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,6 @@ private function parseIdentifiers(string $text, int $ignorePos): array
361361
throw new IgnoreParseException('Missing identifier', 1);
362362
}
363363

364-
/** @phpstan-ignore return.type (return type is correct, not sure why it's being changed from array shape to key-value shape) */
365364
return $identifiers;
366365
}
367366

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
<?php declare(strict_types=1);
2+
3+
namespace ArrayKeyLastExisting;
4+
5+
use function PHPStan\Testing\assertType;
6+
7+
/**
8+
* Mirrors the RichParser pattern: the array starts empty, gets entries
9+
* appended in some loop branches, and an existing entry's nested key is
10+
* updated in others. `if ($key !== null)` should be enough to let PHPStan
11+
* track that `$arr[$key]` exists and the deep write should preserve the
12+
* outer shape, just like `isset($arr[$key])` does.
13+
*/
14+
function appendThenUpdateLast(string $name, string $comment): void
15+
{
16+
$identifiers = [];
17+
$c = rand(100, 200);
18+
for ($i = 0; $i < $c; $i++) {
19+
if (rand(0, 1) === 1) {
20+
$key = array_key_last($identifiers);
21+
if ($key !== null) {
22+
$identifiers[$key]['comment'] = $comment;
23+
}
24+
continue;
25+
}
26+
27+
$identifiers[] = ['name' => $name, 'comment' => null];
28+
}
29+
30+
assertType('list<array{name: string, comment: string|null}>', $identifiers);
31+
}
32+
33+
function appendThenUpdateFirst(string $name, string $comment): void
34+
{
35+
$identifiers = [];
36+
$c = rand(100, 200);
37+
for ($i = 0; $i < $c; $i++) {
38+
if (rand(0, 1) === 1) {
39+
$key = array_key_first($identifiers);
40+
if ($key !== null) {
41+
$identifiers[$key]['comment'] = $comment;
42+
}
43+
continue;
44+
}
45+
46+
$identifiers[] = ['name' => $name, 'comment' => null];
47+
}
48+
49+
assertType('list<array{name: string, comment: string|null}>', $identifiers);
50+
}
51+
52+
/**
53+
* @param list<array{name: 'x', comment: null}> $list
54+
*/
55+
function maybeEmptyArray(array $list): void
56+
{
57+
$key = array_key_last($list);
58+
if ($key !== null) {
59+
assertType('array{name: \'x\', comment: null}', $list[$key]);
60+
$list[$key]['comment'] = 'hello';
61+
assertType('non-empty-list<array{name: \'x\', comment: \'hello\'}>', $list);
62+
}
63+
}

0 commit comments

Comments
 (0)