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

Update PHP-Scoper to v0.16.2 #10407

Closed
swissspidy opened this issue Jan 31, 2022 · 11 comments · Fixed by #10511
Closed

Update PHP-Scoper to v0.16.2 #10407

swissspidy opened this issue Jan 31, 2022 · 11 comments · Fixed by #10511
Assignees
Labels
P2 Should do soon Type: Infrastructure Changes impacting testing infrastructure or build tooling

Comments

@swissspidy
Copy link
Collaborator

swissspidy commented Jan 31, 2022

Task Description

The current version of PHP-Scoper is 0.16.2.

In version 0.16.0, there were lots of improvements to the config, making it a lot simpler for us to configure PHP-Scoper in a WP plugin context.

It means we can get rid of all the manual str_replace calls in the scoper config thanks to expose-global-{constants, classes, functions} and expose-{namespaces, constants, classes, functions}

We could even automate this by setting the config based on the WP stubs, see humbug/php-scoper#303 (comment)

Though we could split that up in its own ticket.

Implementation Brief

  • Install latest version of PHP-Scoper via Composer
  • Remove all the str_replace calls in the PHP-Scoper config
  • Install sniccwp/php-scoper-wordpress-excludes as a dev dependency and configure the exclude-classes, exclude-functions, and exclude-constants options accordingly
@swissspidy swissspidy added P2 Should do soon Type: Infrastructure Changes impacting testing infrastructure or build tooling Pod: WP & Infra labels Jan 31, 2022
@spacedmonkey
Copy link
Contributor

So is the recommendation here to use these packages?

Then removing this ?

'patchers' => [
function ( $file_path, $prefix, $contents ) {
/*
* There is currently no easy way to simply whitelist all global WordPress functions.
*
* This list here is a manual attempt after scanning through the AMP plugin, which means
* it needs to be maintained and kept in sync with any changes to the dependency.
*
* As long as there's no built-in solution in PHP-Scoper for this, an alternative could be
* to generate a list based on php-stubs/wordpress-stubs. devowlio/wp-react-starter/ seems
* to be doing just this successfully.
*
* @see https://github.com/humbug/php-scoper/issues/303
* @see https://github.com/php-stubs/wordpress-stubs
* @see https://github.com/devowlio/wp-react-starter/
*/
$contents = str_replace( "\\$prefix\\_doing_it_wrong", '\\_doing_it_wrong', $contents );
$contents = str_replace( "\\$prefix\\__", '\\__', $contents );
$contents = str_replace( "\\$prefix\\esc_html_e", '\\esc_html_e', $contents );
$contents = str_replace( "\\$prefix\\esc_html", '\\esc_html', $contents );
$contents = str_replace( "\\$prefix\\esc_attr", '\\esc_attr', $contents );
$contents = str_replace( "\\$prefix\\esc_url", '\\esc_url', $contents );
$contents = str_replace( "\\$prefix\\site_url", '\\site_url', $contents );
$contents = str_replace( "\\$prefix\\wp_guess_url", '\\wp_guess_url', $contents );
$contents = str_replace( "\\$prefix\\untrailingslashit", '\\untrailingslashit', $contents );
$contents = str_replace( "\\$prefix\\WP_CONTENT_URL", '\\WP_CONTENT_URL', $contents );
$contents = str_replace( "\\$prefix\\wp_list_pluck", '\\wp_list_pluck', $contents );
$contents = str_replace( "\\$prefix\\is_customize_preview", '\\is_customize_preview', $contents );
$contents = str_replace( "\\$prefix\\do_action", '\\do_action', $contents );
$contents = str_replace( "\\$prefix\\trailingslashit", '\\trailingslashit', $contents );
$contents = str_replace( "\\$prefix\\get_template_directory_uri", '\\get_template_directory_uri', $contents );
$contents = str_replace( "\\$prefix\\get_stylesheet_directory_uri", '\\get_stylesheet_directory_uri', $contents );
$contents = str_replace( "\\$prefix\\includes_url", '\\includes_url', $contents );
$contents = str_replace( "\\$prefix\\wp_styles", '\\wp_styles', $contents );
$contents = str_replace( "\\$prefix\\get_stylesheet", '\\get_stylesheet', $contents );
$contents = str_replace( "\\$prefix\\get_template", '\\get_template', $contents );
$contents = str_replace( "\\$prefix\\wp_parse_url", '\\wp_parse_url', $contents );
$contents = str_replace( "\\$prefix\\is_wp_error", '\\is_wp_error', $contents );
$contents = str_replace( "\\$prefix\\content_url", '\\content_url', $contents );
$contents = str_replace( "\\$prefix\\get_admin_url", '\\get_admin_url', $contents );
$contents = str_replace( "\\$prefix\\WP_CONTENT_DIR", '\\WP_CONTENT_DIR', $contents );
$contents = str_replace( "\\$prefix\\AMP__FILE__", '\\AMP__FILE__', $contents );
$contents = str_replace( "\\$prefix\\AMP__DIR__", '\\AMP__DIR__', $contents );
$contents = str_replace( "\\$prefix\\AMP__VERSION", '\\AMP__VERSION', $contents );
$contents = str_replace( "\\$prefix\\ABSPATH", '\\ABSPATH', $contents );
$contents = str_replace( "\\$prefix\\WPINC", '\\WPINC', $contents );
$contents = str_replace( "\\$prefix\\MINUTE_IN_SECONDS", '\\MINUTE_IN_SECONDS', $contents );
$contents = str_replace( "\\$prefix\\HOUR_IN_SECONDS", '\\HOUR_IN_SECONDS', $contents );
$contents = str_replace( "\\$prefix\\DAY_IN_SECONDS", '\\DAY_IN_SECONDS', $contents );
$contents = str_replace( "\\$prefix\\MONTH_IN_SECONDS", '\\MONTH_IN_SECONDS', $contents );
$contents = str_replace( "\\$prefix\\WP_DEBUG_DISPLAY", '\\WP_DEBUG_DISPLAY', $contents );
$contents = str_replace( "\\$prefix\\home_url", '\\home_url', $contents );
$contents = str_replace( "\\$prefix\\wp_array_slice_assoc", '\\wp_array_slice_assoc', $contents );
$contents = str_replace( "\\$prefix\\wp_json_encode", '\\wp_json_encode', $contents );
$contents = str_replace( "\\$prefix\\get_transient", '\\get_transient', $contents );
$contents = str_replace( "\\$prefix\\wp_cache_get", '\\wp_cache_get', $contents );
$contents = str_replace( "\\$prefix\\set_transient", '\\set_transient', $contents );
$contents = str_replace( "\\$prefix\\wp_cache_set", '\\wp_cache_set', $contents );
$contents = str_replace( "\\$prefix\\wp_using_ext_object_cache", '\\wp_using_ext_object_cache', $contents );
$contents = str_replace( "\\$prefix\\_doing_it_wrong", '\\_doing_it_wrong', $contents );
$contents = str_replace( "\\$prefix\\plugin_dir_url", '\\plugin_dir_url', $contents );
$contents = str_replace( "\\$prefix\\is_admin_bar_showing", '\\is_admin_bar_showing', $contents );
$contents = str_replace( "\\$prefix\\get_bloginfo", '\\get_bloginfo', $contents );
$contents = str_replace( "\\$prefix\\add_filter", '\\add_filter', $contents );
$contents = str_replace( "\\$prefix\\remove_filter", '\\remove_filter', $contents );
$contents = str_replace( "\\$prefix\\apply_filters", '\\apply_filters', $contents );
$contents = str_replace( "\\$prefix\\add_query_arg", '\\add_query_arg', $contents );
$contents = str_replace( "\\$prefix\\remove_query_arg", '\\remove_query_arg', $contents );
$contents = str_replace( "\\$prefix\\get_post", '\\get_post', $contents );
$contents = str_replace( "\\$prefix\\wp_scripts", '\\wp_scripts', $contents );
$contents = str_replace( "\\$prefix\\wp_style_is", '\\wp_style_is', $contents );
$contents = str_replace( "\\$prefix\\WPMU_PLUGIN_DIR", '\\WPMU_PLUGIN_DIR', $contents );
$contents = str_replace( "\\$prefix\\WP_PLUGIN_DIR", '\\WP_PLUGIN_DIR', $contents );
$contents = str_replace( "\\$prefix\\WP_PLUGIN_URL", '\\WP_PLUGIN_URL', $contents );
$contents = str_replace( "\\$prefix\\WPMU_PLUGIN_URL", '\\WPMU_PLUGIN_URL', $contents );
$contents = str_replace( "\\$prefix\\wp_array_slice_assoc", '\\wp_array_slice_assoc', $contents );
$contents = str_replace( "\\$prefix\\wp_json_encode", '\\wp_json_encode', $contents );
$contents = str_replace( "$prefix\\WP_Http", 'WP_Http', $contents );
$contents = str_replace( "$prefix\\WP_Error", 'WP_Error', $contents );
return $contents;
},
],

@swissspidy
Copy link
Collaborator Author

First of all we should remove all our str_replace() calls and use the new config options exclude-functions, exclude-constants, et al.

That should be trivial. Just an array of functions and constants and class names.

Afterwards, most likely in its own ticket, we can look into automating it somehow. It needs more thinking on how best to solve that part.

I would like to avoid using these packages you mentioned. They are not really well maintained and not up-to-date, just proof-of-concepts. However, the underlying https://github.com/sniccowp/php-scoper-excludes utility shows how it could be done.

@calvinalkan
Copy link

calvinalkan commented Feb 3, 2022

@swissspidy Hey there, I'm the creator of these packages.

We'll maintain them for a good time to come, php-scoper is very vital to a big framework we are going to release for WordPress.

I'd like to automate the release of the stubs, but I don't know quite now what the best approach would be since it requires php-stubs/wordpress-stubs.
I thought about a Github Action with a corn schedule. If you have any suggestions I'm all ears. I think this is something that desperately needs solving for the WP plugin ecosystem.

@swissspidy
Copy link
Collaborator Author

@calvinalkan hey there, thanks for chiming in! Since it requires php-stubs/wordpress-stubs and everyone might have a different version of that package, I was wondering whether the exclusion rules generation could be done as part of a composer plugin or something. Perhaps it could even be done on-the-fly in the PHP-Scoper config file itself. After all the scoping only happens on composer install anyway (at least in our case), so that's not gonna be a huge issue.

A GiHub actions sounds interesting too, but then I'd probably set one up for ourselves in this repo, so that we don't have to require an extra composer package for it.

@calvinalkan
Copy link

calvinalkan commented Feb 3, 2022

Okay so there two use cases.

  1. Directly require --dev sniccowp/php-scoper-wordpress-excludes

with whatever version tag you need for your project. IMO this should always be the latest stable tag of WordPress itself that's why Id like to automate this to the maximum degree possible so that your plugin can require it as

"require-dev": {
  "sniccowp/php-scoper-wordpress-excludes" : "^5.0|^6.0"
}

And then in your scoper.inc.php you just require:

'exclude_classes' => array_merge(
  require $path_to_vendor.'/sniccowp/php-scoper-excludes/generated/wordpress-classes.php',
  ... other classes
)

Now, in your build step, you always exclude against the latest version of WP.

  1. If you need to generate custom excludes for something different (maybe a third-party plugin that you integrate with) than the wp, wp-cli, and woocommerce packages that already exist, you should directly require the cli-app as a dev-dependency (or composer plugin) and generate your exclusion list once and put them into VCS. You could also build the exclusion list in CI but I don't know if that adds any benefits.

@calvinalkan
Copy link

See here how the exclusion lists for WordPress itself are generated.

But yeah, for the core/wp-cli excludes I don't see why you wouldn't just require the list of generated files as a dev dependency. The package is basically just 5 files.

@swissspidy
Copy link
Collaborator Author

If you're gonna automate maintenance of your sniccowp/php-scoper-wordpress-excludes package like you mentioned, that'd be great. Otherwise I was merely wondering whether we couldn't just inline all of this work in this repository so that we don't need this extra dependency.

@calvinalkan
Copy link

calvinalkan commented Feb 3, 2022

My initial idea was:

  • A GitHub action that runs on a daily cron
  • fetch latest tag of php-stubs/wordpress-stubs
  • compare with a current_version.txt in the repo
  • update if needed
  • tag new release
  • update latest_version.txt

Do you have any other idea?

@swissspidy
Copy link
Collaborator Author

You wouldn't need current_version.txt if you just look at all the existing Git tags on your end.

@calvinalkan
Copy link

@swissspidy Implemented now. We run a daily check against php-stubs/wordpress stubs and tag a new release automatically. Available at packagist now aswell.

We changed from .php to .json because with .php git would go mad on the numerically indexed array. It would show 6000 additions and deletions basically. With JSON it's much clearer now what got updated and what got removed.

@swissspidy
Copy link
Collaborator Author

Awesome! That definitely makes me more confident that we can use this in this project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 Should do soon Type: Infrastructure Changes impacting testing infrastructure or build tooling
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants