phpstan/phpstan-phpunit

protected dataProvider method reported as required to be public

janedbal opened this issue · 8 comments

There is new check about dataProviders being public introduced in #150, but protected providers work just fine. I just tested private one and that also works (invocation over method reflection used behaind the scenes).

  • Used phpunit/phpunit version: 9.5.26
  • Used phpstan/phpstan-phpunit version: 1.3.2

Hi @janedbal,

May you provide a repro?
On my side I tried to reproduce what you describe this way:

<?php

use PHPUnit\Framework\TestCase;

class DummyTest extends TestCase
{
    /** @dataProvider provideValue */
    public function test($arg): void
    {
        self::assertTrue($arg);
    }

    protected static function provideValue(): iterable
    {
        return [
            [true],
        ];
    }
}

But I got this error:

# vendor/bin/phpunit tests/DummyTest.php 
PHPUnit 9.5.26 by Sebastian Bergmann and contributors.

E                                                                   1 / 1 (100%)

Time: 00:00.035, Memory: 6.00 MB

There was 1 error:

1) Error
The data provider specified for DummyTest::test is invalid.
ReflectionException: Trying to invoke protected method DummyTest::provideValue() from scope ReflectionMethod

ERRORS!
Tests: 1, Assertions: 0, Errors: 1.

I've also checked the documentation:

A data provider method must be public and either return an array of arrays or an object that implements the Iterator interface and yields an array for each iteration step.

EDIT:
Here the PHP version used to run the test:

# php -v
PHP 8.0.25 (cli) (built: Nov 15 2022 03:22:16) ( NTS )
Copyright (c) The PHP Group
Zend Engine v4.0.25, Copyright (c) Zend Technologies
    with Xdebug v3.1.6, Copyright (c) 2002-2022, by Derick Rethans

I'm using PHP 8.1 where you dont need to call setAccessible. That might be the reason.

Confirmed, your example works on PHP 8.1 and does not on PHP 8.0.

composer require phpunit/phpunit
composer require phpstan/phpstan-phpunit
nano DummyTest.php
# paste your contents
docker run --rm -it -v "$PWD":/usr/src/myapp -w /usr/src/myapp php:8.0-cli php vendor/bin/phpunit DummyTest.php # fails
docker run --rm -it -v "$PWD":/usr/src/myapp -w /usr/src/myapp php:8.1-cli php vendor/bin/phpunit DummyTest.php # works

I confirm it too (tried with PHP 8.1.13).

I don't know if it is possible, nor if it's aligned with the PHPStan's way of doing things, but here a proposal:

  • if the minimum PHP version required by the project is lower than 8.1, the rule is always enabled. If there is no version specified I guess we'd fallback on the version used to execute PHPStan.
  • If the version is >= 8.1, the rule would be enabled depending on a parameter (false by default, true if phpstan-strict-rules is installed).

@ondrejmirtes WDYT?

Oh god, what a mess. Well, first I'd ask over at PHPUnit to see whether this is intended or not. Documentation still states that dataProvider must be public. So I did: sebastianbergmann/phpunit#5115

In the future PHPUnit is gonan require dataProviders to be public again: sebastianbergmann/phpunit#5115 (comment)

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.