box icon indicating copy to clipboard operation
box copied to clipboard

Decision logic on which polyfills are added to vendor/autoload.php?

Open ondrejmirtes opened this issue 4 years ago • 6 comments

Hi, when compiling PHPStan this is how the resulting vendor/autoload.php looks like:

<?php

// @generated by Humbug Box

// autoload.php @generated by Composer

require_once __DIR__ . '/composer/autoload_real.php';

$loader = ComposerAutoloaderInite6b4a7db1c3ec772ea250147bc81c6b7::getLoader();

// Aliases for the whitelisted classes. For more information see:
// https://github.com/humbug/php-scoper/blob/master/README.md#class-whitelisting
if (!class_exists('Throwable', false)) {
    class_exists('_HumbugBoxf40854076254\Throwable');
}
if (!class_exists('JsonException', false)) {
    class_exists('_HumbugBoxf40854076254\JsonException');
}
if (!class_exists('Stringable', false)) {
    class_exists('_HumbugBoxf40854076254\Stringable');
}
if (!class_exists('ValueError', false)) {
    class_exists('_HumbugBoxf40854076254\ValueError');
}

// Functions whitelisting. For more information see:
// https://github.com/humbug/php-scoper/blob/master/README.md#functions-whitelisting
if (!function_exists('uv_signal_init')) {
    function uv_signal_init() {
        return \_HumbugBoxf40854076254\uv_signal_init(...func_get_args());
    }
}
if (!function_exists('uv_signal_start')) {
    function uv_signal_start() {
        return \_HumbugBoxf40854076254\uv_signal_start(...func_get_args());
    }
}
if (!function_exists('uv_poll_init_socket')) {
    function uv_poll_init_socket() {
        return \_HumbugBoxf40854076254\uv_poll_init_socket(...func_get_args());
    }
}
if (!function_exists('fdiv')) {
    function fdiv() {
        return \_HumbugBoxf40854076254\fdiv(...func_get_args());
    }
}
if (!function_exists('preg_last_error_msg')) {
    function preg_last_error_msg() {
        return \_HumbugBoxf40854076254\preg_last_error_msg(...func_get_args());
    }
}
if (!function_exists('str_contains')) {
    function str_contains() {
        return \_HumbugBoxf40854076254\str_contains(...func_get_args());
    }
}
if (!function_exists('str_starts_with')) {
    function str_starts_with() {
        return \_HumbugBoxf40854076254\str_starts_with(...func_get_args());
    }
}
if (!function_exists('str_ends_with')) {
    function str_ends_with() {
        return \_HumbugBoxf40854076254\str_ends_with(...func_get_args());
    }
}
if (!function_exists('get_debug_type')) {
    function get_debug_type() {
        return \_HumbugBoxf40854076254\get_debug_type(...func_get_args());
    }
}
if (!function_exists('get_resource_id')) {
    function get_resource_id() {
        return \_HumbugBoxf40854076254\get_resource_id(...func_get_args());
    }
}
if (!function_exists('setproctitle')) {
    function setproctitle() {
        return \_HumbugBoxf40854076254\setproctitle(...func_get_args());
    }
}

return $loader;

The project also has symfony/polyfill-php73 but those functions like is_countable and hrtime aren't added to the autoload.php file.

What decision logic box and php-scoper have for this? Looks like they take into account the PHAR is compiled on PHP 7.3 but that seems a bit counter-intuitive as I target PHP 7.1+.

And actually, my ideal situation would be:

  • Do not add any of these functions to vendor/autoload.php as they mess up PHPStan's analysis itself (see https://github.com/phpstan/phpstan/issues/3431#issuecomment-643086151)
  • If any src/ and vendor/ code uses these functions, prefix their usage as they are prefixed in their source code.

Can this be achieved? WDYT?

ondrejmirtes avatar Jun 14 '20 08:06 ondrejmirtes

