Using MethodProphecy::willThrow() with interface's name causes ThrowPromise runtime error

[alepdev]

Hello,

in my test case I'm using ExceptionInterface::class (that extends \Throwable) as string argument in willThrow() of MethodProphecy object.

Expected Behavior

When resulting double is used in tested object it will throw a ExceptionInterface

Current Behavior

Error: Call to a member function isPublic() on null

The error returns without a stack trace but it is easily traced back to this: https://github.com/phpspec/prophecy/blob/master/src/Prophecy/Promise/ThrowPromise.php#L76

Steps to Reproduce

My real test case using symfony http client contracts:

    public function test_execute_willThrowException()
    {
        $httpClient = $this->prophesize(\Symfony\Contracts\HttpClient\HttpClientInterface::class);

        // HttpClient request will throw ExceptionInterface
        $httpClient
            ->request(Argument::type('string'), Argument::type('string'))
            ->willThrow(\Symfony\Contracts\HttpClient\Exception\ExceptionInterface::class);

        // Instantiate Client
        $client = new \MyApp\Client(
            $httpClient->reveal()
        );

        $client->sendRequest();
    }

Detailed Description

The error is caused by Prophecy\Promise\ThrowPromise\execute(...) because it assumes that reflected class has __constructor method (it must be a class if we want to instantiate it):

$constructor = $reflection->getConstructor();

if ($constructor->isPublic() && 0 == $constructor->getNumberOfRequiredParameters()) {

The assumption is correct if ThrowPromise's constructor can assure that the received argument must be an instantiable class when is a string, but willThrow() returns an instance of ThrowPromise object without errors.

Looking to issues I've found this PR #429 that fixes #428.

Before this patch ThrowPromise's constructor garantueed assumption made in execute().

I extended the example used in the issue #428 (comment) by using the resulting double, so it now issues the same error I've gotten:

<?php

class ReproductionTest extends \PHPUnit\Framework\TestCase
{
    public function testThrowable()
    {
        $client = $this->prophesize(\Psr\Http\Client\ClientInterface::class);
        $client->sendRequest(Argument::type(\Psr\Http\Message\RequestInterface::class))
            ->willThrow(\Psr\Http\Client\ClientExceptionInterface::class);

        // do assertions
        $this->expectException(\Psr\Http\Client\Client\Symfony\Contracts\HttpClient\Exception\ExceptionInterface::class);

+       $client->reveal()
+           ->sendRequest(
+               $this->prophesize(\Psr\Http\Message\RequestInterface::class)->reveal()
+           );
    }
}

To be sure about issue, I try to improve tests written in PR #429 to spec\Prophecy\Promise\ThrowPromiseSpec and they fails:

<?php

// ...

    function it_throws_an_extension_of_throwable_by_class_name(ObjectProphecy $object, MethodProphecy $method)
    {
        if (!interface_exists('\Throwable')) {
            throw new SkippingException('The interface Throwable, introduced in PHP 7, does not exist');
        }

        $this->beConstructedWith('\Fixtures\Prophecy\ThrowableInterface');

        $this->shouldNotThrow('Prophecy\Exception\InvalidArgumentException')->duringInstantiation();

+       $this->shouldNotThrow('\Error')->duringExecute(array(), $object, $method);
    }

    function it_throws_a_throwable_by_class_name(ObjectProphecy $object, MethodProphecy $method)
    {
        if (!interface_exists('\Throwable')) {
            throw new SkippingException('The interface Throwable, introduced in PHP 7, does not exist');
        }

        $this->beConstructedWith('\Throwable');

        $this->shouldNotThrow('Prophecy\Exception\InvalidArgumentException')->duringInstantiation();

+        $this->shouldNotThrow('\Error')->duringExecute(array(), $object, $method);
    }

I think that patch provided by PR #429 allows to use willThrow() with a name of interface that extends \Throwable but it is not allowed to be used it in tests, because with reflection we can instantiate classes not interfaces.

Possible Solution

If we can't instantiate an interface from ThrowPromise, my proposal is:

• revert #429
• keep tests of #429 with updated prediction that must throw InvalidArgumentException when passing an interface or an extension of it as string name
• documents a workaround (I propose one below)

Possible Implementation

Updated / new tests

    function it_not_throws_an_extension_of_throwable_by_class_name()
    {
        if (!interface_exists('\Throwable')) {
            throw new SkippingException('The interface Throwable, introduced in PHP 7, does not exist');
        }

        $this->beConstructedWith('\Fixtures\Prophecy\ThrowableInterface');

        $this->shouldThrow('Prophecy\Exception\InvalidArgumentException')->duringInstantiation();
    }

    function it_not_throws_a_throwable_by_class_name()
    {
        if (!interface_exists('\Throwable')) {
            throw new SkippingException('The interface Throwable, introduced in PHP 7, does not exist');
        }

        $this->beConstructedWith('\Throwable');

        $this->shouldThrow('Prophecy\Exception\InvalidArgumentException')->duringInstantiation();
    }

Names of tests needs to be updated to better describe that I only add "not" in front.

My workaround

In my case I solved the problem using a Prophecy object of ExceptionInterface

    public function test_execute_willThrowException()
    {
        $httpClient = $this->prophesize(\Symfony\Contracts\HttpClient\HttpClientInterface::class);

        // HttpClient request throw ExceptionInterface
        $httpClient
            ->request(Argument::type('string'), Argument::type('string'))
            ->willThrow($this->prophesize(\Symfony\Contracts\HttpClient\Exception\ExceptionInterface::class)->reveal());

        // Instantiate Client
        $client = new \MyApp\Client(
            $httpClient->reveal()
        );

        $this->expectException(MyCustomException::class);

        $client->sendRequest();
    }

Hoping to have been useful, regards

Alessio

[bogdan-dubyk]

Having same issue, any updates????