Fix native type of array after array_push()

Closes phpstan/phpstan#9403
rajyan · Jun 7, 2023 · 564f79f · 564f79f
1 parent 903ffc6
commit 564f79f
Show file tree

Hide file tree

Showing 5 changed files with 158 additions and 100 deletions.
diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -51,6 +51,14 @@ parameters:
 			count: 1
 			path: src/Analyser/MutatingScope.php
 
+		-
+			message: """
+				#^Call to deprecated method doNotTreatPhpDocTypesAsCertain\\(\\) of class PHPStan\\\\Analyser\\\\MutatingScope\\:
+				Use getNativeType\\(\\)$#
+			"""
+			count: 1
+			path: src/Analyser/NodeScopeResolver.php
+
 		-
 			message: "#^Doing instanceof PHPStan\\\\Type\\\\Constant\\\\ConstantBooleanType is error\\-prone and deprecated\\. Use Type\\:\\:isTrue\\(\\) or Type\\:\\:isFalse\\(\\) instead\\.$#"
 			count: 3

diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php
@@ -1978,99 +1978,11 @@ function (MutatingScope $scope) use ($expr, $nodeCallback, $context): Expression
 				&& in_array($functionReflection->getName(), ['array_push', 'array_unshift'], true)
 				&& count($expr->getArgs()) >= 2
 			) {
-				$arrayArg = $expr->getArgs()[0]->value;
-				$arrayType = $scope->getType($arrayArg);
-				$callArgs = array_slice($expr->getArgs(), 1);
-
-				/**
-				 * @param Arg[] $callArgs
-				 * @param callable(?Type, Type, bool): void $setOffsetValueType
-				 */
-				$setOffsetValueTypes = static function (Scope $scope, array $callArgs, callable $setOffsetValueType, ?bool &$nonConstantArrayWasUnpacked = null): void {
-					foreach ($callArgs as $callArg) {
-						$callArgType = $scope->getType($callArg->value);
-						if ($callArg->unpack) {
-							if (count($callArgType->getConstantArrays()) === 1) {
-								$iterableValueTypes = $callArgType->getConstantArrays()[0]->getValueTypes();
-							} else {
-								$iterableValueTypes = [$callArgType->getIterableValueType()];
-								$nonConstantArrayWasUnpacked = true;
-							}
-
-							$isOptional = !$callArgType->isIterableAtLeastOnce()->yes();
-							foreach ($iterableValueTypes as $iterableValueType) {
-								if ($iterableValueType instanceof UnionType) {
-									foreach ($iterableValueType->getTypes() as $innerType) {
-										$setOffsetValueType(null, $innerType, $isOptional);
-									}
-								} else {
-									$setOffsetValueType(null, $iterableValueType, $isOptional);
-								}
-							}
-							continue;
-						}
-						$setOffsetValueType(null, $callArgType, false);
-					}
-				};
-
-				$constantArrays = $arrayType->getConstantArrays();
-				if (count($constantArrays) > 0) {
-					$newArrayTypes = [];
-					$prepend = $functionReflection->getName() === 'array_unshift';
-					foreach ($constantArrays as $constantArray) {
-						$arrayTypeBuilder = $prepend ? ConstantArrayTypeBuilder::createEmpty() : ConstantArrayTypeBuilder::createFromConstantArray($constantArray);
-
-						$setOffsetValueTypes(
-							$scope,
-							$callArgs,
-							static function (?Type $offsetType, Type $valueType, bool $optional) use (&$arrayTypeBuilder): void {
-								$arrayTypeBuilder->setOffsetValueType($offsetType, $valueType, $optional);
-							},
-							$nonConstantArrayWasUnpacked,
-						);
-
-						if ($prepend) {
-							$keyTypes = $constantArray->getKeyTypes();
-							$valueTypes = $constantArray->getValueTypes();
-							foreach ($keyTypes as $k => $keyType) {
-								$arrayTypeBuilder->setOffsetValueType(
-									count($keyType->getConstantStrings()) === 1 ? $keyType->getConstantStrings()[0] : null,
-									$valueTypes[$k],
-									$constantArray->isOptionalKey($k),
-								);
-							}
-						}
+				$arrayType = $this->getArrayFunctionAppendingType($functionReflection, $scope, $expr);
+				$arrayNativeType = $this->getArrayFunctionAppendingType($functionReflection, $scope->doNotTreatPhpDocTypesAsCertain(), $expr);
 
-						$constantArray = $arrayTypeBuilder->getArray();
-
-						if ($constantArray->isConstantArray()->yes() && $nonConstantArrayWasUnpacked) {
-							$array = new ArrayType($constantArray->generalize(GeneralizePrecision::lessSpecific())->getIterableKeyType(), $constantArray->getIterableValueType());
-							$constantArray = $constantArray->isIterableAtLeastOnce()->yes()
-								? TypeCombinator::intersect($array, new NonEmptyArrayType())
-								: $array;
-						}
-
-						$newArrayTypes[] = $constantArray;
-					}
-
-					$arrayType = TypeCombinator::union(...$newArrayTypes);
-				} else {
-					$setOffsetValueTypes(
-						$scope,
-						$callArgs,
-						static function (?Type $offsetType, Type $valueType, bool $optional) use (&$arrayType): void {
-							$isIterableAtLeastOnce = $arrayType->isIterableAtLeastOnce()->yes() || !$optional;
-							$arrayType = $arrayType->setOffsetValueType($offsetType, $valueType);
-							if ($isIterableAtLeastOnce) {
-								return;
-							}
-
-							$arrayType = new ArrayType($arrayType->getIterableKeyType(), $arrayType->getIterableValueType());
-						},
-					);
-				}
-
-				$scope = $scope->invalidateExpression($arrayArg)->assignExpression($arrayArg, $arrayType, $scope->getNativeType($arrayArg));
+				$arrayArg = $expr->getArgs()[0]->value;
+				$scope = $scope->invalidateExpression($arrayArg)->assignExpression($arrayArg, $arrayType, $arrayNativeType);
 			}
 
 			if (
@@ -2927,6 +2839,103 @@ static function (Node $node, Scope $scope) use ($nodeCallback): void {
 		);
 	}
 
