WordPress.org
Get WordPress

Make WordPress Core

Changeset 63019


Ignore:
Timestamp:
08/05/2026 07:16:57 AM (5 weeks 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.

Location:
trunk
Files:
5 added
1 deleted
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/.github/workflows/phpstan-static-analysis.yml

    r62855 r63019  
    1919      - 'phpstan.neon.dist'
    2020      - 'tests/phpstan/base.neon'
    21       - 'tests/phpstan/baseline.php'
     21      - 'tests/phpstan/baselines/**'
    2222      # Confirm any changes to relevant workflow files.
    2323      - '.github/workflows/phpstan-static-analysis.yml'

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

    r62742 r63019  
    3333  # - Configures caching for PHPStan static analysis scans.
    3434  # - Runs PHPStan static analysis (with Pull Request annotations).
     35  # - Checks whether the baselines need regenerating.
    3536  # - Saves the PHPStan result cache.
    3637  # - Ensures version-controlled files are not modified or deleted.
    … …  
    9495      - name: Run PHP static analysis tests
    9596        id: phpstan
    96         run: composer run phpstan -- -vvv --error-format=checkstyle | cs2pr --errors-as-warnings --graceful-warnings
     97        run: |
     98          # The report is written to a file as well as piped to cs2pr, so that the step below
     99          # can look at it.
     100          #
     101          # cs2pr exits successfully so that reported errors annotate the pull request without
     102          # failing the run. A pipeline reports only the status of its last command, so that
     103          # also discards the status of the analysis itself. Recover it from PIPESTATUS.
     104          composer run phpstan -- -vvv --error-format=checkstyle | tee "${RUNNER_TEMP}/phpstan-report.xml" | cs2pr --errors-as-warnings --graceful-warnings
     105          status="${PIPESTATUS[0]}"
     106
     107          # PHPStan exits 1 when it has errors to report, which is the expected case here and
     108          # is what the annotations are for. Anything higher means it did not finish at all,
     109          # which would otherwise pass silently, since the discarded status was the only sign.
     110          if [ "${status}" -gt 1 ]; then
     111            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."
     112            exit "${status}"
     113          fi
     114
     115      # An ignored error that no longer occurs, or occurs a different number of times, is
     116      # reported under an `ignore.*` identifier. That is not something to fix in the code: the
     117      # usual cause is that the error *was* fixed, leaving a baseline describing a state that
     118      # no longer exists. PHPStan does not allow those reports to be ignored or baselined.
     119      #
     120      # The analysis above is reported as warnings, so this would otherwise surface as a
     121      # passing run carrying an annotation that reads like a complaint about a fix. Call it
     122      # out on its own, and say what to do about it.
     123      #
     124      # Detection is on the identifier rather than the message, which is prose and may be
     125      # reworded in any release. The checkstyle format carries it in the `source` attribute.
     126      - name: Check whether the baselines need regenerating
     127        if: ${{ !cancelled() }}
     128        env:
     129          BASELINES_URL: ${{ github.server_url }}/${{ github.repository }}/tree/${{ github.sha }}/tests/phpstan/baselines
     130          README_URL: ${{ github.server_url }}/${{ github.repository }}/blob/${{ github.sha }}/tests/phpstan/README.md
     131        run: |
     132          # This step runs even when the analysis above it failed, in which case the report may
     133          # never have been written. That failure is reported there, so there is nothing to add
     134          # here beyond staying quiet about a file that was never going to exist.
     135          if [ ! -f "${RUNNER_TEMP}/phpstan-report.xml" ]; then
     136            exit 0
     137          fi
     138
     139          # Leave the run alone unless PHPStan reported an ignore error, because everything
     140          # below concerns an ignore configuration that no longer describes the code, and
     141          # nothing else. Those errors are `ignore.unmatched`, where a pattern matched nothing
     142          # at all, and `ignore.count`, where it matched a different number of times than the
     143          # entry records. The `ignore.` prefix is matched rather than those two names so that
     144          # any later addition to the group is caught as well.
     145          if ! grep -q 'source="ignore\.' "${RUNNER_TEMP}/phpstan-report.xml"; then
     146            exit 0
     147          fi
     148
     149          # The summary is Markdown, and its code spans and fences are written literally, so the
     150          # heredoc is quoted to keep the backticks out of the shell's hands. The links are
     151          # written in reference style for the same reason: the URLs are the only part needing
     152          # a variable, so defining them afterwards keeps the whole of the prose in here.
     153          #
     154          # A newline renders as a line break rather than a space, so each paragraph is one
     155          # line however long that makes it, and the rendered summary wraps to its own width.
     156          cat >> "${GITHUB_STEP_SUMMARY}" <<'SUMMARY'
     157          ## PHPStan baselines are out of date
     158
     159          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.
     160
     161          **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.
     162
     163          Regenerate them and commit the result:
     164
     165          ```bash
     166          npm run typecheck:php:baselines
     167          ```
     168
     169          or, outside the Docker environment:
     170
     171          ```bash
     172          composer phpstan:baselines
     173          ```
     174
     175          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`.
     176
     177          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.
     178
     179          See [`tests/phpstan/README.md`][readme] for details.
     180
     181          SUMMARY
     182
     183          {
     184            echo "[baselines]: ${BASELINES_URL}"
     185            echo "[readme]: ${README_URL}"
     186          } >> "${GITHUB_STEP_SUMMARY}"
     187
     188          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}"
     189          exit 1
    97190
    98191      - name: "Save result cache"

  • trunk/composer.json

    r63004 r63019  
    6969        "scripts": {
    7070                "phpstan": "@php ./vendor/bin/phpstan analyse --memory-limit=2G",
     71                "phpstan:baselines": [ "Composer\\Config::disableProcessTimeout", "@php ./tests/phpstan/generate-baselines.php" ],
    7172                "compat": "@php ./vendor/squizlabs/php_codesniffer/bin/phpcs --standard=phpcompat.xml.dist --report=summary,source",
    7273                "format": "@php ./vendor/squizlabs/php_codesniffer/bin/phpcbf --report=summary,source",

  • trunk/package.json

    r62896 r63019  
    142142                "test:e2e": "wp-scripts test-playwright --config tests/e2e/playwright.config.js",
    143143                "test:visual": "wp-scripts test-playwright --config tests/visual-regression/playwright.config.js",
    144                 "typecheck:php": "node ./tools/local-env/scripts/docker.js run --rm php composer phpstan",
     144                "typecheck:php": "node ./tools/local-env/scripts/docker.js run --rm php composer phpstan --",
     145                "typecheck:php:baselines": "node ./tools/local-env/scripts/docker.js run --rm php composer phpstan:baselines --",
    145146                "gutenberg:copy": "node tools/gutenberg/copy.js",
    146147                "gutenberg:verify": "node tools/gutenberg/utils.js",

  • trunk/phpstan.neon.dist

    r62719 r63019  
    1515        - vendor/phpstan/phpstan-phpunit/extension.neon
    1616
    17         # The baseline file includes preexisting errors in the codebase that should be ignored.
     17        # Preexisting errors that should be ignored, one baseline per error identifier
     18        # so that the remaining work on each is visible as a single shrinking file.
     19        # Each is meant to reach zero and be deleted, taking its line below with it.
    1820        # https://phpstan.org/user-guide/baseline
    19         - tests/phpstan/baseline.php
     21        #
     22        # Regenerate with `composer phpstan:baselines`, which rewrites both the files
     23        # and the list between the markers. Do not edit that list by hand.
     24        # phpstan:baselines start
     25        - tests/phpstan/baselines/empty.variable.neon
     26        - tests/phpstan/baselines/isset.variable.neon
     27        - tests/phpstan/baselines/variable.undefined.neon
     28        # phpstan:baselines end
    2029
    2130parameters:
    2231        # https://phpstan.org/user-guide/rule-levels
    23         level: 0
     32        level: 1
    2433        reportUnmatchedIgnoredErrors: true
    2534
     35        # The following ignored errors are not intended to be fixed, as distinct from the baselines
     36        # included above.
     37        #
     38        # A baseline records work still to be done. Every entry in one is in scope to be fixed, and
     39        # each file is meant to reach zero and then be deleted. An entry here is the opposite: a
     40        # decision that the code is right as written and the report is not actionable, whether
     41        # because PHPStan cannot see what makes the code safe, or because satisfying it would mean
     42        # changing code that has no other reason to change.
     43        #
     44        # So prefer fixing an error, and baseline it when it cannot be fixed yet. Add it here only
     45        # when it should never be fixed, and say why.
    2646        ignoreErrors:
    2747                # Level 0:
    … …  
    4161                        path: src/wp-includes/canonical.php
    4262                        count: 1
     63
    4364                # Level 2:
    4465                # ValueError is PHP 8.0+; core throws it conditionally so the docblocks are correct for WP's 7.4+ range,

  • trunk/tests/phpstan/README.md

    r62939 r63019  
    3939composer run phpstan -- -vvv --debug
    4040```
     41
     42Note the `--` in each of those. Composer needs it in order to pass the flags on to PHPStan rather than reading them as its own, and without it they are discarded silently. The npm script supplies it, which is why only one is needed there.
    4143
    4244For available flags, see https://phpstan.org/user-guide/command-line-usage.
    … …  
    9294- Adding the error pattern to the `ignoreErrors` section of the `phpstan.neon.dist` configuration file. This should be used to handle conflicts with WordPress Coding Standards or similar project decisions, or to allowlist legacy code that is not worth refactoring solely to satisfy the tests.
    9395
    94 - Adding an error to the "tech debt" baseline. This should be used for code that needs to be addressed eventually - by fixing, refactoring, or ignoring via one of the above methods - but is not worth addressing right now.
     96- Adding an error to a "tech debt" baseline. This should be used for code that needs to be addressed eventually - by fixing, refactoring, or ignoring via one of the above methods - but is not worth addressing right now.
    9597
    9698        Baselines are a useful triage tool for handling PHPStan errors in legacy code, as they allow us to enforce stricter code quality checks on new code, while gradually chipping away at the existing issues over time. **Avoid adding PHPStan errors from new code whenever possible, and use baselines as a last resort.**
    9799
    98         The baseline file is located at `tests/phpstan/baseline.php` and generated by running PHPStan with the `--generate-baseline` flag:
     100### How the baselines are organized
    99101
    100         ```bash
    101         npm run typecheck:php -- --generate-baseline=tests/phpstan/baseline.php
     102The baselines live in [`baselines/`](baselines), one file per error identifier, such as `variable.undefined.neon`. Splitting them this way keeps each kind of error visible as a single file that should shrink to nothing and then be deleted, rather than as part of one large file in which every kind is mixed together.
    102103
    103         # or, with Composer directly:
    104         composer run phpstan -- --generate-baseline=tests/phpstan/baseline.php
    105         ```
     104Every entry is scoped to the file the error occurs in and carries an exact occurrence count:
    106105
    107         This will regenerate the baseline file with any new errors added to the existing ones. You can then commit the updated baseline file.
     106```neon
     107-
     108        message: '#^Variable \$wpdb might not be defined\.$#'
     109        identifier: variable.undefined
     110        count: 4
     111        path: ../../../src/wp-trackback.php
     112```
     113
     114Both the path and the count matter. A new occurrence of an already baselined error does not match the entry, even in a file that is already listed, and is reported as a new error. That is the point of recording them this way: the baselines describe exactly what exists today, so nothing new slips in behind them.
     115
     116The consequence is that **fixing a baselined error means regenerating its baseline as part of the same change**, because the count no longer matches. A count that no longer matches is reported as an `ignore.count` error, which PHPStan does not allow to be ignored or baselined.
     117
     118### Regenerating the baselines
     119
     120The baselines are generated, and should not be edited by hand. Regenerate them with:
     121
     122```bash
     123npm run typecheck:php:baselines
     124```
     125
     126which will run the generator in the Docker container.
     127
     128As with the analysis itself, flags are passed by adding `--` followed by the flags themselves:
     129
     130```bash
     131# a single identifier:
     132npm run typecheck:php:baselines -- --identifier=variable.undefined
     133
     134# several, either comma separated or by repeating the option:
     135npm run typecheck:php:baselines -- --identifier=variable.undefined,isset.variable
     136npm run typecheck:php:baselines -- --identifier=isset.variable --identifier=empty.variable
     137
     138# print every error as one baseline, writing nothing:
     139npm run typecheck:php:baselines -- --combined
     140
     141# the remaining options:
     142npm run typecheck:php:baselines -- --help
     143```
     144
     145If you are not using the Docker environment, you can run the generator via Composer directly:
     146
     147```bash
     148composer phpstan:baselines
     149
     150composer phpstan:baselines -- --identifier=variable.undefined
     151composer phpstan:baselines -- --combined
     152composer phpstan:baselines -- --help
     153```
     154
     155Note the `--` in each of those. Composer needs it in order to pass the flags on to the script rather than reading them as its own, and without it they are discarded silently, so `composer phpstan:baselines --identifier=variable.undefined` regenerates every baseline rather than that one. The npm script supplies it, which is why only one is needed there.
     156
     157A run also deletes any baseline whose identifier no longer reports anything, and rewrites the list of them between the `# phpstan:baselines` markers in the `includes` of [`phpstan.neon.dist`](../../phpstan.neon.dist) to match. Adding a newly split out baseline, and retiring one that has reached zero, therefore need no edit of the configuration.
     158
     159PHPStan's own `--generate-baseline` is deliberately not used directly. It captures every error a run reports, with no way to restrict it to one identifier, so it cannot refresh a single baseline without sweeping every other kind of error into it.
    108160
    109161## Performance and troubleshooting

  • trunk/tests/phpstan/base.neon

    r62939 r63019  
    7676                - AUTH_SALT
    7777                - AUTOMATIC_UPDATER_DISABLED
     78                - BACKGROUND_COLOR
     79                - BACKGROUND_IMAGE
    7880                - COOKIEPATH
    7981                - CUSTOM_TAGS
    … …  
    8385                - ENFORCE_GZIP
    8486                - FORCE_SSL_LOGIN
     87                - HEADER_IMAGE
     88                - HEADER_IMAGE_HEIGHT
     89                - HEADER_IMAGE_WIDTH
     90                - HEADER_TEXTCOLOR
    8591                - MEDIA_TRASH
    8692                - MULTISITE
     93                - NO_HEADER_TEXT
    8794                - NOBLOGREDIRECT
    8895                - SAVEQUERIES

  • trunk/tests/phpstan/bootstrap.php

    r62965 r63019  
    104104define( 'FS_CHMOD_DIR', 0755 );
    105105define( 'FS_CHMOD_FILE', 0644 );
     106
     107/** @see add_theme_support() */
     108define( 'NO_HEADER_TEXT', false );
     109define( 'HEADER_IMAGE_WIDTH', 0 );
     110define( 'HEADER_IMAGE_HEIGHT', 0 );
     111define( 'HEADER_TEXTCOLOR', '' );
     112define( 'HEADER_IMAGE', '' );
     113define( 'BACKGROUND_COLOR', '' );
     114define( 'BACKGROUND_IMAGE', '' );
Note: See TracChangeset for help on using the changeset viewer.