psalm / psalm-plugin-phpunit

A PHPUnit plugin for Psalm
77 stars 33 forks source link

calling assertInstanceOf on a property set in TestCase::setUp() should result in RedundantCondition #49

Open SignpostMarv opened 4 years ago

SignpostMarv commented 4 years ago

not quite sure on the exact error/test case that should occur, but this is a vague mashup of what I'm thinking:

  Scenario: setUp resolves types
    Given I have the following code
      """
      class MyTestCase extends TestCase
      {
        /** @var \DateTime|null */
        private $date;

        /** @return void */
        public function setUp() {
          $this->date = new \DateTime();
        }

        /** @return void */
        public function testSomething() {
          $this->assertInstanceOf(\DateTime::class, $this->date);
        }
      }
      """
    When I run Psalm
    Then I see these errors
      | Type            | Message                                                                                                     |
      | RedundantConditionGivenDocblockType | Found a redundant condition when evaluating docblock-defined type $this->date and trying to reconcile type 'DateTime' to DateTime |
    And I see no other errors
weirdan commented 4 years ago

I think setUp() method analyzer has to be run with $scope->collect_initializations = true and it'll fix the issue automatically.

SignpostMarv commented 4 years ago

where would I poke around to have that kick in ?

ktomk commented 4 years ago

Depending on the visibility of TestCase::$foo (e.g. not private), this would be runtime behavior. Just saying. Psalm would not be able to detect it unchanged. Even if private PHP has reflection, so again, runtime behavior.

/E: Have you tried to mark the TestCase::$foo property as @psalm immutable? No idea if the phpunit plugin needs to understand Phpunit\TestCase::setUp as eligible for (implicit) setter injection / initialization and immutable has enough introspection on class properties for the class itself, but then technically this should be the outcome (without even taking care of visibility / reflection or runtime behavior)

bapcltd-marv commented 4 years ago

have emailed GitHub re the rather unpleasant commit ref spam caused by rebasing- it should be resolved by an interface change in GitHub "approximately 1-2 quarters from now."