php-http / client-common

Common HTTP Client implementations and tools for HTTPlug
http://httplug.io
MIT License
1.01k stars 53 forks source link

PHPStan error Promise but does not specify its types: T #235

Closed MarcHagen closed 8 months ago

MarcHagen commented 9 months ago

PHP version: 8.1.22

Description

Method handleRequest() has parameter [...] with generic interface Http\Promise\Promise but does not specify its types: T

How to reproduce

<?php

declare(strict_types=1);

use Http\Client\Common\Plugin;
use Http\Promise\Promise;
use Psr\Http\Message\RequestInterface;

final class Authentication implements Plugin
{
    public function handleRequest(RequestInterface $request, callable $next, callable $first): Promise
    {
        $request = $request->withHeader('Authorization', 'token bla');

        return $next($request);
    }
}

PHPStan config:

parameters:
    level: max
    checkMissingIterableValueType: false
    treatPhpDocTypesAsCertain: false
    paths:
        - src

PHPStan playground: https://phpstan.org/r/52113abc-06c9-4058-a970-4be7b8c9b311 ignore Http\Client\Common\Plugin error

Possible Solution You can add a custom PHPDoc with the correct code, but it seems a bit redundant. https://github.com/php-http/client-common/pull/233#issuecomment-1796282251 has an example of this.

    /**
     * @param callable(RequestInterface): Promise<ResponseInterface> $next
     * @param callable(RequestInterface): Promise<ResponseInterface> $first
     *
     * @return Promise<ResponseInterface>
     */
    public function handleRequest(RequestInterface $request, callable $next, callable $first): Promise

Additional context Seems https://github.com/php-http/client-common/pull/234 is already on the way to get it fixed! But I could hardly find ANY information about "issue". So creating a new issue for documentation purposes.

/cc @ste93cry @Ndiritu

Ndiritu commented 9 months ago

Sorry for the delay. Finally getting to focus on this. Working to resolve it today.

dbu commented 8 months ago

i reverted the annotations in https://github.com/php-http/promise/pull/33 until we can figure out if there even is a way to correctly do them.

ybelenko commented 3 months ago

How about this:

/**
 * Adds behavior for when the promise is resolved or rejected (response will be available, or error happens).
 *
 * If you do not care about one of the cases, you can set the corresponding callable to null
 * The callback will be called when the value arrived and never more than once.
 *
 * @param callable(T): V|null          $onFulfilled called when a response will be available
 * @param callable(\Throwable): V|null $onRejected  called when an exception occurs
 *
 * @return Promise<mixed> a new resolved promise with value of the executed callback (onFulfilled / onRejected)
 *
 * @template V
 */
public function then(callable $onFulfilled = null, callable $onRejected = null);

that's a half measure, but at least something. Following chained Promise will have mixed type. We don't chain calls too often in PHP world anyway.

dbu commented 3 months ago

hm, yeah why not. it would help at least those those who want to handle the promise, if they don't chain things.

have you tried this in a project with phpstan, does it report mistakes correctly and not report issues when the callback is used correctly? if you did, do you want to do a pull request for this? i think the return should explain that we can't precisely specify the return but explain in words what happens.