Incorrect behaviour of mb_strpos

[GrahamCampbell]

<?php

var_dump(mb_strpos(null, 'a'));

should print to bool(false) and raise no errors (https://3v4l.org/YXq4M), because the mb_ functions all treat null as the empty string, by design.

On PHP 8, the symfony polyfill triggers a type error.

[GrahamCampbell]

Related: https://bugs.php.net/bug.php?id=80649.

[nicolas-grekas]

Would you mind sending a PR to allow null everywhere on the PHP8 signatures?
See https://wiki.php.net/rfc/deprecate_null_to_scalar_internal_arg for background on this behavior.

[stof]

I suggest that we allow null, but we also prepare for this RFC coming to PHP 8.1, making our polyfills trigger a deprecation warning (using E_DEPRECATED, not the symfony deprecation contract) when passing null on PHP 8.1+ if this RFC gets accepted.

[GrahamCampbell]

Hmmm, simply adding |null is not enough. In strict mode, those functions would still be expected to throw a type error. We somehow need to check if the caller was strict. https://3v4l.org/9OMn9, https://3v4l.org/STGg8.

[stof]

We cannot know that.
Our polyfill can either be correct in strict mode or in coercing mode, but not both (that's one of the reason for this RFC for PHP 8.1: the current behavior of built-in function regarding null is impossible to polyfill in userland)

[GrahamCampbell]

We cannot know that.

We could probably work it out using debug_backtrace and looking for strict types at the top of the caller file.

[stof]

@GrahamCampbell I fear that inspecting the debug backtrace and doing file IO for each call to the polyfills would make them unusable from a performance PoV.

[GrahamCampbell]

Yeh. I guess this is a no-fix. The current implementation is probably best.

[stof]

Well, maybe we should fix that by allowing null, so that code written for coercing mode does not break when using the polyfill.

Code written for strict mode is unlikely to internally catch TypeError triggered by passing null to do a special case for that (such code will rather handle null outside the call instead, which will also be accepted by static analysis tools).

[GrahamCampbell]

I feel it's better to be slightly too strict than to be too weak in strict mode. People will ultimately need to fix their code for it to work on PHP 8.1 anyway.

[stof]

@GrahamCampbell but here, we make their code break without any deprecation warning, making our polyfill match a PHP 9.0 behavior when running on 7.1. that's not good.
I'd rather be correct in coercing mode (note that Symfony itself runs all its code in coercing mode). That won't cause issue for correct code running in strict mode. It will only cause them to receive a deprecation warning instead of a type error for incorrect usage (and static analysis will still catch most of it, as projects using strict mode are also often projects using static analysis).

[nicolas-grekas]

I agree with @stof
Help wanted!

[nicolas-grekas]

Will be fixed by #330