From b90602913847797f2b009375cb1deeb49a761fc3 Mon Sep 17 00:00:00 2001 From: Fabien Villepinte Date: Wed, 7 Dec 2022 14:38:07 +0100 Subject: [PATCH 1/2] Add rule to check @dataProvider --- extension.neon | 2 + rules.neon | 3 + .../PHPUnit/DataProviderDeclarationRule.php | 81 +++++++++++++ src/Rules/PHPUnit/DataProviderHelper.php | 111 ++++++++++++++++++ .../DataProviderDeclarationRuleTest.php | 54 +++++++++ .../data/data-provider-declaration.php | 70 +++++++++++ 6 files changed, 321 insertions(+) create mode 100644 src/Rules/PHPUnit/DataProviderDeclarationRule.php create mode 100644 src/Rules/PHPUnit/DataProviderHelper.php create mode 100644 tests/Rules/PHPUnit/DataProviderDeclarationRuleTest.php create mode 100644 tests/Rules/PHPUnit/data/data-provider-declaration.php diff --git a/extension.neon b/extension.neon index f6f372e..5c6a90d 100644 --- a/extension.neon +++ b/extension.neon @@ -55,6 +55,8 @@ services: class: PHPStan\Rules\PHPUnit\CoversHelper - class: PHPStan\Rules\PHPUnit\AnnotationHelper + - + class: PHPStan\Rules\PHPUnit\DataProviderHelper conditionalTags: PHPStan\PhpDoc\PHPUnit\MockObjectTypeNodeResolverExtension: diff --git a/rules.neon b/rules.neon index 24a28ea..d8068e3 100644 --- a/rules.neon +++ b/rules.neon @@ -8,6 +8,7 @@ rules: services: - class: PHPStan\Rules\PHPUnit\ClassCoversExistsRule - class: PHPStan\Rules\PHPUnit\ClassMethodCoversExistsRule + - class: PHPStan\Rules\PHPUnit\DataProviderDeclarationRule - class: PHPStan\Rules\PHPUnit\NoMissingSpaceInClassAnnotationRule - class: PHPStan\Rules\PHPUnit\NoMissingSpaceInMethodAnnotationRule @@ -16,6 +17,8 @@ conditionalTags: phpstan.rules.rule: %featureToggles.bleedingEdge% PHPStan\Rules\PHPUnit\ClassMethodCoversExistsRule: phpstan.rules.rule: %featureToggles.bleedingEdge% + PHPStan\Rules\PHPUnit\DataProviderDeclarationRule: + phpstan.rules.rule: %featureToggles.bleedingEdge% PHPStan\Rules\PHPUnit\NoMissingSpaceInClassAnnotationRule: phpstan.rules.rule: %featureToggles.bleedingEdge% PHPStan\Rules\PHPUnit\NoMissingSpaceInMethodAnnotationRule: diff --git a/src/Rules/PHPUnit/DataProviderDeclarationRule.php b/src/Rules/PHPUnit/DataProviderDeclarationRule.php new file mode 100644 index 0000000..865d137 --- /dev/null +++ b/src/Rules/PHPUnit/DataProviderDeclarationRule.php @@ -0,0 +1,81 @@ + + */ +class DataProviderDeclarationRule implements Rule +{ + + /** + * Data provider helper. + * + * @var DataProviderHelper + */ + private $dataProviderHelper; + + /** + * The file type mapper. + * + * @var FileTypeMapper + */ + private $fileTypeMapper; + + public function __construct( + DataProviderHelper $dataProviderHelper, + FileTypeMapper $fileTypeMapper + ) + { + $this->dataProviderHelper = $dataProviderHelper; + $this->fileTypeMapper = $fileTypeMapper; + } + + public function getNodeType(): string + { + return Node\Stmt\ClassMethod::class; + } + + public function processNode(Node $node, Scope $scope): array + { + $classReflection = $scope->getClassReflection(); + + if ($classReflection === null || !$classReflection->isSubclassOf(TestCase::class)) { + return []; + } + + $docComment = $node->getDocComment(); + if ($docComment === null) { + return []; + } + + $methodPhpDoc = $this->fileTypeMapper->getResolvedPhpDoc( + $scope->getFile(), + $classReflection->getName(), + $scope->isInTrait() ? $scope->getTraitReflection()->getName() : null, + $node->name->toString(), + $docComment->getText() + ); + + $annotations = $this->dataProviderHelper->getDataProviderAnnotations($methodPhpDoc); + + $errors = []; + + foreach ($annotations as $annotation) { + $errors = array_merge( + $errors, + $this->dataProviderHelper->processDataProvider($scope, $annotation) + ); + } + + return $errors; + } + +} diff --git a/src/Rules/PHPUnit/DataProviderHelper.php b/src/Rules/PHPUnit/DataProviderHelper.php new file mode 100644 index 0000000..5de5e73 --- /dev/null +++ b/src/Rules/PHPUnit/DataProviderHelper.php @@ -0,0 +1,111 @@ + + */ + public function getDataProviderAnnotations(?ResolvedPhpDocBlock $phpDoc): array + { + if ($phpDoc === null) { + return []; + } + + $phpDocNodes = $phpDoc->getPhpDocNodes(); + + $annotations = []; + + foreach ($phpDocNodes as $docNode) { + $annotations = array_merge( + $annotations, + $docNode->getTagsByName('@dataProvider') + ); + } + + return $annotations; + } + + /** + * @return RuleError[] errors + */ + public function processDataProvider( + Scope $scope, + PhpDocTagNode $phpDocTag + ): array + { + $dataProviderName = $this->getDataProviderName($phpDocTag); + if ($dataProviderName === null) { + // Missing name is already handled in NoMissingSpaceInMethodAnnotationRule + return []; + } + + $classReflection = $scope->getClassReflection(); + if ($classReflection === null) { + // Should not happen + return []; + } + + try { + $dataProviderMethodReflection = $classReflection->getNativeMethod($dataProviderName); + } catch (MissingMethodFromReflectionException $missingMethodFromReflectionException) { + $error = RuleErrorBuilder::message(sprintf( + '@dataProvider %s related method not found.', + $dataProviderName + ))->build(); + + return [$error]; + } + + $errors = []; + + if ($dataProviderName !== $dataProviderMethodReflection->getName()) { + $errors[] = RuleErrorBuilder::message(sprintf( + '@dataProvider %s related method is used with incorrect case: %s.', + $dataProviderName, + $dataProviderMethodReflection->getName() + ))->build(); + } + + if (!$dataProviderMethodReflection->isPublic()) { + $errors[] = RuleErrorBuilder::message(sprintf( + '@dataProvider %s related method must be public.', + $dataProviderName + ))->build(); + } + + if (!$dataProviderMethodReflection->isStatic()) { + $errors[] = RuleErrorBuilder::message(sprintf( + '@dataProvider %s related method must be static.', + $dataProviderName + ))->build(); + } + + return $errors; + } + + private function getDataProviderName(PhpDocTagNode $phpDocTag): ?string + { + $value = trim((string) $phpDocTag->value); + + if (preg_match('/^[\S]+/', $value, $matches) !== 1) { + return null; + } + + return $matches[0]; + } + +} diff --git a/tests/Rules/PHPUnit/DataProviderDeclarationRuleTest.php b/tests/Rules/PHPUnit/DataProviderDeclarationRuleTest.php new file mode 100644 index 0000000..27c3e7d --- /dev/null +++ b/tests/Rules/PHPUnit/DataProviderDeclarationRuleTest.php @@ -0,0 +1,54 @@ + + */ +class DataProviderDeclarationRuleTest extends RuleTestCase +{ + + protected function getRule(): Rule + { + return new DataProviderDeclarationRule( + new DataProviderHelper(), + self::getContainer()->getByType(FileTypeMapper::class) + ); + } + + public function testRule(): void + { + $this->analyse([__DIR__ . '/data/data-provider-declaration.php'], [ + [ + '@dataProvider providebaz related method is used with incorrect case: provideBaz.', + 13, + ], + [ + '@dataProvider provideQux related method must be static.', + 13, + ], + [ + '@dataProvider provideQuux related method must be public.', + 13, + ], + [ + '@dataProvider provideNonExisting related method not found.', + 66, + ], + ]); + } + + /** + * @return string[] + */ + public static function getAdditionalConfigFiles(): array + { + return [ + __DIR__ . '/../../../extension.neon', + ]; + } +} diff --git a/tests/Rules/PHPUnit/data/data-provider-declaration.php b/tests/Rules/PHPUnit/data/data-provider-declaration.php new file mode 100644 index 0000000..2690d02 --- /dev/null +++ b/tests/Rules/PHPUnit/data/data-provider-declaration.php @@ -0,0 +1,70 @@ + Date: Wed, 7 Dec 2022 15:30:57 +0100 Subject: [PATCH 2/2] Add rule to check @dataProvider --- rules.neon | 5 ++++- .../PHPUnit/DataProviderDeclarationRule.php | 13 +++++++++++-- src/Rules/PHPUnit/DataProviderHelper.php | 17 ++++------------- .../PHPUnit/DataProviderDeclarationRuleTest.php | 7 ++----- 4 files changed, 21 insertions(+), 21 deletions(-) diff --git a/rules.neon b/rules.neon index d8068e3..195ace0 100644 --- a/rules.neon +++ b/rules.neon @@ -8,7 +8,10 @@ rules: services: - class: PHPStan\Rules\PHPUnit\ClassCoversExistsRule - class: PHPStan\Rules\PHPUnit\ClassMethodCoversExistsRule - - class: PHPStan\Rules\PHPUnit\DataProviderDeclarationRule + - + class: PHPStan\Rules\PHPUnit\DataProviderDeclarationRule + arguments: + checkFunctionNameCase: %checkFunctionNameCase% - class: PHPStan\Rules\PHPUnit\NoMissingSpaceInClassAnnotationRule - class: PHPStan\Rules\PHPUnit\NoMissingSpaceInMethodAnnotationRule diff --git a/src/Rules/PHPUnit/DataProviderDeclarationRule.php b/src/Rules/PHPUnit/DataProviderDeclarationRule.php index 865d137..7d1afd6 100644 --- a/src/Rules/PHPUnit/DataProviderDeclarationRule.php +++ b/src/Rules/PHPUnit/DataProviderDeclarationRule.php @@ -29,13 +29,22 @@ class DataProviderDeclarationRule implements Rule */ private $fileTypeMapper; + /** + * When set to true, it reports data provider method with incorrect name case. + * + * @var bool + */ + private $checkFunctionNameCase; + public function __construct( DataProviderHelper $dataProviderHelper, - FileTypeMapper $fileTypeMapper + FileTypeMapper $fileTypeMapper, + bool $checkFunctionNameCase ) { $this->dataProviderHelper = $dataProviderHelper; $this->fileTypeMapper = $fileTypeMapper; + $this->checkFunctionNameCase = $checkFunctionNameCase; } public function getNodeType(): string @@ -71,7 +80,7 @@ public function processNode(Node $node, Scope $scope): array foreach ($annotations as $annotation) { $errors = array_merge( $errors, - $this->dataProviderHelper->processDataProvider($scope, $annotation) + $this->dataProviderHelper->processDataProvider($scope, $annotation, $this->checkFunctionNameCase) ); } diff --git a/src/Rules/PHPUnit/DataProviderHelper.php b/src/Rules/PHPUnit/DataProviderHelper.php index 5de5e73..ef3bfcd 100644 --- a/src/Rules/PHPUnit/DataProviderHelper.php +++ b/src/Rules/PHPUnit/DataProviderHelper.php @@ -11,7 +11,6 @@ use function array_merge; use function preg_match; use function sprintf; -use function trim; class DataProviderHelper { @@ -44,7 +43,8 @@ public function getDataProviderAnnotations(?ResolvedPhpDocBlock $phpDoc): array */ public function processDataProvider( Scope $scope, - PhpDocTagNode $phpDocTag + PhpDocTagNode $phpDocTag, + bool $checkFunctionNameCase ): array { $dataProviderName = $this->getDataProviderName($phpDocTag); @@ -72,7 +72,7 @@ public function processDataProvider( $errors = []; - if ($dataProviderName !== $dataProviderMethodReflection->getName()) { + if ($checkFunctionNameCase && $dataProviderName !== $dataProviderMethodReflection->getName()) { $errors[] = RuleErrorBuilder::message(sprintf( '@dataProvider %s related method is used with incorrect case: %s.', $dataProviderName, @@ -87,21 +87,12 @@ public function processDataProvider( ))->build(); } - if (!$dataProviderMethodReflection->isStatic()) { - $errors[] = RuleErrorBuilder::message(sprintf( - '@dataProvider %s related method must be static.', - $dataProviderName - ))->build(); - } - return $errors; } private function getDataProviderName(PhpDocTagNode $phpDocTag): ?string { - $value = trim((string) $phpDocTag->value); - - if (preg_match('/^[\S]+/', $value, $matches) !== 1) { + if (preg_match('/^[^ \t]+/', (string) $phpDocTag->value, $matches) !== 1) { return null; } diff --git a/tests/Rules/PHPUnit/DataProviderDeclarationRuleTest.php b/tests/Rules/PHPUnit/DataProviderDeclarationRuleTest.php index 27c3e7d..44434e3 100644 --- a/tests/Rules/PHPUnit/DataProviderDeclarationRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDeclarationRuleTest.php @@ -16,7 +16,8 @@ protected function getRule(): Rule { return new DataProviderDeclarationRule( new DataProviderHelper(), - self::getContainer()->getByType(FileTypeMapper::class) + self::getContainer()->getByType(FileTypeMapper::class), + true ); } @@ -27,10 +28,6 @@ public function testRule(): void '@dataProvider providebaz related method is used with incorrect case: provideBaz.', 13, ], - [ - '@dataProvider provideQux related method must be static.', - 13, - ], [ '@dataProvider provideQuux related method must be public.', 13,