Skip to content

Commit c8d5d0d

Browse files
staabmstaabm
committed
validate dataProvider return types
1 parent f9bfc19 commit c8d5d0d

File tree

4 files changed

+162
-2
lines changed

4 files changed

+162
-2
lines changed

‎src/Rules/PHPUnit/DataProviderDeclarationRule.php

+7-1
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,11 @@ public function processNode(Node $node, Scope $scope): array
8484

8585
$annotations = $this->dataProviderHelper->getDataProviderAnnotations($methodPhpDoc);
8686

87+
$testMethodReflection = null;
88+
if ($classReflection->hasMethod($node->name->toString())) {
89+
$testMethodReflection = $classReflection->getMethod($node->name->toString(), $scope);
90+
}
91+
8792
$errors = [];
8893

8994
foreach ($annotations as $annotation) {
@@ -93,7 +98,8 @@ public function processNode(Node $node, Scope $scope): array
9398
$scope,
9499
$annotation,
95100
$this->checkFunctionNameCase,
96-
$this->deprecationRulesInstalled
101+
$this->deprecationRulesInstalled,
102+
$testMethodReflection
97103
)
98104
);
99105
}

‎src/Rules/PHPUnit/DataProviderHelper.php

+59-1
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,15 @@
55
use PHPStan\Analyser\Scope;
66
use PHPStan\PhpDoc\ResolvedPhpDocBlock;
77
use PHPStan\PhpDocParser\Ast\PhpDoc\PhpDocTagNode;
8+
use PHPStan\Reflection\ExtendedMethodReflection;
89
use PHPStan\Reflection\MissingMethodFromReflectionException;
10+
use PHPStan\Reflection\ParametersAcceptorSelector;
911
use PHPStan\Rules\RuleError;
1012
use PHPStan\Rules\RuleErrorBuilder;
13+
use PHPStan\Type\UnionType;
14+
use PHPStan\Type\VerbosityLevel;
1115
use function array_merge;
16+
use function count;
1217
use function preg_match;
1318
use function sprintf;
1419

@@ -45,7 +50,8 @@ public function processDataProvider(
4550
Scope $scope,
4651
PhpDocTagNode $phpDocTag,
4752
bool $checkFunctionNameCase,
48-
bool $deprecationRulesInstalled
53+
bool $deprecationRulesInstalled,
54+
?ExtendedMethodReflection $testMethodReflection
4955
): array
5056
{
5157
$dataProviderName = $this->getDataProviderName($phpDocTag);
@@ -95,6 +101,58 @@ public function processDataProvider(
95101
))->build();
96102
}
97103

104+
$dataProviderParameterAcceptor = ParametersAcceptorSelector::selectSingle($dataProviderMethodReflection->getVariants());
105+
$providerReturnType = $dataProviderParameterAcceptor->getReturnType();
106+
if ($providerReturnType->isIterable()->yes()) {
107+
$collectionType = $providerReturnType->getIterableValueType();
108+
109+
if ($collectionType->isIterable()->yes()) {
110+
$testParameterAcceptor = ParametersAcceptorSelector::selectSingle($testMethodReflection->getVariants());
111+
112+
$valueType = $collectionType->getIterableValueType();
113+
114+
if ($valueType instanceof UnionType) {
115+
if (count($valueType->getTypes()) !== count($testParameterAcceptor->getParameters())) {
116+
$errors[] = RuleErrorBuilder::message(sprintf(
117+
'@dataProvider %s returns a different number of values the test method expects.',
118+
$dataProviderName
119+
))->build();
120+
121+
return $errors;
122+
}
123+
124+
foreach ($valueType->getTypes() as $i => $innerType) {
125+
if (!$testParameterAcceptor->getParameters()[$i]->getType()->accepts($innerType, $scope->isDeclareStrictTypes())->yes()) {
126+
$errors[] = RuleErrorBuilder::message(sprintf(
127+
'@dataProvider %s returns %s which is not compatible with the test method parameters.',
128+
$dataProviderName,
129+
$providerReturnType->describe(VerbosityLevel::precise())
130+
))->build();
131+
132+
return $errors;
133+
}
134+
}
135+
} else {
136+
if (count($testParameterAcceptor->getParameters()) !== 1) {
137+
$errors[] = RuleErrorBuilder::message(sprintf(
138+
'@dataProvider %s returns a different number of values the test method expects.',
139+
$dataProviderName
140+
))->build();
141+
142+
return $errors;
143+
}
144+
145+
if (!$testParameterAcceptor->getParameters()[0]->getType()->accepts($valueType, $scope->isDeclareStrictTypes())->yes()) {
146+
$errors[] = RuleErrorBuilder::message(sprintf(
147+
'@dataProvider %s returns %s which is not compatible with the test method parameters.',
148+
$dataProviderName,
149+
$providerReturnType->describe(VerbosityLevel::precise())
150+
))->build();
151+
}
152+
}
153+
}
154+
}
155+
98156
return $errors;
99157
}
100158

‎tests/Rules/PHPUnit/DataProviderDeclarationRuleTest.php

+20
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,26 @@ public function testRule(): void
4141
'@dataProvider provideNonExisting related method not found.',
4242
66,
4343
],
44+
[
45+
'@dataProvider provideMultiple returns a different number of values the test method expects.',
46+
79,
47+
],
48+
[
49+
'@dataProvider provideArray returns iterable<int, array<string>> which is not compatible with the test method parameters.',
50+
101,
51+
],
52+
[
53+
'@dataProvider provideIterator returns Iterator<mixed, array<int, string>> which is not compatible with the test method parameters.',
54+
101,
55+
],
56+
[
57+
'@dataProvider provideMultiple returns a different number of values the test method expects.',
58+
101,
59+
],
60+
[
61+
'@dataProvider provideMultiple returns Iterator<mixed, array{string, int}> which is not compatible with the test method parameters.',
62+
116,
63+
],
4464
]);
4565
}
4666

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

+76
Original file line numberDiff line numberDiff line change
@@ -68,3 +68,79 @@ public function testIsNotBar(string $subject): void
6868
self::assertNotSame('bar', $subject);
6969
}
7070
}
71+
72+
class FooBarTestCase extends \PHPUnit\Framework\TestCase
73+
{
74+
/**
75+
* @dataProvider provideArray
76+
* @dataProvider provideIterator
77+
* @dataProvider provideMultiple
78+
*/
79+
public function testIsNotFooBar(string $subject): void
80+
{
81+
self::assertNotSame('foo', $subject);
82+
}
83+
84+
/**
85+
* @dataProvider provideArray
86+
* @dataProvider provideIterator
87+
*
88+
* @param string $subject
89+
*/
90+
public function testIsNotFooBarPhpDoc($subject): void
91+
{
92+
self::assertNotSame('foo', $subject);
93+
}
94+
95+
96+
/**
97+
* @dataProvider provideArray
98+
* @dataProvider provideIterator
99+
* @dataProvider provideMultiple
100+
*/
101+
public function testIsFooBar(int $subject): void
102+
{
103+
self::assertNotSame(123, $subject);
104+
}
105+
106+
/**
107+
* @dataProvider provideMultiple
108+
*/
109+
public function testMultipleParams(string $subject, int $i): void
110+
{
111+
}
112+
113+
/**
114+
* @dataProvider provideMultiple
115+
*/
116+
public function testBogusMultipleParams(float $subject, string $i): void
117+
{
118+
}
119+
120+
121+
/**
122+
* @return list<array<string>>
123+
*/
124+
public static function provideArray(): iterable
125+
{
126+
return [
127+
['bar'],
128+
];
129+
}
130+
131+
/**
132+
* @return \Iterator<list<string>>
133+
*/
134+
public static function provideIterator(): \Iterator
135+
{
136+
yield ['bar'];
137+
}
138+
139+
/**
140+
* @return \Iterator<array{string, int}>
141+
*/
142+
public static function provideMultiple(): \Iterator
143+
{
144+
yield ['bar', 1];
145+
}
146+
}

0 commit comments

Comments
 (0)