Fix broken phpcs excludes and speed up linting - #771
Open
obenland wants to merge 6 commits into
Open
Conversation
…p linting. WPCS 3 renamed several sniffs, so excludes referencing the old names were silently ignored: ~450 alignment-related violations the config documents as allowed were surfacing again, failing PRs that touch those lines. PHPCS never validates error-code-level excludes, so this went unnoticed. - Reference the WordPress meta-standard, explicitly excluding the four sniffs it auto-includes beyond Core/Docs/Extra, and re-add the alignment excludes under their current sniff names. Sniff coverage is otherwise unchanged. - Remove the JS/CSS exclude patterns. PHPCS does not escape dots in patterns, so `*.js[x]?` matched any path containing "js" and hid four PHP files (block-json parser/validator, export-json CLI, gp-js-warnings) from linting. With `extensions=php` the patterns were redundant anyway. - Add parallel, result caching, basepath, and a 512M memory limit. A full-repo lint previously ran out of memory; it now completes in ~27s cold and under a second warm. - Let phpcbf fix multi-line function call formatting (drop the phpcbf-only exclude for PEAR.Functions.FunctionCallSignature). - Batch new-file scans in CI into a single phpcs invocation so parallel processing applies, and cache Composer dependencies in the workflow. - Update wp-coding-standards/wpcs to ^3.4.1 and pin phpcompatibility-wp to ^2.1. The lock update moves PHPCS to 3.13.6, which fixes CVE-2026-67434. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
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 Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
There was a problem hiding this comment.
Pull request overview
Updates the repository’s PHPCS configuration and CI workflow to restore intended excludes after the WPCS 3 rename, reduce false “alignment” failures, and significantly speed up linting locally and in GitHub Actions. It also bumps PHPCS/WPCS-related dev dependencies (including a PHP_CodeSniffer security fix).
Changes:
- Fixes/modernizes
phpcs.xml.dist(new sniff codes, switch toWordPressmeta-standard, remove overly-broad JS/CSS exclude patterns, enable parallel + caching + higher memory limit). - Speeds up CI linting by caching Composer dependencies and batching new-file PHPCS scans into a single invocation.
- Updates dev dependencies (WPCS constraint, PHPCompatibilityWP pin, and PHPCS patch release in
composer.lock).
Reviewed changes
Copilot reviewed 3 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| phpcs.xml.dist | Restores intended sniff excludes post-WPCS-3 rename and adds PHPCS performance settings (parallel, cache, basepath, memory limit). |
| composer.json | Updates dev dependency constraints for WPCS and PHPCompatibilityWP. |
| composer.lock | Locks updated toolchain versions (including PHP_CodeSniffer patch). |
| .gitignore | Ignores the new PHPCS result cache file. |
| .github/workflows/phpcs.yml | Adds Composer dependency caching to speed up CI. |
| .github/bin/phpcs-branch.php | Batches new-file scans to allow PHPCS parallelism to actually take effect. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Paths come from git and are interpolated into a shell command; a filename containing a space or shell metacharacter would split into bogus arguments. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Same issue class as the previous commit: run_phpcs_changed() interpolates git-reported file paths (and names derived from them) unescaped into five shell commands, including redirect targets. A crafted filename among a PR's modified files would break the scan the same way it did for new files. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CI only enforces the ruleset on lines a branch touches and on new files, so sniff exclusions only ever shield newly written code. Remove them all, keeping just the deliberate deviations: direct/meta DB queries (custom tables are the architecture here), the I18nTextDomainFixer utility, short array syntax, and unknown capabilities pending the allowlist in PR WordPress#583. Supersedes PR WordPress#714, which proposed the same standard via a second config file; a single config can't drift apart from what CI enforces and keeps local lint results identical to CI. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
obenland
changed the title
obenland
changed the title
…h one. PHPUnit requires a discovered test file to be named after the class it declares, which conflicts with the WordPress file-naming convention. WPCS exempts recognized test classes from the sniff, but recognition fails for classes extending project-local base classes and, in namespaced files, for parent classes resolved via use imports. Disable the sniff for test directories in the ruleset and remove the seven per-file annotations that worked around it. Test harness files named after their class (phpunit/includes/testcase.php) can be renamed in a follow-up. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Problem
The WPCS 3 upgrade renamed several sniffs, and PHPCS never validates error-code-level excludes — it just sets severity 0 for whatever string it's given. Two excludes in
phpcs.xml.distreferenced the old sniff names and had been silently doing nothing:WordPress.WhiteSpace.PrecisionAlignment.Found→ nowUniversal.WhiteSpace.PrecisionAlignment.FoundWordPress.Arrays.ArrayDeclarationSpacing.SpaceBeforeArrayCloser→ nowNormalizedArrays.Arrays.ArrayBraceSpacing.*As a result, ~450 alignment-related violations the config explicitly documents as allowed ("Aligning things can make the code more readable") were surfacing again — the
ArrayBraceSpacingones as errors, failingphpcs-changedCI for anyone touching an aligned line.Separately, a full-repo
composer run lintran out of memory at the default 128M limit, and CI scanned files onephpcsinvocation at a time with no dependency caching.Changes
phpcs.xml.distWordPressmeta-standard, explicitly excluding the four sniffs it auto-includes beyond Core/Docs/Extra (DirectDatabaseQuery,SlowDBQuery,ValidatedSanitizedInput,I18nTextDomainFixer), and restore the alignment excludes under their current sniff names. Verified: per-sniff violation output overwp-contentis identical to the old config, except the six alignment codes dropping to zero.*.js[x]?matched any path containing "js" and accidentally hid four real PHP files from linting (plugin-directory/block-json/class-parser.php,class-validator.php,wporg-gp-customizations/.../class-export-json.php,wporg-gp-js-warnings.php). Withextensions=phpthe patterns were redundant for their stated purpose anyway.parallel=10, result caching,basepath, and a 512M memory limit. Full-repo lint: previously OOM → now ~27s cold, <1s warm.phpcbf-onlyexclude forPEAR.Functions.FunctionCallSignature, socomposer run formatcan fix multi-line call formatting.CI
.github/bin/phpcs-branch.php: batch new-file scans into a singlephpcsinvocation so parallel processing applies (per-file invocations never fork workers)..github/workflows/phpcs.yml: cache Composer dependencies keyed oncomposer.lock.Dependencies
wp-coding-standards/wpcs^3.3.0→^3.4.1(constraint catch-up to PR #742); pinphpcompatibility/phpcompatibility-wp*→^2.1.Testing
phpcsoutput overwp-contentbetween old and new config: only the intended alignment codes change, plus the four rescued files entering scope.phpcs-changedmodified-file flow end-to-end.md5_file), so CI's rewrite-temp-file pattern cannot be served stale results.🤖 Generated with Claude Code
Update: full WordPress standard for new code (supersedes #714)
Since CI enforces this ruleset only on changed lines and new files (via
phpcs-changed), sniff exclusions only ever shield newly written code. This PR now removes all of them, adopting the goal of #714 without its second config file — a single config can't drift from what CI enforces (exactly the failure mode this PR fixes), and localcomposer run lintmatches CI.The only deliberate deviations kept:
WordPress.DB.DirectDatabaseQuery/SlowDBQuery(custom tables are the architecture here),WordPress.Utils.I18nTextDomainFixer(config-driven rewrite utility), short array syntax (used throughout the codebase), andWordPress.WP.Capabilities.Unknown(pending the allowlist in #583).