From 399b888474021aee557e54e1c24f1821e69a613b Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Wed, 10 Jul 2024 11:47:37 +0200 Subject: [PATCH 1/6] RegexArrayShapeMatcher - trailling groups are not optional when PREG_UNMATCHED_AS_NULL --- src/Php/PhpVersion.php | 7 +++++++ src/Type/Php/RegexArrayShapeMatcher.php | 15 ++++++++++++++- tests/PHPStan/Analyser/nsrt/bug-11311-php72.php | 12 ++++++++++++ tests/PHPStan/Analyser/nsrt/bug-11311.php | 12 ++++++++++++ 4 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-11311-php72.php create mode 100644 tests/PHPStan/Analyser/nsrt/bug-11311.php diff --git a/src/Php/PhpVersion.php b/src/Php/PhpVersion.php index d666115d0a..0f21c9a549 100644 --- 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 like described in the report 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 index 9feef8b4d9..16d40aa4af 100644 --- 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(); @@ -247,6 +258,8 @@ private function buildArrayType( } else { if ($i < $countGroups - $trailingOptionals) { $optional = false; + } elseif (($flags & PREG_UNMATCHED_AS_NULL) !== 0 && $this->phpVersion->supportsPregUnmatchedAsNull()) { + $optional = false; } else { $optional = $captureGroup->isOptional(); } @@ -285,7 +298,7 @@ private function getValueType(int $flags): Type { $valueType = new StringType(); $offsetType = IntegerRangeType::fromInterval(0, null); - if (($flags & PREG_UNMATCHED_AS_NULL) !== 0) { + if (($flags & PREG_UNMATCHED_AS_NULL) !== 0 && $this->phpVersion->supportsPregUnmatchedAsNull()) { $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 new file mode 100644 index 0000000000..098d703179 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-11311-php72.php @@ -0,0 +1,12 @@ +\d+)\.(?\d+)(?:\.(?\d+))?/', '12.5', $matches, PREG_UNMATCHED_AS_NULL)) { + // on PHP < 7.4, unmatched-as-null does not return null values; see https://3v4l.org/v3HE4 + assertType('array{0: string, major: string, 1: string, minor: string, 2: string, patch?: string, 3?: string}', $matches); + } +} diff --git a/tests/PHPStan/Analyser/nsrt/bug-11311.php b/tests/PHPStan/Analyser/nsrt/bug-11311.php new file mode 100644 index 0000000000..2bb6bcad72 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-11311.php @@ -0,0 +1,12 @@ += 7.4 + +namespace Bug11311; + +use function PHPStan\Testing\assertType; + +function doFoo() { + if (1 === preg_match('/(?\d+)\.(?\d+)(?:\.(?\d+))?/', '12.5', $matches, PREG_UNMATCHED_AS_NULL)) { + + assertType('array{0: string, major: string|null, 1: string|null, minor: string|null, 2: string|null, patch: string|null, 3: string|null}', $matches); + } +} From fd3c3ec6e86456ac69d412b7008f3efae61ad69b Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Wed, 10 Jul 2024 11:50:00 +0200 Subject: [PATCH 2/6] fix test --- tests/PHPStan/Analyser/nsrt/bug-11311-php72.php | 4 ++-- tests/PHPStan/Analyser/nsrt/bug-11311.php | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/PHPStan/Analyser/nsrt/bug-11311-php72.php b/tests/PHPStan/Analyser/nsrt/bug-11311-php72.php index 098d703179..eb0d32c698 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-11311-php72.php +++ b/tests/PHPStan/Analyser/nsrt/bug-11311-php72.php @@ -4,8 +4,8 @@ use function PHPStan\Testing\assertType; -function doFoo() { - if (1 === preg_match('/(?\d+)\.(?\d+)(?:\.(?\d+))?/', '12.5', $matches, PREG_UNMATCHED_AS_NULL)) { +function doFoo(string $s) { + if (1 === preg_match('/(?\d+)\.(?\d+)(?:\.(?\d+))?/', $s, $matches, PREG_UNMATCHED_AS_NULL)) { // on PHP < 7.4, unmatched-as-null does not return null values; see https://3v4l.org/v3HE4 assertType('array{0: string, major: string, 1: string, minor: string, 2: string, patch?: string, 3?: string}', $matches); } diff --git a/tests/PHPStan/Analyser/nsrt/bug-11311.php b/tests/PHPStan/Analyser/nsrt/bug-11311.php index 2bb6bcad72..bb626412f5 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-11311.php +++ b/tests/PHPStan/Analyser/nsrt/bug-11311.php @@ -4,8 +4,8 @@ use function PHPStan\Testing\assertType; -function doFoo() { - if (1 === preg_match('/(?\d+)\.(?\d+)(?:\.(?\d+))?/', '12.5', $matches, PREG_UNMATCHED_AS_NULL)) { +function doFoo(string $s) { + if (1 === preg_match('/(?\d+)\.(?\d+)(?:\.(?\d+))?/', $s, $matches, PREG_UNMATCHED_AS_NULL)) { assertType('array{0: string, major: string|null, 1: string|null, minor: string|null, 2: string|null, patch: string|null, 3: string|null}', $matches); } From 462035032f0ecf389bd3628213734c30824544b7 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Wed, 10 Jul 2024 11:53:30 +0200 Subject: [PATCH 3/6] move tests which got php version sensitive --- tests/PHPStan/Analyser/nsrt/bug-11311-php72.php | 11 ++++++++++- tests/PHPStan/Analyser/nsrt/bug-11311.php | 7 +++++++ tests/PHPStan/Analyser/nsrt/preg_match_shapes.php | 7 ------- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/tests/PHPStan/Analyser/nsrt/bug-11311-php72.php b/tests/PHPStan/Analyser/nsrt/bug-11311-php72.php index eb0d32c698..588b7cd5a1 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-11311-php72.php +++ b/tests/PHPStan/Analyser/nsrt/bug-11311-php72.php @@ -4,9 +4,18 @@ 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('/(?\d+)\.(?\d+)(?:\.(?\d+))?/', $s, $matches, PREG_UNMATCHED_AS_NULL)) { - // on PHP < 7.4, unmatched-as-null does not return null values; see https://3v4l.org/v3HE4 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 index bb626412f5..0536c4a297 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-11311.php +++ b/tests/PHPStan/Analyser/nsrt/bug-11311.php @@ -10,3 +10,10 @@ function doFoo(string $s) { assertType('array{0: string, major: string|null, 1: string|null, minor: string|null, 2: string|null, 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 index 6e82020c4d..6878a77011 100644 --- 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}|string|null>', $matches); From 4adedbac6576dd2fb0cb35c13afc8a5067cc5f6a Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Wed, 10 Jul 2024 11:55:55 +0200 Subject: [PATCH 4/6] fixed namespace collision --- tests/PHPStan/Analyser/nsrt/bug-11311-php72.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/PHPStan/Analyser/nsrt/bug-11311-php72.php b/tests/PHPStan/Analyser/nsrt/bug-11311-php72.php index 588b7cd5a1..5acaade1e6 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-11311-php72.php +++ b/tests/PHPStan/Analyser/nsrt/bug-11311-php72.php @@ -1,6 +1,6 @@ Date: Wed, 10 Jul 2024 12:06:30 +0200 Subject: [PATCH 5/6] non-optional groups cannot be null when PREG_UNMATCHED_AS_NULL --- src/Type/Php/RegexArrayShapeMatcher.php | 18 ++++++++++++++---- tests/PHPStan/Analyser/nsrt/bug-11311.php | 2 +- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/Type/Php/RegexArrayShapeMatcher.php b/src/Type/Php/RegexArrayShapeMatcher.php index 16d40aa4af..794579695f 100644 --- a/src/Type/Php/RegexArrayShapeMatcher.php +++ b/src/Type/Php/RegexArrayShapeMatcher.php @@ -253,12 +253,17 @@ 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; - } elseif (($flags & PREG_UNMATCHED_AS_NULL) !== 0 && $this->phpVersion->supportsPregUnmatchedAsNull()) { + if ($this->containsUnmatchedAsNull($flags)) { + $groupValueType = TypeCombinator::removeNull($groupValueType); + } + } elseif ($this->containsUnmatchedAsNull($flags)) { $optional = false; } else { $optional = $captureGroup->isOptional(); @@ -268,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, ); @@ -285,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)) { @@ -298,7 +308,7 @@ private function getValueType(int $flags): Type { $valueType = new StringType(); $offsetType = IntegerRangeType::fromInterval(0, null); - if (($flags & PREG_UNMATCHED_AS_NULL) !== 0 && $this->phpVersion->supportsPregUnmatchedAsNull()) { + 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.php b/tests/PHPStan/Analyser/nsrt/bug-11311.php index 0536c4a297..a52044feb7 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-11311.php +++ b/tests/PHPStan/Analyser/nsrt/bug-11311.php @@ -7,7 +7,7 @@ function doFoo(string $s) { if (1 === preg_match('/(?\d+)\.(?\d+)(?:\.(?\d+))?/', $s, $matches, PREG_UNMATCHED_AS_NULL)) { - assertType('array{0: string, major: string|null, 1: string|null, minor: string|null, 2: string|null, patch: string|null, 3: string|null}', $matches); + assertType('array{0: string, major: string, 1: string, minor: string, 2: string, patch: string|null, 3: string|null}', $matches); } } From 58018532d37f750a838b1e6d4737543f8b9dc409 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Wed, 10 Jul 2024 12:17:51 +0200 Subject: [PATCH 6/6] fix comment --- src/Php/PhpVersion.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Php/PhpVersion.php b/src/Php/PhpVersion.php index 0f21c9a549..c5afa1e22f 100644 --- a/src/Php/PhpVersion.php +++ b/src/Php/PhpVersion.php @@ -311,7 +311,7 @@ public function supportsNeverReturnTypeInArrowFunction(): bool public function supportsPregUnmatchedAsNull(): bool { - // while PREG_UNMATCHED_AS_NULL is defined in php-src since 7.2.x it starts working like described in the report with 7.4.x + // 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; }