Fix PHPStan Issues#291
Conversation
Made the constructor final in Collection.php to prevent overriding, ensuring safe usage of new static(). This resolves the PHPStan issue where extending the class with a different constructor signature could cause runtime errors.
Added missing return true; statements in validatePayload() and validateRefresh() methods of Expiration.php, IssuedAt.php, and NotBefore.php. This resolves PHPStan errors about missing return values in methods expected to return bool.
Refactored return $this->displayKey($key); to explicitly call displayKey($key); before returning. The issue was returning a value from a method (displayKey) that has a void return type, which PHPStan flagged as an error.
Added entries to the CHANGELOG for fixes addressing PHPStan issues: - Ensured displayKey($key); is called before returning in JWTGenerateSecretCommand. - Added missing return true; statements in Expiration.php, IssuedAt.php, and NotBefore.php. - Made the constructor final in Collection.php to fix new static() usage.
mfn
left a comment
mfn
left a comment
There was a problem hiding this comment.
I'm strongly against making the constructor final, defeats a lot oft the purpose have a non-final collection at all.
The rest of the changes LGTM. It's interesting that the default validatePayload returns the value itself 🤷🏼
| * @return void | ||
| */ | ||
| public function __construct($items = []) | ||
| final public function __construct($items = []) |
There was a problem hiding this comment.
I'm against this change, it doesn't make sense and also no explanation.
If you want to fix the phpstan, I suggest to take a look at hasAllClaims() if there's a solution without new static possible.
There was a problem hiding this comment.
I refactored hasAllClaims and removed final from constructor.
- Removed `final` keyword from the constructor to allow subclassing. - Refactored `hasAllClaims` method to replace `new static($claims)->diff($this->keys())->isEmpty()` with a foreach loop.
mfn
left a comment
mfn
left a comment
There was a problem hiding this comment.
Nice work!
Please update the PR description, since the constructor has not been made final, thank you
|
Thanks, I have updated the description. |
3 participants
Description
This update resolves multiple PHPStan issues:
JWTGenerateSecretCommandby ensuringdisplayKey($key);is called before returning, avoiding returning a void method.return true;statements inExpiration.php,IssuedAt.php, andNotBefore.phpto comply with PHPStan expectations.hasAllClaimsmethod to avoid unsafe usage ofnew static().CHANGELOG.mdto reflect these fixes.Checklist:
CHANGELOG.md