Skip to content

RegexArrayShapeMatcher - trailling groups are not optional when PREG_UNMATCHED_AS_NULL #3219

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jul 10, 2024
Merged

RegexArrayShapeMatcher - trailling groups are not optional when PREG_UNMATCHED_AS_NULL #3219

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
399b888
RegexArrayShapeMatcher - trailling groups are not optional when PREG_…
staabm Jul 10, 2024
fd3c3ec
fix test
staabm Jul 10, 2024
4620350
move tests which got php version sensitive
staabm Jul 10, 2024
4adedba
fixed namespace collision
staabm Jul 10, 2024
c28d43e
non-optional groups cannot be null when PREG_UNMATCHED_AS_NULL
staabm Jul 10, 2024
5801853
fix comment
staabm Jul 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/Php/PhpVersion.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
15 changes: 14 additions & 1 deletion src/Type/Php/RegexArrayShapeMatcher.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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()) {
Expand Down Expand Up @@ -111,6 +118,7 @@ private function matchRegex(string $regex, ?int $flags, TrinaryLogic $wasMatched
$valueType,
$wasMatched,
$trailingOptionals,
$flags ?? 0,
);

return TypeCombinator::union(
Expand Down Expand Up @@ -145,6 +153,7 @@ private function matchRegex(string $regex, ?int $flags, TrinaryLogic $wasMatched
$valueType,
$wasMatched,
$trailingOptionals,
$flags ?? 0,
);

$combiTypes[] = $combiType;
Expand All @@ -167,6 +176,7 @@ private function matchRegex(string $regex, ?int $flags, TrinaryLogic $wasMatched
$valueType,
$wasMatched,
$trailingOptionals,
$flags ?? 0,
);
}

Expand Down Expand Up @@ -228,6 +238,7 @@ private function buildArrayType(
Type $valueType,
TrinaryLogic $wasMatched,
int $trailingOptionals,
int $flags,
): Type
{
$builder = ConstantArrayTypeBuilder::createEmpty();
Expand All @@ -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();
}
Expand Down Expand Up @@ -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);
Expand Down
21 changes: 21 additions & 0 deletions tests/PHPStan/Analyser/nsrt/bug-11311-php72.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<?php // lint < 7.4

namespace Bug11311;

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);
}

19 changes: 19 additions & 0 deletions tests/PHPStan/Analyser/nsrt/bug-11311.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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|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);
}
7 changes: 0 additions & 7 deletions tests/PHPStan/Analyser/nsrt/preg_match_shapes.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this pre-existing test no longer has the same expectations across all php versions.
therefore I moved it into the bug-11311 tests and adjusted expectations accordingly

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);
Expand Down
Loading