Changeset 63019 for trunk/.github/workflows/reusable-phpstan-static-analysis-v1.yml

Timestamp:

08/05/2026 07:16:57 AM (2 months ago)

Author:

westonruter

Message:

Build/Test Tools: Raise the PHPStan rule level to 1.

Level 1 adds detection of possibly undefined variables, and of unknown magic methods and properties on classes with __call and __get. The 494 errors this surfaces in existing code are recorded in baselines rather than being fixed here, so that new code is held to level 1 straight away while the existing reports are worked through separately. No files under src are changed.

The tests/phpstan/baseline.php file is replaced by one baseline per error identifier under tests/phpstan/baselines, so that the remaining work on each kind of error is visible as a single file that should shrink to nothing and then be deleted. Every entry is scoped to the file which the error occurs in and carries an exact occurrence count, so that a new occurrence of an already baselined error is reported rather than absorbed. The consequence is that fixing a baselined error means regenerating its baseline in the same change, because the count no longer matches.

PHPStan's own --generate-baseline captures every error a run reports, with no way to restrict it to one identifier, so tests/phpstan/generate-baselines.php is added to write the files instead, exposed as composer phpstan:baselines and as npm run typecheck:php:baselines. A run also deletes any baseline whose identifier no longer reports anything, and rewrites the list of baselines in phpstan.neon.dist. The ignoreErrors in that file now has a comment explaining how it is distinct from a baseline: an entry there is a decision that the code is right as written, whereas a baseline entry is work still to be done. The constants that add_theme_support() defines are declared in the configuration so that the errors around them are resolved rather than recorded, and tests/phpstan/README.md is updated throughout.

Three problems in the static analysis GHA workflow are fixed as well. Fixing a baselined error makes PHPStan report an unmatched ignore, which surfaced only as an annotation reading like a complaint about a correct fix; the job now detects any ignore.* report and fails with an explanation of what to run. An analysis that did not finish passed as a green run, because the status of the pipeline was that of cs2pr rather than of PHPStan; that status is now recovered and a run that did not finish fails. The path filter deciding whether the workflow runs named only the old baseline file, so a pull request that merely regenerated the baselines would not have run the analysis that checks them.

Developed in ​https://github.com/WordPress/wordpress-develop/pull/11151.
Follow-up to r61699.

Props westonruter, sabernhardt, apermo, johnjamesjacoby, adamsilverstein, justlevine.
See #61175.
Fixes #64680.

File:

• trunk/.github/workflows/reusable-phpstan-static-analysis-v1.yml (modified) (2 diffs)

trunk/.github/workflows/reusable-phpstan-static-analysis-v1.yml

@@ -33 +33 @@
   # - Configures caching for PHPStan static analysis scans.
   # - Runs PHPStan static analysis (with Pull Request annotations).
+  # - Checks whether the baselines need regenerating.
   # - Saves the PHPStan result cache.
   # - Ensures version-controlled files are not modified or deleted.
@@ -94 +95 @@
       - name: Run PHP static analysis tests
         id: phpstan
