From 6b415e933c55924129377846a94f95b6fbc67f01 Mon Sep 17 00:00:00 2001 From: Alessandro Chitolina Date: Wed, 19 Feb 2025 08:48:21 +0100 Subject: [PATCH] chore: update rules to be compatible with phpstan 2 --- .gitignore | 3 +- composer.json | 6 ++-- phpunit.xml.dist | 30 ++++++++----------- .../SwitchMustContainDefaultRule.php | 18 ++++++----- .../DoNotThrowExceptionBaseClassRule.php | 27 +++++++++-------- src/Rules/Exceptions/EmptyExceptionRule.php | 11 +++++-- src/Rules/Exceptions/MustRethrowRule.php | 13 +++++--- .../ThrowMustBundlePreviousExceptionRule.php | 27 ++++++++++------- src/Rules/Superglobals/NoSuperglobalsRule.php | 15 ++++++---- .../SwitchMustContainDefaultRuleTest.php | 3 +- .../DoNotThrowExceptionBaseClassRuleTest.php | 2 +- .../Exceptions/EmptyExceptionRuleTest.php | 4 +-- .../Rules/Exceptions/data/throw_exception.php | 3 +- 13 files changed, 93 insertions(+), 69 deletions(-) diff --git a/.gitignore b/.gitignore index 9b8adcf..cdecda1 100644 --- a/.gitignore +++ b/.gitignore @@ -1,7 +1,8 @@ *~ /vendor/* .*~ +/.phpunit.result.cache /composer.lock /phpunit.xml /tmp -/build/ \ No newline at end of file +/build/ diff --git a/composer.json b/composer.json index bba9329..8a9ba57 100644 --- a/composer.json +++ b/composer.json @@ -10,11 +10,11 @@ } ], "require": { - "php": "^7.1|^8.0", - "phpstan/phpstan": "^1.0" + "php": "^7.4|^8.0", + "phpstan/phpstan": "^2.0" }, "require-dev": { - "phpunit/phpunit": "^7.1", + "phpunit/phpunit": "^9.5", "php-coveralls/php-coveralls": "^2.1" }, "autoload": { diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 245ec37..71da406 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -1,4 +1,5 @@ - + + xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/9.3/phpunit.xsd"> + + + ./src + + + + + + + ./tests/ - - - ./src - - - - - - - diff --git a/src/Rules/Conditionals/SwitchMustContainDefaultRule.php b/src/Rules/Conditionals/SwitchMustContainDefaultRule.php index a0c5278..2547687 100644 --- a/src/Rules/Conditionals/SwitchMustContainDefaultRule.php +++ b/src/Rules/Conditionals/SwitchMustContainDefaultRule.php @@ -5,10 +5,10 @@ use PhpParser\Node; use PhpParser\Node\Stmt\Switch_; -use PhpParser\NodeTraverser; -use PhpParser\NodeVisitorAbstract; use PHPStan\Analyser\Scope; use PHPStan\Rules\Rule; +use PHPStan\Rules\RuleError; +use PHPStan\Rules\RuleErrorBuilder; use TheCodingMachine\PHPStan\Utils\PrefixGenerator; /** @@ -24,15 +24,15 @@ public function getNodeType(): string } /** - * @param Switch_ $switch + * @param Switch_ $node * @param \PHPStan\Analyser\Scope $scope - * @return string[] + * @return RuleError[] */ - public function processNode(Node $switch, Scope $scope): array + public function processNode(Node $node, Scope $scope): array { $errors = []; $defaultFound = false; - foreach ($switch->cases as $case) { + foreach ($node->cases as $case) { if ($case->cond === null) { $defaultFound = true; break; @@ -40,7 +40,11 @@ public function processNode(Node $switch, Scope $scope): array } if (!$defaultFound) { - $errors[] = sprintf(PrefixGenerator::generatePrefix($scope).'switch statement does not have a "default" case. If your code is supposed to enter at least one "case" or another, consider adding a "default" case that throws an exception. More info: http://bit.ly/switchdefault'); + $errors[] = RuleErrorBuilder::message(sprintf(PrefixGenerator::generatePrefix($scope).'switch statement does not have a "default" case.')) + ->file($scope->getFile()) + ->line($node->getStartLine()) + ->tip('If your code is supposed to enter at least one "case" or another, consider adding a "default" case that throws an exception. More info: http://bit.ly/switchdefault') + ->build(); } return $errors; diff --git a/src/Rules/Exceptions/DoNotThrowExceptionBaseClassRule.php b/src/Rules/Exceptions/DoNotThrowExceptionBaseClassRule.php index 578d48c..09f195f 100644 --- a/src/Rules/Exceptions/DoNotThrowExceptionBaseClassRule.php +++ b/src/Rules/Exceptions/DoNotThrowExceptionBaseClassRule.php @@ -6,25 +6,26 @@ use PhpParser\Node; use PHPStan\Analyser\Scope; use PHPStan\Rules\Rule; -use PHPStan\Type\ObjectType; +use PHPStan\Rules\RuleError; +use PHPStan\Rules\RuleErrorBuilder; /** * This rule checks that the base \Exception class is never thrown. Instead, developers should subclass the \Exception * base class and throw the sub-type. * - * @implements Rule + * @implements Rule */ class DoNotThrowExceptionBaseClassRule implements Rule { public function getNodeType(): string { - return Node\Stmt\Throw_::class; + return Node\Expr\Throw_::class; } /** - * @param \PhpParser\Node\Stmt\Throw_ $node + * @param \PhpParser\Node\Expr\Throw_ $node * @param \PHPStan\Analyser\Scope $scope - * @return string[] + * @return RuleError[] */ public function processNode(Node $node, Scope $scope): array { @@ -35,14 +36,14 @@ public function processNode(Node $node, Scope $scope): array $type = $scope->getType($node->expr); - if ($type instanceof ObjectType) { - $class = $type->getClassName(); - - if ($class === 'Exception') { - return [ - 'Do not throw the \Exception base class. Instead, extend the \Exception base class. More info: http://bit.ly/subtypeexception' - ]; - } + if ($type->getObjectClassNames() === ['Exception']) { + return [ + RuleErrorBuilder::message('Do not throw the \Exception base class.') + ->file($scope->getFile()) + ->line($node->getStartLine()) + ->tip('Instead, extend the \Exception base class. More info: http://bit.ly/subtypeexception') + ->build(), + ]; } return []; diff --git a/src/Rules/Exceptions/EmptyExceptionRule.php b/src/Rules/Exceptions/EmptyExceptionRule.php index fdef067..f4140c9 100644 --- a/src/Rules/Exceptions/EmptyExceptionRule.php +++ b/src/Rules/Exceptions/EmptyExceptionRule.php @@ -6,8 +6,9 @@ use PhpParser\Node; use PhpParser\Node\Stmt\Catch_; use PHPStan\Analyser\Scope; -use PHPStan\Broker\Broker; use PHPStan\Rules\Rule; +use PHPStan\Rules\RuleError; +use PHPStan\Rules\RuleErrorBuilder; use function strpos; /** @@ -23,13 +24,17 @@ public function getNodeType(): string /** * @param \PhpParser\Node\Stmt\Catch_ $node * @param \PHPStan\Analyser\Scope $scope - * @return string[] + * @return RuleError[] */ public function processNode(Node $node, Scope $scope): array { if ($this->isEmpty($node->stmts)) { return [ - 'Empty catch block. If you are sure this is meant to be empty, please add a "// @ignoreException" comment in the catch block.' + RuleErrorBuilder::message('Empty catch block.') + ->tip('If you are sure this is meant to be empty, please add a "// @ignoreException" comment in the catch block.') + ->file($scope->getFile()) + ->line($node->getStartLine()) + ->build(), ]; } diff --git a/src/Rules/Exceptions/MustRethrowRule.php b/src/Rules/Exceptions/MustRethrowRule.php index 11393de..b4e8902 100644 --- a/src/Rules/Exceptions/MustRethrowRule.php +++ b/src/Rules/Exceptions/MustRethrowRule.php @@ -4,13 +4,14 @@ namespace TheCodingMachine\PHPStan\Rules\Exceptions; use Exception; +use PHPStan\Rules\RuleError; +use PHPStan\Rules\RuleErrorBuilder; use function in_array; use PhpParser\Node; use PhpParser\Node\Stmt\Catch_; use PhpParser\NodeTraverser; use PhpParser\NodeVisitorAbstract; use PHPStan\Analyser\Scope; -use PHPStan\Broker\Broker; use PHPStan\Rules\Rule; use RuntimeException; use TheCodingMachine\PHPStan\Utils\PrefixGenerator; @@ -32,7 +33,7 @@ public function getNodeType(): string /** * @param Catch_ $node * @param \PHPStan\Analyser\Scope $scope - * @return string[] + * @return RuleError[] */ public function processNode(Node $node, Scope $scope): array { @@ -58,7 +59,7 @@ public function processNode(Node $node, Scope $scope): array public function leaveNode(Node $node) { - if ($node instanceof Node\Stmt\Throw_) { + if ($node instanceof Node\Stmt\Expression && $node->expr instanceof Node\Expr\Throw_) { $this->throwFound = true; } return null; @@ -82,7 +83,11 @@ public function isThrowFound(): bool $errors = []; if (!$visitor->isThrowFound()) { - $errors[] = sprintf('%scaught "%s" must be rethrown. Either catch a more specific exception or add a "throw" clause in the "catch" block to propagate the exception. More info: http://bit.ly/failloud', PrefixGenerator::generatePrefix($scope), $exceptionType); + $message = sprintf('%scaught "%s" must be rethrown. Either catch a more specific exception or add a "throw" clause in the "catch" block to propagate the exception. More info: http://bit.ly/failloud', PrefixGenerator::generatePrefix($scope), $exceptionType); + $errors[] = RuleErrorBuilder::message($message) + ->line($node->getStartLine()) + ->file($scope->getFile()) + ->build(); } return $errors; diff --git a/src/Rules/Exceptions/ThrowMustBundlePreviousExceptionRule.php b/src/Rules/Exceptions/ThrowMustBundlePreviousExceptionRule.php index 36fd5d2..acff4c7 100644 --- a/src/Rules/Exceptions/ThrowMustBundlePreviousExceptionRule.php +++ b/src/Rules/Exceptions/ThrowMustBundlePreviousExceptionRule.php @@ -8,8 +8,9 @@ use PhpParser\NodeTraverser; use PhpParser\NodeVisitorAbstract; use PHPStan\Analyser\Scope; -use PHPStan\Broker\Broker; use PHPStan\Rules\Rule; +use PHPStan\Rules\RuleError; +use PHPStan\Rules\RuleErrorBuilder; /** * When throwing into a catch block, checks that the previous exception is passed to the new "throw" clause @@ -27,10 +28,14 @@ public function getNodeType(): string /** * @param Catch_ $node * @param \PHPStan\Analyser\Scope $scope - * @return string[] + * @return RuleError[] */ public function processNode(Node $node, Scope $scope): array { + if ($node->var === null) { + return []; + } + $visitor = new class($node->var->name) extends NodeVisitorAbstract { /** * @var string @@ -41,11 +46,11 @@ public function processNode(Node $node, Scope $scope): array */ private $exceptionUsedCount = 0; /** - * @var Node\Stmt\Throw_[] + * @var Node\Expr\Throw_[] */ private $unusedThrows = []; - public function __construct(?string $catchedVariableName) + public function __construct(string $catchedVariableName) { $this->catchedVariableName = $catchedVariableName; } @@ -66,18 +71,14 @@ public function leaveNode(Node $node) } } - if (PHP_VERSION_ID >= 80000 && is_null($this->catchedVariableName)) { - $this->exceptionUsedCount--; - } - - if ($node instanceof Node\Stmt\Throw_ && $this->exceptionUsedCount === 0) { + if ($node instanceof Node\Expr\Throw_ && $this->exceptionUsedCount === 0) { $this->unusedThrows[] = $node; } return null; } /** - * @return Node\Stmt\Throw_[] + * @return Node\Expr\Throw_[] */ public function getUnusedThrows(): array { @@ -94,7 +95,11 @@ public function getUnusedThrows(): array $errors = []; foreach ($visitor->getUnusedThrows() as $throw) { - $errors[] = sprintf('Thrown exceptions in a catch block must bundle the previous exception (see throw statement line %d). More info: http://bit.ly/bundleexception', $throw->getLine()); + $message = sprintf('Thrown exceptions in a catch block must bundle the previous exception (see throw statement line %d). More info: http://bit.ly/bundleexception', $throw->getLine()); + $errors[] = RuleErrorBuilder::message($message) + ->line($node->getStartLine()) + ->file($scope->getFile()) + ->build(); } return $errors; diff --git a/src/Rules/Superglobals/NoSuperglobalsRule.php b/src/Rules/Superglobals/NoSuperglobalsRule.php index 2bd72fa..bd18d89 100644 --- a/src/Rules/Superglobals/NoSuperglobalsRule.php +++ b/src/Rules/Superglobals/NoSuperglobalsRule.php @@ -5,10 +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 PHPStan\Rules\RuleError; +use PHPStan\Rules\RuleErrorBuilder; use TheCodingMachine\PHPStan\Utils\PrefixGenerator; /** @@ -26,7 +25,7 @@ public function getNodeType(): string /** * @param Node\Expr\Variable $node * @param \PHPStan\Analyser\Scope $scope - * @return string[] + * @return RuleError[] */ public function processNode(Node $node, Scope $scope): array { @@ -42,7 +41,13 @@ public function processNode(Node $node, Scope $scope): array ]; if (\in_array($node->name, $forbiddenGlobals, true)) { - return [PrefixGenerator::generatePrefix($scope).'you should not use the $'.$node->name.' superglobal. You should instead rely on your framework that provides you with a "request" object (for instance a PSR-7 RequestInterface or a Symfony Request). More info: http://bit.ly/nosuperglobals']; + $message = PrefixGenerator::generatePrefix($scope).'you should not use the $'.$node->name.' superglobal. You should instead rely on your framework that provides you with a "request" object (for instance a PSR-7 RequestInterface or a Symfony Request). More info: http://bit.ly/nosuperglobals'; + return [ + RuleErrorBuilder::message($message) + ->line($node->getStartLine()) + ->file($scope->getFile()) + ->build(), + ]; } return []; diff --git a/tests/Rules/Conditionals/SwitchMustContainDefaultRuleTest.php b/tests/Rules/Conditionals/SwitchMustContainDefaultRuleTest.php index b590c9a..b358180 100644 --- a/tests/Rules/Conditionals/SwitchMustContainDefaultRuleTest.php +++ b/tests/Rules/Conditionals/SwitchMustContainDefaultRuleTest.php @@ -15,7 +15,8 @@ public function testProcessNode() { $this->analyse([__DIR__ . '/data/switch.php'], [ [ - 'In function "baz", switch statement does not have a "default" case. If your code is supposed to enter at least one "case" or another, consider adding a "default" case that throws an exception. More info: http://bit.ly/switchdefault', + "In function \"baz\", switch statement does not have a \"default\" case.\n" . + " 💡 If your code is supposed to enter at least one \"case\" or another, consider adding a \"default\" case that throws an exception. More info: http://bit.ly/switchdefault", 11, ], ]); diff --git a/tests/Rules/Exceptions/DoNotThrowExceptionBaseClassRuleTest.php b/tests/Rules/Exceptions/DoNotThrowExceptionBaseClassRuleTest.php index d6cdbb8..18935d1 100644 --- a/tests/Rules/Exceptions/DoNotThrowExceptionBaseClassRuleTest.php +++ b/tests/Rules/Exceptions/DoNotThrowExceptionBaseClassRuleTest.php @@ -16,7 +16,7 @@ public function testCheckCatchedException() { $this->analyse([__DIR__ . '/data/throw_exception.php'], [ [ - 'Do not throw the \Exception base class. Instead, extend the \Exception base class. More info: http://bit.ly/subtypeexception', + "Do not throw the \\Exception base class.\n 💡 Instead, extend the \\Exception base class. More info: http://bit.ly/subtypeexception", 16, ], ]); diff --git a/tests/Rules/Exceptions/EmptyExceptionRuleTest.php b/tests/Rules/Exceptions/EmptyExceptionRuleTest.php index ef2260f..b47b8ca 100644 --- a/tests/Rules/Exceptions/EmptyExceptionRuleTest.php +++ b/tests/Rules/Exceptions/EmptyExceptionRuleTest.php @@ -16,11 +16,11 @@ public function testCheckCatchedException() { $this->analyse([__DIR__ . '/data/catch.php'], [ [ - 'Empty catch block. If you are sure this is meant to be empty, please add a "// @ignoreException" comment in the catch block.', + "Empty catch block.\n 💡 If you are sure this is meant to be empty, please add a \"// @ignoreException\" comment in the catch block.", 14, ], [ - 'Empty catch block. If you are sure this is meant to be empty, please add a "// @ignoreException" comment in the catch block.', + "Empty catch block.\n 💡 If you are sure this is meant to be empty, please add a \"// @ignoreException\" comment in the catch block.", 24, ], ]); diff --git a/tests/Rules/Exceptions/data/throw_exception.php b/tests/Rules/Exceptions/data/throw_exception.php index bd19510..eb36d22 100644 --- a/tests/Rules/Exceptions/data/throw_exception.php +++ b/tests/Rules/Exceptions/data/throw_exception.php @@ -19,7 +19,8 @@ function bar() function baz() { try { - //... + // We need to do something or phpstan will skip the evaluation of the catch block + bar(); } catch (\Exception $e) { // This is ok throw $e;