Build/Test Tools: Update PHPStan to 2.3.5 and document --fix

.github/workflows/phpstan-pull-request-comments.yml

@@ -261,21 +261,21 @@ jobs:
                     '> [!IMPORTANT]',
                     '> **The PHPStan baselines are out of date, so the PHPStan check fails.**',
                     '>',
-                    '> An error recorded in a baseline no longer occurs, or occurs a different number of times. If this pull request fixed it, that is expected: nothing needs to change in the code, but each baseline entry records an exact count for a specific file, and the baselines need to catch up. Regenerate them and commit the result:',
+                    '> An error recorded in a baseline no longer occurs, or occurs a different number of times. If this pull request fixed it, that is expected: nothing needs to change in the code, but each baseline entry records an exact count for a specific file, and the baselines need to catch up. Remove the stale entries by running the analysis with `--fix`, and commit the result:',
                     '>',
                     '> ```bash',
-                    '> npm run typecheck:php:baselines',
+                    '> npm run typecheck:php -- --fix',
                     '> ```',
                     '>',
                     '> or, outside the Docker environment:',
                     '>',
                     '> ```bash',
-                    '> composer phpstan:baselines',
+                    '> composer phpstan -- --fix',
                     '> ```',
                     '>',
-                    '> Where an error below names an `@phpstan-ignore` comment in the code rather than a baseline entry, remove that comment instead; regenerating will not clear it.',
+                    '> This also removes any `@phpstan-ignore` comment in the code that no longer ignores anything.',
                     '>',
-                    `> See [Regenerating the baselines](${ readmeUrl }#regenerating-the-baselines) for details.`,
+                    `> See [Removing stale entries with \`--fix\`](${ readmeUrl }#removing-stale-entries-with---fix) for details, and for regenerating the baselines instead.`,
                   ].join( '\n' ) );
                 } else {
                   sections.push( [

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

@@ -150,16 +150,20 @@ jobs:
                   identifier: message.identifier ?? null,
                   tip: message.tip ?? null,
                   ignorable: message.ignorable,
+                  fixable: message.fixable ?? false,
                 } );
               }
             }
 
-            // Errors that are not specific to a file, such as an `ignoreErrors` entry without a
-            // path that no longer matches. These cannot be ignored either.
+            // Errors that are not specific to a file, such as an `ignoreErrors` entry from a config
+            // file PHPStan cannot locate it in. These cannot be ignored either.
             for ( const message of report.errors ) {
-              errors.push( { file: null, line: null, message, identifier: null, tip: null, ignorable: false } );
+              errors.push( { file: null, line: null, message, identifier: null, tip: null, ignorable: false, fixable: false } );
             }
 
+            // PHPStan marks an error it can fix itself, such as a stale baseline entry, as fixable.
+            const fixHint = 'This can be fixed automatically by running `composer phpstan -- --fix` (or `npm run typecheck:php -- --fix`).';
+
             for ( const error of errors ) {
               const properties = { title: error.identifier ? `PHPStan: ${ error.identifier }` : 'PHPStan' };
               if ( error.file ) {
@@ -168,15 +172,16 @@ jobs:
               if ( error.line ) {
                 properties.startLine = error.line;
               }
-              const message = error.tip ? `${ error.message }\n${ error.tip }` : error.message;
+              const message = [ error.message, error.tip, error.fixable ? fixHint : null ].filter( Boolean ).join( '\n' );
               if ( error.ignorable ) {
                 core.warning( message, properties );
               } else {
                 core.error( message, properties );
               }
             }
 
-            // Tips are left out of the results, since they are only shown in the annotations, next to the code.
+            // Tips and whether an error is fixable are left out of the results, since they are only shown in the
+            // annotations, next to the code.
             //
             // The commit analyzed is the pull request merged into its base branch, whose second parent is the head
             // commit. The files with errors that differ between the two are those the base branch has changed, in
@@ -196,7 +201,7 @@ jobs:
               fs.writeFileSync( `${ process.env.RUNNER_TEMP }/phpstan-results/results.json`, JSON.stringify( {
                 pull_request: context.payload.pull_request.number,
                 changed_in_base: changedInBase,
-                errors: errors.map( ( { tip, ...error } ) => error ),
+                errors: errors.map( ( { tip, fixable, ...error } ) => error ),
               } ) );
             }
 
@@ -266,21 +271,21 @@ jobs:
 
           **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:
+          Remove the stale entries by running the analysis with `--fix`, and commit the result:
 
           ```bash
-          npm run typecheck:php:baselines
+          npm run typecheck:php -- --fix
           ```
 
           or, outside the Docker environment:
 
           ```bash
-          composer phpstan:baselines
+          composer phpstan -- --fix
           ```
 
-          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`.
+          That lowers the counts in the files under [`tests/phpstan/baselines`][baselines], removes the entries that no longer match, and removes any `@phpstan-ignore` annotation in the code that no longer ignores anything.
 
-          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.
+          Alternatively, regenerate the baselines with `npm run typecheck:php:baselines` or `composer phpstan:baselines`. That also deletes any baseline whose errors are now all fixed, and updates the list of them in `phpstan.neon.dist`, but it does not clear an `@phpstan-ignore` annotation.
 
           See [`tests/phpstan/README.md`][readme] for details.
 
@@ -294,7 +299,7 @@ jobs:
               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}"
+            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 -- --fix\` or \`composer phpstan -- --fix\` and commit the updated baselines. See ${README_URL}"
           else
             echo "::error title=PHPStan reported errors that cannot be ignored::PHPStan reported ${unignorable} error(s) that cannot be ignored or reported as warnings. See the error annotations for details, and ${README_URL} for how to run the analysis locally."
           fi

composer.json

@@ -52,7 +52,7 @@
 		"squizlabs/php_codesniffer": "3.13.5",
 		"wp-coding-standards/wpcs": "~3.4.1",
 		"phpcompatibility/phpcompatibility-wp": "~2.1.3",
-		"phpstan/phpstan": "2.3.4",
+		"phpstan/phpstan": "2.3.5",
 		"phpstan/phpstan-phpunit": "2.1.1",
 		"phpstan/phpstan-strict-rules": "2.1.1",
 		"yoast/phpunit-polyfills": "^1.1.0"

tests/phpstan/README.md

@@ -50,7 +50,7 @@ The analysis also runs on every pull request that changes PHP files or the PHPSt
 - As annotations on the changed files. Most errors are annotated as warnings, which do not fail the check.
 - In a comment on the pull request, which lists every error in a table along with the commit it was first reported for. The comment is updated as new commits are pushed, moving each error that a commit resolves into a collapsed list of resolved errors, which also names that commit. Once all of the errors are resolved, the comment is marked as resolved and collapsed.
 
-Some errors cannot be ignored, and so cannot be reported as warnings either. These fail the check. The most common is an [out-of-date baseline](#regenerating-the-baselines), which is reported when an error recorded in a baseline was fixed without the baseline being regenerated. Such an error is reported against a file rather than a line in it, so its annotation does not appear alongside the changes, and it would easily go unnoticed if the check passed.
+Some errors cannot be ignored, and so cannot be reported as warnings either. These fail the check. The most common is an [out-of-date baseline](#removing-stale-entries-with---fix), which is reported when an error recorded in a baseline was fixed without the baseline being updated. Such an error is reported against a file rather than a line in it, so its annotation does not appear alongside the changes, and it would easily go unnoticed if the check passed.
 
 ## The PHPStan configuration
 
@@ -216,11 +216,36 @@ Every entry is scoped to the file the error occurs in and carries an exact occur
 
 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.
+The consequence is that **fixing a baselined error means updating 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, and an entry that matches nothing at all as an `ignore.unmatched` error, neither of which PHPStan allows to be ignored or baselined. Either [remove the stale entries with `--fix`](#removing-stale-entries-with---fix), or [regenerate the baselines](#regenerating-the-baselines).
+
+### Removing stale entries with `--fix`
+
+PHPStan marks an error it can fix automatically with 🔧, and ends its report by saying how many there are. Stale ignores are among them: a baseline entry whose count is now too high, or that matches nothing at all, and an `@phpstan-ignore` comment that no longer ignores anything. Run the analysis with `--fix` to fix them in place:
+
+```bash
+npm run typecheck:php -- --fix
+```
+
+or, outside the Docker environment:
+
+```bash
+composer phpstan -- --fix
+```
+
+This lowers the count of the entry, or removes the entry, leaving the rest of the baseline as it was. It is usually much faster than regenerating, since it reuses the result cache of the run that reported the errors. Unlike regenerating, it also removes an `@phpstan-ignore` comment that no longer matches.
+
+Keep in mind:
+
+- **Run it on the whole tree.** PHPStan only reports stale ignores when it analyzes every path, so a run limited to some files, such as `composer phpstan -- --fix src/wp-includes/template.php`, fixes none of them.
+- **A baseline it empties is left behind**, as a file with `ignoreErrors: []`. That still passes, but to delete the file and its `includes` entry in `phpstan.neon.dist`, regenerate that one baseline, as described [below](#regenerating-the-baselines), e.g. `composer phpstan:baselines -- --identifier=variable.undefined`.
+- **It fixes every fixable error the run reports**, not only stale ignores. A few other errors can also be fixed automatically, such as a [useless cast](https://phpstan.org/error-identifiers/cast.useless), so check the diff before committing it.
+- `--fix-ignored` goes further and also fixes the fixable errors which a baseline currently ignores. That is a way to work through a baseline, but expect larger diffs.
+
+The feature is marked experimental in PHPStan. See [the PHPStan documentation](https://phpstan.org/user-guide/command-line-usage) for details.
 
 ### Regenerating the baselines
 
-The baselines are generated, and should not be edited by hand. Regenerate them with:
+The baselines are generated, and should not be edited by hand, other than by `--fix` as described [above](#removing-stale-entries-with---fix). Regenerate them with:
 
 ```bash
 npm run typecheck:php:baselines