-        run: composer run phpstan -- -vvv --error-format=checkstyle | cs2pr --errors-as-warnings --graceful-warnings
+        run: |
+          # The report is written to a file as well as piped to cs2pr, so that the step below
+          # can look at it.
+          #
+          # cs2pr exits successfully so that reported errors annotate the pull request without
+          # failing the run. A pipeline reports only the status of its last command, so that
+          # also discards the status of the analysis itself. Recover it from PIPESTATUS.
+          composer run phpstan -- -vvv --error-format=checkstyle | tee "${RUNNER_TEMP}/phpstan-report.xml" | cs2pr --errors-as-warnings --graceful-warnings
+          status="${PIPESTATUS[0]}"
+
+          # PHPStan exits 1 when it has errors to report, which is the expected case here and
+          # is what the annotations are for. Anything higher means it did not finish at all,
+          # which would otherwise pass silently, since the discarded status was the only sign.
+          if [ "${status}" -gt 1 ]; then
+            echo "::error title=PHPStan did not complete::The analysis exited with status ${status}, so the code was not fully checked. This is a failure of the run itself rather than a problem found in the code."
+            exit "${status}"
+          fi
+
+      # An ignored error that no longer occurs, or occurs a different number of times, is
+      # reported under an `ignore.*` identifier. That is not something to fix in the code: the
+      # usual cause is that the error *was* fixed, leaving a baseline describing a state that
+      # no longer exists. PHPStan does not allow those reports to be ignored or baselined.
+      #
+      # The analysis above is reported as warnings, so this would otherwise surface as a
+      # passing run carrying an annotation that reads like a complaint about a fix. Call it
+      # out on its own, and say what to do about it.
+      #
+      # Detection is on the identifier rather than the message, which is prose and may be
+      # reworded in any release. The checkstyle format carries it in the `source` attribute.
+      - name: Check whether the baselines need regenerating
+        if: ${{ !cancelled() }}
+        env:
+          BASELINES_URL: ${{ github.server_url }}/${{ github.repository }}/tree/${{ github.sha }}/tests/phpstan/baselines
+          README_URL: ${{ github.server_url }}/${{ github.repository }}/blob/${{ github.sha }}/tests/phpstan/README.md
+        run: |
+          # This step runs even when the analysis above it failed, in which case the report may
+          # never have been written. That failure is reported there, so there is nothing to add
+          # here beyond staying quiet about a file that was never going to exist.
+          if [ ! -f "${RUNNER_TEMP}/phpstan-report.xml" ]; then
+            exit 0
+          fi
+
+          # Leave the run alone unless PHPStan reported an ignore error, because everything
+          # below concerns an ignore configuration that no longer describes the code, and
+          # nothing else. Those errors are `ignore.unmatched`, where a pattern matched nothing
+          # at all, and `ignore.count`, where it matched a different number of times than the
+          # entry records. The `ignore.` prefix is matched rather than those two names so that
+          # any later addition to the group is caught as well.
+          if ! grep -q 'source="ignore\.' "${RUNNER_TEMP}/phpstan-report.xml"; then
+            exit 0
+          fi
+
+          # The summary is Markdown, and its code spans and fences are written literally, so the
+          # heredoc is quoted to keep the backticks out of the shell's hands. The links are
+          # written in reference style for the same reason: the URLs are the only part needing
+          # a variable, so defining them afterwards keeps the whole of the prose in here.
+          #
+          # A newline renders as a line break rather than a space, so each paragraph is one
+          # line however long that makes it, and the rendered summary wraps to its own width.
+          cat >> "${GITHUB_STEP_SUMMARY}" <<'SUMMARY'
+          ## PHPStan baselines are out of date
+
+          An ignored error no longer occurs, or occurs a different number of times, so PHPStan reported it under an `ignore.unmatched` or `ignore.count` identifier.
+
+          **If you fixed the error, this is expected.** Each baseline entry records an exact count for a specific file, so that a new occurrence of an already baselined error is reported rather than absorbed. That same exactness means fixing one leaves the baseline describing a state that no longer exists. There is nothing to fix in the code; the baselines just need to catch up.
+
+          Regenerate them and commit the result:
+
+          ```bash
+          npm run typecheck:php:baselines
+          ```
+
+          or, outside the Docker environment:
+
+          ```bash
+          composer phpstan:baselines
+          ```
+
+          That rewrites the files under [`tests/phpstan/baselines`][baselines], deletes any whose errors are now all fixed, and updates the list of them in `phpstan.neon.dist`.
+
+          Where the report names an `@phpstan-ignore` annotation in the code rather than a baseline entry, remove that annotation instead; regenerating will not clear it.
+
+          See [`tests/phpstan/README.md`][readme] for details.
+
+          SUMMARY
+
+          {
+            echo "[baselines]: ${BASELINES_URL}"
+            echo "[readme]: ${README_URL}"
+          } >> "${GITHUB_STEP_SUMMARY}"
+
+          echo "::error title=PHPStan baselines are out of date::An ignored error no longer occurs, or occurs a different number of times. If you fixed it, that is expected: run \`npm run typecheck:php:baselines\` or \`composer phpstan:baselines\` and commit the updated baselines. See ${README_URL}"
+          exit 1
 
       - name: "Save result cache"