Skip to content

feat: scope the PHPCS gate to the lines a change actually touches - #358

Merged
arifulhoque7 merged 3 commits into
weDevsOfficial:developfrom
arifulhoque7:feat/phpcs-line-scoped
Sep 8, 2026
Merged

arifulhoque7 merged 3 commits into
weDevsOfficial:developfrom
arifulhoque7:feat/phpcs-line-scoped

Conversation

@arifulhoque7

Copy link
Copy Markdown
Contributor

PHPCS reports whole files. On a codebase that already has violations, that means editing one line of a legacy file inherits every finding already in it, so the job is red however careful the author is, and people learn to ignore it.

Measured on the Elementor widgets branch (wedocs-plugin#324), which touches 16 PHP files:

count
Security findings in those files 175
Security findings on lines that branch wrote 63

The other 112 sit in Ajax.php, API.php and functions.php and predate the branch entirely. Asking a feature PR to fix core AJAX and REST paths it merely touched is the wrong trade: it bloats the diff, risks regressions, and makes review impossible.

What this does

bin/phpcs-changed-lines.php reads the PHPCS JSON report, keeps only findings that land on added or modified lines, emits a GitHub annotation for each, and sets the exit status.

Only errors fail the run. Warnings are annotated but do not block, mirroring cs2pr --graceful-warnings, which the Dokan and WPUF workflows already use.

$ phpcs --standard=phpcs.xml.dist <changed files> -q --report=json \
    | php bin/phpcs-changed-lines.php "$(git rev-parse HEAD^1)"

PHPCS: 106 finding(s) on changed lines, ... (348 findings in these files overall).

The base is HEAD^1 of the pull_request merge commit, which is the base branch tip, so the same commit the file list is already derived from.

Effect

On #324, before its own fixes: 348 findings in the files, 106 on changed lines. That branch then closed all of its errors, and the gate passes there now.

Nothing about the ruleset changes. A file with pre-existing violations is still reported in full when you run composer phpcs locally; this only decides what blocks a merge.

Related: weDevsOfficial/plugin-internal-tasks#9

https://claude.ai/code/session_019wvVjJ8agR8TWAVrrKVESH

PHPCS reports whole files. On a codebase with existing violations that means
editing one line of a legacy file inherits every finding already in it, so the
job is red no matter what the author does and people learn to ignore it.

Measured on the Elementor widgets branch: the files it touches hold 175
security findings, but only 63 sit on lines it wrote. The other 112 are in
Ajax.php, API.php and functions.php and predate the branch entirely.

bin/phpcs-changed-lines.php reads the PHPCS JSON report, keeps only findings
on added or modified lines, emits GitHub annotations for those, and exits
non-zero if any survive.

Claude-Session: https://claude.ai/code/session_019wvVjJ8agR8TWAVrrKVESH
Mirrors cs2pr --graceful-warnings, which the Dokan and WPUF workflows already
use. Every finding on a changed line is still annotated; a warning no longer
blocks the merge.

Claude-Session: https://claude.ai/code/session_019wvVjJ8agR8TWAVrrKVESH
bin/phpcs-changed-lines.php is a CLI tool, not plugin runtime, so the
WordPress sniffs for global prefixes, WP_Filesystem and system calls do not
apply to it. The WPUF rulesets exclude their bin/ directory for the same
reason.

Claude-Session: https://claude.ai/code/session_019wvVjJ8agR8TWAVrrKVESH
@arifulhoque7
arifulhoque7 merged commit aee6957 into weDevsOfficial:develop Sep 8, 2026
4 checks passed
@arifulhoque7
arifulhoque7 deleted the feat/phpcs-line-scoped branch September 8, 2026 07:17
arifulhoque7 added a commit that referenced this pull request Sep 8, 2026
Brings in the line-scoped PHPCS gate (#358) and the bin/ exclusion.

Claude-Session: https://claude.ai/code/session_019wvVjJ8agR8TWAVrrKVESH
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant