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

Remove array_filter mixed handling which breaks type specification #1012

Conversation

herndlm
Copy link
Contributor

@herndlm herndlm commented Feb 10, 2022

This adapts ArrayFilterFunctionReturnTypeReturnTypeExtension to not handle a mixed argument anymore with a BenevolentUnionType.

This fixes truthy type specification with is_array && array_filter if the input is mixed, but I guess this is just a side-effect and there might be a better fix..
Context / use case: phpstan/phpstan-webmozart-assert#101

Without the modifications here I'd get

1) PHPStan\Analyser\TypeSpecifierTest::testCondition with data set #90 (PhpParser\Node\Expr\BinaryOp\BooleanAnd Object (...), array('array<string, mixed>', 'array<string, mixed>'), array())
if (is_array($foo) && array_filter($foo, 'is_string', ARRAY_FILTER_USE_KEY) === $foo)
Failed asserting that two arrays are identical.
--- Expected
+++ Actual
@@ @@
 Array &0 (
-    '$foo' => 'array<string, mixed>'
-    'array_filter($foo, 'is_string', ARRAY_FILTER_USE_KEY)' => 'array<string, mixed>'
+    '$foo' => 'array'
+    'array_filter($foo, 'is_string', ARRAY_FILTER_USE_KEY)' => '(array|null)'
 )

BUT I'm also aware that the changes here might be negatively affecting array_filter behaviour pre PHP 8, see https://3v4l.org/sOWbb. Is there a better / more pragmatical way to improve this? On the other hand, is PHPStan actively supporting PHP quirks with warnings and such?

The main problem / limitation that I have is apparently how TypeSpecifier handles the BooleanAnd expression.

Any other ideas how to improve that? It looks like NodeScopeResolver is handling this a bit different and not loosing the information so to say. Or I'm misunderstanding it somehow :)

@herndlm herndlm force-pushed the remove-array-filter-mixed-handling-which-breaks-type-specification branch from 42d58ca to e23d2ce Compare February 10, 2022 21:33
@herndlm
Copy link
Contributor Author

herndlm commented Feb 15, 2022

This is unfortunately not the right fix but merely a workaround. I'm currently working on a more generic improvement

@herndlm herndlm closed this Feb 15, 2022
@herndlm herndlm deleted the remove-array-filter-mixed-handling-which-breaks-type-specification branch February 15, 2022 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant