Skip to content

Commit

Permalink
Error identifiers
Browse files Browse the repository at this point in the history
  • Loading branch information
ondrejmirtes committed Aug 25, 2023
1 parent e468b76 commit 908e232
Show file tree
Hide file tree
Showing 10 changed files with 64 additions and 43 deletions.
6 changes: 3 additions & 3 deletions src/Rules/PHPUnit/AnnotationHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -30,7 +30,7 @@ class AnnotationHelper
];

/**
* @return RuleError[] errors
* @return list<IdentifierRuleError> errors
*/
public function processDocComment(Doc $docComment): array
{
Expand All @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions src/Rules/PHPUnit/AssertSameBooleanExpectedRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
];
}

Expand Down
2 changes: 1 addition & 1 deletion src/Rules/PHPUnit/AssertSameNullExpectedRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
];
}

Expand Down
8 changes: 6 additions & 2 deletions src/Rules/PHPUnit/AssertSameWithCountRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
];
}

Expand All @@ -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(),
];
}
}
Expand Down
14 changes: 7 additions & 7 deletions src/Rules/PHPUnit/ClassCoversExistsRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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();
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/Rules/PHPUnit/ClassMethodCoversExistsRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -94,15 +94,15 @@ 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) {
if (in_array((string) $covers->value, $classCoversStrings, true)) {
$errors[] = RuleErrorBuilder::message(sprintf(
'Class already @covers %s so the method @covers is redundant.',
$covers->value
))->build();
))->identifier('phpunit.coversDuplicate')->build();
}

$errors = array_merge(
Expand Down
14 changes: 8 additions & 6 deletions src/Rules/PHPUnit/CoversHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -61,7 +61,7 @@ public function getCoverAnnotations(?ResolvedPhpDocBlock $phpDoc): array
}

/**
* @return RuleError[] errors
* @return list<IdentifierRuleError> errors
*/
public function processCovers(
Node $node,
Expand All @@ -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;
}
Expand All @@ -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;
Expand All @@ -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.');
Expand Down
49 changes: 32 additions & 17 deletions src/Rules/PHPUnit/DataProviderHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -130,7 +130,7 @@ private function getDataProviderAnnotations(?ResolvedPhpDocBlock $phpDoc): array
}

/**
* @return RuleError[] errors
* @return list<IdentifierRuleError> errors
*/
public function processDataProvider(
string $dataProviderValue,
Expand All @@ -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 = [];
Expand All @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions src/Rules/PHPUnit/MockMethodCallRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/Rules/PHPUnit/ShouldCallParentMethodsRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
];
}

Expand Down

0 comments on commit 908e232

Please sign in to comment.