RegexArrayShapeMatcher - trailling groups are not optional when PREG_…

…UNMATCHED_AS_NULL
phpstan · Jul 10, 2024 · 66a7578 · 66a7578
1 parent 10ae71d
commit 66a7578
Show file tree

Hide file tree

Showing 5 changed files with 73 additions and 10 deletions.
diff --git a/src/Php/PhpVersion.php b/src/Php/PhpVersion.php
@@ -309,6 +309,13 @@ public function supportsNeverReturnTypeInArrowFunction(): bool
 		return $this->versionId >= 80200;
 	}
 
+	public function supportsPregUnmatchedAsNull(): bool
+	{
+		// while PREG_UNMATCHED_AS_NULL is defined in php-src since 7.2.x it starts working as expected with 7.4.x
+		// https://3v4l.org/v3HE4
+		return $this->versionId >= 70400;
+	}
+
 	public function hasDateTimeExceptions(): bool
 	{
 		return $this->versionId >= 80300;

diff --git a/src/Type/Php/RegexArrayShapeMatcher.php b/src/Type/Php/RegexArrayShapeMatcher.php
@@ -7,6 +7,7 @@
 use Hoa\Compiler\Llk\TreeNode;
 use Hoa\Exception\Exception;
 use Hoa\File\Read;
+use PHPStan\Php\PhpVersion;
 use PHPStan\TrinaryLogic;
 use PHPStan\Type\Constant\ConstantArrayType;
 use PHPStan\Type\Constant\ConstantArrayTypeBuilder;
@@ -33,6 +34,12 @@ final class RegexArrayShapeMatcher
 
 	private static ?Parser $parser = null;
 
+	public function __construct(
+		private PhpVersion $phpVersion,
+	)
+	{
+	}
+
 	public function matchType(Type $patternType, ?Type $flagsType, TrinaryLogic $wasMatched): ?Type
 	{
 		if ($wasMatched->no()) {
@@ -111,6 +118,7 @@ private function matchRegex(string $regex, ?int $flags, TrinaryLogic $wasMatched
 				$valueType,
 				$wasMatched,
 				$trailingOptionals,
+				$flags ?? 0,
 			);
 
 			return TypeCombinator::union(
@@ -145,6 +153,7 @@ private function matchRegex(string $regex, ?int $flags, TrinaryLogic $wasMatched
 					$valueType,
 					$wasMatched,
 					$trailingOptionals,
+					$flags ?? 0,
 				);
 
 				$combiTypes[] = $combiType;
@@ -167,6 +176,7 @@ private function matchRegex(string $regex, ?int $flags, TrinaryLogic $wasMatched
 			$valueType,
 			$wasMatched,
 			$trailingOptionals,
+			$flags ?? 0,
 		);
 	}
 
@@ -228,6 +238,7 @@ private function buildArrayType(
 		Type $valueType,
 		TrinaryLogic $wasMatched,
 		int $trailingOptionals,
+		int $flags,
 	): Type
 	{
 		$builder = ConstantArrayTypeBuilder::createEmpty();
@@ -242,11 +253,18 @@ private function buildArrayType(
 		$countGroups = count($captureGroups);
 		$i = 0;
 		foreach ($captureGroups as $captureGroup) {
+			$groupValueType = $valueType;
+
 			if (!$wasMatched->yes()) {
 				$optional = true;
 			} else {
 				if ($i < $countGroups - $trailingOptionals) {
 					$optional = false;
+					if ($this->containsUnmatchedAsNull($flags)) {
+						$groupValueType = TypeCombinator::removeNull($groupValueType);
+					}
+				} elseif ($this->containsUnmatchedAsNull($flags)) {
+					$optional = false;
 				} else {
 					$optional = $captureGroup->isOptional();
 				}
@@ -255,14 +273,14 @@ private function buildArrayType(
 			if ($captureGroup->isNamed()) {
 				$builder->setOffsetValueType(
 					$this->getKeyType($captureGroup->getName()),
-					$valueType,
+					$groupValueType,
 					$optional,
 				);
 			}
 
 			$builder->setOffsetValueType(
 				$this->getKeyType($i + 1),
-				$valueType,
+				$groupValueType,
 				$optional,
 			);
 
@@ -272,6 +290,11 @@ private function buildArrayType(
 		return $builder->getArray();
 	}
 
+	private function containsUnmatchedAsNull(int $flags): bool
+	{
+		return ($flags & PREG_UNMATCHED_AS_NULL) !== 0 && $this->phpVersion->supportsPregUnmatchedAsNull();
+	}
+
 	private function getKeyType(int|string $key): Type
 	{
 		if (is_string($key)) {
@@ -285,7 +308,7 @@ private function getValueType(int $flags): Type
 	{
 		$valueType = new StringType();
 		$offsetType = IntegerRangeType::fromInterval(0, null);
-		if (($flags & PREG_UNMATCHED_AS_NULL) !== 0) {
+		if ($this->containsUnmatchedAsNull($flags)) {
 			$valueType = TypeCombinator::addNull($valueType);
 			// unmatched groups return -1 as offset
 			$offsetType = IntegerRangeType::fromInterval(-1, null);

diff --git a/tests/PHPStan/Analyser/nsrt/bug-11311-php72.php b/tests/PHPStan/Analyser/nsrt/bug-11311-php72.php
@@ -0,0 +1,21 @@
+<?php // lint < 7.4
+
+namespace Bug11311Php72;
+
+use function PHPStan\Testing\assertType;
+
+// on PHP < 7.4, unmatched-as-null does not return null values; see https://3v4l.org/v3HE4
+
+function doFoo(string $s) {
+	if (1 === preg_match('/(?<major>\d+)\.(?<minor>\d+)(?:\.(?<patch>\d+))?/', $s, $matches, PREG_UNMATCHED_AS_NULL)) {
+		assertType('array{0: string, major: string, 1: string, minor: string, 2: string, patch?: string, 3?: string}', $matches);
+	}
+}
+
+function doUnmatchedAsNull(string $s): void {
+	if (preg_match('/(foo)?(bar)?(baz)?/', $s, $matches, PREG_UNMATCHED_AS_NULL)) {
+		assertType('array{0: string, 1?: string, 2?: string, 3?: string}', $matches);
+	}
+	assertType('array{}|array{0: string, 1?: string, 2?: string, 3?: string}', $matches);
+}
+
diff --git a/tests/PHPStan/Analyser/nsrt/bug-11311.php b/tests/PHPStan/Analyser/nsrt/bug-11311.php
@@ -0,0 +1,19 @@
+<?php // lint >= 7.4
+
+namespace Bug11311;
+
+use function PHPStan\Testing\assertType;
+
+function doFoo(string $s) {
+	if (1 === preg_match('/(?<major>\d+)\.(?<minor>\d+)(?:\.(?<patch>\d+))?/', $s, $matches, PREG_UNMATCHED_AS_NULL)) {
+
+		assertType('array{0: string, major: string, 1: string, minor: string, 2: string, patch: string|null, 3: string|null}', $matches);
+	}
+}
+
+function doUnmatchedAsNull(string $s): void {
+	if (preg_match('/(foo)?(bar)?(baz)?/', $s, $matches, PREG_UNMATCHED_AS_NULL)) {
+		assertType('array{string, string|null, string|null, string|null}', $matches);
+	}
+	assertType('array{}|array{string, string|null, string|null, string|null}', $matches);
+}
diff --git a/tests/PHPStan/Analyser/nsrt/preg_match_shapes.php b/tests/PHPStan/Analyser/nsrt/preg_match_shapes.php
@@ -116,13 +116,6 @@ function doOffsetCapture(string $s): void {
 	assertType('array{}|array{array{string, int<0, max>}, array{string, int<0, max>}, array{string, int<0, max>}, array{string, int<0, max>}}', $matches);
 }
 
-function doUnmatchedAsNull(string $s): void {
-	if (preg_match('/(foo)?(bar)?(baz)?/', $s, $matches, PREG_UNMATCHED_AS_NULL)) {
-		assertType('array{0: string, 1?: string|null, 2?: string|null, 3?: string|null}', $matches);
-	}
-	assertType('array{}|array{0: string, 1?: string|null, 2?: string|null, 3?: string|null}', $matches);
-}
-
 function doUnknownFlags(string $s, int $flags): void {
 	if (preg_match('/(foo)(bar)(baz)/xyz', $s, $matches, $flags)) {
 		assertType('array<array{string|null, int<-1, max>}|string|null>', $matches);