feat: update phpstan 2.0 #11
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #11 +/- ##
=========================================
Coverage 92.20% 92.20%
Complexity 52 52
=========================================
Files 7 7
Lines 231 231
=========================================
Hits 213 213
Misses 18 18 ☔ View full report in Codecov by Sentry. |
| @@ -24,7 +24,7 @@ | |||
| final readonly class WebhookEventHandlerChain | |||
| { | |||
| /** | |||
| * @var list<WebhookHandlerInterface> | |||
| * @var array<WebhookHandlerInterface> | |||
There was a problem hiding this comment.
| * @var array<WebhookHandlerInterface> | |
| * @var list<WebhookHandlerInterface> |
Lets keep this and add it to the baseline instead.
There was a problem hiding this comment.
Actually i think WebhookHandlerInterface[] is already implying its a list.
There was a problem hiding this comment.
can you help me understand why we need to keep list and add it to baseline? Is this an issue with phpstan?
There was a problem hiding this comment.
It looks like a false positive to me
There was a problem hiding this comment.
but iterator_to_array could actually preserve the keys of the $handlers array, so list I think is inaccurate in this case.
We can solve this if we explicitly don't preserve the keys:
$this->handlers = iterator_to_array($handlers, false);What do you think?
There was a problem hiding this comment.
We know 100% that there is a list coming with numeric ascending keys. So list is still the right type here
There was a problem hiding this comment.
@silasjoisten ok so if we set preserve_keys to false it should be valid, and this would also make the annotation valid and phpstan happy
There was a problem hiding this comment.
Can you please also release a 0.3 afterwards? Many thanks
|
@OskarStark @silasjoisten can you please have a look at the new diff, I changed both the constructor body and the getHandlers to have list type |
|
Looks good to me, but can you use a bed argument for the false argument? Thanks |
No description provided.