Add PHPStan checks by kasparsd · Pull Request #382 · fairpm/fair-plugin

kasparsd · 2025-12-15T11:47:00Z

Fixes #376.

Add szepeviktor/phpstan-wordpress (includes both phpstan and the WP core stubs) at v2.0 which supports the lowest supported PHP version 7.4 by the plugin.
Update the existing coding standard CI checks to run the PHPStan checks inside the same predictable wp-env environment. This is part of larger strategy of running all CI tasks using the same tooling as used for local development (wp-env in this case instead of a soup of bash scripts that rely on global mysql installs, etc.)
Update contributor documentation with all of the available helper scripts and their usage. Explain how to configure specific WP and PHP versions.

github-actions · 2025-12-15T11:47:09Z

kasparsd · 2025-12-15T11:50:34Z

        with:
-          show-progress: ${{ runner.debug == '1' && 'true' || 'false' }}
+          key: ${{ runner.os }}-${{ hashFiles('composer.json') }} # Note that lock file will change between runs.
+          path: .cache


This is where both phpcs and phpstan store their caches.

kasparsd · 2025-12-15T11:52:03Z

-      - name: Make Composer packages available globally
-        run: echo "${PWD}/vendor/bin" >> $GITHUB_PATH
+      - name: Start the development environment
+        run: npm run env start


Run the checks inside the same development environment as used locally. Ensures consistent results across local dev environments and the CI runs.

I've tested this with updated PHPUnit runs and they take the same ~1 min. as the runs currently.

I'm rather 👎 on wp-env in general: it's running Node to run Docker to run PHP, when PHP directly will do fine in a controlled environment like a github runner. This is a wordpress plugin, so PHP is going to be a dev requirement regardless, and phpstan's results are going to be identical across OS distributions.

It looks like we're already using wp-env anyway, so if there's no impact on runner time, I don't have a problem either way. It just seems far more convoluted than it has to be.

kasparsd · 2025-12-15T11:53:52Z

-        run: phpcs . -n --report-full
+      - name: Lint PHPCS
+        # TODO: Remove the flag once all warnings are fixed.
+        run: npm run lint:php:phpcs -- -- -- --warning-severity=0


