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
str_contains behaviour differs from php8 behaviour #282
Comments
@PatrickRose Good catch! |
Ah, of course. Forgot about that behaviour! Looks like it's the same for I think the method definition should be Shall I get a PR together? |
Absolutely no pressure btw, but what's the release schedule for polyfills? We spotted this because we got hit by it, so it'd be great if the fix could be released quickly (but we can easily work around it in our code) |
Well, for now, nobody submitted a PR with a fix. So there is nothing to release yet. Releases are done when needed. There is no fixed schedule. |
Thanks @stof - I'll get a PR together now. |
The previous version of the polyfills didn't accept null as a value as either the haystack or the needle because of the string typehint. In PHP8, these are silently cast to an empty string. This matches that behaviour, by making the typehints nullable. Fixes symfony#282
The previous version of the polyfills didn't accept null as a value as either the haystack or the needle because of the string typehint. In PHP8, these are silently cast to an empty string. This matches that behaviour, by making the typehints nullable. Fixes symfony#282
Isn't this issue a PHP8 one? Shouldn't PHP8 also reject null, since the types are not nullable? Can you please open a bug report to bugs.php.net? Simple reproducer: function my_str_starts_with(string $haystack, string $needle): bool
{
return true;
}
var_dump(str_starts_with(null, 'abc'));
var_dump(my_str_starts_with(null, 'abc')); the second call fails but the first succeeds, unless strict more is enabled. |
@stof also raised this in the linked PR. I'm not too familiar with how it works on an internal level, but my guess is that weak mode just casts the args to the relevant type in a way that can't be replicated in userland. I'm happy to open a bug but it feels like it could possibly be a major BC break since that would need to be applied to all internal functions? |
I think it's worth discussing with php-internals yes, please open a bug report there. Userland should be able to polyfill to me... /cc @nikic in case you have some bandwidth to have a look here. |
Yes, this is an open bug in the way scalar types are treated by internal functions. I expect it will get deprecated in PHP 8.1, once I pick up php/php-src#4227 again. For the purposes of the polyfill, I would recommend using a normal |
It's been closed as a won't fix on the bug tracker and since we have to choose between making it work the same for |
Thanks for reporting and investigating! |
It looks like the PHP8 behaviour for
str_contains
actually silently accepts a null value (casting it to the empty string), whereas the polyfill doesn't. See this comparison on 3v4l, where you can see that the polyfill throws a TypeError but PHP8 doesn't. The same is probably true for the other str methods, but I haven't confirmed it yet.Happy to send a PR (possibly just easiest to remove the types from the params?)
The text was updated successfully, but these errors were encountered: