diff --git a/.github/workflows/continuous-integration.yml b/.github/workflows/continuous-integration.yml index bc69a475e59dcb13935983c43b4f7b22458c592a..3851b6324e80f5c11182608668a76836d3a3b123 100644 --- a/.github/workflows/continuous-integration.yml +++ b/.github/workflows/continuous-integration.yml @@ -5,7 +5,6 @@ on: - pull_request env: - COMPOSER_FLAGS: "--ansi --no-interaction --no-progress --prefer-dist" SYMFONY_PHPUNIT_REMOVE_RETURN_TYPEHINT: "1" jobs: @@ -13,6 +12,7 @@ jobs: name: "CI" runs-on: ubuntu-latest + continue-on-error: ${{ matrix.experimental }} strategy: matrix: @@ -22,7 +22,10 @@ jobs: - "8.1" - "8.2" - "8.3" - - "8.4" + experimental: [false] + include: + - php-version: "8.4" + experimental: true steps: - name: "Checkout" @@ -47,9 +50,9 @@ jobs: - name: "Install latest dependencies" run: | - # Remove PHPStan as it requires a newer PHP - composer remove phpstan/phpstan phpstan/phpstan-strict-rules --dev --no-update composer update ${{ env.COMPOSER_FLAGS }} - name: "Run tests" - run: "vendor/bin/simple-phpunit --verbose" + run: | + vendor/bin/phpunit + vendor/bin/phpunit --testsuite phpstan diff --git a/.github/workflows/phpstan.yml b/.github/workflows/phpstan.yml index 809cd24c40192911f421a8fca88324f0d39d3c04..26d30c7ccc2ccacfe441826dd8956c2ac688734d 100644 --- a/.github/workflows/phpstan.yml +++ b/.github/workflows/phpstan.yml @@ -44,8 +44,5 @@ jobs: - name: "Install latest dependencies" run: "composer update ${{ env.COMPOSER_FLAGS }}" - - name: "Initialize PHPUnit sources" - run: "vendor/bin/simple-phpunit --filter NO_TEST_JUST_AUTOLOAD_THANKS" - - name: "Run PHPStan" run: "composer phpstan" diff --git a/README.md b/README.md index 973b17d8daddda40466784dfc03e3e97f1738769..49065149918350409a93a1b7e1fe5d1953ea0f81 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,8 @@ 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. +are quite packed with edge cases. As of v2.2.0 / v3.2.0 the library also comes with a +[PHPStan extension](#phpstan-extension) for parsing regular expressions and giving you even better output types. This library is a thin wrapper around `preg_*` functions with [some limitations](#restrictions--limitations). If you are looking for a richer API to handle regular expressions have a look at @@ -175,6 +176,13 @@ preg_match('/(a)(b)*(c)(d)*/', 'ac', $matches, $flags); | 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` | +PHPStan Extension +----------------- + +To use the PHPStan extension if you do not use `phpstan/extension-installer` you can include `vendor/composer/pcre/extension.neon` in your PHPStan config. + +The extension provides much better type information for $matches as well as regex validation where possible. + License ------- diff --git a/composer.json b/composer.json index 40477ff4e2f20b2ab4661640921475a2b7895d07..9e827a9ede2e28299e98a91cafec079e0a573ae2 100644 --- a/composer.json +++ b/composer.json @@ -20,10 +20,13 @@ "php": "^7.4 || ^8.0" }, "require-dev": { - "symfony/phpunit-bridge": "^5", - "phpstan/phpstan": "^1.3", + "phpunit/phpunit": "^8 || ^9", + "phpstan/phpstan": "^1.11.8", "phpstan/phpstan-strict-rules": "^1.1" }, + "conflict": { + "phpstan/phpstan": "<1.11.8" + }, "autoload": { "psr-4": { "Composer\\Pcre\\": "src" @@ -37,10 +40,15 @@ "extra": { "branch-alias": { "dev-main": "3.x-dev" + }, + "phpstan": { + "includes": [ + "extension.neon" + ] } }, "scripts": { - "test": "vendor/bin/simple-phpunit", - "phpstan": "phpstan analyse" + "test": "@php vendor/bin/phpunit", + "phpstan": "@php phpstan analyse" } } diff --git a/extension.neon b/extension.neon new file mode 100644 index 0000000000000000000000000000000000000000..3a81d3005426ef3b530593ad7de29b3a32b9ffaa --- /dev/null +++ b/extension.neon @@ -0,0 +1,22 @@ +# composer/pcre PHPStan extensions +# +# 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% + Composer\Pcre\PHPStan\UnsafeStrictGroupsCallRule: + phpstan.rules.rule: %featureToggles.narrowPregMatches% + +services: + - + class: Composer\Pcre\PHPStan\PregMatchParameterOutTypeExtension + - + class: Composer\Pcre\PHPStan\PregMatchTypeSpecifyingExtension + +rules: + - Composer\Pcre\PHPStan\UnsafeStrictGroupsCallRule + - Composer\Pcre\PHPStan\InvalidRegexPatternRule diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 910c674c5ff573dd92b1fabfd0bd7c796753f43a..f8b6b8b4219bf793d63e07a8db736613a7924ba5 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -9,3 +9,93 @@ parameters: message: "#^Parameter &\\$matches @param\\-out type of method Composer\\\\Pcre\\\\Preg\\:\\:matchAllWithOffsets\\(\\) expects array\\<int\\|string, list\\<array\\{string\\|null, int\\<\\-1, max\\>\\}\\>\\>, array given\\.$#" count: 1 path: src/Preg.php + + - + message: "#^Regex pattern is invalid\\: No ending matching delimiter '\\}' found$#" + count: 2 + path: tests/PregTests/GrepTest.php + + - + message: "#^Regex pattern is invalid\\: No ending matching delimiter '\\}' found$#" + count: 2 + path: tests/PregTests/IsMatchAllTest.php + + - + message: "#^Regex pattern is invalid\\: No ending matching delimiter '\\}' found$#" + count: 2 + path: tests/PregTests/IsMatchAllWithOffsetsTest.php + + - + message: "#^Regex pattern is invalid\\: No ending matching delimiter '\\}' found$#" + count: 2 + path: tests/PregTests/IsMatchTest.php + + - + message: "#^Regex pattern is invalid\\: No ending matching delimiter '\\}' found$#" + count: 2 + path: tests/PregTests/IsMatchWithOffsetsTest.php + + - + message: "#^Regex pattern is invalid\\: No ending matching delimiter '\\}' found$#" + count: 2 + path: tests/PregTests/MatchAllTest.php + + - + message: "#^Regex pattern is invalid\\: No ending matching delimiter '\\}' found$#" + count: 2 + path: tests/PregTests/MatchTest.php + + - + message: "#^Regex pattern is invalid\\: No ending matching delimiter '\\}' found$#" + count: 2 + path: tests/PregTests/ReplaceCallbackArrayTest.php + + - + message: "#^Regex pattern is invalid\\: No ending matching delimiter '\\}' found$#" + count: 2 + path: tests/PregTests/ReplaceCallbackTest.php + + - + message: "#^Regex pattern is invalid\\: No ending matching delimiter '\\}' found$#" + count: 2 + path: tests/PregTests/ReplaceTest.php + + - + message: "#^Regex pattern is invalid\\: No ending matching delimiter '\\}' found$#" + count: 2 + path: tests/PregTests/SplitTest.php + + - + message: "#^Regex pattern is invalid\\: No ending matching delimiter '\\}' found$#" + count: 2 + path: tests/PregTests/SplitWithOffsetsTest.php + + - + message: "#^Regex pattern is invalid\\: No ending matching delimiter '\\}' found$#" + count: 2 + path: tests/RegexTests/IsMatchTest.php + + - + message: "#^Regex pattern is invalid\\: No ending matching delimiter '\\}' found$#" + count: 2 + path: tests/RegexTests/MatchAllTest.php + + - + message: "#^Regex pattern is invalid\\: No ending matching delimiter '\\}' found$#" + count: 2 + path: tests/RegexTests/MatchTest.php + + - + message: "#^Regex pattern is invalid\\: No ending matching delimiter '\\}' found$#" + count: 2 + path: tests/RegexTests/ReplaceCallbackArrayTest.php + + - + message: "#^Regex pattern is invalid\\: No ending matching delimiter '\\}' found$#" + count: 2 + path: tests/RegexTests/ReplaceCallbackTest.php + + - + message: "#^Regex pattern is invalid\\: No ending matching delimiter '\\}' found$#" + count: 2 + path: tests/RegexTests/ReplaceTest.php diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 4670cbd0feba884915aca8b3450058033fa3ef13..e8ae6e5e7d676584a172166d7c491d321f5730b3 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -7,9 +7,14 @@ parameters: treatPhpDocTypesAsCertain: false bootstrapFiles: - - tests/phpstan-locate-phpunit-autoloader.php + - vendor/autoload.php + + excludePaths: + - tests/PHPStanTests/nsrt/* + - tests/PHPStanTests/fixtures/* includes: + - extension.neon - vendor/phpstan/phpstan/conf/bleedingEdge.neon - vendor/phpstan/phpstan-strict-rules/rules.neon - phpstan-baseline.neon diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 4a7697ffd3334f797f86b7fe1446bbe47b3f17e3..ea52b724a2ca2cd1e174c373e3fdf7d3b48ae5a2 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -5,10 +5,15 @@ backupGlobals="false" colors="true" bootstrap="vendor/autoload.php" + defaultTestSuite="pcre" > <testsuites> - <testsuite name="PCRE Test Suite"> - <directory>tests</directory> + <testsuite name="pcre"> + <directory>tests/PregTests</directory> + <directory>tests/RegexTests</directory> + </testsuite> + <testsuite name="phpstan"> + <directory>tests/PHPStanTests</directory> </testsuite> </testsuites> diff --git a/src/PHPStan/InvalidRegexPatternRule.php b/src/PHPStan/InvalidRegexPatternRule.php new file mode 100644 index 0000000000000000000000000000000000000000..8a05fb24a91a4e06c25538e0e9a5ac3a19db1a67 --- /dev/null +++ b/src/PHPStan/InvalidRegexPatternRule.php @@ -0,0 +1,142 @@ +<?php declare(strict_types = 1); + +namespace Composer\Pcre\PHPStan; + +use Composer\Pcre\Preg; +use Composer\Pcre\Regex; +use Composer\Pcre\PcreException; +use Nette\Utils\RegexpException; +use Nette\Utils\Strings; +use PhpParser\Node; +use PhpParser\Node\Expr\StaticCall; +use PhpParser\Node\Name\FullyQualified; +use PHPStan\Analyser\Scope; +use PHPStan\Rules\Rule; +use PHPStan\Rules\RuleErrorBuilder; +use function in_array; +use function sprintf; + +/** + * Copy of PHPStan's RegularExpressionPatternRule + * + * @implements Rule<StaticCall> + */ +class InvalidRegexPatternRule implements Rule +{ + public function getNodeType(): string + { + return StaticCall::class; + } + + public function processNode(Node $node, Scope $scope): array + { + $patterns = $this->extractPatterns($node, $scope); + + $errors = []; + foreach ($patterns as $pattern) { + $errorMessage = $this->validatePattern($pattern); + if ($errorMessage === null) { + continue; + } + + $errors[] = RuleErrorBuilder::message(sprintf('Regex pattern is invalid: %s', $errorMessage))->identifier('regexp.pattern')->build(); + } + + return $errors; + } + + /** + * @return string[] + */ + private function extractPatterns(StaticCall $node, Scope $scope): array + { + if (!$node->class instanceof FullyQualified) { + return []; + } + $isRegex = $node->class->toString() === Regex::class; + $isPreg = $node->class->toString() === Preg::class; + if (!$isRegex && !$isPreg) { + return []; + } + if (!$node->name instanceof Node\Identifier || !Preg::isMatch('{^(match|isMatch|grep|replace|split)}', $node->name->name)) { + return []; + } + + $functionName = $node->name->name; + if (!isset($node->getArgs()[0])) { + return []; + } + + $patternNode = $node->getArgs()[0]->value; + $patternType = $scope->getType($patternNode); + + $patternStrings = []; + + foreach ($patternType->getConstantStrings() as $constantStringType) { + if ($functionName === 'replaceCallbackArray') { + continue; + } + + $patternStrings[] = $constantStringType->getValue(); + } + + foreach ($patternType->getConstantArrays() as $constantArrayType) { + if ( + in_array($functionName, [ + 'replace', + 'replaceCallback', + ], true) + ) { + foreach ($constantArrayType->getValueTypes() as $arrayKeyType) { + foreach ($arrayKeyType->getConstantStrings() as $constantString) { + $patternStrings[] = $constantString->getValue(); + } + } + } + + if ($functionName !== 'replaceCallbackArray') { + continue; + } + + foreach ($constantArrayType->getKeyTypes() as $arrayKeyType) { + foreach ($arrayKeyType->getConstantStrings() as $constantString) { + $patternStrings[] = $constantString->getValue(); + } + } + } + + return $patternStrings; + } + + private function validatePattern(string $pattern): ?string + { + try { + $msg = null; + $prev = set_error_handler(function (int $severity, string $message, string $file) use (&$msg): bool { + $msg = preg_replace("#^preg_match(_all)?\\(.*?\\): #", '', $message); + + return true; + }); + + if ($pattern === '') { + return 'Empty string is not a valid regular expression'; + } + + Preg::match($pattern, ''); + if ($msg !== null) { + return $msg; + } + } catch (PcreException $e) { + if ($e->getCode() === PREG_INTERNAL_ERROR && $msg !== null) { + return $msg; + } + + return preg_replace('{.*? failed executing ".*": }', '', $e->getMessage()); + } finally { + restore_error_handler(); + } + + return null; + } + +} diff --git a/src/PHPStan/PregMatchFlags.php b/src/PHPStan/PregMatchFlags.php new file mode 100644 index 0000000000000000000000000000000000000000..ace3e8af07bfbb2d909da00d3ad1c9bc8e6cf6f7 --- /dev/null +++ b/src/PHPStan/PregMatchFlags.php @@ -0,0 +1,37 @@ +<?php declare(strict_types=1); + +namespace Composer\Pcre\PHPStan; + +use PHPStan\Analyser\Scope; +use PHPStan\Type\Constant\ConstantIntegerType; +use PHPStan\Type\TypeCombinator; +use PHPStan\Type\Type; +use PhpParser\Node\Arg; +use PHPStan\Type\Php\RegexArrayShapeMatcher; + +final class PregMatchFlags +{ + static public function getType(?Arg $flagsArg, Scope $scope): ?Type + { + if ($flagsArg === null) { + return new ConstantIntegerType(PREG_UNMATCHED_AS_NULL | RegexArrayShapeMatcher::PREG_UNMATCHED_AS_NULL_ON_72_73); + } + + $flagsType = $scope->getType($flagsArg->value); + + $constantScalars = $flagsType->getConstantScalarValues(); + if ($constantScalars === []) { + return null; + } + + $internalFlagsTypes = []; + foreach ($flagsType->getConstantScalarValues() as $constantScalarValue) { + if (!is_int($constantScalarValue)) { + return null; + } + + $internalFlagsTypes[] = new ConstantIntegerType($constantScalarValue | PREG_UNMATCHED_AS_NULL | RegexArrayShapeMatcher::PREG_UNMATCHED_AS_NULL_ON_72_73); + } + return TypeCombinator::union(...$internalFlagsTypes); + } +} diff --git a/src/PHPStan/PregMatchParameterOutTypeExtension.php b/src/PHPStan/PregMatchParameterOutTypeExtension.php new file mode 100644 index 0000000000000000000000000000000000000000..b9dac623237903b2003f6c86ded0d2679dc191de --- /dev/null +++ b/src/PHPStan/PregMatchParameterOutTypeExtension.php @@ -0,0 +1,58 @@ +<?php declare(strict_types=1); + +namespace Composer\Pcre\PHPStan; + +use Composer\Pcre\Preg; +use PhpParser\Node\Expr\StaticCall; +use PHPStan\Analyser\Scope; +use PHPStan\Reflection\MethodReflection; +use PHPStan\Reflection\ParameterReflection; +use PHPStan\TrinaryLogic; +use PHPStan\Type\Php\RegexArrayShapeMatcher; +use PHPStan\Type\StaticMethodParameterOutTypeExtension; +use PHPStan\Type\Type; + +final class PregMatchParameterOutTypeExtension implements StaticMethodParameterOutTypeExtension +{ + /** + * @var RegexArrayShapeMatcher + */ + private $regexShapeMatcher; + + public function __construct( + RegexArrayShapeMatcher $regexShapeMatcher + ) + { + $this->regexShapeMatcher = $regexShapeMatcher; + } + + public function isStaticMethodSupported(MethodReflection $methodReflection, ParameterReflection $parameter): bool + { + return + $methodReflection->getDeclaringClass()->getName() === Preg::class + && in_array($methodReflection->getName(), ['match', 'isMatch', 'matchStrictGroups', 'isMatchStrictGroups'], true) + && $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; + } + + $flagsType = PregMatchFlags::getType($flagsArg, $scope); + if ($flagsType === null) { + return null; + } + + return $this->regexShapeMatcher->matchExpr($patternArg->value, $flagsType, TrinaryLogic::createMaybe(), $scope); + } + +} diff --git a/src/PHPStan/PregMatchTypeSpecifyingExtension.php b/src/PHPStan/PregMatchTypeSpecifyingExtension.php new file mode 100644 index 0000000000000000000000000000000000000000..6347f3a57838cc909add861764c6bace7491f335 --- /dev/null +++ b/src/PHPStan/PregMatchTypeSpecifyingExtension.php @@ -0,0 +1,106 @@ +<?php declare(strict_types=1); + +namespace Composer\Pcre\PHPStan; + +use Composer\Pcre\Preg; +use PhpParser\Node\Expr\StaticCall; +use PHPStan\Analyser\Scope; +use PHPStan\Analyser\SpecifiedTypes; +use PHPStan\Analyser\TypeSpecifier; +use PHPStan\Analyser\TypeSpecifierAwareExtension; +use PHPStan\Analyser\TypeSpecifierContext; +use PHPStan\Reflection\MethodReflection; +use PHPStan\TrinaryLogic; +use PHPStan\Type\Constant\ConstantArrayType; +use PHPStan\Type\Php\RegexArrayShapeMatcher; +use PHPStan\Type\StaticMethodTypeSpecifyingExtension; +use PHPStan\Type\TypeCombinator; +use PHPStan\Type\Type; + +final class PregMatchTypeSpecifyingExtension implements StaticMethodTypeSpecifyingExtension, TypeSpecifierAwareExtension +{ + /** + * @var TypeSpecifier + */ + private $typeSpecifier; + + /** + * @var RegexArrayShapeMatcher + */ + private $regexShapeMatcher; + + public function __construct(RegexArrayShapeMatcher $regexShapeMatcher) + { + $this->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 in_array($methodReflection->getName(), ['match', 'isMatch', 'matchStrictGroups', 'isMatchStrictGroups'], true) && !$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(); + } + + $flagsType = PregMatchFlags::getType($flagsArg, $scope); + if ($flagsType === null) { + return new SpecifiedTypes(); + } + + $matchedType = $this->regexShapeMatcher->matchExpr($patternArg->value, $flagsType, TrinaryLogic::createFromBoolean($context->true()), $scope); + if ($matchedType === null) { + return new SpecifiedTypes(); + } + + if ( + in_array($methodReflection->getName(), ['matchStrictGroups', 'isMatchStrictGroups'], true) + && 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() + ); + } + + $overwrite = false; + if ($context->false()) { + $overwrite = true; + $context = $context->negate(); + } + + return $this->typeSpecifier->create( + $matchesArg->value, + $matchedType, + $context, + $overwrite, + $scope, + $node + ); + } +} diff --git a/src/PHPStan/UnsafeStrictGroupsCallRule.php b/src/PHPStan/UnsafeStrictGroupsCallRule.php new file mode 100644 index 0000000000000000000000000000000000000000..5bced507097a3771ead1f73342995812cecfd4c4 --- /dev/null +++ b/src/PHPStan/UnsafeStrictGroupsCallRule.php @@ -0,0 +1,112 @@ +<?php declare(strict_types=1); + +namespace Composer\Pcre\PHPStan; + +use Composer\Pcre\Preg; +use Composer\Pcre\Regex; +use PhpParser\Node; +use PhpParser\Node\Expr\StaticCall; +use PhpParser\Node\Name\FullyQualified; +use PHPStan\Analyser\Scope; +use PHPStan\Analyser\SpecifiedTypes; +use PHPStan\Rules\Rule; +use PHPStan\Rules\RuleErrorBuilder; +use PHPStan\TrinaryLogic; +use PHPStan\Type\ObjectType; +use PHPStan\Type\Type; +use PHPStan\Type\TypeCombinator; +use PHPStan\Type\Php\RegexArrayShapeMatcher; +use function sprintf; + +/** + * @implements Rule<StaticCall> + */ +final class UnsafeStrictGroupsCallRule implements Rule +{ + /** + * @var RegexArrayShapeMatcher + */ + private $regexShapeMatcher; + + public function __construct(RegexArrayShapeMatcher $regexShapeMatcher) + { + $this->regexShapeMatcher = $regexShapeMatcher; + } + + public function getNodeType(): string + { + return StaticCall::class; + } + + public function processNode(Node $node, Scope $scope): array + { + if (!$node->class instanceof FullyQualified) { + return []; + } + $isRegex = $node->class->toString() === Regex::class; + $isPreg = $node->class->toString() === Preg::class; + if (!$isRegex && !$isPreg) { + return []; + } + if (!$node->name instanceof Node\Identifier || !in_array($node->name->name, ['matchStrictGroups', 'isMatchStrictGroups', 'matchAllStrictGroups', 'isMatchAllStrictGroups'], true)) { + return []; + } + + $args = $node->getArgs(); + if (!isset($args[0])) { + return []; + } + + $patternArg = $args[0] ?? null; + if ($isPreg) { + if (!isset($args[2])) { // no matches set, skip as the matches won't be used anyway + return []; + } + $flagsArg = $args[3] ?? null; + } else { + $flagsArg = $args[2] ?? null; + } + + if ($patternArg === null) { + return []; + } + + $flagsType = PregMatchFlags::getType($flagsArg, $scope); + if ($flagsType === null) { + return []; + } + + $matchedType = $this->regexShapeMatcher->matchExpr($patternArg->value, $flagsType, TrinaryLogic::createYes(), $scope); + if ($matchedType === null) { + return [ + RuleErrorBuilder::message(sprintf('The %s call is potentially unsafe as $matches\' type could not be inferred.', $node->name->name)) + ->identifier('composerPcre.maybeUnsafeStrictGroups') + ->build(), + ]; + } + + if (count($matchedType->getConstantArrays()) === 1) { + $matchedType = $matchedType->getConstantArrays()[0]; + $nullableGroups = []; + foreach ($matchedType->getValueTypes() as $index => $type) { + if (TypeCombinator::containsNull($type)) { + $nullableGroups[] = $matchedType->getKeyTypes()[$index]->getValue(); + } + } + + if (\count($nullableGroups) > 0) { + return [ + RuleErrorBuilder::message(sprintf( + 'The %s call is unsafe as match group%s "%s" %s optional and may be null.', + $node->name->name, + \count($nullableGroups) > 1 ? 's' : '', + implode('", "', $nullableGroups), + \count($nullableGroups) > 1 ? 'are' : 'is' + ))->identifier('composerPcre.unsafeStrictGroups')->build(), + ]; + } + } + + return []; + } +} diff --git a/src/Regex.php b/src/Regex.php index 21564a471b22c82d750b15a6e98c8292031cfaaa..038cf0696a78bf501b9b6647ec68dc4c9c33aad5 100644 --- a/src/Regex.php +++ b/src/Regex.php @@ -43,6 +43,7 @@ class Regex */ public static function matchStrictGroups(string $pattern, string $subject, int $flags = 0, int $offset = 0): MatchStrictGroupsResult { + // @phpstan-ignore composerPcre.maybeUnsafeStrictGroups $count = Preg::matchStrictGroups($pattern, $subject, $matches, $flags, $offset); return new MatchStrictGroupsResult($count, $matches); @@ -87,6 +88,7 @@ class Regex self::checkOffsetCapture($flags, 'matchAllWithOffsets'); self::checkSetOrder($flags); + // @phpstan-ignore composerPcre.maybeUnsafeStrictGroups $count = Preg::matchAllStrictGroups($pattern, $subject, $matches, $flags, $offset); return new MatchAllStrictGroupsResult($count, $matches); diff --git a/tests/BaseTestCase.php b/tests/BaseTestCase.php index 2bcf134b16fb3d031ab91dc73a6fc8598232179b..827f64632fcb8077f82e5c22a83d4381c58879de 100644 --- a/tests/BaseTestCase.php +++ b/tests/BaseTestCase.php @@ -51,16 +51,12 @@ class BaseTestCase extends TestCase // Only use a message if the error can be reliably determined if (PHP_VERSION_ID >= 80000) { $error = 'Internal error'; - } elseif (PHP_VERSION_ID >= 70201) { + } else { $error = 'PREG_INTERNAL_ERROR'; } } - if (null !== $error) { - $message = sprintf('%s: failed executing "%s": %s', $this->pregFunction, $pattern, $error); - } else { - $message = null; - } + $message = sprintf('%s: failed executing "%s": %s', $this->pregFunction, $pattern, $error); $this->doExpectException('Composer\Pcre\PcreException', $message); } diff --git a/tests/PHPStanTests/InvalidRegexPatternRuleTest.php b/tests/PHPStanTests/InvalidRegexPatternRuleTest.php new file mode 100644 index 0000000000000000000000000000000000000000..86c3334e97f99b08bed4f60655a4fd309d6b25a7 --- /dev/null +++ b/tests/PHPStanTests/InvalidRegexPatternRuleTest.php @@ -0,0 +1,71 @@ +<?php + +/* + * This file is part of composer/pcre. + * + * (c) Composer <https://github.com/composer> + * + * 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\RuleTestCase; +use Composer\Pcre\PHPStan\InvalidRegexPatternRule; +use PHPStan\Type\Php\RegexArrayShapeMatcher; + +/** + * Run with "vendor/bin/phpunit --testsuite phpstan" + * + * This is excluded by default to avoid side effects with the library tests + * + * @extends RuleTestCase<InvalidRegexPatternRule> + */ +class InvalidRegexPatternRuleTest extends RuleTestCase +{ + protected function getRule(): \PHPStan\Rules\Rule + { + return new InvalidRegexPatternRule(); + } + + public function testRule(): void + { + $missing = PHP_VERSION_ID < 70300 ? ')' : 'closing parenthesis'; + + $this->analyse([__DIR__ . '/fixtures/invalid-patterns.php'], [ + [ + 'Regex pattern is invalid: Compilation failed: missing '.$missing.' at offset 1', + 11, + ], + [ + 'Regex pattern is invalid: Compilation failed: missing '.$missing.' at offset 1', + 13, + ], + [ + 'Regex pattern is invalid: Compilation failed: missing '.$missing.' at offset 1', + 15, + ], + [ + 'Regex pattern is invalid: Compilation failed: missing '.$missing.' at offset 1', + 17, + ], + [ + 'Regex pattern is invalid: Compilation failed: missing '.$missing.' at offset 1', + 19, + ], + [ + 'Regex pattern is invalid: Compilation failed: missing '.$missing.' at offset 1', + 21, + ], + ]); + } + + public static function getAdditionalConfigFiles(): array + { + return [ + 'phar://' . __DIR__ . '/../../vendor/phpstan/phpstan/phpstan.phar/conf/bleedingEdge.neon', + __DIR__ . '/../../extension.neon', + ]; + } +} diff --git a/tests/PHPStanTests/TypeInferenceTest.php b/tests/PHPStanTests/TypeInferenceTest.php new file mode 100644 index 0000000000000000000000000000000000000000..04ee3575af636d799257ef1068845754101d22c6 --- /dev/null +++ b/tests/PHPStanTests/TypeInferenceTest.php @@ -0,0 +1,51 @@ +<?php + +/* + * This file is part of composer/pcre. + * + * (c) Composer <https://github.com/composer> + * + * 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; + +/** + * Run with "vendor/bin/phpunit --testsuite phpstan" + * + * This is excluded by default to avoid side effects with the library tests + */ +class TypeInferenceTest extends TypeInferenceTestCase +{ + /** + * @return iterable<mixed> + */ + 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 [ + 'phar://' . __DIR__ . '/../../vendor/phpstan/phpstan/phpstan.phar/conf/bleedingEdge.neon', + __DIR__ . '/../../extension.neon', + ]; + } +} diff --git a/tests/PHPStanTests/UnsafeStrictGroupsCallRuleTest.php b/tests/PHPStanTests/UnsafeStrictGroupsCallRuleTest.php new file mode 100644 index 0000000000000000000000000000000000000000..2264efd7133dd68a0509393aad05d80d6b71f5f3 --- /dev/null +++ b/tests/PHPStanTests/UnsafeStrictGroupsCallRuleTest.php @@ -0,0 +1,57 @@ +<?php + +/* + * This file is part of composer/pcre. + * + * (c) Composer <https://github.com/composer> + * + * 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\RuleTestCase; +use Composer\Pcre\PHPStan\UnsafeStrictGroupsCallRule; +use PHPStan\Type\Php\RegexArrayShapeMatcher; + +/** + * Run with "vendor/bin/phpunit --testsuite phpstan" + * + * This is excluded by default to avoid side effects with the library tests + * + * @extends RuleTestCase<UnsafeStrictGroupsCallRule> + */ +class UnsafeStrictGroupsCallRuleTest extends RuleTestCase +{ + protected function getRule(): \PHPStan\Rules\Rule + { + return new UnsafeStrictGroupsCallRule(self::getContainer()->getByType(RegexArrayShapeMatcher::class)); + } + + public function testRule(): void + { + $this->analyse([__DIR__ . '/nsrt/preg-match.php'], [ + [ + 'The matchStrictGroups call is unsafe as match group "1" is optional and may be null.', + 80, + ], + [ + 'The matchAllStrictGroups call is unsafe as match groups "foo", "2" are optional and may be null.', + 82, + ], + [ + 'The isMatchStrictGroups call is potentially unsafe as $matches\' type could not be inferred.', + 86, + ], + ]); + } + + public static function getAdditionalConfigFiles(): array + { + return [ + 'phar://' . __DIR__ . '/../../vendor/phpstan/phpstan/phpstan.phar/conf/bleedingEdge.neon', + __DIR__ . '/../../extension.neon', + ]; + } +} diff --git a/tests/PHPStanTests/fixtures/invalid-patterns.php b/tests/PHPStanTests/fixtures/invalid-patterns.php new file mode 100644 index 0000000000000000000000000000000000000000..fdb84ed3ae4027304f5d7baed62640a1e649b945 --- /dev/null +++ b/tests/PHPStanTests/fixtures/invalid-patterns.php @@ -0,0 +1,22 @@ +<?php + +namespace PregMatchShapes; + +use Composer\Pcre\Preg; +use Composer\Pcre\Regex; +use function PHPStan\Testing\assertType; + +function doMatch(string $s): void +{ + Preg::match('/(/i', $s, $matches); + + Regex::isMatch('/(/i', $s); + + Preg::grep('/(/i', [$s]); + + Preg::replaceCallback('/(/i', function ($match) { return ''; }, $s); + + Preg::replaceCallback(['/(/i', '{}'], function ($match) { return ''; }, $s); + + Preg::replaceCallbackArray(['/(/i' => function ($match) { return ''; }], $s); +} diff --git a/tests/PHPStanTests/nsrt/preg-match.php b/tests/PHPStanTests/nsrt/preg-match.php new file mode 100644 index 0000000000000000000000000000000000000000..922da895c3a383d8ba9ddab04772008586679702 --- /dev/null +++ b/tests/PHPStanTests/nsrt/preg-match.php @@ -0,0 +1,111 @@ +<?php + +namespace PregMatchShapes; + +use Composer\Pcre\Preg; +use Composer\Pcre\Regex; +use function PHPStan\Testing\assertType; + +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); + + if (Preg::match('/Price: (£|€)\d+/', $s, $matches)) { + assertType('array{string, non-empty-string}', $matches); + } else { + assertType('array{}', $matches); + } + assertType('array{}|array{string, non-empty-string}', $matches); + + if (Preg::match('/Price: (£|€)?\d+/', $s, $matches)) { + assertType('array{string, non-empty-string|null}', $matches); + } else { + assertType('array{}', $matches); + } + assertType('array{}|array{string, non-empty-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{string, non-empty-string|null}', $matches); + } else { + assertType('array{}', $matches); + } + assertType('array{}|array{string, non-empty-string|null}', $matches); + + if (Preg::isMatch('/Price: (?<currency>£|€)\d+/', $s, $matches)) { + assertType('array{0: string, currency: non-empty-string, 1: non-empty-string}', $matches); + } else { + assertType('array{}', $matches); + } + assertType('array{}|array{0: string, currency: non-empty-string, 1: non-empty-string}', $matches); +} + +function doMatchStrictGroups(string $s): void +{ + if (Preg::matchStrictGroups('/Price: /i', $s, $matches)) { + assertType('array{string}', $matches); + } else { + assertType('array{}', $matches); + } + assertType('array{}|array{string}', $matches); + + if (Preg::matchStrictGroups('/Price: (£|€)\d+/', $s, $matches)) { + assertType('array{string, non-empty-string}', $matches); + } else { + assertType('array{}', $matches); + } + assertType('array{}|array{string, non-empty-string}', $matches); + + if (Preg::isMatchStrictGroups('/Price: (?<test>£|€)\d+/', $s, $matches)) { + assertType('array{0: string, test: non-empty-string, 1: non-empty-string}', $matches); + } else { + assertType('array{}', $matches); + } + assertType('array{}|array{0: string, test: non-empty-string, 1: non-empty-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); + } + + // should error as it is unsafe due to the optional group 1 + Regex::matchStrictGroups('{Configure Command(?: *</td><td class="v">| *=> *)(.*)?(?:</td>|$)}m', $s); + + if (Preg::matchAllStrictGroups('{((?<foo>.)?z)}m', $s, $matches)) { + // should error as it is unsafe due to the optional group foo/2 + } + + if (Preg::isMatchStrictGroups('{'.$s.'}', $s, $matches)) { + // should error as it is unsafe due not being introspectable with the dynamic string + } +} + +// disabled until https://github.com/phpstan/phpstan-src/pull/3185 can be resolved +// +//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); +//} diff --git a/tests/PregTests/MatchAllTest.php b/tests/PregTests/MatchAllTest.php index 862d234b1e100bcd1bcc1784be47c8c3a7fee8a8..dada88dd2c5ce2a561aebd0e951be50b9361878f 100644 --- a/tests/PregTests/MatchAllTest.php +++ b/tests/PregTests/MatchAllTest.php @@ -44,7 +44,7 @@ class MatchAllTest extends BaseTestCase public function testSuccessStrictGroups(): void { - $count = Preg::matchAllStrictGroups('{(?P<m>\d)(?<matched>a)?}', '3a', $matches); + $count = Preg::matchAllStrictGroups('{(?<m>\d)(?<matched>a)}', '3a', $matches); self::assertSame(1, $count); self::assertSame(array(0 => ['3a'], 'm' => ['3'], 1 => ['3'], 'matched' => ['a'], 2 => ['a']), $matches); } @@ -52,8 +52,9 @@ class MatchAllTest extends BaseTestCase public function testFailStrictGroups(): void { self::expectException(UnexpectedNullMatchException::class); - self::expectExceptionMessage('Pattern "{(?P<m>\d)(?<unmatched>b)?}" had an unexpected unmatched group "unmatched", make sure the pattern always matches or use matchAll() instead.'); - Preg::matchAllStrictGroups('{(?P<m>\d)(?<unmatched>b)?}', '123', $matches); + self::expectExceptionMessage('Pattern "{(?<m>\d)(?<unmatched>b)?}" had an unexpected unmatched group "unmatched", make sure the pattern always matches or use matchAll() instead.'); + // @phpstan-ignore composerPcre.unsafeStrictGroups + Preg::matchAllStrictGroups('{(?<m>\d)(?<unmatched>b)?}', '123', $matches); } public function testBadPatternThrowsIfWarningsAreNotThrowing(): void diff --git a/tests/PregTests/MatchTest.php b/tests/PregTests/MatchTest.php index 40384053b8125a9466c9e7fd9a90227293c7b77a..ae993d9f7d84b5f3c026b32c360fa777e02cc3b9 100644 --- a/tests/PregTests/MatchTest.php +++ b/tests/PregTests/MatchTest.php @@ -38,7 +38,7 @@ class MatchTest extends BaseTestCase public function testSuccessStrictGroups(): void { - $count = Preg::matchStrictGroups('{(?P<m>\d)(?<matched>a)?}', '3a', $matches); + $count = Preg::matchStrictGroups('{(?<m>\d)(?<matched>a)}', '3a', $matches); self::assertSame(1, $count); self::assertSame(array(0 => '3a', 'm' => '3', 1 => '3', 'matched' => 'a', 2 => 'a'), $matches); } @@ -46,8 +46,9 @@ class MatchTest extends BaseTestCase public function testFailStrictGroups(): void { self::expectException(UnexpectedNullMatchException::class); - self::expectExceptionMessage('Pattern "{(?P<m>\d)(?<unmatched>b)?}" had an unexpected unmatched group "unmatched", make sure the pattern always matches or use match() instead.'); - Preg::matchStrictGroups('{(?P<m>\d)(?<unmatched>b)?}', '123', $matches); + self::expectExceptionMessage('Pattern "{(?<m>\d)(?<unmatched>b)?}" had an unexpected unmatched group "unmatched", make sure the pattern always matches or use match() instead.'); + // @phpstan-ignore composerPcre.unsafeStrictGroups + Preg::matchStrictGroups('{(?<m>\d)(?<unmatched>b)?}', '123', $matches); } public function testTypeErrorWithNull(): void diff --git a/tests/RegexTests/MatchTest.php b/tests/RegexTests/MatchTest.php index 519e935b76a5279ca246719a594812e02cd0b11e..1dcdffebf78514da3ca2ad0ce9d8428780315eab 100644 --- a/tests/RegexTests/MatchTest.php +++ b/tests/RegexTests/MatchTest.php @@ -40,15 +40,16 @@ class MatchTest extends BaseTestCase public function testSuccessStrictGroups(): void { - $result = Regex::matchStrictGroups('{(?P<m>\d)(?<matched>a)?}', '3a'); + $result = Regex::matchStrictGroups('{(?<m>\d)(?<matched>a)}', '3a'); self::assertSame(array(0 => '3a', 'm' => '3', 1 => '3', 'matched' => 'a', 2 => 'a'), $result->matches); } public function testFailStrictGroups(): void { self::expectException(UnexpectedNullMatchException::class); - self::expectExceptionMessage('Pattern "{(?P<m>\d)(?<unmatched>b)?}" had an unexpected unmatched group "unmatched", make sure the pattern always matches or use match() instead.'); - Regex::matchStrictGroups('{(?P<m>\d)(?<unmatched>b)?}', '123'); + self::expectExceptionMessage('Pattern "{(?<m>\d)(?<unmatched>b)?}" had an unexpected unmatched group "unmatched", make sure the pattern always matches or use match() instead.'); + // @phpstan-ignore composerPcre.unsafeStrictGroups + Regex::matchStrictGroups('{(?<m>\d)(?<unmatched>b)?}', '123'); } public function testBadPatternThrowsIfWarningsAreNotThrowing(): void diff --git a/tests/phpstan-locate-phpunit-autoloader.php b/tests/phpstan-locate-phpunit-autoloader.php deleted file mode 100644 index a9e670cfb8ffdef11cf7346ce2bb272f6126e424..0000000000000000000000000000000000000000 --- a/tests/phpstan-locate-phpunit-autoloader.php +++ /dev/null @@ -1,22 +0,0 @@ -<?php - -$bestDirFound = null; -$dirs = (array) glob(__DIR__.'/../vendor/bin/.phpunit/phpunit-*', GLOB_ONLYDIR); -natsort($dirs); - -foreach (array_reverse($dirs) as $dir) { - $bestDirFound = $dir; - if (PHP_VERSION_ID >= 80000 && false !== strpos((string) $dir, 'phpunit-9')) { - break; - } - if (PHP_VERSION_ID < 80000 && false !== strpos((string) $dir, 'phpunit-8')) { - break; - } -} - -if (null === $bestDirFound) { - echo 'Run "composer test" to initialize PHPUnit sources before running PHPStan'.PHP_EOL; - exit(1); -} - -include $bestDirFound.'/vendor/autoload.php';