diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml new file mode 100644 index 0000000..0669b89 --- /dev/null +++ b/.github/workflows/tests.yaml @@ -0,0 +1,50 @@ +name: CI + +on: + pull_request: ~ + push: + branches: + - master + schedule: + - cron: "47 6 * * 1" # once a month, to surface issues with newer dependencies + +jobs: + Tests: + runs-on: 'ubuntu-latest' + strategy: + matrix: + php: + - '7.4' + - '8.0' + - '8.1' + - '8.2' + - '8.3' + - '8.4' + dependencies: ['highest'] + include: + - description: '(lowest)' + php: '7.4' + dependencies: 'lowest' + + name: PHP ${{ matrix.php }} ${{ matrix.description }} + steps: + - name: "Checkout" + uses: actions/checkout@v4 + - name: "Install PHP" + uses: shivammathur/setup-php@v2 + with: + php-version: ${{ matrix.php }} + coverage: xdebug + - name: "Install dependencies" + uses: ramsey/composer-install@v3 + with: + dependency-versions: ${{ matrix.dependencies }} + - name: "Run PHPStan analysis" + run: composer phpstan + - name: "Run tests" + run: vendor/bin/phpunit --coverage-clover=coverage.xml --colors=always + - name: "Upload test coverage" + uses: codecov/codecov-action@v5 + with: + files: './coverage.xml' + fail_ci_if_error: true diff --git a/.gitignore b/.gitignore index 9b8adcf..bcada90 100644 --- a/.gitignore +++ b/.gitignore @@ -4,4 +4,5 @@ /composer.lock /phpunit.xml /tmp -/build/ \ No newline at end of file +/build/ +*.cache diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index 43967a5..0000000 --- a/.travis.yml +++ /dev/null @@ -1,34 +0,0 @@ -language: php -sudo: false -jobs: - include: - - php: 7.4 - env: PREFER_LOWEST="" - before_script: - - &composerupdate - composer update --no-interaction --no-progress --optimize-autoloader $PREFER_LOWEST - script: - - &phpunit - "./vendor/bin/phpunit" - - composer phpstan - - composer cs-check - after_script: - - travis_retry php vendor/bin/php-coveralls -v - - php: 7.1 - env: PREFER_LOWEST="" - before_script: - - *composerupdate - script: - - *phpunit - - php: 7.1 - env: PREFER_LOWEST="--prefer-lowest" - before_script: - - *composerupdate - script: - - *phpunit - -cache: - directories: - - "$HOME/.composer/cache" -after_success: -- vendor/bin/coveralls -v diff --git a/composer.json b/composer.json index d9962b9..14771e8 100644 --- a/composer.json +++ b/composer.json @@ -10,12 +10,12 @@ } ], "require": { - "php": "^7.1 || ^8.0", - "phpstan/phpstan": "^1.0", + "php": "^7.4 || ^8.0", + "phpstan/phpstan": "^2.0", "thecodingmachine/safe": "^1.0 || ^2.0" }, "require-dev": { - "phpunit/phpunit": "^7.5.2 || ^8.0", + "phpunit/phpunit": "^9.6", "php-coveralls/php-coveralls": "^2.1", "squizlabs/php_codesniffer": "^3.4" }, @@ -30,13 +30,13 @@ } }, "scripts": { - "phpstan": "phpstan analyse src -c phpstan.neon --level=7 --no-progress -vvv", + "phpstan": "phpstan analyse -c phpstan.neon --no-progress -vvv", "cs-fix": "phpcbf", "cs-check": "phpcs" }, "extra": { "branch-alias": { - "dev-master": "1.1-dev" + "dev-master": "2.0-dev" }, "phpstan": { "includes": [ diff --git a/phpstan.neon b/phpstan.neon index 3a54d92..46b5aa0 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -1,4 +1,24 @@ parameters: + level: max + paths: + - src ignoreErrors: + - + message: '#^Implementing PHPStan\\Rules\\IdentifierRuleError is not covered by backward compatibility promise\. The interface might change in a minor PHPStan version\.$#' + identifier: phpstanApi.interface + count: 1 + path: src/Rules/Error/SafeRuleError.php + + - + message: '#^Implementing PHPStan\\Rules\\LineRuleError is not covered by backward compatibility promise\. The interface might change in a minor PHPStan version\.$#' + identifier: phpstanApi.interface + count: 1 + path: src/Rules/Error/SafeRuleError.php + + - + message: '#^Implementing PHPStan\\Rules\\RuleError is not covered by backward compatibility promise\. The interface might change in a minor PHPStan version\.$#' + identifier: phpstanApi.interface + count: 1 + path: src/Rules/Error/SafeRuleError.php includes: - phpstan-safe-rule.neon diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 245ec37..ac73057 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -1,36 +1,33 @@ - + - - - ./tests/ - - - - - ./src - - - - - - - + + + ./tests/ + + + + + ./src + + + + + + + diff --git a/src/Rules/Error/SafeClassRuleError.php b/src/Rules/Error/SafeClassRuleError.php new file mode 100644 index 0000000..ad48573 --- /dev/null +++ b/src/Rules/Error/SafeClassRuleError.php @@ -0,0 +1,21 @@ +toString(); + + parent::__construct( + "Function $functionName is unsafe to use. It can return FALSE instead of throwing an exception. Please add 'use function Safe\\$functionName;' at the beginning of the file to use the variant provided by the 'thecodingmachine/safe' library.", + $line, + ); + } + + public function getIdentifier(): string + { + return self::IDENTIFIER_PREFIX . 'class'; + } +} diff --git a/src/Rules/Error/SafeRuleError.php b/src/Rules/Error/SafeRuleError.php new file mode 100644 index 0000000..5e82745 --- /dev/null +++ b/src/Rules/Error/SafeRuleError.php @@ -0,0 +1,31 @@ +message = $message; + $this->line = $line; + } + + public function getMessage(): string + { + return $this->message; + } + + public function getLine(): int + { + return $this->line; + } +} diff --git a/src/Rules/UseSafeClassesRule.php b/src/Rules/UseSafeClassesRule.php index 6dfab48..3db731d 100644 --- a/src/Rules/UseSafeClassesRule.php +++ b/src/Rules/UseSafeClassesRule.php @@ -5,15 +5,9 @@ use PhpParser\Node; use PHPStan\Analyser\Scope; -use PHPStan\Reflection\FunctionReflection; -use PHPStan\Reflection\MethodReflection; use PHPStan\Rules\Rule; -use PHPStan\ShouldNotHappenException; +use TheCodingMachine\Safe\PHPStan\Rules\Error\SafeClassRuleError; use TheCodingMachine\Safe\PHPStan\Utils\ClassListLoader; -use TheCodingMachine\Safe\PHPStan\Utils\FunctionListLoader; -use PhpParser\Node\Arg; -use PhpParser\Node\Expr; -use PhpParser\Node\Scalar; /** * This rule checks that no "unsafe" classes are instantiated in code. @@ -27,11 +21,6 @@ public function getNodeType(): string return Node\Expr\New_::class; } - /** - * @param Node\Expr\New_ $node - * @param \PHPStan\Analyser\Scope $scope - * @return string[] - */ public function processNode(Node $node, Scope $scope): array { $classNode = $node->class; @@ -43,7 +32,9 @@ public function processNode(Node $node, Scope $scope): array $unsafeClasses = ClassListLoader::getClassList(); if (isset($unsafeClasses[$className])) { - return ["Class $className is unsafe to use. Its methods can return FALSE instead of throwing an exception. Please add 'use Safe\\$className;' at the beginning of the file to use the variant provided by the 'thecodingmachine/safe' library."]; + return [ + new SafeClassRuleError($classNode, $node->getStartLine()), + ]; } return []; diff --git a/src/Rules/UseSafeFunctionsRule.php b/src/Rules/UseSafeFunctionsRule.php index cdbf205..d649184 100644 --- a/src/Rules/UseSafeFunctionsRule.php +++ b/src/Rules/UseSafeFunctionsRule.php @@ -4,15 +4,13 @@ namespace TheCodingMachine\Safe\PHPStan\Rules; use PhpParser\Node; -use PHPStan\Analyser\Scope; -use PHPStan\Reflection\FunctionReflection; -use PHPStan\Reflection\MethodReflection; -use PHPStan\Rules\Rule; -use PHPStan\ShouldNotHappenException; -use TheCodingMachine\Safe\PHPStan\Utils\FunctionListLoader; use PhpParser\Node\Arg; use PhpParser\Node\Expr; use PhpParser\Node\Scalar; +use PHPStan\Analyser\Scope; +use PHPStan\Rules\Rule; +use TheCodingMachine\Safe\PHPStan\Rules\Error\SafeFunctionRuleError; +use TheCodingMachine\Safe\PHPStan\Utils\FunctionListLoader; /** * This rule checks that no "unsafe" functions are used in code. @@ -26,11 +24,6 @@ public function getNodeType(): string return Node\Expr\FuncCall::class; } - /** - * @param Node\Expr\FuncCall $node - * @param \PHPStan\Analyser\Scope $scope - * @return string[] - */ public function processNode(Node $node, Scope $scope): array { if (!$node->name instanceof Node\Name) { @@ -40,37 +33,38 @@ public function processNode(Node $node, Scope $scope): array $unsafeFunctions = FunctionListLoader::getFunctionList(); if (isset($unsafeFunctions[$functionName])) { - if (version_compare(PHP_VERSION, '7.3.0', '>=')) { - if ($functionName === "json_decode") { - if (count($node->args) == 4) { - if ($this->argValueIncludeJSONTHROWONERROR($node->args[3])) { - return []; - } - } - } - if ($functionName === "json_encode") { - if (count($node->args) >= 2) { - if ($this->argValueIncludeJSONTHROWONERROR($node->args[1])) { - return []; - } - } - } + if ( + $functionName === "json_decode" + && $this->argValueIncludeJSONTHROWONERROR($node->getArgs()[3] ?? null) + ) { + return []; + } + + if ( + $functionName === "json_encode" + && $this->argValueIncludeJSONTHROWONERROR($node->getArgs()[1] ?? null) + ) { + return []; } - return ["Function $functionName is unsafe to use. It can return FALSE instead of throwing an exception. Please add 'use function Safe\\$functionName;' at the beginning of the file to use the variant provided by the 'thecodingmachine/safe' library."]; + return [new SafeFunctionRuleError($node->name, $node->getStartLine())]; } return []; } - private function argValueIncludeJSONTHROWONERROR(Arg $arg): bool + private function argValueIncludeJSONTHROWONERROR(?Arg $arg): bool { - $parseValue = function ($expr, array $options) use (&$parseValue): array { + if ($arg === null) { + return false; + } + + $parseValue = static function ($expr, array $options) use (&$parseValue): array { if ($expr instanceof Expr\BinaryOp\BitwiseOr) { return array_merge($parseValue($expr->left, $options), $parseValue($expr->right, $options)); } elseif ($expr instanceof Expr\ConstFetch) { - return array_merge($options, $expr->name->parts); - } elseif ($expr instanceof Scalar\LNumber) { + return array_merge($options, $expr->name->getParts()); + } elseif ($expr instanceof Scalar\Int_) { return array_merge($options, [$expr->value]); } else { return $options; @@ -78,7 +72,7 @@ private function argValueIncludeJSONTHROWONERROR(Arg $arg): bool }; $options = $parseValue($arg->value, []); - if (in_array("JSON_THROW_ON_ERROR", $options)) { + if (in_array("JSON_THROW_ON_ERROR", $options, true)) { return true; } @@ -87,6 +81,6 @@ private function argValueIncludeJSONTHROWONERROR(Arg $arg): bool return ($element & 4194304) == 4194304; }, array_filter($options, function ($element) { return is_int($element); - }))); + })), true); } } diff --git a/src/Type/Php/ReplaceSafeFunctionsDynamicReturnTypeExtension.php b/src/Type/Php/ReplaceSafeFunctionsDynamicReturnTypeExtension.php index 529b157..02711e1 100644 --- a/src/Type/Php/ReplaceSafeFunctionsDynamicReturnTypeExtension.php +++ b/src/Type/Php/ReplaceSafeFunctionsDynamicReturnTypeExtension.php @@ -3,6 +3,7 @@ namespace TheCodingMachine\Safe\PHPStan\Type\Php; +use PhpParser\Node\Arg; use PhpParser\Node\Expr\FuncCall; use PHPStan\Analyser\Scope; use PHPStan\Reflection\FunctionReflection; @@ -35,7 +36,12 @@ public function getTypeFromFunctionCall( ): Type { $type = $this->getPreliminarilyResolvedTypeFromFunctionCall($functionReflection, $functionCall, $scope); - $possibleTypes = ParametersAcceptorSelector::selectSingle($functionReflection->getVariants())->getReturnType(); + $possibleTypes = ParametersAcceptorSelector::selectFromArgs( + $scope, + $functionCall->getArgs(), + $functionReflection->getVariants() + ) + ->getReturnType(); if (TypeCombinator::containsNull($possibleTypes)) { $type = TypeCombinator::addNull($type); @@ -50,28 +56,36 @@ private function getPreliminarilyResolvedTypeFromFunctionCall( Scope $scope ): Type { $argumentPosition = $this->functions[$functionReflection->getName()]; + $defaultReturnType = ParametersAcceptorSelector::selectFromArgs( + $scope, + $functionCall->getArgs(), + $functionReflection->getVariants() + ) + ->getReturnType(); + if (count($functionCall->args) <= $argumentPosition) { - return ParametersAcceptorSelector::selectSingle($functionReflection->getVariants())->getReturnType(); + return $defaultReturnType; } - $subjectArgumentType = $scope->getType($functionCall->args[$argumentPosition]->value); - $defaultReturnType = ParametersAcceptorSelector::selectSingle($functionReflection->getVariants())->getReturnType(); - if ($subjectArgumentType instanceof MixedType) { + $subjectArgument = $functionCall->args[$argumentPosition]; + if (!$subjectArgument instanceof Arg) { + return $defaultReturnType; + } + + $subjectArgumentType = $scope->getType($subjectArgument->value); + $mixedType = new MixedType(); + if ($subjectArgumentType->isSuperTypeOf($mixedType)->yes()) { return TypeUtils::toBenevolentUnion($defaultReturnType); } - $stringType = new StringType(); - $arrayType = new ArrayType(new MixedType(), new MixedType()); - $isStringSuperType = $stringType->isSuperTypeOf($subjectArgumentType); - $isArraySuperType = $arrayType->isSuperTypeOf($subjectArgumentType); - $compareSuperTypes = $isStringSuperType->compareTo($isArraySuperType); - if ($compareSuperTypes === $isStringSuperType) { + $stringType = new StringType(); + if ($stringType->isSuperTypeOf($subjectArgumentType)->yes()) { return $stringType; - } elseif ($compareSuperTypes === $isArraySuperType) { - if ($subjectArgumentType instanceof ArrayType) { - return $subjectArgumentType->generalizeValues(); - } - return $subjectArgumentType; + } + + $arrayType = new ArrayType($mixedType, $mixedType); + if ($arrayType->isSuperTypeOf($subjectArgumentType)->yes()) { + return $arrayType; } return $defaultReturnType; diff --git a/src/Utils/FunctionListLoader.php b/src/Utils/FunctionListLoader.php index 2f1ad2a..ed351e7 100644 --- a/src/Utils/FunctionListLoader.php +++ b/src/Utils/FunctionListLoader.php @@ -3,33 +3,49 @@ namespace TheCodingMachine\Safe\PHPStan\Utils; -use PHPStan\Analyser\Scope; -use PHPStan\Reflection\FunctionReflection; -use PHPStan\Reflection\MethodReflection; - class FunctionListLoader { /** - * @var string[] + * @var array */ - private static $functions; + private static array $functions; /** - * @return string[] + * @return array */ public static function getFunctionList(): array { - if (self::$functions === null) { - if (\file_exists(__DIR__.'/../../../safe/generated/functionsList.php')) { - $functions = require __DIR__.'/../../../safe/generated/functionsList.php'; - } elseif (\file_exists(__DIR__.'/../../vendor/thecodingmachine/safe/generated/functionsList.php')) { - $functions = require __DIR__.'/../../vendor/thecodingmachine/safe/generated/functionsList.php'; - } else { - throw new \RuntimeException('Could not find thecodingmachine/safe\'s functionsList.php file.'); + return self::$functions ??= self::fetchIndexedFunctions(); + } + + /** + * @return array + */ + private static function fetchIndexedFunctions(): array + { + if (\file_exists(__DIR__ . '/../../../safe/generated/functionsList.php')) { + $functions = require __DIR__ . '/../../../safe/generated/functionsList.php'; + } elseif (\file_exists(__DIR__ . '/../../vendor/thecodingmachine/safe/generated/functionsList.php')) { + $functions = require __DIR__ . '/../../vendor/thecodingmachine/safe/generated/functionsList.php'; + } else { + throw new \RuntimeException('Could not find thecodingmachine/safe\'s functionsList.php file.'); + } + + if (!is_array($functions)) { + throw new \RuntimeException('The functions list should be an array.'); + } + + $indexedFunctions = []; + + foreach ($functions as $function) { + if (!is_string($function)) { + throw new \RuntimeException('The functions list should contain only strings, got ' . get_debug_type($function)); } + // Let's index these functions by their name - self::$functions = \Safe\array_combine($functions, $functions); + $indexedFunctions[$function] = $function; } - return self::$functions; + + return $indexedFunctions; } }