-
Notifications
You must be signed in to change notification settings - Fork 11.5k
[11.x] Change phpdoc for typed config getters #50287
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
Conversation
Could be a closure which returns these types. |
@crynobone should the null type als be added as as a valid return type? Because currently the integer only can return integer. Otherwise I would make these changes with Closures, which only return a string/integer/... and not allow a null value. |
that's the correct value of |
Co-authored-by: Mior Muhammad Zaki <crynobone@gmail.com>
Co-authored-by: Mior Muhammad Zaki <crynobone@gmail.com>
Co-authored-by: Mior Muhammad Zaki <crynobone@gmail.com>
Co-authored-by: Mior Muhammad Zaki <crynobone@gmail.com>
Co-authored-by: Mior Muhammad Zaki <crynobone@gmail.com>
I believe the return type are correct. When using these typed getters you should except the value to be in a correct format. If the configuration is not, then it's because it's not correct. Thus expecting an exception is, for me, the right way. Config::integer('key'); The default value should also be null by default as it is right now, as it is an optional argument. If you want Config::integer('int-key', 0);
Config::string('str-key', ''); |
I saw after I expanded the lines. That is why i removed the comment. Thanks. EDIT. |
Follow up from #50140
You can only provide the correct typed parameter, otherwise it will always throw an
InvalidArgumentException
Since this is for Laravel 11, you could also add a php type instead of phpdoc.