1269 new hooks#1327
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Introduces four new actions around the runtime environment setup and cleanup, allowing integrations to react to the lifecycle without monkey-patching PCP internals: - wp_plugin_check_before_setup_environment - wp_plugin_check_after_setup_environment - wp_plugin_check_before_cleanup_environment - wp_plugin_check_after_cleanup_environment Each action receives the Runtime_Environment_Setup instance. The after_* actions are wrapped in a try/finally block so they always fire once the method has been entered, including on early returns, keeping the before/after pair balanced for subscribers. See WordPress#1269.
Adds a new constant, `WP_PLUGIN_CHECK_BOOTSTRAP_FILE`, that holds an absolute path to a PHP file loaded on the two early-execution paths of Plugin Check: - inside `drop-ins/object-cache.copy.php`, right after the autoloader is loaded and before the check runner is initialized; and - inside `cli.php`, right after the autoloader is loaded and before the WP-CLI command is registered. Both paths currently run before mu-plugins, which leaves integrations without a reliable place to register listeners for plugin-check actions and filters. The bootstrap file fills that gap without requiring consumers to modify generated drop-ins. When the constant is defined but the file is missing, a warning is emitted (PHP `E_USER_WARNING` in the drop-in, `WP_CLI::warning()` in CLI) and execution continues; the constant being unset is a no-op. See WordPress#1269.
PHPUnit: verify that the new setup and cleanup actions fire as a balanced before/after pair and that each call receives the Runtime_Environment_Setup instance as its argument. Behat: exercise the WP_PLUGIN_CHECK_BOOTSTRAP_FILE extension point in the WP-CLI path with a second --require file defining the constant. Cover the happy path, the missing-file warning path, and the no-op when the constant is not defined. See WordPress#1269.
Adds a 1.10.0 section to the changelog summarizing the four new setup and cleanup actions on Runtime_Environment_Setup and the WP_PLUGIN_CHECK_BOOTSTRAP_FILE constant. See WordPress#1269.
Extends coverage for the setup/cleanup hooks and the bootstrap-file mechanism introduced earlier on this branch: - PHPUnit: `test_set_up_fires_after_action_on_early_return` pins the non-obvious guarantee that `wp_plugin_check_after_setup_environment` still fires when `set_up()` returns early via the `WP_PLUGIN_CHECK_OBJECT_CACHE_DROPIN_VERSION` short-circuit, thanks to the `finally` block wrapping the body. - Behat: a scenario that registers listeners on `wp_plugin_check_before_setup_environment` and `wp_plugin_check_after_cleanup_environment` from a bootstrap file and asserts both lines appear in STDOUT during a real `plugin check` runtime check. End-to-end verifies that the bootstrap-file mechanism reaches the hooks, which is the realistic consumer flow. See WordPress#1269.
Introduces a new filter applied inside Abstract_PHP_CodeSniffer_Check::run() to the arguments returned by the check's get_args() implementation. This is the single call site for the entire PHPCS-based check family (plugin_review_phpcs, i18n_usage, late_escaping, direct_db, nonce_verification, prefixing, safe_redirect, setting_sanitization, localhost, minified_files, performant_wp_query_params, enqueued_scripts_in_footer, enqueued_resources, offloading_files), so one hook covers them all. Integrations can now override the PHPCS standard, runtime-set values, extensions, sniffs, exclude list, and installed_paths without subclassing the check and swapping it via wp_plugin_check_checks. The filter receives three arguments: the PHPCS args array, the check instance (Abstract_PHP_CodeSniffer_Check), and the Check_Result. Tests: - PHPUnit Abstract_PHP_CodeSniffer_Check_Tests covers the filter contract (correct args, check instance, result) and end-to-end effect on the I18n_Usage_Check using the existing i18n-usage-errors fixture. - Behat plugin-check-phpcs-args-filter.feature verifies that a bootstrap-registered filter can suppress a sniff that the default ruleset would otherwise emit, and that without the filter the same sniff still fires (control scenario). See WordPress#1269.
Introduces a filter applied inside Check_Result::add_message() — the single choke point through which every error and warning entry flows before being stored. Integrations can: - Return null (or any non-array) to suppress an individual finding. - Return a modified array to mutate the entry's message, severity, file, line, column, code, link, or docs. This is the third extension point in this series (after the four runtime setup/cleanup actions and the wp_plugin_check_phpcs_args filter). It addresses use cases where a check is mostly correct but emits a known false positive that the integration wants to silence without disabling the entire check — e.g. the Trademarks check flagging the substring "wp" inside legitimate brand names, or Direct_File_Access false positives on framework entry files. The filter receives three arguments: the entry data array, the Check_Result, and the original $is_error flag (for context only — promotion/demotion is intentionally out of scope; the original argument continues to drive bucketing). File paths are normalised before the filter fires so consumers see the same value that will be stored. Tests: - PHPUnit Check_Result_Tests gets three new tests covering the contract (correct args, result instance, is_error), suppression via null return, and mutation of the stored entry. - Behat plugin-check-result-filter.feature verifies suppression and message mutation end-to-end through a bootstrap-registered listener, plus a control scenario without the filter. See WordPress#1269.
CI run 26458958448 surfaced five Behat failures across the three new feature files; all originate in the fixtures, not the production code. 1) `add_action()` / `add_filter()` undefined. Four scenarios authored a `pcp-bootstrap.php` fixture that called `add_action()` or `add_filter()` at the top level. PCP loads this file from `cli.php` via `WP_PLUGIN_CHECK_BOOTSTRAP_FILE` before wp-settings.php runs, so the plugin API is not yet in memory and PHP fatals. Fix: defer registration with `WP_CLI::add_hook( 'after_wp_config_load', ... )` and explicitly require `wp-includes/plugin.php` if the plugin API is still missing — mirrors the pattern used by real-world consumer bootstrap files. 2) `When I run` rejects non-empty STDERR. The scenario that asserts the missing-bootstrap-file warning used `When I run`, which maps to `Process::run_check_stderr()` in wp-cli-tests and fails any non-empty STDERR. Our warning is the test point, so STDERR must be allowed. Fix: switch to `When I try` and pin the exit code explicitly with `Then the return code should be 0`. 3) Stale hook names after the trunk merge. The "register listeners that fire on setup/cleanup hooks" scenario still referenced `wp_plugin_check_before_setup_environment` / `wp_plugin_check_after_cleanup_environment` from the original PR. Trunk now exposes these as `wp_plugin_check_before_runtime_setup` / `wp_plugin_check_after_runtime_cleanup`; the scenario was silently subscribing to non-existent hooks once the fatal was resolved. Fix: update both names. No production code changes.
# Conflicts: # includes/Checker/Runtime_Environment_Setup.php # tests/phpunit/tests/Checker/Runtime_Environment_Setup_Tests.php
kagg-design
force-pushed
the
1269-new-hooks
branch
from
8e921f0 to
e84a83d
Compare
The Behat scenario asserting that `wp_plugin_check_before_runtime_setup` and `wp_plugin_check_after_runtime_cleanup` fire still failed on PHP 8.0 even after the deferred-registration fix in 8e921f0. Root cause: `CLI_Runner::allow_runtime_checks()` requires `is_plugin_active( $basename )` to return true. The fixture only *created* `foo-single.php`; it was never activated. With the target plugin inactive, `Abstract_Check_Runner::get_checks_to_run()` restricts the set to `TYPE_STATIC`, `has_runtime_check()` returns false, and `Runtime_Environment_Setup::set_up()` is never called — so the hooks that the bootstrap subscribes to never fire. Fix: activate `foo-single` via `wp plugin activate` before the check runs, mirroring the pattern used by the existing runtime-check scenarios in plugin-check.feature (e.g. lines 535, 635 which activate `foo-sample` and `foo-dependency foo-sample`). No production code changes.
Three follow-up fixes after the trunk merge surfaced via PR WordPress#1327 CI. 1) PHPMD — `Check_Result::add_message()` NPath complexity (256 > 200). The new `wp_plugin_check_check_result` filter added one branch (the `is_array( $data )` short-circuit) on top of the existing `if ( $error )` / nested per-line/column branches. The method is the single choke point for every error and warning recorded, so the branching is inherent and refactoring would obscure intent. Suppress the metric with `@SuppressWarnings(PHPMD.NPathComplexity)` — same approach already used on `Abstract_PHP_CodeSniffer_Check::run()`. 2) PHPMD — `Abstract_PHP_CodeSniffer_Check::run()` length (102 > 100). Adding the `apply_filters( 'wp_plugin_check_phpcs_args', ... )` call with its docblock pushed the method just over the 100-line threshold. The method already carries `@SuppressWarnings(PHPMD.NPathComplexity)`; add `@SuppressWarnings(PHPMD.ExcessiveMethodLength)` next to it. Breaking the method up would split the PHPCS lifecycle logic for marginal benefit. 3) PHPUnit — `Runtime_Environment_Setup_Tests` hook-argument assertions. The trunk merge changed the `wp_plugin_check_*_runtime_setup` and `*_runtime_cleanup` action signature: hooks now receive an `array( 'early_exit' => bool )` payload, not the `Runtime_Environment_Setup` instance. The original PR's `assertSame( $runtime_setup, $calls[X][1] )` therefore fails. Update both `test_set_up_fires_setup_environment_hooks` and `test_clean_up_fires_cleanup_environment_hooks` to assert the payload shape (array with `early_exit` key) rather than instance identity. The test still verifies the hooks fire in the right order with a payload — which is the API contract that matters for consumers. 4) Drop brittle PHPUnit mutation test. `Abstract_PHP_CodeSniffer_Check_Tests::test_phpcs_args_filter_mutation_affects_check_output` tried to widen the i18n_usage `runtime-set.text_domain` and assert that all `WordPress.WP.I18n.TextDomainMismatch` errors disappear from the i18n-usage-errors fixture. The fixture references several different text domains, only some of which the widened list covered, so the assertion was unreliable. The contract that the filter receives `$args`, `$check`, and `$result` is already covered by `test_phpcs_args_filter_receives_args_check_and_result`; the end-to-end "mutation reaches PHPCS" claim is covered by `plugin-check-phpcs-args-filter.feature`, which excludes a sniff via the filter and asserts the sniff's message vanishes from STDOUT. The mutation test added nothing the other two didn't.
Real-world consumer feedback — WPFormsWe've been running this PR's branch end-to-end against the WPForms (Pro/Lite) codebase as a stress test. Wanted to share what these four extension points unlocked, since concrete numbers might be useful context for the review. TL;DR
Why these hooks matter — the WPForms caseWPForms is a large plugin with a deep legacy tree. A vanilla
That file ended up at ~530 lines — roughly half of it nonce validation, capability checks, filesystem bookkeeping, and the object-cache patching dance, none of which had anything to do with actually configuring PCP. After all of that, the output still carried hundreds of lines of irrelevant findings on every run, because the only available knob for noise reduction was "remove an entire check" via What we rewrote it intoUsing the four extension points this PR adds —
After these changes, the long tail of irrelevant findings collapses to zero. What's left is the signal — actual new problems that came in through diff. A separate finding worth flaggingWhile testing, we hit an interesting case that ended up being solved by this same PR. Plugin teams that ship a custom PHPCS standard (e.g. For WPForms specifically this meant that only a fraction of the team's actual ruleset was being applied in PCP runs before — the WPForms-specific sniffs (
add_filter(
'wp_plugin_check_phpcs_args',
static function ( array $args, $check ): array {
if ( $check instanceof Plugin_Review_PHPCS_Check ) {
$current_paths = (string) \PHP_CodeSniffer\Config::getConfigData( 'installed_paths' );
$paths = array_filter( array_map( 'trim', explode( ',', $current_paths ) ) );
$paths[] = WP_PLUGIN_DIR . '/.vendor/awesomemotive/wpforms-phpcs';
$args['standard'] = WPMU_PLUGIN_DIR . '/wpf-pcp-phpcs.xml';
$args['installed_paths'] = array_values( array_unique( $paths ) );
}
return $args;
},
10,
2
);After the fix, the full WPForms standard loads, and PCP starts reporting the violations the team actually cares about — which is the entire point of running PCP in CI in the first place. Reference integration (attached)For maintainers curious what a real-world consumer integration looks like with these hooks, attached to this comment are four files: OLD_wpf-pcp.php — the ~530-line integration from before this PR. Most of it is the object-cache patching dance. NEW_wpf-pcp.php We've also written internal documentation walking through setup, usage (both WP-CLI and the admin UI), and customisation patterns — happy to share that if it's useful as a reference for other consumers. RolloutThe plan is to ship the new integration to WPForms's internal Developer Tools plugin once PCP 2.0 (or whichever release these hooks land in) is tagged. We'll keep the old integration in place until then so existing dev installs don't break. Broader valueThe simpler model is reusable. Any plugin team with a similar shape — large legacy tree, team-internal PHPCS standard, known PCP false positives — can now drop in a single mu-plugin file referencing these four hooks and have a working PCP pre-flight in CI without the gymnastics our old integration required. That feels like the right outcome for an extension API: the integration code is mostly what you want changed, not how to thread the change through PCP's internals. Happy to elaborate on any of these specifics if useful. |
2 participants
What?
Closes #1269
Why?
How?
Testing Instructions
AI Usage Disclosure
If AI tools were used, please describe how they were used:
Screenshots or screencast