diff --git a/src/Rules/PHPUnit/DataProviderDeclarationRule.php b/src/Rules/PHPUnit/DataProviderDeclarationRule.php index 612cf06..37c586d 100644 --- a/src/Rules/PHPUnit/DataProviderDeclarationRule.php +++ b/src/Rules/PHPUnit/DataProviderDeclarationRule.php @@ -5,7 +5,6 @@ use PhpParser\Node; use PHPStan\Analyser\Scope; use PHPStan\Rules\Rule; -use PHPStan\Type\FileTypeMapper; use PHPUnit\Framework\TestCase; use function array_merge; @@ -22,13 +21,6 @@ class DataProviderDeclarationRule implements Rule */ private $dataProviderHelper; - /** - * The file type mapper. - * - * @var FileTypeMapper - */ - private $fileTypeMapper; - /** * When set to true, it reports data provider method with incorrect name case. * @@ -45,13 +37,11 @@ class DataProviderDeclarationRule implements Rule public function __construct( DataProviderHelper $dataProviderHelper, - FileTypeMapper $fileTypeMapper, bool $checkFunctionNameCase, bool $deprecationRulesInstalled ) { $this->dataProviderHelper = $dataProviderHelper; - $this->fileTypeMapper = $fileTypeMapper; $this->checkFunctionNameCase = $checkFunctionNameCase; $this->deprecationRulesInstalled = $deprecationRulesInstalled; } @@ -69,29 +59,16 @@ public function processNode(Node $node, Scope $scope): array 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) { + foreach ($this->dataProviderHelper->getDataProviderMethods($scope, $node, $classReflection) as $dataProviderValue => [$dataProviderClassReflection, $dataProviderMethodName, $lineNumber]) { $errors = array_merge( $errors, $this->dataProviderHelper->processDataProvider( - $scope, - $annotation, + $dataProviderValue, + $dataProviderClassReflection, + $dataProviderMethodName, + $lineNumber, $this->checkFunctionNameCase, $this->deprecationRulesInstalled ) diff --git a/src/Rules/PHPUnit/DataProviderHelper.php b/src/Rules/PHPUnit/DataProviderHelper.php index 3007960..6651b05 100644 --- a/src/Rules/PHPUnit/DataProviderHelper.php +++ b/src/Rules/PHPUnit/DataProviderHelper.php @@ -2,6 +2,11 @@ namespace PHPStan\Rules\PHPUnit; +use PhpParser\Node\Attribute; +use PhpParser\Node\Expr\ClassConstFetch; +use PhpParser\Node\Name; +use PhpParser\Node\Scalar\String_; +use PhpParser\Node\Stmt\ClassMethod; use PHPStan\Analyser\Scope; use PHPStan\PhpDoc\ResolvedPhpDocBlock; use PHPStan\PhpDocParser\Ast\PhpDoc\PhpDocTagNode; @@ -10,6 +15,7 @@ use PHPStan\Reflection\ReflectionProvider; use PHPStan\Rules\RuleError; use PHPStan\Rules\RuleErrorBuilder; +use PHPStan\Type\FileTypeMapper; use function array_merge; use function count; use function explode; @@ -26,19 +32,84 @@ class DataProviderHelper */ private $reflectionProvider; + /** + * The file type mapper. + * + * @var FileTypeMapper + */ + private $fileTypeMapper; + /** @var bool */ private $phpunit10OrNewer; - public function __construct(ReflectionProvider $reflectionProvider, bool $phpunit10OrNewer) + public function __construct( + ReflectionProvider $reflectionProvider, + FileTypeMapper $fileTypeMapper, + bool $phpunit10OrNewer + ) { $this->reflectionProvider = $reflectionProvider; + $this->fileTypeMapper = $fileTypeMapper; $this->phpunit10OrNewer = $phpunit10OrNewer; } + /** + * @return iterable + */ + public function getDataProviderMethods( + Scope $scope, + ClassMethod $node, + ClassReflection $classReflection + ): iterable + { + $docComment = $node->getDocComment(); + if ($docComment !== null) { + $methodPhpDoc = $this->fileTypeMapper->getResolvedPhpDoc( + $scope->getFile(), + $classReflection->getName(), + $scope->isInTrait() ? $scope->getTraitReflection()->getName() : null, + $node->name->toString(), + $docComment->getText() + ); + foreach ($this->getDataProviderAnnotations($methodPhpDoc) as $annotation) { + $dataProviderValue = $this->getDataProviderAnnotationValue($annotation); + if ($dataProviderValue === null) { + // Missing value is already handled in NoMissingSpaceInMethodAnnotationRule + continue; + } + + $dataProviderMethod = $this->parseDataProviderAnnotationValue($scope, $dataProviderValue); + $dataProviderMethod[] = $node->getLine(); + + yield $dataProviderValue => $dataProviderMethod; + } + } + + if (!$this->phpunit10OrNewer) { + return; + } + + foreach ($node->attrGroups as $attrGroup) { + foreach ($attrGroup->attrs as $attr) { + $dataProviderMethod = null; + if ($attr->name->toLowerString() === 'phpunit\\framework\\attributes\\dataprovider') { + $dataProviderMethod = $this->parseDataProviderAttribute($attr, $classReflection); + } elseif ($attr->name->toLowerString() === 'phpunit\\framework\\attributes\\dataproviderexternal') { + $dataProviderMethod = $this->parseDataProviderExternalAttribute($attr); + } + if ($dataProviderMethod === null) { + continue; + } + + yield from $dataProviderMethod; + } + } + } + /** * @return array */ - public function getDataProviderAnnotations(?ResolvedPhpDocBlock $phpDoc): array + private function getDataProviderAnnotations(?ResolvedPhpDocBlock $phpDoc): array { if ($phpDoc === null) { return []; @@ -62,67 +133,62 @@ public function getDataProviderAnnotations(?ResolvedPhpDocBlock $phpDoc): array * @return RuleError[] errors */ public function processDataProvider( - Scope $scope, - PhpDocTagNode $phpDocTag, + string $dataProviderValue, + ?ClassReflection $classReflection, + string $methodName, + int $lineNumber, bool $checkFunctionNameCase, bool $deprecationRulesInstalled ): array { - $dataProviderValue = $this->getDataProviderValue($phpDocTag); - if ($dataProviderValue === null) { - // Missing value is already handled in NoMissingSpaceInMethodAnnotationRule - return []; - } - - [$classReflection, $method] = $this->parseDataProviderValue($scope, $dataProviderValue); if ($classReflection === null) { $error = RuleErrorBuilder::message(sprintf( '@dataProvider %s related class not found.', $dataProviderValue - ))->build(); + ))->line($lineNumber)->build(); return [$error]; } try { - $dataProviderMethodReflection = $classReflection->getNativeMethod($method); + $dataProviderMethodReflection = $classReflection->getNativeMethod($methodName); } catch (MissingMethodFromReflectionException $missingMethodFromReflectionException) { $error = RuleErrorBuilder::message(sprintf( '@dataProvider %s related method not found.', $dataProviderValue - ))->build(); + ))->line($lineNumber)->build(); return [$error]; } $errors = []; - if ($checkFunctionNameCase && $method !== $dataProviderMethodReflection->getName()) { + if ($checkFunctionNameCase && $methodName !== $dataProviderMethodReflection->getName()) { $errors[] = RuleErrorBuilder::message(sprintf( '@dataProvider %s related method is used with incorrect case: %s.', $dataProviderValue, $dataProviderMethodReflection->getName() - ))->build(); + ))->line($lineNumber)->build(); } if (!$dataProviderMethodReflection->isPublic()) { $errors[] = RuleErrorBuilder::message(sprintf( '@dataProvider %s related method must be public.', $dataProviderValue - ))->build(); + ))->line($lineNumber)->build(); } if ($deprecationRulesInstalled && $this->phpunit10OrNewer && !$dataProviderMethodReflection->isStatic()) { $errors[] = RuleErrorBuilder::message(sprintf( '@dataProvider %s related method must be static in PHPUnit 10 and newer.', $dataProviderValue - ))->build(); + ))->line($lineNumber)->build(); } return $errors; } - private function getDataProviderValue(PhpDocTagNode $phpDocTag): ?string + private function getDataProviderAnnotationValue(PhpDocTagNode $phpDocTag): ?string { if (preg_match('/^[^ \t]+/', (string) $phpDocTag->value, $matches) !== 1) { return null; @@ -134,7 +200,7 @@ private function getDataProviderValue(PhpDocTagNode $phpDocTag): ?string /** * @return array{ClassReflection|null, string} */ - private function parseDataProviderValue(Scope $scope, string $dataProviderValue): array + private function parseDataProviderAnnotationValue(Scope $scope, string $dataProviderValue): array { $parts = explode('::', $dataProviderValue, 2); if (count($parts) <= 1) { @@ -148,4 +214,62 @@ private function parseDataProviderValue(Scope $scope, string $dataProviderValue) return [null, $dataProviderValue]; } + /** + * @return array|null + */ + private function parseDataProviderExternalAttribute(Attribute $attribute): ?array + { + if (count($attribute->args) !== 2) { + return null; + } + $methodNameArg = $attribute->args[1]->value; + if (!$methodNameArg instanceof String_) { + return null; + } + $classNameArg = $attribute->args[0]->value; + if ($classNameArg instanceof ClassConstFetch && $classNameArg->class instanceof Name) { + $className = $classNameArg->class->toString(); + } elseif ($classNameArg instanceof String_) { + $className = $classNameArg->value; + } else { + return null; + } + + $dataProviderClassReflection = null; + if ($this->reflectionProvider->hasClass($className)) { + $dataProviderClassReflection = $this->reflectionProvider->getClass($className); + $className = $dataProviderClassReflection->getName(); + } + + return [ + sprintf('%s::%s', $className, $methodNameArg->value) => [ + $dataProviderClassReflection, + $methodNameArg->value, + $attribute->getLine(), + ], + ]; + } + + /** + * @return array|null + */ + private function parseDataProviderAttribute(Attribute $attribute, ClassReflection $classReflection): ?array + { + if (count($attribute->args) !== 1) { + return null; + } + $methodNameArg = $attribute->args[0]->value; + if (!$methodNameArg instanceof String_) { + return null; + } + + return [ + $methodNameArg->value => [ + $classReflection, + $methodNameArg->value, + $attribute->getLine(), + ], + ]; + } + } diff --git a/src/Rules/PHPUnit/DataProviderHelperFactory.php b/src/Rules/PHPUnit/DataProviderHelperFactory.php index a93ecfd..7fc8af0 100644 --- a/src/Rules/PHPUnit/DataProviderHelperFactory.php +++ b/src/Rules/PHPUnit/DataProviderHelperFactory.php @@ -3,6 +3,7 @@ namespace PHPStan\Rules\PHPUnit; use PHPStan\Reflection\ReflectionProvider; +use PHPStan\Type\FileTypeMapper; use PHPUnit\Framework\TestCase; use function dirname; use function explode; @@ -16,9 +17,13 @@ class DataProviderHelperFactory /** @var ReflectionProvider */ private $reflectionProvider; - public function __construct(ReflectionProvider $reflectionProvider) + /** @var FileTypeMapper */ + private $fileTypeMapper; + + public function __construct(ReflectionProvider $reflectionProvider, FileTypeMapper $fileTypeMapper) { $this->reflectionProvider = $reflectionProvider; + $this->fileTypeMapper = $fileTypeMapper; } public function create(): DataProviderHelper @@ -46,7 +51,7 @@ public function create(): DataProviderHelper } } - return new DataProviderHelper($this->reflectionProvider, $phpUnit10OrNewer); + return new DataProviderHelper($this->reflectionProvider, $this->fileTypeMapper, $phpUnit10OrNewer); } } diff --git a/tests/Rules/PHPUnit/DataProviderDeclarationRuleTest.php b/tests/Rules/PHPUnit/DataProviderDeclarationRuleTest.php index 03c44ec..18cc12b 100644 --- a/tests/Rules/PHPUnit/DataProviderDeclarationRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDeclarationRuleTest.php @@ -17,8 +17,7 @@ protected function getRule(): Rule $reflection = $this->createReflectionProvider(); return new DataProviderDeclarationRule( - new DataProviderHelper($reflection, true), - self::getContainer()->getByType(FileTypeMapper::class), + new DataProviderHelper($reflection, self::getContainer()->getByType(FileTypeMapper::class),true), true, true ); @@ -29,23 +28,39 @@ public function testRule(): void $this->analyse([__DIR__ . '/data/data-provider-declaration.php'], [ [ '@dataProvider providebaz related method is used with incorrect case: provideBaz.', - 14, + 16, ], [ '@dataProvider provideQux related method must be static in PHPUnit 10 and newer.', - 14, + 16, ], [ '@dataProvider provideQuux related method must be public.', - 14, + 16, ], [ '@dataProvider provideNonExisting related method not found.', - 68, + 70, ], [ '@dataProvider NonExisting::provideNonExisting related class not found.', - 68, + 70, + ], + [ + '@dataProvider provideNonExisting related method not found.', + 85, + ], + [ + '@dataProvider provideNonExisting2 related method not found.', + 86, + ], + [ + '@dataProvider ExampleTestCase\\BarTestCase::providetootherclass related method is used with incorrect case: provideToOtherClass.', + 87, + ], + [ + '@dataProvider ExampleTestCase\\BarTestCase::providetootherclass related method is used with incorrect case: provideToOtherClass.', + 88, ], ]); } diff --git a/tests/Rules/PHPUnit/data/data-provider-declaration.php b/tests/Rules/PHPUnit/data/data-provider-declaration.php index 23af826..176be2d 100644 --- a/tests/Rules/PHPUnit/data/data-provider-declaration.php +++ b/tests/Rules/PHPUnit/data/data-provider-declaration.php @@ -2,6 +2,8 @@ namespace ExampleTestCase; +use \PHPUnit\Framework\Attributes\DataProvider; + class FooTestCase extends \PHPUnit\Framework\TestCase { /** @@ -77,3 +79,15 @@ public static function provideToOtherClass(): iterable ]; } } + +class BazTestCase extends \PHPUnit\Framework\TestCase +{ + #[\PHPUnit\Framework\Attributes\DataProvider('provideNonExisting')] + #[DataProvider('provideNonExisting2')] + #[\PHPUnit\Framework\Attributes\DataProviderExternal('\\ExampleTestCase\\BarTestCase', 'providetootherclass')] + #[\PHPUnit\Framework\Attributes\DataProviderExternal(\ExampleTestCase\BarTestCase::class, 'providetootherclass')] + public function testIsNotBaz(string $subject): void + { + self::assertNotSame('baz', $subject); + } +}