Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tools: Add and configure PHPStan static analysis #66693

Draft
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

justlevine
Copy link
Contributor

What?

This PR adds a PHPStan configuration along with error baselines though Level 7.

PHPStan errors are not remediated in this PR.

Why?

Mirrors efforts in https://core.trac.wordpress.org/ticket/61175

Error baselines can be used as a list of preexisting tech debt that can be remediated independently of this PR. Baselines go up to PHPStan Level 7, as Level 8 baselines currently conflict with previous levels.

Once a decision is made regarding the PHPStan Level to target, the baselines can be merged/adjusted.

Part of #66598

How?

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 tools/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
    composer analyse --generate-baseline tools/phpstan/baseline/level-{%N}.php 

Testing Instructions

  1. Run composer analyse and confirm PHPStan passes.
  2. Copy phpstan.neon.dist to phpstan.neon, comment out one of the baseline includes:, rerun composer analyse and confirm PHPStan fails.

Todo

( PRs open. )

  • Implement NPM scripts
  • Implement GH Workflow
  • Handle missing gutenberg_*() functions and *_Gutenberg classes
  • Triage baselines for good ignoreErrors candidates.

Copy link

github-actions bot commented Nov 2, 2024

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @justlevine! In case you missed it, we'd love to have you join us in our Slack community.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Nov 2, 2024

ignoreErrors:
# @TODO: need to discover these symbols.
- '#Function gutenberg_.* not found.#'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

E.g. gutenberg_style_engine_get_styles(). Only thing close I could find was wp_style_engine_get_styles() in packages/style-engine.

Is there some aliasing or build step search-replace that happens? Am I missing a path in base.neon, or do we need to alias/stub these functions ourselves?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant