Update docblock for Process::$pipes #80
Conversation
clue
left a comment
There was a problem hiding this comment.
@gdejong Thanks for spotting, the existing definition is indeed incorrect! 👍
It looks like this should be array<ReadableStreamInterface|WritableStreamInterface> instead of ReadableStreamInterface[]|WritableStreamInterface[].
This subtle different means it can contain a mix of both readable and writable streams, instead of an array of either just readable or just writable streams.
|
Good catch! I'll update the PR! |
Found this when running Psalm in a project of mine that is using reactphp/child-process.
```
Possibly undesired iteration over regular object React\Stream\ReadableStreamInterface
```
for the code:
```
foreach ($this->process->pipes as $pipe) {
```
The docblock now better reflects the fact that `$pipes` is an array of either `ReadableStreamInterface` or `WritableStreamInterface`.
|
Updated (amended to the same commit). Unfortunately the current version of PHPStorm ( For now it means that code like |
clue
left a comment
There was a problem hiding this comment.
@gdejong Thanks for the update, the changes LGTM! 
If static analysis and IDE autocompletion is an issue, you can always add an additional assert($pipe instanceof ReadableStreamInterface || $pipe instanceof WritableStreamInterface) in front of the respective method call.
Found this when running Psalm in a project of mine that is using
reactphp/child-process.for the code:
The docblock now better reflects the fact that
$pipesis an array of eitherReadableStreamInterfaceorWritableStreamInterface.