Skip to content

Commit 0401189

Browse files
staabmstaabm
authored
RegexArrayShapeMatcher - enforce list type when no named captures
1 parent f2f2ddf commit 0401189

File tree

7 files changed

+66
-19
lines changed

7 files changed

+66
-19
lines changed

Diff for: ‎src/Type/Php/RegexArrayShapeMatcher.php

+17-5
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
use PHPStan\TrinaryLogic;
99
use PHPStan\Type\Accessory\AccessoryArrayListType;
1010
use PHPStan\Type\ArrayType;
11-
use PHPStan\Type\Constant\ConstantArrayType;
1211
use PHPStan\Type\Constant\ConstantArrayTypeBuilder;
1312
use PHPStan\Type\Constant\ConstantIntegerType;
1413
use PHPStan\Type\Constant\ConstantStringType;
@@ -62,7 +61,7 @@ public function matchExpr(Expr $patternExpr, ?Type $flagsType, TrinaryLogic $was
6261
private function matchPatternType(Type $patternType, ?Type $flagsType, TrinaryLogic $wasMatched, bool $matchesAll): ?Type
6362
{
6463
if ($wasMatched->no()) {
65-
return new ConstantArrayType([], []);
64+
return ConstantArrayTypeBuilder::createEmpty()->getArray();
6665
}
6766

6867
$constantStrings = $patternType->getConstantStrings();
@@ -146,8 +145,11 @@ private function matchRegex(string $regex, ?int $flags, TrinaryLogic $wasMatched
146145

147146
if (!$this->containsUnmatchedAsNull($flags, $matchesAll)) {
148147
// positive match has a subject but not any capturing group
148+
$builder = ConstantArrayTypeBuilder::createEmpty();
149+
$builder->setOffsetValueType(new ConstantIntegerType(0), $this->createSubjectValueType($subjectBaseType, $flags, $matchesAll));
150+
149151
$combiType = TypeCombinator::union(
150-
new ConstantArrayType([new ConstantIntegerType(0)], [$this->createSubjectValueType($subjectBaseType, $flags, $matchesAll)], [1], [], TrinaryLogic::createYes()),
152+
$builder->getArray(),
151153
$combiType,
152154
);
153155
}
@@ -206,7 +208,10 @@ private function matchRegex(string $regex, ?int $flags, TrinaryLogic $wasMatched
206208
)
207209
) {
208210
// positive match has a subject but not any capturing group
209-
$combiTypes[] = new ConstantArrayType([new ConstantIntegerType(0)], [$this->createSubjectValueType($subjectBaseType, $flags, $matchesAll)], [1], [], TrinaryLogic::createYes());
211+
$builder = ConstantArrayTypeBuilder::createEmpty();
212+
$builder->setOffsetValueType(new ConstantIntegerType(0), $this->createSubjectValueType($subjectBaseType, $flags, $matchesAll));
213+
214+
$combiTypes[] = $builder->getArray();
210215
}
211216

212217
return TypeCombinator::union(...$combiTypes);
@@ -238,6 +243,7 @@ private function buildArrayType(
238243
bool $matchesAll,
239244
): Type
240245
{
246+
$forceList = count($markVerbs) === 0;
241247
$builder = ConstantArrayTypeBuilder::createEmpty();
242248

243249
// first item in matches contains the overall match.
@@ -256,6 +262,8 @@ private function buildArrayType(
256262
$optional = $this->isGroupOptional($captureGroup, $wasMatched, $flags, $isTrailingOptional, $matchesAll);
257263

258264
if ($captureGroup->isNamed()) {
265+
$forceList = false;
266+
259267
$builder->setOffsetValueType(
260268
$this->getKeyType($captureGroup->getName()),
261269
$groupValueType,
@@ -288,13 +296,17 @@ private function buildArrayType(
288296
$arrayType = TypeCombinator::intersect(new ArrayType(new IntegerType(), $builder->getArray()), new AccessoryArrayListType());
289297
if (!$wasMatched->yes()) {
290298
$arrayType = TypeCombinator::union(
291-
new ConstantArrayType([], []),
299+
ConstantArrayTypeBuilder::createEmpty()->getArray(),
292300
$arrayType,
293301
);
294302
}
295303
return $arrayType;
296304
}
297305

306+
if ($forceList) {
307+
return TypeCombinator::intersect($builder->getArray(), new AccessoryArrayListType());
308+
}
309+
298310
return $builder->getArray();
299311
}
300312

Diff for: ‎tests/PHPStan/Analyser/nsrt/bug-11311.php

+3-3
Original file line numberDiff line numberDiff line change
@@ -191,12 +191,12 @@ function (string $s): void {
191191

192192
function (string $s): void {
193193
preg_match('/%a(\d*)/', $s, $matches, PREG_UNMATCHED_AS_NULL);
194-
assertType("array{0?: string, 1?: ''|numeric-string|null}", $matches); // could be array{0?: string, 1?: ''|numeric-string}
194+
assertType("list{0?: string, 1?: ''|numeric-string|null}", $matches); // could be array{0?: string, 1?: ''|numeric-string}
195195
};
196196

197197
function (string $s): void {
198198
preg_match('/%a(\d*)?/', $s, $matches, PREG_UNMATCHED_AS_NULL);
199-
assertType("array{0?: string, 1?: ''|numeric-string|null}", $matches); // could be array{0?: string, 1?: ''|numeric-string}
199+
assertType("list{0?: string, 1?: ''|numeric-string|null}", $matches); // could be array{0?: string, 1?: ''|numeric-string}
200200
};
201201

202202
function (string $s): void {
@@ -222,5 +222,5 @@ function (string $s): void {
222222

223223
function (string $s): void {
224224
preg_match('~a|(\d)|(\s)~', $s, $matches, PREG_UNMATCHED_AS_NULL);
225-
assertType("array{0?: string, 1?: numeric-string|null, 2?: non-empty-string|null}", $matches);
225+
assertType("list{0?: string, 1?: numeric-string|null, 2?: non-empty-string|null}", $matches);
226226
};

Diff for: ‎tests/PHPStan/Analyser/nsrt/bug-11580.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public function bad2(string $in): void
2626
public function bad3(string $in): void
2727
{
2828
$result = preg_match('~^/xxx/([\w\-]+)/?([\w\-]+)?/?$~', $in, $matches);
29-
assertType('array{0?: string, 1?: non-empty-string, 2?: non-empty-string}', $matches);
29+
assertType('list{0?: string, 1?: non-empty-string, 2?: non-empty-string}', $matches);
3030
if ($result) {
3131
assertType('array{0: non-falsy-string, 1: non-empty-string, 2?: non-empty-string}', $matches);
3232
}

Diff for: ‎tests/PHPStan/Analyser/nsrt/preg_match_shapes.php

+14-9
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,7 @@ function (string $size): void {
300300
if (preg_match('~^a\.(b)?(c)?d~', $size, $matches) !== 1) {
301301
throw new InvalidArgumentException(sprintf('Invalid size "%s"', $size));
302302
}
303-
assertType("array{0: non-falsy-string, 1?: ''|'b', 2?: 'c'}", $matches);
303+
assertType("list{0: non-falsy-string, 1?: ''|'b', 2?: 'c'}", $matches);
304304
};
305305

306306
function (string $size): void {
@@ -525,15 +525,15 @@ function bug11323(string $s): void {
525525

526526
function (string $s): void {
527527
preg_match('/%a(\d*)/', $s, $matches);
528-
assertType("array{0?: string, 1?: ''|numeric-string}", $matches);
528+
assertType("list{0?: string, 1?: ''|numeric-string}", $matches);
529529
};
530530

531531
class Bug11376
532532
{
533533
public function test(string $str): void
534534
{
535535
preg_match('~^(?:(\w+)::)?(\w+)$~', $str, $matches);
536-
assertType('array{0?: string, 1?: string, 2?: non-empty-string}', $matches);
536+
assertType('list{0?: string, 1?: string, 2?: non-empty-string}', $matches);
537537
}
538538

539539
public function test2(string $str): void
@@ -564,7 +564,7 @@ function (string $s): void {
564564
}
565565

566566
if (preg_match($p, $s, $matches)) {
567-
assertType("array{0: non-falsy-string, 1: 'x'|'£'|numeric-string, 2?: ''|numeric-string, 3?: 'x'}", $matches);
567+
assertType("list{0: non-falsy-string, 1: 'x'|'£'|numeric-string, 2?: ''|numeric-string, 3?: 'x'}", $matches);
568568
}
569569
};
570570

@@ -730,7 +730,7 @@ function (string $s): void {
730730

731731
function (string $s): void {
732732
preg_match('~a|(\d)|(\s)~', $s, $matches);
733-
assertType("array{0?: string, 1?: '', 2?: non-empty-string}|array{0?: string, 1?: numeric-string}", $matches);
733+
assertType("list{0?: string, 1?: '', 2?: non-empty-string}|list{0?: string, 1?: numeric-string}", $matches);
734734
};
735735

736736
function bug11490 (string $expression): void {
@@ -762,13 +762,13 @@ function bug11604 (string $string): void {
762762
return;
763763
}
764764

765-
assertType("array{0: non-empty-string, 1?: ''|'XX', 2?: 'YY'}", $matches);
765+
assertType("list{0: non-empty-string, 1?: ''|'XX', 2?: 'YY'}", $matches);
766766
// could be array{string, '', 'YY'}|array{string, 'XX'}|array{string}
767767
}
768768

769769
function bug11604b (string $string): void {
770770
if (preg_match('/(XX)|(YY)?(ZZ)/', $string, $matches)) {
771-
assertType("array{0: non-empty-string, 1?: ''|'XX', 2?: ''|'YY', 3?: 'ZZ'}", $matches);
771+
assertType("list{0: non-empty-string, 1?: ''|'XX', 2?: ''|'YY', 3?: 'ZZ'}", $matches);
772772
}
773773
}
774774

@@ -935,11 +935,11 @@ function bugEmptySubexpression (string $string): void {
935935
}
936936

937937
if (preg_match('~((a)||(b))~', $string, $matches)) {
938-
assertType("array{0: string, 1: ''|'a'|'b', 2?: ''|'a', 3?: 'b'}", $matches);
938+
assertType("list{0: string, 1: ''|'a'|'b', 2?: ''|'a', 3?: 'b'}", $matches);
939939
}
940940

941941
if (preg_match('~((a)|()|(b))~', $string, $matches)) {
942-
assertType("array{0: string, 1: ''|'a'|'b', 2?: ''|'a', 3?: '', 4?: 'b'}", $matches);
942+
assertType("list{0: string, 1: ''|'a'|'b', 2?: ''|'a', 3?: '', 4?: 'b'}", $matches);
943943
}
944944
}
945945

@@ -1010,3 +1010,8 @@ function bug12749f(string $str): void
10101010
assertType('array{non-empty-string}', $match); // could be numeric-string
10111011
}
10121012
}
1013+
1014+
function bug12397(string $string) : array {
1015+
$m = preg_match('#\b([A-Z]{2,})-(\d+)#', $string, $match);
1016+
assertType('list{0?: string, 1?: non-falsy-string, 2?: numeric-string}', $match);
1017+
}

Diff for: ‎tests/PHPStan/Analyser/nsrt/preg_replace_callback_shapes.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ function (string $s): void {
2222
preg_replace_callback(
2323
'/(foo)?(bar)?(baz)?/',
2424
function ($matches) {
25-
assertType("array{0: array{string, int<-1, max>}, 1?: array{''|'foo', int<-1, max>}, 2?: array{''|'bar', int<-1, max>}, 3?: array{'baz', int<-1, max>}}", $matches);
25+
assertType("list{0: array{string, int<-1, max>}, 1?: array{''|'foo', int<-1, max>}, 2?: array{''|'bar', int<-1, max>}, 3?: array{'baz', int<-1, max>}}", $matches);
2626
return '';
2727
},
2828
$s,

Diff for: ‎tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php

+7
Original file line numberDiff line numberDiff line change
@@ -900,4 +900,11 @@ public function testNarrowSuperglobals(): void
900900
$this->analyse([__DIR__ . '/data/narrow-superglobal.php'], []);
901901
}
902902

903+
public function testBug11602(): void
904+
{
905+
$this->reportPossiblyNonexistentGeneralArrayOffset = true;
906+
907+
$this->analyse([__DIR__ . '/data/bug-11602.php'], []);
908+
}
909+
903910
}

Diff for: ‎tests/PHPStan/Rules/Arrays/data/bug-11602.php

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<?php
2+
3+
namespace Bug11602;
4+
5+
class HelloWorld
6+
{
7+
public function parseRef(string $coordinate, string $ref): string
8+
{
9+
if (preg_match('/^([A-Z]{1,3})([0-9]{1,7})(:([A-Z]{1,3})([0-9]{1,7}))?$/', $ref, $matches) !== 1) {
10+
return $ref;
11+
}
12+
if (!isset($matches[3])) { // single cell, not range
13+
return $coordinate;
14+
}
15+
$minRow = (int) $matches[2];
16+
$maxRow = (int) $matches[5];
17+
$rows = $maxRow - $minRow + 1;
18+
$minCol = $matches[1];
19+
$maxCol = $matches[4];
20+
21+
return "$minCol$minRow:$maxCol$maxRow";
22+
}
23+
}

0 commit comments

Comments
 (0)