fix: skip phpcs when no PHP files changed, handle filenames with spaces by WayneRocha · Pull Request #22 · stellarwp/github-actions

WayneRocha · 2026-06-29T03:03:49Z

When CHANGED_FILES is empty (no PHP files in the PR), phpcs was called with no file arguments. Depending on the repo phpcs.xml, this either produced a non-JSON error string (failing jq validation → exit 1) or scanned the entire codebase unintentionally.

Also switches from bare variable expansion to xargs so filenames containing spaces are passed as individual arguments rather than being split on whitespace.

Additional context:

https://lw.slack.com/archives/C09T9KPV9HV/p1782497266594449

Troubleshooting Screenshots

When CHANGED_FILES is empty (no PHP files in the PR), phpcs was called with no file arguments. Depending on the repo phpcs.xml, this either produced a non-JSON error string (failing jq validation → exit 1) or scanned the entire codebase unintentionally. Also switches from bare variable expansion to xargs so filenames containing spaces are passed as individual arguments rather than being split on whitespace. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

shvlv · 2026-06-29T14:29:09Z

+          fi
+
+          # Run phpcs — use xargs so filenames with spaces are handled correctly
+          JSON_REPORT=$(echo \"${{ env.CHANGED_FILES }}\" | xargs -d '\n' vendor/bin/phpcs --report=json -q)


I think escaping quotes produces something like "file1.php file2.php" and PHPCS will treat it as a single argument. I'm not a bash expert, but Claude suggests printf '%s\n' "$CHANGED_FILES" | xargs -d '\n'.

shvlv · 2026-06-29T14:31:12Z

      - name: Get list of changed files
        id: files
        run: |
-          echo "CHANGED_FILES=$(git diff --name-only --diff-filter=AM ${{ github.event.pull_request.base.sha }} ${{ github.event.pull_request.head.sha }} -- . ':!tests' | grep '\.php$' | tr '\n' ' ')" >> $GITHUB_ENV
+          {
+            echo 'CHANGED_FILES<<EOF'
+            git diff --name-only --diff-filter=AM ${{ github.event.pull_request.base.sha }} ${{ github.event.pull_request.head.sha }} -- . ':!tests' | grep '\.php$' || true
+            echo EOF
+          } >> $GITHUB_ENV


Maybe we can detect this super-early after the actions/checkout@v4 step and gate the rest of the steps with an if step condition?

Like this @shvlv? d2afe85

- Move changed files detection right after checkout so expensive steps (setup-php, composer-install) are skipped entirely when no PHP files changed - Gate all remaining steps with has_php_files output instead of an early exit inside the shell script - Use printf + xargs -d newline instead of echo with escaped quotes so filenames containing spaces are passed as single arguments to phpcs Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

shvlv

@WayneRocha it looks good from my side. Did you try it on the real PR?

Let's await other folks review.

WayneRocha · 2026-06-29T15:44:15Z

@shvlv I only runned the phpcs commands on my machine (PR images), but I couldn't run the github workflow itself locally.
Other plugins rely on this repo/file on main branch and idk how exactly I could test it on the other plugins

Co-authored-by: Volodymyr Shelmuk <shvv86@gmail.com>

shvlv · 2026-06-29T15:52:07Z

Other plugins rely on this repo/file on main branch and idk how exactly I could test it on the other plugins

You can create a PR and use stellarwp/github-actions/.github/workflows/phpcs.yml@fix/phpcs-empty-changed-files in that PR.

WayneRocha · 2026-06-29T18:32:01Z

All the refered PR's are for test. Will be closed later, but reviews can se them to know if it's working well

redscar

This looks great. I see that you fixed file names with spaces and a few other pieces as well. Do you have a small list of scenarios this fixes?

WayneRocha · 2026-06-29T22:02:13Z

@redscar Basically...

Don't error if no PHP files changed
If path has spaces
Sometimes even real violations were not processed correctly

Nothing elaborated, only guarantee that the phpcs CI don't error anymore on our PR's

…ode (#23) ## Problem After #22 merged, phpcs CI still fails incorrectly when violations are found. `xargs` translates phpcs exit code `1` (violations found) into exit code `123`. GitHub Actions runs steps with `bash -e` by default, so when `xargs` returns `123` the step dies immediately — reviewdog never runs, no inline comments are posted, and the job fails with a cryptic exit code instead of the actual violations. **Practical impact:** - PRs with real phpcs violations get a job failure with exit code 123 and no inline comments - Developers have no idea what they need to fix ## Fix Replace `printf | xargs` with `mapfile` to read `CHANGED_FILES` into a bash array. This: - Gives us phpcs's **real exit code** (0 = clean, 1 = violations) instead of xargs's 123 - Uses `|| PHPCS_EXIT_CODE=$?` to prevent `bash -e` from killing the step before reviewdog runs - Still correctly handles **filenames with spaces** (each array element is one file path) ```bash # Before JSON_REPORT=$(printf '%s\n' "$CHANGED_FILES" | xargs -d '\n' vendor/bin/phpcs --report=json -q) PHPCS_EXIT_CODE=$? # always 123 when violations found, never 1 # After mapfile -t FILES <<< "$CHANGED_FILES" PHPCS_EXIT_CODE=0 JSON_REPORT=$(vendor/bin/phpcs --report=json -q "${FILES[@]}") || PHPCS_EXIT_CODE=$? ``` ## Test PRs All affected repos have test PRs pointing to `fix/phpcs-xargs-exit-code` (updated from #22 test PRs). ## Examples the-events-calendar/the-events-calendar#5676 the-events-calendar/tribe-common#2939 the-events-calendar/event-tickets#4280 More in #22

…de (#24) ## Problem After #22 and #23, the phpcs job still fails on PRs that introduce **no new violations**. The `Run reviewdog` step ends with: ```bash exit $PHPCS_EXIT_CODE ``` So the job's pass/fail is driven by **phpcs's** exit code. But phpcs scans the whole changed file, so its exit code reflects **every** issue in that file — including pre-existing ones on lines the PR never touched. `reviewdog`, by contrast, runs with `-filter-mode=added` and only reports/fails on issues introduced on the PR's **added lines**. The two disagree, and the job exits on the phpcs code. ### Reproduction — the-events-calendar#5687 "clean" test (`bf090fc`) From the job log: ``` mapfile -t FILES <<< "src/functions/views/provider.php" ##[error]Process completed with exit code 2. ``` - phpcs scanned `provider.php`, found pre-existing issues → exited **2**. - Those issues are **not** on the PR's added lines, so reviewdog filtered them out → **no inline comment** ("No bot display"). - Then `exit $PHPCS_EXIT_CODE` ran `exit 2` → **job failed anyway**. `#22`/`#23` focused on faithfully *preserving* the phpcs exit code — which is exactly the value that should **not** gate the job. ## Fix - Run phpcs with `--report=checkstyle` and pipe straight to reviewdog (`-f=checkstyle`), dropping the `jq` transform. - **Exit with reviewdog's exit code, not phpcs's.** `-filter-mode=added -fail-level=any` means reviewdog exits `1` only when it reports a violation on a line this PR changed. - Keep a guard: if phpcs produces **no** checkstyle output *and* exits non-zero, it genuinely failed to run (bad standard, parse error, …) → fail the job with phpcs's code. ```bash set +e CHECKSTYLE_REPORT=$(vendor/bin/phpcs --report=checkstyle -q "${FILES[@]}") PHPCS_EXIT_CODE=$? set -e if [ -z "$CHECKSTYLE_REPORT" ] && [ "$PHPCS_EXIT_CODE" -ne 0 ]; then echo "phpcs failed to run (exit code $PHPCS_EXIT_CODE)" exit "$PHPCS_EXIT_CODE" fi REVIEWDOG_EXIT_CODE=0 echo "$CHECKSTYLE_REPORT" \ | reviewdog -f=checkstyle -name="phpcs" -filter-mode="added" -fail-level=any -reporter=github-pr-review \ || REVIEWDOG_EXIT_CODE=$? exit "$REVIEWDOG_EXIT_CODE" ``` ## Expected behavior across the-events-calendar#5687 test commits | Commit | Introduces new violations? | reviewdog comment | Job | |---|---|---|---| | Test 1 / Test 2 | yes | posted | **fails** ✅ | | Test 3 (clean) | no (pre-existing only) | none | **passes** ✅ | ## Test PR the-events-calendar#5687 repointed to `@fix/phpcs-reviewdog-exit-code` to re-run all three scenarios. Created by claude 🤖

WayneRocha force-pushed the fix/phpcs-empty-changed-files branch from ec48262 to be43506 Compare June 29, 2026 03:55

WayneRocha requested review from redscar and shvlv and removed request for redscar June 29, 2026 13:41

shvlv reviewed Jun 29, 2026

View reviewed changes

shvlv requested review from defunctl and dpanta94 June 29, 2026 14:31

shvlv reviewed Jun 29, 2026

View reviewed changes

Comment thread .github/workflows/phpcs.yml Outdated

Update .github/workflows/phpcs.yml

f356436

Co-authored-by: Volodymyr Shelmuk <shvv86@gmail.com>

redscar approved these changes Jun 29, 2026

View reviewed changes

dpanta94 approved these changes Jun 30, 2026

View reviewed changes

WayneRocha removed the request for review from defunctl June 30, 2026 13:59

WayneRocha merged commit e92d10b into main Jun 30, 2026

WayneRocha deleted the fix/phpcs-empty-changed-files branch June 30, 2026 14:00

WayneRocha mentioned this pull request Jun 30, 2026

fix: use mapfile array instead of xargs to preserve real phpcs exit code #23

Merged

WayneRocha mentioned this pull request Jul 2, 2026

fix: gate phpcs job on reviewdog result, not phpcs whole-file exit code #24

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: skip phpcs when no PHP files changed, handle filenames with spaces#22

fix: skip phpcs when no PHP files changed, handle filenames with spaces#22
WayneRocha merged 3 commits into
mainfrom
fix/phpcs-empty-changed-files

WayneRocha commented Jun 29, 2026 •

edited

Loading

Uh oh!

shvlv Jun 29, 2026

Uh oh!

shvlv Jun 29, 2026

Uh oh!

WayneRocha Jun 29, 2026

Uh oh!

shvlv left a comment

Uh oh!

Uh oh!

WayneRocha commented Jun 29, 2026 •

edited

Loading

Uh oh!

shvlv commented Jun 29, 2026

Uh oh!

WayneRocha commented Jun 29, 2026

Uh oh!

redscar left a comment

Uh oh!

WayneRocha commented Jun 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

WayneRocha commented Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Additional context:

Troubleshooting Screenshots

Uh oh!

shvlv Jun 29, 2026

Choose a reason for hiding this comment

Uh oh!

shvlv Jun 29, 2026

Choose a reason for hiding this comment

Uh oh!

WayneRocha Jun 29, 2026

Choose a reason for hiding this comment

Uh oh!

shvlv left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

WayneRocha commented Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shvlv commented Jun 29, 2026

Uh oh!

WayneRocha commented Jun 29, 2026

Uh oh!

redscar left a comment

Choose a reason for hiding this comment

Uh oh!

WayneRocha commented Jun 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

WayneRocha commented Jun 29, 2026 •

edited

Loading

WayneRocha commented Jun 29, 2026 •

edited

Loading