Compare the element type of a variadic by-ref parameter against its out type

src/Rules/TooWideTypehints/TooWideParameterOutTypeCheck.php

@@ -94,6 +94,14 @@ private function processSingleParameter(
 		$variableExpr = new Variable($parameter->getName());
 		$variableType = $scope->getType($variableExpr);
 
+		// a variadic out type describes one argument - see ParameterOutTypeCheck
+		if ($parameter->isVariadic()) {
+			if (!$variableType->isArray()->yes()) {
+				return [];
+			}
+			$variableType = $variableType->getIterableValueType();
+		}
+
 		return $this->tooWideTypeCheck->checkParameterOutType(
 			$outType,
 			$variableType,

src/Rules/Variables/ParameterOutTypeCheck.php

@@ -22,6 +22,12 @@
  * The promise is either an explicit `@param-out` or, in its absence, the parameter's own type.
  * Which one it is only shows in the error message, so callers report it via $isParamOutType.
  *
+ * For a variadic parameter the promise describes a single argument, as NodeScopeResolver applies it at
+ * the call site, while the variable holds the packed array of them, so its element type is compared.
+ * Once the variable no longer holds an array, rebinding it has discarded the references and nothing
+ * reaches a caller. A write through an offset, `$refs[0] = ...`, does reach the caller and leaves an
+ * array, so an array is always compared.
+ *
  * @internal
  */
 #[AutowiredService]
@@ -47,6 +53,8 @@ public function check(
 		bool $isParamOutType,
 	): array
 	{
+		$isVariadic = $parameter->isVariadic();
+
 		$typeResult = $this->ruleLevelHelper->findTypeToCheck(
 			$scope,
 			$checkedExpr,
@@ -58,6 +66,13 @@ public function check(
 		}
 
 		$assignedExprType = $scope->getType($checkedExpr);
+		if ($isVariadic) {
+			if (!$assignedExprType->isArray()->yes()) {
+				return [];
+			}
+			$assignedExprType = $assignedExprType->getIterableValueType();
+		}
+
 		if ($outType->isSuperTypeOf($assignedExprType)->yes()) {
 			return [];
 		}

tests/PHPStan/Rules/TooWideTypehints/TooWideFunctionParameterOutTypeRuleTest.php

@@ -5,6 +5,7 @@
 use PHPStan\Rules\Properties\PropertyReflectionFinder;
 use PHPStan\Rules\Rule as TRule;
 use PHPStan\Testing\RuleTestCase;
+use PHPUnit\Framework\Attributes\RequiresPhp;
 
 /**
  * @extends RuleTestCase<TooWideFunctionParameterOutTypeRule>
@@ -50,4 +51,22 @@ public function testNestedTooWideType(): void
 		]);
 	}
 
+	#[RequiresPhp('>= 8.0.0')]
+	public function testBug15066(): void
+	{
+		$this->analyse([__DIR__ . '/data/bug-15066.php'], [
+			[
+				'Function Bug15066\\variadicNeverNull() never assigns null to &$refs so it can be removed from the by-ref type.',
+				24,
+				'You can narrow the parameter out type with @param-out PHPDoc tag.',
+			],
+			// rebinding the packed variable to a non-array is silent, to an array is not - see the fixture
+			[
+				'Function Bug15066\\variadicRebindOnlyString() never assigns null to &$refs so it can be removed from the by-ref type.',
+				62,
+				'You can narrow the parameter out type with @param-out PHPDoc tag.',
+			],
+		]);
+	}
+
 }

tests/PHPStan/Rules/TooWideTypehints/TooWideMethodParameterOutTypeRuleTest.php

@@ -5,6 +5,7 @@
 use PHPStan\Rules\Properties\PropertyReflectionFinder;
 use PHPStan\Rules\Rule as TRule;
 use PHPStan\Testing\RuleTestCase;
+use PHPUnit\Framework\Attributes\RequiresPhp;
 
 /**
  * @extends RuleTestCase<TooWideMethodParameterOutTypeRule>
@@ -144,4 +145,16 @@ public function testNestedTooWideType(): void
 		]);
 	}
 
+	#[RequiresPhp('>= 8.0.0')]
+	public function testBug15066(): void
+	{
+		$this->analyse([__DIR__ . '/data/bug-15066.php'], [
+			[
+				'Method Bug15066\\Foo::variadicNeverNull() never assigns null to &$refs so it can be removed from the by-ref type.',
+				45,
+				'You can narrow the parameter out type with @param-out PHPDoc tag.',
+			],
+		]);
+	}
+
 }

tests/PHPStan/Rules/TooWideTypehints/data/bug-15066.php

@@ -0,0 +1,78 @@
+<?php declare(strict_types = 1); // lint >= 8.0
+
+namespace Bug15066;
+
+function variadicByRef(string|null &...$refs): void
+{
+	foreach ($refs as &$ref) {
+		$ref = $ref === null ? null : trim($ref);
+	}
+}
+
+function singleByRef(string|null &$ref): void
+{
+	$ref = $ref === null ? null : trim($ref);
+}
+
+function variadicByIndex(string|null &...$refs): void
+{
+	foreach ($refs as $key => $value) {
+		$refs[$key] = $value === null ? null : trim($value);
+	}
+}
+
+function variadicNeverNull(string|null &...$refs): void
+{
+	foreach ($refs as $key => $value) {
+		$refs[$key] = 'foo';
+	}
+}
+
+function variadicNeverWritten(string|null &...$refs): void
+{
+}
+
+class Foo
+{
+
+	public function variadicByIndex(string|null &...$refs): void
+	{
+		foreach ($refs as $key => $value) {
+			$refs[$key] = $value === null ? null : trim($value);
+		}
+	}
+
+	public function variadicNeverNull(string|null &...$refs): void
+	{
+		foreach ($refs as $key => $value) {
+			$refs[$key] = 'foo';
+		}
+	}
+
+}
+
+// Rebinding the packed variable discards the references it held, so PHP writes nothing back and
+// there is no out value left to check. An array is still compared, because a write through an
+// offset reaches the caller and leaves the variable as an array too.
+function variadicRebindNonArray(string|null &...$refs): void
+{
+	$refs = 42;
+}
+
+function variadicRebindOnlyString(string|null &...$refs): void
+{
+	$refs = ['ok'];
+}
+
+// An iterable that is not an array still has an element type, but rebinding to it discards the
+// references just the same, so it stays silent too.
+function variadicRebindTraversable(string|null &...$refs): void
+{
+	$refs = new \ArrayIterator(['ok']);
+}
+
+// Maybe an array is not compared either, even when the other types also have an element type.
+function variadicRebindMaybeArray(string|null &...$refs): void
+{
+	$refs = rand(0, 1) === 1 ? ['ok'] : new \ArrayIterator(['ok']);
+}

tests/PHPStan/Rules/Variables/ParameterOutAssignedTypeRuleTest.php

@@ -117,4 +117,27 @@ public function testCatchVariable(): void
 		]);
 	}
 
+	#[RequiresPhp('>= 8.0.0')]
+	public function testBug15066(): void
+	{
+		$this->analyse([__DIR__ . '/data/bug-15066.php'], [
+			[
+				'Parameter &$refs by-ref type of function Bug15066Variables\\variadicWrongType() expects string|null, int|string|null given.',
+				22,
+				'You can change the parameter out type with @param-out PHPDoc tag.',
+			],
+			[
+				'Parameter &$refs by-ref type of method Bug15066Variables\\Foo::variadicWrongType() expects string|null, int|string|null given.',
+				52,
+				'You can change the parameter out type with @param-out PHPDoc tag.',
+			],
+			// rebinding the packed variable to a non-array is silent, to an array is not - see the fixture
+			[
+				'Parameter &$refs by-ref type of function Bug15066Variables\\variadicRebindWrongArray() expects string|null, int given.',
+				69,
+				'You can change the parameter out type with @param-out PHPDoc tag.',
+			],
+		]);
+	}
+
 }

tests/PHPStan/Rules/Variables/ParameterOutExecutionEndTypeRuleTest.php

@@ -5,6 +5,7 @@
 use PHPStan\Rules\Rule;
 use PHPStan\Rules\RuleLevelHelper;
 use PHPStan\Testing\RuleTestCase;
+use PHPUnit\Framework\Attributes\RequiresPhp;
 
 /**
  * @extends RuleTestCase<ParameterOutExecutionEndTypeRule>
@@ -74,4 +75,15 @@ public function testBug12330(): void
 		$this->analyse([__DIR__ . '/data/bug-12330.php'], []);
 	}
 
+	#[RequiresPhp('>= 8.0.0')]
+	public function testBug15066(): void
+	{
+		$this->analyse([__DIR__ . '/data/bug-15066.php'], [
+			[
+				'Parameter &$refs @param-out type of function Bug15066Variables\\variadicParamOutNeverWritten() expects string, string|null given.',
+				35,
+			],
+		]);
+	}
+
 }

tests/PHPStan/Rules/Variables/data/bug-15066.php

@@ -0,0 +1,83 @@
+<?php declare(strict_types = 1); // lint >= 8.0
+
+namespace Bug15066Variables;
+
+function variadicByRef(string|null &...$refs): void
+{
+	foreach ($refs as &$ref) {
+		$ref = $ref === null ? null : trim($ref);
+	}
+}
+
+function variadicByIndex(string|null &...$refs): void
+{
+	foreach ($refs as $key => $value) {
+		$refs[$key] = $value === null ? null : trim($value);
+	}
+}
+
+function variadicWrongType(string|null &...$refs): void
+{
+	foreach ($refs as $key => $value) {
+		$refs[$key] = 42;
+	}
+}
+
+/** @param-out string|null $refs */
+function variadicParamOut(string|null &...$refs): void
+{
+	foreach ($refs as $key => $value) {
+		$refs[$key] = $value === null ? null : trim($value);
+	}
+}
+
+/** @param-out string $refs */
+function variadicParamOutNeverWritten(string|null &...$refs): void
+{
+}
+
+class Foo
+{
+
+	public function variadicByIndex(string|null &...$refs): void
+	{
+		foreach ($refs as $key => $value) {
+			$refs[$key] = $value === null ? null : trim($value);
+		}
+	}
+
+	public function variadicWrongType(string|null &...$refs): void
+	{
+		foreach ($refs as $key => $value) {
+			$refs[$key] = 42;
+		}
+	}
+
+}
+
+// Rebinding the packed variable discards the references it held, so PHP writes nothing back to any
+// caller. Nothing is reported for a non-array, because no out value is left to check. An array is
+// still reported: a write through an offset reaches the caller and leaves the variable as an array
+// too, so the two cannot be told apart here, and the offset write is the case that matters.
+function variadicRebindNonArray(string|null &...$refs): void
+{
+	$refs = 42;
+}
+
+function variadicRebindWrongArray(string|null &...$refs): void
+{
+	$refs = [42];
+}
+
+function variadicRebindOkArray(string|null &...$refs): void
+{
+	$refs = ['ok'];
+}
+
+// The packed variable may end up only maybe holding an array. There is then no element type to
+// speak of, so the comparison is skipped rather than run against a nonexistent one.
+function variadicRebindMaybeArray(string|null &...$refs): void
+{
+	$refs = rand(0, 1) === 1 ? [42] : null;
+}
+