From caf9684ac7d642d8862e802b2faeec1e807e7e7f Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 28 Mar 2025 15:20:42 +0100 Subject: [PATCH 1/4] Fix lost list-type if substituted a element via loop --- src/Analyser/NodeScopeResolver.php | 4 ++ tests/PHPStan/Analyser/nsrt/bug-12274.php | 37 +++++++++++++++++++ .../Rules/Functions/ReturnTypeRuleTest.php | 14 +++++++ 3 files changed, 55 insertions(+) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-12274.php diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 0135b2dab7..a4d50d37b7 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -5482,6 +5482,10 @@ private function processAssignVar( } if ($varType->isArray()->yes() || !(new ObjectType(ArrayAccess::class))->isSuperTypeOf($varType)->yes()) { + if ($varType->isList()->yes() && $scope->hasExpressionType($originalVar)->yes()) { + $valueToWrite = TypeCombinator::intersect($valueToWrite, new AccessoryArrayListType()); + } + if ($var instanceof Variable && is_string($var->name)) { $nodeCallback(new VariableAssignNode($var, $assignedPropertyExpr, $isAssignOp), $scope); $scope = $scope->assignVariable($var->name, $valueToWrite, $nativeValueToWrite, TrinaryLogic::createYes()); diff --git a/tests/PHPStan/Analyser/nsrt/bug-12274.php b/tests/PHPStan/Analyser/nsrt/bug-12274.php new file mode 100644 index 0000000000..0469c6ce18 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-12274.php @@ -0,0 +1,37 @@ + $items + * + * @return non-empty-list + */ +function getItems(array $items): array +{ + foreach ($items as $index => $item) { + $items[$index] = 1; + } + + assertType('non-empty-list', $items); + return $items; +} + +/** + * @param non-empty-list $items + * + * @return non-empty-list + */ +function getItemsByModifiedIndex(array $items): array +{ + foreach ($items as $index => $item) { + $index++; + + $items[$index] = 1; + } + + assertType('non-empty-array, int>', $items); + return $items; +} diff --git a/tests/PHPStan/Rules/Functions/ReturnTypeRuleTest.php b/tests/PHPStan/Rules/Functions/ReturnTypeRuleTest.php index 54206ff2de..12307bf5bc 100644 --- a/tests/PHPStan/Rules/Functions/ReturnTypeRuleTest.php +++ b/tests/PHPStan/Rules/Functions/ReturnTypeRuleTest.php @@ -345,4 +345,18 @@ public function testBug11301(): void ]); } + public function testBug12274(): void + { + $this->checkExplicitMixed = true; + $this->checkNullables = true; + + $this->analyse([__DIR__ . '/../../Analyser/nsrt/bug-12274.php'], [ + [ + 'Function Bug12274\getItemsByModifiedIndex() should return non-empty-list but returns non-empty-array, int>.', + 36, + 'non-empty-array, int> might not be a list.', + ], + ]); + } + } From ad2b177bd0bc201ad55604f913360ebf3d3f7c5d Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sat, 29 Mar 2025 09:10:01 +0100 Subject: [PATCH 2/4] test ast based offset --- tests/PHPStan/Analyser/nsrt/bug-12274.php | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/PHPStan/Analyser/nsrt/bug-12274.php b/tests/PHPStan/Analyser/nsrt/bug-12274.php index 0469c6ce18..565dc68802 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-12274.php +++ b/tests/PHPStan/Analyser/nsrt/bug-12274.php @@ -35,3 +35,14 @@ function getItemsByModifiedIndex(array $items): array assertType('non-empty-array, int>', $items); return $items; } + +/** @param list $list */ +function testKeepListAfterIssetIndex(array $list, int $i): void +{ + if (isset($list[$i])) { + assertType('list', $list); + $list[$i] = 21; + assertType('non-empty-list', $list); + } + assertType('list', $list); +} From b1b70081ae2a2a813305ec0ff74576485bd1bf04 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sat, 29 Mar 2025 09:29:32 +0100 Subject: [PATCH 3/4] keep list for $list[1 + $index] assignments --- src/Analyser/NodeScopeResolver.php | 22 ++++++++++- tests/PHPStan/Analyser/nsrt/bug-12274.php | 48 +++++++++++++++++++++++ 2 files changed, 68 insertions(+), 2 deletions(-) diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index a4d50d37b7..3877c675a8 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -5482,8 +5482,26 @@ private function processAssignVar( } if ($varType->isArray()->yes() || !(new ObjectType(ArrayAccess::class))->isSuperTypeOf($varType)->yes()) { - if ($varType->isList()->yes() && $scope->hasExpressionType($originalVar)->yes()) { - $valueToWrite = TypeCombinator::intersect($valueToWrite, new AccessoryArrayListType()); + if ($varType->isList()->yes()) { + if ($scope->hasExpressionType($originalVar)->yes()) { // keep list for $list[$index] assignments + $valueToWrite = TypeCombinator::intersect($valueToWrite, new AccessoryArrayListType()); + } elseif ($originalVar->dim instanceof BinaryOp\Plus) { + if ( // keep list for $list[$index + 1] assignments + $originalVar->dim->right instanceof Variable + && $originalVar->dim->left instanceof Node\Scalar\Int_ + && $originalVar->dim->left->value === 1 + && $scope->hasExpressionType(new ArrayDimFetch($originalVar->var, $originalVar->dim->right))->yes() + ) { + $valueToWrite = TypeCombinator::intersect($valueToWrite, new AccessoryArrayListType()); + } elseif ( // keep list for $list[1 + $index] assignments + $originalVar->dim->left instanceof Variable + && $originalVar->dim->right instanceof Node\Scalar\Int_ + && $originalVar->dim->right->value === 1 + && $scope->hasExpressionType(new ArrayDimFetch($originalVar->var, $originalVar->dim->left))->yes() + ) { + $valueToWrite = TypeCombinator::intersect($valueToWrite, new AccessoryArrayListType()); + } + } } if ($var instanceof Variable && is_string($var->name)) { diff --git a/tests/PHPStan/Analyser/nsrt/bug-12274.php b/tests/PHPStan/Analyser/nsrt/bug-12274.php index 565dc68802..7eda592205 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-12274.php +++ b/tests/PHPStan/Analyser/nsrt/bug-12274.php @@ -43,6 +43,54 @@ function testKeepListAfterIssetIndex(array $list, int $i): void assertType('list', $list); $list[$i] = 21; assertType('non-empty-list', $list); + $list[$i+1] = 21; + assertType('non-empty-list', $list); + } + assertType('list', $list); +} + +/** @param list $list */ +function testKeepListAfterIssetIndexPlusOne(array $list, int $i): void +{ + if (isset($list[$i])) { + assertType('list', $list); + $list[$i+1] = 21; + assertType('non-empty-list', $list); + } + assertType('list', $list); +} + +/** @param list $list */ +function testKeepListAfterIssetIndexOnePlus(array $list, int $i): void +{ + if (isset($list[$i])) { + assertType('list', $list); + $list[1+$i] = 21; + assertType('non-empty-list', $list); } assertType('list', $list); } + +/** @param list $list */ +function testShouldLooseListbyAst(array $list, int $i): void +{ + if (isset($list[$i])) { + $i++; + + assertType('list', $list); + $list[1+$i] = 21; + assertType('non-empty-array', $list); + } + assertType('array', $list); +} + +/** @param list $list */ +function testShouldLooseListbyAst2(array $list, int $i): void +{ + if (isset($list[$i])) { + assertType('list', $list); + $list[2+$i] = 21; + assertType('non-empty-array', $list); + } + assertType('array', $list); +} From 4ee0d7341ec913efb3470560a2d2f6d7f8eb2de5 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sun, 30 Mar 2025 08:39:35 +0200 Subject: [PATCH 4/4] Support nested assigns --- src/Analyser/NodeScopeResolver.php | 57 ++++++++++++----------- tests/PHPStan/Analyser/nsrt/bug-12274.php | 13 ++++++ 2 files changed, 44 insertions(+), 26 deletions(-) diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 3877c675a8..668c3aa9bd 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -5450,14 +5450,15 @@ private function processAssignVar( $offsetValueType = $varType; $offsetNativeValueType = $varNativeType; - $valueToWrite = $this->produceArrayDimFetchAssignValueToWrite($offsetTypes, $offsetValueType, $valueToWrite); + $valueToWrite = $this->produceArrayDimFetchAssignValueToWrite($dimFetchStack, $offsetTypes, $offsetValueType, $valueToWrite, $scope); if (!$offsetValueType->equals($offsetNativeValueType) || !$valueToWrite->equals($nativeValueToWrite)) { - $nativeValueToWrite = $this->produceArrayDimFetchAssignValueToWrite($offsetNativeTypes, $offsetNativeValueType, $nativeValueToWrite); + $nativeValueToWrite = $this->produceArrayDimFetchAssignValueToWrite($dimFetchStack, $offsetNativeTypes, $offsetNativeValueType, $nativeValueToWrite, $scope); } else { $rewritten = false; foreach ($offsetTypes as $i => $offsetType) { $offsetNativeType = $offsetNativeTypes[$i]; + if ($offsetType === null) { if ($offsetNativeType !== null) { throw new ShouldNotHappenException(); @@ -5471,7 +5472,7 @@ private function processAssignVar( continue; } - $nativeValueToWrite = $this->produceArrayDimFetchAssignValueToWrite($offsetNativeTypes, $offsetNativeValueType, $nativeValueToWrite); + $nativeValueToWrite = $this->produceArrayDimFetchAssignValueToWrite($dimFetchStack, $offsetNativeTypes, $offsetNativeValueType, $nativeValueToWrite, $scope); $rewritten = true; break; } @@ -5482,28 +5483,6 @@ private function processAssignVar( } if ($varType->isArray()->yes() || !(new ObjectType(ArrayAccess::class))->isSuperTypeOf($varType)->yes()) { - if ($varType->isList()->yes()) { - if ($scope->hasExpressionType($originalVar)->yes()) { // keep list for $list[$index] assignments - $valueToWrite = TypeCombinator::intersect($valueToWrite, new AccessoryArrayListType()); - } elseif ($originalVar->dim instanceof BinaryOp\Plus) { - if ( // keep list for $list[$index + 1] assignments - $originalVar->dim->right instanceof Variable - && $originalVar->dim->left instanceof Node\Scalar\Int_ - && $originalVar->dim->left->value === 1 - && $scope->hasExpressionType(new ArrayDimFetch($originalVar->var, $originalVar->dim->right))->yes() - ) { - $valueToWrite = TypeCombinator::intersect($valueToWrite, new AccessoryArrayListType()); - } elseif ( // keep list for $list[1 + $index] assignments - $originalVar->dim->left instanceof Variable - && $originalVar->dim->right instanceof Node\Scalar\Int_ - && $originalVar->dim->right->value === 1 - && $scope->hasExpressionType(new ArrayDimFetch($originalVar->var, $originalVar->dim->left))->yes() - ) { - $valueToWrite = TypeCombinator::intersect($valueToWrite, new AccessoryArrayListType()); - } - } - } - if ($var instanceof Variable && is_string($var->name)) { $nodeCallback(new VariableAssignNode($var, $assignedPropertyExpr, $isAssignOp), $scope); $scope = $scope->assignVariable($var->name, $valueToWrite, $nativeValueToWrite, TrinaryLogic::createYes()); @@ -5806,9 +5785,10 @@ static function (): void { } /** + * @param list $dimFetchStack * @param list $offsetTypes */ - private function produceArrayDimFetchAssignValueToWrite(array $offsetTypes, Type $offsetValueType, Type $valueToWrite): Type + private function produceArrayDimFetchAssignValueToWrite(array $dimFetchStack, array $offsetTypes, Type $offsetValueType, Type $valueToWrite, Scope $scope): Type { $offsetValueTypeStack = [$offsetValueType]; foreach (array_slice($offsetTypes, 0, -1) as $offsetType) { @@ -5843,6 +5823,31 @@ private function produceArrayDimFetchAssignValueToWrite(array $offsetTypes, Type $offsetValueType = TypeCombinator::intersect($offsetValueType, TypeCombinator::union(...$types)); } $valueToWrite = $offsetValueType->setOffsetValueType($offsetType, $valueToWrite, $i === 0); + + $arrayDimFetch = $dimFetchStack[$i] ?? null; + if ($arrayDimFetch === null || !$offsetValueType->isList()->yes()) { + continue; + } + + if ($scope->hasExpressionType($arrayDimFetch)->yes()) { // keep list for $list[$index] assignments + $valueToWrite = TypeCombinator::intersect($valueToWrite, new AccessoryArrayListType()); + } elseif ($arrayDimFetch->dim instanceof BinaryOp\Plus) { + if ( // keep list for $list[$index + 1] assignments + $arrayDimFetch->dim->right instanceof Variable + && $arrayDimFetch->dim->left instanceof Node\Scalar\Int_ + && $arrayDimFetch->dim->left->value === 1 + && $scope->hasExpressionType(new ArrayDimFetch($arrayDimFetch->var, $arrayDimFetch->dim->right))->yes() + ) { + $valueToWrite = TypeCombinator::intersect($valueToWrite, new AccessoryArrayListType()); + } elseif ( // keep list for $list[1 + $index] assignments + $arrayDimFetch->dim->left instanceof Variable + && $arrayDimFetch->dim->right instanceof Node\Scalar\Int_ + && $arrayDimFetch->dim->right->value === 1 + && $scope->hasExpressionType(new ArrayDimFetch($arrayDimFetch->var, $arrayDimFetch->dim->left))->yes() + ) { + $valueToWrite = TypeCombinator::intersect($valueToWrite, new AccessoryArrayListType()); + } + } } return $valueToWrite; diff --git a/tests/PHPStan/Analyser/nsrt/bug-12274.php b/tests/PHPStan/Analyser/nsrt/bug-12274.php index 7eda592205..437dc09ae3 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-12274.php +++ b/tests/PHPStan/Analyser/nsrt/bug-12274.php @@ -49,6 +49,19 @@ function testKeepListAfterIssetIndex(array $list, int $i): void assertType('list', $list); } +/** @param list> $nestedList */ +function testKeepNestedListAfterIssetIndex(array $nestedList, int $i, int $j): void +{ + if (isset($nestedList[$i][$j])) { + assertType('list>', $nestedList); + assertType('list', $nestedList[$i]); + $nestedList[$i][$j] = 21; + assertType('non-empty-list>', $nestedList); + assertType('non-empty-list', $nestedList[$i]); + } + assertType('list>', $nestedList); +} + /** @param list $list */ function testKeepListAfterIssetIndexPlusOne(array $list, int $i): void {