Bump PHPStan rule level to 1

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

@@ -18,7 +18,7 @@ on:
       # These files configure PHPStan. Changes could affect the outcome.
       - 'phpstan.neon.dist'
       - 'tests/phpstan/base.neon'
-      - 'tests/phpstan/baseline.php'
+      - 'tests/phpstan/baselines/**'
       # Confirm any changes to relevant workflow files.
       - '.github/workflows/phpstan-static-analysis.yml'
       - '.github/workflows/reusable-phpstan-static-analysis-v1.yml'

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

@@ -32,6 +32,7 @@ jobs:
   # - Builds WordPress.
   # - 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.
   phpstan:
@@ -93,7 +94,99 @@ jobs:
 
       - 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"
         uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5

composer.json

@@ -68,6 +68,7 @@
 	},
 	"scripts": {
 		"phpstan": "@php ./vendor/bin/phpstan analyse --memory-limit=2G",
+		"phpstan:baselines": [ "Composer\\Config::disableProcessTimeout", "@php ./tests/phpstan/generate-baselines.php" ],
 		"compat": "@php ./vendor/squizlabs/php_codesniffer/bin/phpcs --standard=phpcompat.xml.dist --report=summary,source",
 		"format": "@php ./vendor/squizlabs/php_codesniffer/bin/phpcbf --report=summary,source",
 		"lint": "@php ./vendor/squizlabs/php_codesniffer/bin/phpcs --report=summary,source",

package.json

@@ -141,7 +141,8 @@
 		"test:coverage": "npm run test:php -- --coverage-html ./coverage/html/ --coverage-php ./coverage/php/report.php --coverage-text=./coverage/text/report.txt",
 		"test:e2e": "wp-scripts test-playwright --config tests/e2e/playwright.config.js",
 		"test:visual": "wp-scripts test-playwright --config tests/visual-regression/playwright.config.js",
-		"typecheck:php": "node ./tools/local-env/scripts/docker.js run --rm php composer phpstan",
+		"typecheck:php": "node ./tools/local-env/scripts/docker.js run --rm php composer phpstan --",
+		"typecheck:php:baselines": "node ./tools/local-env/scripts/docker.js run --rm php composer phpstan:baselines --",
 		"gutenberg:copy": "node tools/gutenberg/copy.js",
 		"gutenberg:verify": "node tools/gutenberg/utils.js",
 		"gutenberg:download": "node tools/gutenberg/download.js && grunt build:gutenberg"

phpstan.neon.dist

@@ -14,15 +14,35 @@ includes:
 	# new strict rules.
 	- vendor/phpstan/phpstan-phpunit/extension.neon
 
-	# The baseline file includes preexisting errors in the codebase that should be ignored.
+	# Preexisting errors that should be ignored, one baseline per error identifier
+	# so that the remaining work on each is visible as a single shrinking file.
+	# Each is meant to reach zero and be deleted, taking its line below with it.
 	# https://phpstan.org/user-guide/baseline
-	- tests/phpstan/baseline.php
+	#
+	# Regenerate with `composer phpstan:baselines`, which rewrites both the files
+	# and the list between the markers. Do not edit that list by hand.
+	# phpstan:baselines start
+	- tests/phpstan/baselines/empty.variable.neon
+	- tests/phpstan/baselines/isset.variable.neon
+	- tests/phpstan/baselines/variable.undefined.neon
+	# phpstan:baselines end
 
 parameters:
 	# https://phpstan.org/user-guide/rule-levels
-	level: 0
+	level: 1
 	reportUnmatchedIgnoredErrors: true
 
+	# The following ignored errors are not intended to be fixed, as distinct from the baselines
+	# included above.
+	#
+	# A baseline records work still to be done. Every entry in one is in scope to be fixed, and
+	# each file is meant to reach zero and then be deleted. An entry here is the opposite: a
+	# decision that the code is right as written and the report is not actionable, whether
+	# because PHPStan cannot see what makes the code safe, or because satisfying it would mean
+	# changing code that has no other reason to change.
+	#
+	# So prefer fixing an error, and baseline it when it cannot be fixed yet. Add it here only
+	# when it should never be fixed, and say why.
 	ignoreErrors:
 		# Level 0:
 		- # Inner functions aren't supported by PHPStan.
@@ -40,6 +60,7 @@ parameters:
 			identifier: function.inner
 			path: src/wp-includes/canonical.php
 			count: 1
+
 		# Level 2:
 		# ValueError is PHP 8.0+; core throws it conditionally so the docblocks are correct for WP's 7.4+ range,
 		# but bleedingEdge's version-aware check treats the class as non-existent against the PHP 7.4 floor.

tests/phpstan/README.md

@@ -39,6 +39,8 @@ composer run phpstan -- src/wp-includes/template.php
 composer run phpstan -- -vvv --debug
 ```
 
+Note 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.
+
 For available flags, see https://phpstan.org/user-guide/command-line-usage.
 
 ## The PHPStan configuration
@@ -91,20 +93,70 @@ PHPStan errors can be ignored in the following ways:
 
 - 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.
 
-- 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.
+- 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.
 
 	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.**
 
-	The baseline file is located at `tests/phpstan/baseline.php` and generated by running PHPStan with the `--generate-baseline` flag:
+### How the baselines are organized
+
+The 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.
+
+Every entry is scoped to the file the error occurs in and carries an exact occurrence count:
+
+```neon
+-
+	message: '#^Variable \$wpdb might not be defined\.$#'
+	identifier: variable.undefined
+	count: 4
+	path: ../../../src/wp-trackback.php
+```
+
+Both 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.
+
+The 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.
+
+### Regenerating the baselines
+
+The baselines are generated, and should not be edited by hand. Regenerate them with:
+
+```bash
+npm run typecheck:php:baselines
+```
+
+which will run the generator in the Docker container.
+
+As with the analysis itself, flags are passed by adding `--` followed by the flags themselves:
+
+```bash
+# a single identifier:
+npm run typecheck:php:baselines -- --identifier=variable.undefined
+
+# several, either comma separated or by repeating the option:
+npm run typecheck:php:baselines -- --identifier=variable.undefined,isset.variable
+npm run typecheck:php:baselines -- --identifier=isset.variable --identifier=empty.variable
+
+# print every error as one baseline, writing nothing:
+npm run typecheck:php:baselines -- --combined
+
+# the remaining options:
+npm run typecheck:php:baselines -- --help
+```
+
+If you are not using the Docker environment, you can run the generator via Composer directly:
+
+```bash
+composer phpstan:baselines
+
+composer phpstan:baselines -- --identifier=variable.undefined
+composer phpstan:baselines -- --combined
+composer phpstan:baselines -- --help
+```
 
-	```bash
-	npm run typecheck:php -- --generate-baseline=tests/phpstan/baseline.php
+Note 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.
 
-	# or, with Composer directly:
-	composer run phpstan -- --generate-baseline=tests/phpstan/baseline.php
-	```
+A 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.
 
-	This will regenerate the baseline file with any new errors added to the existing ones. You can then commit the updated baseline file.
+PHPStan'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.
 
 ## Performance and troubleshooting
 

tests/phpstan/base.neon

@@ -75,15 +75,22 @@ parameters:
 		- ALLOW_SUBDIRECTORY_INSTALL
 		- AUTH_SALT
 		- AUTOMATIC_UPDATER_DISABLED
+		- BACKGROUND_COLOR
+		- BACKGROUND_IMAGE
 		- COOKIEPATH
 		- CUSTOM_TAGS
 		- DISALLOW_FILE_EDIT
 		- DISALLOW_UNFILTERED_HTML
 		- EMPTY_TRASH_DAYS
 		- ENFORCE_GZIP
 		- FORCE_SSL_LOGIN
+		- HEADER_IMAGE
+		- HEADER_IMAGE_HEIGHT
+		- HEADER_IMAGE_WIDTH
+		- HEADER_TEXTCOLOR
 		- MEDIA_TRASH
 		- MULTISITE
+		- NO_HEADER_TEXT
 		- NOBLOGREDIRECT
 		- SAVEQUERIES
 		- SCRIPT_DEBUG

tests/phpstan/baselines/empty.variable.neon

@@ -0,0 +1,65 @@
+# PHPStan baseline for the `empty.variable` errors in WordPress core.
+#
+# https://phpstan.org/error-identifiers/empty.variable
+#
+# Each entry is scoped to a single file and carries an exact occurrence count,
+# so that a new instance is reported as a new error rather than being absorbed
+# silently. Fixing an occurrence therefore means decrementing or removing its
+# entry here as part of the same change.
+#
+# The goal is to empty this file and delete it, along with the `includes` entry
+# for it in phpstan.neon.dist.
+#
+# Generated by `composer phpstan:baselines`. Do not edit by hand; regenerate with
+#
+#     composer phpstan:baselines -- --identifier=empty.variable
+#
+# which reruns the analysis with this file suppressed so the errors surface again.
+
+parameters:
+	ignoreErrors:
+		-
+			message: '#^Variable \$class in empty\(\) always exists and is always falsy\.$#'
+			identifier: empty.variable
+			count: 1
+			path: ../../../src/wp-admin/includes/class-wp-posts-list-table.php
+		-
+			message: '#^Variable \$title in empty\(\) always exists and is always falsy\.$#'
+			identifier: empty.variable
+			count: 1
+			path: ../../../src/wp-admin/includes/plugin.php
+		-
+			message: '#^Variable \$parent_file in empty\(\) always exists and is not falsy\.$#'
+			identifier: empty.variable
+			count: 1
+			path: ../../../src/wp-admin/themes.php
+		-
+			message: '#^Variable \$addl_path in empty\(\) always exists and is always falsy\.$#'
+			identifier: empty.variable
+			count: 1
+			path: ../../../src/wp-includes/canonical.php
+		-
+			message: '#^Variable \$search in empty\(\) always exists and is not falsy\.$#'
+			identifier: empty.variable
+			count: 1
+			path: ../../../src/wp-includes/class-wp-query.php
+		-
+			message: '#^Variable \$status_type_clauses in empty\(\) always exists and is not falsy\.$#'
+			identifier: empty.variable
+			count: 1
+			path: ../../../src/wp-includes/class-wp-query.php
+		-
+			message: '#^Variable \$schema in empty\(\) is never defined\.$#'
+			identifier: empty.variable
+			count: 1
+			path: ../../../src/wp-includes/rest-api/endpoints/class-wp-rest-attachments-controller.php
+		-
+			message: '#^Variable \$the_parent in empty\(\) always exists and is not falsy\.$#'
+			identifier: empty.variable
+			count: 1
+			path: ../../../src/wp-includes/taxonomy.php
+		-
+			message: '#^Variable \$old_user_data in empty\(\) always exists and is not falsy\.$#'
+			identifier: empty.variable
+			count: 1
+			path: ../../../src/wp-includes/user.php