diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 79d4be017b..6d4f467c46 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -2373,7 +2373,7 @@ public function enterForeach(self $originalScope, Expr $iteratee, string $valueN if ($keyName !== null) { $scope = $scope->enterForeachKey($originalScope, $iteratee, $keyName); - if ($valueByRef && $iterateeType->isArray()->yes() && $iterateeType->isConstantArray()->no()) { + if ($valueByRef && $iterateeType->isArray()->yes()) { $scope = $scope->assignExpression( new IntertwinedVariableByReferenceWithExpr($valueName, new Expr\ArrayDimFetch($iteratee, new Variable($keyName)), new Variable($valueName)), $valueType, diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 89a65fca31..6abcd0fb7e 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -1294,6 +1294,10 @@ public function processStmtNode( $originalKeyVarExpr = null; $continueExitPointHasUnoriginalKeyType = false; + $byRefWithoutKey = $stmt->byRef + && $stmt->keyVar === null + && $stmt->valueVar instanceof Variable && is_string($stmt->valueVar->name); + if ($stmt->keyVar instanceof Variable && is_string($stmt->keyVar->name)) { $originalKeyVarExpr = new OriginalForeachKeyExpr($stmt->keyVar->name); if ($finalScope->hasExpressionType($originalKeyVarExpr)->yes()) { @@ -1301,12 +1305,30 @@ public function processStmtNode( } else { $continueExitPointHasUnoriginalKeyType = true; } + } elseif ($byRefWithoutKey) { + $originalValueExpr = new OriginalForeachValueExpr($stmt->valueVar->name); + if (!$finalScope->hasExpressionType($originalValueExpr)->yes()) { + $scopesWithIterableValueType[] = $finalScope; + } else { + $continueExitPointHasUnoriginalKeyType = true; + } } foreach ($finalScopeResult->getExitPointsByType(Continue_::class) as $continueExitPoint) { $continueScope = $continueExitPoint->getScope(); $finalScope = $continueScope->mergeWith($finalScope); - if ($originalKeyVarExpr === null || !$continueScope->hasExpressionType($originalKeyVarExpr)->yes()) { + if ($originalKeyVarExpr !== null) { + if (!$continueScope->hasExpressionType($originalKeyVarExpr)->yes()) { + $continueExitPointHasUnoriginalKeyType = true; + continue; + } + } elseif ($byRefWithoutKey) { + $originalValueExpr = new OriginalForeachValueExpr($stmt->valueVar->name); + if ($continueScope->hasExpressionType($originalValueExpr)->yes()) { + $continueExitPointHasUnoriginalKeyType = true; + continue; + } + } else { $continueExitPointHasUnoriginalKeyType = true; continue; } @@ -1327,58 +1349,76 @@ public function processStmtNode( count($breakExitPoints) === 0 && count($scopesWithIterableValueType) > 0 && !$continueExitPointHasUnoriginalKeyType - && $stmt->keyVar !== null + && ($stmt->keyVar !== null || $byRefWithoutKey) && (!$hasExpr->no() || !$stmt->expr instanceof Variable) && $exprType->isArray()->yes() - && $exprType->isConstantArray()->no() + && ($exprType->isConstantArray()->no() || $stmt->byRef) ) { - $arrayExprDimFetch = new ArrayDimFetch($stmt->expr, $stmt->keyVar); - $originalValueExpr = null; - if ($stmt->valueVar instanceof Variable && is_string($stmt->valueVar->name)) { - $originalValueExpr = new OriginalForeachValueExpr($stmt->valueVar->name); - } - $arrayDimFetchLoopTypes = []; - $keyLoopTypes = []; - foreach ($scopesWithIterableValueType as $scopeWithIterableValueType) { - $dimFetchType = $scopeWithIterableValueType->getType($arrayExprDimFetch); - // Condition-based narrowings like `is_string($type)` apply to the value - // variable but not automatically to the array dim fetch, even though the - // two describe the same element for a given iteration. If the value var - // hasn't been reassigned (OriginalForeachValueExpr still tracked) we use - // the narrowed value-var type in place of the broader dim fetch type so - // the loop's final array rewrite below picks up the sharper element type. - if ($originalValueExpr !== null && $scopeWithIterableValueType->hasExpressionType($originalValueExpr)->yes()) { - $valueVarType = $scopeWithIterableValueType->getType($stmt->valueVar); - if ($dimFetchType->isSuperTypeOf($valueVarType)->yes()) { - $dimFetchType = $valueVarType; + $nativeExprType = $scope->getNativeType($stmt->expr); + $arrayDimFetchLoopType = $exprType->getIterableValueType(); + $arrayDimFetchLoopNativeType = $nativeExprType->getIterableValueType(); + $keyLoopType = $exprType->getIterableKeyType(); + $keyLoopNativeType = $nativeExprType->getIterableKeyType(); + $valueTypeChanged = false; + $keyTypeChanged = false; + + if ($stmt->keyVar !== null) { + $arrayExprDimFetch = new ArrayDimFetch($stmt->expr, $stmt->keyVar); + $originalValueExpr = null; + if ($stmt->valueVar instanceof Variable && is_string($stmt->valueVar->name)) { + $originalValueExpr = new OriginalForeachValueExpr($stmt->valueVar->name); + } + $arrayDimFetchLoopTypes = []; + $keyLoopTypes = []; + foreach ($scopesWithIterableValueType as $scopeWithIterableValueType) { + $dimFetchType = $scopeWithIterableValueType->getType($arrayExprDimFetch); + if ($originalValueExpr !== null && $scopeWithIterableValueType->hasExpressionType($originalValueExpr)->yes()) { + $valueVarType = $scopeWithIterableValueType->getType($stmt->valueVar); + if ($dimFetchType->isSuperTypeOf($valueVarType)->yes()) { + $dimFetchType = $valueVarType; + } } + $arrayDimFetchLoopTypes[] = $dimFetchType; + $keyLoopTypes[] = $scopeWithIterableValueType->getType($stmt->keyVar); } - $arrayDimFetchLoopTypes[] = $dimFetchType; - $keyLoopTypes[] = $scopeWithIterableValueType->getType($stmt->keyVar); - } - $arrayDimFetchLoopType = TypeCombinator::union(...$arrayDimFetchLoopTypes); - $keyLoopType = TypeCombinator::union(...$keyLoopTypes); - - $arrayDimFetchLoopNativeTypes = []; - $keyLoopNativeTypes = []; - foreach ($scopesWithIterableValueType as $scopeWithIterableValueType) { - $dimFetchNativeType = $scopeWithIterableValueType->getNativeType($arrayExprDimFetch); - if ($originalValueExpr !== null && $scopeWithIterableValueType->hasExpressionType($originalValueExpr)->yes()) { - $valueVarNativeType = $scopeWithIterableValueType->getNativeType($stmt->valueVar); - if ($dimFetchNativeType->isSuperTypeOf($valueVarNativeType)->yes()) { - $dimFetchNativeType = $valueVarNativeType; + $arrayDimFetchLoopType = TypeCombinator::union(...$arrayDimFetchLoopTypes); + $keyLoopType = TypeCombinator::union(...$keyLoopTypes); + + $arrayDimFetchLoopNativeTypes = []; + $keyLoopNativeTypes = []; + foreach ($scopesWithIterableValueType as $scopeWithIterableValueType) { + $dimFetchNativeType = $scopeWithIterableValueType->getNativeType($arrayExprDimFetch); + if ($originalValueExpr !== null && $scopeWithIterableValueType->hasExpressionType($originalValueExpr)->yes()) { + $valueVarNativeType = $scopeWithIterableValueType->getNativeType($stmt->valueVar); + if ($dimFetchNativeType->isSuperTypeOf($valueVarNativeType)->yes()) { + $dimFetchNativeType = $valueVarNativeType; + } } + $arrayDimFetchLoopNativeTypes[] = $dimFetchNativeType; + $keyLoopNativeTypes[] = $scopeWithIterableValueType->getType($stmt->keyVar); } - $arrayDimFetchLoopNativeTypes[] = $dimFetchNativeType; - $keyLoopNativeTypes[] = $scopeWithIterableValueType->getType($stmt->keyVar); - } - $arrayDimFetchLoopNativeType = TypeCombinator::union(...$arrayDimFetchLoopNativeTypes); - $keyLoopNativeType = TypeCombinator::union(...$keyLoopNativeTypes); + $arrayDimFetchLoopNativeType = TypeCombinator::union(...$arrayDimFetchLoopNativeTypes); + $keyLoopNativeType = TypeCombinator::union(...$keyLoopNativeTypes); + + $valueTypeChanged = !$arrayDimFetchLoopType->equals($exprType->getIterableValueType()); + $keyTypeChanged = !$keyLoopType->equals($exprType->getIterableKeyType()); + } elseif ($byRefWithoutKey) { + $arrayDimFetchLoopTypes = []; + foreach ($scopesWithIterableValueType as $scopeWithIterableValueType) { + $arrayDimFetchLoopTypes[] = $scopeWithIterableValueType->getType($stmt->valueVar); + } + $arrayDimFetchLoopType = TypeCombinator::union(...$arrayDimFetchLoopTypes); + + $arrayDimFetchLoopNativeTypes = []; + foreach ($scopesWithIterableValueType as $scopeWithIterableValueType) { + $arrayDimFetchLoopNativeTypes[] = $scopeWithIterableValueType->getNativeType($stmt->valueVar); + } + $arrayDimFetchLoopNativeType = TypeCombinator::union(...$arrayDimFetchLoopNativeTypes); - $valueTypeChanged = !$arrayDimFetchLoopType->equals($exprType->getIterableValueType()); - $keyTypeChanged = !$keyLoopType->equals($exprType->getIterableKeyType()); + $valueTypeChanged = !$arrayDimFetchLoopType->equals($exprType->getIterableValueType()); + } if ($valueTypeChanged || $keyTypeChanged) { $newExprType = TypeTraverser::map($exprType, static function (Type $type, callable $traverse) use ($arrayDimFetchLoopType, $keyLoopType, $valueTypeChanged, $keyTypeChanged): Type { @@ -1386,6 +1426,10 @@ public function processStmtNode( return $traverse($type); } + if ($type->isConstantArray()->yes() && $valueTypeChanged && !$keyTypeChanged) { + return $type->traverse(static fn () => $arrayDimFetchLoopType); + } + if (!$type instanceof ArrayType) { return $type; } @@ -1395,12 +1439,15 @@ public function processStmtNode( $valueTypeChanged ? $arrayDimFetchLoopType : $type->getIterableValueType(), ); }); - $nativeExprType = $scope->getNativeType($stmt->expr); $newExprNativeType = TypeTraverser::map($nativeExprType, static function (Type $type, callable $traverse) use ($arrayDimFetchLoopNativeType, $keyLoopNativeType, $valueTypeChanged, $keyTypeChanged): Type { if ($type instanceof UnionType || $type instanceof IntersectionType) { return $traverse($type); } + if ($type->isConstantArray()->yes() && $valueTypeChanged && !$keyTypeChanged) { + return $type->traverse(static fn () => $arrayDimFetchLoopNativeType); + } + if (!$type instanceof ArrayType) { return $type; } diff --git a/tests/PHPStan/Analyser/nsrt/bug-1311.php b/tests/PHPStan/Analyser/nsrt/bug-1311.php new file mode 100644 index 0000000000..257d4baee7 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-1311.php @@ -0,0 +1,147 @@ + + */ + private $list = []; + + /** + * @param array $temp + */ + public function convertList(array $temp): void + { + foreach ($temp as &$item) { + $item = (string) $item; + } + + assertType('array', $temp); + + $this->list = $temp; + } + + public function convertListByRefWithoutKey(): void + { + $temp = [1, 2, 3]; + + foreach ($temp as &$item) { + $item = (string) $item; + } + + assertType("array{'1'|'2'|'3', '1'|'2'|'3', '1'|'2'|'3'}", $temp); + + $this->list = $temp; + } + + public function convertListByRefWithKey(): void + { + $temp = [1, 2, 3]; + + foreach ($temp as $k => &$item) { + $item = (string) $item; + } + + assertType("array{'1'|'2'|'3', '1'|'2'|'3', '1'|'2'|'3'}", $temp); + + $this->list = $temp; + } + + public function byRefConstantArrayConditional(): void + { + $temp = [1, 2, 3]; + + foreach ($temp as &$item) { + if (rand(0, 1)) { + $item = (string) $item; + } + } + + assertType("array{1|2|3|'1'|'2'|'3', 1|2|3|'1'|'2'|'3', 1|2|3|'1'|'2'|'3'}", $temp); + } + + public function byRefConstantArrayWithBreak(): void + { + $temp = [1, 2, 3]; + + foreach ($temp as &$item) { + $item = (string) $item; + if (rand(0, 1)) { + break; + } + } + + assertType('array{1, 2, 3}', $temp); + } + + public function byRefConstantArrayIntval(): void + { + $temp = ['a', 'b', 'c']; + + foreach ($temp as &$item) { + $item = strlen($item); + } + + assertType('array{1, 1, 1}', $temp); + } + + public function byRefConstantArrayStringKeys(): void + { + $temp = ['x' => 1, 'y' => 2]; + + foreach ($temp as &$v) { + $v = (string) $v; + } + + assertType("array{x: '1'|'2', y: '1'|'2'}", $temp); + } + + public function byRefConstantArrayNoOverwrite(): void + { + $temp = [1, 2, 3]; + + foreach ($temp as &$item) { + echo $item; + } + + assertType('array{1, 2, 3}', $temp); + } + + public function constantArrayByRefSubElement(): void + { + $a = [ + [ + 'one' => 'one', + 'two' => 'two', + ], + [ + 'one' => 'one', + ], + ]; + + foreach ($a as &$testArray) { + $testArray['two'] = 'two'; + } + unset($testArray); + + assertType("array{array{one: 'one', two: 'two'}, array{one: 'one', two: 'two'}}", $a); + + $key = 'three'; + + foreach ($a as $offset => $testArray) { + $a[$offset][$key] = $key; + } + + assertType("array{array{one: 'one', two: 'two', three: 'three'}, array{one: 'one', two: 'two', three: 'three'}}", $a); + + foreach ($a as $testArray) { + assertType("array{one: 'one', two: 'two', three: 'three'}", $testArray); + $testArray['two']; + $testArray['three']; + } + } +} diff --git a/tests/PHPStan/Analyser/nsrt/bug-13809.php b/tests/PHPStan/Analyser/nsrt/bug-13809.php index 9cdb9d5fa5..b91c64f7fd 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-13809.php +++ b/tests/PHPStan/Analyser/nsrt/bug-13809.php @@ -13,7 +13,7 @@ function foo(array $list): void $value = 'foo'; } - assertType('list', $list); + assertType("list<'foo'>", $list); } /** @@ -56,7 +56,7 @@ function bar3(array $list): void } } - assertType("list", $list); // could be list<'foo'|'maybe'> + assertType("list<'foo'|'maybe'>", $list); } /** @@ -68,7 +68,7 @@ function baz(array $list): void $value = 'bar'; } - assertType('list', $list); + assertType("list<'bar'>", $list); } /** diff --git a/tests/PHPStan/Analyser/nsrt/bug-14083.php b/tests/PHPStan/Analyser/nsrt/bug-14083.php index 8da92d43d3..14fcbef77a 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-14083.php +++ b/tests/PHPStan/Analyser/nsrt/bug-14083.php @@ -13,7 +13,7 @@ function example(array $convert): void { foreach ($convert as &$item) { $item = strtoupper($item); } - assertType('list', $convert); + assertType('list', $convert); } /** diff --git a/tests/PHPStan/Analyser/nsrt/bug-14084.php b/tests/PHPStan/Analyser/nsrt/bug-14084.php index 3f044aa559..006510dea4 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-14084.php +++ b/tests/PHPStan/Analyser/nsrt/bug-14084.php @@ -16,7 +16,7 @@ function example(array $convert): void $val = strtoupper($val); // https://github.com/phpstan/phpstan/issues/14083 } } - assertType('array>', $convert); + assertType('array>', $convert); } /** @@ -29,7 +29,7 @@ function example2(array $convert): void $inner[$key] = strtoupper($val); } } - assertType('array>', $convert); + assertType('array>', $convert); } /** diff --git a/tests/PHPStan/Analyser/nsrt/bug-1940.php b/tests/PHPStan/Analyser/nsrt/bug-1940.php new file mode 100644 index 0000000000..1e220e9a4c --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-1940.php @@ -0,0 +1,231 @@ + $arr + */ +function byRefWithoutKey(array $arr): void +{ + foreach ($arr as &$v) { + $v = 1; + } + + assertType('array', $arr); +} + +/** + * @param array $arr + */ +function byRefWithoutKeyConditional(array $arr): void +{ + foreach ($arr as &$v) { + if (rand(0, 1)) { + $v = 1; + } + } + + assertType('array', $arr); +} + +/** + * @param array $arr + */ +function byRefWithoutKeyAlwaysOverwritten(array $arr): void +{ + foreach ($arr as &$v) { + if (rand(0, 1)) { + $v = 1; + } else { + $v = 2; + } + } + + assertType('array', $arr); +} + +/** + * @param list $arr + */ +function byRefWithoutKeyList(array $arr): void +{ + foreach ($arr as &$v) { + $v = 'replaced'; + } + + assertType("list<'replaced'>", $arr); +} + +/** + * @param array $arr + */ +function byRefWithoutKeyStringKeys(array $arr): void +{ + foreach ($arr as &$v) { + $v = 'hello'; + } + + assertType("array", $arr); +} + +/** + * @param non-empty-array $arr + */ +function byRefWithoutKeyNonEmpty(array $arr): void +{ + foreach ($arr as &$v) { + $v = 42; + } + + assertType('non-empty-array', $arr); +} + +/** + * @param array $arr + */ +function byRefWithoutKeyBreak(array $arr): void +{ + foreach ($arr as &$v) { + $v = 1; + if (rand(0, 1)) { + break; + } + } + + assertType('array', $arr); +} + +/** + * @param array $arr + */ +function byRefWithoutKeyContinue(array $arr): void +{ + foreach ($arr as &$v) { + $v = 1; + if (rand(0, 1)) { + continue; + } + } + + assertType('array', $arr); +} + +/** + * @param array $arr + */ +function byRefWithoutKeyContinueBranches(array $arr): void +{ + foreach ($arr as &$v) { + if (rand(0, 1)) { + $v = 1; + continue; + } + $v = 2; + } + + assertType('array', $arr); +} + +/** + * @param array $arr + */ +function byRefWithoutKeyContinuePartial(array $arr): void +{ + foreach ($arr as &$v) { + if (rand(0, 1)) { + continue; + } + $v = 1; + } + + assertType('array', $arr); +} + +class Foo +{ + /** @var array */ + private array $prop; + + public function byRefWithoutKeyProperty(): void + { + foreach ($this->prop as &$v) { + $v = 'hello'; + } + + assertType("array", $this->prop); + } +} + +/** + * @param array $arr + */ +function byRefWithoutKeyTransform(array $arr): void +{ + foreach ($arr as &$v) { + $v = intval($v); + } + + assertType('array', $arr); +} + +/** + * @param array $arr + */ +function byRefWithoutKeyNoOverwrite(array $arr): void +{ + foreach ($arr as &$v) { + echo $v; + } + + assertType('array', $arr); +} + +/** + * @param list $a + */ +function byRefWithKeyModifySubElement(array $a): void +{ + foreach ($a as $k => &$testArray) { + $testArray['two'] = 'two'; + } + + assertType("list", $a); +} + +/** + * @param list $a + */ +function byRefWithoutKeyModifySubElement(array $a): void +{ + foreach ($a as &$testArray) { + $testArray['two'] = 'two'; + } + + assertType("list", $a); +} + +/** + * @param array $arr + */ +function byRefWithKeyDirectOverwrite(array $arr): void +{ + foreach ($arr as $k => &$v) { + $v = 1; + } + + assertType('array', $arr); +} + +/** + * @param array $arr + */ +function byRefWithoutKeyDirectOverwrite(array $arr): void +{ + foreach ($arr as &$v) { + $v = 1; + } + + assertType('array', $arr); +} diff --git a/tests/PHPStan/Analyser/nsrt/overwritten-arrays.php b/tests/PHPStan/Analyser/nsrt/overwritten-arrays.php index 878ae9d082..1ec5dd8b9e 100644 --- a/tests/PHPStan/Analyser/nsrt/overwritten-arrays.php +++ b/tests/PHPStan/Analyser/nsrt/overwritten-arrays.php @@ -127,7 +127,7 @@ public function doFoo7(array $a): void $v = 1; } - assertType('array', $a); // could be array + assertType('array', $a); } /** diff --git a/tests/PHPStan/Rules/Variables/data/bug-14124.php b/tests/PHPStan/Rules/Variables/data/bug-14124.php index be3ad1a8de..3a7303fb85 100644 --- a/tests/PHPStan/Rules/Variables/data/bug-14124.php +++ b/tests/PHPStan/Rules/Variables/data/bug-14124.php @@ -15,7 +15,7 @@ function example3a(array &$convert): void $val = strtoupper($val); } } - assertType('array>', $convert); + assertType('array>', $convert); } /** diff --git a/tests/PHPStan/Rules/Variables/data/bug-14124b.php b/tests/PHPStan/Rules/Variables/data/bug-14124b.php index 0ad84eb7a3..822e9f9ac6 100644 --- a/tests/PHPStan/Rules/Variables/data/bug-14124b.php +++ b/tests/PHPStan/Rules/Variables/data/bug-14124b.php @@ -17,7 +17,7 @@ function example3a(array &$convert): void } } } - assertType('array>>', $convert); + assertType('array>>', $convert); } /**