Skip to content

Psalm TooManyTemplateParams error #401

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
gdejong opened this issue Jan 12, 2021 · 4 comments
Closed

Psalm TooManyTemplateParams error #401

gdejong opened this issue Jan 12, 2021 · 4 comments

Comments

@gdejong
Copy link

gdejong commented Jan 12, 2021

Given:

<?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?

@clue
Copy link
Member

clue commented 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 @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?

@zmitic
Copy link

zmitic commented Aug 12, 2021

@clue Question about this:

do indeed use a return PromiseInterface<ResponseInterface,Exception>

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:

/**
 * @template TResolved
 *
 * @param PromiseInterface<TResolved> $promise
 *
 * @return TResolved
 */
function await(PromiseInterface $promise, LoopInterface $loop, $timeout = null){}

@clue
Copy link
Member

clue commented 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.

@clue clue closed this as completed Jan 21, 2023
@WyriHaximus
Copy link
Member

FYI the PR's to address this across our packages are listed here: reactphp/promise#223 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants