Switch to use PREG_UNMATCHED_AS_NULL always, drop PHP < 7.2, fixes #1 (#9)

* Switch to use PREG_UNMATCHED_AS_NULL always, drop PHP < 7.2, fixes #1

* Migrate to scalar hints and add return types

* Mark consts internal

Author: Seldaek

.github/workflows/continuous-integration.yml

@@ -17,12 +17,6 @@ jobs:
     strategy:
       matrix:
         php-version:
-          - "5.3"
-          - "5.4"
-          - "5.5"
-          - "5.6"
-          - "7.0"
-          - "7.1"
           - "7.2"
           - "7.3"
           - "7.4"

.github/workflows/lint.yml

@@ -13,7 +13,7 @@ jobs:
     strategy:
       matrix:
         php-version:
-          - "5.3"
+          - "7.2"
           - "8.1"
 
     steps:

.github/workflows/phpstan.yml

@@ -17,7 +17,8 @@ jobs:
     strategy:
       matrix:
         php-version:
-          - "7.4"
+          - "7.2"
+          - "8.1"
 
     steps:
       - name: "Checkout"
@@ -43,8 +44,8 @@ jobs:
       - name: "Install latest dependencies"
         run: "composer update ${{ env.COMPOSER_FLAGS }}"
 
-      - name: Run PHPStan
-        # Locked to phpunit 7.5 here as newer ones have void return types which break inheritance
-        run: |
-          composer require --dev phpunit/phpunit:^7.5.20 --with-all-dependencies ${{ env.COMPOSER_FLAGS }}
-          vendor/bin/phpstan analyse
+      - name: "Initialize PHPUnit sources"
+        run: "vendor/bin/simple-phpunit --filter NO_TEST_JUST_AUTOLOAD_THANKS"
+
+      - name: "Run PHPStan"
+        run: "composer phpstan"

README.md

@@ -6,7 +6,9 @@ PCRE wrapping library that offers type-safe `preg_*` replacements.
 This library gives you a way to ensure `preg_*` functions do not fail silently, returning
 unexpected `null`s that may not be handled.
 
-It also makes it easier to work with static analysis tools like PHPStan or Psalm as it
+As of 2.0 this library also enforces [`PREG_UNMATCHED_AS_NULL`](#preg_unmatched_as_null) usage for all matching functions, [read more below](#preg_unmatched_as_null) to understand the implications.
+
+It thus makes it easier to work with static analysis tools like PHPStan or Psalm as it
 simplifies and reduces the possible return values from all the `preg_*` functions which
 are quite packed with edge cases.
 
@@ -122,11 +124,39 @@ Due to type safety requirements a few restrictions are in place.
   use `Preg::grep` in combination with some loop and `Preg::replace`.
 - `replace`, `replaceCallback` and `replaceCallbackArray` do not support an array `$subject`,
   only simple strings.
-- in 2.x, we plan to always implicitly use `PREG_UNMATCHED_AS_NULL` for matching, which offers much
-  saner/predictable results. This is currently not doable due to the PHP version requirement and to
-  keep things working the same across all PHP versions. If you use the library on a PHP 7.2+ project
-  however we highly recommend using `PREG_UNMATCHED_AS_NULL` with all `match*` and `replaceCallback*`
-  methods.
+- As of 2.0, the library always uses `PREG_UNMATCHED_AS_NULL` for matching, which offers [much
+  saner/more predictable results](#preg_unmatched_as_null). 3.x will also use the flag for
+  `replaceCallback` and `replaceCallbackArray`. This is currently not doable due to the PHP
+  version requirement and to keep things working the same across all PHP versions. If you use
+  the library on a PHP 7.4+ project however we highly recommend already passing
+  `PREG_UNMATCHED_AS_NULL` to `replaceCallback` and `replaceCallbackArray`.
+
+#### PREG_UNMATCHED_AS_NULL
+
+As of 2.0, this library always uses PREG_UNMATCHED_AS_NULL for all `match*` and `isMatch*`
+functions (unfortunately `preg_replace_callback[_array]` only support this from PHP 7.4
+onwards so we cannot do the same there yet).
+
+This means your matches will always contain all matching groups, either as null if unmatched
+or as string if it matched.
+
+The advantages in clarity and predictability are clearer if you compare the two outputs of
+running this with and without PREG_UNMATCHED_AS_NULL in $flags:
+
+```php
+preg_match('/(a)(b)*(c)(d)*/', 'ac', $matches, $flags);
+```
+
+| no flag | PREG_UNMATCHED_AS_NULL |
+| --- | --- |
+| array (size=4)              | array (size=5) |
+| 0 => string 'ac' (length=2) |   0 => string 'ac' (length=2) |
+| 1 => string 'a' (length=1)  |   1 => string 'a' (length=1) |
+| 2 => string '' (length=0)   |   2 => null |
+| 3 => string 'c' (length=1)  |   3 => string 'c' (length=1) |
+|                             |   4 => null |
+| group 2 (any unmatched group preceding one that matched) is set to `''`. You cannot tell if it matched an empty string or did not match at all | group 2 is `null` when unmatched and a string if it matched, easy to check for |
+| group 4 (any optional group without a matching one following) is missing altogether. So you have to check with `isset()`, but really you want `isset($m[4]) && $m[4] !== ''` for safety unless you are very careful to check that a non-optional group follows it | group 4 is always set, and null in this case as there was no match, easy to check for with `$m[4] !== null` |
 
 License
 -------

phpstan-baseline.neon

@@ -0,0 +1,27 @@
+parameters:
+	ignoreErrors:
+		-
+			message: "#^Method Composer\\\\Pcre\\\\Preg\\:\\:matchAll\\(\\) should return int\\<0, max\\> but returns int\\.$#"
+			count: 1
+			path: src/Preg.php
+
+		-
+			message: "#^Method Composer\\\\Pcre\\\\Preg\\:\\:matchAllWithOffsets\\(\\) should return int\\<0, max\\> but returns int\\.$#"
+			count: 1
+			path: src/Preg.php
+
+		-
+			message: "#^Strict comparison using \\=\\=\\= between int and null will always evaluate to false\\.$#"
+			count: 2
+			path: src/Preg.php
+
+		-
+			message: "#^Function preg_replace_callback invoked with 6 parameters, 3\\-5 required\\.$#"
+			count: 1
+			path: src/Preg.php
+
+		-
+			message: "#^Function preg_replace_callback_array invoked with 5 parameters, 2\\-4 required\\.$#"
+			count: 1
+			path: src/Preg.php
+

phpstan.neon.dist

@@ -4,5 +4,12 @@ parameters:
         - src
         - tests
 
+    reportUnmatchedIgnoredErrors: false
+    treatPhpDocTypesAsCertain: false
+
+    bootstrapFiles:
+        - tests/phpstan-locate-phpunit-autoloader.php
+
 includes:
-    - vendor/phpstan/phpstan-strict-rules/rules.neon
+    - vendor/phpstan/phpstan-strict-rules/rules.neon
+    - phpstan-baseline.neon

src/PcreException.php

@@ -35,7 +35,7 @@ public static function fromFunction($function, $pattern)
      */
     private static function pcreLastErrorMessage($code)
     {
-        if (PHP_VERSION_ID >= 80000) {
+        if (function_exists('preg_last_error_msg')) {
             return preg_last_error_msg();
         }