Skip to content

Sync PHPStan pre-push hook and workflow fixes [no_release] - #272

Open
snake14 wants to merge 8 commits into
5.x-devfrom
PG-4897-hook-fixes
Open

Sync PHPStan pre-push hook and workflow fixes [no_release]#272
snake14 wants to merge 8 commits into
5.x-devfrom
PG-4897-hook-fixes

Conversation

@snake14

@snake14 snake14 commented Aug 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

Applies the PHPStan tooling fixes from the PG-4897 rollout template to this plugin's copy:

  • The pre-push hook now diffs against the merge base with origin/5.x-dev instead of the local 5.x-dev branch, which could be stale or missing and silently widened the analysis to files the push doesn't touch. It now fails with a clear message when the merge base can't be resolved.
  • The hook falls back to ddev when a local PHP exists but PHPStan isn't installed in the Matomo vendor dir (previously this combination dead-ended and the hook reported no way to run PHPStan).
  • The github-action-tests checkout in the PHPStan workflow no longer persists credentials.
  • Newly created files are now checked at PHPStan level 9, and the CI base and modified-files hook levels are raised to level 5 — the highest level the existing code passes after fixing the three findings in Processor.php that the raise surfaced. The hook config no longer overrides treatPhpDocTypesAsCertain, so the hook and CI report identical errors.

Tooling-only change: no version bump or changelog entry (no_release).

Issue No

Related to PG-4897 (JIRA)

Steps to Replicate the Issue

  1. Push a branch from a checkout whose local 5.x-dev is stale or missing.
  2. Expected: the pre-push hook analyses only the files the push touches.
  3. Actual: the hook diffed against the stale local branch and analysed unrelated files, or failed to resolve the branch entirely.

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 added 5 commits August 4, 2026 10:09
@snake14
Sync PHPStan pre-push hook and workflow fixes [no_release]
22c5b57 
Diff against the merge base with origin/5.x-dev instead of the possibly stale
local branch, fall back to ddev when local PHP lacks PHPStan, and stop
persisting credentials on the github-action-tests checkout.
@snake14
Align the modified-files hook level with the CI base level [no_release]
b75d0ee 
A higher level in the pre-push hook than in CI blocks pushes on pre-existing
errors in touched files that CI accepts.
@snake14
Raise PHPStan levels: new files to 9, existing code to its clean maxi…
608bb44 
…mum [no_release]

Level 5 is the highest the existing code passes with zero errors; new
files carry no pre-existing debt, so they are held to the strictest level.
@snake14
Fix the level-5 PHPStan findings in Processor.php [no_release]
f01b4fd 
getValueForAlertInPast() returns a single aggregated value or null, not an
array as documented; the two Version::VERSION guards get reasoned ignores since
the constant resolves from the analysed checkout.
@snake14
Drop the hook-only treatPhpDocTypesAsCertain override [no_release]
f0c5254 
The hook and CI must report the same errors: inline ignores that match under
CI semantics become unmatched-ignore errors under the hook's, blocking pushes
that touch those files.
@snake14
snake14 requested a review from a team August 4, 2026 01:06
snake14 added 3 commits August 4, 2026 13:52
@snake14
Check pushed commits rather than the working tree in the pre-push hook
ef8bd97 
Uncommitted local changes were analyzed and could block a push for
files the push doesn't contain. Also corrects the hook's log label.
@snake14
Analyse the commits supplied on the hook's stdin rather than HEAD
245dd40 
Pushing another local branch, or several refs at once, analysed the
checked-out branch instead of what was actually pushed.
@snake14
Skip the hook instead of rejecting pushes from standalone clones
dda5164 
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

Reviewers

No reviews

Assignees

No one assigned

Labels

Needs Review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@snake14