The CI check for PHP-CS-Fixer isn't working

[Sesquipedalian]

The continuous integration check that is suppose to check PRs using PHP-CS-Fixer is not working correctly. It always reports that the test was passed successfully, even when a PR contains code that should be rejected by php-cs-fixer check.

[Sesquipedalian]

@live627, can you take a look at this?

[live627]

The issue looks to be in the shell script that lists changed files, added by @jdarwood007 so maybe he has an idea. I just wing it. All the ones I build are made of hopes and prayers, trial and error while testing each iteration in a new virtualized run. The actions tab on my repo/fork is quite the graveyard of failed runs.

From Copilot::

Short answer: yes — there are multiple bugs/fragile bits in how the workflow determines "changed files". I fixed them and included a more robust workflow below.

What was wrong (summary)

1. Misnamed output: you set all_changed_files_count but wrote a list of filenames (not a count).
2. The git-diff pipeline can fail the step when no PHP files changed because grep returns non-zero; workflow will stop.
3. Using xargs to join file names then grepping for '.php$' is unreliable (multiple names concatenated).
4. The logic in "Get extra arguments" is broken:
   • The test if echo "${CHANGED_FILES}" eq "1"; is invalid bash (it always succeeds because echo returns 0), so CHANGED_FILES was being cleared.
   • The snippet in the paste is truncated ("EXTRA_[...]") and incomplete.
5. For push comparisons you rely on git history that may not be fetched. actions/checkout must fetch the relevant commits (use fetch-depth: 0 or explicitly fetch the SHAs).
6. Using ${{ github.* }} expressions directly in shell without careful quoting/handling can produce wrong behavior in some edge-cases; it’s clearer to compute BASE/HEAD and fetch those SHAs explicitly.
7. The step names/variables are a bit confusing and fragile (e.g., using "count" for list).

What I changed / why

• Make checkout fetch full history (fetch-depth: 0) so we can diff arbitrary SHAs.
• Compute BASE/HEAD explicitly for push vs pull_request and fetch those refs.
• Use robust bash (set -euo pipefail) and safe fallbacks (grep || true) so the step won't fail when there are no matches.
• Produce a multiline GitHub Action output for the changed-files list.
• Build PHPCS_EXTRA_ARGS correctly only when there are PHP files to check and exclude php-cs-fixer config files and composer.lock.
• Use --path-mode=intersection and pass the file list after a -- separator when appropriate.

Here is a fixed workflow file you can drop in replacing the existing one:

on:
  push:
    branches:
      - release-3.0
  pull_request:

name: PHP Check
jobs:
  php-cs-fixer:
    name: PHP-CS-Fixer
    runs-on: ubuntu-latest
    steps:
      - name: Checkout repository (full history)
        uses: actions/checkout@v4
        with:
          fetch-depth: 0

      - name: Get changed PHP files
        id: changed-files
        run: |
          set -euo pipefail

          # Determine BASE and HEAD depending on event type
          EVENT_NAME="${{ github.event_name }}"
          if [ "$EVENT_NAME" = "pull_request" ]; then
            BASE_SHA="${{ github.event.pull_request.base.sha }}"
            HEAD_SHA="${{ github.event.pull_request.head.sha }}"
          else
            BASE_SHA="${{ github.event.before }}"
            HEAD_SHA="${{ github.event.after }}"
          fi

          # If BASE is all zeros (new branch), fall back to comparing HEAD^..HEAD
          if [ "$BASE_SHA" = "0000000000000000000000000000000000000000" ]; then
            echo "BASE is zero; fetching HEAD and comparing with its parent"
            git fetch --no-tags --prune --depth=2 origin "$HEAD_SHA"
            CHANGED=$(git diff --name-only "${HEAD_SHA}^" "$HEAD_SHA" || true)
          else
            # Ensure we have both SHAs
            git fetch --no-tags --prune --depth=1 origin "$BASE_SHA" "$HEAD_SHA" || git fetch --no-tags --prune origin "$BASE_SHA" "$HEAD_SHA"
            CHANGED=$(git diff --name-only "$BASE_SHA" "$HEAD_SHA" || true)
          fi

          # Keep only PHP files (one per line)
          PHP_FILES=$(printf '%s\n' "$CHANGED" | sed '/^\s*$/d' | grep -E '\.php$' || true)

          # Output as a multiline output so downstream steps can iterate safely
          echo "all_changed_files<<EOF" >> $GITHUB_OUTPUT
          printf '%s\n' "$PHP_FILES" >> $GITHUB_OUTPUT
          echo "EOF" >> $GITHUB_OUTPUT

      - name: Get extra arguments for PHP-CS-Fixer
        id: phpcs-intersection
        run: |
          set -euo pipefail

          # Read the multiline output into a variable
          CHANGED_FILES="${{ steps.changed-files.outputs.all_changed_files }}"
          # Normalize newlines -> spaces for passing to the fixer
          CHANGED_FILES_ONELINE=$(echo "$CHANGED_FILES" | tr '\n' ' ' | xargs || true)

          EXTRA_ARGS=""
          # Exclude config files from intersection list (allow full run when only config changed)
          if [ -n "$CHANGED_FILES_ONELINE" ]; then
            # Filter out the php-cs-fixer config files and composer.lock
            FILES_TO_USE=$(printf '%s\n' $CHANGED_FILES_ONELINE | grep -Ev '(^\.php-cs-fixer(\.dist)?\.php$|^composer\.lock$)' || true)
            if [ -n "$FILES_TO_USE" ]; then
              # Build the intersection args: --path-mode=intersection -- file1 file2 ...
              EXTRA_ARGS="--path-mode=intersection --"
              for f in $FILES_TO_USE; do
                EXTRA_ARGS="$EXTRA_ARGS $f"
              done
            fi
          fi

          # Export to environment for use in the next step
          echo "PHPCS_EXTRA_ARGS<<EOF" >> $GITHUB_ENV
          echo "$EXTRA_ARGS" >> $GITHUB_ENV
          echo "EOF" >> $GITHUB_ENV

      - name: PHP-CS-Fixer
        uses: docker://oskarstark/php-cs-fixer-ga
        with:
          args: --config=.php-cs-fixer.dist.php -v --dry-run --stop-on-violation --using-cache=no --allow-risky=yes ${{ env.PHPCS_EXTRA_ARGS }}

Notes / next steps

• This version is defensive: it won't fail the job if no PHP files changed; in that case PHPCS_EXTRA_ARGS will be empty and php-cs-fixer will run on the whole repository (you can change that behavior if you want—e.g., skip the job when there are no PHP files).
• If you prefer the job to be skipped when there are no PHP files, I can add a conditional that sets job: if: steps.changed-files.outputs.all_changed_files != '' (or use an output-based job-level if).
• If you want to rely on an action instead of custom bash, I can convert this to use dorny/paths-filter or dorny/changed-files for clearer intent.

[jdarwood007]

I also stumble with GH Actions because my laptop is to old to be able to run docker to do the local tester for actions. So its a lot of trial and error for me. If the suggested action fixes the problem, lets do it.