From e991fadb30ab4762b5422b774998307aea2136c5 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sun, 14 Jul 2024 07:53:10 +0200 Subject: [PATCH 1/3] RegexArrayShapeMatcher - Fix PREG_UNMATCHED_AS_NULL with optional leading groups --- src/Type/Php/RegexArrayShapeMatcher.php | 2 +- .../Analyser/nsrt/preg_match_shapes.php | 38 +++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/src/Type/Php/RegexArrayShapeMatcher.php b/src/Type/Php/RegexArrayShapeMatcher.php index d2fcc02336..ec7c2f51d0 100644 --- a/src/Type/Php/RegexArrayShapeMatcher.php +++ b/src/Type/Php/RegexArrayShapeMatcher.php @@ -271,7 +271,7 @@ private function buildArrayType( } else { if ($i < $countGroups - $trailingOptionals) { $optional = false; - if ($this->containsUnmatchedAsNull($flags)) { + if ($this->containsUnmatchedAsNull($flags) && !$captureGroup->isOptional()) { $groupValueType = TypeCombinator::removeNull($groupValueType); } } elseif ($this->containsUnmatchedAsNull($flags)) { diff --git a/tests/PHPStan/Analyser/nsrt/preg_match_shapes.php b/tests/PHPStan/Analyser/nsrt/preg_match_shapes.php index 2b1b774a48..ecf3539f64 100644 --- a/tests/PHPStan/Analyser/nsrt/preg_match_shapes.php +++ b/tests/PHPStan/Analyser/nsrt/preg_match_shapes.php @@ -353,6 +353,44 @@ function bug11277b(string $value): void } } +function bug11331a(string $url):void { + // group a is actually optional as the entire (?:...) around it is optional + if (preg_match('{^ + (?: + (?.+) + )? + (?.+)}mix', $url, $matches, PREG_UNMATCHED_AS_NULL)) { + assertType('array{0: string, a: string|null, 1: string|null, b: string, 2: string}', $matches); + } +} + +function bug11331b(string $url):void { + if (preg_match('{^ + (?: + (?.+) + )? + (?.+)?}mix', $url, $matches, PREG_UNMATCHED_AS_NULL)) { + assertType('array{0: string, a: string|null, 1: string|null, b: string|null, 2: string|null}', $matches); + } +} + +function bug11331c(string $url):void { + if (preg_match('{^ + (?: + (?:https?|git)://([^/]+)/ (?# group 1 here can be null if group 2 matches) + | (?# the alternation making it so that only either should match) + git@([^:]+):/? (?# group 2 here can be null if group 1 matches) + ) + (?# removing what follows makes the two first groups nullable, although it then has group 2 unsettable which looks buggy too as PREG_UNMATCHED_AS_NULL is present) + ([^/]+) + / + ([^/]+?) + (?:\.git|/)? +$}x', $url, $matches, PREG_UNMATCHED_AS_NULL)) { + assertType('array{string, string|null, string|null, string, string}', $matches); + } +} + // https://www.pcre.org/current/doc/html/pcre2pattern.html#dupgroupnumber // https://3v4l.org/09qdT function bug11291(string $s): void { From 1a20bb8d635a1af414947abad1b2d0e23fb84644 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sun, 14 Jul 2024 07:58:29 +0200 Subject: [PATCH 2/3] bring PREG_UNMATCHED_AS_NULL tests togehter --- tests/PHPStan/Analyser/nsrt/bug-11311.php | 37 ++++++++++++++++++ .../Analyser/nsrt/preg_match_shapes.php | 38 ------------------- 2 files changed, 37 insertions(+), 38 deletions(-) diff --git a/tests/PHPStan/Analyser/nsrt/bug-11311.php b/tests/PHPStan/Analyser/nsrt/bug-11311.php index 8db253bf7d..b7f4fae94f 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-11311.php +++ b/tests/PHPStan/Analyser/nsrt/bug-11311.php @@ -29,3 +29,40 @@ function unmatchedAsNullWithOptionalGroup(string $s): void { assertType('array{}|array{string, string|null}', $matches); } +function bug11331a(string $url):void { + // group a is actually optional as the entire (?:...) around it is optional + if (preg_match('{^ + (?: + (?.+) + )? + (?.+)}mix', $url, $matches, PREG_UNMATCHED_AS_NULL)) { + assertType('array{0: string, a: string|null, 1: string|null, b: string, 2: string}', $matches); + } +} + +function bug11331b(string $url):void { + if (preg_match('{^ + (?: + (?.+) + )? + (?.+)?}mix', $url, $matches, PREG_UNMATCHED_AS_NULL)) { + assertType('array{0: string, a: string|null, 1: string|null, b: string|null, 2: string|null}', $matches); + } +} + +function bug11331c(string $url):void { + if (preg_match('{^ + (?: + (?:https?|git)://([^/]+)/ (?# group 1 here can be null if group 2 matches) + | (?# the alternation making it so that only either should match) + git@([^:]+):/? (?# group 2 here can be null if group 1 matches) + ) + (?# removing what follows makes the two first groups nullable, although it then has group 2 unsettable which looks buggy too as PREG_UNMATCHED_AS_NULL is present) + ([^/]+) + / + ([^/]+?) + (?:\.git|/)? +$}x', $url, $matches, PREG_UNMATCHED_AS_NULL)) { + assertType('array{string, string|null, string|null, string, string}', $matches); + } +} diff --git a/tests/PHPStan/Analyser/nsrt/preg_match_shapes.php b/tests/PHPStan/Analyser/nsrt/preg_match_shapes.php index ecf3539f64..2b1b774a48 100644 --- a/tests/PHPStan/Analyser/nsrt/preg_match_shapes.php +++ b/tests/PHPStan/Analyser/nsrt/preg_match_shapes.php @@ -353,44 +353,6 @@ function bug11277b(string $value): void } } -function bug11331a(string $url):void { - // group a is actually optional as the entire (?:...) around it is optional - if (preg_match('{^ - (?: - (?.+) - )? - (?.+)}mix', $url, $matches, PREG_UNMATCHED_AS_NULL)) { - assertType('array{0: string, a: string|null, 1: string|null, b: string, 2: string}', $matches); - } -} - -function bug11331b(string $url):void { - if (preg_match('{^ - (?: - (?.+) - )? - (?.+)?}mix', $url, $matches, PREG_UNMATCHED_AS_NULL)) { - assertType('array{0: string, a: string|null, 1: string|null, b: string|null, 2: string|null}', $matches); - } -} - -function bug11331c(string $url):void { - if (preg_match('{^ - (?: - (?:https?|git)://([^/]+)/ (?# group 1 here can be null if group 2 matches) - | (?# the alternation making it so that only either should match) - git@([^:]+):/? (?# group 2 here can be null if group 1 matches) - ) - (?# removing what follows makes the two first groups nullable, although it then has group 2 unsettable which looks buggy too as PREG_UNMATCHED_AS_NULL is present) - ([^/]+) - / - ([^/]+?) - (?:\.git|/)? -$}x', $url, $matches, PREG_UNMATCHED_AS_NULL)) { - assertType('array{string, string|null, string|null, string, string}', $matches); - } -} - // https://www.pcre.org/current/doc/html/pcre2pattern.html#dupgroupnumber // https://3v4l.org/09qdT function bug11291(string $s): void { From ea579dcbdcd1066c0fb3eb1501e888b634962451 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sun, 14 Jul 2024 09:06:23 +0200 Subject: [PATCH 3/3] remove misleading comment --- tests/PHPStan/Analyser/nsrt/bug-11311.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/PHPStan/Analyser/nsrt/bug-11311.php b/tests/PHPStan/Analyser/nsrt/bug-11311.php index b7f4fae94f..7d5a8d0c80 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-11311.php +++ b/tests/PHPStan/Analyser/nsrt/bug-11311.php @@ -57,7 +57,6 @@ function bug11331c(string $url):void { | (?# the alternation making it so that only either should match) git@([^:]+):/? (?# group 2 here can be null if group 1 matches) ) - (?# removing what follows makes the two first groups nullable, although it then has group 2 unsettable which looks buggy too as PREG_UNMATCHED_AS_NULL is present) ([^/]+) / ([^/]+?)