thephpleague / route

Fast PSR-7 based routing and dispatch component including PSR-15 middleware, built on top of FastRoute.
http://route.thephpleague.com
MIT License
651 stars 126 forks source link

MiddlewareAwareInterface has invalid return type #209

Closed shadowhand closed 5 years ago

shadowhand commented 6 years ago

The shiftMiddleware() method cannot ensure that it will always return a middleware. If the stack is empty, array_shift of MiddlewareAwareTrait will return null.

The correct return type is MiddlewareInterface|null, as evidenced by Dispatcher. In fact, this code block cannot ever be executed, because PHP will throw an error when shiftMiddleware() returns null due to strict types.

I do not believe this can be fixed without a major version change, as it will change the return type of shiftMiddleware().

philipobenito commented 6 years ago

I think this can be considered minor if we remove the method from the interface. That's not an ideal solution though. If we can get the other 5.x features together I think lazy loaded middleware is enough to warrant a major so I'd go with that. Question is whether it is worth a fix in 4.x, it's basically a confusing error for something that would error anyway.

delboy1978uk commented 5 years ago

I found this too. The other option is to grab the array, and if the count is zero throw an exception and catch it where it was called instead? That way the interface won't change and the fix can go in this version.

delboy1978uk commented 5 years ago

Thanks @philipobenito :-D