psalm / psalm-plugin-phpunit

A PHPUnit plugin for Psalm
77 stars 33 forks source link

Erroneous TooFewArguments on combination of data provider and dependent test #113

Open remorhaz opened 3 years ago

remorhaz commented 3 years ago

In my case, test case class has the following structure:

<?php

class MyTest extends \PHPUnit\Framework\TestCase
{

    public function testOne(): string
    {
        // ...
        return 'bar';
    }

    /**
     * @dataProvider providerFoo
     * @depends testOne
     */
    public function testSecond(string $foo, string $bar): void
    {
        // ...
    }

    /**
     * @return iterable<string, array{string}>
    public function providerFoo(): iterable
    {
        return [['foo']];
    }
}

In this case, testSecond() method gets $foo = 'foo' and $bar = 'bar' arguments; but Psalm reports TooFewArguments (probably ignoring dependency if data provider is set).

P.S.: Anyone please tell me how to suppress this bug until it gets fixed. Adding @psalm-suppress TooFewArguments both in provider or test method docblocks doesn't help.

weirdan commented 3 years ago

Currently the plugin has no idea what @depends mean.

Anyone please tell me how to suppress this bug until it gets fixed. Adding @psalm-suppress TooFewArguments both in provider or test method docblocks doesn't help.

You may try to suppress it via config file. Didn't test it myself, but I think it might work:

        <TooFewArguments>
            <errorLevel type="suppress">
                <referencedFunction name="Tests\TestCase::testThatFails" />
            </errorLevel>
        </TooFewArguments>
weirdan commented 3 years ago

Dev notes:

When a test receives input from both a @dataProvider method and from one or more tests it @depends on, the arguments from the data provider will come before the ones from depended-upon tests

The following is not really clear:

The result of a test that uses data providers cannot be injected into a depending test.

weirdan commented 3 years ago

You may try to suppress it via config file. Didn't test it myself, but I think it might work

Yep, the following does work:

Feature: Suppression
  In order to avoid false positives
  As a Psalm user
  I need to be able to suppress issues emitted by the plugin

  Scenario: Can suppress TooFewArguments via config
    Given I have the following config
      """
      <?xml version="1.0"?>
      <psalm totallyTyped="true" %s>
        <projectFiles>
          <directory name="."/>
          <ignoreFiles> <directory name="../../vendor"/> </ignoreFiles>
        </projectFiles>
        <plugins>
          <pluginClass class="Psalm\PhpUnitPlugin\Plugin"/>
        </plugins>
        <issueHandlers>
          <TooFewArguments>
            <errorLevel type="suppress">
              <referencedFunction name="NS\MyTestCase::testConsumer" />
            </errorLevel>
          </TooFewArguments>
        </issueHandlers>
      </psalm>
      """
    And I have the following code
      """
      <?php
      namespace NS;
      use PHPUnit\Framework\TestCase;

      class MyTestCase extends TestCase
      {
        public function testProducer(): int
        {
          return 42;
        }

        /**
         * @depends testProducer
         * @dataProvider provide
         */
        public function testConsumer(int $provided, int $dependedUpon): void {}

        /** @return iterable<int,array{int}> */
        public function provide() {
          yield [1];
        }
      }
      """
    When I run Psalm
    Then I see no errors
remorhaz commented 3 years ago

The result of a test that uses data providers cannot be injected into a depending test.

This means that if you make testSecond() dependent of testOne() (like in my example), then testOne() cannot use data provider (but testSecond() can, as it does in my example).

weirdan commented 3 years ago

I'm curious what would happen if it does?

remorhaz commented 3 years ago

I'm curious what would happen if it does?

Just as written - it doesn't inject any data from data-provided test, I'm just getting null in "dependent" argument (or TypeError if it's type-hinted).