Looks like at least part of the problem can be solved with 'whitelist-global-functions' => false and 'whitelist-global-classes' => false but that leaves calls to is_countable inside Symfony code unprefixed. PHP-Scoper might be confused about whether is_countable is internal to PHP or not, but that might just be a different manifestation of the "compiled on PHP 7.3, but can run on PHP 7.1" problem.

ondrejmirtes avatar Jun 14 '20 09:06 ondrejmirtes

PHP-Scoper checks if the function is internal or not and if not will mark it to append it to the autoload. So it depends on:

  • Which PHP version you are on
  • What version of PHPStorm stubs are used
  • What is fit in PHP-Scoper's Reflector

Being able to tell what versions has which symbol been added on would be really nice and solve your issue, but I suspect it would be quite hard to achieve in a reliable way

theofidry avatar Jun 15 '20 07:06 theofidry

What version of PHPStorm stubs are used

What's this one? How can I force it?

phpstorm-stubs use the @since annotation: https://github.com/JetBrains/phpstorm-stubs/blob/eff2ef85adf6be7aea7f84882f2df174208a4cf5/standard/standard_5.php#L317

So if Scoper filtered the symbols by this annotation and you could tell it you intend to use it on PHP 7.1, it could do something with that.

I do this in the scope of BetterReflection library already: https://github.com/ondrejmirtes/BetterReflection/commit/8cc773644db20b22c9db9f13ea58797b80b7b74b#diff-acc4fc64b69196c0bfd65b95b4130601

The current solution with 'whitelist-global-functions' => false is dangerous because the called functions do not get prefixed even if the polyfill is available (and prefixed). So I kind of rely on those paths with newer PHP functions not to be executed in vendor...

ondrejmirtes avatar Jun 15 '20 12:06 ondrejmirtes

What's this one? How can I force it?

This is from the stub dependency installed via Composer hence the one shipped in the PHAR. There is no hard constraint there in terms of Composer version so it's more about when I update it and update the PHAR.

So if Scoper filtered the symbols by this annotation and you could tell it you intend to use it on PHP 7.1, it could do something with that.

There is two reasons I'm not keen (but it might be necessary) to go down that path:

  • It's far from being a trivial amount of work – also keep in mind I'm using the stubs generated map only, so no parsing of the stubs is being done in PHP-Scoper so either it will result in a much slower solution or the stubs should be adapted to provide that since version in a more convenient way
  • I don't know how reliable they are

The current solution with 'whitelist-global-functions' => false is dangerous because the called functions do not get prefixed even if the polyfill is available (and prefixed). So I kind of rely on those paths with newer PHP functions not to be executed in vendor...

Yes I think the global function whitelist should be enabled.

But since PHP-Scoper does rely on the autoloaded code to perform its work, the PHP version you currently execute has an influence, hence ideally you want to match that PHP version with the version you target.

theofidry avatar Jun 15 '20 13:06 theofidry

I recently moved the PHAR compilation to PHP 7.2 but I haven't seen any changes to is_countable call. What should happen in your opinion?

ondrejmirtes avatar Jun 15 '20 14:06 ondrejmirtes

I recently moved the PHAR compilation to PHP 7.2 but I haven't seen any changes to is_countable call. What should happen in your opinion?

I think if it's in the stubs it will be considered as internal... Damnit

theofidry avatar Jun 15 '20 14:06 theofidry

Reviewing this.

On Box side, or to be more accurate the RequirementChecker side, it will list all the required extensions based on the composer.lock and filter out the ones covered by polyfills (which are documented via the composer replace setting).

But I assume this issue is more about what is added in vendor/autoload.php by PHP-Scoper. In which case this depends on two things:

  • Whether global symbols are exposed
  • Whether a global function alias was registered. More details can be found here in the documented exception (and why).

TL:DR; it is to avoid to have broken cases and allow polyfills to work out of the box, although it does mean a bit of a polluted vendor/autoload.php (which I think is a fair tradeoff unless someone comes up with an amazing idea/PoC).

theofidry avatar Nov 27 '23 09:11 theofidry