Skip to content

Commit daf5378

Browse files
villfavillfa
committed
Add rule to check @dataProvider 
1 parent 8313d41 commit daf5378

5 files changed

+318
-0
lines changed

‎extension.neon

+2
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@ services:
5555
class: PHPStan\Rules\PHPUnit\CoversHelper
5656
-
5757
class: PHPStan\Rules\PHPUnit\AnnotationHelper
58+
-
59+
class: PHPStan\Rules\PHPUnit\DataProviderHelper
5860

5961
conditionalTags:
6062
PHPStan\PhpDoc\PHPUnit\MockObjectTypeNodeResolverExtension:

‎src/Rules/PHPUnit/DataProviderDeclarationRule.php

+81
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\PHPUnit;
4+
5+
use PhpParser\Node;
6+
use PHPStan\Analyser\Scope;
7+
use PHPStan\Rules\Rule;
8+
use PHPStan\Type\FileTypeMapper;
9+
use PHPUnit\Framework\TestCase;
10+
use function array_merge;
11+
12+
/**
13+
* @implements Rule<Node\Stmt\ClassMethod>
14+
*/
15+
class DataProviderDeclarationRule implements Rule
16+
{
17+
18+
/**
19+
* Data provider helper.
20+
*
21+
* @var DataProviderHelper
22+
*/
23+
private $dataProviderHelper;
24+
25+
/**
26+
* The file type mapper.
27+
*
28+
* @var FileTypeMapper
29+
*/
30+
private $fileTypeMapper;
31+
32+
public function __construct(
33+
DataProviderHelper $dataProviderHelper,
34+
FileTypeMapper $fileTypeMapper
35+
)
36+
{
37+
$this->dataProviderHelper = $dataProviderHelper;
38+
$this->fileTypeMapper = $fileTypeMapper;
39+
}
40+
41+
public function getNodeType(): string
42+
{
43+
return Node\Stmt\ClassMethod::class;
44+
}
45+
46+
public function processNode(Node $node, Scope $scope): array
47+
{
48+
$classReflection = $scope->getClassReflection();
49+
50+
if ($classReflection === null || !$classReflection->isSubclassOf(TestCase::class)) {
51+
return [];
52+
}
53+
54+
$docComment = $node->getDocComment();
55+
if ($docComment === null) {
56+
return [];
57+
}
58+
59+
$methodPhpDoc = $this->fileTypeMapper->getResolvedPhpDoc(
60+
$scope->getFile(),
61+
$classReflection->getName(),
62+
$scope->isInTrait() ? $scope->getTraitReflection()->getName() : null,
63+
$node->name->toString(),
64+
$docComment->getText()
65+
);
66+
67+
$annotations = $this->dataProviderHelper->getDataProviderAnnotations($methodPhpDoc);
68+
69+
$errors = [];
70+
71+
foreach ($annotations as $annotation) {
72+
$errors = array_merge(
73+
$errors,
74+
$this->dataProviderHelper->processDataProvider($scope, $annotation)
75+
);
76+
}
77+
78+
return $errors;
79+
}
80+
81+
}

‎src/Rules/PHPUnit/DataProviderHelper.php

+111
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\PHPUnit;
4+
5+
use PHPStan\Analyser\Scope;
6+
use PHPStan\PhpDoc\ResolvedPhpDocBlock;
7+
use PHPStan\PhpDocParser\Ast\PhpDoc\PhpDocTagNode;
8+
use PHPStan\Reflection\MissingMethodFromReflectionException;
9+
use PHPStan\Rules\RuleError;
10+
use PHPStan\Rules\RuleErrorBuilder;
11+
use function array_merge;
12+
use function preg_match;
13+
use function sprintf;
14+
use function trim;
15+
16+
class DataProviderHelper
17+
{
18+
19+
/**
20+
* @return array<PhpDocTagNode>
21+
*/
22+
public function getDataProviderAnnotations(?ResolvedPhpDocBlock $phpDoc): array
23+
{
24+
if ($phpDoc === null) {
25+
return [];
26+
}
27+
28+
$phpDocNodes = $phpDoc->getPhpDocNodes();
29+
30+
$annotations = [];
31+
32+
foreach ($phpDocNodes as $docNode) {
33+
$annotations = array_merge(
34+
$annotations,
35+
$docNode->getTagsByName('@dataProvider')
36+
);
37+
}
38+
39+
return $annotations;
40+
}
41+
42+
/**
43+
* @return RuleError[] errors
44+
*/
45+
public function processDataProvider(
46+
Scope $scope,
47+
PhpDocTagNode $phpDocTag
48+
): array
49+
{
50+
$dataProviderName = $this->getDataProviderName($phpDocTag);
51+
if ($dataProviderName === null) {
52+
// Missing name is already handled in NoMissingSpaceInMethodAnnotationRule
53+
return [];
54+
}
55+
56+
$classReflection = $scope->getClassReflection();
57+
if ($classReflection === null) {
58+
// Should not happen
59+
return [];
60+
}
61+
62+
try {
63+
$dataProviderMethodReflection = $classReflection->getMethod($dataProviderName, $scope);
64+
} catch (MissingMethodFromReflectionException $missingMethodFromReflectionException) {
65+
$error = RuleErrorBuilder::message(sprintf(
66+
'@dataProvider %s related method not found.',
67+
$dataProviderName
68+
))->build();
69+
70+
return [$error];
71+
}
72+
73+
$errors = [];
74+
75+
if ($dataProviderName !== $dataProviderMethodReflection->getName()) {
76+
$errors[] = RuleErrorBuilder::message(sprintf(
77+
'@dataProvider %s related method is used with incorrect case: %s.',
78+
$dataProviderName,
79+
$dataProviderMethodReflection->getName()
80+
))->build();
81+
}
82+
83+
if (!$dataProviderMethodReflection->isPublic()) {
84+
$errors[] = RuleErrorBuilder::message(sprintf(
85+
'@dataProvider %s related method must be public.',
86+
$dataProviderName
87+
))->build();
88+
}
89+
90+
if (!$dataProviderMethodReflection->isStatic()) {
91+
$errors[] = RuleErrorBuilder::message(sprintf(
92+
'@dataProvider %s related method must be static.',
93+
$dataProviderName
94+
))->build();
95+
}
96+
97+
return $errors;
98+
}
99+
100+
private function getDataProviderName(PhpDocTagNode $phpDocTag): ?string
101+
{
102+
$value = trim((string) $phpDocTag->value);
103+
104+
if (preg_match('/^[\S]+/', $value, $matches) !== 1) {
105+
return null;
106+
}
107+
108+
return $matches[0];
109+
}
110+
111+
}

‎tests/Rules/PHPUnit/DataProviderDeclarationRuleTest.php

+54
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
<?php declare(strict_types=1);
2+
3+
namespace PHPStan\Rules\PHPUnit;
4+
5+
use PHPStan\Rules\Rule;
6+
use PHPStan\Testing\RuleTestCase;
7+
use PHPStan\Type\FileTypeMapper;
8+
9+
/**
10+
* @extends RuleTestCase<DataProviderDeclarationRule>
11+
*/
12+
class DataProviderDeclarationRuleTest extends RuleTestCase
13+
{
14+
15+
protected function getRule(): Rule
16+
{
17+
return new DataProviderDeclarationRule(
18+
new DataProviderHelper(),
19+
self::getContainer()->getByType(FileTypeMapper::class)
20+
);
21+
}
22+
23+
public function testRule(): void
24+
{
25+
$this->analyse([__DIR__ . '/data/data-provider-declaration.php'], [
26+
[
27+
'@dataProvider providebaz related method is used with incorrect case: provideBaz.',
28+
13,
29+
],
30+
[
31+
'@dataProvider provideQux related method must be static.',
32+
13,
33+
],
34+
[
35+
'@dataProvider provideQuux related method must be public.',
36+
13,
37+
],
38+
[
39+
'@dataProvider provideNonExisting related method not found.',
40+
66,
41+
],
42+
]);
43+
}
44+
45+
/**
46+
* @return string[]
47+
*/
48+
public static function getAdditionalConfigFiles(): array
49+
{
50+
return [
51+
__DIR__ . '/../../../extension.neon',
52+
];
53+
}
54+
}

‎tests/Rules/PHPUnit/data/data-provider-declaration.php

+70
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
<?php declare(strict_types=1);
2+
3+
namespace ExampleTestCase;
4+
5+
class FooTestCase extends \PHPUnit\Framework\TestCase
6+
{
7+
/**
8+
* @dataProvider provideBar Comment.
9+
* @dataProvider providebaz
10+
* @dataProvider provideQux
11+
* @dataProvider provideQuux
12+
*/
13+
public function testIsNotFoo(string $subject): void
14+
{
15+
self::assertNotSame('foo', $subject);
16+
}
17+
18+
public static function provideBar(): iterable
19+
{
20+
return [
21+
['bar'],
22+
];
23+
}
24+
25+
public static function provideBaz(): iterable
26+
{
27+
return [
28+
['baz'],
29+
];
30+
}
31+
32+
public function provideQux(): iterable
33+
{
34+
return [
35+
['qux'],
36+
];
37+
}
38+
39+
protected static function provideQuux(): iterable
40+
{
41+
42+
return [
43+
['quux'],
44+
];
45+
}
46+
}
47+
48+
trait BarProvider
49+
{
50+
public static function provideCorge(): iterable
51+
{
52+
return [
53+
['corge'],
54+
];
55+
}
56+
}
57+
58+
class BarTestCase extends \PHPUnit\Framework\TestCase
59+
{
60+
use BarProvider;
61+
62+
/**
63+
* @dataProvider provideNonExisting
64+
* @dataProvider provideCorge
65+
*/
66+
public function testIsNotBar(string $subject): void
67+
{
68+
self::assertNotSame('bar', $subject);
69+
}
70+
}

0 commit comments

Comments
 (0)