Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Route::getName() return type #15880

Closed
jarnstedt opened this issue Jan 26, 2022 · 5 comments · Fixed by #15881
Closed

Route::getName() return type #15880

jarnstedt opened this issue Jan 26, 2022 · 5 comments · Fixed by #15881
Assignees
Labels
5.0 The issues we want to solve in the 5.0 release breaks bc Functionality that breaks Backwards Compatibility bug A bug report status: medium Medium

Comments

@jarnstedt
Copy link

Hi!
The \Phalcon\Mvc\Router\Route::getName() return type string seems to be incorrect.

$route = new \Phalcon\Mvc\Router\Route('example');
var_dump(\Phalcon\Version::get());
var_dump($route->getName());

outputs

string(5) "4.1.2"
NULL

https://github.com/phalcon/ide-stubs/blob/v4.1.0/src/Phalcon/Mvc/Router/Route.php#L202

@niden
Copy link
Sponsor Member

niden commented Jan 26, 2022

That is correct. You have not set the name of the route. The first parameter in the constructor is the pattern not the name

$route = new \Phalcon\Mvc\Router\Route('example');

var_dump($route->getName()); // null

$route->setName('my-route');
var_dump($route->getName()); // my-route

@dcolt
Copy link

dcolt commented Jan 27, 2022

@niden The feature might work as expected, but according to the stub file the return value can never be null.
It's not an actual issue in the implementation, just with the stub file giving an incorrect expectation of what values can be returned.
I.e using tools like PHPStan reads the stubs and will be giving incorrect results due to an invalid typehint.
Also take this code as an example:

$route = new \Phalcon\Mvc\Router\Route('example');

function doStuff(string $name) {
    echo $name;
}

doStuff($route->getName()); // Fatal error: Uncaught TypeError: Argument 1 passed to doStuff() must be of the type string, null given

$route->setName('my-route');
doStuff($route->getName()); // my-route

@niden
Copy link
Sponsor Member

niden commented Jan 27, 2022

@dcolt Ack sorry. I misunderstood. That is indeed a bug we need to address.

@niden
Copy link
Sponsor Member

niden commented Jan 27, 2022

Transferring this to the main repo for fixing

@niden niden transferred this issue from phalcon/ide-stubs Jan 27, 2022
@niden niden self-assigned this Jan 27, 2022
@niden niden added 5.0 The issues we want to solve in the 5.0 release bug A bug report status: medium Medium labels Jan 27, 2022
@niden niden added this to Working on it in Phalcon Roadmap Jan 27, 2022
@niden niden mentioned this issue Jan 27, 2022
5 tasks
@niden niden linked a pull request Jan 27, 2022 that will close this issue
5 tasks
@niden niden added the breaks bc Functionality that breaks Backwards Compatibility label Jan 27, 2022
@niden
Copy link
Sponsor Member

niden commented Jan 27, 2022

Resolved in #15881

Thank you @jarnstedt and @dcolt

@niden niden closed this as completed Jan 27, 2022
Phalcon Roadmap automation moved this from Working on it to Implemented Jan 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5.0 The issues we want to solve in the 5.0 release breaks bc Functionality that breaks Backwards Compatibility bug A bug report status: medium Medium
Projects
Archived in project
Phalcon Roadmap
  
Implemented
Development

Successfully merging a pull request may close this issue.

3 participants