You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
<?php
use React\EventLoop\Factory;
use React\Http\Browser;
$loop = Factory::create();
$browser = new Browser($loop);
$browser->get("");
and running Psalm on it with level 6 or lower, I get the following error:
ERROR: TooManyTemplateParams - test.php:10:1 - React\Promise\PromiseInterface<Psr\Http\Message\ResponseInterface> has too many template params, expecting 0 (see https://psalm.dev/184)
$browser->get("");
I am not sure if this is the right place, but I was wondering if you could shed some light on this.
Perhaps Psalm doesn't like the @return PromiseInterface<ResponseInterface> type hint in \React\Http\Browser::get?
Maybe the questions should be, does this project aims to be compatible with Psalm?
The get() method (any many others) do indeed use a @return PromiseInterface<ResponseInterface,Exception> annotation.
This was added a while ago, i.e. before PHPStan/Psalm started supporting the @template annotation.
Nowadays, we should probably add a @template annotation to the promise component. In this case, the definition in this repository will actually become "valid".
As an alternative, we may also change this annotation to only @return PromiseInterface without using any templates. In this case we would make static analysis tools "happy", but would loose additional information that can be helpful for developers.
Does anybody feel like looking into this and filing a PR?
Promise template types will be added in reactphp/promise#227 (and referenced tickets) soon, so I don't think there's much that needs to be done here. I'll close this ticket for now, but please report back if there's anything that is missing here.
Activity
clue commentedon Jan 20, 2021
@gdejong Thanks for reporting!
The
get()method (any many others) do indeed use a@return PromiseInterface<ResponseInterface,Exception>annotation.This was added a while ago, i.e. before PHPStan/Psalm started supporting the
@templateannotation.Nowadays, we should probably add a
@templateannotation to the promise component. In this case, the definition in this repository will actually become "valid".As an alternative, we may also change this annotation to only
@return PromiseInterfacewithout using any templates. In this case we would make static analysis tools "happy", but would loose additional information that can be helpful for developers.Does anybody feel like looking into this and filing a PR?
zmitic commentedon Aug 12, 2021
@clue Question about this:
For reference, I am using https://github.com/Bocmah/psalm-reactphp-promise-plugin/blob/main/stubs/PromiseInterface.phpstub and have no problems with psalm on level 1 but there is just 1 templated param.
Stub for await also gives me correct return type:
clue commentedon Jan 21, 2023
Promise template types will be added in reactphp/promise#227 (and referenced tickets) soon, so I don't think there's much that needs to be done here. I'll close this ticket for now, but please report back if there's anything that is missing here.
WyriHaximus commentedon Jan 21, 2023
FYI the PR's to address this across our packages are listed here: reactphp/promise#223 (comment)