From a72ae08442348fa9ecb0afacb417c035161ab556 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Thu, 11 Jul 2024 18:16:21 +0200 Subject: [PATCH 1/5] RegexArrayShapeMatcher - Fix optional groups with PREG_UNMATCHED_AS_NULL --- src/Type/Php/RegexArrayShapeMatcher.php | 11 +++++++---- tests/PHPStan/Analyser/nsrt/preg_match_shapes.php | 11 +++++++++++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/Type/Php/RegexArrayShapeMatcher.php b/src/Type/Php/RegexArrayShapeMatcher.php index 794579695f..0facf36b1a 100644 --- a/src/Type/Php/RegexArrayShapeMatcher.php +++ b/src/Type/Php/RegexArrayShapeMatcher.php @@ -121,10 +121,13 @@ private function matchRegex(string $regex, ?int $flags, TrinaryLogic $wasMatched $flags ?? 0, ); - return TypeCombinator::union( - new ConstantArrayType([new ConstantIntegerType(0)], [new StringType()], [0], [], true), - $combiType, - ); + if (!$this->containsUnmatchedAsNull($flags ?? 0)) { + $combiType = TypeCombinator::union( + new ConstantArrayType([new ConstantIntegerType(0)], [new StringType()], [0], [], true), + $combiType, + ); + } + return $combiType; } elseif ( $wasMatched->yes() && $onlyTopLevelAlternationId !== null diff --git a/tests/PHPStan/Analyser/nsrt/preg_match_shapes.php b/tests/PHPStan/Analyser/nsrt/preg_match_shapes.php index 768787194d..b31aaeb2f2 100644 --- a/tests/PHPStan/Analyser/nsrt/preg_match_shapes.php +++ b/tests/PHPStan/Analyser/nsrt/preg_match_shapes.php @@ -353,6 +353,17 @@ function bug11277b(string $value): void } } +// see https://3v4l.org/VeDob +function unmatchedAsNullWithOptionalGroup(string $s): void { + if (preg_match('/Price: (£|€)?\d+/', $s, $matches, PREG_UNMATCHED_AS_NULL)) { + // with PREG_UNMATCHED_AS_NULL the offset 1 will always exist. It is correct that it's nullable because it's optional though + assertType('array{string, string|null}', $matches); + } else { + assertType('array{}', $matches); + } + assertType('array{}|array{string, string|null}', $matches); +} + // https://www.pcre.org/current/doc/html/pcre2pattern.html#dupgroupnumber // https://3v4l.org/09qdT function bug11291(string $s): void { From ccc0ae221d2068e19c2a553ecdacb12bf7a190a0 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Thu, 11 Jul 2024 18:26:41 +0200 Subject: [PATCH 2/5] separate test expectations --- tests/PHPStan/Analyser/nsrt/bug-11311-php72.php | 9 +++++++++ tests/PHPStan/Analyser/nsrt/bug-11311.php | 12 ++++++++++++ tests/PHPStan/Analyser/nsrt/preg_match_shapes.php | 11 ----------- 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/tests/PHPStan/Analyser/nsrt/bug-11311-php72.php b/tests/PHPStan/Analyser/nsrt/bug-11311-php72.php index 5acaade1e6..6e768b5c01 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-11311-php72.php +++ b/tests/PHPStan/Analyser/nsrt/bug-11311-php72.php @@ -19,3 +19,12 @@ function doUnmatchedAsNull(string $s): void { assertType('array{}|array{0: string, 1?: string, 2?: string, 3?: string}', $matches); } +// see https://3v4l.org/VeDob#veol +function unmatchedAsNullWithOptionalGroup(string $s): void { + if (preg_match('/Price: (£|€)?\d+/', $s, $matches, PREG_UNMATCHED_AS_NULL)) { + assertType('array{0: string, 1?: string}', $matches); + } else { + assertType('array{}', $matches); + } + assertType('array{}|array{0: string, 1?: string}', $matches); +} diff --git a/tests/PHPStan/Analyser/nsrt/bug-11311.php b/tests/PHPStan/Analyser/nsrt/bug-11311.php index a52044feb7..8db253bf7d 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-11311.php +++ b/tests/PHPStan/Analyser/nsrt/bug-11311.php @@ -17,3 +17,15 @@ function doUnmatchedAsNull(string $s): void { } assertType('array{}|array{string, string|null, string|null, string|null}', $matches); } + +// see https://3v4l.org/VeDob +function unmatchedAsNullWithOptionalGroup(string $s): void { + if (preg_match('/Price: (£|€)?\d+/', $s, $matches, PREG_UNMATCHED_AS_NULL)) { + // with PREG_UNMATCHED_AS_NULL the offset 1 will always exist. It is correct that it's nullable because it's optional though + assertType('array{string, string|null}', $matches); + } else { + assertType('array{}', $matches); + } + assertType('array{}|array{string, string|null}', $matches); +} + diff --git a/tests/PHPStan/Analyser/nsrt/preg_match_shapes.php b/tests/PHPStan/Analyser/nsrt/preg_match_shapes.php index b31aaeb2f2..768787194d 100644 --- a/tests/PHPStan/Analyser/nsrt/preg_match_shapes.php +++ b/tests/PHPStan/Analyser/nsrt/preg_match_shapes.php @@ -353,17 +353,6 @@ function bug11277b(string $value): void } } -// see https://3v4l.org/VeDob -function unmatchedAsNullWithOptionalGroup(string $s): void { - if (preg_match('/Price: (£|€)?\d+/', $s, $matches, PREG_UNMATCHED_AS_NULL)) { - // with PREG_UNMATCHED_AS_NULL the offset 1 will always exist. It is correct that it's nullable because it's optional though - assertType('array{string, string|null}', $matches); - } else { - assertType('array{}', $matches); - } - assertType('array{}|array{string, string|null}', $matches); -} - // https://www.pcre.org/current/doc/html/pcre2pattern.html#dupgroupnumber // https://3v4l.org/09qdT function bug11291(string $s): void { From 64ab476d3c2bf75e35a55e384179bf314b1e00de Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Thu, 11 Jul 2024 19:54:12 +0200 Subject: [PATCH 3/5] add test with mandatory group --- .../Analyser/nsrt/preg_match_shapes.php | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/tests/PHPStan/Analyser/nsrt/preg_match_shapes.php b/tests/PHPStan/Analyser/nsrt/preg_match_shapes.php index 768787194d..f386bf99ba 100644 --- a/tests/PHPStan/Analyser/nsrt/preg_match_shapes.php +++ b/tests/PHPStan/Analyser/nsrt/preg_match_shapes.php @@ -93,9 +93,10 @@ function doNonCapturingGroup(string $s): void { function doNamedSubpattern(string $s): void { if (preg_match('/\w-(?P\d+)-(\w)/', $s, $matches)) { - assertType('array{0: string, num: string, 1: string, 2: string}', $matches); + // could be assertType('array{0: string, num: string, 1: string, 2: string, 3: string}', $matches); + assertType('array', $matches); } - assertType('array{}|array{0: string, num: string, 1: string, 2: string}', $matches); + assertType('array', $matches); if (preg_match('/^(?\S+::\S+)/', $s, $matches)) { assertType('array{0: string, name: string, 1: string}', $matches); @@ -364,9 +365,8 @@ function bug11291(string $s): void { assertType('array{}|array{0: string, 1: string, 2?: string, 3?: string}', $matches); } -function bug11323a(string $s): void -{ - if (preg_match('/Price: (?P£|€)\d+/', $s, $matches)) { +function unmatchedAsNullWithMandatoryGroup(string $s): void { + if (preg_match('/Price: (?£|€)\d+/', $s, $matches, PREG_UNMATCHED_AS_NULL)) { assertType('array{0: string, currency: string, 1: string}', $matches); } else { assertType('array{}', $matches); @@ -374,12 +374,3 @@ function bug11323a(string $s): void assertType('array{}|array{0: string, currency: string, 1: string}', $matches); } -function bug11323b(string $s): void -{ - if (preg_match('/Price: (?£|€)\d+/', $s, $matches)) { - assertType('array{0: string, currency: string, 1: string}', $matches); - } else { - assertType('array{}', $matches); - } - assertType('array{}|array{0: string, currency: string, 1: string}', $matches); -} From 57a6087220e62491c41ecedeb4efc5f37a98a9f9 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 12 Jul 2024 13:49:08 +0200 Subject: [PATCH 4/5] fix conflict --- .../Analyser/nsrt/preg_match_shapes.php | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/PHPStan/Analyser/nsrt/preg_match_shapes.php b/tests/PHPStan/Analyser/nsrt/preg_match_shapes.php index f386bf99ba..567f5e80f1 100644 --- a/tests/PHPStan/Analyser/nsrt/preg_match_shapes.php +++ b/tests/PHPStan/Analyser/nsrt/preg_match_shapes.php @@ -365,6 +365,26 @@ function bug11291(string $s): void { assertType('array{}|array{0: string, 1: string, 2?: string, 3?: string}', $matches); } +function bug11323a(string $s): void +{ + if (preg_match('/Price: (?P£|€)\d+/', $s, $matches)) { + assertType('array{0: string, currency: string, 1: string}', $matches); + } else { + assertType('array{}', $matches); + } + assertType('array{}|array{0: string, currency: string, 1: string}', $matches); +} + +function bug11323b(string $s): void +{ + if (preg_match('/Price: (?£|€)\d+/', $s, $matches)) { + assertType('array{0: string, currency: string, 1: string}', $matches); + } else { + assertType('array{}', $matches); + } + assertType('array{}|array{0: string, currency: string, 1: string}', $matches); +} + function unmatchedAsNullWithMandatoryGroup(string $s): void { if (preg_match('/Price: (?£|€)\d+/', $s, $matches, PREG_UNMATCHED_AS_NULL)) { assertType('array{0: string, currency: string, 1: string}', $matches); From 330dab27855fdd012fb2e39c646d49c9cb618c96 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 12 Jul 2024 13:51:14 +0200 Subject: [PATCH 5/5] fix bad merge --- tests/PHPStan/Analyser/nsrt/preg_match_shapes.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/PHPStan/Analyser/nsrt/preg_match_shapes.php b/tests/PHPStan/Analyser/nsrt/preg_match_shapes.php index 567f5e80f1..2b1b774a48 100644 --- a/tests/PHPStan/Analyser/nsrt/preg_match_shapes.php +++ b/tests/PHPStan/Analyser/nsrt/preg_match_shapes.php @@ -93,10 +93,9 @@ function doNonCapturingGroup(string $s): void { function doNamedSubpattern(string $s): void { if (preg_match('/\w-(?P\d+)-(\w)/', $s, $matches)) { - // could be assertType('array{0: string, num: string, 1: string, 2: string, 3: string}', $matches); - assertType('array', $matches); + assertType('array{0: string, num: string, 1: string, 2: string}', $matches); } - assertType('array', $matches); + assertType('array{}|array{0: string, num: string, 1: string, 2: string}', $matches); if (preg_match('/^(?\S+::\S+)/', $s, $matches)) { assertType('array{0: string, name: string, 1: string}', $matches);