Skip to content

Improve EmptyDataProviderRule - #354

Merged
staabm merged 2 commits into
phpstan:2.1.xfrom
staabm:impr
Nov 12, 2026
Merged

staabm merged 2 commits into
phpstan:2.1.xfrom
staabm:impr

Conversation

@staabm

@staabm staabm commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor

triggers a error which was missing in the initial implementation

Comment on lines +66 to +69
$errors[] = [
'Data provider method provideData() provides no data sets, which is an error in PHPUnit 10 and newer.',
190,
];

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

➜  phpstan-phpunit git:(impr) ✗ cat test.php                
<?php

use PHPUnit\Framework\TestCase;

class BareReturnTest extends TestCase
{

        /**
         * @dataProvider provideData
         */
        public function testFoo(string $input): void
        {
        }

        public static function provideData(bool $flag = false)
        {
                if ($flag) {
                        return;
                }

                return [];
        }

}
➜  phpstan-phpunit git:(impr) ✗ vendor/bin/phpunit test.php
PHPUnit 11.5.57 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.5.12
Configuration: /Users/m.staab/dvl/phpstan-phpunit/phpunit.xml
Random Seed:   1794502306

There was 1 PHPUnit error:

1) BareReturnTest::testFoo
The data provider specified for BareReturnTest::testFoo is invalid
Data Provider method BareReturnTest::provideData() expects an argument

/Users/m.staab/dvl/phpstan-phpunit/test.php:11

--

There was 1 PHPUnit test runner warning:

1) No tests found in class "BareReturnTest".

No tests executed!

@staabm
staabm marked this pull request as ready for review November 12, 2026 16:52
@staabm

staabm commented Nov 12, 2026

Copy link
Copy Markdown
Contributor Author

//cc @SanderMuller

@SanderMuller SanderMuller left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change is right. A bare return; gives PHPUnit null, and PHPUnit 11.5.57 and 12.5.43 reject that provider too. On 12 the message is "does not return an iterable". So a provider that returns either nothing or [] fails on every path. I ran a version of BareReturnTest without the parameter, with both paths on both versions, and every run ended with "The data provider ... is invalid".

The new expectation on line 190 fails when I put return []; back in place of continue. On ff664b0 the test suite, make phpstan and phpcs pass, and CI is green.

What I checked with the rule on this branch (PHPUnit 12 in the test harness):

  • A provider with a bare return; and a non-empty return [['a']]; is not reported. That is correct, because the second path provides data.
  • A provider whose only statement is return; is not reported.
  • A method with the same return; / return []; shape that no test uses as a provider is not reported.

About your inline comment: the bool $flag = false parameter on BareReturnTest::provideData() comes from my #328, and MaybeIterableTest has the same parameter. PHPUnit 11 and 12 reject both providers with "expects an argument". So these two fixtures do not show what happens at runtime. The rule result is the same without the parameter. I can open a follow-up that replaces the parameter with a runtime condition, for example getenv().

I did not time this. The change only replaces an early exit with continue in the loop over the return statements. The rule does not reach that loop when it detects PHPUnit 9 or older.

@staabm
staabm merged commit 4041c82 into phpstan:2.1.x Nov 12, 2026
113 checks passed
@staabm

staabm commented Nov 12, 2026

Copy link
Copy Markdown
Contributor Author

Thank you!

@staabm
staabm deleted the impr branch November 13, 2026 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants