Fix phpstan/phpstan#11339: filter_input with FILTER_FORCE_ARRAY has incorrect return type#5315
Closed
phpstan-bot wants to merge 5 commits intophpstan:2.1.xfrom
Closed
Fix phpstan/phpstan#11339: filter_input with FILTER_FORCE_ARRAY has incorrect return type#5315phpstan-bot wants to merge 5 commits intophpstan:2.1.xfrom
filter_input with FILTER_FORCE_ARRAY has incorrect return type#5315phpstan-bot wants to merge 5 commits intophpstan:2.1.xfrom
Conversation
…ncorrect return type - When FILTER_FORCE_ARRAY or FILTER_REQUIRE_ARRAY is used, input values can be arbitrarily nested arrays (e.g. from query strings like ?a[a][b][]=val), so the array value type must include array when input could be an array - Updated FilterFunctionReturnTypeHelper::getType() to union ArrayType into the value type when inputIsArray is not no - New regression test in tests/PHPStan/Analyser/nsrt/bug-11339.php - Updated existing test expectations in filterVar.php, filter-var.php, filter-input.php, and discussion-9134.php to reflect the corrected types
Contributor
VincentLanglet
left a comment
VincentLanglet
left a comment
There was a problem hiding this comment.
We can do a little better
For instance we should have something like
assertType('array<array<int|null>>|null', filter_var($arrayOfArrayOfInt, FILTER_VALIDATE_INT, ['flags' => FILTER_REQUIRE_ARRAY|FILTER_NULL_ON_FAILURE]));
assertType('array<int|null>|null', filter_var($arrayOfInt, FILTER_VALIDATE_INT, ['flags' => FILTER_REQUIRE_ARRAY|FILTER_NULL_ON_FAILURE]));
assertType('array<array<int|null>int|null>|null', filter_var($arrayOfIntOrArrayOfInt, FILTER_VALIDATE_INT, ['flags' => FILTER_REQUIRE_ARRAY|FILTER_NULL_ON_FAILURE]));
…rray inputs When the input array has a known value type (e.g. array<int>, array<array<int>>), use that type information instead of always adding generic array to the value type. - When value type is definitely not array (e.g. array<int>): don't add array - When value type is definitely array (e.g. array<array<int>>): recursively compute the precise nested filtered type - When value type is a union of scalar and array (e.g. array<int|array<int>>): extract scalar/array parts and handle each precisely - For mixed/unknown value types: fall back to array<mixed, mixed> as before Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Collaborator
Author
|
The |
| return $type; | ||
| } | ||
|
|
||
| private function addNestedArrayType(Type $type, ?Type $inputValueType, TrinaryLogic $inputIsArray, ?Type $filterType, ?Type $flagsType, MixedType $mixedType): Type |
Contributor
There was a problem hiding this comment.
This might be simpler to implement with the TypeTraverser::map method.
Collaborator
Author
There was a problem hiding this comment.
Both background tasks confirmed complete — tests (11,677 passed) and phpstan (0 errors). The commit addressing the review is already pushed.
Replace the manual addNestedArrayType method with TypeTraverser::map to recursively filter each component of the array value type. Extract filterScalarType method for cleaner separation of scalar filtering logic from array wrapping logic. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
VincentLanglet
left a comment
VincentLanglet
left a comment
There was a problem hiding this comment.
Similar strategy should be used for place you added
TypeCombinator::union($type, new ArrayType($mixedType, $mixedType));
Replace conservative `array<mixed, mixed>` with precise nested array type computation using TypeTraverser::map when input type carries array structure information (e.g., `int|array<int>`). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Collaborator
Author
|
Both background tasks from the earlier run also completed successfully. All clear. |
Contributor
VincentLanglet
left a comment
VincentLanglet
left a comment
There was a problem hiding this comment.
You're using TypeTraverser::map in two different way in this PR while it could be refactor/simplified into a single method reusable
Replaces the inline TypeTraverser::map and the separate computeNestedArrayType method with a single reusable filterTypeComponents method. This also simplifies the REQUIRE_ARRAY and FORCE_ARRAY code paths by using the unified method directly instead of filterScalarType + computeNestedArrayType + union. As a side effect, union-typed inputs now produce more precise types since each union member is filtered individually rather than the union being treated as a non-scalar whole. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Collaborator
Author
|
Both background tasks from the earlier run also completed successfully. All clear. |
Contributor
|
Gonna try a fresh start another day |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
filter_inputandfilter_varwithFILTER_FORCE_ARRAYorFILTER_REQUIRE_ARRAYflags can return arbitrarily nested arrays (e.g. from query strings like?a[a][b][]=val), but PHPStan was reporting the return type asarray<string|false>without accounting for nested array values. This fix addsarrayto the value type when the input could contain arrays.Changes
src/Type/Php/FilterFunctionReturnTypeHelper.php: In thegetType()method, when wrapping the result in an array forFILTER_REQUIRE_ARRAYorFILTER_FORCE_ARRAY, unionArrayType(MixedType, MixedType)into the value type when the input could be an array ($inputIsArrayis notno)tests/PHPStan/Analyser/nsrt/filterVar.phpfor all cases with mixed inputstests/PHPStan/Analyser/nsrt/filter-var.phpfor cases with mixed/array inputstests/PHPStan/Analyser/nsrt/filter-input.phpfor filter_input with FORCE_ARRAYtests/PHPStan/Analyser/nsrt/discussion-9134.phpRoot cause
The
FilterFunctionReturnTypeHelper::getType()method wraps the filtered scalar type in anArrayTypewhenFILTER_FORCE_ARRAYorFILTER_REQUIRE_ARRAYis set, but did not account for the fact that input values (from superglobals or mixed-type variables) can themselves be arrays. PHP's filter functions recursively apply the filter to nested arrays, producing nested array output. The fix addsarrayto the value type union when the input type could be an array.Test
Added
tests/PHPStan/Analyser/nsrt/bug-11339.phpwith assertions for:filter_inputwithFILTER_FORCE_ARRAYandFILTER_REQUIRE_ARRAYusingFILTER_DEFAULTfilter_inputwithFILTER_VALIDATE_INTand both array flagsfilter_varwith known scalar inputs (verifying no false positivearrayin value type)Fixes phpstan/phpstan#11339