Invalidate maybe-impure function return values after impure method/static calls

src/Analyser/TypeSpecifier.php

@@ -18,7 +18,9 @@
 use PhpParser\Node\Expr\PropertyFetch;
 use PhpParser\Node\Expr\StaticCall;
 use PhpParser\Node\Expr\StaticPropertyFetch;
+use PhpParser\Node\Identifier;
 use PhpParser\Node\Name;
+use PhpParser\NodeFinder;
 use PHPStan\Analyser\ExprHandler\BooleanAndHandler;
 use PHPStan\DependencyInjection\AutowiredService;
 use PHPStan\Node\Expr\AlwaysRememberedExpr;
@@ -2539,91 +2541,17 @@
 		if (
 			$expr instanceof FuncCall
 			&& $expr->name instanceof Name
+			&& !$this->reflectionProvider->hasFunction($expr->name, $scope)
 		) {
-			$has = $this->reflectionProvider->hasFunction($expr->name, $scope);
-			if (!$has) {
-				// backwards compatibility with previous behaviour
-				return new SpecifiedTypes([], []);
-			}
-
-			$functionReflection = $this->reflectionProvider->getFunction($expr->name, $scope);
-			$hasSideEffects = $functionReflection->hasSideEffects();
-			if ($hasSideEffects->yes()) {
-				return new SpecifiedTypes([], []);
-			}
-
-			if (!$this->rememberPossiblyImpureFunctionValues && !$hasSideEffects->no()) {
-				return new SpecifiedTypes([], []);
-			}
+			return new SpecifiedTypes([], []);
 		}
 
-		if (
-			$expr instanceof FuncCall
-			&& !$expr->name instanceof Name
-		) {
-			$nameType = $scope->getType($expr->name);
-			if ($nameType->isCallable()->yes()) {
-				$isPure = null;
-				foreach ($nameType->getCallableParametersAcceptors($scope) as $variant) {
-					$variantIsPure = $variant->isPure();
-					$isPure = $isPure === null ? $variantIsPure : $isPure->and($variantIsPure);
-				}
-
-				if ($isPure !== null) {
-					if ($isPure->no()) {
-						return new SpecifiedTypes([], []);
-					}
-
-					if (!$this->rememberPossiblyImpureFunctionValues && !$isPure->yes()) {
-						return new SpecifiedTypes([], []);
-					}
-				}
-			}
-		}
-
-		if (
-			$expr instanceof MethodCall
-			&& $expr->name instanceof Node\Identifier
-		) {
-			$methodName = $expr->name->toString();
-			$calledOnType = $scope->getType($expr->var);
-			$methodReflection = $scope->getMethodReflection($calledOnType, $methodName);
-			if (
-				$methodReflection === null
-				|| $methodReflection->hasSideEffects()->yes()
-				|| (!$this->rememberPossiblyImpureFunctionValues && !$methodReflection->hasSideEffects()->no())
-			) {
-				if (isset($containsNull) && !$containsNull) {
-					return $this->createNullsafeTypes($originalExpr, $scope, $context, $type);
-				}
-
-				return new SpecifiedTypes([], []);
-			}
-		}
-
-		if (
-			$expr instanceof StaticCall
-			&& $expr->name instanceof Node\Identifier
-		) {
-			$methodName = $expr->name->toString();
-			if ($expr->class instanceof Name) {
-				$calledOnType = $scope->resolveTypeByName($expr->class);
-			} else {
-				$calledOnType = $scope->getType($expr->class);
+		if (!($expr instanceof AlwaysRememberedExpr) && $this->expressionContainsNonPureCall($expr, $scope)) {
+			if (isset($containsNull) && !$containsNull) {
+				return $this->createNullsafeTypes($originalExpr, $scope, $context, $type);
 			}
 
-			$methodReflection = $scope->getMethodReflection($calledOnType, $methodName);
-			if (
-				$methodReflection === null
-				|| $methodReflection->hasSideEffects()->yes()
-				|| (!$this->rememberPossiblyImpureFunctionValues && !$methodReflection->hasSideEffects()->no())
-			) {
-				if (isset($containsNull) && !$containsNull) {
-					return $this->createNullsafeTypes($originalExpr, $scope, $context, $type);
-				}
-
-				return new SpecifiedTypes([], []);
-			}
+			return new SpecifiedTypes([], []);
 		}
 
 		$sureTypes = [];
@@ -2654,6 +2582,74 @@
 		return $types;
 	}
 
+	private function expressionContainsNonPureCall(Expr $expr, Scope $scope): bool
+	{
+		$nodeFinder = new NodeFinder();
+		$found = $nodeFinder->findFirst([$expr], function (Node $node) use ($scope): bool {
+			if ($node instanceof FuncCall) {
+				if ($node->name instanceof Name) {
+					if (!$this->reflectionProvider->hasFunction($node->name, $scope)) {
+						return false;
+					}
+					$hasSideEffects = $this->reflectionProvider->getFunction($node->name, $scope)->hasSideEffects();
+					return $hasSideEffects->yes()
+						|| (!$this->rememberPossiblyImpureFunctionValues && !$hasSideEffects->no());
+				}
+
+				$nameType = $scope->getType($node->name);
+				if ($nameType->isCallable()->yes()) {
+					$isPure = null;
+					foreach ($nameType->getCallableParametersAcceptors($scope) as $variant) {
+						$variantIsPure = $variant->isPure();
+						$isPure = $isPure === null ? $variantIsPure : $isPure->and($variantIsPure);
+					}
+					if ($isPure !== null) {
+						return $isPure->no()
+							|| (!$this->rememberPossiblyImpureFunctionValues && !$isPure->yes());
+					}
+				}
+
+				return false;
+			}
+
+			if ($node instanceof MethodCall) {
+				if ($node->name instanceof Identifier) {
+					$calledOnType = $scope->getType($node->var);
+					$methodReflection = $scope->getMethodReflection($calledOnType, $node->name->name);
+					if ($methodReflection === null) {
+						return true;
+					}
+					$hasSideEffects = $methodReflection->hasSideEffects();
+					return $hasSideEffects->yes()
+						|| (!$this->rememberPossiblyImpureFunctionValues && !$hasSideEffects->no());
+				}
+				return true;
+			}
+
+			if ($node instanceof StaticCall) {
+				if ($node->name instanceof Identifier) {
+					if ($node->class instanceof Name) {
+						$calledOnType = $scope->resolveTypeByName($node->class);
+					} else {
+						$calledOnType = $scope->getType($node->class);
+					}
+					$methodReflection = $scope->getMethodReflection($calledOnType, $node->name->name);
+					if ($methodReflection === null) {
+						return true;
+					}
+					$hasSideEffects = $methodReflection->hasSideEffects();
+					return $hasSideEffects->yes()
+						|| (!$this->rememberPossiblyImpureFunctionValues && !$hasSideEffects->no());
+				}
+				return true;
+			}
+
+			return false;
+		});
+
+		return $found !== null;
+	}
+
 	private function createNullsafeTypes(Expr $expr, Scope $scope, TypeSpecifierContext $context, ?Type $type): SpecifiedTypes
 	{
 		if ($expr instanceof Expr\NullsafePropertyFetch) {

tests/PHPStan/Analyser/TypeSpecifierTest.php

@@ -26,6 +26,7 @@
 use PHPStan\Type\ArrayType;
 use PHPStan\Type\ClassStringType;
 use PHPStan\Type\Constant\ConstantBooleanType;
+use PHPStan\Type\Constant\ConstantIntegerType;
 use PHPStan\Type\FloatType;
 use PHPStan\Type\Generic\GenericClassStringType;
 use PHPStan\Type\IntegerType;
@@ -1354,6 +1355,22 @@ private static function createInstanceOf(string $className, string $variableName
 		return new Expr\Instanceof_(new Variable($variableName), new Name($className));
 	}
 
+	public function testUnknownFunctionSubExpressionDoesNotPreventNarrowing(): void
+	{
+		$fauxFuncCall = new FuncCall(new Name('FAUX_FUNCTION'), [new Arg(new Variable('foo'))]);
+		$countCall = new FuncCall(new Name('count'), [new Arg($fauxFuncCall)]);
+
+		$specifiedTypes = $this->typeSpecifier->create(
+			$countCall,
+			new ConstantIntegerType(1),
+			TypeSpecifierContext::createTrue(),
+			$this->scope,
+		);
+
+		$result = $this->toReadableResult($specifiedTypes);
+		$this->assertSame(['count(FAUX_FUNCTION($foo))' => '1'], $result);
+	}
+
 	/**
 	 * @param non-empty-string $functionName
 	 */

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

@@ -0,0 +1,147 @@
+<?php
+
+declare(strict_types = 1);
+
+namespace Bug13416;
+
+use function PHPStan\Testing\assertType;
+
+class MyRecord
+{
+	/** @var list<self> */
+	private static array $storage = [];
+
+	/**
+	 * @return list<self>
+	 * @phpstan-impure
+	 */
+	public static function find(): array
+	{
+		return self::$storage;
+	}
+
+	/** @phpstan-impure */
+	public function insert(): void
+	{
+		self::$storage[] = $this;
+	}
+
+	/**
+	 * @return non-empty-string
+	 * @phpstan-impure
+	 */
+	public function getName(): string
+	{
+		return 'test';
+	}
+}
+
+class Repository
+{
+	/**
+	 * @return list<MyRecord>
+	 * @phpstan-impure
+	 */
+	public function findAll(): array
+	{
+		return [];
+	}
+
+	/** @phpstan-impure */
+	public function save(MyRecord $record): void
+	{
+	}
+}
+
+function testImpureStaticCallNotNarrowedByCount(): void
+{
+	assert(count(MyRecord::find()) === 1);
+	// Impure call result should not be narrowed
+	assertType('int<0, max>', count(MyRecord::find()));
+}
+
+function testImpureMethodCallNotNarrowedByCount(): void
+{
+	$repo = new Repository();
+
+	assert(count($repo->findAll()) === 1);
+	// Impure call result should not be narrowed
+	assertType('int<0, max>', count($repo->findAll()));
+}
+
+function testStrlenOfImpureCallNotNarrowed(): void
+{
+	$record = new MyRecord();
+
+	assert(strlen($record->getName()) === 3);
+	// strlen wrapping an impure call should not be narrowed
+	assertType('int<1, max>', strlen($record->getName()));
+}
+
+function testPureFunctionStaysNarrowed(): void
+{
+	/** @var list<int> $arr */
+	$arr = [1];
+	assert(count($arr) === 1);
+	assertType('1', count($arr));
+
+	$x = rand(0, 10);
+
+	// Pure expressions stay narrowed
+	assertType('1', count($arr));
+}
+
+function testImpureArrowFunctionIIFE(): void
+{
+	assert(count((fn() => MyRecord::find())()) === 1);
+	assertType('int<0, max>', count((fn() => MyRecord::find())()));
+}
+
+function testImpureClosureIIFE(): void
+{
+	assert(count((function() { return MyRecord::find(); })()) === 1);
+	assertType('int<0, max>', count((function() { return MyRecord::find(); })()));
+}
+
+function testStrlenOfImpureArrowFunctionIIFE(): void
+{
+	$record = new MyRecord();
+	assert(strlen((fn() => $record->getName())()) === 3);
+	assertType('int<1, max>', strlen((fn() => $record->getName())()));
+}
+
+function testImpureClosureViaVariable(): void
+{
+	$fn = function(): array { return MyRecord::find(); };
+	assert(count($fn()) === 1);
+	assertType('int<0, max>', count($fn()));
+}
+
+function testImpureClosureWithEchoIIFE(): void
+{
+	assert(strlen((function() { echo 'side-effect'; return MyRecord::find()[0]->getName(); })()) === 5);
+	assertType('int<1, max>', strlen((function() { echo 'side-effect'; return MyRecord::find()[0]->getName(); })()));
+}
+
+function testPureClosureIIFEStaysNarrowed(): void
+{
+	/** @var list<int> $arr */
+	$arr = [1, 2, 3];
+	assert(count((fn() => $arr)()) === 3);
+	assertType('3', count((fn() => $arr)()));
+}
+
+/**
+ * @param string|null $val
+ * @phpstan-impure
+ */
+function impureFunction(?string $val): ?string
+{
+	return $val;
+}
+
+function testPureOfImpureNotNarrowedByCoalesce(): void
+{
+	$a = strlen(impureFunction('hello') ?? '') > 0;
+	assertType('bool', strlen(impureFunction('hello') ?? '') > 0);
+}