Correct return type to match functionality #54148
Conversation
| @@ -7,7 +7,7 @@ interface HasMiddleware | |||
| /** | |||
| * Get the middleware that should be assigned to the controller. | |||
| * | |||
| * @return \Illuminate\Routing\Controllers\Middleware[] | |||
| * @return array<int,\Illuminate\Routing\Controllers\Middleware|\Closure|string> | |||
There was a problem hiding this comment.
Wouldn't this have sufficed?
| * @return array<int,\Illuminate\Routing\Controllers\Middleware|\Closure|string> | |
| * @return (\Illuminate\Routing\Controllers\Middleware|\Closure|string)[] |
There was a problem hiding this comment.
Also, is string the closest we can get? Have you tried callable?
| * @return array<int,\Illuminate\Routing\Controllers\Middleware|\Closure|string> | |
| * @return array<int,\Illuminate\Routing\Controllers\Middleware|callable> |
There was a problem hiding this comment.
using (...)[] seems to work just as well but I can't say I have seen that syntax before and tend to defer to the error output from phpstan which currently uses array<...>
it does seem I can remove \Closure|string and replace with callable so that is a nice improvement
I have also seen list<...> is valid now so technically @return list<\Illuminate\Routing\Controllers\Middleware|callable> is an option in this case
I'll wait for github to support polls and or others to vote on the best combination and way forward 😅
There was a problem hiding this comment.
That would actually be great for such cases 😂👍🏻
|
Thanks! |
As discussed in #50766 (comment), the revised return type is still incorrect as it needs to include both string and closure in order to match all the functionality available.
I was fortunate enough to be upgrading a project where I ended up using all three types so I can confirm that phpstan is now happy with this particular combination.
The docs don't reference another type currently so this should be sufficient.