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

mb_str_split result can be an empty array #7580

Closed
mvorisek opened this issue Jul 5, 2022 · 7 comments · Fixed by phpstan/phpstan-src#1496
Closed

mb_str_split result can be an empty array #7580

mvorisek opened this issue Jul 5, 2022 · 7 comments · Fixed by phpstan/phpstan-src#1496

Comments

@mvorisek
Copy link
Contributor

mvorisek commented Jul 5, 2022

Bug report

Code snippet that reproduces the problem

demo: https://phpstan.org/r/b78dcbd0-6791-4090-b50b-599fa5593ac2

Expected output

no error

@herndlm
Copy link
Contributor

herndlm commented Jul 5, 2022

Maybe your snippet is not a good example, but I think in this case PHPStan actually knows that the array is always empty if you split '' and the error is correct, right?

@mvorisek
Copy link
Contributor Author

mvorisek commented Jul 5, 2022

You are right, I updated the example. The issue is present when non-empty-string type is passed.

@herndlm
Copy link
Contributor

herndlm commented Jul 5, 2022

Ok, that looks better, I guess the ''|'x' is not handled correctly via mb_str_split in https://github.com/phpstan/phpstan-src/blob/1.8.0/src/Type/Php/StrSplitFunctionReturnTypeExtension.php. Maybe L89 and the stuff below should somehow use TypeTraverser::map instead or something like that, but I did not take a close look. Do you want to @mvorisek? :)

@herndlm
Copy link
Contributor

herndlm commented Jul 6, 2022

I continued looking at this and apparently there are 2 issues here.
The first one being that the extension can't handle compound types like Union basically. I have a fix for that ready.
I don't fully understand the second one though. Apparently mb_str_split can return an empty array while str_split won't. Any ideas why? E.g. https://3v4l.org/r6sMe

Maybe I'll open a PR already for the first thing while figuring the second out hmm. I just could adapt all default types for mb_str_split, but I'd like to understand it first.

@mvorisek
Copy link
Contributor Author

mvorisek commented Jul 6, 2022

Apparently mb_str_split can return an empty array while str_split won't. Any ideas why? E.g. https://3v4l.org/r6sMe

This is how PHP works since the mb_str_split function has been introduced in PHP 7.4. Yes, it is confusing, and I opened an issue in php-src for it. But for now, mb_str_split can return an empty array for '' while str_split returns at least one element.

@herndlm
Copy link
Contributor

herndlm commented Jul 6, 2022

Ok thx, I saw in an old PR the info that this was changed in psalm as well to a non-empty-array but now I found vimeo/psalm#6493. That's enough for me to revert it back here too :)

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants