Skip to content

Commit 73c7a86

Browse files
staabmstaabm
authored
RegexArrayShapeMatcher: fix nested capturing counting
1 parent 752e8d9 commit 73c7a86

13 files changed

+166
-156
lines changed

‎src/Type/Php/PregMatchParameterOutTypeExtension.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,9 @@ public function __construct(
2323

2424
public function isFunctionSupported(FunctionReflection $functionReflection, ParameterReflection $parameter): bool
2525
{
26-
return in_array(strtolower($functionReflection->getName()), ['preg_match'], true) && $parameter->getName() === 'matches';
26+
return in_array(strtolower($functionReflection->getName()), ['preg_match'], true)
27+
// the parameter is named different, depending on PHP version.
28+
&& in_array($parameter->getName(), ['subpatterns', 'matches'], true);
2729
}
2830

2931
public function getParameterOutTypeFromFunctionCall(FunctionReflection $functionReflection, FuncCall $funcCall, ParameterReflection $parameter, Scope $scope): ?Type

‎src/Type/Php/RegexArrayShapeMatcher.php

Lines changed: 53 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@
77
use Hoa\Compiler\Llk\TreeNode;
88
use Hoa\Exception\Exception;
99
use Hoa\File\Read;
10-
use Nette\Utils\RegexpException;
11-
use Nette\Utils\Strings;
1210
use PHPStan\TrinaryLogic;
1311
use PHPStan\Type\Constant\ConstantArrayType;
1412
use PHPStan\Type\Constant\ConstantArrayTypeBuilder;
@@ -18,14 +16,11 @@
1816
use PHPStan\Type\StringType;
1917
use PHPStan\Type\Type;
2018
use PHPStan\Type\TypeCombinator;
21-
use function array_key_last;
22-
use function array_keys;
19+
use function array_reverse;
2320
use function count;
2421
use function in_array;
25-
use function is_int;
2622
use function is_string;
2723
use function str_contains;
28-
use const PHP_VERSION_ID;
2924
use const PREG_OFFSET_CAPTURE;
3025
use const PREG_UNMATCHED_AS_NULL;
3126

@@ -43,13 +38,6 @@ public function matchType(Type $patternType, ?Type $flagsType, TrinaryLogic $was
4338
return new ConstantArrayType([], []);
4439
}
4540

46-
if (PHP_VERSION_ID < 70400) {
47-
// see https://www.php.net/manual/en/migration74.incompatible.php#migration74.incompatible.pcre
48-
// When PREG_UNMATCHED_AS_NULL mode is used, trailing unmatched capturing groups will now also be set to null (or [null, -1] if offset capture is enabled).
49-
// This means that the size of the $matches will always be the same.
50-
return null;
51-
}
52-
5341
$constantStrings = $patternType->getConstantStrings();
5442
if (count($constantStrings) === 0) {
5543
return null;
@@ -85,56 +73,53 @@ public function matchType(Type $patternType, ?Type $flagsType, TrinaryLogic $was
8573
*/
8674
private function matchRegex(string $regex, ?int $flags, TrinaryLogic $wasMatched): ?Type
8775
{
88-
// add one capturing group to the end so all capture group keys
89-
// are present in the $matches
90-
// see https://3v4l.org/sOXbn, https://3v4l.org/3SdDM
91-
$captureGroupsRegex = Strings::replace($regex, '~.[a-z\s]*$~i', '|(?<phpstanNamedCaptureGroupLast>)$0');
92-
93-
try {
94-
$matches = Strings::match('', $captureGroupsRegex, PREG_UNMATCHED_AS_NULL);
95-
if ($matches === null) {
96-
return null;
97-
}
98-
} catch (RegexpException) {
99-
return null;
100-
}
101-
102-
unset($matches[array_key_last($matches)]);
103-
unset($matches['phpstanNamedCaptureGroupLast']);
104-
105-
$remainingNonOptionalGroupCount = $this->countNonOptionalGroups($regex);
106-
if ($remainingNonOptionalGroupCount === null) {
76+
$captureGroups = $this->parseGroups($regex);
77+
if ($captureGroups === null) {
10778
// regex could not be parsed by Hoa/Regex
10879
return null;
10980
}
11081

11182
$builder = ConstantArrayTypeBuilder::createEmpty();
11283
$valueType = $this->getValueType($flags ?? 0);
11384

114-
foreach (array_keys($matches) as $key) {
115-
if ($key === 0) {
116-
// first item in matches contains the overall match.
117-
$builder->setOffsetValueType(
118-
$this->getKeyType($key),
119-
TypeCombinator::removeNull($valueType),
120-
!$wasMatched->yes(),
121-
);
122-
123-
continue;
85+
// first item in matches contains the overall match.
86+
$builder->setOffsetValueType(
87+
$this->getKeyType(0),
88+
TypeCombinator::removeNull($valueType),
89+
!$wasMatched->yes(),
90+
);
91+
92+
$trailingOptionals = 0;
93+
foreach (array_reverse($captureGroups) as $captureGroup) {
94+
if (!$captureGroup->isOptional()) {
95+
break;
12496
}
97+
$trailingOptionals++;
98+
}
99+
100+
for ($i = 0; $i < count($captureGroups); $i++) {
101+
$captureGroup = $captureGroups[$i];
125102

126103
if (!$wasMatched->yes()) {
127104
$optional = true;
128105
} else {
129-
$optional = $remainingNonOptionalGroupCount <= 0;
130-
131-
if (is_int($key)) {
132-
$remainingNonOptionalGroupCount--;
106+
if ($i < count($captureGroups) - $trailingOptionals) {
107+
$optional = false;
108+
} else {
109+
$optional = $captureGroup->isOptional();
133110
}
134111
}
135112

113+
if ($captureGroup->isNamed()) {
114+
$builder->setOffsetValueType(
115+
$this->getKeyType($captureGroup->getName()),
116+
$valueType,
117+
$optional,
118+
);
119+
}
120+
136121
$builder->setOffsetValueType(
137-
$this->getKeyType($key),
122+
$this->getKeyType($i + 1),
138123
$valueType,
139124
$optional,
140125
);
@@ -180,7 +165,10 @@ private function getValueType(int $flags): Type
180165
return $valueType;
181166
}
182167

183-
private function countNonOptionalGroups(string $regex): ?int
168+
/**
169+
* @return list<RegexCapturingGroup>|null
170+
*/
171+
private function parseGroups(string $regex): ?array
184172
{
185173
if (self::$parser === null) {
186174
/** @throws void */
@@ -193,16 +181,25 @@ private function countNonOptionalGroups(string $regex): ?int
193181
return null;
194182
}
195183

196-
return $this->walkRegexAst($ast, 0, 0);
184+
$capturings = [];
185+
$this->walkRegexAst($ast, 0, 0, $capturings);
186+
187+
return $capturings;
197188
}
198189

199-
private function walkRegexAst(TreeNode $ast, int $inAlternation, int $inOptionalQuantification): int
190+
/**
191+
* @param list<RegexCapturingGroup> $capturings
192+
*/
193+
private function walkRegexAst(TreeNode $ast, int $inAlternation, int $inOptionalQuantification, array &$capturings): void
200194
{
201-
if (
202-
in_array($ast->getId(), ['#capturing', '#namedcapturing'], true)
203-
&& !($inAlternation > 0 || $inOptionalQuantification > 0)
204-
) {
205-
return 1;
195+
if ($ast->getId() === '#capturing') {
196+
$capturings[] = RegexCapturingGroup::unnamed($inAlternation > 0 || $inOptionalQuantification > 0);
197+
} elseif ($ast->getId() === '#namedcapturing') {
198+
$name = $ast->getChild(0)->getValue()['value'];
199+
$capturings[] = RegexCapturingGroup::named(
200+
$name,
201+
$inAlternation > 0 || $inOptionalQuantification > 0,
202+
);
206203
}
207204

208205
if ($ast->getId() === '#alternation') {
@@ -222,12 +219,9 @@ private function walkRegexAst(TreeNode $ast, int $inAlternation, int $inOptional
222219
}
223220
}
224221

225-
$count = 0;
226222
foreach ($ast->getChildren() as $child) {
227-
$count += $this->walkRegexAst($child, $inAlternation, $inOptionalQuantification);
223+
$this->walkRegexAst($child, $inAlternation, $inOptionalQuantification, $capturings);
228224
}
229-
230-
return $count;
231225
}
232226

233227
}

‎src/Type/Php/RegexCapturingGroup.php

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Type\Php;
4+
5+
class RegexCapturingGroup
6+
{
7+
8+
private function __construct(private ?string $name, private bool $optional)
9+
{
10+
}
11+
12+
public static function unnamed(bool $optional): self
13+
{
14+
return new self(null, $optional);
15+
}
16+
17+
public static function named(string $name, bool $optional): self
18+
{
19+
return new self($name, $optional);
20+
}
21+
22+
public function isOptional(): bool
23+
{
24+
return $this->optional;
25+
}
26+
27+
/** @phpstan-assert-if-true !null $this->getName() */
28+
public function isNamed(): bool
29+
{
30+
return $this->name !== null;
31+
}
32+
33+
public function getName(): ?string
34+
{
35+
return $this->name;
36+
}
37+
38+
}

‎src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
use PHPStan\Internal\CombinationsHelper;
88
use PHPStan\Reflection\FunctionReflection;
99
use PHPStan\Reflection\InitializerExprTypeResolver;
10-
use PHPStan\ShouldNotHappenException;
1110
use PHPStan\Type\Accessory\AccessoryNonEmptyStringType;
1211
use PHPStan\Type\Accessory\AccessoryNonFalsyStringType;
1312
use PHPStan\Type\Accessory\AccessoryNumericStringType;
@@ -74,10 +73,6 @@ public function getTypeFromFunctionCall(
7473
// of stringy type, then the return value will be of the same type
7574
$checkArgType = $scope->getType($args[$checkArg]->value);
7675

77-
if (!array_key_exists(2, $matches)) {
78-
throw new ShouldNotHappenException();
79-
}
80-
8176
if ($matches[2] === 's' && $checkArgType->isString()->yes()) {
8277
$singlePlaceholderEarlyReturn = $checkArgType;
8378
} elseif ($matches[2] !== 's') {

‎tests/PHPStan/Analyser/LegacyNodeScopeResolverTest.php

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,7 @@ public function dataAssignInIf(): array
302302
$testScope,
303303
'matches',
304304
TrinaryLogic::createYes(),
305-
PHP_VERSION_ID <= 80000 ? 'array<string>' : 'array{0?: string}',
305+
'array{0?: string}',
306306
],
307307
[
308308
$testScope,
@@ -343,7 +343,7 @@ public function dataAssignInIf(): array
343343
$testScope,
344344
'matches2',
345345
TrinaryLogic::createMaybe(),
346-
PHP_VERSION_ID <= 80000 ? 'array<string>' : 'array{0?: string}',
346+
'array{0?: string}',
347347
],
348348
[
349349
$testScope,
@@ -355,19 +355,13 @@ public function dataAssignInIf(): array
355355
$testScope,
356356
'matches3',
357357
TrinaryLogic::createYes(),
358-
PHP_VERSION_ID <= 80000 ? 'array<string>' : 'array{0?: string}',
358+
'array{0?: string}',
359359
],
360360
[
361361
$testScope,
362362
'matches4',
363363
TrinaryLogic::createMaybe(),
364-
PHP_VERSION_ID <= 80000 ?
365-
(
366-
PHP_VERSION_ID < 70400 ?
367-
'array<string>' :
368-
'array{}|array{string}'
369-
)
370-
: 'array{}|array{string}',
364+
'array{}|array{string}',
371365
],
372366
[
373367
$testScope,
@@ -421,7 +415,7 @@ public function dataAssignInIf(): array
421415
$testScope,
422416
'ternaryMatches',
423417
TrinaryLogic::createYes(),
424-
PHP_VERSION_ID <= 80000 ? 'array<string>' : 'array{0?: string}',
418+
'array{0?: string}',
425419
],
426420
[
427421
$testScope,
@@ -8009,7 +8003,7 @@ public function dataPassedByReference(): array
80098003
'$arr',
80108004
],
80118005
[
8012-
'array<string>',
8006+
'array{0?: string}',
80138007
'$matches',
80148008
],
80158009
[

‎tests/PHPStan/Analyser/NodeScopeResolverTest.php

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -172,10 +172,6 @@ public function dataFileAsserts(): iterable
172172
yield from $this->gatherAssertTypes(__DIR__ . '/../Rules/Comparison/data/bug-9499.php');
173173
}
174174

175-
if (PHP_VERSION_ID >= 70300 && PHP_VERSION_ID < 70400) {
176-
yield from $this->gatherAssertTypes(__DIR__ . '/data/preg_match_shapes_php73.php');
177-
}
178-
179175
yield from $this->gatherAssertTypes(__DIR__ . '/../Rules/PhpDoc/data/bug-8609-function.php');
180176
yield from $this->gatherAssertTypes(__DIR__ . '/../Rules/Comparison/data/bug-5365.php');
181177
yield from $this->gatherAssertTypes(__DIR__ . '/../Rules/Comparison/data/bug-6551.php');

‎tests/PHPStan/Analyser/ParamOutTypeTest.php

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,19 +3,12 @@
33
namespace PHPStan\Analyser;
44

55
use PHPStan\Testing\TypeInferenceTestCase;
6-
use const PHP_VERSION_ID;
76

87
class ParamOutTypeTest extends TypeInferenceTestCase
98
{
109

1110
public function dataFileAsserts(): iterable
1211
{
13-
if (PHP_VERSION_ID < 80000) {
14-
yield from $this->gatherAssertTypes(__DIR__ . '/data/param-out-php7.php');
15-
}
16-
if (PHP_VERSION_ID >= 80000) {
17-
yield from $this->gatherAssertTypes(__DIR__ . '/data/param-out-php8.php');
18-
}
1912
yield from $this->gatherAssertTypes(__DIR__ . '/data/param-out.php');
2013
}
2114

‎tests/PHPStan/Analyser/data/param-out-php7.php

Lines changed: 0 additions & 23 deletions
This file was deleted.

‎tests/PHPStan/Analyser/data/param-out-php8.php

Lines changed: 0 additions & 22 deletions
This file was deleted.

0 commit comments

Comments
 (0)