Fix errors for phpstan level 6 #684
Conversation
|
I have approved the workflow actions; which have raised several issues. Also it seems your proposed fix makes the tests fail, so I am not sure if that works as you intended. |
|
I'll try to take a look later in the day. I run the tests locally and there was one failing, but it was also failing on |
|
I think they're failing now because the config has been raised to 6 in this PR. The config applies to both Lines 5 to 7 in 5edef22 |
|
I stand corrected. It was failing on level 5 for |
| @@ -87,6 +99,12 @@ protected function isOneOfNamesInList(array $names, array $list): bool | |||
| return false; | |||
| } | |||
|
|
|||
| /** | |||
| * | |||
| * @param string $name | |||
There was a problem hiding this comment.
Some @param and @return annotations are redundant with method signature; is it really required by phpstan?
There was a problem hiding this comment.
I don't believe so.
These was a discussion about the superfluous phpdoc rule; but so far consensus was to keep the rule in place.
There was a problem hiding this comment.
It is also more consistent for the time being.
And to be honest:
Removing some and adding others is usually counterproductive in readability, it really confuses people as to the positioning of those lines then if they ever need to be added.
|
That merge commit was not what I intended to do. Let me see if I can resolve this from CLI. |
|
See #731 |
Hi, I tried to fix all errors from phpstan on level 6. Not sure if you have a specific wish for the format for iterable types (
array<int,string>vsstring[]) but for the rest it should be fine.Actually, I think I've found a bug on the implementation of strip-hosts, but please double check that part before merging just in case I misunderstood how it's supposed to work.
It should fix #658