Allow specifying errorIdentifier to RuleError

Ultimately every error in PHPStan should have its own unique identifier so that its possible to group by error type and ignore errors based on identifier. phpstan/phpstan#1686 (comment) phpstan/phpstan#3065
spaze · Dec 13, 2021 · 35af166 · 35af166
1 parent fda967a
commit 35af166
Show file tree

Hide file tree

Showing 23 changed files with 97 additions and 34 deletions.
diff --git a/README.md b/README.md
@@ -100,6 +100,7 @@ parameters:
         -
             method: 'Redis::connect()'
             message: 'use our own Redis instead'
+            errorIdentifier: 'redis.connect'
 
     disallowedStaticCalls:
         -
@@ -136,6 +137,8 @@ parameters:
 
 The `message` key is optional. Functions and methods can be specified with or without `()`. Omitting `()` is not recommended though to avoid confusing method calls with class constants.
 
+The `errorIdentifier` key is optional. It can be used to provide a unique identifier to the PHPStan error.
+
 Use wildcard (`*`) to ignore all functions, methods, namespaces starting with a prefix, for example:
 ```neon
 parameters:

diff --git a/phpstan.neon b/phpstan.neon
@@ -3,7 +3,7 @@ parameters:
 		- src
 	level: max
 	typeAliases:
-		ForbiddenCallsConfig: 'array<array{function?:string, method?:string, message?:string, allowIn?:string[], allowInFunctions?:string[], allowInMethods?:string[], allowParamsInAllowed?:array<integer, integer|boolean|string>, allowParamsInAllowedAnyValue?:array<integer, integer>, allowParamsAnywhere?:array<integer, integer|boolean|string>, allowParamsAnywhereAnyValue?:array<integer, integer>, allowExceptParamsInAllowed?:array<integer, integer|boolean|string>, allowExceptParams?:array<integer, integer|boolean|string>, allowExceptCaseInsensitiveParams?:array<integer, integer|boolean|string>}>'
+		ForbiddenCallsConfig: 'array<array{function?:string, method?:string, message?:string, allowIn?:string[], allowInFunctions?:string[], allowInMethods?:string[], allowParamsInAllowed?:array<integer, integer|boolean|string>, allowParamsInAllowedAnyValue?:array<integer, integer>, allowParamsAnywhere?:array<integer, integer|boolean|string>, allowParamsAnywhereAnyValue?:array<integer, integer>, allowExceptParamsInAllowed?:array<integer, integer|boolean|string>, allowExceptParams?:array<integer, integer|boolean|string>, allowExceptCaseInsensitiveParams?:array<integer, integer|boolean|string>, errorIdentifier?:string}>'
 
 includes:
 	- vendor/phpstan/phpstan/conf/bleedingEdge.neon

diff --git a/src/Calls/EchoCalls.php b/src/Calls/EchoCalls.php
@@ -7,6 +7,7 @@
 use PhpParser\Node\Stmt\Echo_;
 use PHPStan\Analyser\Scope;
 use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleError;
 use PHPStan\ShouldNotHappenException;
 use Spaze\PHPStan\Rules\Disallowed\DisallowedCall;
 use Spaze\PHPStan\Rules\Disallowed\DisallowedCallFactory;
@@ -52,7 +53,7 @@ public function getNodeType(): string
 	/**
 	 * @param Echo_ $node
 	 * @param Scope $scope
-	 * @return string[]
+	 * @return RuleError[]
 	 */
 	public function processNode(Node $node, Scope $scope): array
 	{

diff --git a/src/Calls/EmptyCalls.php b/src/Calls/EmptyCalls.php
@@ -7,6 +7,7 @@
 use PhpParser\Node\Expr\Empty_;
 use PHPStan\Analyser\Scope;
 use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleError;
 use PHPStan\ShouldNotHappenException;
 use Spaze\PHPStan\Rules\Disallowed\DisallowedCall;
 use Spaze\PHPStan\Rules\Disallowed\DisallowedCallFactory;
@@ -52,7 +53,7 @@ public function getNodeType(): string
 	/**
 	 * @param Empty_ $node
 	 * @param Scope $scope
-	 * @return string[]
+	 * @return RuleError[]
 	 */
 	public function processNode(Node $node, Scope $scope): array
 	{

diff --git a/src/Calls/EvalCalls.php b/src/Calls/EvalCalls.php
@@ -7,6 +7,7 @@
 use PhpParser\Node\Expr\Eval_;
 use PHPStan\Analyser\Scope;
 use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleError;
 use PHPStan\ShouldNotHappenException;
 use Spaze\PHPStan\Rules\Disallowed\DisallowedCall;
 use Spaze\PHPStan\Rules\Disallowed\DisallowedCallFactory;
@@ -52,7 +53,7 @@ public function getNodeType(): string
 	/**
 	 * @param Eval_ $node
 	 * @param Scope $scope
-	 * @return string[]
+	 * @return RuleError[]
 	 */
 	public function processNode(Node $node, Scope $scope): array
 	{

diff --git a/src/Calls/ExitDieCalls.php b/src/Calls/ExitDieCalls.php
@@ -7,6 +7,7 @@
 use PhpParser\Node\Expr\Exit_;
 use PHPStan\Analyser\Scope;
 use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleError;
 use PHPStan\ShouldNotHappenException;
 use Spaze\PHPStan\Rules\Disallowed\DisallowedCall;
 use Spaze\PHPStan\Rules\Disallowed\DisallowedCallFactory;
@@ -52,7 +53,7 @@ public function getNodeType(): string
 	/**
 	 * @param Exit_ $node
 	 * @param Scope $scope
-	 * @return string[]
+	 * @return RuleError[]
 	 */
 	public function processNode(Node $node, Scope $scope): array
 	{

diff --git a/src/Calls/FunctionCalls.php b/src/Calls/FunctionCalls.php
@@ -8,6 +8,7 @@
 use PhpParser\Node\Name;
 use PHPStan\Analyser\Scope;
 use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleError;
 use PHPStan\ShouldNotHappenException;
 use Spaze\PHPStan\Rules\Disallowed\DisallowedCall;
 use Spaze\PHPStan\Rules\Disallowed\DisallowedCallFactory;
@@ -53,7 +54,7 @@ public function getNodeType(): string
 	/**
 	 * @param FuncCall $node
 	 * @param Scope $scope
-	 * @return string[]
+	 * @return RuleError[]
 	 */
 	public function processNode(Node $node, Scope $scope): array
 	{

diff --git a/src/Calls/MethodCalls.php b/src/Calls/MethodCalls.php
@@ -8,6 +8,7 @@
 use PHPStan\Analyser\Scope;
 use PHPStan\Broker\ClassNotFoundException;
 use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleError;
 use PHPStan\ShouldNotHappenException;
 use Spaze\PHPStan\Rules\Disallowed\DisallowedCall;
 use Spaze\PHPStan\Rules\Disallowed\DisallowedCallFactory;
@@ -55,7 +56,7 @@ public function getNodeType(): string
 	/**
 	 * @param MethodCall $node
 	 * @param Scope $scope
-	 * @return string[]
+	 * @return RuleError[]
 	 * @throws ClassNotFoundException
 	 */
 	public function processNode(Node $node, Scope $scope): array

diff --git a/src/Calls/NewCalls.php b/src/Calls/NewCalls.php
@@ -9,6 +9,7 @@
 use PhpParser\Node\Name;
 use PHPStan\Analyser\Scope;
 use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleError;
 use PHPStan\ShouldNotHappenException;
 use PHPStan\Type\ConstantScalarType;
 use Spaze\PHPStan\Rules\Disallowed\DisallowedCall;
@@ -55,7 +56,7 @@ public function getNodeType(): string
 	/**
 	 * @param New_ $node
 	 * @param Scope $scope
-	 * @return string[]
+	 * @return RuleError[]
 	 */
 	public function processNode(Node $node, Scope $scope): array
 	{

diff --git a/src/Calls/PrintCalls.php b/src/Calls/PrintCalls.php
@@ -7,6 +7,7 @@
 use PhpParser\Node\Expr\Print_;
 use PHPStan\Analyser\Scope;
 use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleError;
 use PHPStan\ShouldNotHappenException;
 use Spaze\PHPStan\Rules\Disallowed\DisallowedCall;
 use Spaze\PHPStan\Rules\Disallowed\DisallowedCallFactory;
@@ -52,7 +53,7 @@ public function getNodeType(): string
 	/**
 	 * @param Print_ $node
 	 * @param Scope $scope
-	 * @return string[]
+	 * @return RuleError[]
 	 */
 	public function processNode(Node $node, Scope $scope): array
 	{

diff --git a/src/Calls/ShellExecCalls.php b/src/Calls/ShellExecCalls.php
@@ -7,6 +7,7 @@
 use PhpParser\Node\Expr\ShellExec;
 use PHPStan\Analyser\Scope;
 use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleError;
 use PHPStan\ShouldNotHappenException;
 use Spaze\PHPStan\Rules\Disallowed\DisallowedCall;
 use Spaze\PHPStan\Rules\Disallowed\DisallowedCallFactory;
@@ -56,7 +57,7 @@ public function getNodeType(): string
 	/**
 	 * @param ShellExec $node
 	 * @param Scope $scope
-	 * @return string[]
+	 * @return RuleError[]
 	 */
 	public function processNode(Node $node, Scope $scope): array
 	{

diff --git a/src/Calls/StaticCalls.php b/src/Calls/StaticCalls.php
@@ -8,6 +8,7 @@
 use PHPStan\Analyser\Scope;
 use PHPStan\Broker\ClassNotFoundException;
 use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleError;
 use PHPStan\ShouldNotHappenException;
 use Spaze\PHPStan\Rules\Disallowed\DisallowedCall;
 use Spaze\PHPStan\Rules\Disallowed\DisallowedCallFactory;
@@ -55,7 +56,7 @@ public function getNodeType(): string
 	/**
 	 * @param StaticCall $node
 	 * @param Scope $scope
-	 * @return string[]
+	 * @return RuleError[]
 	 * @throws ClassNotFoundException
 	 */
 	public function processNode(Node $node, Scope $scope): array

diff --git a/src/DisallowedCall.php b/src/DisallowedCall.php
@@ -32,6 +32,9 @@ class DisallowedCall
 	/** @var array<int, DisallowedCallParam> */
 	private $allowExceptParams;
 
+	/** @var string */
+	private $errorIdentifier;
+
 
 	/**
 	 * DisallowedCall constructor.
@@ -44,8 +47,9 @@ class DisallowedCall
 	 * @param array<int, DisallowedCallParam> $allowParamsAnywhere
 	 * @param array<int, DisallowedCallParam> $allowExceptParamsInAllowed
 	 * @param array<int, DisallowedCallParam> $allowExceptParams
+	 * @param string $errorIdentifier
 	 */
-	public function __construct(string $call, ?string $message, array $allowIn, array $allowInCalls, array $allowParamsInAllowed, array $allowParamsAnywhere, array $allowExceptParamsInAllowed, array $allowExceptParams)
+	public function __construct(string $call, ?string $message, array $allowIn, array $allowInCalls, array $allowParamsInAllowed, array $allowParamsAnywhere, array $allowExceptParamsInAllowed, array $allowExceptParams, string $errorIdentifier)
 	{
 		$this->call = $call;
 		$this->message = $message;
@@ -55,6 +59,7 @@ public function __construct(string $call, ?string $message, array $allowIn, arra
 		$this->allowParamsAnywhere = $allowParamsAnywhere;
 		$this->allowExceptParamsInAllowed = $allowExceptParamsInAllowed;
 		$this->allowExceptParams = $allowExceptParams;
+		$this->errorIdentifier = $errorIdentifier;
 	}
 
 
@@ -124,6 +129,12 @@ public function getAllowExceptParams(): array
 	}
 
 
+	public function getErrorIdentifier(): string
+	{
+		return $this->errorIdentifier;
+	}
+
+
 	public function getKey(): string
 	{
 		// The key consists of "initial" config values that would be overwritten with more specific details in a custom config.

diff --git a/src/DisallowedCallFactory.php b/src/DisallowedCallFactory.php
@@ -61,7 +61,8 @@ public function createFromConfig(array $config): array
 				$allowParamsInAllowed,
 				$allowParamsAnywhere,
 				$allowExceptParamsInAllowed,
-				$allowExceptParams
+				$allowExceptParams,
+				$disallowedCall['errorIdentifier'] ?? ''
 			);
 			$calls[$disallowedCall->getKey()] = $disallowedCall;
 		}

diff --git a/src/DisallowedConstant.php b/src/DisallowedConstant.php
@@ -15,19 +15,24 @@ class DisallowedConstant
 	/** @var string[] */
 	private $allowIn;
 
+	/** @var string */
+	private $errorIdentifier;
+
 
 	/**
 	 * DisallowedCall constructor.
 	 *
 	 * @param string $constant
 	 * @param string|null $message
 	 * @param string[] $allowIn
+	 * @param string $errorIdentifier
 	 */
-	public function __construct(string $constant, ?string $message, array $allowIn)
+	public function __construct(string $constant, ?string $message, array $allowIn, string $errorIdentifier)
 	{
 		$this->constant = ltrim($constant, '\\');
 		$this->message = $message;
 		$this->allowIn = $allowIn;
+		$this->errorIdentifier = $errorIdentifier;
 	}
 
 
@@ -51,4 +56,10 @@ public function getAllowIn(): array
 		return $this->allowIn;
 	}
 
+
+	public function getErrorIdentifier(): string
+	{
+		return $this->errorIdentifier;
+	}
+
 }
diff --git a/src/DisallowedConstantFactory.php b/src/DisallowedConstantFactory.php
@@ -9,7 +9,7 @@ class DisallowedConstantFactory
 {
 
 	/**
-	 * @param array<array{class?:string, constant?:string, message?:string, allowIn?:string[]}> $config
+	 * @param array<array{class?:string, constant?:string, message?:string, allowIn?:string[], errorIdentifier?:string}> $config
 	 * @return DisallowedConstant[]
 	 * @throws ShouldNotHappenException
 	 */
@@ -25,7 +25,8 @@ public function createFromConfig(array $config): array
 			$disallowedConstant = new DisallowedConstant(
 				$class ? "{$class}::{$constant}" : $constant,
 				$disallowedConstant['message'] ?? null,
-				$disallowedConstant['allowIn'] ?? []
+				$disallowedConstant['allowIn'] ?? [],
+				$disallowedConstant['errorIdentifier'] ?? ''
 			);
 			$constants[$disallowedConstant->getConstant()] = $disallowedConstant;
 		}

diff --git a/src/DisallowedHelper.php b/src/DisallowedHelper.php
@@ -11,6 +11,8 @@
 use PHPStan\Broker\ClassNotFoundException;
 use PHPStan\Reflection\FunctionReflection;
 use PHPStan\Reflection\MethodReflection;
+use PHPStan\Rules\RuleError;
+use PHPStan\Rules\RuleErrorBuilder;
 use PHPStan\Type\ConstantScalarType;
 use PHPStan\Type\ObjectType;
 use PHPStan\Type\Type;
@@ -111,19 +113,21 @@ private function getArgType(CallLike $node, Scope $scope, int $param): ?Type
 	 * @param string|null $displayName
 	 * @param DisallowedCall[] $disallowedCalls
 	 * @param string|null $message
-	 * @return string[]
+	 * @return RuleError[]
 	 */
 	public function getDisallowedMessage(?CallLike $node, Scope $scope, string $name, ?string $displayName, array $disallowedCalls, ?string $message = null): array
 	{
 		foreach ($disallowedCalls as $disallowedCall) {
 			if ($this->callMatches($disallowedCall, $name) && !$this->isAllowed($scope, $node, $disallowedCall)) {
 				return [
-					sprintf(
+					RuleErrorBuilder::message(sprintf(
 						$message ?? 'Calling %s is forbidden, %s%s',
 						($displayName && $displayName !== $name) ? "{$name}() (as {$displayName}())" : "{$name}()",
 						$disallowedCall->getMessage(),
 						$disallowedCall->getCall() !== $name ? " [{$name}() matches {$disallowedCall->getCall()}()]" : ''
-					),
+					))
+						->identifier($disallowedCall->getErrorIdentifier())
+						->build(),
 				];
 			}
 		}
@@ -145,7 +149,7 @@ private function callMatches(DisallowedCall $disallowedCall, string $name): bool
 	 * @param CallLike $node
 	 * @param Scope $scope
 	 * @param DisallowedCall[] $disallowedCalls
-	 * @return string[]
+	 * @return RuleError[]
 	 * @throws ClassNotFoundException
 	 */
 	public function getDisallowedMethodMessage($class, CallLike $node, Scope $scope, array $disallowedCalls): array
@@ -215,19 +219,21 @@ private function isAllowedPath(Scope $scope, DisallowedConstant $disallowedConst
 	 * @param Scope $scope
 	 * @param string|null $displayName
 	 * @param DisallowedConstant[] $disallowedConstants
-	 * @return string[]
+	 * @return RuleError[]
 	 */
 	public function getDisallowedConstantMessage(string $constant, Scope $scope, ?string $displayName, array $disallowedConstants): array
 	{
 		foreach ($disallowedConstants as $disallowedConstant) {
 			if ($disallowedConstant->getConstant() === $constant && !$this->isAllowedPath($scope, $disallowedConstant)) {
 				return [
-					sprintf(
+					RuleErrorBuilder::message(sprintf(
 						'Using %s%s is forbidden, %s',
 						$disallowedConstant->getConstant(),
 						$displayName && $displayName !== $disallowedConstant->getConstant() ? ' (as ' . $displayName . ')' : '',
 						$disallowedConstant->getMessage()
-					),
+					))
+						->identifier($disallowedConstant->getErrorIdentifier())
+						->build(),
 				];
 			}
 		}