Skip to content

05: Static analysis in CI - #5

Merged
nikolaystrikhar merged 5 commits into
mainfrom
05-ci-static-analysis
Aug 11, 2026
Merged

05: Static analysis in CI#5
nikolaystrikhar merged 5 commits into
mainfrom
05-ci-static-analysis

Conversation

@nikolaystrikhar

@nikolaystrikhar nikolaystrikhar commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

PHPStan over src/, plus the workflow that runs it.

Stacked on #4 — this PR's diff is the two new files.

  • Level 8. The package analyses clean at 6, and 7 and 8 report the same single error, so 8 costs nothing beyond fixing that one. Level 9 is not taken: its five findings are all in Sub_Plugin's untyped config reads, which are the runtime guards treatPhpDocTypesAsCertain: false exists to keep.

- --ignore-platform-reqs also disabled the config.platform.php pin, which
  resolved dev dependencies requiring PHP 8.1+; waive only the ext-*
  requirements a static-analysis run genuinely has no use for.
- Run on 7.4, the floor the pin already targets, instead of an EOL 8.0 that
  matches neither end of the test matrix.
- Fail when src/ holds no PHP files: PHPStan 1.x exits 0 on nothing to
  analyse, so a moved source root would leave the check green forever.
- Narrow the token, cancel superseded PR runs, and cap the job, matching the
  tests workflow.
- Drop reportUnmatchedIgnoredErrors: false; with no ignores yet it does
  nothing except guarantee the first stale one never gets reported.
- Key the cache off head_ref, which is a branch name on pull_request.
Level 5 left coverage unused: the package analyses clean at 6, and 7 and 8
report the same single error, so 8 costs nothing beyond it. Level 9 is not
taken -- its five findings are all in Sub_Plugin's untyped config reads, which
are the runtime guards treatPhpDocTypesAsCertain: false exists to keep.
Comment thread phpstan.neon.dist
Comment on lines +5639 to +5642
- **G** — PHPStan analyses `src/` only. Hand-written test-support code is thin for now, and adding
`tests/` wholesale would drag in Codeception's generated actor and `WPTestCase` module magic —
a well-known source of level-5 false positives. Once real support classes accumulate, put
`tests/_support` alone under a second config rather than widening `paths`.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We could just be more specific with excluded paths in our config to ensure these generated files aren't analyzed, right? I don't know if I like excluding tests/ from checks. While they can be annoying sometimes (intentionally providing weird data, having to force types sometimes, etc.) it helps ensure that our tests are strongly written and work exactly as intended without types, etc. being ignored.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I had this concern tbh :D Fixed

Comment thread phpstan.neon.dist
parameters:
phpVersion: 70400
level: 8
tmpDir: phpstan-cache

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We'll want to ensure this is in .gitignore and even .gitattributes just to be safe.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added

Updated the PHPStan configuration to include the 'tests' directory for analysis while excluding generated test-support files to prevent false positives. Also, added 'phpstan-cache' to the export-ignore list in .gitattributes.
Base automatically changed from 04-config to main August 11, 2026 09:03
@nikolaystrikhar
nikolaystrikhar merged commit 1a70fe9 into main Aug 11, 2026
5 checks passed
@nikolaystrikhar
nikolaystrikhar deleted the 05-ci-static-analysis branch August 11, 2026 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants