-
-
Notifications
You must be signed in to change notification settings - Fork 148
[WIP][2.x] Add template annotations #223
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
base: 2.x
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
parameters: | ||
paths: | ||
- types | ||
level: max |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,8 +13,10 @@ | |
* | ||
* If `$promiseOrValue` is a promise, it will be returned as is. | ||
* | ||
* @param mixed $promiseOrValue | ||
* @return PromiseInterface | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like this actually returns There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
* @template-covariant T | ||
* @template TFulfilled as PromiseInterface<T>|T | ||
* @param TFulfilled $promiseOrValue | ||
* @return (TFulfilled is PromiseInterface ? TFulfilled : PromiseInterface<TFulfilled>) | ||
*/ | ||
function resolve($promiseOrValue = null) | ||
{ | ||
|
@@ -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) | ||
{ | ||
|
@@ -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) | ||
{ | ||
|
@@ -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) | ||
{ | ||
|
@@ -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) | ||
{ | ||
|
@@ -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) | ||
{ | ||
|
@@ -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) | ||
{ | ||
|
@@ -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) | ||
{ | ||
|
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())])); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
|
||
/** | ||
* 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)); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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.