typecheck dataproviders

[staabm]

it would be great when phpstan could typecheck a dataprovider function against the type-signature of the methods which is using it.

see this code taken from composer/semver

the dataprovider will never provide a null value for $min.

    /**
     * @dataProvider hyphenConstraints
     *
     * @param string     $input
     * @param Constraint|null $min
     * @param Constraint $max
     */
    public function testParseHyphen($input, $min, $max)
    {
        $parser = new VersionParser();
        if ($min) {
            $expected = new MultiConstraint(array($min, $max));
        } else {
            $expected = $max;
        }

        $this->assertSame((string) $expected, (string) $parser->parseConstraints($input));
    }

    /**
     * @return array
     */
    public function hyphenConstraints()
    {
        return array(
            array('v1 - v2', new Constraint('>=', '1.0.0.0-dev'), new Constraint('<', '3.0.0.0-dev')),
            array('1.2.3 - 2.3.4.5', new Constraint('>=', '1.2.3.0-dev'), new Constraint('<=', '2.3.4.5')),
            array('1.2-beta - 2.3', new Constraint('>=', '1.2.0.0-beta'), new Constraint('<', '2.4.0.0-dev')),
            array('1.2-beta - 2.3-dev', new Constraint('>=', '1.2.0.0-beta'), new Constraint('<=', '2.3.0.0-dev')),
            array('1.2-RC - 2.3.1', new Constraint('>=', '1.2.0.0-RC'), new Constraint('<=', '2.3.1.0')),
            array('1.2.3-alpha - 2.3-RC', new Constraint('>=', '1.2.3.0-alpha'), new Constraint('<=', '2.3.0.0-RC')),
            array('1 - 2.0', new Constraint('>=', '1.0.0.0-dev'), new Constraint('<', '2.1.0.0-dev')),
            array('1 - 2.1', new Constraint('>=', '1.0.0.0-dev'), new Constraint('<', '2.2.0.0-dev')),
            array('1.2 - 2.1.0', new Constraint('>=', '1.2.0.0-dev'), new Constraint('<=', '2.1.0.0')),
            array('1.3 - 2.1.3', new Constraint('>=', '1.3.0.0-dev'), new Constraint('<=', '2.1.3.0')),
        );
    }

[ondrejmirtes]

Yeah, would be nice, but traditionally we're checking if the parameter type accepts the argument type, so extra |null is not reported (because the method can be called from a lot of places which we're not gathering). So this would need a different approach than the CallMethodsRule in PHPStan core.

Also, this is complicated by multiple @dataProvider above single test method which PHPUnit also supports.

[gnutix]

If this issue is not addressed (because of complexity or whatever), I was wondering if the return type of functions using as data providers could be simply ignored ? Currently, all my data providers have /** @return iterable<mixed> */ just to satisfy PHPStan. WDYT @ondrejmirtes ? would it be easy to implement ?

[ondrejmirtes]

It's not addressed because no one has implemented it yet. It should be quite easy to do it.

When I want to ignore the missing typehints, I usually add a configuration that looks like this:

parameters:
	ignoreErrors:
		- '#Test::data[a-zA-Z0-9_]+\(\) return type has no value type specified in iterable type#'

[gnutix]

Lucky you to have a data prefix to all your data provider methods.. :'( I would just end up with a ton of these ignored entries.

[julienfalque]

Is someone working on this? If not, I'd like to give it a try! I'd focus first on checking that the test methods accept the data returned by providers (so not reporting that |null can be dropped in the example above). Any advice? :)

[staabm]

stumbled over this also again. please feel free to work on it. I will try to support you as good as I can.

feel free to ping me if you have questions.

[staabm]

so my suggestion on this topic would be:

• create a new DataProviderSignatureRule, which works on class-methods
• the rule will ignore all methods but those starting with test* in the name
• the rule will detect whether a @dataProvier is declared in the test-methods phpdoc
• for-each @dataProvier we will utilize the ReflectionProvider to determine the return-type of the data-provider method
• the data-provider methods return type will be checked against the test-methods parameter types

wdyt?

[ondrejmirtes]

Tes, that's generally what you need to do. Also you need to look into other ways that test methods and data providers can be marked.

[stof]

There is already a DataProviderHelper in that extension that finds the corresponding data providers (to check they are defined). This logic can probably be reused (extracting the parts that are not about implementing the existing rule)