From 61c91d53a1ed54896dea9f9dbc83e697fcaa3995 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sun, 9 Mar 2025 14:59:58 +0100 Subject: [PATCH 1/4] Try fixing assigning array offsets on possibly undefined nested array offsets --- src/Analyser/NodeScopeResolver.php | 13 ++++-- tests/PHPStan/Analyser/nsrt/bug-10025.php | 25 +++++++++++ tests/PHPStan/Analyser/nsrt/bug-10089.php | 29 ++++++++++++ tests/PHPStan/Analyser/nsrt/bug-10640.php | 16 +++++++ tests/PHPStan/Analyser/nsrt/bug-12078.php | 44 +++++++++++++++++++ ...nexistentOffsetInArrayDimFetchRuleTest.php | 6 +++ tests/PHPStan/Rules/Arrays/data/bug-11679.php | 31 +++++++++++++ 7 files changed, 160 insertions(+), 4 deletions(-) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-10025.php create mode 100644 tests/PHPStan/Analyser/nsrt/bug-10089.php create mode 100644 tests/PHPStan/Analyser/nsrt/bug-10640.php create mode 100644 tests/PHPStan/Analyser/nsrt/bug-12078.php create mode 100644 tests/PHPStan/Rules/Arrays/data/bug-11679.php diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 12672c9d53..27007e9bf3 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -5756,24 +5756,26 @@ static function (): void { */ private function produceArrayDimFetchAssignValueToWrite(array $offsetTypes, Type $offsetValueType, Type $valueToWrite): Type { - $offsetValueTypeStack = [$offsetValueType]; + $offsetValueTypeStack = [[$offsetValueType, TrinaryLogic::createYes()]]; foreach (array_slice($offsetTypes, 0, -1) as $offsetType) { if ($offsetType === null) { + $has = TrinaryLogic::createYes(); $offsetValueType = new ConstantArrayType([], []); } else { + $has = $offsetValueType->hasOffsetValueType($offsetType); $offsetValueType = $offsetValueType->getOffsetValueType($offsetType); if ($offsetValueType instanceof ErrorType) { $offsetValueType = new ConstantArrayType([], []); } } - $offsetValueTypeStack[] = $offsetValueType; + $offsetValueTypeStack[] = [$offsetValueType, $has]; } foreach (array_reverse($offsetTypes) as $i => $offsetType) { /** @var Type $offsetValueType */ - $offsetValueType = array_pop($offsetValueTypeStack); + [$offsetValueType, $has] = array_pop($offsetValueTypeStack); if ( !$offsetValueType instanceof MixedType && !$offsetValueType->isConstantArray()->yes() @@ -5788,7 +5790,10 @@ private function produceArrayDimFetchAssignValueToWrite(array $offsetTypes, Type } $offsetValueType = TypeCombinator::intersect($offsetValueType, TypeCombinator::union(...$types)); } - $valueToWrite = $offsetValueType->setOffsetValueType($offsetType, $valueToWrite, $i === 0); + if (!$has->yes()) { + $offsetValueType = TypeCombinator::union($offsetValueType, new ConstantArrayType([], [])); + } + $valueToWrite = $offsetValueType->setOffsetValueType($offsetType, $valueToWrite); } return $valueToWrite; diff --git a/tests/PHPStan/Analyser/nsrt/bug-10025.php b/tests/PHPStan/Analyser/nsrt/bug-10025.php new file mode 100644 index 0000000000..bd6637c531 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-10025.php @@ -0,0 +1,25 @@ + $foos + * @param list $bars + */ +function x(array $foos, array $bars): void { + $arr = []; + foreach ($foos as $foo) { + $arr[$foo->groupId]['foo'][] = $foo; + } + foreach ($bars as $bar) { + $arr[$bar->groupId]['bar'][] = $bar; + } + + assertType('array, bar?: non-empty-list}>', $arr); +} diff --git a/tests/PHPStan/Analyser/nsrt/bug-10089.php b/tests/PHPStan/Analyser/nsrt/bug-10089.php new file mode 100644 index 0000000000..1756119067 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-10089.php @@ -0,0 +1,29 @@ +>', $matrix); + + $matrix[$size - 1][8] = 3; + + assertType('non-empty-array, 0|3>>', $matrix); + + return $matrix; + } + +} + + diff --git a/tests/PHPStan/Analyser/nsrt/bug-10640.php b/tests/PHPStan/Analyser/nsrt/bug-10640.php new file mode 100644 index 0000000000..28388ab2c8 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-10640.php @@ -0,0 +1,16 @@ +, del?: non-empty-list<2>}>', $changes); +}; diff --git a/tests/PHPStan/Analyser/nsrt/bug-12078.php b/tests/PHPStan/Analyser/nsrt/bug-12078.php new file mode 100644 index 0000000000..4bf6fef7f5 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-12078.php @@ -0,0 +1,44 @@ + + */ +function returnsData6M(): array +{ + return ["A"=>'data A',"B"=>'Data B']; +} +/** + * @return array + */ +function returnsData3M(): array +{ + return ["A"=>'data A',"C"=>'Data C']; +} + +function main(){ + $arrDataByKey = []; + + $arrData6M = returnsData6M(); + if([] === $arrData6M){ + echo "No data for 6M\n"; + }else{ + foreach($arrData6M as $key => $data){ + $arrDataByKey[$key]['6M'][] = $data; + } + } + + $arrData3M = returnsData3M(); + if([] === $arrData3M){ + echo "No data for 3M\n"; + }else{ + foreach($arrData3M as $key => $data){ + $arrDataByKey[$key]['3M'][] = $data; + } + } + + assertType('array, 3M?: non-empty-list}>', $arrDataByKey); +} diff --git a/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php b/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php index 518fabc0b9..1f68f3000c 100644 --- a/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php +++ b/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php @@ -829,4 +829,10 @@ public function testArrayDimFetchAfterArraySearch(): void ]); } + public function testBug11679(): void + { + $this->reportPossiblyNonexistentGeneralArrayOffset = true; + $this->analyse([__DIR__ . '/data/bug-11679.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Arrays/data/bug-11679.php b/tests/PHPStan/Rules/Arrays/data/bug-11679.php new file mode 100644 index 0000000000..e038897bb2 --- /dev/null +++ b/tests/PHPStan/Rules/Arrays/data/bug-11679.php @@ -0,0 +1,31 @@ +arr['foo'])) { + $this->arr['foo'] = true; + } + return $this->arr['foo']; + } +} + +class NonworkingExample +{ + /** @var array */ + private array $arr = []; + + public function sayHello(int $index): bool + { + if (!isset($this->arr[$index]['foo'])) { + $this->arr[$index]['foo'] = true; + } + return $this->arr[$index]['foo']; + } +} From e25dfea7b5570442055355f681c8ed4c1d4b4c2f Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sun, 9 Mar 2025 16:38:24 +0100 Subject: [PATCH 2/4] it works, kinda --- src/Analyser/NodeScopeResolver.php | 20 +++++++++---------- tests/PHPStan/Analyser/data/bug-10922.php | 2 +- .../TypesAssignedToPropertiesRuleTest.php | 5 +++++ 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 27007e9bf3..653ed5b23d 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -5384,12 +5384,12 @@ private function processAssignVar( } if ($dimExpr === null) { - $offsetTypes[] = null; - $offsetNativeTypes[] = null; + $offsetTypes[] = [null, TrinaryLogic::createYes()]; + $offsetNativeTypes[] = [null, TrinaryLogic::createYes()]; } else { - $offsetTypes[] = $scope->getType($dimExpr); - $offsetNativeTypes[] = $scope->getNativeType($dimExpr); + $offsetTypes[] = [$scope->getType($dimExpr), $scope->getType($dimFetch->var)->hasOffsetValueType($scope->getType($dimExpr))->or($scope->hasExpressionType($dimFetch))]; + $offsetNativeTypes[] = [$scope->getNativeType($dimExpr), $scope->getNativeType($dimFetch->var)->hasOffsetValueType($scope->getNativeType($dimExpr))->or($scope->hasExpressionType($dimFetch))]; if ($enterExpressionAssign) { $scope->enterExpressionAssign($dimExpr); @@ -5436,8 +5436,8 @@ private function processAssignVar( $nativeValueToWrite = $this->produceArrayDimFetchAssignValueToWrite($offsetNativeTypes, $offsetNativeValueType, $nativeValueToWrite); } else { $rewritten = false; - foreach ($offsetTypes as $i => $offsetType) { - $offsetNativeType = $offsetNativeTypes[$i]; + foreach ($offsetTypes as $i => [$offsetType]) { + [$offsetNativeType] = $offsetNativeTypes[$i]; if ($offsetType === null) { if ($offsetNativeType !== null) { throw new ShouldNotHappenException(); @@ -5752,18 +5752,16 @@ static function (): void { } /** - * @param list $offsetTypes + * @param list $offsetTypes */ private function produceArrayDimFetchAssignValueToWrite(array $offsetTypes, Type $offsetValueType, Type $valueToWrite): Type { $offsetValueTypeStack = [[$offsetValueType, TrinaryLogic::createYes()]]; - foreach (array_slice($offsetTypes, 0, -1) as $offsetType) { + foreach (array_slice($offsetTypes, 0, -1) as [$offsetType, $has]) { if ($offsetType === null) { - $has = TrinaryLogic::createYes(); $offsetValueType = new ConstantArrayType([], []); } else { - $has = $offsetValueType->hasOffsetValueType($offsetType); $offsetValueType = $offsetValueType->getOffsetValueType($offsetType); if ($offsetValueType instanceof ErrorType) { $offsetValueType = new ConstantArrayType([], []); @@ -5773,7 +5771,7 @@ private function produceArrayDimFetchAssignValueToWrite(array $offsetTypes, Type $offsetValueTypeStack[] = [$offsetValueType, $has]; } - foreach (array_reverse($offsetTypes) as $i => $offsetType) { + foreach (array_reverse($offsetTypes) as $i => [$offsetType]) { /** @var Type $offsetValueType */ [$offsetValueType, $has] = array_pop($offsetValueTypeStack); if ( diff --git a/tests/PHPStan/Analyser/data/bug-10922.php b/tests/PHPStan/Analyser/data/bug-10922.php index 62ee393f7b..bf025896ee 100644 --- a/tests/PHPStan/Analyser/data/bug-10922.php +++ b/tests/PHPStan/Analyser/data/bug-10922.php @@ -12,7 +12,7 @@ public function sayHello(array $array): void foreach ($array as $key => $item) { $array[$key]['bar'] = ''; } - assertType("array", $array); + assertType("array", $array); } /** @param array $array */ diff --git a/tests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest.php b/tests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest.php index 5e57ee2994..fd498ae219 100644 --- a/tests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest.php +++ b/tests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest.php @@ -502,6 +502,11 @@ public function testBug6356b(): void 26, "Offset 'age' (int) does not accept type int|string.", ], + [ + 'Property Bug6356b\HelloWorld2::$nestedDetails (array) does not accept non-empty-array.', + 29, + 'Offset \'age\' (int) does not accept type int|string.', + ], ]); } From 37d53130aa922c962ee6d266c2f851ead50cbb7e Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sun, 9 Mar 2025 16:50:51 +0100 Subject: [PATCH 3/4] another test --- src/Analyser/NodeScopeResolver.php | 7 +++--- tests/PHPStan/Analyser/data/bug-10922.php | 2 +- tests/PHPStan/Analyser/nsrt/bug-6173.php | 28 +++++++++++++++++++++++ 3 files changed, 33 insertions(+), 4 deletions(-) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-6173.php diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 653ed5b23d..c9a2562322 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -5771,9 +5771,10 @@ private function produceArrayDimFetchAssignValueToWrite(array $offsetTypes, Type $offsetValueTypeStack[] = [$offsetValueType, $has]; } - foreach (array_reverse($offsetTypes) as $i => [$offsetType]) { - /** @var Type $offsetValueType */ - [$offsetValueType, $has] = array_pop($offsetValueTypeStack); + foreach (array_reverse($offsetTypes) as [$offsetType]) { + /** @var array{Type, TrinaryLogic} $stackItem */ + $stackItem = array_pop($offsetValueTypeStack); + [$offsetValueType, $has] = $stackItem; if ( !$offsetValueType instanceof MixedType && !$offsetValueType->isConstantArray()->yes() diff --git a/tests/PHPStan/Analyser/data/bug-10922.php b/tests/PHPStan/Analyser/data/bug-10922.php index bf025896ee..036d49266f 100644 --- a/tests/PHPStan/Analyser/data/bug-10922.php +++ b/tests/PHPStan/Analyser/data/bug-10922.php @@ -38,6 +38,6 @@ public function sayHello3(array $array): void foreach ($array as $key => $item) { $array[$key]['bar'] = ''; } - assertType("non-empty-array", $array); + assertType("non-empty-array", $array); } } diff --git a/tests/PHPStan/Analyser/nsrt/bug-6173.php b/tests/PHPStan/Analyser/nsrt/bug-6173.php new file mode 100644 index 0000000000..89b16880f8 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-6173.php @@ -0,0 +1,28 @@ +', $res); + + return true; + } +} From e74b0210489a488aa682430145089878c9f63b1f Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sun, 9 Mar 2025 22:09:11 +0100 Subject: [PATCH 4/4] try specifying AST certainty in scope --- src/Analyser/NodeScopeResolver.php | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index c9a2562322..a7db7e66cf 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -5489,6 +5489,26 @@ private function processAssignVar( ); } } + + $arrayDimFetchVar = $originalVar; + while ($arrayDimFetchVar instanceof ArrayDimFetch) { + $dimExpr = $arrayDimFetchVar->dim; + if ($dimExpr === null) { + $arrayDimFetchVar = $arrayDimFetchVar->var; + continue; + } + + $dimVar = $arrayDimFetchVar->var; + $dimVarType = $scope->getType($dimVar); + $dimDimType = $scope->getType($dimExpr); + if ($dimVarType->hasOffsetValueType($dimDimType)->yes()) { + $arrayDimFetchVar = $arrayDimFetchVar->var; + continue; + } + + $scope = $scope->specifyExpressionType($arrayDimFetchVar, $scope->getType($arrayDimFetchVar), $scope->getNativeType($arrayDimFetchVar), TrinaryLogic::createYes()); + $arrayDimFetchVar = $arrayDimFetchVar->var; + } } else { if ($var instanceof Variable) { $nodeCallback(new VariableAssignNode($var, $assignedPropertyExpr, $isAssignOp), $scope);