Skip to content

Commit e8b9695

Browse files
staabmondrejmirtes
authored andcommitted
Don't infer constant types for "i"-modfied patterns
1 parent 4e37828 commit e8b9695

File tree

2 files changed

+42
-20
lines changed

2 files changed

+42
-20
lines changed

src/Type/Php/RegexGroupParser.php

Lines changed: 30 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,9 @@ public function parseGroups(string $regex): ?array
6767
}
6868

6969
$captureOnlyNamed = false;
70+
$modifiers = $this->regexExpressionHelper->getPatternModifiers($regex) ?? '';
7071
if ($this->phpVersion->supportsPregCaptureOnlyNamedGroups()) {
71-
$modifiers = $this->regexExpressionHelper->getPatternModifiers($regex);
72-
$captureOnlyNamed = str_contains($modifiers ?? '', 'n');
72+
$captureOnlyNamed = str_contains($modifiers, 'n');
7373
}
7474

7575
$capturingGroups = [];
@@ -90,6 +90,7 @@ public function parseGroups(string $regex): ?array
9090
$markVerbs,
9191
$captureOnlyNamed,
9292
false,
93+
$modifiers,
9394
);
9495

9596
return [$capturingGroups, $groupCombinations, $markVerbs];
@@ -113,38 +114,29 @@ private function walkRegexAst(
113114
array &$markVerbs,
114115
bool $captureOnlyNamed,
115116
bool $repeatedMoreThanOnce,
117+
string $patternModifiers,
116118
): void
117119
{
118120
$group = null;
119121
if ($ast->getId() === '#capturing') {
120-
$maybeConstant = !$repeatedMoreThanOnce;
121-
if ($parentGroup !== null && $parentGroup->resetsGroupCounter()) {
122-
$maybeConstant = false;
123-
}
124-
125122
$group = new RegexCapturingGroup(
126123
$captureGroupId++,
127124
null,
128125
$inAlternation ? $alternationId : null,
129126
$inOptionalQuantification,
130127
$parentGroup,
131-
$this->createGroupType($ast, $maybeConstant),
128+
$this->createGroupType($ast, $this->allowConstantTypes($patternModifiers, $repeatedMoreThanOnce, $parentGroup)),
132129
);
133130
$parentGroup = $group;
134131
} elseif ($ast->getId() === '#namedcapturing') {
135-
$maybeConstant = !$repeatedMoreThanOnce;
136-
if ($parentGroup !== null && $parentGroup->resetsGroupCounter()) {
137-
$maybeConstant = false;
138-
}
139-
140132
$name = $ast->getChild(0)->getValueValue();
141133
$group = new RegexCapturingGroup(
142134
$captureGroupId++,
143135
$name,
144136
$inAlternation ? $alternationId : null,
145137
$inOptionalQuantification,
146138
$parentGroup,
147-
$this->createGroupType($ast, $maybeConstant),
139+
$this->createGroupType($ast, $this->allowConstantTypes($patternModifiers, $repeatedMoreThanOnce, $parentGroup)),
148140
);
149141
$parentGroup = $group;
150142
} elseif ($ast->getId() === '#noncapturing') {
@@ -217,6 +209,7 @@ private function walkRegexAst(
217209
$markVerbs,
218210
$captureOnlyNamed,
219211
$repeatedMoreThanOnce,
212+
$patternModifiers,
220213
);
221214

222215
if ($ast->getId() !== '#alternation') {
@@ -227,6 +220,29 @@ private function walkRegexAst(
227220
}
228221
}
229222

223+
private function allowConstantTypes(
224+
string $patternModifiers,
225+
bool $repeatedMoreThanOnce,
226+
RegexCapturingGroup|RegexNonCapturingGroup|null $parentGroup,
227+
): bool
228+
{
229+
if (str_contains($patternModifiers, 'i')) {
230+
// if caseless, we don't use constant types
231+
// because it likely yields too many combinations
232+
return false;
233+
}
234+
235+
if ($repeatedMoreThanOnce) {
236+
return false;
237+
}
238+
239+
if ($parentGroup !== null && $parentGroup->resetsGroupCounter()) {
240+
return false;
241+
}
242+
243+
return true;
244+
}
245+
230246
/** @return array{?int, ?int} */
231247
private function getQuantificationRange(TreeNode $node): array
232248
{

tests/PHPStan/Analyser/nsrt/preg_match_shapes.php

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -549,9 +549,9 @@ public function test2(string $str): void
549549

550550
function (string $s): void {
551551
if (rand(0,1)) {
552-
$p = '/Price: (£)(abc)/i';
552+
$p = '/Price: (£)(abc)/';
553553
} else {
554-
$p = '/Price: (\d)(b)/i';
554+
$p = '/Price: (\d)(b)/';
555555
}
556556

557557
if (preg_match($p, $s, $matches)) {
@@ -561,9 +561,9 @@ function (string $s): void {
561561

562562
function (string $s): void {
563563
if (rand(0,1)) {
564-
$p = '/Price: (£)/i';
564+
$p = '/Price: (£)/';
565565
} else {
566-
$p = '/Price: (£|(\d)|(x))/i';
566+
$p = '/Price: (£|(\d)|(x))/';
567567
}
568568

569569
if (preg_match($p, $s, $matches)) {
@@ -585,18 +585,24 @@ function (string $s): void {
585585

586586
function (string $s): void {
587587
if (preg_match('/Price: ([xXa])/i', $s, $matches)) {
588+
assertType("array{string, non-empty-string}", $matches);
589+
}
590+
};
591+
592+
function (string $s): void {
593+
if (preg_match('/Price: ([xXa])/', $s, $matches)) {
588594
assertType("array{string, 'a'|'X'|'x'}", $matches);
589595
}
590596
};
591597

592598
function (string $s): void {
593-
if (preg_match('/Price: (ba[rz])/i', $s, $matches)) {
599+
if (preg_match('/Price: (ba[rz])/', $s, $matches)) {
594600
assertType("array{string, 'bar'|'baz'}", $matches);
595601
}
596602
};
597603

598604
function (string $s): void {
599-
if (preg_match('/Price: (b[ao][mn])/i', $s, $matches)) {
605+
if (preg_match('/Price: (b[ao][mn])/', $s, $matches)) {
600606
assertType("array{string, 'bam'|'ban'|'bom'|'bon'}", $matches);
601607
}
602608
};

0 commit comments

Comments
 (0)