+	private function getArrayFunctionAppendingType(FunctionReflection $functionReflection, Scope $scope, FuncCall $expr): Type
+	{
+		$arrayArg = $expr->getArgs()[0]->value;
+		$arrayType = $scope->getType($arrayArg);
+		$callArgs = array_slice($expr->getArgs(), 1);
+
+		/**
+		 * @param Arg[] $callArgs
+		 * @param callable(?Type, Type, bool): void $setOffsetValueType
+		 */
+		$setOffsetValueTypes = static function (Scope $scope, array $callArgs, callable $setOffsetValueType, ?bool &$nonConstantArrayWasUnpacked = null): void {
+			foreach ($callArgs as $callArg) {
+				$callArgType = $scope->getType($callArg->value);
+				if ($callArg->unpack) {
+					if (count($callArgType->getConstantArrays()) === 1) {
+						$iterableValueTypes = $callArgType->getConstantArrays()[0]->getValueTypes();
+					} else {
+						$iterableValueTypes = [$callArgType->getIterableValueType()];
+						$nonConstantArrayWasUnpacked = true;
+					}
+
+					$isOptional = !$callArgType->isIterableAtLeastOnce()->yes();
+					foreach ($iterableValueTypes as $iterableValueType) {
+						if ($iterableValueType instanceof UnionType) {
+							foreach ($iterableValueType->getTypes() as $innerType) {
+								$setOffsetValueType(null, $innerType, $isOptional);
+							}
+						} else {
+							$setOffsetValueType(null, $iterableValueType, $isOptional);
+						}
+					}
+					continue;
+				}
+				$setOffsetValueType(null, $callArgType, false);
+			}
+		};
+
+		$constantArrays = $arrayType->getConstantArrays();
+		if (count($constantArrays) > 0) {
+			$newArrayTypes = [];
+			$prepend = $functionReflection->getName() === 'array_unshift';
+			foreach ($constantArrays as $constantArray) {
+				$arrayTypeBuilder = $prepend ? ConstantArrayTypeBuilder::createEmpty() : ConstantArrayTypeBuilder::createFromConstantArray($constantArray);
+
+				$setOffsetValueTypes(
+					$scope,
+					$callArgs,
+					static function (?Type $offsetType, Type $valueType, bool $optional) use (&$arrayTypeBuilder): void {
+						$arrayTypeBuilder->setOffsetValueType($offsetType, $valueType, $optional);
+					},
+					$nonConstantArrayWasUnpacked,
+				);
+
+				if ($prepend) {
+					$keyTypes = $constantArray->getKeyTypes();
+					$valueTypes = $constantArray->getValueTypes();
+					foreach ($keyTypes as $k => $keyType) {
+						$arrayTypeBuilder->setOffsetValueType(
+							count($keyType->getConstantStrings()) === 1 ? $keyType->getConstantStrings()[0] : null,
+							$valueTypes[$k],
+							$constantArray->isOptionalKey($k),
+						);
+					}
+				}
+
+				$constantArray = $arrayTypeBuilder->getArray();
+
+				if ($constantArray->isConstantArray()->yes() && $nonConstantArrayWasUnpacked) {
+					$array = new ArrayType($constantArray->generalize(GeneralizePrecision::lessSpecific())->getIterableKeyType(), $constantArray->getIterableValueType());
+					$constantArray = $constantArray->isIterableAtLeastOnce()->yes()
+						? TypeCombinator::intersect($array, new NonEmptyArrayType())
+						: $array;
+				}
+
+				$newArrayTypes[] = $constantArray;
+			}
+
+			return TypeCombinator::union(...$newArrayTypes);
+		}
+
+		$setOffsetValueTypes(
+			$scope,
+			$callArgs,
+			static function (?Type $offsetType, Type $valueType, bool $optional) use (&$arrayType): void {
+				$isIterableAtLeastOnce = $arrayType->isIterableAtLeastOnce()->yes() || !$optional;
+				$arrayType = $arrayType->setOffsetValueType($offsetType, $valueType);
+				if ($isIterableAtLeastOnce) {
+					return;
+				}
+
+				$arrayType = new ArrayType($arrayType->getIterableKeyType(), $arrayType->getIterableValueType());
+			},
+		);
+
+		return $arrayType;
+	}
+
 	private function getFunctionThrowPoint(
 		FunctionReflection $functionReflection,
 		?ParametersAcceptor $parametersAcceptor,

diff --git a/tests/PHPStan/Analyser/NodeScopeResolverTest.php b/tests/PHPStan/Analyser/NodeScopeResolverTest.php
@@ -1247,6 +1247,7 @@ public function dataFileAsserts(): iterable
 		yield from $this->gatherAssertTypes(__DIR__ . '/data/invalid-type-aliases.php');
 		yield from $this->gatherAssertTypes(__DIR__ . '/data/asymmetric-properties.php');
 		yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-9062.php');
+		yield from $this->gatherAssertTypes(__DIR__ . '/../Rules/Variables/data/bug-9403.php');
 		yield from $this->gatherAssertTypes(__DIR__ . '/data/object-shape.php');
 		yield from $this->gatherAssertTypes(__DIR__ . '/data/memcache-get.php');
 		yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-4302b.php');

diff --git a/tests/PHPStan/Rules/Variables/EmptyRuleTest.php b/tests/PHPStan/Rules/Variables/EmptyRuleTest.php
@@ -96,14 +96,6 @@ public function testBug6974(): void
 				'Variable $a in empty() always exists and is always falsy.',
 				12,
 			],
