zendframework / zend-diactoros

PSR-7 HTTP Message implementation
BSD 3-Clause "New" or "Revised" License
1.55k stars 152 forks source link

Stream construction not possible with string #334

Closed steffenbrand closed 6 years ago

steffenbrand commented 6 years ago

Stream constructor says it accepts a string, but it doesn't. Providing a string results in an InvalidArgumentException. In my opinion, this is misleading.

/**
 * @param string|resource $stream
 * @param string $mode Mode with which to open stream
 * @throws Exception\InvalidArgumentException
 */
public function __construct($stream, string $mode = 'r')
{
    $this->setStream($stream, $mode);
}

Suggestion

Simply remove string from @param string|resource $stream, because I have a feeling providing a simple string was never intended.

Feel free to correct me ;)

froschdesign commented 6 years ago

@steffenbrand Are your sure? Please look at:

https://github.com/zendframework/zend-diactoros/blob/83743b4558265e01e74b9da9987441628915d67a/src/Stream.php#L331-L341

https://github.com/zendframework/zend-diactoros/blob/83743b4558265e01e74b9da9987441628915d67a/test/StreamTest.php#L52

https://github.com/zendframework/zend-diactoros/blob/83743b4558265e01e74b9da9987441628915d67a/test/StreamTest.php#L62-L65

steffenbrand commented 6 years ago

Ah, sure php://memory is a string as well.

Maybe removing the string is not the right suggestion ;) But I think an additional hint would help that the string must actually be a stream identifier, maybe with a link to http://php.net/manual/de/wrappers.php.php.

To be honest, it actually says must be a string stream identifier or stream resource in the exception, but I didn't get the meaning of string stream identifier at first.

My expectation was, that I could use it like new Stream('my string that this class is going to wrap in some kind of I/O stream by itself.');

weierophinney commented 6 years ago

@steffenbrand If you are able to upgrade to version 2 (which, unless you are using the emitters or on pre-7.1 versions of PHP, is fully backwards compatible), you can use Zend\Diactoros\StreamFactory::createString() to do what you suggest.

We'll definitely accept a patch that would clarify the constructor argument, however.

steffenbrand commented 6 years ago

Thank you for the answer! I guess we can update to ZE 3.2 / Diactoros 2 and use the StreamFactory, as you suggested. :v:

If I come up with a clarifying sentence, I'll provide a PR.