From d68dd8af3559c15008705b001914258e2cb75407 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 21 Jun 2024 13:10:25 +0200 Subject: [PATCH 1/5] Implement array shapes for `Preg::match` $matches by-ref parameter --- composer.json | 4 +- extension.neon | 14 +++ phpstan.neon.dist | 8 +- .../PregMatchParameterOutTypeExtension.php | 63 +++++++++++++ .../PregMatchTypeSpecifyingExtension.php | 93 +++++++++++++++++++ tests/PHPStanTests/TypeInferenceTest.php | 43 +++++++++ tests/PHPStanTests/nsrt/preg-match.php | 21 +++++ 7 files changed, 242 insertions(+), 4 deletions(-) create mode 100644 extension.neon create mode 100644 src/PHPStan/PregMatchParameterOutTypeExtension.php create mode 100644 src/PHPStan/PregMatchTypeSpecifyingExtension.php create mode 100644 tests/PHPStanTests/TypeInferenceTest.php create mode 100644 tests/PHPStanTests/nsrt/preg-match.php diff --git a/composer.json b/composer.json index 40477ff..a22ec96 100644 --- a/composer.json +++ b/composer.json @@ -20,8 +20,8 @@ "php": "^7.4 || ^8.0" }, "require-dev": { - "symfony/phpunit-bridge": "^5", - "phpstan/phpstan": "^1.3", + "phpunit/phpunit": "^9.5", + "phpstan/phpstan": "1.11.x-dev", "phpstan/phpstan-strict-rules": "^1.1" }, "autoload": { diff --git a/extension.neon b/extension.neon new file mode 100644 index 0000000..d779288 --- /dev/null +++ b/extension.neon @@ -0,0 +1,14 @@ +# composer/pcre PHPStan extensions +# +# These can be reused by third party packages by including 'vendor/composer/pcre/extension.neon' +# in your phpstan config + +services: + - + class: Composer\Pcre\PHPStan\PregMatchParameterOutTypeExtension + tags: + - phpstan.staticMethodParameterOutTypeExtension + - + class: Composer\Pcre\PHPStan\PregMatchTypeSpecifyingExtension + tags: + - phpstan.typeSpecifier.staticMethodTypeSpecifyingExtension \ No newline at end of file diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 4670cbd..ec5adff 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -6,10 +6,14 @@ parameters: treatPhpDocTypesAsCertain: false - bootstrapFiles: - - tests/phpstan-locate-phpunit-autoloader.php + ignoreErrors: + - '#Test::data[a-zA-Z0-9_]+\(\) return type has no value type specified in iterable type#' + + excludePaths: + - tests/PHPStanTests/nsrt/* includes: + - extension.neon - vendor/phpstan/phpstan/conf/bleedingEdge.neon - vendor/phpstan/phpstan-strict-rules/rules.neon - phpstan-baseline.neon diff --git a/src/PHPStan/PregMatchParameterOutTypeExtension.php b/src/PHPStan/PregMatchParameterOutTypeExtension.php new file mode 100644 index 0000000..5ab9015 --- /dev/null +++ b/src/PHPStan/PregMatchParameterOutTypeExtension.php @@ -0,0 +1,63 @@ +regexShapeMatcher = $regexShapeMatcher; + } + + public function isStaticMethodSupported(MethodReflection $methodReflection, ParameterReflection $parameter): bool + { + return + $methodReflection->getDeclaringClass()->getName() === Preg::class + && $methodReflection->getName() === 'match' + && $parameter->getName() === 'matches' + ; + } + + public function getParameterOutTypeFromStaticMethodCall(MethodReflection $methodReflection, StaticCall $methodCall, ParameterReflection $parameter, Scope $scope): ?Type + { + $args = $methodCall->getArgs(); + $patternArg = $args[0] ?? null; + $matchesArg = $args[2] ?? null; + $flagsArg = $args[3] ?? null; + + if ( + $patternArg === null || $matchesArg === null + ) { + return null; + } + + $patternType = $scope->getType($patternArg->value); + $flagsType = null; + if ($flagsArg !== null) { + $flagsType = $scope->getType($flagsArg->value); + } + + return $this->regexShapeMatcher->matchType($patternType, $flagsType, TrinaryLogic::createMaybe()); + } + +} diff --git a/src/PHPStan/PregMatchTypeSpecifyingExtension.php b/src/PHPStan/PregMatchTypeSpecifyingExtension.php new file mode 100644 index 0000000..4f9f1e7 --- /dev/null +++ b/src/PHPStan/PregMatchTypeSpecifyingExtension.php @@ -0,0 +1,93 @@ +regexShapeMatcher = $regexShapeMatcher; + } + + + public function setTypeSpecifier(TypeSpecifier $typeSpecifier): void + { + $this->typeSpecifier = $typeSpecifier; + } + + public function getClass(): string { + return Preg::class; + } + + public function isStaticMethodSupported(MethodReflection $methodReflection, StaticCall $node, TypeSpecifierContext $context) : bool + { + return $methodReflection->getName() === 'match' && !$context->null(); + } + + public function specifyTypes(MethodReflection $methodReflection, StaticCall $node, Scope $scope, TypeSpecifierContext $context) : SpecifiedTypes + { + $args = $node->getArgs(); + $patternArg = $args[0] ?? null; + $matchesArg = $args[2] ?? null; + $flagsArg = $args[3] ?? null; + + if ( + $patternArg === null || $matchesArg === null + ) { + return new SpecifiedTypes(); + } + + $patternType = $scope->getType($patternArg->value); + $flagsType = null; + if ($flagsArg !== null) { + $flagsType = $scope->getType($flagsArg->value); + } + + $matchedType = $this->regexShapeMatcher->matchType($patternType, $flagsType, TrinaryLogic::createFromBoolean($context->true())); + if ($matchedType === null) { + return new SpecifiedTypes(); + } + + $overwrite = false; + if ($context->false()) { + $overwrite = true; + $context = $context->negate(); + } + + return $this->typeSpecifier->create( + $matchesArg->value, + $matchedType, + $context, + $overwrite, + $scope, + $node, + ); + } + +} diff --git a/tests/PHPStanTests/TypeInferenceTest.php b/tests/PHPStanTests/TypeInferenceTest.php new file mode 100644 index 0000000..ab0509e --- /dev/null +++ b/tests/PHPStanTests/TypeInferenceTest.php @@ -0,0 +1,43 @@ + + * + * For the full copyright and license information, please view + * the LICENSE file that was distributed with this source code. + */ + +namespace Composer\Pcre\PHPStanTests; + +use PHPStan\Testing\TypeInferenceTestCase; + +class TypeInferenceTest extends TypeInferenceTestCase +{ + public function dataFileAsserts(): iterable + { + yield from $this->gatherAssertTypesFromDirectory(__DIR__ . '/nsrt'); + + } + + /** + * @dataProvider dataFileAsserts + * @param mixed ...$args + */ + public function testFileAsserts( + string $assertType, + string $file, + ...$args + ): void + { + $this->assertFileAsserts($assertType, $file, ...$args); + } + + public static function getAdditionalConfigFiles(): array + { + return [ + __DIR__ . '/../../extension.neon', + ]; + } +} \ No newline at end of file diff --git a/tests/PHPStanTests/nsrt/preg-match.php b/tests/PHPStanTests/nsrt/preg-match.php new file mode 100644 index 0000000..1ae633b --- /dev/null +++ b/tests/PHPStanTests/nsrt/preg-match.php @@ -0,0 +1,21 @@ + Date: Fri, 21 Jun 2024 13:14:36 +0200 Subject: [PATCH 2/5] added more tests --- tests/PHPStanTests/nsrt/preg-match.php | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/PHPStanTests/nsrt/preg-match.php b/tests/PHPStanTests/nsrt/preg-match.php index 1ae633b..eacc5a1 100644 --- a/tests/PHPStanTests/nsrt/preg-match.php +++ b/tests/PHPStanTests/nsrt/preg-match.php @@ -9,6 +9,8 @@ function doMatch(string $s): void { if (Preg::match('/Price: /i', $s, $matches)) { assertType('array{string}', $matches); + } else { + assertType('array{}', $matches); } assertType('array{}|array{string}', $matches); @@ -19,3 +21,23 @@ function doMatch(string $s): void } assertType('array{}|array{string, string}', $matches); } + +function identicalMatch(string $s): void +{ + if (Preg::match('/Price: /i', $s, $matches) === 1) { + assertType('array{string}', $matches); + } else { + assertType('array{}', $matches); + } + assertType('array{}|array{string}', $matches); +} + +function equalMatch(string $s): void +{ + if (Preg::match('/Price: /i', $s, $matches) == 1) { + assertType('array{string}', $matches); + } else { + assertType('array{}', $matches); + } + assertType('array{}|array{string}', $matches); +} \ No newline at end of file From a83519c92b20f4f670229ad838c99311449c6e33 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sat, 22 Jun 2024 16:39:47 +0200 Subject: [PATCH 3/5] reflect flags behaviour --- src/PHPStan/PregMatchFlags.php | 35 ++++++ .../PregMatchParameterOutTypeExtension.php | 60 +++++------ .../PregMatchTypeSpecifyingExtension.php | 100 ++++++++---------- tests/PHPStanTests/nsrt/preg-match.php | 11 +- 4 files changed, 118 insertions(+), 88 deletions(-) create mode 100644 src/PHPStan/PregMatchFlags.php diff --git a/src/PHPStan/PregMatchFlags.php b/src/PHPStan/PregMatchFlags.php new file mode 100644 index 0000000..cd0f841 --- /dev/null +++ b/src/PHPStan/PregMatchFlags.php @@ -0,0 +1,35 @@ +getType($flagsArg->value); + + $constantScalars = $flagsType->getConstantScalarValues(); + if ($constantScalars === []) { + return null; + } + + $internalFlagsTypes = []; + foreach ($flagsType->getConstantScalarValues() as $constantScalarValue) { + if (!is_int($constantScalarValue)) { + return null; + } + + $internalFlagsTypes[] = $constantScalarValue | PREG_UNMATCHED_AS_NULL; + } + return TypeCombinator::union(...$internalFlagsTypes); + } +} \ No newline at end of file diff --git a/src/PHPStan/PregMatchParameterOutTypeExtension.php b/src/PHPStan/PregMatchParameterOutTypeExtension.php index 5ab9015..2947d64 100644 --- a/src/PHPStan/PregMatchParameterOutTypeExtension.php +++ b/src/PHPStan/PregMatchParameterOutTypeExtension.php @@ -1,63 +1,57 @@ -regexShapeMatcher = $regexShapeMatcher; - } + } public function isStaticMethodSupported(MethodReflection $methodReflection, ParameterReflection $parameter): bool { return $methodReflection->getDeclaringClass()->getName() === Preg::class && $methodReflection->getName() === 'match' - && $parameter->getName() === 'matches' - ; + && $parameter->getName() === 'matches'; } public function getParameterOutTypeFromStaticMethodCall(MethodReflection $methodReflection, StaticCall $methodCall, ParameterReflection $parameter, Scope $scope): ?Type { - $args = $methodCall->getArgs(); - $patternArg = $args[0] ?? null; - $matchesArg = $args[2] ?? null; - $flagsArg = $args[3] ?? null; - - if ( - $patternArg === null || $matchesArg === null - ) { - return null; - } - - $patternType = $scope->getType($patternArg->value); - $flagsType = null; - if ($flagsArg !== null) { - $flagsType = $scope->getType($flagsArg->value); - } - - return $this->regexShapeMatcher->matchType($patternType, $flagsType, TrinaryLogic::createMaybe()); - } + $args = $methodCall->getArgs(); + $patternArg = $args[0] ?? null; + $matchesArg = $args[2] ?? null; + $flagsArg = $args[3] ?? null; + + if ( + $patternArg === null || $matchesArg === null + ) { + return null; + } + + $flagsType = PregMatchFlags::getType($flagsArg, $scope); + if ($flagsType === null) { + return null; + } + $patternType = $scope->getType($patternArg->value); + + return $this->regexShapeMatcher->matchType($patternType, $flagsType, TrinaryLogic::createMaybe()); + } } diff --git a/src/PHPStan/PregMatchTypeSpecifyingExtension.php b/src/PHPStan/PregMatchTypeSpecifyingExtension.php index 4f9f1e7..eed8aca 100644 --- a/src/PHPStan/PregMatchTypeSpecifyingExtension.php +++ b/src/PHPStan/PregMatchTypeSpecifyingExtension.php @@ -1,30 +1,23 @@ -typeSpecifier = $typeSpecifier; - } + public function setTypeSpecifier(TypeSpecifier $typeSpecifier): void + { + $this->typeSpecifier = $typeSpecifier; + } - public function getClass(): string { + public function getClass(): string + { return Preg::class; } - public function isStaticMethodSupported(MethodReflection $methodReflection, StaticCall $node, TypeSpecifierContext $context) : bool + public function isStaticMethodSupported(MethodReflection $methodReflection, StaticCall $node, TypeSpecifierContext $context): bool { return $methodReflection->getName() === 'match' && !$context->null(); } - public function specifyTypes(MethodReflection $methodReflection, StaticCall $node, Scope $scope, TypeSpecifierContext $context) : SpecifiedTypes + public function specifyTypes(MethodReflection $methodReflection, StaticCall $node, Scope $scope, TypeSpecifierContext $context): SpecifiedTypes { - $args = $node->getArgs(); - $patternArg = $args[0] ?? null; - $matchesArg = $args[2] ?? null; - $flagsArg = $args[3] ?? null; - - if ( - $patternArg === null || $matchesArg === null - ) { - return new SpecifiedTypes(); - } - - $patternType = $scope->getType($patternArg->value); - $flagsType = null; - if ($flagsArg !== null) { - $flagsType = $scope->getType($flagsArg->value); - } - - $matchedType = $this->regexShapeMatcher->matchType($patternType, $flagsType, TrinaryLogic::createFromBoolean($context->true())); - if ($matchedType === null) { - return new SpecifiedTypes(); - } - - $overwrite = false; - if ($context->false()) { - $overwrite = true; - $context = $context->negate(); - } - - return $this->typeSpecifier->create( - $matchesArg->value, - $matchedType, - $context, - $overwrite, - $scope, - $node, - ); - } + $args = $node->getArgs(); + $patternArg = $args[0] ?? null; + $matchesArg = $args[2] ?? null; + $flagsArg = $args[3] ?? null; + + if ( + $patternArg === null || $matchesArg === null + ) { + return new SpecifiedTypes(); + } + + $flagsType = PregMatchFlags::getType($flagsArg, $scope); + if ($flagsType === null) { + return new SpecifiedTypes(); + } + $patternType = $scope->getType($patternArg->value); + + $matchedType = $this->regexShapeMatcher->matchType($patternType, $flagsType, TrinaryLogic::createFromBoolean($context->true())); + if ($matchedType === null) { + return new SpecifiedTypes(); + } + + $overwrite = false; + if ($context->false()) { + $overwrite = true; + $context = $context->negate(); + } + + return $this->typeSpecifier->create( + $matchesArg->value, + $matchedType, + $context, + $overwrite, + $scope, + $node, + ); + } } diff --git a/tests/PHPStanTests/nsrt/preg-match.php b/tests/PHPStanTests/nsrt/preg-match.php index eacc5a1..7d8993d 100644 --- a/tests/PHPStanTests/nsrt/preg-match.php +++ b/tests/PHPStanTests/nsrt/preg-match.php @@ -15,11 +15,18 @@ function doMatch(string $s): void assertType('array{}|array{string}', $matches); if (Preg::match('/Price: (£|€)\d+/', $s, $matches)) { - assertType('array{string, string}', $matches); + assertType('array{string, string|null}', $matches); } else { assertType('array{}', $matches); } - assertType('array{}|array{string, string}', $matches); + assertType('array{}|array{string, string|null}', $matches); + + if (Preg::match('/Price: (£|€)?\d+/', $s, $matches)) { + assertType('array{0: string, 1?: string|null}', $matches); + } else { + assertType('array{}', $matches); + } + assertType('array{}|array{0: string, 1?: string|null}', $matches); } function identicalMatch(string $s): void From 79219b389d8feb2ad751cee89eedb62e31ab1b14 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sat, 22 Jun 2024 16:58:38 +0200 Subject: [PATCH 4/5] utilize phpstan feature flag --- extension.neon | 12 +++++++----- tests/PHPStanTests/TypeInferenceTest.php | 1 + 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/extension.neon b/extension.neon index d779288..8fbd453 100644 --- a/extension.neon +++ b/extension.neon @@ -3,12 +3,14 @@ # These can be reused by third party packages by including 'vendor/composer/pcre/extension.neon' # in your phpstan config +conditionalTags: + Composer\Pcre\PHPStan\PregMatchParameterOutTypeExtension: + phpstan.staticMethodParameterOutTypeExtension: %featureToggles.narrowPregMatches% + Composer\Pcre\PHPStan\PregMatchTypeSpecifyingExtension: + phpstan.typeSpecifier.staticMethodTypeSpecifyingExtension: %featureToggles.narrowPregMatches% + services: - class: Composer\Pcre\PHPStan\PregMatchParameterOutTypeExtension - tags: - - phpstan.staticMethodParameterOutTypeExtension - - class: Composer\Pcre\PHPStan\PregMatchTypeSpecifyingExtension - tags: - - phpstan.typeSpecifier.staticMethodTypeSpecifyingExtension \ No newline at end of file + class: Composer\Pcre\PHPStan\PregMatchTypeSpecifyingExtension \ No newline at end of file diff --git a/tests/PHPStanTests/TypeInferenceTest.php b/tests/PHPStanTests/TypeInferenceTest.php index ab0509e..aa59b8f 100644 --- a/tests/PHPStanTests/TypeInferenceTest.php +++ b/tests/PHPStanTests/TypeInferenceTest.php @@ -37,6 +37,7 @@ public function testFileAsserts( public static function getAdditionalConfigFiles(): array { return [ + 'phar://' . __DIR__ . '/../../vendor/phpstan/phpstan/phpstan.phar/conf/bleedingEdge.neon', __DIR__ . '/../../extension.neon', ]; } From b88c4b997d63b820c79a891511e3af9d5fa8edd7 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sat, 22 Jun 2024 16:58:51 +0200 Subject: [PATCH 5/5] declare conflict with phpstan < 1.11.6 --- composer.json | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index a22ec96..f2b1815 100644 --- a/composer.json +++ b/composer.json @@ -21,9 +21,12 @@ }, "require-dev": { "phpunit/phpunit": "^9.5", - "phpstan/phpstan": "1.11.x-dev", + "phpstan/phpstan": "^1.11.6", "phpstan/phpstan-strict-rules": "^1.1" }, + "conflict": { + "phpstan/phpstan": "<1.11.6" + }, "autoload": { "psr-4": { "Composer\\Pcre\\": "src"