Fix 2.0.0 audit findings + pin generated plugins to PHP 8.3 - #10
Merged
Conversation
…affold
`npm run test:js` failed 4/4 suites (0 tests run) and `npm run lint:js`
threw ~20 errors on a freshly generated block/interactivity/React plugin.
Verified fixed end to end against a real `npm install`:
- jest.config.js extends @wordpress/scripts' own jest-unit config instead
of setting `preset:` (which dropped the Babel/JSX transform).
- tests/js/*.test.js: `{ virtual: true }` on every jest.mock() of a
@wordpress/* runtime package (they're webpack externals, not on disk);
dropped App.test.js's redundant @jest-environment docblock (jsdom is the
preset default) and view.test.js's dead top-level import.
- Added the @wordpress/* packages the authored JS imports (blocks,
block-editor, i18n, element, interactivity) to devDependencies —
dependency-extraction still externalises them at build, but it clears
import-resolution lint and gives editors types.
- lint:js scoped to `assets/src tests/js` (the wp-scripts-managed source,
not the hand-written jQuery/Elementor enqueue scripts) with
--no-error-on-unmatched-pattern; lint:style scoped + --allow-empty-input
(it errored on scaffolds with no CSS). Dropped the now-redundant
lint-assets CI job.
- prettier fixes in save.js / the two block tests / view.test.js / the
React entry / the generated webpack.config.js string.
- self-consumption CI now runs lint:js + lint:style + test:js on the
generated plugin — the blind spot that let all of this ship in 2.0.0.
Generator suite: 66/66.…ic bundle Interactivity.php registered the view as a script module pointing at assets/build/view.js, but wp-scripts built that entry as a classic IIFE reading `window.wp.interactivity` — a global core never defines (the Interactivity API ships only as a script module). The demo threw on click. wp-scripts can only build a view *module* from a block.json `viewScriptModule` field, which would force the interactivity module to become a block. But the view here only imports `@wordpress/interactivity`, a bare specifier WordPress resolves via its import map — so it needs no bundler at all. - view.js is now hand-written ESM at assets/js/view.js (alongside main.js / the widget scripts), and Interactivity.php's wp_register_script_module() points straight at it. - Dropped the `view` webpack entry and `hasInteractivity` from the webpack.config.js-override guard. New `has_webpack_build` flag (react || block || woo-js) gates the build/start scripts, so interactivity alone no longer ships a no-op `npm run build`; Jest + Playwright still ship (needs_build_pipeline). - view.test.js requires ../../assets/js/view.js; generator tests updated. Verified: interactivity+block+react scaffold builds, lint:js clean, test:js 4/4 (view.test.js included). Generator suite: 66/66.
…tainer_Test) A singleton() factory that resolves to null had its result stored but isset() reported the slot empty, so the factory re-ran on every get() and has() returned false for it. Both instance-slot checks in get()/has() now use array_key_exists(). Adds tests/Unit/Container_Test.php — the composition root's one moving part had no test at all. Eight cases: bind fresh-per-call, singleton once, singleton caching a null, instance identity, factory-gets-container, re-bind clears cache, unknown id throws Not_Found_Exception. Verified the null-singleton path directly against a generated scaffold. Generator suite: 66/66.
…author
Free-text values were escaped by file extension, not by where they land:
inside a .php file every {{PLUGIN_NAME}} got `'` -> `\'`, so a plain
docblock became `Composition root for Bob\'s Plugin.` (WordPress renders
the backslash). And a `*/` in --description closed the plugin-header
comment early -> parse error; a newline in --name injected a second
`Plugin Name:` header line.
Now:
- {{PLUGIN_NAME}} / {{DESCRIPTION}} / {{AUTHOR}} in any non-JSON file are
comment-safe: `*/` -> `* /`, newlines collapsed to spaces.
- a new {{PLUGIN_NAME_ESC}} carries the PHP-single-quoted-string escaping
(backslash first, then quote); the 18 `__( '{{PLUGIN_NAME}} …' )` /
sprintf sites now use it.
- JSON keeps its JSON.stringify path (unchanged).
Verified: `--name "Bob's */ Plugin" --description $'…*/…\nRequires PHP: 5.2'
--author "O'Brien */ & Co"` generates a plugin where every PHP file parses,
the header stays one line per field, and composer.json is valid JSON.
Generator suite: 66/66.…defaults - #3 validateMinPhp() now rejects < 8.0 (templates use `mixed`, typed properties, promotion — sub-8.0 generates code that fatals). - #6 uninstall.php's multisite loop passes get_sites( 'number' => 0 ) so networks past 100 sites are cleaned; comment points at a batched path for very large networks. - #8 the plain-text email template used esc_html() — which turns O'Brien into "O'Brien" in the inbox. Switched to wp_strip_all_tags() (what WooCommerce core's plain templates do). - #9 Rest_Controller::get_items_permissions_check() defaults to current_user_can( 'read' ) instead of a bare `return true;`; test asserts it gates rather than blanket-allows. - #10 Gateway::process_payment() fails closed (wc_add_notice + result 'failure') instead of calling payment_complete() on an unconfigured stub; the success flow is shown in the docblock. New test. - #11 phpcs.xml now scans ./assets/src (block render.php files) when there's a webpack build, with a scoped exclude for PrefixAllGlobals.NonPrefixedVariableFound on */blocks/*/render.php (those "globals" are WP_Block::render() locals). Generator suite: 66/66.
…up to
- Generated node-build CI job drops `cache: 'npm'`: setup-node fails the
step when there's no committed package-lock.json (there isn't on a
fresh scaffold).
- The single-version phpcs / integration CI jobs use {{MIN_PHP}} instead
of a hardcoded '8.2', so they never run below the version the plugin
claims to support. (The PHPUnit matrix was already dynamic.)
- Generated package.json gets `"test": "npm run test:js"` when Jest is
present, so `npm test` no longer errors "missing script".
- {{TESTED_UP_TO}} default 6.8 -> 6.9.
Generator suite: 66/66.CI's wp-org lint (WordPress-Docs) flagged three docblocks added/edited by the audit fixes: - Container_Test.php: four test-method short descriptions opened with a lowercase method name (bind(), singleton(), instance(), get()) -> Squiz.Commenting.FunctionComment.ShortNotCapital. - Interactivity.php::register_script_module(): the long description opened with the lowercase path "assets/js/view.js". - Gateway_Test.php: the '__' => ... stub arrow was one space short of the 'wc_add_notice' => key, tripping WordPress.Arrays.MultipleStatementAlignment. Reworded to lead with a capital and realigned the array. No behaviour change.
CI's self-consumption lint (the first run of `npm run lint:js` against a real scaffold, added for audit #1) surfaced five ESLint/Prettier errors in three Woo integration files that had never been linted: - blocks/cart-summary/index.js: useBlockProps() was called from a lowercase `edit: () => {}` arrow -> react-hooks/rules-of-hooks. Extracted a named `Edit()` component and hoisted the placeholder string to a module const ("Cart Summary (live on the frontend)" -> "Cart summary" so the __() call stays on one line for any realistic slug). - blocks-integration.js: `'{{SLUG}}-order-meta'` inline in createElement pushed the line past printWidth once the slug was substituted. Hoisted to `const cls`. - wc-gateway-block.js: the label fallback `__( '{{PLUGIN_NAME}}', '{{SLUG}}' )` went over printWidth with real values (and was unescaped). Replaced with a raw `{{PLUGIN_NAME_ESC}}` const (brand names aren't translated anyway), dropped the now-unused `__` alias, and pre-broke the `Content` createElement to the multi-line form Prettier wants. Verified end to end on a regenerated all-modules+React scaffold: lint-js / lint-style / build / test-unit-js all exit 0.
The 8.0 floor from audit #3 was heavier than needed: the only 8.0-only construct in the whole template set is the `: mixed` return type on Container::get(). Everything else the templates use (arrow functions, typed properties, null-coalescing assignment) is PHP 7.4. - validateMinPhp: floor at 7.4 (was 8.0); reject < 7.4. - requiredPhpFor(modules): new helper, mirrors requiredWpVersion — returns the highest of 7.4 and any per-module floor (MODULE_PHP_FLOOR, empty today). Non-interactive uses it as the default and rejects an explicit --min-php below it; interactive reconciles after module selection since the PHP prompt runs first. - templateFlags.php_8_0 (minPhp >= 8.0) gates `: mixed` in Container.php via an inline {{#if}} — 8.0+ targets keep the hint, 7.4 drops it. - Generated PHPUnit matrix + phpcs testVersion + composer "php" + plugin header all already follow {{MIN_PHP}}; CI matrix list gains a 7.4 leg. Verified: 68/68 unit tests; all-modules 7.4 scaffold — php -l clean, JSON valid, no `: mixed` / stray tags, CI matrix ['7.4'..'8.3'].
BREAKING: --min-php is gone and the interactive "Minimum PHP version" prompt (#9) is removed. Every scaffold now requires PHP 8.3 — a single `export const MIN_PHP = '8.3'`, not configurable. validateMinPhp, requiredPhpFor and MODULE_PHP_FLOOR are deleted; the generated PHPUnit matrix is a fixed ['8.3', '8.4']; {{MIN_PHP}} resolves to 8.3 in the plugin header, composer.json, readme.txt, ci.yml and phpcs.xml (testVersion 8.3-). Templates modernised to that baseline (unconditional — no {{#if}} forks): - Constructor property promotion + readonly: Plugin (container/providers) and the four Woo\Providers\*_Provider service-override constructors lose their property declarations and assignment bodies. - Container::get(): mixed is unconditional again (the php_8_0 gate is gone). - First-class callable syntax: all 38 `array( $this, 'method' )` and the 3 `array( Class::class, 'method' )` hook/command callbacks become `$this->method(...)` / `Class::method(...)`. match()/enum were considered but have no natural home — no switch statements, no related-constant clusters. Skipped rather than forced. Verified: 67 generator + 13 engine tests; full-module and minimal 8.3 scaffolds — php -l clean, {{MIN_PHP}} -> 8.3 everywhere, no stray tags. composer lint / PHPCS on the modern output is exercised by the self-consumption + verify CI jobs.
…tures
The self-consumption and verify jobs set up PHP 8.2, so `composer install`
in a generated scaffold now fails ("requires php >=8.3 but your php
version (8.2.33)"). Bump both setup-php steps to 8.3.
scripts/verify.js passed `--min-php 8.2` to the elementor fixture — that
flag no longer exists and made the generator exit non-zero. Removed.wp-scripts/WPCS's FunctionCallSignature sniff treats `$this->m(...)` as a zero-space function call and flags "Expected 1 space after/before parenthesis" (phpcbf rewrites it to `( ... )`). PHP accepts whitespace around the `...` token, so emit `$this->m( ... )` / `Class::m( ... )` directly. Test assertions updated to match.
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses the external audit of
create-wp-plugin-cli@2.0.0, then pins the generated baseline to PHP 8.3 and modernises the templates. One item per commit; 67 generator + 13 engine tests throughout; all 6 CI checks green.PHP 8.3 baseline (breaking)
--min-phpand the interactive "Minimum PHP version" prompt are removed. Every scaffold now requires PHP 8.3 —export const MIN_PHP = '8.3', not configurable.validateMinPhp/requiredPhpFordeleted; generated PHPUnit matrix fixed at['8.3', '8.4'];{{MIN_PHP}}→ 8.3 in plugin header,composer.json,readme.txt,ci.yml,phpcs.xml(testVersion 8.3-). The CLI's own PHP CI jobs move to 8.3 socomposer installresolves in a generated scaffold.Templates modernised to that baseline, unconditionally (no
{{#if}}forks):readonly—Plugin(container/providers) and the fourWoo\Providers\*_Providerservice-override constructors lose their property declarations and assignment bodies.Container::get(): mixedis unconditional (supersedes the interimphp_8_0gate / the 7.4 work in the earlier commit).array( $this, 'method' )and 3array( Class::class, 'method' )hook/command callbacks become$this->method( ... )/Class::method( ... )(the( ... )spacing is what WPCS's FunctionCallSignature sniff accepts).match()/enumwere considered but have no natural home (noswitch, no related-constant clusters) — skipped rather than forced.Audit findings
test:js/lint:jsbroken on a fresh scaffold{ virtual: true }mocks; scoped lint; self-consumption CI now runs lint:js/lint:style/test:jsview.jsis hand-written ESM atassets/js/view.js, served raw via WP import map--min-php 7.4emitted PHP 8 syntax*/, newline,\'in comments){{PLUGIN_NAME_ESC}}for PHP stringsuninstall.phpskipped multisite sites past 100get_sites( 'number' => 0 )Containerre-resolved anullsingleton forever (isset)array_key_exists()+ newContainer_Test.phpHi O'Brien,)wp_strip_all_tags()aloneget_items_permissions_check()returned baretruecurrent_user_can( 'read' )·get_item_schema()— not doneGateway::process_payment()marked orders paidwc_add_notice+'failure')phpcs.xmldidn't scan blockrender.php./assets/srcadded with a scoped exclude.*/templates/*exclude kept — CI proves the WC email/My-Account templates genuinely failwp-orgWPCSvendorinfilesships into the zipcomposer install --no-devbeforeplugin-ziptestscript,cache:'npm', stalereadme.txt/ non-matrixphp-versionNot in this PR
Schema::boot()hooksplugins_loadedfrom insideplugins_loaded;dbDeltaon a frontend request — still open.Rest_Controllerhas noget_item_schema().src/classes untested" andphpunit.xml.dist's PHPUnit-10-removed attributes (B6.12).