Do not infer by-ref use effects at invocations for a closure assigned to its own by-ref variable

src/Analyser/Generics/ClosureSignatureInference.php

@@ -87,6 +87,8 @@ final class ClosureSignatureInference
 
 	private const ASSIGNED_CLOSURES_ATTRIBUTE = 'closureSignatureAssignedClosures';
 
+	private const NESTED_ASSIGNED_CLOSURES_ATTRIBUTE = 'closureSignatureNestedAssignedClosures';
+
 	private const REBOUND_VARIABLES_ATTRIBUTE = 'closureSignatureReboundVariables';
 
 	public function __construct(
@@ -118,7 +120,7 @@ public static function isByRefMarker(UnresolvedTemplateArgumentType $marker): bo
 	 * by variable name - while observing, and in the second pass for a site the
 	 * observation resolved. Empty for a closure whose invocations cannot be
 	 * followed: a generator (invoking it does not run the body), a closure
-	 * invoking its own by-ref variable.
+	 * invoking its own by-ref variable or assigned to it.
 	 *
 	 * @return array<string, Type>
 	 */
@@ -143,12 +145,19 @@ public function getByRefUseMarkers(MutatingScope $scope, Closure|ArrowFunction $
 			return [];
 		}
 		$reboundNames = self::getReboundVariableNames($body, $frame->getClosureSignatureStmts());
+		$assignedClosures = self::getAssignedClosures($body, $frame->getClosureSignatureStmts(), true);
 		foreach ($names as $name) {
 			// unset or bound to another reference, the variable no longer is the
 			// one the closure writes to - its effects stay the creation-time fixpoint
 			if (isset($reboundNames[$name])) {
 				return [];
 			}
+			// `$f = function () use (&$f)`: the body reaches the closure itself
+			// through the variable - aliased, captured, passed on - so its
+			// invocations cannot all be followed
+			if (in_array($expr, $assignedClosures[$name] ?? [], true)) {
+				return [];
+			}
 		}
 
 		$markers = [];
@@ -1120,13 +1129,17 @@ public function findAssignedClosures(MutatingScope $scope, string $name): array
 	}
 
 	/**
+	 * With $nested, also the closures assigned in the function-likes the body
+	 * contains - to the variables of those.
+	 *
 	 * @param Node\Stmt[] $stmts
 	 * @return array<string, list<Closure|ArrowFunction>>
 	 */
-	private static function getAssignedClosures(Node $body, array $stmts): array
+	private static function getAssignedClosures(Node $body, array $stmts, bool $nested = false): array
 	{
+		$attribute = $nested ? self::NESTED_ASSIGNED_CLOSURES_ATTRIBUTE : self::ASSIGNED_CLOSURES_ATTRIBUTE;
 		/** @var array<string, list<Closure|ArrowFunction>>|null $cached */
-		$cached = $body->getAttribute(self::ASSIGNED_CLOSURES_ATTRIBUTE);
+		$cached = $body->getAttribute($attribute);
 		if ($cached !== null) {
 			return $cached;
 		}
@@ -1135,7 +1148,7 @@ private static function getAssignedClosures(Node $body, array $stmts): array
 		$stack = $stmts;
 		while (count($stack) > 0) {
 			$node = array_pop($stack);
-			if ($node instanceof Node\FunctionLike || $node instanceof Node\Stmt\ClassLike) {
+			if (!$nested && ($node instanceof Node\FunctionLike || $node instanceof Node\Stmt\ClassLike)) {
 				continue;
 			}
 			if (
@@ -1160,7 +1173,7 @@ private static function getAssignedClosures(Node $body, array $stmts): array
 				}
 			}
 		}
-		$body->setAttribute(self::ASSIGNED_CLOSURES_ATTRIBUTE, $assigned);
+		$body->setAttribute($attribute, $assigned);
 
 		return $assigned;
 	}

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

@@ -0,0 +1,113 @@
+<?php // lint >= 7.4
+
+declare(strict_types=1);
+
+namespace Bug15442;
+
+use function PHPStan\Testing\assertType;
+
+class GridLayout
+{
+	protected function buildTemplate(): void
+	{
+		$fx = static function () use (&$fx) {
+			(static function () use ($fx) {
+				assertType('static-Closure(): void', $fx);
+				$fx();
+
+				dummy(); // @phpstan-ignore-line
+			})();
+		};
+
+		dummy($fx); // @phpstan-ignore-line
+	}
+
+	public function nestedArrowFunction(): void
+	{
+		$fx = static function () use (&$fx) {
+			(static fn () => $fx())();
+			assertType('static-Closure(): void', $fx);
+		};
+	}
+
+	public function nestedByRefCapture(): void
+	{
+		$fx = static function () use (&$fx) {
+			(static function () use (&$fx) {
+				assertType('static-Closure(): void', $fx);
+				$fx();
+			})();
+		};
+	}
+
+	public function callUserFunc(): void
+	{
+		$fx = static function () use (&$fx) {
+			assertType('static-Closure(): void', $fx);
+			call_user_func($fx);
+		};
+	}
+
+	public function invokeMethod(): void
+	{
+		$fx = static function () use (&$fx) {
+			assertType('static-Closure(): void', $fx);
+			$fx->__invoke();
+		};
+	}
+
+	public function alias(): void
+	{
+		$fx = static function () use (&$fx) {
+			$alias = $fx;
+			assertType('static-Closure(): void', $alias);
+			$alias();
+		};
+	}
+
+	/** @param list<int> $a */
+	public function passedAsCallback(array $a): void
+	{
+		$fx = static function (array $a) use (&$fx): void {
+			assertType('static-Closure(array): void', $fx);
+			array_map($fx, $a);
+		};
+		$fx($a);
+	}
+
+	public function readOnly(): void
+	{
+		$fx = static function () use (&$fx) {
+			assertType('static-Closure(): void', $fx);
+		};
+		$fx();
+	}
+
+	public function insideClosure(): void
+	{
+		$outer = function (): void {
+			$fx = static function () use (&$fx) {
+				(static function () use ($fx) {
+					assertType('static-Closure(): void', $fx);
+					$fx();
+				})();
+			};
+			$fx();
+		};
+		$outer();
+	}
+
+	public function otherByRefUseStillFollowed(): void
+	{
+		$x = 1;
+		$c = function () use (&$x): void {
+			assertType('1', $x);
+			(function () use ($x): void {
+				assertType('1', $x);
+			})();
+			$x = 'a';
+		};
+		$c();
+		assertType("'a'", $x);
+	}
+}

tests/PHPStan/Rules/Functions/CallCallablesRuleTest.php

@@ -461,6 +461,12 @@ public function testBug15285(): void
 		$this->analyse([__DIR__ . '/data/bug-15285.php'], []);
 	}
 
+	#[RequiresPhp('>= 7.4.0')]
+	public function testBug15442(): void
+	{
+		$this->analyse([__DIR__ . '/data/bug-15442.php'], []);
+	}
+
 	#[RequiresPhp('>= 8.0.0')]
 	public function testBug11935WithCheckExplicitMixed(): void
 	{

tests/PHPStan/Rules/Functions/data/bug-15442.php

@@ -0,0 +1,30 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Bug15442Rule;
+
+class GridLayout
+{
+    protected function buildTemplate(): void
+    {
+		$fx = static function () use (&$fx) {
+			(static function () use ($fx) {
+				$fx();
+
+				dummy();
+			})();
+		};
+
+		dummy($fx);
+    }
+
+    protected function arrowFunction(): void
+    {
+		$fx = static function () use (&$fx) {
+			(static fn () => $fx())();
+		};
+
+		dummy($fx);
+    }
+}

turbo-ext/src/ClosureSignatureInference.cpp

@@ -30,6 +30,7 @@ zend_string *pt_csi_return_template_name = nullptr;
 zend_string *pt_csi_closed_body_attribute = nullptr;
 zend_string *pt_csi_returns_context_typed_attribute = nullptr;
 zend_string *pt_csi_assigned_closures_attribute = nullptr;
+zend_string *pt_csi_nested_assigned_closures_attribute = nullptr;
 /* the by-ref follow-up's template names and attribute names */
 zend_string *pt_csi_invocation_template_name = nullptr;
 zend_string *pt_csi_by_ref_uses_attribute = nullptr;
@@ -230,9 +231,10 @@ void containsMarkerBody(zval *contains, zval *state1, uint32_t argc, zval *argv,
 }
 
 /* Mirrors the private static getAssignedClosures(); UNDEF = pending exception */
-zv::Val getAssignedClosures(zval *body, zval *stmts)
+zv::Val getAssignedClosures(zval *body, zval *stmts, bool nested = false)
 {
-	zv::Val cached = pt_engine_node_get_attribute(Z_OBJ_P(body), ZSTR_VAL(pt_csi_assigned_closures_attribute), ZSTR_LEN(pt_csi_assigned_closures_attribute));
+	zend_string *attribute = nested ? pt_csi_nested_assigned_closures_attribute : pt_csi_assigned_closures_attribute;
+	zv::Val cached = pt_engine_node_get_attribute(Z_OBJ_P(body), ZSTR_VAL(attribute), ZSTR_LEN(attribute));
 	if (UNEXPECTED(cached.isUndef())) return zv::Val();
 	if (!cached.isNull()) return cached;
 
@@ -246,15 +248,17 @@ zv::Val getAssignedClosures(zval *body, zval *stmts)
 	for (;;) {
 		zv::Val node = popNode(stack);
 		if (node.isUndef()) break;
-		bool skip;
-		if (UNEXPECTED(!isA(node.raw(), PT_CLASS_FUNCTION_LIKE, skip))) return zv::Val();
-		if (!skip && UNEXPECTED(!isA(node.raw(), PT_CLASS_CLASS_LIKE_STMT, skip))) return zv::Val();
-		if (skip) continue;
+		if (!nested) {
+			bool skip;
+			if (UNEXPECTED(!isA(node.raw(), PT_CLASS_FUNCTION_LIKE, skip))) return zv::Val();
+			if (!skip && UNEXPECTED(!isA(node.raw(), PT_CLASS_CLASS_LIKE_STMT, skip))) return zv::Val();
+			if (skip) continue;
+		}
 		if (UNEXPECTED(!collectAssignedClosure(node.raw(), assigned))) return zv::Val();
 		if (UNEXPECTED(!pushSubNodes(node.raw(), stack))) return zv::Val();
 	}
 	zv::Val result(std::move(assigned));
-	if (UNEXPECTED(!pt_engine_node_set_attribute(Z_OBJ_P(body), ZSTR_VAL(pt_csi_assigned_closures_attribute), ZSTR_LEN(pt_csi_assigned_closures_attribute), result.raw()))) return zv::Val();
+	if (UNEXPECTED(!pt_engine_node_set_attribute(Z_OBJ_P(body), ZSTR_VAL(attribute), ZSTR_LEN(attribute), result.raw()))) return zv::Val();
 	return result;
 }
 
@@ -1007,13 +1011,28 @@ class ClosureSignatureInference
 		if (UNEXPECTED(stmts.isUndef())) return zv::Val();
 		zv::Val reboundNames = getReboundVariableNames(body.raw(), stmts.raw());
 		if (UNEXPECTED(reboundNames.isUndef())) return zv::Val();
+		zv::Val assignedClosures = getAssignedClosures(body.raw(), stmts.raw(), true);
+		if (UNEXPECTED(assignedClosures.isUndef())) return zv::Val();
 		for (auto entry : zv::ArrRef(names.raw())) {
 			zval *name = entry.value().deref().raw();
+			if (Z_TYPE_P(name) != IS_STRING) continue;
 			// unset or bound to another reference, the variable no longer is the
 			// one the closure writes to - its effects stay the creation-time fixpoint
-			if (Z_TYPE_P(name) == IS_STRING && zend_symtable_find(Z_ARRVAL_P(reboundNames.raw()), Z_STR_P(name)) != NULL) {
+			if (zend_symtable_find(Z_ARRVAL_P(reboundNames.raw()), Z_STR_P(name)) != NULL) {
 				return zv::Val(zv::Arr::empty());
 			}
+			// `$f = function () use (&$f)`: the body reaches the closure itself
+			// through the variable - aliased, captured, passed on - so its
+			// invocations cannot all be followed
+			zval *closures = zend_symtable_find(Z_ARRVAL_P(assignedClosures.raw()), Z_STR_P(name));
+			if (closures != NULL && Z_TYPE_P(closures) == IS_ARRAY) {
+				for (auto closure : zv::ArrRef(closures)) {
+					zval *candidate = closure.value().deref().raw();
+					if (Z_TYPE_P(candidate) == IS_OBJECT && Z_OBJ_P(candidate) == Z_OBJ_P(expr)) {
+						return zv::Val(zv::Arr::empty());
+					}
+				}
+			}
 		}
 
 		zv::Arr markers = zv::Arr::empty();
@@ -1917,6 +1936,7 @@ PT_MINIT_REGISTRATION(pt_register_closure_signature_inference)
 	pt_csi_closed_body_attribute = zend_string_init_interned(PT_LC("closureSignatureClosedBody"), 1);
 	pt_csi_returns_context_typed_attribute = zend_string_init_interned(PT_LC("closureSignatureReturnsContextTyped"), 1);
 	pt_csi_assigned_closures_attribute = zend_string_init_interned(PT_LC("closureSignatureAssignedClosures"), 1);
+	pt_csi_nested_assigned_closures_attribute = zend_string_init_interned(PT_LC("closureSignatureNestedAssignedClosures"), 1);
 	pt_csi_invocation_template_name = zend_string_init_interned(PT_LC("@invokes"), 1);
 	pt_csi_by_ref_uses_attribute = zend_string_init_interned(PT_LC("closureSignatureByRefUses"), 1);
 	pt_csi_arrow_function_outer_variables_attribute = zend_string_init_interned(PT_LC("closureSignatureArrowFunctionOuterVariables"), 1);
@@ -1933,6 +1953,7 @@ PT_MINIT_REGISTRATION(pt_register_closure_signature_inference)
 	cls.privateClassConstantString("ARROW_FUNCTION_OUTER_VARIABLES_ATTRIBUTE", "closureSignatureArrowFunctionOuterVariables");
 	cls.privateClassConstantString("CLOSED_BODY_ATTRIBUTE", "closureSignatureClosedBody");
 	cls.privateClassConstantString("ASSIGNED_CLOSURES_ATTRIBUTE", "closureSignatureAssignedClosures");
+	cls.privateClassConstantString("NESTED_ASSIGNED_CLOSURES_ATTRIBUTE", "closureSignatureNestedAssignedClosures");
 	cls.privateClassConstantString("REBOUND_VARIABLES_ATTRIBUTE", "closureSignatureReboundVariables");
 
 	/* the real parameter types: the DI container autowires the service by

turbo-ext/src/generated/ClosureSignatureInference.h

@@ -104,9 +104,11 @@ inline constexpr char strings[] =
 	"scanClosedBody\0" /* 1340 */
 	"infersInvocationReturnType\0" /* 1355 */
 	"findAssignedClosures\0" /* 1382 */
-	"getAssignedClosures\0" /* 1403 */
-	"collectTargetNames\0" /* 1423 */
-	"isContextTyped"; /* 1442 */
+	"nested\0" /* 1403 */
+	"false\0" /* 1410 */
+	"getAssignedClosures\0" /* 1416 */
+	"collectTargetNames\0" /* 1436 */
+	"isContextTyped"; /* 1455 */
 inline constexpr reg::PackedArg args[] = {
 	reg::packed(0, MAY_BE_BOOL), /* __construct $enabled */
 	reg::packed(20, 0, 27), /* isClosureSignatureMarker $marker */
@@ -212,6 +214,7 @@ inline constexpr reg::PackedArg args[] = {
 	reg::packed(79, MAY_BE_ARRAY), /* findAssignedClosures return */
 	reg::packed(868, 0, 873), /* getAssignedClosures $body */
 	reg::packed(836, MAY_BE_ARRAY), /* getAssignedClosures $stmts */
+	reg::packed(1403, MAY_BE_BOOL, reg::NoString, false, false, 1410), /* getAssignedClosures $nested */
 	reg::packed(79, MAY_BE_ARRAY), /* getAssignedClosures return */
 	reg::packed(937, 0, 649), /* collectTargetNames $target */
 	reg::packed(842, MAY_BE_ARRAY, reg::NoString, true, false), /* collectTargetNames $names */
@@ -262,9 +265,9 @@ inline constexpr sigtab::Sig isClosedBody = { { 1327 /* isClosedBody */, 2, 90,
 inline constexpr sigtab::Sig scanClosedBody = { { 1340 /* scanClosedBody */, 2, 93, 2, 95, ZEND_ACC_PRIVATE | ZEND_ACC_STATIC } };
 inline constexpr sigtab::Sig infersInvocationReturnType = { { 1355 /* infersInvocationReturnType */, 2, 96, 2, 98, ZEND_ACC_PUBLIC } };
 inline constexpr sigtab::Sig findAssignedClosures = { { 1382 /* findAssignedClosures */, 2, 99, 2, 101, ZEND_ACC_PUBLIC } };
-inline constexpr sigtab::Sig getAssignedClosures = { { 1403 /* getAssignedClosures */, 2, 102, 2, 104, ZEND_ACC_PRIVATE | ZEND_ACC_STATIC } };
-inline constexpr sigtab::Sig collectTargetNames = { { 1423 /* collectTargetNames */, 2, 105, 2, 107, ZEND_ACC_PRIVATE | ZEND_ACC_STATIC } };
-inline constexpr sigtab::Sig isContextTyped = { { 1442 /* isContextTyped */, 1, 108, 1, 109, ZEND_ACC_PRIVATE | ZEND_ACC_STATIC } };
+inline constexpr sigtab::Sig getAssignedClosures = { { 1416 /* getAssignedClosures */, 2, 102, 3, 105, ZEND_ACC_PRIVATE | ZEND_ACC_STATIC } };
+inline constexpr sigtab::Sig collectTargetNames = { { 1436 /* collectTargetNames */, 2, 106, 2, 108, ZEND_ACC_PRIVATE | ZEND_ACC_STATIC } };
+inline constexpr sigtab::Sig isContextTyped = { { 1455 /* isContextTyped */, 1, 109, 1, 110, ZEND_ACC_PRIVATE | ZEND_ACC_STATIC } };
 } // namespace sig
 
 } // namespace ptdecl::ClosureSignatureInference