There are several existing warnings and errors reported (not sure why previously they didn't come up). I chose to not fix them as part of this pull request to make merging this easier.

FILE: inc/packages/admin/info.php ------------------------------------------------------------------------------------------------ FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE ------------------------------------------------------------------------------------------------ 498 | WARNING | Strings should not be wrapped in HTML (WordPress.WP.I18n.NoHtmlWrappedStrings) ------------------------------------------------------------------------------------------------ FILE: inc/packages/admin/namespace.php ------------------------------------------------------------------------------------------------------------------------------------ FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE ------------------------------------------------------------------------------------------------------------------------------------ 502 | WARNING | strip_tags() is discouraged. Use the more comprehensive wp_strip_all_tags() instead. | | (WordPress.WP.AlternativeFunctions.strip_tags_strip_tags) ------------------------------------------------------------------------------------------------------------------------------------ FILE: inc/packages/namespace.php ------------------------------------------------------------------------------------------------------------------------------------ FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE ------------------------------------------------------------------------------------------------------------------------------------ 627 | WARNING | strip_tags() is discouraged. Use the more comprehensive wp_strip_all_tags() instead. | | (WordPress.WP.AlternativeFunctions.strip_tags_strip_tags) ------------------------------------------------------------------------------------------------------------------------------------ FILE: inc/site-health/namespace.php ------------------------------------------------------------------------------------------------------------------------------------ FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE ------------------------------------------------------------------------------------------------------------------------------------ 1 | WARNING | A file should declare new symbols (classes, functions, constants, etc.) and cause no other side effects, or it | | should execute logic with side effects, but should not do both. The first symbol is defined on line 13 and the first | | side effect is on line 110. (PSR1.Files.SideEffects.FoundWithSymbols) ------------------------------------------------------------------------------------------------------------------------------------ Time: 241ms; Memory: 16MB

kasparsd · 2025-12-15T11:54:44Z

@@ -0,0 +1,109 @@
+parameters:


To avoid making changes outside of adding the phpstan tooling to this pull request, we generate a baseline of "known issues" which can be fixed in follow-up pull requests.

kasparsd · 2025-12-15T11:55:29Z

-		"format": "@php ./vendor/bin/phpcbf .",
+        "lint:phpcs": "phpcs .",
+        "lint:phpstan": "phpstan analyse --verbose --memory-limit=2G",
+        "lint": [


We keep the existing composer lint as the combined task and introduce individual scripts for just phpcs and phpstan for convenience.

kasparsd · 2025-12-15T11:57:19Z

-# Stop the development server
-$ npm run env stop
-```
+- `npm run lint:php:phpcs` to run PHPCS (configured in [`phpcs.xml.dist`](phpcs.xml.dist)).


Documented a bunch of existing scripts along with pointers to their configuration.

Added instructions for overriding PHP and WP versions locally for running the tooling with any kind of setup. Also useful when debugging version-specific bugs and issues.

kasparsd · 2025-12-15T11:57:57Z

 	</rule>

+	<!-- Cache for PHPStan and PHPCS -->
+	<exclude-pattern>.cache</exclude-pattern>


These were being caught by the phpcs.

kasparsd · 2025-12-15T11:58:17Z

@@ -0,0 +1,10 @@
+includes:
+	- tests/phpstan-baseline.neon # TODO: Fix all these issues.


This can be removed once we've fixed all the reported issues.

Signed-off-by: Kaspars Dambis <hi@kaspars.net>

kasparsd · 2026-01-04T12:03:32Z

@toderash Noticed this was moved to icebox. I'm not familiar with the triage/planning lingo for the project. Could you please confirm if there is any interest in merging this (or solving the associated #376) in near term? Or if there is anything required to get this merged?

I have a other follow-up pull requests ready that would fix #383, for example, and it relies on the cleanup and documentation updates in this pull request. Wanted to get a sense of when and how that could happen, if at all. Thanks!

kasparsd · 2026-01-04T12:06:09Z

Noting that #394 would have been caught by phpstan if I'm not mistaken.

chuckadams · 2026-04-06T17:38:01Z

-      - name: Make Composer packages available globally
-        run: echo "${PWD}/vendor/bin" >> $GITHUB_PATH
+      - name: Start the development environment
+        run: npm run env start


I'm rather 👎 on wp-env in general: it's running Node to run Docker to run PHP, when PHP directly will do fine in a controlled environment like a github runner. This is a wordpress plugin, so PHP is going to be a dev requirement regardless, and phpstan's results are going to be identical across OS distributions.

It looks like we're already using wp-env anyway, so if there's no impact on runner time, I don't have a problem either way. It just seems far more convoluted than it has to be.

chuckadams · 2026-04-06T17:52:59Z

-        run: phpcs . -n --report-full
+      - name: Lint PHPCS
+        # TODO: Remove the flag once all warnings are fixed.
+        run: npm run lint:php:phpcs -- -- -- --warning-severity=0


run: npm run lint:php:phpcs -- -- -- --warning-severity=0

Are there really supposed to be three occurrences of -- here? ~~Mainly though, if we have a baseline file, we shouldn't need to bring down the severity, no?~~ (edit: nvm I see this was for phpcs, not phpstan)

chuckadams · 2026-04-06T18:19:00Z

I'm all for phpstan, but I refuse to deal with wp-env anymore:

❯ npm run lint:php:phpcs

> lint:php:phpcs
> wp-env run cli --env-cwd=wp-content/plugins/plugin composer run lint:phpcs

ℹ Starting 'composer run lint:phpcs' on the cli container. 

open /Users/chuck/.wp-env/ea95b44cf8f0dbec465691910de921ef/docker-compose.yml: no such file or directory
✖ Command failed with exit code 1
Command failed with exit code 1

composer run lint:phpstan does work fine though.

chuckadams · 2026-04-06T18:25:23Z

In the interests of getting this merged before 1.4 release (even though it's a dev feature and the release doesn't matter) I'm going to patch this PR up myself so that the GH action does not use wp-env, then we can merge the rest to add the phpstan checks. While an individual CI run might not have added much overhead, we have a large test matrix that magnifies the extra overhead of Node+Docker significantly.

Signed-off-by: Chuck Adams <chaz@chaz.works>

chuckadams

Updated coding-standards.yml to more directly invoke the linting CI scripts, but leaving the wp-env stuff as-is in CONTRIBUTING.md, assuming it works for others. Approved the workflow and it passes now.

Signed-off-by: Chuck Adams <chaz@chaz.works>

kasparsd force-pushed the add-phpstan-min branch from 5859af1 to 7cf5f57 Compare December 15, 2025 11:48

kasparsd commented Dec 15, 2025

View reviewed changes

kasparsd added 11 commits December 15, 2025 13:58

Add phpstan checks

4be50ac

Signed-off-by: Kaspars Dambis <hi@kaspars.net>

Ignore the cache directories

145069d

Signed-off-by: Kaspars Dambis <hi@kaspars.net>

Keep at known version

10f873d

Signed-off-by: Kaspars Dambis <hi@kaspars.net>

Document tooling

8fd4716

Signed-off-by: Kaspars Dambis <hi@kaspars.net>

Introduce a generic CLI helper

bdb7d3d

Signed-off-by: Kaspars Dambis <hi@kaspars.net>

Ignore the wp-env override

c5bb464

Signed-off-by: Kaspars Dambis <hi@kaspars.net>

Keep the task level permissions

a546913

Signed-off-by: Kaspars Dambis <hi@kaspars.net>

Ignore more caches

fe3a9f5

Signed-off-by: Kaspars Dambis <hi@kaspars.net>

Include allow baseline until fixed

8fb5ccd

Signed-off-by: Kaspars Dambis <hi@kaspars.net>

Add this back in

2107b43

Signed-off-by: Kaspars Dambis <hi@kaspars.net>

Run it on all pull requests

e14a813

Signed-off-by: Kaspars Dambis <hi@kaspars.net>

kasparsd force-pushed the add-phpstan-min branch from 12d798e to e14a813 Compare December 15, 2025 11:58

Keep the same name

51e174f

Signed-off-by: Kaspars Dambis <hi@kaspars.net>

kasparsd mentioned this pull request Dec 15, 2025

🚀 Feature Request: Fix static analysis issues reported by PHPStan level 0 (lowest) #384

Open

toderash moved this to Icebox in FAIR Connect Jan 2, 2026

toderash added this to FAIR Connect Jan 2, 2026

This was referenced Jan 4, 2026

Add PHPStan and simplify PHPUnit with wp-env kasparsd/fair-plugin#1

Open

🛠️ Unit Testing: Establish Robust Unit Testing #405

Open

toderash moved this from Icebox to Review in FAIR Connect Jan 5, 2026

Ipstenu requested a review from rmccue February 9, 2026 17:48

cdils added this to the 1.4.0 milestone Feb 9, 2026

Ipstenu changed the base branch from main to release_1.4.0 February 12, 2026 17:18

cdils requested review from chuckadams April 6, 2026 16:58

cdils removed this from the 1.4.0 milestone Apr 6, 2026

cdils modified the milestones: 1.5.0, 1.4.0 Apr 6, 2026

chuckadams reviewed Apr 6, 2026

View reviewed changes

chuckadams added 3 commits April 6, 2026 12:26

Merge branch 'release_1.4.0' into add-phpstan-min

8869454

invoke composer run lint without wp-env

0cafeb5

Signed-off-by: Chuck Adams <chaz@chaz.works>

update baseline

41e00db

Signed-off-by: Chuck Adams <chaz@chaz.works>

chuckadams previously approved these changes Apr 6, 2026

View reviewed changes

cdils previously approved these changes Apr 6, 2026

View reviewed changes

shut up phpcs warnings

2c21b15

Signed-off-by: Chuck Adams <chaz@chaz.works>

chuckadams dismissed stale reviews from cdils and themself via 2c21b15 April 6, 2026 19:10

cdils approved these changes Apr 6, 2026

View reviewed changes

chuckadams merged commit 0966d2a into fairpm:release_1.4.0 Apr 6, 2026
2 checks passed

github-project-automation Bot moved this from Review to Done in FAIR Connect Apr 6, 2026

		@@ -0,0 +1,10 @@
		includes:
		- tests/phpstan-baseline.neon # TODO: Fix all these issues.

Conversation

kasparsd commented Dec 15, 2025

Uh oh!

github-actions Bot commented Dec 15, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kasparsd commented Jan 4, 2026

Uh oh!

kasparsd commented Jan 4, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chuckadams Apr 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chuckadams commented Apr 6, 2026

Uh oh!

chuckadams commented Apr 6, 2026

Uh oh!

chuckadams left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

chuckadams Apr 6, 2026 •

edited

Loading