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.

Sign up for GitHub

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

Jump to bottom

Specify return type for filter_input() #2010

Merged
merged 8 commits into from Apr 7, 2023
Merged

Specify return type for filter_input() #2010

merged 8 commits into from Apr 7, 2023

Conversation

herndlm
Copy link
Contributor

@herndlm herndlm commented Nov 18, 2022 •
edited

Closes phpstan/phpstan#6261

See https://www.php.net/manual/en/function.filter-input.php, the important difference to filter_var is the handling of the non-set variables in the superglobal arrays

Return Values
Value of the requested variable on success, false if the filter fails, or null if the var_name variable is not set. If the flag FILTER_NULL_ON_FAILURE is used, it returns false if the variable is not set and null if the filter fails.

@herndlm herndlm force-pushed the filter-input branch 5 times, most recently from 3c28408 to 41e927e Compare November 18, 2022 20:42
@herndlm herndlm marked this pull request as ready for review November 18, 2022 20:52
@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.

@herndlm herndlm marked this pull request as draft November 19, 2022 12:00
@herndlm herndlm force-pushed the filter-input branch 3 times, most recently from 22740d4 to ff19000 Compare November 20, 2022 19:48
@herndlm herndlm marked this pull request as ready for review November 20, 2022 19:57
@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.

@herndlm herndlm force-pushed the filter-input branch 2 times, most recently from 3ca69d1 to 2ec1ef0 Compare December 11, 2022 20:41
@herndlm
Copy link
Contributor Author

herndlm commented Dec 21, 2022

do you think we can get that this one in the next release? 😊 it's 90% refactor and would help me get rid of some errors in a legacy app which helps me finish the 7.4 -> 8.x upgrade. at least that's what I was telling myself when I was last procrastinating when working on it :D

@herndlm herndlm marked this pull request as draft December 27, 2022 14:45
@herndlm herndlm marked this pull request as ready for review January 20, 2023 16:23
@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.

@herndlm herndlm marked this pull request as draft January 30, 2023 17:25
@herndlm
Copy link
Contributor Author

herndlm commented Jan 31, 2023 •
edited

Fyi the return type in case the array offset does not exist and the force array flag is set is not handled properly. It's a bit tricky and I might need to refactor the helper maybe. Good that you didn't merge it yet 😅

@herndlm herndlm mentioned this pull request Feb 25, 2023
Merged
@herndlm herndlm changed the base branch from 1.9.x to 1.10.x February 25, 2023 21:09
@herndlm
Copy link
Contributor Author

herndlm commented Feb 25, 2023

I'm happy with this now, maybe the new method in the helper is useful for #2257 too.

@herndlm herndlm marked this pull request as ready for review February 25, 2023 21:21
@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.

@herndlm herndlm force-pushed the filter-input branch 2 times, most recently from 23874d7 to 7db3ed6 Compare February 27, 2023 08:04
@herndlm herndlm mentioned this pull request Mar 4, 2023
Merged
@herndlm herndlm mentioned this pull request Apr 3, 2023
Merged
ondrejmirtes
ondrejmirtes requested changes Apr 5, 2023
View reviewed changes
src/Type/Php/FilterInputDynamicReturnTypeExtension.php Outdated
$typeExpr = $functionCall->getArgs()[0]->value;
if (
!($typeExpr instanceof ConstFetch)
|| !in_array((string) $typeExpr->name, ['INPUT_GET', 'INPUT_POST', 'INPUT_COOKIE', 'INPUT_SERVER', 'INPUT_ENV'], true)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like that this condition only works for literal INPUT_GET ConstFetch in the code, but doesn't work for @param INPUT_GET|INPUT_POST $abc from PHPDocs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so rebasing does summon you :D

good point, I also dislike that. I'm not good with constants, do you have any better ideas you can think of? Do I need to resolve them somehow I suppose?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought this is already a solved problem, FilterFunctionReturnTypeHelper does this with hasFlag, $this->getConstant() etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah that would work I guess, I thought there's a better approach. I'll switch it to use those helpers then :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adapted. I could not re-use those methods but found another approach. not sure yet if there's a better one though

@ondrejmirtes ondrejmirtes merged commit 9c72fc1 into phpstan:1.10.x Apr 7, 2023
372 of 377 checks passed
@ondrejmirtes
Copy link
Member

Thank you!

@herndlm herndlm deleted the filter-input branch April 7, 2023 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ondrejmirtes ondrejmirtes ondrejmirtes requested changes

@staabm staabm staabm left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@herndlm @phpstan-bot @ondrejmirtes @staabm