-
-
Notifications
You must be signed in to change notification settings - Fork 75
Crash on polyfilled function declaration #963
Description
Findings from investigating the failures of https://github.com/phpstan/phpstan-src/actions/runs/8545970407/job/23415521083?pr=2905.
If a package declares a polyfill for a function like so:
<?php
if (!function_exists('mb_str_split') {
function mb_str_split(...) {...};
}It was scoped as follows:
<?php
namespace Prefix;
if (!function_exists('Prefix\mb_str_split') {
function mb_str_split(...) {...};
}This was causing issues in the past as the function mb_str_split is no longer polyfilled correctly. There has been a few back and forth but this statement would now be changed to:
<?php
namespace Prefix;
if (!function_exists('mb_str_split')) { // no longer touches internal symbols; does rely on having an up
// to date list but it is also configurable if this is a problem
function mb_str_split(...) {...};
}
// in the scoper-autoload.php
if (!function_exists('mb_str_split')) {
function ctype_alnum() { return \Prefix\mb_str_split(...func_get_args()); }
}This solves all cases found so far. The issue shown in PHPStan is caused by the following problem: the polyfill above is declared twice (from different polyfills).
So in theory the fix should be something along the lines of:
<?php
namespace Prefix;
if (!function_exists('mb_str_split') && !function_exists('Prefix\mb_str_split')) {
function mb_str_split(...) {...};
}
// in the scoper-autoload.php
if (!function_exists('mb_str_split')) {
function ctype_alnum() { return \Prefix\mb_str_split(...func_get_args()); }
}Note the extra function exists checks. Hopefully this should have no side effects. The only case I can think of is that the function is already declared so the new duplicate one cannot be declared but this would already be a problem in non-scoped code so I do not think we need to care about it in PHP-Scoper.