Integrate PHPStan into the core development workflow

[westonruter]

​PHPStan is a vital static analysis tool for identifying bugs in code. Previously it has been used to identify problems in core by manually running PHPStan on the core codebase:

• #58852
• #57782
• #51423
• #52217
• #59653

As @johnbillion suggests in #52217:

Consider whether it's beneficial to add PHPStan (or Psalm or Phan) analysis to the build tooling and CI.

I suggest we incorporate PHPStan now into the workflow with a populated ​baseline that captures all of the existing issues and ignores them so that everything doesn't have to be fixed up front. This baseline will then allow new issues to be reported as they are introduced in the codebase, again without having to fix everything up-front.

We can start with either ​rule level 0 or 1 and then go from there as we fix issues in the codebase. It may not make sense to implement the highest levels.

For reference, the Performance team has implemented PHPStan as part of the Performance Lab codebase and there have been separate PRs fixing issues for each level (​1, ​2, ​3, ​4, ​5, ​6, ​7). It is remarkable how effective it is at identifying problems.

PHPStan should be run alongside PHPCS in GHA and locally as part of the pre-commit checks.

[jorbin]

There is a PR in ​https://github.com/WordPress/wordpress-develop/pull/853/files that is stalled that I think could serve as the starting point once the discussions there can come to a satisfactory conclusion.

[oglekler]

We have 1 week before Beta 1, and it looks like patch is not exactly ready. If adding PHPStan itself is not affecting code directly, then changing some default values can. I propose to split PHPStan addition and suggested code fixes.

[oglekler]

We have just a few days before Beta 1, so, I am moving this to the next milestone for further investigation.

[chaion07]

Thanks @westonruter for reporting this. We reviewed this ticket during a recent bug-scrub session.

We received the following feedback:

1. To work closely with the Performance Team for more solutions
2. As some discussions are from 2021 and back, we feel the need for some confirmation

Props to @khoipro for the suggestion

Cheers

[desrosj]

With no meaningful progress during the 6.7 release cycle, I'm goign to punt this one.

[justlevine]

Replying to jorbin:

There is a PR in ​https://github.com/WordPress/wordpress-develop/pull/853/files that is stalled that I think could serve as the starting point once the discussions there can come to a satisfactory conclusion.

I opened up ​https://github.com/johnbillion/wordpress-develop/pull/4 to refresh the above PR. It merges in the latest trunk and updates PHPStan to v1.12.7

I plan to go through the remaining list of exposed issues, and see what can be addressed in isolation towards #52217 and #59653, but am also happy to help advance this ticket directly.

(I'm wondering if this should perhaps be distilled from PR-853, so it only contains config + annotations instead of code changes. Hopefully I'll understand more once I finish reviewing all the PR comments).

Trac ticket: https://core.trac.wordpress.org/ticket/61175

This PR adds a PHPStan configuration along with error baselines through Level 6.

The hope is the limited scope will make this easier to review/merge, with actual code remediations occuring in future (or even parallel) PRs.

## Usage

### Run PHPStan

composer run analyse

# Generate a report
# https://phpstan.org/user-guide/output-format
composer run analyse -- --error-format=checkstyle

### Create a local phpstan.neon for remediating errors

1. Copy phpstan.neon.dist to phpstan.neon.
2. Comment out the unnecessary baselines, change the parameters.level to the desired level, and filter out the errors by adding them to the parameters.ignoreErrors list.
3. Run PHPStan as usual: composer run analyse.

### Remediating errors using the error baselines

While parameters.ignoreErrors is used to filter out "unsupported" errors, error baselines are used to suppress _preexisting_ tech debt.

This allows for the PR to be merged as is to enforce the rules on new code, while allowing contributors to remediate the existing errors at their own pace. This is in the spirit of 'Avoid unnecessary refactoring'.

To remediate a baselined error, remove the error from the tests/phpstan/baseline/level-{%N} file and run PHPStan again.

#### Triaging errors and regenerating baselines

If an error is found to be a false positive (or otherwise not worth fixing), it should be added to the parameters.ignoreErrors list in the phpstan.neon.dist file. When this happens, the baseline file suppressing the error will cause PHPStan to fail.

To avoid manual remediation, the baseline files can be regenerated by following the following steps:

1. Identify the baseline level that contains the error. (Search for the error in tests/phpstan/baseline ).
2. Change the parameters.level in phpstan.neon to the level identified in step 1.
3. Comment out the includes for that level _and all levels above it_.
4. Run the following command:

# Replace {%N} with the level identified in step 1
   vendor/bin/phpstan analyse --generate-baseline tests/phpstan/baseline/level-{%N}.php

## Prior Art

This is based off the work done in #853 with some notable differences:

• Latest trunk and PHPStan (1.12.7) - as of writing.

• No changes to WordPress core codebase.

This is to limit the scope of the PR and hopefuly prevent it from going stale.

• The phpstan.neon.dist file wraps the tests/phpstan/base.neon config that holds the codebase configuration (what to scan/skip, etc), with the top-level file holding the "rules".

This makes it easier for contributors to remediate errors by creating a local phpstan.neon file.

• Removed many of the parameters.ignoreErrors in favor of error baselines (one per each PHPStan level).

See Remediating errors using the error baselines section above.

• The tests/phpstan/bootstrap.php file has been reorganized to mirror the order that the constants are defined in the WP lifecycle.

This will hopefully make it easier to maintain in the future.

• Added inline annotations to the various configs.

## Additional Notes

• PHPStan Level 6 is the highest level that can be baselined without making changes to core code. While, I'd personally recommend baselining at level 8 (with ignoreErrors) - ​Level 9+ is primarily about mixed types - that can be handled incrementally in a future PR that contains the necessary code remediations.

## Next Steps

• [ ] Add a GH Workflow to run PHPStan on PRs.
• [ ] Triage the baselines for good candidates for ignoreErrors.

---

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See ​GitHub Pull Requests for Code Review in the Core Handbook for more details.

[justlevine]

Per @johnbillion 's request (​https://github.com/johnbillion/wordpress-develop/pull/4#issuecomment-2429389292), I opened up a new PR against trunk that focuses solely on PHPStan config and doesn't make any code changes.

​https://github.com/WordPress/wordpress-develop/pull/7619

My next step is re-reviewing the comment history in ​https://github.com/WordPress/wordpress-develop/pull/853 to see if there's any additional obvious candidates to be moved from the baselines to ignoreErrors.

PS: I'm part of the current Mentorship Cohort, so any and feedback or direction (even what might seem "obvious" to you) is much appreciated.

[justlevine]

I've begun remediating errors surfaced by PHPStan in _individual PRs_ against https://core.trac.wordpress.org/ticket/52217 so they can be reviewed independently of implementing the tooling (and decrease the changes of things going stale or getting polluted with merge conflicts).

Will update this comment periodically with links to the specific PRs to avoid continuously spamming everyone who's just here for the tooling. Too time consuming.

• ​https://github.com/WordPress/wordpress-develop/pull/7672
• ​https://github.com/WordPress/wordpress-develop/pull/7673
• ​https://github.com/WordPress/wordpress-develop/pull/7674

[justlevine]

In ​https://github.com/WordPress/wordpress-develop/pull/7619/commits/94cf5e494a1f62055746635d16b1b84e8ee66476, I added wp-includes/blocks to the list of excludePaths.

Per ​https://github.com/WordPress/wordpress-develop/blob/trunk/tools/release/sync-stable-blocks.js, this directory is sourced from WordPress/gutenberg (h/t @thelovekesh 🙇), so we can't remediate them directly.

My understanding is that things are different in Gutenberg land, and as such efforts to introduce PHPStan to Gutenberg code needs to happen separately from this ticket.