Issue with Mockery and Stream::close()

[designcise]

In PHP8, https://github.com/Nyholm/psr7/blob/master/src/Stream.php#L128 causes an issue with mockery:

Fatal error: Declaration of Mockery_0_Nyholm_Psr7_Stream_Nyholm_Psr7_Stream_Psr_Http_Message_StreamInterface::close() must be compatible with Nyholm\Psr7\Stream::close(): void

[sunkan]

Just one question

Why are you mocking the implementation and not the interface?

[designcise]

i am mocking the interface, and comparing the result with one from implemented library:

public function testReturnsBodyWhenNotReadableButIsSeekable(): void
    {
        $stream = HttpFactory::createStream('Hello world!');
        $mockedStream = Mockery::mock($stream, StreamInterface::class)->makePartial();
        $mockedStream->shouldReceive('isSeekable')->andReturn(true);
        $mockedStream->shouldReceive('isReadable')->andReturn(false);

        $response = HttpFactory::createResponse()
            ->withBody($mockedStream);

        ob_start();
        $this->emitter->emit($response);
        self::assertSame('Hello world!', ob_get_clean());
    }

Might be flaky, but this is to test if the library is compatible with the framework. In any case though, shouldn't the method signatures be fully compliant with the PSR interfaces?

Edit: Maybe I need to rethink the test.

[Nyholm]

Hm.. Im not sure why Mockery is giving you that error. It seams like a mockery issue because this is all valid PHP code:

https://3v4l.org/98tQc

[Zegnat]

Could you share the test you are running with Nyholm/psr7 to have a look at?

I wanted to see if I could reproduce this, but I do not know what code to write to do so. The test you shared is using an HttpFactory that isn‘t shipped by this library.

[designcise]

Sorry, I've been super busy, totally forgot about this until I got back into developing my project again.

The HttpFactory::createStream method is simply creating a Stream (see source here: https://github.com/designcise/bitframe/blob/master/src/Factory/HttpFactory.php#L132) using Nyholm\Psr7\Factory\Psr17Factory. The problem is not there though. It is because of a return type mismatch. The error started popping up after installing PHP 8. Perhaps PHP 8.0+ has stricter type checking. Anyway, the error is as follows:

Fatal error: Declaration of Mockery_0_Nyholm_Psr7_Stream_Nyholm_Psr7_Stream_Psr_Http_Message_StreamInterface::close() must be compatible with Nyholm\Psr7\Stream::close(): void

It happens because of return types mismatching between https://github.com/php-fig/http-message/blob/master/src/StreamInterface.php#L35 and https://github.com/Nyholm/psr7/blob/master/src/Stream.php#L128.

Perhaps you could consider strictly adhering to the return types + type hints in the PSR interfaces? Which in this case would mean you remove :void from the close() method for example. What do you think?

[designcise]

Actually the same problem exists in other places as well; for example:

Fatal error: Declaration of Mockery_0_Nyholm_Psr7_Stream_Nyholm_Psr7_Stream_Psr_Http_Message_StreamInterface::getSize() must be compatible with Nyholm\Psr7\Stream::getSize(): ?int

See https://github.com/php-fig/http-message/blob/master/src/StreamInterface.php#L51 vs. https://github.com/Nyholm/psr7/blob/master/src/Stream.php#L161

[Zegnat]

What exactly is the declared return type for Mockery_0_Nyholm_Psr7_Stream_Nyholm_Psr7_Stream_Psr_Http_Message_StreamInterface::close() or Mockery_0_Nyholm_Psr7_Stream_Nyholm_Psr7_Stream_Psr_Http_Message_StreamInterface::getSize()?

The Nyholm/psr7 library is making use of a feature from PHP’s type system called covariance (cf. Wikipedia):

Covariance allows a child's method to return a more specific type than the return type of its parent's method.

The PSR-7 interface was written (released 2016-05-18) when return typing was not yet part of PHP (introduced in PHP 7, 2019-01-10). But as Nyholm/psr7 only supports PHP versions 7.1 and up that should not be holding it back from using return types, and thanks to covariance we can.

Could it be that Mockery does not support covariance between interfaces and implementations?

---

I pulled down Mockery and had another go and recreating the problem, and the following fixed it for me:

  public function testReturnsBodyWhenNotReadableButIsSeekable(): void
      {
          $stream = HttpFactory::createStream('Hello world!');
-         $mockedStream = Mockery::mock($stream, StreamInterface::class)->makePartial();
+         $mockedStream = Mockery::mock($stream)->makePartial();
          $mockedStream->shouldReceive('isSeekable')->andReturn(true);
          $mockedStream->shouldReceive('isReadable')->andReturn(false);

It looks like it might be trying to apply the StreamInterface to its own mock after having created a Nyholm/psr7 Stream compatible mock. This means it might try to overwrite the more precise void with no return type. That is the oposite of covariance and is specifically not allowed by the language.

Replacing StreamInterface::class with something like $stream::class or \Nyholm\Psr7\Stream::class seems to work as well. But leaving the second argument off from the Mockery::mock() call is probably better? Unsure, as I have never used Mockery myself.

[designcise]

I thought about whether this could be a Mockery specific issue, but then it doesn't explain why it only happens in PHP 8 and works totally fine up until PHP 7.4. I will try to investigate this more if I find the time, but for now, your suggestion about $stream::class should do the trick for PHP 8+. Thanks for pointing out that out, it totally slipped my mind that class name literals are allowed on objects in PHP 8+ -- that should solve my problem for now.

[Zegnat]

I thought about whether this could be a Mockery specific issue, but then it doesn't explain why it only happens in PHP 8 and works totally fine up until PHP 7.4.

For me it does not work under earlier PHP versions either. I have the following little file:

<?php declare(strict_types = 1);

require_once __DIR__ . "/vendor/autoload.php";

error_reporting(\E_ALL);
echo phpversion() . PHP_EOL;

$stream = (new \Nyholm\Psr7\Factory\Psr17Factory())->createStream('');
\Mockery::mock($stream, \Psr\Http\Message\StreamInterface::class)->makePartial();

And I ran this using the official PHP Docker images to test different versions of PHP that are supported by Mockery (php:8.0-cli, php:7.4-cli, php:7.3-cli). Outputs:

8.0.8

Fatal error: Declaration of Mockery_0_Nyholm_Psr7_Stream_Nyholm_Psr7_Stream_Psr_Http_Message_StreamInterface::close() must be compatible with Nyholm\Psr7\Stream::close(): void in /usr/src/myapp/vendor/mockery/mockery/library/Mockery/Loader/EvalLoader.php(34) : eval()'d code on line 973

7.4.21

Fatal error: Declaration of Mockery_0_Nyholm_Psr7_Stream_Nyholm_Psr7_Stream_Psr_Http_Message_StreamInterface::close() must be compatible with Nyholm\Psr7\Stream::close(): void in /usr/src/myapp/vendor/mockery/mockery/library/Mockery/Loader/EvalLoader.php(34) : eval()'d code on line 973

7.3.29

Fatal error: Declaration of Mockery_0_Nyholm_Psr7_Stream_Nyholm_Psr7_Stream_Psr_Http_Message_StreamInterface::close() must be compatible with Nyholm\Psr7\Stream::close(): void in /usr/src/myapp/vendor/mockery/mockery/library/Mockery/Loader/EvalLoader.php(34) : eval()'d code on line 30

I am happy to have a more indepth look myself if you can provide me with a barebones setup that works on 7.4 but has stopped working on 8.0. But as far as I can see, Mockery has never worked with this code and probably just does not support adding an interface to a mock of a class that has narrowed the return types.

Just removing the second constructor argument and running \Mockery::mock($stream)->makePartial() works for me under all the above versions.

I am going to close this issue as – at least to me – this feels very much as a Mockery issue (if it is an issue at all) and not something we have done. I see nothing actionable for us here. But feel free to continue posting your findings here, as I am interested to hear what you figure out. And if you do find something actionable on our part the issue will just be reopened :)