wordpress-develop icon indicating copy to clipboard operation
wordpress-develop copied to clipboard

Tests / Build Scripts: Configure PHPStan levels 1-6 [Exploratory]

Open justlevine opened this issue 1 year ago • 7 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

  • [x] 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 avatar Oct 22 '24 21:10 justlevine