Skip to content
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

[WIP][2.x] Add template annotations #223

Draft
wants to merge 1 commit into
base: 2.x
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,16 @@ jobs:
uses: docker://hhvm/hhvm:3.30-lts-latest
with:
args: hhvm vendor/bin/phpunit

static-analysis:
name: PHPStan
runs-on: ubuntu-20.04
continue-on-error: true
steps:
- uses: actions/checkout@v3
- uses: shivammathur/setup-php@v2
with:
php-version: 8.1
- run: composer require phpstan/phpstan
- name: Execute type checking
run: vendor/bin/phpstan --configuration="phpstan.types.neon.dist"
1 change: 1 addition & 0 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
"php": ">=5.4.0"
},
"require-dev": {
"phpstan/phpstan": "^1.9",
"phpunit/phpunit": "^9.5 || ^5.7 || ^4.8.36"
},
"autoload": {
Expand Down
4 changes: 4 additions & 0 deletions phpstan.types.neon.dist
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
parameters:
paths:
- types
level: max
5 changes: 3 additions & 2 deletions src/ExtendedPromiseInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,9 @@ public function otherwise(callable $onRejected);
* ->always('cleanup');
* ```
*
* @param callable $onFulfilledOrRejected
* @return ExtendedPromiseInterface
* @template TReturn of mixed
* @param callable(T): TReturn $onFulfilledOrRejected
* @return (TReturn is ExtendedPromiseInterface ? TReturn : ExtendedPromiseInterface<TReturn>)
*/
public function always(callable $onFulfilledOrRejected);

Expand Down
13 changes: 13 additions & 0 deletions src/FulfilledPromise.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,19 @@

/**
* @deprecated 2.8.0 External usage of FulfilledPromise is deprecated, use `resolve()` instead.
* @template-implements PromiseInterface
* @template-covariant T
*/
class FulfilledPromise implements ExtendedPromiseInterface, CancellablePromiseInterface
{
/**
* @var T
*/
private $value;

/**
* @param T $value
*/
public function __construct($value = null)
{
if ($value instanceof PromiseInterface) {
Expand All @@ -18,6 +26,11 @@ public function __construct($value = null)
$this->value = $value;
}

/**
* @template TFulfilled as PromiseInterface<T>|T
* @param (callable(T): TFulfilled)|null $onFulfilled
* @return ($onFulfilled is null ? $this : (TFulfilled is PromiseInterface ? TFulfilled : PromiseInterface<TFulfilled>))
*/
public function then(callable $onFulfilled = null, callable $onRejected = null, callable $onProgress = null)
{
if (null === $onFulfilled) {
Expand Down
13 changes: 13 additions & 0 deletions src/Promise.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,17 @@

namespace React\Promise;

/**
* @template-implements PromiseInterface
* @template-covariant T
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to also have a type parameter for rejection reason. But I have tried doing that for guzzlehttp/promisees and it was a pain.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if it's a pain, does it work? Guess, based on no PR associated with those changes it doesn't?

One of the issues is literally this: guzzle/promises@master...jtojnar:guzzle-promises:generic-types-2#diff-08c91937f94e1c62ad95bd704218d97cff992fb41a9b71b7fa9519442f8d67dbR33 that checks what is passed in to handle both scenarios not what is coming out of the previous promise.

That is one of the reasons we decided not to bother with rejections (for now).

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made a PR but gave up because the various handler propagations were too difficult to reason about and I ran out of time I allocated for the experiment.

*/
class Promise implements ExtendedPromiseInterface, CancellablePromiseInterface
{
private $canceller;

/**
* @var PromiseInterface<T>
*/
private $result;

private $handlers = [];
Expand All @@ -25,6 +33,11 @@ public function __construct(callable $resolver, callable $canceller = null)
$this->call($cb);
}

/**
* @template TFulfilled as PromiseInterface<T>|T
* @param (callable(T): TFulfilled)|null $onFulfilled
* @return ($onFulfilled is null ? $this : (TFulfilled is PromiseInterface ? TFulfilled : PromiseInterface<TFulfilled>))
*/
public function then(callable $onFulfilled = null, callable $onRejected = null, callable $onProgress = null)
{
if (null !== $this->result) {
Expand Down
10 changes: 6 additions & 4 deletions src/PromiseInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

namespace React\Promise;

/**
* @template-covariant T
*/
interface PromiseInterface
{
/**
Expand Down Expand Up @@ -32,10 +35,9 @@ interface PromiseInterface
* than once.
* 3. `$onProgress` (deprecated) may be called multiple times.
*
* @param callable|null $onFulfilled
* @param callable|null $onRejected
* @param callable|null $onProgress This argument is deprecated and should not be used anymore.
* @return PromiseInterface
* @template TFulfilled as PromiseInterface<T>|T
* @param (callable(T): TFulfilled)|null $onFulfilled
* @return ($onFulfilled is null ? $this : (TFulfilled is PromiseInterface ? TFulfilled : PromiseInterface<TFulfilled>))
*/
public function then(callable $onFulfilled = null, callable $onRejected = null, callable $onProgress = null);
}
47 changes: 29 additions & 18 deletions src/functions.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@
*
* If `$promiseOrValue` is a promise, it will be returned as is.
*
* @param mixed $promiseOrValue
* @return PromiseInterface
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this actually returns ExtendedPromiseInterface.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically yes, but that isn't the base contract we're using. This is also why in v3 ExtendedPromiseInterface has been merged into PromiseInterface.

* @template-covariant T
* @template TFulfilled as PromiseInterface<T>|T
* @param TFulfilled $promiseOrValue
* @return (TFulfilled is PromiseInterface ? TFulfilled : PromiseInterface<TFulfilled>)
*/
function resolve($promiseOrValue = null)
{
Expand Down Expand Up @@ -52,8 +54,10 @@ function resolve($promiseOrValue = null)
* throwing an exception. For example, it allows you to propagate a rejection with
* the value of another promise.
*
* @param mixed $promiseOrValue
* @return PromiseInterface
* @template T is null
* @template R
* @param R $promiseOrValue
* @return PromiseInterface<T>
*/
function reject($promiseOrValue = null)
{
Expand All @@ -72,8 +76,9 @@ function reject($promiseOrValue = null)
* will be an array containing the resolution values of each of the items in
* `$promisesOrValues`.
*
* @param array $promisesOrValues
* @return PromiseInterface
* @template T
* @param array<PromiseInterface<T>|T> $promisesOrValues
* @return PromiseInterface<array<T>>
*/
function all($promisesOrValues)
{
Expand All @@ -89,8 +94,9 @@ function all($promisesOrValues)
* The returned promise will become **infinitely pending** if `$promisesOrValues`
* contains 0 items.
*
* @param array $promisesOrValues
* @return PromiseInterface
* @template T
* @param array<PromiseInterface<T>|T> $promisesOrValues
* @return PromiseInterface<T>
*/
function race($promisesOrValues)
{
Expand Down Expand Up @@ -126,8 +132,9 @@ function race($promisesOrValues)
* The returned promise will also reject with a `React\Promise\Exception\LengthException`
* if `$promisesOrValues` contains 0 items.
*
* @param array $promisesOrValues
* @return PromiseInterface
* @template T
* @param array<PromiseInterface<T>|T> $promisesOrValues
* @return PromiseInterface<T>
*/
function any($promisesOrValues)
{
Expand All @@ -151,9 +158,10 @@ function any($promisesOrValues)
* The returned promise will also reject with a `React\Promise\Exception\LengthException`
* if `$promisesOrValues` contains less items than `$howMany`.
*
* @param array $promisesOrValues
* @template T
* @param array<PromiseInterface<T>|T> $promisesOrValues
* @param int $howMany
* @return PromiseInterface
* @return PromiseInterface<array<T>>
*/
function some($promisesOrValues, $howMany)
{
Expand Down Expand Up @@ -228,9 +236,11 @@ function some($promisesOrValues, $howMany)
* The map function receives each item as argument, where item is a fully resolved
* value of a promise or value in `$promisesOrValues`.
*
* @param array $promisesOrValues
* @param callable $mapFunc
* @return PromiseInterface
* @template-covariant T
* @template TFulfilled as PromiseInterface<T>|T
* @param array<PromiseInterface<T>|T> $promisesOrValues
* @param callable(T): TFulfilled $mapFunc
* @return PromiseInterface<array<T>>
*/
function map($promisesOrValues, callable $mapFunc)
{
Expand Down Expand Up @@ -276,10 +286,11 @@ function ($mapped) use ($i, &$values, &$toResolve, $resolve) {
* promise, *and* `$initialValue` may be a promise or a value for the starting
* value.
*
* @param array $promisesOrValues
* @param callable $reduceFunc
* @template T
* @param array<PromiseInterface<T>|T> $promisesOrValues
* @param callable(T): bool $reduceFunc
* @param mixed $initialValue
* @return PromiseInterface
* @return PromiseInterface<array<T>>
*/
function reduce($promisesOrValues, callable $reduceFunc, $initialValue = null)
{
Expand Down
75 changes: 75 additions & 0 deletions types/Promises.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
<?php

use React\Promise\FulfilledPromise;
use React\Promise\PromiseInterface;
use Throwable;

use function PHPStan\Testing\assertType;
use function React\Promise\all;
use function React\Promise\any;
use function React\Promise\race;
use function React\Promise\reject;
use function React\Promise\resolve;

$passThroughBoolFn = static fn (bool $bool): bool => $bool;
$passThroughThrowable = static function (Throwable $t): PromiseInterface {
return reject($t);
};
$stringOrInt = function (): int|string {
return time() % 2 ? 'string' : time();
};
$tosseable = new Exception('Oops I did it again!');

/**
* basic
*/
assertType('React\Promise\PromiseInterface<bool>', resolve(true));
assertType('React\Promise\PromiseInterface<int|string>', resolve($stringOrInt()));
assertType('React\Promise\PromiseInterface<bool>', resolve(resolve(true)));

/**
* chaining
*/
assertType('React\Promise\PromiseInterface<bool>', resolve(true)->then($passThroughBoolFn));
assertType('React\Promise\PromiseInterface<bool>', resolve(true)->then()->then($passThroughBoolFn));
assertType('React\Promise\PromiseInterface<bool>', resolve(true)->then(null)->then($passThroughBoolFn));
assertType('React\Promise\PromiseInterface<bool>', resolve(true)->then($passThroughBoolFn)->then($passThroughBoolFn));
assertType('React\Promise\PromiseInterface<bool>', resolve(true)->then($passThroughBoolFn, $passThroughThrowable)->then($passThroughBoolFn));
assertType('React\Promise\PromiseInterface<bool>', resolve(true)->then(null, $passThroughThrowable)->then($passThroughBoolFn));
assertType('React\Promise\PromiseInterface<bool>', resolve(true)->then()->then(null, $passThroughThrowable)->then($passThroughBoolFn));

/**
* all
*/
assertType('React\Promise\PromiseInterface<array<bool>>', all([resolve(true), resolve(false)]));
assertType('React\Promise\PromiseInterface<array<bool>>', all([resolve(true), false]));
assertType('React\Promise\PromiseInterface<array<bool|int>>', all([true, time()]));
assertType('React\Promise\PromiseInterface<array<bool|int>>', all([resolve(true), resolve(time())]));
assertType('React\Promise\PromiseInterface<array<bool|float>>', all([resolve(true), microtime(true)]));
assertType('React\Promise\PromiseInterface<array<bool|int>>', all([true, resolve(time())]));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps it would be fine to comment these out for now. Most of the time people will be using homogeneous lists anyway. And if someone needs to use a heterogeneous one, they can always add it to ignore list in phpstan.neon.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lines 46 and 47 work fine, lines 48 and 49 don't. If the bug in PHPStan doesn't get fixed before this PR will be merged we'll comment 48 and 49 out. As that's not a very common situation. 46 and maybe 47 I have a lot in applications where I don't directly control the typing and accept multiple. An example is the GitHub client I've been generating having a Repository and a SimpleRepository. Both work for where I use them and it gets either depending on where something originates from. So something returning an array with both is perfectly possible.


/**
* any
*/
assertType('React\Promise\PromiseInterface<bool>', any([resolve(true), resolve(false)]));
assertType('React\Promise\PromiseInterface<bool>', any([resolve(true), false]));
assertType('React\Promise\PromiseInterface<bool|int>', any([true, time()]));
assertType('React\Promise\PromiseInterface<bool|int>', any([resolve(true), resolve(time())]));
assertType('React\Promise\PromiseInterface<bool|float>', any([resolve(true), microtime(true)]));
assertType('React\Promise\PromiseInterface<bool|int>', any([true, resolve(time())]));

/**
* race
*/
assertType('React\Promise\PromiseInterface<bool>', race([resolve(true), resolve(false)]));
assertType('React\Promise\PromiseInterface<bool>', race([resolve(true), false]));
assertType('React\Promise\PromiseInterface<bool|int>', race([true, time()]));
assertType('React\Promise\PromiseInterface<bool|int>', race([resolve(true), resolve(time())]));
assertType('React\Promise\PromiseInterface<bool|float>', race([resolve(true), microtime(true)]));
assertType('React\Promise\PromiseInterface<bool|int>', race([true, resolve(time())]));

/**
* direct class access (deprecated!!!)
*/
assertType('React\Promise\FulfilledPromise<bool>', new FulfilledPromise(true));
assertType('React\Promise\PromiseInterface<bool>', (new FulfilledPromise(true))->then($passThroughBoolFn));