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

Fix PHP8.2 str_split function returns empty arrays for empty strings #485

Closed

Conversation

HuongNV13
Copy link

In PHP 8.2, the str_split function will returns empty arrays for empty strings.
See: https://php.watch/versions/8.2/str_split-empty-string-empty-array

We can use mb_str_split() instead

@google-cla
Copy link

google-cla bot commented Feb 3, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@bshaffer
Copy link
Collaborator

bshaffer commented Feb 5, 2023

I'm not sure why exactly, but something is different in mb_str_split that is causing all our tests to fail

@HuongNV13 HuongNV13 force-pushed the php82-str-split-function-return branch from 7b9364a to d4600f8 Compare February 13, 2023 02:54
@HuongNV13
Copy link
Author

Hi @bshaffer,
The failed tests are related to the CLA.
I have signed the CLA and rebased the code.

@SoulPancake
Copy link

@HuongNV13 CLA has passed but the test suites are still failing as you can see, Can you check once again?

@bshaffer
Copy link
Collaborator

The two functions behave differently when dealing with a binary string, which is the case here:

str_split() will split into bytes, rather than characters when dealing with a multi-byte encoded string. Use mb_str_split() to split the string into code points.

If there's a specific error you're trying to prevent, perhaps you can do it by verifying the input beforehand.

@bshaffer bshaffer closed this May 23, 2023
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.

None yet

3 participants