Remove the unused PHPCS rulesets and widen the workflow triggers - #258
Merged
Conversation
The package carried its own phpcs ruleset from before this repo had any PHP linting. Commit 609203b added the root `phpcs.xml.dist` and the coding standards workflow in April 2024, and the root ruleset already scans `./packages/php`, so the package copy has been redundant since. The dependency existed only to supply the `WooCommerce-Core` standard that ruleset referenced. The root install has never provided that standard and never needed to, since the package source already passes the root ruleset. Nothing runs the package copy. CI lints from the repo root, and Composer never installs a dependency's require-dev, so the extensions consuming this library never saw it either. Dropping it also drops the lock file that carried the vulnerable WPCS version, which resolves the advisory by removing the dependency rather than bumping it.
Like the `compat-checker` one removed in the previous commit, this ruleset predates repo-wide PHP linting. Commit 609203b added the root `phpcs.xml.dist` in April 2024, and its `<file>` entries already cover `./plugins`. This copy has never been runnable here. It references the `WooCommerce-Core` standard, which ships in `woocommerce/woocommerce-sniffs`, and the plugin has required only `composer/installers` since it was added. Running it fails at `Referenced sniff "WooCommerce-Core" does not exist` before reaching any rule. The plugin's `composer.json` and `composer.lock` stay, since `composer/installers` drives the `wordpress-plugin` install path.
The `composer.json` and `composer.lock` trigger paths are root anchored, so a change to `packages/php/compat-checker/composer.json` or any other nested manifest starts no run. That is how the two removals in this branch would have shipped unchecked. `phpcs.xml.dist` is missing from the list too, so a change to the ruleset the job actually enforces never starts a run either. Both the bare and the `**/` prefixed forms are listed because a bare entry matches the root file only, and whether a `**/` prefix also matches the root is not stated in the filter pattern reference.
There was a problem hiding this comment.
Pull request overview
This PR removes unused, non-functional PHPCS rulesets/development dependencies from subpackages and updates the PHP coding standards workflow so CI runs when relevant package manifests change.
Changes:
- Removed plugin/package-local PHPCS rulesets that referenced unavailable sniffs and were not used by CI.
- Removed
packages/php/compat-checkerdev-only Composer artifacts (composer.lock,require-dev, andconfig.allow-plugins) tied to the unused ruleset. - Expanded
.github/workflows/php-coding-standards.ymlpath triggers to include nestedcomposer.json/composer.lockchanges.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
plugins/grow-smooth-generator/phpcs.xml.dist |
Removes an unused local PHPCS ruleset. |
packages/php/compat-checker/phpcs.xml |
Removes an unused local PHPCS ruleset. |
packages/php/compat-checker/composer.lock |
Removes a dev-only lockfile tied to the unused PHPCS setup. |
packages/php/compat-checker/composer.json |
Drops dev-only PHPCS/sniffs configuration from the package. |
.github/workflows/php-coding-standards.yml |
Broadens workflow triggers to include nested Composer manifest changes. |
Suppressed comments (1)
.github/workflows/php-coding-standards.yml:23
- Same as the
pushtrigger:phpcs.xml.distinpull_request.pathsonly matches the repo root. Add**/phpcs.xml.distso changes to nested rulesets (or deletions like the one in this PR) reliably trigger the check.
- "**/composer.json"
- "**/composer.lock"
- phpcs.xml.dist
- .github/workflows/php-coding-standards.yml
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+11
to
14
| - "**/composer.json" | ||
| - "**/composer.lock" | ||
| - phpcs.xml.dist | ||
| - .github/workflows/php-coding-standards.yml |
Member
Author
There was a problem hiding this comment.
This one does not apply here. Those nested rulesets are exactly what this PR removes, so the root phpcs.xml.dist ends up being the only one left.
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Changes proposed in this Pull Request:
packages/php/compat-checker: removesphpcs.xml,composer.lock, and therequire-devandconfig.allow-pluginsblocks fromcomposer.json. All of it existed to supply theWooCommerce-Corestandard for that one ruleset.wp-coding-standards/wpcs2.3.0 inpackages/php/compat-checker/composer.lockfor arbitrary command execution in theWordPress.WP.EnqueuedResourceParameterssniff (GHSA-3pwp-g2mj-5p3v).plugins/grow-smooth-generator: removesphpcs.xml.dist. Itscomposer.jsonandcomposer.lockstay, sincecomposer/installersis a real dependency that driveswordpress-plugininstall paths..github/workflows/php-coding-standards.yml: adds**/composer.json,**/composer.lockandphpcs.xml.distto the trigger paths. The existing manifest entries are root anchored, so a change like the two above started no run at all.Why removing the two rulesets is safe:
phpcs.xml.distalready scans./pluginsand./packages/php, which is exactly the source both local rulesets pointed at. No source file changes here, and CI still lints all of it.require-dev, so the extensions consumingcompat-checkernever saw it either.WooCommerce-Core, which the root install has never provided, so running either fails atERROR: Referenced sniff "WooCommerce-Core" does not exist.before reaching any rule.The published
compat-checkerbranch keeps serving the extensions that install the library through"woocommerce/grow": "dev-compat-checker". The package has no production dependencies, so their installs are unchanged.Detailed test instructions:
compat-checkermanifest is still valid, from insidepackages/php/compat-checker.