From 908e232dab1297e5ad6e492b5dfa2d828727915d Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sun, 28 May 2023 18:00:30 +0200 Subject: [PATCH] Error identifiers --- src/Rules/PHPUnit/AnnotationHelper.php | 6 +-- .../PHPUnit/AssertSameBooleanExpectedRule.php | 4 +- .../PHPUnit/AssertSameNullExpectedRule.php | 2 +- src/Rules/PHPUnit/AssertSameWithCountRule.php | 8 ++- src/Rules/PHPUnit/ClassCoversExistsRule.php | 14 +++--- .../PHPUnit/ClassMethodCoversExistsRule.php | 4 +- src/Rules/PHPUnit/CoversHelper.php | 14 +++--- src/Rules/PHPUnit/DataProviderHelper.php | 49 ++++++++++++------- src/Rules/PHPUnit/MockMethodCallRule.php | 4 +- .../PHPUnit/ShouldCallParentMethodsRule.php | 2 +- 10 files changed, 64 insertions(+), 43 deletions(-) diff --git a/src/Rules/PHPUnit/AnnotationHelper.php b/src/Rules/PHPUnit/AnnotationHelper.php index f5529a8..4335bc8 100644 --- a/src/Rules/PHPUnit/AnnotationHelper.php +++ b/src/Rules/PHPUnit/AnnotationHelper.php @@ -3,7 +3,7 @@ namespace PHPStan\Rules\PHPUnit; use PhpParser\Comment\Doc; -use PHPStan\Rules\RuleError; +use PHPStan\Rules\IdentifierRuleError; use PHPStan\Rules\RuleErrorBuilder; use function array_key_exists; use function in_array; @@ -30,7 +30,7 @@ class AnnotationHelper ]; /** - * @return RuleError[] errors + * @return list errors */ public function processDocComment(Doc $docComment): array { @@ -57,7 +57,7 @@ public function processDocComment(Doc $docComment): array $errors[] = RuleErrorBuilder::message( 'Annotation "' . $matches['annotation'] . '" is invalid, "@' . $matches['property'] . '" should be followed by a space and a value.' - )->build(); + )->identifier('phpunit.invalidPhpDoc')->build(); } return $errors; diff --git a/src/Rules/PHPUnit/AssertSameBooleanExpectedRule.php b/src/Rules/PHPUnit/AssertSameBooleanExpectedRule.php index b286067..969f7b3 100644 --- a/src/Rules/PHPUnit/AssertSameBooleanExpectedRule.php +++ b/src/Rules/PHPUnit/AssertSameBooleanExpectedRule.php @@ -41,13 +41,13 @@ public function processNode(Node $node, Scope $scope): array if ($expectedArgumentValue->name->toLowerString() === 'true') { return [ - RuleErrorBuilder::message('You should use assertTrue() instead of assertSame() when expecting "true"')->build(), + RuleErrorBuilder::message('You should use assertTrue() instead of assertSame() when expecting "true"')->identifier('phpunit.assertTrue')->build(), ]; } if ($expectedArgumentValue->name->toLowerString() === 'false') { return [ - RuleErrorBuilder::message('You should use assertFalse() instead of assertSame() when expecting "false"')->build(), + RuleErrorBuilder::message('You should use assertFalse() instead of assertSame() when expecting "false"')->identifier('phpunit.assertFalse')->build(), ]; } diff --git a/src/Rules/PHPUnit/AssertSameNullExpectedRule.php b/src/Rules/PHPUnit/AssertSameNullExpectedRule.php index cf6fc76..a2d0cfa 100644 --- a/src/Rules/PHPUnit/AssertSameNullExpectedRule.php +++ b/src/Rules/PHPUnit/AssertSameNullExpectedRule.php @@ -41,7 +41,7 @@ public function processNode(Node $node, Scope $scope): array if ($expectedArgumentValue->name->toLowerString() === 'null') { return [ - RuleErrorBuilder::message('You should use assertNull() instead of assertSame(null, $actual).')->build(), + RuleErrorBuilder::message('You should use assertNull() instead of assertSame(null, $actual).')->identifier('phpunit.assertNull')->build(), ]; } diff --git a/src/Rules/PHPUnit/AssertSameWithCountRule.php b/src/Rules/PHPUnit/AssertSameWithCountRule.php index 209614a..38fdf9a 100644 --- a/src/Rules/PHPUnit/AssertSameWithCountRule.php +++ b/src/Rules/PHPUnit/AssertSameWithCountRule.php @@ -43,7 +43,9 @@ public function processNode(Node $node, Scope $scope): array && $right->name->toLowerString() === 'count' ) { return [ - RuleErrorBuilder::message('You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, count($variable)).')->build(), + RuleErrorBuilder::message('You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, count($variable)).') + ->identifier('phpunit.assertCount') + ->build(), ]; } @@ -57,7 +59,9 @@ public function processNode(Node $node, Scope $scope): array if ((new ObjectType(Countable::class))->isSuperTypeOf($type)->yes()) { return [ - RuleErrorBuilder::message('You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, $variable->count()).')->build(), + RuleErrorBuilder::message('You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, $variable->count()).') + ->identifier('phpunit.assertCount') + ->build(), ]; } } diff --git a/src/Rules/PHPUnit/ClassCoversExistsRule.php b/src/Rules/PHPUnit/ClassCoversExistsRule.php index a5a0cb7..bbced02 100644 --- a/src/Rules/PHPUnit/ClassCoversExistsRule.php +++ b/src/Rules/PHPUnit/ClassCoversExistsRule.php @@ -56,18 +56,18 @@ public function processNode(Node $node, Scope $scope): array return []; } - $errors = []; $classPhpDoc = $classReflection->getResolvedPhpDoc(); [$classCovers, $classCoversDefaultClasses] = $this->coversHelper->getCoverAnnotations($classPhpDoc); if (count($classCoversDefaultClasses) >= 2) { - $errors[] = RuleErrorBuilder::message(sprintf( - '@coversDefaultClass is defined multiple times.' - ))->build(); - - return $errors; + return [ + RuleErrorBuilder::message(sprintf( + '@coversDefaultClass is defined multiple times.' + ))->identifier('phpunit.coversDuplicate')->build(), + ]; } + $errors = []; $coversDefaultClass = array_shift($classCoversDefaultClasses); if ($coversDefaultClass !== null) { @@ -76,7 +76,7 @@ public function processNode(Node $node, Scope $scope): array $errors[] = RuleErrorBuilder::message(sprintf( '@coversDefaultClass references an invalid class %s.', $className - ))->build(); + ))->identifier('phpunit.coversClass')->build(); } } diff --git a/src/Rules/PHPUnit/ClassMethodCoversExistsRule.php b/src/Rules/PHPUnit/ClassMethodCoversExistsRule.php index 69693de..92cb1e2 100644 --- a/src/Rules/PHPUnit/ClassMethodCoversExistsRule.php +++ b/src/Rules/PHPUnit/ClassMethodCoversExistsRule.php @@ -94,7 +94,7 @@ public function processNode(Node $node, Scope $scope): array $errors[] = RuleErrorBuilder::message(sprintf( '@coversDefaultClass defined on class method %s.', $node->name - ))->build(); + ))->identifier('phpunit.covers')->build(); } foreach ($methodCovers as $covers) { @@ -102,7 +102,7 @@ public function processNode(Node $node, Scope $scope): array $errors[] = RuleErrorBuilder::message(sprintf( 'Class already @covers %s so the method @covers is redundant.', $covers->value - ))->build(); + ))->identifier('phpunit.coversDuplicate')->build(); } $errors = array_merge( diff --git a/src/Rules/PHPUnit/CoversHelper.php b/src/Rules/PHPUnit/CoversHelper.php index f17a206..55dbe8d 100644 --- a/src/Rules/PHPUnit/CoversHelper.php +++ b/src/Rules/PHPUnit/CoversHelper.php @@ -7,7 +7,7 @@ use PHPStan\PhpDoc\ResolvedPhpDocBlock; use PHPStan\PhpDocParser\Ast\PhpDoc\PhpDocTagNode; use PHPStan\Reflection\ReflectionProvider; -use PHPStan\Rules\RuleError; +use PHPStan\Rules\IdentifierRuleError; use PHPStan\Rules\RuleErrorBuilder; use function array_merge; use function explode; @@ -61,7 +61,7 @@ public function getCoverAnnotations(?ResolvedPhpDocBlock $phpDoc): array } /** - * @return RuleError[] errors + * @return list errors */ public function processCovers( Node $node, @@ -73,7 +73,9 @@ public function processCovers( $covers = (string) $phpDocTag->value; if ($covers === '') { - $errors[] = RuleErrorBuilder::message('@covers value does not specify anything.')->build(); + $errors[] = RuleErrorBuilder::message('@covers value does not specify anything.') + ->identifier('phpunit.covers') + ->build(); return $errors; } @@ -99,14 +101,14 @@ public function processCovers( $errors[] = RuleErrorBuilder::message(sprintf( '@covers value %s references an interface.', $fullName - ))->build(); + ))->identifier('phpunit.coversInterface')->build(); } if (isset($method) && $method !== '' && !$class->hasMethod($method)) { $errors[] = RuleErrorBuilder::message(sprintf( '@covers value %s references an invalid method.', $fullName - ))->build(); + ))->identifier('phpunit.coversMethod')->build(); } } elseif (isset($method) && $this->reflectionProvider->hasFunction(new Name($method, []), null)) { return $errors; @@ -117,7 +119,7 @@ public function processCovers( '@covers value %s references an invalid %s.', $fullName, $isMethod ? 'method' : 'class or function' - )); + ))->identifier(sprintf('phpunit.covers%s', $isMethod ? 'Method' : '')); if (strpos($className, '\\') === false) { $error->tip('The @covers annotation requires a fully qualified name.'); diff --git a/src/Rules/PHPUnit/DataProviderHelper.php b/src/Rules/PHPUnit/DataProviderHelper.php index 6651b05..9d1b160 100644 --- a/src/Rules/PHPUnit/DataProviderHelper.php +++ b/src/Rules/PHPUnit/DataProviderHelper.php @@ -13,7 +13,7 @@ use PHPStan\Reflection\ClassReflection; use PHPStan\Reflection\MissingMethodFromReflectionException; use PHPStan\Reflection\ReflectionProvider; -use PHPStan\Rules\RuleError; +use PHPStan\Rules\IdentifierRuleError; use PHPStan\Rules\RuleErrorBuilder; use PHPStan\Type\FileTypeMapper; use function array_merge; @@ -130,7 +130,7 @@ private function getDataProviderAnnotations(?ResolvedPhpDocBlock $phpDoc): array } /** - * @return RuleError[] errors + * @return list errors */ public function processDataProvider( string $dataProviderValue, @@ -142,23 +142,29 @@ public function processDataProvider( ): array { if ($classReflection === null) { - $error = RuleErrorBuilder::message(sprintf( - '@dataProvider %s related class not found.', - $dataProviderValue - ))->line($lineNumber)->build(); - - return [$error]; + return [ + RuleErrorBuilder::message(sprintf( + '@dataProvider %s related class not found.', + $dataProviderValue + )) + ->line($lineNumber) + ->identifier('phpunit.dataProviderClass') + ->build(), + ]; } try { $dataProviderMethodReflection = $classReflection->getNativeMethod($methodName); } catch (MissingMethodFromReflectionException $missingMethodFromReflectionException) { - $error = RuleErrorBuilder::message(sprintf( - '@dataProvider %s related method not found.', - $dataProviderValue - ))->line($lineNumber)->build(); - - return [$error]; + return [ + RuleErrorBuilder::message(sprintf( + '@dataProvider %s related method not found.', + $dataProviderValue + )) + ->line($lineNumber) + ->identifier('phpunit.dataProviderMethod') + ->build(), + ]; } $errors = []; @@ -168,21 +174,30 @@ public function processDataProvider( '@dataProvider %s related method is used with incorrect case: %s.', $dataProviderValue, $dataProviderMethodReflection->getName() - ))->line($lineNumber)->build(); + )) + ->line($lineNumber) + ->identifier('method.nameCase') + ->build(); } if (!$dataProviderMethodReflection->isPublic()) { $errors[] = RuleErrorBuilder::message(sprintf( '@dataProvider %s related method must be public.', $dataProviderValue - ))->line($lineNumber)->build(); + )) + ->line($lineNumber) + ->identifier('phpunit.dataProviderPublic') + ->build(); } if ($deprecationRulesInstalled && $this->phpunit10OrNewer && !$dataProviderMethodReflection->isStatic()) { $errors[] = RuleErrorBuilder::message(sprintf( '@dataProvider %s related method must be static in PHPUnit 10 and newer.', $dataProviderValue - ))->line($lineNumber)->build(); + )) + ->line($lineNumber) + ->identifier('phpunit.dataProviderStatic') + ->build(); } return $errors; diff --git a/src/Rules/PHPUnit/MockMethodCallRule.php b/src/Rules/PHPUnit/MockMethodCallRule.php index 90dc094..f93ef16 100644 --- a/src/Rules/PHPUnit/MockMethodCallRule.php +++ b/src/Rules/PHPUnit/MockMethodCallRule.php @@ -65,7 +65,7 @@ public function processNode(Node $node, Scope $scope): array 'Trying to mock an undefined method %s() on class %s.', $method, implode('&', $mockClasses) - ))->build(); + ))->identifier('phpunit.mockMethod')->build(); continue; } @@ -83,7 +83,7 @@ public function processNode(Node $node, Scope $scope): array 'Trying to mock an undefined method %s() on class %s.', $method, implode('|', $classNames) - ))->build(); + ))->identifier('phpunit.mockMethod')->build(); } return $errors; diff --git a/src/Rules/PHPUnit/ShouldCallParentMethodsRule.php b/src/Rules/PHPUnit/ShouldCallParentMethodsRule.php index 5a640a1..917d0bf 100644 --- a/src/Rules/PHPUnit/ShouldCallParentMethodsRule.php +++ b/src/Rules/PHPUnit/ShouldCallParentMethodsRule.php @@ -57,7 +57,7 @@ public function processNode(Node $node, Scope $scope): array return [ RuleErrorBuilder::message( sprintf('Missing call to parent::%s() method.', $methodName) - )->build(), + )->identifier('phpunit.callParent')->build(), ]; }