Fix type narrowing persisting across impure function calls

- TypeSpecifier::createForExpr() checked if the top-level function call was pure before remembering its type, but did not check whether arguments contained impure sub-expressions
- Added containsImpureCall() helper to recursively detect impure function/method/static calls within expression trees
- Applied the check to FuncCall, MethodCall, and StaticCall branches in createForExpr()
- Added regression test in tests/PHPStan/Analyser/nsrt/bug-13416.php

Closes phpstan/phpstan#13416

Author: staabm

src/Analyser/TypeSpecifier.php

@@ -87,6 +87,7 @@
 use function array_shift;
 use function count;
 use function in_array;
+use function is_array;
 use function is_string;
 use function strtolower;
 use function substr;
@@ -1972,6 +1973,12 @@ private function createForExpr(
 			if (!$this->rememberPossiblyImpureFunctionValues && !$hasSideEffects->no()) {
 				return new SpecifiedTypes([], []);
 			}
+
+			foreach ($expr->getArgs() as $arg) {
+				if ($this->containsImpureCall($arg->value, $scope)) {
+					return new SpecifiedTypes([], []);
+				}
+			}
 		}
 
 		if (
@@ -1992,6 +1999,24 @@ private function createForExpr(
 
 				return new SpecifiedTypes([], []);
 			}
+
+			if ($this->containsImpureCall($expr->var, $scope)) {
+				if (isset($containsNull) && !$containsNull) {
+					return $this->createNullsafeTypes($originalExpr, $scope, $context, $type);
+				}
+
+				return new SpecifiedTypes([], []);
+			}
+
+			foreach ($expr->getArgs() as $arg) {
+				if ($this->containsImpureCall($arg->value, $scope)) {
+					if (isset($containsNull) && !$containsNull) {
+						return $this->createNullsafeTypes($originalExpr, $scope, $context, $type);
+					}
+
+					return new SpecifiedTypes([], []);
+				}
+			}
 		}
 
 		if (
@@ -2017,6 +2042,16 @@ private function createForExpr(
 
 				return new SpecifiedTypes([], []);
 			}
+
+			foreach ($expr->getArgs() as $arg) {
+				if ($this->containsImpureCall($arg->value, $scope)) {
+					if (isset($containsNull) && !$containsNull) {
+						return $this->createNullsafeTypes($originalExpr, $scope, $context, $type);
+					}
+
+					return new SpecifiedTypes([], []);
+				}
+			}
 		}
 
 		$sureTypes = [];
@@ -2047,6 +2082,74 @@ private function createForExpr(
 		return $types;
 	}
 
+	private function containsImpureCall(Expr $expr, Scope $scope): bool
+	{
+		if ($expr instanceof FuncCall && $expr->name instanceof Name) {
+			if (!$this->reflectionProvider->hasFunction($expr->name, $scope)) {
+				return true;
+			}
+			$functionReflection = $this->reflectionProvider->getFunction($expr->name, $scope);
+			$hasSideEffects = $functionReflection->hasSideEffects();
+			if ($hasSideEffects->yes()) {
+				return true;
+			}
+			if (!$this->rememberPossiblyImpureFunctionValues && !$hasSideEffects->no()) {
+				return true;
+			}
+		}
+
+		if ($expr instanceof MethodCall && $expr->name instanceof Node\Identifier) {
+			$calledOnType = $scope->getType($expr->var);
+			$methodReflection = $scope->getMethodReflection($calledOnType, $expr->name->toString());
+			if (
+				$methodReflection === null
+				|| $methodReflection->hasSideEffects()->yes()
+				|| (!$this->rememberPossiblyImpureFunctionValues && !$methodReflection->hasSideEffects()->no())
+			) {
+				return true;
+			}
+		}
+
+		if ($expr instanceof StaticCall && $expr->name instanceof Node\Identifier) {
+			if ($expr->class instanceof Name) {
+				$calledOnType = $scope->resolveTypeByName($expr->class);
+			} else {
+				$calledOnType = $scope->getType($expr->class);
+			}
+			$methodReflection = $scope->getMethodReflection($calledOnType, $expr->name->toString());
+			if (
+				$methodReflection === null
+				|| $methodReflection->hasSideEffects()->yes()
+				|| (!$this->rememberPossiblyImpureFunctionValues && !$methodReflection->hasSideEffects()->no())
+			) {
+				return true;
+			}
+		}
+
+		foreach ($expr->getSubNodeNames() as $name) {
+			$subNode = $expr->{$name};
+			if ($subNode instanceof Expr) {
+				if ($this->containsImpureCall($subNode, $scope)) {
+					return true;
+				}
+			}
+			if (!is_array($subNode)) {
+				continue;
+			}
+
+			foreach ($subNode as $item) {
+				if ($item instanceof Node\Arg && $this->containsImpureCall($item->value, $scope)) {
+					return true;
+				}
+				if ($item instanceof Expr && $this->containsImpureCall($item, $scope)) {
+					return true;
+				}
+			}
+		}
+
+		return false;
+	}
+
 	private function createNullsafeTypes(Expr $expr, Scope $scope, TypeSpecifierContext $context, ?Type $type): SpecifiedTypes
 	{
 		if ($expr instanceof Expr\NullsafePropertyFetch) {

tests/PHPStan/Analyser/nsrt/bug-13416.php

@@ -0,0 +1,46 @@
+<?php declare(strict_types = 1);
+
+namespace Bug13416;
+
+use function PHPStan\Testing\assertType;
+
+class MyRecord {
+	/** @var array<int, self> */
+	private static array $storage = [];
+
+	/** @phpstan-impure */
+	public function insert(): void {
+		self::$storage[] = $this;
+	}
+
+	/**
+	 * @return array<int, self>
+	 * @phpstan-impure
+	 */
+	public static function find(): array {
+		return self::$storage;
+	}
+}
+
+class TestCase {
+	public function testMinimalBug(): void {
+		$msg1 = new MyRecord();
+		$msg1->insert();
+
+		assert(
+			count(MyRecord::find()) === 1,
+			'should have 1 record initially'
+		);
+
+		$msg2 = new MyRecord();
+		$msg2->insert();
+
+		assertType('array<int, Bug13416\MyRecord>', MyRecord::find());
+		assertType('int<0, max>', count(MyRecord::find()));
+
+		assert(
+			count(MyRecord::find()) === 2,
+			'should have 2 messages after adding one'
+		);
+	}
+}