Skip to content

Add PHPStan CI check and pre-push hook, PG-4897 [no_release] - #457

Open
snake14 wants to merge 9 commits into
5.x-devfrom
PG-4897-phpstan
Open

Add PHPStan CI check and pre-push hook, PG-4897 [no_release]#457
snake14 wants to merge 9 commits into
5.x-devfrom
PG-4897-phpstan

Conversation

@snake14

@snake14 snake14 commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Description

Enforces the PHPStan static analysis this plugin already had configured: the level-5 phpstan.neon existed but no CI check or git hook ran it, so the codebase had drifted to 18 analysis errors. This adds the same setup used by other Matomo plugins (e.g. plugin-Slack, plugin-GoogleAnalyticsImporter): a GitHub Actions check on every pull request, plus an optional pre-push git hook that analyses changed files (enable it with git config core.hooksPath .git-hooks-matomo).

Includes the fixes needed for a clean level-5 run. Nearly all are documentation/type corrections with no behaviour change; the two call-site changes (version_compare string argument, Option::set string value, and passing false instead of a meaningless true as updateUser()'s unused password confirmation) are behaviour-neutral. Where the analysis error stems from a too-narrow docblock in Matomo core (AuthResult, Auth::setLogin, UserUpdater), a targeted inline @phpstan-ignore with the reason is used instead of changing behaviour.

Issue No

PG-4897

Steps to Replicate the Issue

  1. Run PHPStan for this plugin in a Matomo checkout: composer run phpstan -- -c plugins/LoginLdap/phpstan.neon.
  2. Expected: no errors, since the plugin pins level 5.
  3. Actual: 18 errors, and nothing in CI prevented new ones.

Checklist

  • [✔] Tested locally or on demo2/demo3?
  • [NA] New test case added/updated?
  • [NA] Are all newly added texts included via translation?
  • [NA] Are text sanitized properly? (Eg use of v-text v/s v-html for vue)
  • [NA] Version bumped?
  • [✔] I have understood, reviewed, and tested all AI outputs before use
  • [✔] All AI instructions respect security, IP, and privacy rules
  • [NA] Documentation updated?

snake14 and others added 4 commits August 3, 2026 20:01
The level-5 phpstan.neon already existed but nothing enforced it;
includes the minor type and PHPDoc fixes needed for a clean run.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Released Updates/*.php are immutable history, so their two analysis
findings are ignored via path-scoped config entries instead.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
phpVersion pins analysis to 7.2 where the class does not exist, so a
scan-only stub makes the full connection union analyzable.
@snake14
snake14 requested a review from a team August 3, 2026 08:52
@snake14 snake14 changed the title Add PHPStan CI check and pre-push hook, PG-4897 Add PHPStan CI check and pre-push hook, PG-4897 [no_release] Aug 3, 2026
snake14 added 4 commits August 4, 2026 11:47
New files carry no pre-existing debt, so hold them to the strictest level.
Uncommitted local changes were analyzed and could block a push for
files the push doesn't contain. Also corrects the hook's log label.
Pushing another local branch, or several refs at once, analysed the
checked-out branch instead of what was actually pushed.
A plugin cloned outside a Matomo checkout cannot be checked, which is
no reason to block its pushes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant