From dec4d55331fb73df9190ea588933fbee75323143 Mon Sep 17 00:00:00 2001 From: Sander Muller Date: Sat, 12 Sep 2026 22:33:54 +0200 Subject: [PATCH] Report a data provider that provides no data sets PHPUnit 10 and newer rejects a data provider that provides nothing. PHPUnit 9 skipped the test instead. EmptyDataProviderRule reports a provider method whose every return statement returns an iterable that is provably empty. It skips generators, because whether a generator yields at all depends on control flow the rule cannot decide. Closes https://github.com/phpstan/phpstan-phpunit/issues/251 Co-Authored-By: Claude Opus 5 (1M context) --- rules.neon | 6 + src/Rules/PHPUnit/EmptyDataProviderRule.php | 109 +++++++ src/Rules/PHPUnit/PHPUnitVersion.php | 8 + .../PHPUnit/EmptyDataProviderRuleTest.php | 96 ++++++ .../PHPUnit/data/empty-data-provider.php | 302 ++++++++++++++++++ 5 files changed, 521 insertions(+) create mode 100644 src/Rules/PHPUnit/EmptyDataProviderRule.php create mode 100644 tests/Rules/PHPUnit/EmptyDataProviderRuleTest.php create mode 100644 tests/Rules/PHPUnit/data/empty-data-provider.php diff --git a/rules.neon b/rules.neon index 9d2d9477..cd7f0ec8 100644 --- a/rules.neon +++ b/rules.neon @@ -19,6 +19,9 @@ conditionalTags: PHPStan\Rules\PHPUnit\ClassAttributeRequiresPhpVersionRule: phpstan.rules.rule: %featureToggles.bleedingEdge% + PHPStan\Rules\PHPUnit\EmptyDataProviderRule: + phpstan.rules.rule: %featureToggles.bleedingEdge% + services: - class: PHPStan\Rules\PHPUnit\DataProviderDeclarationRule @@ -41,3 +44,6 @@ services: - class: PHPStan\Rules\PHPUnit\DataProviderDataRule + + - + class: PHPStan\Rules\PHPUnit\EmptyDataProviderRule diff --git a/src/Rules/PHPUnit/EmptyDataProviderRule.php b/src/Rules/PHPUnit/EmptyDataProviderRule.php new file mode 100644 index 00000000..59bf067c --- /dev/null +++ b/src/Rules/PHPUnit/EmptyDataProviderRule.php @@ -0,0 +1,109 @@ + + */ +class EmptyDataProviderRule implements Rule +{ + + private TestMethodsHelper $testMethodsHelper; + + private DataProviderHelper $dataProviderHelper; + + private PHPUnitVersion $PHPUnitVersion; + + public function __construct( + TestMethodsHelper $testMethodsHelper, + DataProviderHelper $dataProviderHelper, + PHPUnitVersion $PHPUnitVersion + ) + { + $this->testMethodsHelper = $testMethodsHelper; + $this->dataProviderHelper = $dataProviderHelper; + $this->PHPUnitVersion = $PHPUnitVersion; + } + + public function getNodeType(): string + { + return MethodReturnStatementsNode::class; + } + + public function processNode(Node $node, Scope $scope): array + { + if ($this->PHPUnitVersion->rejectsEmptyDataProviders()->no()) { + return []; + } + + if (!$node->getClassReflection()->is(TestCase::class)) { + return []; + } + + // A generator yields its data sets one at a time, so whether it provides + // any depends on control flow this rule cannot decide. + if ($node->isGenerator()) { + return []; + } + + $returnStatements = $node->getReturnStatements(); + if ($returnStatements === []) { + return []; + } + + foreach ($returnStatements as $returnStatement) { + $returnExpr = $returnStatement->getReturnNode()->expr; + if ($returnExpr === null) { + return []; + } + + $returnType = $returnStatement->getScope()->getType($returnExpr); + if (!$returnType->isIterable()->yes() || !$returnType->isIterableAtLeastOnce()->no()) { + return []; + } + } + + if (!$this->isDataProvider($node, $scope)) { + return []; + } + + return [ + RuleErrorBuilder::message(sprintf( + 'Data provider method %s() provides no data sets, which is an error in PHPUnit 10 and newer.', + $node->getMethodName(), + )) + ->identifier('phpunit.dataProviderEmpty') + ->build(), + ]; + } + + private function isDataProvider(MethodReturnStatementsNode $node, Scope $scope): bool + { + $classReflection = $node->getClassReflection(); + + foreach ($this->testMethodsHelper->getTestMethods($classReflection, $scope) as $testMethod) { + foreach ($this->dataProviderHelper->getDataProviderMethods($scope, $testMethod, $classReflection) as [$providerClassReflection, $providerMethodName]) { + // A provider declared in another class is checked when that class is analysed. + if ($providerClassReflection === null || $providerClassReflection->getName() !== $classReflection->getName()) { + continue; + } + + if (strcasecmp($providerMethodName, $node->getMethodName()) === 0) { + return true; + } + } + } + + return false; + } + +} diff --git a/src/Rules/PHPUnit/PHPUnitVersion.php b/src/Rules/PHPUnit/PHPUnitVersion.php index e6176b0a..0b6522b4 100644 --- a/src/Rules/PHPUnit/PHPUnitVersion.php +++ b/src/Rules/PHPUnit/PHPUnitVersion.php @@ -58,6 +58,14 @@ public function requiresStaticDataProviders(): TrinaryLogic return TrinaryLogic::createFromBoolean($this->majorVersion >= 10); } + public function rejectsEmptyDataProviders(): TrinaryLogic + { + if ($this->majorVersion === null) { + return TrinaryLogic::createMaybe(); + } + return TrinaryLogic::createFromBoolean($this->majorVersion >= 10); + } + public function supportsNamedArgumentsInDataProvider(): TrinaryLogic { if ($this->majorVersion === null) { diff --git a/tests/Rules/PHPUnit/EmptyDataProviderRuleTest.php b/tests/Rules/PHPUnit/EmptyDataProviderRuleTest.php new file mode 100644 index 00000000..c06ef571 --- /dev/null +++ b/tests/Rules/PHPUnit/EmptyDataProviderRuleTest.php @@ -0,0 +1,96 @@ + + */ +class EmptyDataProviderRuleTest extends RuleTestCase +{ + + private ?int $phpunitVersion; + + protected function getRule(): Rule + { + $phpunitVersion = new PHPUnitVersion($this->phpunitVersion, 0); + + return new EmptyDataProviderRule( + new TestMethodsHelper( + self::getContainer()->getByType(FileTypeMapper::class), + $phpunitVersion, + ), + new DataProviderHelper( + $this->createReflectionProvider(), + self::getContainer()->getByType(FileTypeMapper::class), + self::getContainer()->getService('defaultAnalysisParser'), + $phpunitVersion, + ), + $phpunitVersion, + ); + } + + /** + * @dataProvider provideVersions + */ + #[DataProvider('provideVersions')] + public function testRule(?int $version): void + { + if (PHP_VERSION_ID < 80000) { + self::markTestSkipped('Test requires PHP 8.0 for attributes.'); + } + + $this->phpunitVersion = $version; + + $errors = []; + + // PHPUnit 9 skips an empty data provider instead of erroring. + if ($version === null || $version >= 10) { + $errors[] = [ + 'Data provider method provideData() provides no data sets, which is an error in PHPUnit 10 and newer.', + 21, + ]; + $errors[] = [ + 'Data provider method provideData() provides no data sets, which is an error in PHPUnit 10 and newer.', + 109, + ]; + $errors[] = [ + 'Data provider method provideData() provides no data sets, which is an error in PHPUnit 10 and newer.', + 158, + ]; + } + + // DataProviderHelper only reads the attribute when the detected major version supports it. + if ($version !== null && $version >= 10) { + $errors[] = [ + 'Data provider method provideData() provides no data sets, which is an error in PHPUnit 10 and newer.', + 173, + ]; + } + + $this->analyse([__DIR__ . '/data/empty-data-provider.php'], $errors); + } + + /** + * @return iterable + */ + public static function provideVersions(): iterable + { + yield [null]; + yield [9]; + yield [10]; + yield [11]; + yield [12]; + } + + public static function getAdditionalConfigFiles(): array + { + return [__DIR__ . '/../../../extension.neon']; + } + +} diff --git a/tests/Rules/PHPUnit/data/empty-data-provider.php b/tests/Rules/PHPUnit/data/empty-data-provider.php new file mode 100644 index 00000000..4e163497 --- /dev/null +++ b/tests/Rules/PHPUnit/data/empty-data-provider.php @@ -0,0 +1,302 @@ + + */ + public static function provideData(): iterable + { + return self::build(); + } + + /** + * @return array + */ + private static function build(): array + { + return []; + } + +} + +class WrongCaseTest extends TestCase +{ + + /** + * @dataProvider provideDATA + */ + public function testFoo(string $input): void + { + } + + public static function provideData(): iterable + { + return []; + } + +} + +class AttributeOnlyTest extends TestCase +{ + + #[DataProvider('provideData')] + public function testFoo(string $input): void + { + } + + public static function provideData(): iterable + { + return []; + } + +} + +class BareReturnTest extends TestCase +{ + + /** + * @dataProvider provideData + */ + public function testFoo(string $input): void + { + } + + public static function provideData(bool $flag = false) + { + if ($flag) { + return; + } + + return []; + } + +} + +class NoReturnTest extends TestCase +{ + + /** + * @dataProvider provideData + */ + public function testFoo(string $input): void + { + } + + public static function provideData(): iterable + { + } + +} + +class NonIterableTest extends TestCase +{ + + /** + * @dataProvider provideData + */ + public function testFoo(string $input): void + { + } + + public static function provideData(): string + { + return 'nope'; + } + +} + +class ExternalProviderHolder +{ + + /** + * @return iterable + */ + public static function provideData(): iterable + { + return [['a']]; + } + +} + +class ExternalProviderTest extends TestCase +{ + + /** + * @dataProvider \EmptyDataProviderTest\ExternalProviderHolder::provideData + */ + #[DataProviderExternal(ExternalProviderHolder::class, 'provideData')] + public function testFoo(string $input): void + { + } + + public static function provideData(): iterable + { + return []; + } + +} + +class UnknownProviderClassTest extends TestCase +{ + + /** + * @dataProvider \EmptyDataProviderTest\ThisClassDoesNotExist::provideData + */ + public function testFoo(string $input): void + { + } + + public static function provideData(): iterable + { + return []; + } + +} + +class MaybeIterableTest extends TestCase +{ + + /** + * @dataProvider provideData + */ + public function testFoo(string $input): void + { + } + + /** + * @return array{}|string + */ + public static function provideData(bool $flag = false) + { + $data = $flag ? '' : []; + + return $data; + } + +}