You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
RouteCollection get method has wrong return type in PHPDoc #54507
RouteCollection get method has wrong return type in PHPDoc
resulting in warnings in static analytics and code sniffers and other tools
The method \Illuminate\Routing\RouteCollection::get
/**
* Get routes from the collection by method.
*
* @param string|null $method
* @return \Illuminate\Routing\Route[]
*/
public function get($method = null)
{
return is_null($method) ? $this->getRoutes() : Arr::get($this->routes, $method, []);
}
says it returns an array of Route but actually it can also return a route object
if you take a look at the implementation of Arr::get($this->routes, $method, []) it return mixed
it seems that all if conditions will return an array of Route
except for this one which can iterate to the deepest level of the array and return the Route object
if (static::accessible($array) && static::exists($array, $segment)) {
$array = $array[$segment];
}
this can give warnings in static analytics and code sniffers and other tools
please correct me if I'm wrong
so I guess that the return type should be \Illuminate\Routing\Route | \Illuminate\Routing\Route[][]
Steps To Reproduce
declare a simple route
Route::view('about, 'about') ->name('about');
somewhere in the code call $route = $this->get(GET./about);
if according to the PHPDoc you assume that the return value is an array of route and you try to access $route[0] it will throw an error Cannot use object of type Illuminate\Routing\Route as array
changed the title [-]RouteCollection have wrong return type in PHPDoc[/-][+]RouteCollection get method has wrong return type in PHPDoc[/+]on Feb 7, 2025
Yes the doc description and the parameter names are clear.
However, the function is still allowing us to search by dot notation.
If you look at the implementation of Arr::get($this->routes, $method, [])
You will see that it check for the dot notation and return the route as object
So i would say that doc description and parameter name does not match with what the actual implementation do.
It actually do return array of routes if you search by method and will return one route as object if you search by mehod.uri
But bear in mind, this class implements against an interface. Not just the PHP docs, but an actual interface/contract (Illuminate\Routing\RouteCollectionInterface).
So I would say the implementation needs to be fixed to comply with the interface expectations, and not the interface to be loosened to allow for an implementation oversight.
Also, there are other implementations (e.g. CompiledRouteCollection), and 3rd-party packages might be even providing their own.
You can propose a PR to fix the implementation. I guess instead of using Arr::get() we could use the null-coalescing operator instead.
In the meanwhile, just don't use the dot notation. It is not meant to be used by like that.
Activity
[-]RouteCollection have wrong return type in PHPDoc[/-][+]RouteCollection get method has wrong return type in PHPDoc[/+]rodrigopedra commentedon Feb 8, 2025
The doc block says:
And the parameter is called
$method.Why are you querying using dot notation?
Maybe we could filter out valid methods (
GET,POST, etc.) and error out if and invalid string, such asGET./about, is given.From the method's documentation, and expectations, there are no unexpected outcomes.
abdel-aouby commentedon Feb 8, 2025
Yes the doc description and the parameter names are clear.
However, the function is still allowing us to search by dot notation.
If you look at the implementation of
Arr::get($this->routes, $method, [])You will see that it check for the dot notation and return the route as object
So i would say that doc description and parameter name does not match with what the actual implementation do.
It actually do return array of routes if you search by method and will return one route as object if you search by mehod.uri
rodrigopedra commentedon Feb 8, 2025
I see what you mean.
But bear in mind, this class implements against an interface. Not just the PHP docs, but an actual interface/contract (
Illuminate\Routing\RouteCollectionInterface).So I would say the implementation needs to be fixed to comply with the interface expectations, and not the interface to be loosened to allow for an implementation oversight.
Also, there are other implementations (e.g.
CompiledRouteCollection), and 3rd-party packages might be even providing their own.You can propose a PR to fix the implementation. I guess instead of using
Arr::get()we could use the null-coalescing operator instead.In the meanwhile, just don't use the dot notation. It is not meant to be used by like that.
abdel-aouby commentedon Feb 8, 2025
Exactly, I agree with you.
The method is just doing too much than it promise from its name and parameter.
I will prepare a PR for tha matter
Thanks for the discussion.