-			[
-				'Variable $a in empty() always exists and is always falsy.',
-				21,
-			],
-			[
-				'Variable $a in empty() always exists and is always falsy.',
-				30,
-			],
 		]);
 	}
 
@@ -221,4 +213,21 @@ public function testBug9126(): void
 		$this->analyse([__DIR__ . '/data/bug-9126.php'], []);
 	}
 
+	public function dataBug9403(): iterable
+	{
+		yield [true];
+		yield [false];
+	}
+
+	/**
+	 * @dataProvider dataBug9403
+	 */
+	public function testBug9403(bool $treatPhpDocTypesAsCertain): void
+	{
+		$this->treatPhpDocTypesAsCertain = $treatPhpDocTypesAsCertain;
+		$this->strictUnnecessaryNullsafePropertyFetch = false;
+
+		$this->analyse([__DIR__ . '/data/bug-9403.php'], []);
+	}
+
 }
diff --git a/tests/PHPStan/Rules/Variables/data/bug-9403.php b/tests/PHPStan/Rules/Variables/data/bug-9403.php
@@ -0,0 +1,31 @@
+<?php
+
+namespace Bug9403;
+
+use function PHPStan\Testing\assertNativeType;
+use function PHPStan\Testing\assertType;
+
+class HelloWorld
+{
+
+	/**
+	 * @param int $max
+	 * @return int[]
+	 */
+	public function testMe(int $max): array
+	{
+		$result = [];
+		for ($i = 0; $i < $max; $i++) {
+			array_push($result, $i);
+		}
+
+		assertType('list<int<0, max>>', $result);
+		assertNativeType('list<int<0, max>>', $result);
+
+		if (!empty($result)) {
+			rsort($result);
+		}
+		return $result;
+	}
+
+}