Trigger error on unsafe StrictGroups usages

Author: Seldaek

composer.json

@@ -21,7 +21,7 @@
     },
     "require-dev": {
         "phpunit/phpunit": "^8 || ^9",
-        "phpstan/phpstan": "^1.11.6",
+        "phpstan/phpstan": "^1.11.6@dev",
         "phpstan/phpstan-strict-rules": "^1.1"
     },
     "conflict": {

src/PHPStan/PregMatchTypeSpecifyingExtension.php

@@ -78,15 +78,11 @@ public function specifyTypes(MethodReflection $methodReflection, StaticCall $nod
             && count($matchedType->getConstantArrays()) === 1
         ) {
             $matchedType = $matchedType->getConstantArrays()[0];
-            $matchedType = new ConstantArrayType(
-                $matchedType->getKeyTypes(),
-                array_map(static function (Type $valueType): Type {
-                    return TypeCombinator::removeNull($valueType);
-                }, $matchedType->getValueTypes()),
-                $matchedType->getNextAutoIndexes(),
-                [],
-                $matchedType->isList()
-            );
+            foreach ($matchedType as $type) {
+                if ($type->containsNull()) {
+                    // TODO trigger an error here that $methodReflection->getName() should not be used, or the pattern needs to make sure that $type->?? get key name is not nullable
+                }
+            }
         }
 
         $overwrite = false;

tests/PHPStanTests/nsrt/preg-match.php

@@ -15,25 +15,33 @@ function doMatch(string $s): void
     assertType('array{}|array{string}', $matches);
 
     if (Preg::match('/Price: (£|€)\d+/', $s, $matches)) {
-        assertType('array{string, string|null}', $matches);
+        assertType('array{string, string}', $matches);
     } else {
         assertType('array{}', $matches);
     }
-    assertType('array{}|array{string, string|null}', $matches);
+    assertType('array{}|array{string, string}', $matches);
 
     if (Preg::match('/Price: (£|€)?\d+/', $s, $matches)) {
-        assertType('array{0: string, 1?: string|null}', $matches);
+        assertType('array{0: string, 1: string|null}', $matches);
     } else {
         assertType('array{}', $matches);
     }
-    assertType('array{}|array{0: string, 1?: string|null}', $matches);
+    assertType('array{}|array{0: string, 1: string|null}', $matches);
+
+    // passing the PREG_UNMATCHED_AS_NULL should change nothing compared to above as it is always set
+    if (Preg::match('/Price: (£|€)?\d+/', $s, $matches, PREG_UNMATCHED_AS_NULL)) {
+        assertType('array{0: string, 1: string|null}', $matches);
+    } else {
+        assertType('array{}', $matches);
+    }
+    assertType('array{}|array{0: string, 1: string|null}', $matches);
 
     if (Preg::isMatch('/Price: (?<currency>£|€)\d+/', $s, $matches)) {
-        assertType('array{0: string, currency: string|null, 1: string|null}', $matches);
+        assertType('array{0: string, currency: string, 1: string}', $matches);
     } else {
         assertType('array{}', $matches);
     }
-    assertType('array{}|array{0: string, currency: string|null, 1: string|null}', $matches);
+    assertType('array{}|array{0: string, currency: string, 1: string}', $matches);
 }
 
 function doMatchStrictGroups(string $s): void
@@ -60,6 +68,18 @@ function doMatchStrictGroups(string $s): void
     assertType('array{}|array{0: string, test: string, 1: string}', $matches);
 }
 
+function doMatchStrictGroupsUnsafe(string $s): void
+{
+    if (Preg::isMatchStrictGroups('{Configure Command(?: *</td><td class="v">| *=> *)(.*)(?:</td>|$)}m', $s, $matches)) {
+        // does not error because the match group might be empty but is not optional
+        assertType('array{string, string}', $matches);
+    }
+
+    if (Preg::isMatchStrictGroups('{Configure Command(?: *</td><td class="v">| *=> *)(.*)?(?:</td>|$)}m', $s, $matches)) {
+        // should error as it is unsafe due to the optional group
+    }
+}
+
 // disabled until https://github.com/phpstan/phpstan-src/pull/3185 can be resolved
 //
 //function identicalMatch(string $s): void