Return types on mocked methods cause static analysis failures

[asgrim]

A method being mocked might be called like:

$this->mockedClass->someMethod()->willReturn(123);

Even if $mockedClass is annotated correctly, such as:

/** @var MyClass|ObjectProphecy */
private $mockedClass;

someMethod() for example might return an int, so you'd get a warning from phpstan like:

Cannot call method willReturn() on int. 

Not sure if this is even possible to handle really, but thought I'd at least bring it up for discussion! Thanks!

[asgrim]

FWIW, as a workaround I have:

parameters:
    ignoreErrors:
        - '#Cannot call method (?:willReturn|shouldBeCalledOnce|shouldNotBeCalled|shouldBeCalled|shouldNotHaveBeenCalled)\(\) on .*\.#'

[Jean85]

The point is that with the annotation, you lost the special dynamic type provided by this extension. I personally dislike using class properties to store mocks, and this is one of the reasons.

[asgrim]

Ya, but if you annotate without ObjectProphecy, you also get something like:

Property MyTest::$mockedClass (MyClass) does not accept Prophecy\Prophecy\ObjectProphecy<MyClass>.

Doesn't work either way 😞

[asgrim]

Another related case, if the mocked method is marked as @return void (and presumably in 7.1+ : void) then you'll get:

Result of method MyClass::myMethodReturningVoid() (void) is used.

Which is a little more difficult to suppress only for the Prophecy-specific use cases (as the regex may match something where we really ARE using a void return type) 😟

[DASPRiD]

Technically, the prophecy is not an instance of MyClass, but the result of ObjectProphecy::reveal() is. Thus the type hint should be for ObjectProphecy. Correct me if I'm wrong :)

[asgrim]

Technically, the prophecy is not an instance of MyClass, but the result of ObjectProphecy::reveal() is. Thus the type hint should be for ObjectProphecy. Correct me if I'm wrong :)

Ah interesting, yes indeed that actually fixes the problem, and that makes a lot of sense. The extension works on ObjectProphecy but the variance introduced by the MyType|Prophecy annotation (which is unnecessary) confuses things. I'll try this approach on this codebase I'm working on, and I'll personally hold @DASPRiD responsible if it doesn't work 😁

[ondrejmirtes]

Shouldn’t it be an intersection type instead of union?

(Btw an extension can be written so that Foo|ObjectProphecy is also implemented as intersection because that’s usully what people mean.)

[asgrim]

Yes, you're right. Foo&ObjectProphecy works, but not quite supported by phpstorm yet, but will be soon! Gonna close this as it's not this library issue, just correct annotations required :D thanks folks!

[asgrim]

Hmm, seems that doesn't work for everything: https://phpstan.org/r/0175d49a784ee8f429587b4295093e7b

Interfaces don't like being intersected for some reason: MyInterface&ObjectProphecy will complain with something like:

Property MyInterface::$property (MyInterface&Prophecy\Prophecy\ObjectProphecy) does not accept Prophecy\Prophecy\ObjectProphecy<MyInterface>.

[Jean85]

@asgrim that's an incomplete example, that wouldn't work without Prophecy and the exension. Intersection means that the object must implement both interfaces/class, so the only way to have it is to use a proper prophesized object.

[asgrim]

Indeed; https://twitter.com/JanTvrdik/status/1057601523524530176 Jan explained this because of the way phpstan resolves my example. The extension doesn't make any difference at all here.

I think the correct annotation is ObjectProphecy<TheType> I'm just trying it out now :)

[asgrim]

Yes, indeed that seems to work better 👍 Thanks folks!

[DASPRiD]

Wait, we do not have generics yet! 😜

[asgrim]

I think it needs at least ^0.10.4 of phpstan 👍

[ondrejmirtes]

Nope, the ObjectProphecy<TheType> wouldn't currently work in 0.10.4, that's just for iterable types.

There's an open PR #19 that makes TheType|ObjectProphecy the thing you want automatically, it could be updated to support ObjectProphecy<TheType> with a few lines of code.

[asgrim]

Hmm, why does it work already then? O_O (using phpstan 0.10.5 and phpstan-prophecy 0.2.0).... /me confused

[ondrejmirtes]

My tip is that it makes it an ErrorType which is basically mixed. There’s yet no rule for checking unresolvable property types...

[asgrim]

Can confirm TheType|ObjectProphecy works with your PR #19 👍