phpstan-prophecy icon indicating copy to clipboard operation
phpstan-prophecy copied to clipboard

Remove not longer required stubs

Open alexander-schranz opened this issue 1 year ago • 8 comments

This pull request replaces https://github.com/Jan0707/phpstan-prophecy/pull/298 which did replacer @template with @template-covariant

Since Prophecy 1.17.0 the template are correctly added to prophecy itself and so the stubs are not longer required. Thx to @stof.

alexander-schranz avatar Sep 16 '23 11:09 alexander-schranz

@alexander-schranz

Would you be interested in #285?

localheinz avatar Sep 16 '23 11:09 localheinz

@localheinz Sure it looks like it is not a lot of work todo here and I'm if some issue appear it looks like I'm already the first one stumble over it as I use prophecy with the plugin in most projects.

alexander-schranz avatar Sep 16 '23 12:09 alexander-schranz

I'm actually wondering whether this extension is still needed at all. Prophecy also has generic signatures for prophesize() methods

I verified this and there is it still doesn't work without the extension (PHP 8.2, PhpUnit 10, phpstan 1.10).

Test case:

final class SimpleProphecyTestCase extends TestCase
{
    use ProphecyTrait;

    public function testCreatedViaTrait(): void
    {
        $foo = $this->prophesize(Foo::class);
        $foo->foo()->willReturn('foo');
        $foo = $foo->reveal();

        self::assertSame('foo', $foo->foo());
    }

     public function testCreatedInline(): void
    {
        $foo = (new Prophet())->prophesize(Foo::class);
        $foo->foo()->willReturn('foo');
        $foo = $foo->reveal();

        self::assertSame('foo', $foo->foo());
    }
}
class Foo
{
    public function foo(): string
    {
        return 'foo';
    }
}

Output:

 ------ -----------------------------------------------------------------------------------
  Line   SimpleProphecyTestCase.php
 ------ -----------------------------------------------------------------------------------
  20     Call to an undefined method Prophecy\Prophecy\ObjectProphecy::foo().
  27     Call to an undefined method Prophecy\Prophecy\ObjectProphecy<AppTest\Foo>::foo().
 ------ -----------------------------------------------------------------------------------

To make dynamic return types working prophesize method should return T instead of ObjectProphecy<T> but this won't be technically correct.

Note that method in ProphecyTrait does not specify generic types 🤔

It seems this extension is still required.

jseparovic1 avatar Mar 13 '24 10:03 jseparovic1

I'm actually wondering whether this extension is still needed at all. Prophecy also has generic signatures for prophesize() methods

Two answers to this question.

  1. When using the ProphecyTrait ($this->prophesize) instead of (new Prophet())->prophesize, the Prophecy generics are not used because the prophesize method override does not include them.
  2. Even when the ProphecyTrait override PHPDoc is amended or the Prophecy method is used directly, without this extension here I get lots of PHPStan errors like Call to an undefined method Prophecy\Prophecy\ObjectProphecy<SomeClass>::someMethod() in cases where I create a prophecy object and then use like $prophecy->someMethod()->willReturn('something');

InvisibleSmiley avatar Mar 13 '24 10:03 InvisibleSmiley

ah indeed. I forgot the part covering the magic calls.

But I think WillExtendOrImplementDynamicReturnTypeExtension and ProphesizeDynamicReturnTypeExtension are not needed anymore.

Note that method in ProphecyTrait does not specify generic types

this should be solved by doing a PR on the prophecy-phpunit repo IMO.

stof avatar Mar 13 '24 10:03 stof

Note that method in ProphecyTrait does not specify generic types

this should be solved by doing a PR on the prophecy-phpunit repo IMO.

Obviously. :)

InvisibleSmiley avatar Mar 13 '24 10:03 InvisibleSmiley

And the TypeNodeResolverExtension should probably be deprecated because it is about transforming types like @var ObjectProphecy|Foo into ObjectProphecy<Foo> but there is no valid reason to write ObjectProphecy|Foo in phpdoc anymore as a hack (especially given that phpstan will complain about the missing generic type for ObjectProphecy)

stof avatar Mar 13 '24 10:03 stof

Actually, I just did the PR for the ProphecyTrait myself: https://github.com/phpspec/prophecy-phpunit/pull/63

stof avatar Mar 13 '24 11:03 stof