Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions .github/workflows/phpstan-pull-request-comments.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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, 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 deletes any baseline left with no entries, and 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( [
Expand Down
29 changes: 17 additions & 12 deletions .github/workflows/reusable-phpstan-static-analysis-v1.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 ) {
Expand All @@ -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
Expand All @@ -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 ),
} ) );
}

Expand Down Expand Up @@ -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, 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 runs PHPStan with `--fix`, which 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. It then deletes any baseline left with no entries, 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.
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.
Comment thread
westonruter marked this conversation as resolved.
Resolved

See [`tests/phpstan/README.md`][readme] for details.

Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@
"scripts": {
"phpstan": "@php ./vendor/bin/phpstan analyse --memory-limit=2G",
"phpstan:baselines": [ "Composer\\Config::disableProcessTimeout", "@php ./tests/phpstan/generate-baselines.php" ],
"phpstan:fix": [ "Composer\\Config::disableProcessTimeout", "@phpstan --configuration=phpstan.neon.dist --fix", "@phpstan:baselines --prune", "@phpstan --configuration=phpstan.neon.dist" ],
"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",
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@
"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:baselines": "node ./tools/local-env/scripts/docker.js run --rm php composer phpstan:baselines --",
"typecheck:php:fix": "node ./tools/local-env/scripts/docker.js run --rm php composer phpstan:fix --",
"gutenberg:copy": "node tools/gutenberg/copy.js",
"gutenberg:verify": "node tools/gutenberg/utils.js",
"gutenberg:download": "node tools/gutenberg/download.js && npm run build:gutenberg"
Expand Down
39 changes: 36 additions & 3 deletions tests/phpstan/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -216,11 +216,40 @@ 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. Fix them in place with:

```bash
npm run typecheck:php:fix
```

or, outside the Docker environment:

```bash
composer phpstan:fix
```

This runs three steps, each only when the one before it succeeds:

1. PHPStan with its `--fix` option, which 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.
2. `composer phpstan:baselines -- --prune`, which deletes any baseline left with no entries, along with its `includes` entry in `phpstan.neon.dist`. Removing the last entry of a baseline leaves the file behind, as `ignoreErrors: []`. Pruning analyzes nothing, so it can also be run on its own.
3. The analysis once more, which reuses the result cache and so is quick. PHPStan's `--fix` reports only what it fixed, and succeeds even when other errors remain, so this is what shows the errors that are left.

Keep in mind:

- **It fails when nothing is fixable.** PHPStan then says `No fixable errors found` and stops the script there, so run `composer phpstan` to see whether any errors remain.
- **It always analyzes the whole tree with `phpstan.neon.dist`**, whatever a local `phpstan.neon` says. PHPStan only reports stale ignores when it analyzes every path, and a configuration at another rule level can report a baselined error with a different message, in which case `--fix` would remove an entry that the default configuration still needs.
- **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.
- PHPStan's `--fix-ignored`, run as `composer phpstan -- --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, and follow it with `composer phpstan:baselines -- --prune`.

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
Expand All @@ -241,6 +270,9 @@ npm run typecheck:php:baselines -- --identifier=isset.variable --identifier=empt
# print every error as one baseline, writing nothing:
npm run typecheck:php:baselines -- --combined

# without analyzing anything, delete the baselines that list no entries:
npm run typecheck:php:baselines -- --prune

# the remaining options:
npm run typecheck:php:baselines -- --help
```
Expand All @@ -252,6 +284,7 @@ composer phpstan:baselines

composer phpstan:baselines -- --identifier=variable.undefined
composer phpstan:baselines -- --combined
composer phpstan:baselines -- --prune
composer phpstan:baselines -- --help
```

Expand Down
Loading
Loading