Tests / Build Scripts: Configure PHPStan levels 1-6 [Exploratory] #7619
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -81,3 +81,25 @@ PHPStan can be resource-intensive, especially on large codebases like WordPress. | |
| PHPStan caches analysis results to speed up subsequent runs. You can see information about the results cache by running `analyse` with the `-vv` or `-vvv` flag. | ||
|
|
||
| Sometimes, due to the lack of type information in legacy code, PHPStan may still struggle to analyze certain parts of the codebase. In such cases, you can use the `--debug` flag to disable caching and see which files are causing issues. | ||
|
|
||
| ## Triage - using `generate-baselines.sh` | ||
|
|
||
| > [!IMPORTANT] | ||
| > This tool is internal to the branch and is not intended for wordpress-core. Use at your own discretion. | ||
|
Member
There was a problem hiding this comment. What does this mean? Internal to the branch? How is this not for core?
Author
There was a problem hiding this comment. Core only has a
Author
There was a problem hiding this comment. IOW once PHPStan level 1 is enforced by core, it doesnt matter if a new error we want to baseline is level 1 or level 0, it's just core "Tech debt" that we're baselining.
Member
There was a problem hiding this comment. I was thinking we'd commit those baselines to core, even up to level 8. Then contributors who want to work on level 8 can do so via their local But let's say we do bump to level 6 now, it would be great to have level-specific baselines to help us as we start to fix the issues incrementally until we reach a point where there are no errors left in a level-specific baseline.
Author
There was a problem hiding this comment.
in that case sure, though I'd recommend a real review of the script first. I feel the likelihood of that getting approved is very low though.
Member
There was a problem hiding this comment. Getting what approved? Level 6? I don't think it will be difficult to do that, if we have the baseline. It means new code will begin to have level 6 issues reported, and we can start fixing the issues and removing them from the baseline(s) concurrently.
Author
There was a problem hiding this comment. I hope you're right, but I'd assume there's some chicken-egg work at foot based on the discussions and feedback from folks before merge. Commiters (assumedly) want to know the impact of the particular level/ruleset before we enforce the requirements on new code. And there's also the level-specific prepwork that needs to be done so that every new PR isn't generating "noise" (more of the same error types that need to be baselined). So yeah I do assume for new-enforcement on new code we'll need to bump level by level, so we can point and say e.g. "these are the new violations that are getting checked, these are (currently) ignored etc" and that the codebase is in a place where we can enforce it and those baselines remain stable.
So this was the scenario I was originally trying to address in my reply. Levels are just shorthand convenience groupings for getting things into a codebase. Once inside the codebase tho, it doesn't matter for remediation if the violation is from a level 1 rule or a level 9, its tech debt that needs to be remediated, so having to maintain multiple lists that nerd to be non-natively generated feels like undesired friction for core. I do see value in tools like https://github.com/staabm/phpstan-baseline-analysis to help us pull out and do rule-based triage of the baselines, but that too doesn't seem like something that should be merged into core. (maybe I have the wrong POV: but I see a difference between committing an evergreen tool/flow versus one thats only a comparative short term triage tool. Like I wouldn't commit a script that e.g. generates all the PHPUnit tests that aren't compatible with v11.5+, or one that generates a report of any non php8.0 compatible code, since one those triage tasks are done the script no longer has value) |
||
|
|
||
| The `generate-baselines.sh` script is a helper tool to generate per-level baseline files for triaging PHPStan errors. It iterates through the specified levels, generates a baseline for each level, and outputs a summary of the results. | ||
|
|
||
| The process relies on iteration, and assumes that the baseline for the previous level has been freshly generated to capture any new/remediated errors on that level before generating the next level's baseline. | ||
|
|
||
| ### Usage | ||
|
|
||
| To run the script, use the following command: | ||
|
|
||
| ```bash | ||
| bash tests/phpstan/generate-baselines.sh --all # to generate baselines for all levels | ||
| bash tests/phpstan/generate-baselines.sh --start=1 --end=3 # to generate baselines for levels 1 to 3 | ||
| bash tests/phpstan/generate-baselines.sh --level=6 # to generate a baseline for level 2 only. ASSUMES that the baseline for level 1 has been freshly generated. | ||
| bash tests/phpstan/generate-baselines.sh --summary-only # to only output the summary of results for the existing baselines, without regenerating them. | ||
|
|
||
| ``` | ||
| To change the max level, update the `MAX_LEVEL` variable at the top of the script, alongside any changes to the root `phpstan.neon.dist` file to add the new level. | ||
|
Member
There was a problem hiding this comment. Couldn't the
Author
There was a problem hiding this comment. Yup, cool idea. feel free to throw some tokens at it if you want or maybe i will next time |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,185 @@ | ||
| <?php declare(strict_types = 1); | ||
|
|
||
| $ignoreErrors = []; | ||
| $ignoreErrors[] = [ | ||
| 'message' => '#^Call to function compact\\(\\) contains possibly undefined variable \\$comment_author\\.$#', | ||
| 'identifier' => 'variable.undefined', | ||
| 'count' => 1, | ||
| 'path' => __DIR__ . '/../../../src/wp-admin/includes/ajax-actions.php', | ||
| ]; | ||
| $ignoreErrors[] = [ | ||
| 'message' => '#^Call to function compact\\(\\) contains possibly undefined variable \\$comment_author_email\\.$#', | ||
| 'identifier' => 'variable.undefined', | ||
| 'count' => 1, | ||
| 'path' => __DIR__ . '/../../../src/wp-admin/includes/ajax-actions.php', | ||
| ]; | ||
| $ignoreErrors[] = [ | ||
| 'message' => '#^Call to function compact\\(\\) contains possibly undefined variable \\$comment_author_url\\.$#', | ||
| 'identifier' => 'variable.undefined', | ||
| 'count' => 1, | ||
| 'path' => __DIR__ . '/../../../src/wp-admin/includes/ajax-actions.php', | ||
| ]; | ||
| $ignoreErrors[] = [ | ||
| 'message' => '#^Call to function compact\\(\\) contains possibly undefined variable \\$user_id\\.$#', | ||
| 'identifier' => 'variable.undefined', | ||
| 'count' => 1, | ||
| 'path' => __DIR__ . '/../../../src/wp-admin/includes/ajax-actions.php', | ||
| ]; | ||
| $ignoreErrors[] = [ | ||
| 'message' => '#^Variable \\$_POST in isset\\(\\) always exists and is not nullable\\.$#', | ||
| 'identifier' => 'isset.variable', | ||
| 'count' => 1, | ||
| 'path' => __DIR__ . '/../../../src/wp-admin/includes/class-custom-image-header.php', | ||
| ]; | ||
| $ignoreErrors[] = [ | ||
| 'message' => '#^Constructor of class WP_Filesystem_Direct has an unused parameter \\$arg\\.$#', | ||
| 'identifier' => 'constructor.unusedParameter', | ||
| 'count' => 1, | ||
| 'path' => __DIR__ . '/../../../src/wp-admin/includes/class-wp-filesystem-direct.php', | ||
| ]; | ||
| $ignoreErrors[] = [ | ||
| 'message' => '#^Variable \\$class in empty\\(\\) always exists and is always falsy\\.$#', | ||
| 'identifier' => 'empty.variable', | ||
| 'count' => 1, | ||
| 'path' => __DIR__ . '/../../../src/wp-admin/includes/class-wp-posts-list-table.php', | ||
| ]; | ||
| $ignoreErrors[] = [ | ||
| 'message' => '#^Variable \\$_POST in isset\\(\\) always exists and is not nullable\\.$#', | ||
| 'identifier' => 'isset.variable', | ||
| 'count' => 1, | ||
| 'path' => __DIR__ . '/../../../src/wp-admin/includes/media.php', | ||
| ]; | ||
| $ignoreErrors[] = [ | ||
| 'message' => '#^Variable \\$parent_file in empty\\(\\) always exists and is not falsy\\.$#', | ||
| 'identifier' => 'empty.variable', | ||
| 'count' => 1, | ||
| 'path' => __DIR__ . '/../../../src/wp-admin/themes.php', | ||
| ]; | ||
| $ignoreErrors[] = [ | ||
| 'message' => '#^Constant HEADER_IMAGE_HEIGHT not found\\.$#', | ||
| 'identifier' => 'constant.notFound', | ||
| 'count' => 1, | ||
| 'path' => __DIR__ . '/../../../src/wp-content/themes/twentyeleven/functions.php', | ||
| ]; | ||
| $ignoreErrors[] = [ | ||
| 'message' => '#^Constant HEADER_IMAGE_WIDTH not found\\.$#', | ||
| 'identifier' => 'constant.notFound', | ||
| 'count' => 1, | ||
| 'path' => __DIR__ . '/../../../src/wp-content/themes/twentyeleven/functions.php', | ||
| ]; | ||
| $ignoreErrors[] = [ | ||
| 'message' => '#^Constant HEADER_TEXTCOLOR not found\\.$#', | ||
| 'identifier' => 'constant.notFound', | ||
| 'count' => 2, | ||
| 'path' => __DIR__ . '/../../../src/wp-content/themes/twentyeleven/functions.php', | ||
| ]; | ||
| $ignoreErrors[] = [ | ||
| 'message' => '#^Constant HEADER_IMAGE_WIDTH not found\\.$#', | ||
| 'identifier' => 'constant.notFound', | ||
| 'count' => 1, | ||
| 'path' => __DIR__ . '/../../../src/wp-content/themes/twentyeleven/header.php', | ||
| ]; | ||
| $ignoreErrors[] = [ | ||
| 'message' => '#^Constant HEADER_IMAGE_WIDTH not found\\.$#', | ||
| 'identifier' => 'constant.notFound', | ||
| 'count' => 1, | ||
| 'path' => __DIR__ . '/../../../src/wp-content/themes/twentyeleven/showcase.php', | ||
| ]; | ||
| $ignoreErrors[] = [ | ||
| 'message' => '#^Function twentyseventeen_edit_link invoked with 1 parameter, 0 required\\.$#', | ||
| 'identifier' => 'arguments.count', | ||
| 'count' => 1, | ||
| 'path' => __DIR__ . '/../../../src/wp-content/themes/twentyseventeen/template-parts/page/content-front-page-panels.php', | ||
| ]; | ||
| $ignoreErrors[] = [ | ||
| 'message' => '#^Function twentyseventeen_edit_link invoked with 1 parameter, 0 required\\.$#', | ||
| 'identifier' => 'arguments.count', | ||
| 'count' => 1, | ||
| 'path' => __DIR__ . '/../../../src/wp-content/themes/twentyseventeen/template-parts/page/content-front-page.php', | ||
| ]; | ||
| $ignoreErrors[] = [ | ||
| 'message' => '#^Function twentyseventeen_edit_link invoked with 1 parameter, 0 required\\.$#', | ||
| 'identifier' => 'arguments.count', | ||
| 'count' => 1, | ||
| 'path' => __DIR__ . '/../../../src/wp-content/themes/twentyseventeen/template-parts/page/content-page.php', | ||
| ]; | ||
| $ignoreErrors[] = [ | ||
| 'message' => '#^Constant HEADER_IMAGE_HEIGHT not found\\.$#', | ||
| 'identifier' => 'constant.notFound', | ||
| 'count' => 1, | ||
| 'path' => __DIR__ . '/../../../src/wp-content/themes/twentyten/functions.php', | ||
| ]; | ||
| $ignoreErrors[] = [ | ||
| 'message' => '#^Constant HEADER_IMAGE_WIDTH not found\\.$#', | ||
| 'identifier' => 'constant.notFound', | ||
| 'count' => 1, | ||
| 'path' => __DIR__ . '/../../../src/wp-content/themes/twentyten/functions.php', | ||
| ]; | ||
| $ignoreErrors[] = [ | ||
| 'message' => '#^Constant HEADER_IMAGE_WIDTH not found\\.$#', | ||
| 'identifier' => 'constant.notFound', | ||
| 'count' => 1, | ||
| 'path' => __DIR__ . '/../../../src/wp-content/themes/twentyten/header.php', | ||
| ]; | ||
| $ignoreErrors[] = [ | ||
| 'message' => '#^Variable \\$addl_path in empty\\(\\) always exists and is always falsy\\.$#', | ||
| 'identifier' => 'empty.variable', | ||
| 'count' => 1, | ||
| 'path' => __DIR__ . '/../../../src/wp-includes/canonical.php', | ||
| ]; | ||
| $ignoreErrors[] = [ | ||
| 'message' => '#^Variable \\$namespace in isset\\(\\) always exists and is not nullable\\.$#', | ||
| 'identifier' => 'isset.variable', | ||
| 'count' => 1, | ||
| 'path' => __DIR__ . '/../../../src/wp-includes/class-wp-block-parser.php', | ||
| ]; | ||
| $ignoreErrors[] = [ | ||
| 'message' => '#^Variable \\$block_type in empty\\(\\) always exists and is not falsy\\.$#', | ||
| 'identifier' => 'empty.variable', | ||
| 'count' => 1, | ||
| 'path' => __DIR__ . '/../../../src/wp-includes/class-wp-block-supports.php', | ||
| ]; | ||
| $ignoreErrors[] = [ | ||
| 'message' => '#^Variable \\$loader in isset\\(\\) always exists and is not nullable\\.$#', | ||
| 'identifier' => 'isset.variable', | ||
| 'count' => 1, | ||
| 'path' => __DIR__ . '/../../../src/wp-includes/class-wp-oembed.php', | ||
| ]; | ||
| $ignoreErrors[] = [ | ||
| 'message' => '#^Variable \\$search in empty\\(\\) always exists and is not falsy\\.$#', | ||
| 'identifier' => 'empty.variable', | ||
| 'count' => 1, | ||
| 'path' => __DIR__ . '/../../../src/wp-includes/class-wp-query.php', | ||
| ]; | ||
| $ignoreErrors[] = [ | ||
| 'message' => '#^Variable \\$status_type_clauses in empty\\(\\) always exists and is not falsy\\.$#', | ||
| 'identifier' => 'empty.variable', | ||
| 'count' => 1, | ||
| 'path' => __DIR__ . '/../../../src/wp-includes/class-wp-query.php', | ||
| ]; | ||
| $ignoreErrors[] = [ | ||
| 'message' => '#^Variable \\$deprecated in empty\\(\\) always exists and is always falsy\\.$#', | ||
| 'identifier' => 'empty.variable', | ||
| 'count' => 1, | ||
| 'path' => __DIR__ . '/../../../src/wp-includes/pluggable.php', | ||
| ]; | ||
| $ignoreErrors[] = [ | ||
| 'message' => '#^Variable \\$schema in empty\\(\\) is never defined\\.$#', | ||
| 'identifier' => 'empty.variable', | ||
| 'count' => 1, | ||
| 'path' => __DIR__ . '/../../../src/wp-includes/rest-api/endpoints/class-wp-rest-attachments-controller.php', | ||
| ]; | ||
| $ignoreErrors[] = [ | ||
| 'message' => '#^Variable \\$the_parent in empty\\(\\) always exists and is not falsy\\.$#', | ||
| 'identifier' => 'empty.variable', | ||
| 'count' => 1, | ||
| 'path' => __DIR__ . '/../../../src/wp-includes/taxonomy.php', | ||
| ]; | ||
| $ignoreErrors[] = [ | ||
| 'message' => '#^Variable \\$s in isset\\(\\) is never defined\\.$#', | ||
| 'identifier' => 'isset.variable', | ||
| 'count' => 1, | ||
| 'path' => __DIR__ . '/../../../src/wp-includes/template.php', | ||
| ]; | ||
|
|
||
| return ['parameters' => ['ignoreErrors' => $ignoreErrors]]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@justlevine How were these generated? It seems
level-2.phpomits the errors from level 1? Andlevel-3.phpomits the errors from level 2, and so on? When I runcomposer phpstan -- --level=3 --generate-baseline=tests/phpstan/baseline/level-3.phpit includes errors from levels 1-3.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah you need to do it level by level. I've been doing it manually but hold on ai should be able to bash it for us.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should make a script that does that for us and register it as a new composer command. That will be super useful going forward.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, that's what you did? 😅
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup 😇
written by zai-coding-plan/glm-5, reviewed by openai/gpt-5.2-codex (both in opencode), tested by github-copilot/gpt-5-mini (Vscode)
"lightly reviewed" (aka skimmed for nothing crazy) by myself, and manually tested in wsl2 ubuntu24.04
Keep in mind that it iterates levels, so doing
npm run test/phpstan/generate-baselines.sh --allwill take a few minutes, since there's no caches. More in the update doc. NOT MEANT FOR CORE MERGE just a fun toolThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should have something merged into core for for future reuse.
Also, I think writing it in PHP would be more compatible across systems when it is used.