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

src/Rules/TooWideTypehints/TooWideParameterOutTypeCheck.php

@@ -9,6 +9,7 @@
 use PHPStan\Node\ReturnStatement;
 use PHPStan\Reflection\ExtendedParameterReflection;
 use PHPStan\Rules\IdentifierRuleError;
+use PHPStan\Rules\VariadicByRefParameterOutType;
 use function lcfirst;
 use function sprintf;
 
@@ -94,6 +95,13 @@ private function processSingleParameter(
 		$variableExpr = new Variable($parameter->getName());
 		$variableType = $scope->getType($variableExpr);
 
+		if ($parameter->isVariadic()) {
+			$variableType = VariadicByRefParameterOutType::elementType($variableType);
+			if ($variableType === null) {
+				return [];
+			}
+		}
+
 		return $this->tooWideTypeCheck->checkParameterOutType(
 			$outType,
 			$variableType,

src/Rules/Variables/ParameterOutTypeCheck.php

@@ -11,6 +11,7 @@
 use PHPStan\Rules\IdentifierRuleError;
 use PHPStan\Rules\RuleErrorBuilder;
 use PHPStan\Rules\RuleLevelHelper;
+use PHPStan\Rules\VariadicByRefParameterOutType;
 use PHPStan\Type\ErrorType;
 use PHPStan\Type\Type;
 use PHPStan\Type\VerbosityLevel;
@@ -22,6 +23,9 @@
  * 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 while the variable holds the
+ * packed array of them, so the two sides are reconciled through VariadicByRefParameterOutType.
+ *
  * @internal
  */
 #[AutowiredService]
@@ -47,6 +51,8 @@ public function check(
 		bool $isParamOutType,
 	): array
 	{
+		$isVariadic = $parameter->isVariadic();
+
 		$typeResult = $this->ruleLevelHelper->findTypeToCheck(
 			$scope,
 			$checkedExpr,
@@ -58,6 +64,13 @@ public function check(
 		}
 
 		$assignedExprType = $scope->getType($checkedExpr);
+		if ($isVariadic) {
+			$assignedExprType = VariadicByRefParameterOutType::elementType($assignedExprType);
+			if ($assignedExprType === null) {
+				return [];
+			}
+		}
+
 		if ($outType->isSuperTypeOf($assignedExprType)->yes()) {
 			return [];
 		}

src/Rules/VariadicByRefParameterOutType.php

@@ -0,0 +1,35 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules;
+
+use PHPStan\Type\Type;
+
+/**
+ * The out type of a variadic by-ref parameter describes a single argument: that is how it is applied
+ * at the call site, where NodeScopeResolver writes the out type - or the declared type when there is
+ * no @param-out - back to each argument individually. Inside the body the variable holds the packed
+ * array of those arguments instead, so the element type is the side to compare against the out type.
+ *
+ * @internal
+ */
+final class VariadicByRefParameterOutType
+{
+
+	/**
+	 * Returns the type to compare against the out type, or null when there is nothing to compare.
+	 *
+	 * Null means the variable no longer holds an array. Rebinding the packed variable discards the
+	 * references it held, so PHP writes nothing back to any caller and no out value is left to check.
+	 * An array is still compared, because a write through an offset - `$refs[0] = ...`, which does
+	 * reach the caller - leaves the variable as an array too, and the two are indistinguishable here.
+	 */
+	public static function elementType(Type $packedType): ?Type
+	{
+		if (!$packedType->isArray()->yes()) {
+			return null;
+		}
+
+		return $packedType->getIterableValueType();
+	}
+
+}

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,65 @@
+<?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'];
+}

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;
+}
+