-
-
Notifications
You must be signed in to change notification settings - Fork 150
Update PHPStan and fix type annotations #267
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: 3.x
Are you sure you want to change the base?
Conversation
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.
@PaulRotmann Thanks for looking into this, updating PHPStan sounds good. I've added some remarks below.
/** | ||
* @param callable(): (void|PromiseInterface<void>) $onFulfilledOrRejected | ||
* @return PromiseInterface<T> | ||
*/ |
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.
Tested this locally and this doesn't appear to be needed, as its types would be inherited directly from the PromiseInterface
:
/** | |
* @param callable(): (void|PromiseInterface<void>) $onFulfilledOrRejected | |
* @return PromiseInterface<T> | |
*/ |
May I ask you do confirm?
/** | ||
* @param callable(): (void|PromiseInterface<void>) $onFulfilledOrRejected | ||
* @return PromiseInterface<T> | ||
*/ |
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.
Tested this locally and this doesn't appear to be needed, as its types would be inherited directly from the PromiseInterface
:
/** | |
* @param callable(): (void|PromiseInterface<void>) $onFulfilledOrRejected | |
* @return PromiseInterface<T> | |
*/ |
May I ask you do confirm?
public function finally(callable $onFulfilledOrRejected): PromiseInterface | ||
{ | ||
return $this->then(function ($value) use ($onFulfilledOrRejected): PromiseInterface { | ||
/** @var T $value */ |
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.
nit: Mixed whitespace, should use spaces only
/** @var T $value */ | |
/** @var T $value */ |
public function finally(callable $onFulfilledOrRejected): PromiseInterface | ||
{ | ||
return $this->then(static function ($value) use ($onFulfilledOrRejected): PromiseInterface { | ||
/** @var T $value */ |
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.
nit: Mixed whitespace, should use spaces only
/** @var T $value */ | |
/** @var T $value */ |
@@ -28,7 +28,7 @@ | |||
"php": ">=7.1.0" | |||
}, | |||
"require-dev": { | |||
"phpstan/phpstan": "1.10.39 || 1.4.10", | |||
"phpstan/phpstan": "1.12.19 || 1.4.10", |
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.
@PaulRotmann This is quite old, are you sure we don't want to go to 1.12.28 at least?
The updated PHPStan version provides improved type checking capabilities. The added type annotations resolve static analysis warnings and improve code documentation, making the generic type flow more explicit in the finally() method implementations.
Builds on top of #251.
This PR breaks down #266 into smaller parts.