Skip to content

Run PHPStan at level 5 [ignore_release] - #148

Merged
snake14 merged 4 commits into
6.x-devfrom
enable-phpstan
Sep 14, 2026
Merged

Run PHPStan at level 5 [ignore_release]#148
snake14 merged 4 commits into
6.x-devfrom
enable-phpstan

Conversation

@snake14

@snake14 snake14 commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Description

This plugin had no phpstan.neon, so the umbrella skipped the check and it has never been analysed. The configuration matches the one the rest of the fleet uses; level 5 is what the comparably sized plugins already run at, and the plugin is clean at that level.

Nineteen findings, which split into four annotation corrections, two dead constructs, three tracker returns, and three ignores.

Corrected annotations. getDataTable() documented @param Date $date, but every caller passes the request's date string; and @param int $idSubtable, while one call site passes null. Correcting those orphaned the Piwik\Date import, which PHPCS then flagged, so it goes too. An inline @var DataTable $customVarUsages sat above a StaticContainer::get() call rather than the variable it describes, three lines later. And aggregateCustomVariable() declared string $slot although its only caller passes the loop counter — PHP was coercing the int on every call.

Dead constructs. A guard on $message, built as a concatenation that always contains a space, could never be false.

Tracker returns. processRequestParams() and afterRequestProcessed() now return the bool core documents. Core reads both as $abort = …; if ($abort), so the previous implicit null and an explicit false are equivalent — this states the contract rather than changing it, and no test asserts on either return value.

Three ignores, each with its reason inline. queueFilter() and queueFilterSubtables() document @param array but both wrap a non-array parameter themselves, so passing a bare column name is supported usage — verified by running it. Row::setColumn() and deleteColumn() are untyped and only document @param string, while archiving addresses columns by the integer Metrics::INDEX_* constants throughout. And the version_compare() guard reads as decided because the analysis only ever sees one Matomo, while the plugin supports 5 and 6.

No version bump or changelog entry: the substance is static-analysis setup.

Issue No

No GitHub issue.

Steps to Replicate the Issue

  1. Note that the umbrella passes skip-phpstan for this plugin because there is no config to run.
  2. Expected result: static analysis runs, as it does for the other migrated plugins.
  3. Actual result: the check is skipped.

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?

getDataTable documented @PARAM Date for a value its callers pass as the request date string, and @PARAM int for an idSubtable one call site passes as null; the Date import went with it. An inline @var DataTable sat above a StaticContainer call rather than the variable it describes. aggregateCustomVariable declared string $slot although its only caller passes the loop counter, and PHP was coercing it. processRequestParams and afterRequestProcessed now return the bool core documents and reads as $abort - previously null, which is equally falsy. A guard on a concatenation that always contains a space could never be false.
Three ignores are recorded with their reasons: queueFilter and queueFilterSubtables wrap a non-array parameter themselves, Row::setColumn and deleteColumn are untyped while archiving addresses columns by integer Metrics::INDEX_* constants, and the Matomo version guard reads as decided because the analysis only sees one Matomo.
…g [ignore_release]

queueFilter() and queueFilterSubtables() document @PARAM array, and every other plugin passes one -- PagePerformance, Bandwidth, CrashAnalytics, MediaAnalytics and BotTracking all do. DataTable wraps a non-array itself, so this is identical at runtime, but it matches the contract and the house style, which is better than recording an exception for it.
…ease]

A message regex over a whole file absorbs any future finding that happens to match it. Each exception now names the PHPStan identifier, the file it belongs to, and exactly how many occurrences are expected, so a new one fails the build and is reported on its own line rather than being silently suppressed.
@snake14
snake14 requested a review from a team September 14, 2026 04:41
@snake14
snake14 enabled auto-merge (squash) September 14, 2026 04:41
@snake14
snake14 merged commit 0ec41f5 into 6.x-dev Sep 14, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants