Keep a by-ref writeback's native type only when the written-back type fits it (#6564)

Author: SanderMuller

src/Analyser/NodeScopeResolver.php

@@ -2368,11 +2368,19 @@ public function processArgs(
 						}
 
 						// what the call writes back is described by PHPDoc (@param, @param-out,
-						// a parameter-out extension) - natively only the parameter's own
-						// type declaration is guaranteed
-						$byRefNativeType = $currentParameter instanceof ExtendedParameterReflection
-							? $currentParameter->getNativeType()
-							: $byRefType;
+						// a parameter-out extension) - natively only the parameter's own type
+						// can describe it: its declaration, or for a builtin its signature map
+						// entry. PHP checks a declaration only on the way in and a signature
+						// map entry not at all, so it holds only when the written-back type
+						// fits inside it. preg_match() with PREG_OFFSET_CAPTURE writes arrays
+						// into a slot the signature map declares as string[].
+						$byRefNativeType = $byRefType;
+						if ($currentParameter instanceof ExtendedParameterReflection) {
+							$byRefNativeType = $currentParameter->getNativeType();
+							if (!$byRefNativeType->isSuperTypeOf($byRefType)->yes()) {
+								$byRefNativeType = new MixedType();
+							}
+						}
 
 						$scope = $this->processVirtualAssign(
 							$scope,

tests/PHPStan/Analyser/nsrt/by-ref-writeback-native-type.php

@@ -0,0 +1,41 @@
+<?php declare(strict_types = 1);
+
+namespace ByRefWritebackNativeType;
+
+use function PHPStan\Testing\assertNativeType;
+use function PHPStan\Testing\assertType;
+
+function offsetCapture(string $s): void
+{
+	if (! preg_match('/(a)(b)?/', $s, $m, PREG_OFFSET_CAPTURE)) {
+		return;
+	}
+
+	assertType("array{0: array{non-falsy-string, int<-1, max>}, 1: array{'a', int<-1, max>}, 2?: array{'b', int<-1, max>}}", $m);
+	assertNativeType("array{0: array{non-falsy-string, int<-1, max>}, 1: array{'a', int<-1, max>}, 2?: array{'b', int<-1, max>}}", $m);
+}
+
+function noFlags(string $s): void
+{
+	if (! preg_match('/(a)(b)?/', $s, $m)) {
+		return;
+	}
+
+	assertType("array{0: non-falsy-string, 1: 'a', 2?: 'b'}", $m);
+	assertNativeType("array{0: non-falsy-string, 1: 'a', 2?: 'b'}", $m);
+}
+
+/** @param-out string $v */
+function paramOutContradictsDeclaration(int &$v): void
+{
+	$v = 'now a string';
+}
+
+function userlandParamOut(): void
+{
+	$x = 1;
+	paramOutContradictsDeclaration($x);
+
+	assertType('string', $x);
+	assertNativeType('mixed', $x);
+}

tests/PHPStan/Rules/Variables/IssetRuleTest.php

@@ -671,4 +671,11 @@ public function testBug14416(): void
 		$this->analyse([__DIR__ . '/data/bug-14416.php'], []);
 	}
 
+	public function testPregMatchOffsetCaptureWithoutTreatPhpDocTypesAsCertain(): void
+	{
+		$this->treatPhpDocTypesAsCertain = false;
+
+		$this->analyse([__DIR__ . '/data/preg-match-offset-capture-isset.php'], []);
+	}
+
 }

tests/PHPStan/Rules/Variables/data/preg-match-offset-capture-isset.php

@@ -0,0 +1,12 @@
+<?php declare(strict_types = 1);
+
+namespace PregMatchOffsetCaptureIsset;
+
+function offsetCapture(string $s): ?string
+{
+	if (! preg_match('/(a)(b)?/', $s, $m, PREG_OFFSET_CAPTURE)) {
+		return null;
+	}
+
+	return isset($m[2]) ? $m[2][0] : null;
+}