thecodingmachine · alekitto · Feb 19, 2025
diff --git a/.gitignore b/.gitignore
@@ -1,7 +1,8 @@
 *~
 /vendor/*
 .*~
+/.phpunit.result.cache
 /composer.lock
 /phpunit.xml
 /tmp
-/build/
+/build/
diff --git 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
@@ -1,4 +1,5 @@
-<phpunit
+<?xml version="1.0"?>
+<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
 	bootstrap="tests/bootstrap.php"
 	colors="true"
 	backupGlobals="false"
@@ -12,25 +13,20 @@
 	convertErrorsToExceptions="true"
 	convertNoticesToExceptions="true"
 	convertWarningsToExceptions="true"
->
+	xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/9.3/phpunit.xsd">
+	<coverage>
+		<include>
+			<directory suffix=".php">./src</directory>
+		</include>
+		<report>
+			<clover outputFile="build/logs/clover.xml"/>
+			<html outputDirectory="build/coverage"/>
+			<text outputFile="php://stdout" showUncoveredFiles="true" showOnlySummary="true"/>
+		</report>
+	</coverage>
 	<testsuites>
 		<testsuite name="Test suite">
 			<directory>./tests/</directory>
 		</testsuite>
 	</testsuites>
-	<filter>
-		<whitelist>
-			<directory suffix=".php">./src</directory>
-		</whitelist>
-	</filter>
-	<logging>
-		<log
-			type="coverage-text"
-			target="php://stdout"
-			showUncoveredFiles="true"
-			showOnlySummary="true"
-		/>
-		<log type="coverage-html" target="build/coverage"/>
-		<log type="coverage-clover" target="build/logs/clover.xml"/>
-	</logging>
 </phpunit>
diff --git 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,23 +24,27 @@ 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;
             }
         }
 
         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
@@ -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<Node\Stmt\Throw_>
+ * @implements Rule<Node\Expr\Throw_>
  */
 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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,
             ],
         ]);
-Original file line number
+Diff line change
@@ Expand Up / @@ -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",
 ,
                 ],
             ]);
@@ Expand Down @@