phpspec / prophecy

Highly opinionated mocking framework for PHP 5.3+
MIT License
8.53k stars 242 forks source link

Propehisizing a method with false in the return type leads to a fatal error in PHP8.0.2 #524

Closed pardeep26k closed 3 years ago

pardeep26k commented 3 years ago

Here are the issue details. If I try to mock this interface method it will fail with the below error in PHP 8.0.2 >

PHP Fatal error: Type declaration 'false' must be unqualified in /app/vendor/phpspec/prophecy/src/Prophecy/Doubler/Generator/ClassCreator.php(49) : eval()'d code on line 5

Interface Example:

interface TestInterface
{
    public function runTest(): array|false;
}

class InterfaceTest extends TestCase
{
    use ProphecyTrait;
    public function test_porpecy(): void
    {
        $mockTest = $this->prophesize(TestInterface::class);
        v$mockTest->runTest()->willReturn([]);

    }
}

Discovery

We are currently upgrading PHP to version 8 and our unit test fails with the error described above.

I think the main reason is multiple methods has return type like this


 public  function getMultiByKey(string $server_key, array $keys, int $get_flags = 0): array|\false {
return $this->getProphecy()->makeProphecyMethodCall(__FUNCTION__, func_get_args());
}

false is a keyword, not a global name so it cannot have \

ciaranmcnulty commented 3 years ago

Thanks for the report, it looks like the double's type hint is array|\false where the \false is what's in error

From what I remember it's ReturnTypeNode that would be responsible for knowing which types (classes) need a prefixed \

stof commented 3 years ago

https://github.com/phpspec/prophecy/blob/3f22964cebf655ce6da5acc066ce3a1c7666f279/src/Prophecy/Doubler/Generator/Node/TypeNodeAbstract.php#L45 needs to be updated to account for false on newer PHP versions

pardeep26k commented 3 years ago

https://github.com/phpspec/prophecy/blob/3f22964cebf655ce6da5acc066ce3a1c7666f279/src/Prophecy/Doubler/Generator/Node/TypeNodeAbstract.php#L45

needs to be updated to account for false on newer PHP versions

@stof Any plan to work on this fix?

ciaranmcnulty commented 3 years ago

There's some other behaviour we need to account for too (i.e. false can only be part of a union)

pardeep26k commented 3 years ago

There's some other behaviour we need to account for too (i.e. false can only be part of a union)

@ciaranmcnulty Any idea we merge and release that change?

ciaranmcnulty commented 3 years ago

No ETA, please use a fork if it's blocking you