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

Correct ServerRequest::getParam method PHPDoc #135

Conversation

hhovakimyan
Copy link
Contributor

The PHPDoc type of ServerRequest class getParam method's $default argument
is changed from "string" to "mixed", as dependent on request parameter its
default value can be expected to have another type(e.g. boolean). And if
there is a PHPStan or other analyser running in project an error is reported on
cases, where $default argument type is not string. So as there is no obstacle for
doing this change, it is correct to change PHPDoc type of the argument.

The PHPDoc type of ServerRequest class getParam method's $default argument
is changed from "string" to "mixed", as dependent on request parameter its
default value can be expected to have another type(e.g. boolean). And if
there is a PHPStan or other analyser running in project an error is reported on
cases, where $default argument type is not string.  So as there is no obstacle for
doing this change, it is correct to change PHPDoc type of the argument.
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 3b90321 on hhovakimyan:change-ServerRequest-getParam-method-default-arg-phpdoc into 90506ff on slimphp:master.

@hhovakimyan
Copy link
Contributor Author

@l0gicgate can you please merge this PR as soon as possible?

@l0gicgate l0gicgate added this to the 1.1 milestone Aug 18, 2020
@l0gicgate l0gicgate merged commit ad2abe4 into slimphp:master Aug 18, 2020
@hhovakimyan hhovakimyan deleted the change-ServerRequest-getParam-method-default-arg-phpdoc branch August 18, 2020 06:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants