From d830f5d2b5961f6f0a4dc8b937c8d9e62d21d445 Mon Sep 17 00:00:00 2001 From: Jacob Ransom Date: Mon, 14 Sep 2026 15:06:58 +1200 Subject: [PATCH 1/4] Correct four annotations and state the tracker returns [ignore_release] 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. --- API.php | 7 +++---- RecordBuilders/CustomVariables.php | 2 +- Reports/GetCustomVariables.php | 4 +--- Tracker/CustomVariablesRequestProcessor.php | 6 +++++- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/API.php b/API.php index c8066a4..66aeba6 100644 --- a/API.php +++ b/API.php @@ -14,7 +14,6 @@ use Piwik\Archive; use Piwik\Container\StaticContainer; use Piwik\DataTable; -use Piwik\Date; use Piwik\Metrics; use Piwik\Piwik; @@ -28,10 +27,10 @@ class API extends \Piwik\Plugin\API /** * @param int $idSite * @param string $period - * @param Date $date + * @param string $date * @param string $segment * @param bool $expanded - * @param int $idSubtable + * @param int|null $idSubtable * * @return DataTable|DataTable\Map */ @@ -166,9 +165,9 @@ public function getUsagesOfSlots($idSite) 'page' => array_fill(1, $numVars, array()), ); - /** @var DataTable $customVarUsages */ $today = StaticContainer::get('CustomVariables.today'); $date = '2008-12-12,' . $today; + /** @var DataTable $customVarUsages */ $customVarUsages = Request::processRequest( 'CustomVariables.getCustomVariables', array('idSite' => $idSite, 'period' => 'range', 'date' => $date, diff --git a/RecordBuilders/CustomVariables.php b/RecordBuilders/CustomVariables.php index 890cc09..46224ff 100644 --- a/RecordBuilders/CustomVariables.php +++ b/RecordBuilders/CustomVariables.php @@ -74,7 +74,7 @@ protected function aggregate(ArchiveProcessor $archiveProcessor): array } - protected function aggregateCustomVariable(DataTable $record, array &$metadata, array &$metadataFlat, LogAggregator $logAggregator, string $slot): void + protected function aggregateCustomVariable(DataTable $record, array &$metadata, array &$metadataFlat, LogAggregator $logAggregator, int $slot): void { $keyField = "custom_var_k" . $slot; $valueField = "custom_var_v" . $slot; diff --git a/Reports/GetCustomVariables.php b/Reports/GetCustomVariables.php index 0f01ce2..448c10a 100644 --- a/Reports/GetCustomVariables.php +++ b/Reports/GetCustomVariables.php @@ -71,9 +71,7 @@ public function getFooterMessageExplanationMissingMetrics() // no footer message for subtables $out = ''; Piwik::postEvent('Template.afterCustomVariablesReport', array(&$out)); - if (!empty($message)) { - $message .= $out; - } + $message .= $out; } return $message; diff --git a/Tracker/CustomVariablesRequestProcessor.php b/Tracker/CustomVariablesRequestProcessor.php index e7752eb..fe15ded 100644 --- a/Tracker/CustomVariablesRequestProcessor.php +++ b/Tracker/CustomVariablesRequestProcessor.php @@ -46,6 +46,8 @@ public function processRequestParams(VisitProperties $visitProperties, Request $ } $request->setMetadata('CustomVariables', 'visitCustomVariables', $visitorCustomVariables); + + return false; } public function onNewVisit(VisitProperties $visitProperties, Request $request) @@ -71,7 +73,7 @@ public function afterRequestProcessed(VisitProperties $visitProperties, Request $action = $request->getMetadata('Actions', 'action'); if (empty($action) || !($action instanceof Action)) { - return; + return false; } $customVariables = self::getCustomVariablesInPageScope($request); @@ -84,6 +86,8 @@ public function afterRequestProcessed(VisitProperties $visitProperties, Request $action->setCustomField($field, $value); } } + + return false; } public static function getCustomVariablesInVisitScope(Request $request) From 47d7f37d97fd198f3b03e76dd2c8bf5095b7ef56 Mon Sep 17 00:00:00 2001 From: Jacob Ransom Date: Mon, 14 Sep 2026 15:06:58 +1200 Subject: [PATCH 2/4] Run PHPStan at level 5 [ignore_release] 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. --- .github/workflows/ci.yml | 3 --- phpstan.neon | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 3 deletions(-) create mode 100644 phpstan.neon diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6fe9d7b..b6d9a84 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,6 +18,3 @@ jobs: uses: matomo-org/plugin-ci-workflows/.github/workflows/plugin-ci.yml@main with: plugin-name: CustomVariables - # PHPStan has never run here and there is no phpstan.neon to run it with; - # adding one is a decision of its own. - skip-phpstan: true diff --git a/phpstan.neon b/phpstan.neon new file mode 100644 index 0000000..49595e4 --- /dev/null +++ b/phpstan.neon @@ -0,0 +1,37 @@ +parameters: + level: 5 + phpVersion: 80100 + tmpDir: /tmp/phpstan/CustomVariables/main + paths: + - . + excludePaths: + analyseAndScan: + - tests/* + # The shared workflow leaves this helper checkout in the workspace, and `paths: .` + # would otherwise analyse it as if it were plugin code. + - github-action-tests/* + ignoreErrors: + # DataTable::queueFilter() and queueFilterSubtables() document `@param array`, but both + # wrap a non-array parameter themselves before storing it, so a bare column name is + # supported usage rather than a mistake. + - + message: '#expects array, string given#' + path: API.php + # Row::setColumn() and deleteColumn() are untyped and only document `@param string`, while + # archiving addresses columns by the integer Metrics::INDEX_* constants throughout. + - + message: '#expects string, int given#' + path: RecordBuilders/CustomVariables.php + # This plugin supports Matomo 5 and 6, but the analysis only ever sees one of them, so the + # version guard reads as decided. It is load-bearing on the other line. + - + message: '#Ternary operator condition is always true#' + path: RecordBuilders/CustomVariables.php + bootstrapFiles: + - ../../bootstrap-phpstan.php + universalObjectCratesClasses: + - Piwik\Config + - Piwik\View + - Piwik\ViewDataTable\Config + scanDirectories: + - ../../ From 8cadbf1f0fc99e92c5787ee326eab838afe656a8 Mon Sep 17 00:00:00 2001 From: Jacob Ransom Date: Mon, 14 Sep 2026 15:39:55 +1200 Subject: [PATCH 3/4] Pass the filter parameters as an array instead of ignoring the finding [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. --- API.php | 4 ++-- phpstan.neon | 6 ------ 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/API.php b/API.php index 66aeba6..08cb9c3 100644 --- a/API.php +++ b/API.php @@ -37,11 +37,11 @@ class API extends \Piwik\Plugin\API protected function getDataTable($idSite, $period, $date, $segment, $expanded, $flat, $idSubtable) { $dataTable = Archive::createDataTableFromArchive(Archiver::CUSTOM_VARIABLE_RECORD_NAME, $idSite, $period, $date, $segment, $expanded, $flat, $idSubtable); - $dataTable->queueFilter('ColumnDelete', 'nb_uniq_visitors'); + $dataTable->queueFilter('ColumnDelete', ['nb_uniq_visitors']); if ($flat) { $dataTable->filterSubtables('Sort', array(Metrics::INDEX_NB_ACTIONS, 'desc', $naturalSort = false, $expanded)); - $dataTable->queueFilterSubtables('ColumnDelete', 'nb_uniq_visitors'); + $dataTable->queueFilterSubtables('ColumnDelete', ['nb_uniq_visitors']); } return $dataTable; diff --git a/phpstan.neon b/phpstan.neon index 49595e4..eb70c2c 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -11,12 +11,6 @@ parameters: # would otherwise analyse it as if it were plugin code. - github-action-tests/* ignoreErrors: - # DataTable::queueFilter() and queueFilterSubtables() document `@param array`, but both - # wrap a non-array parameter themselves before storing it, so a bare column name is - # supported usage rather than a mistake. - - - message: '#expects array, string given#' - path: API.php # Row::setColumn() and deleteColumn() are untyped and only document `@param string`, while # archiving addresses columns by the integer Metrics::INDEX_* constants throughout. - From 702fb085be88a8ed2788e1f0c9042c0e0b8c0499 Mon Sep 17 00:00:00 2001 From: Jacob Ransom Date: Mon, 14 Sep 2026 15:47:25 +1200 Subject: [PATCH 4/4] Pin the analysis exceptions to identifier, path and count [ignore_release] 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. --- phpstan.neon | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/phpstan.neon b/phpstan.neon index eb70c2c..e8f7e2b 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -12,15 +12,19 @@ parameters: - github-action-tests/* ignoreErrors: # Row::setColumn() and deleteColumn() are untyped and only document `@param string`, while - # archiving addresses columns by the integer Metrics::INDEX_* constants throughout. + # archiving addresses columns by the integer Metrics::INDEX_* constants throughout. Pinned + # to the five call sites that do so, so a sixth has to be looked at rather than absorbed. - - message: '#expects string, int given#' + identifier: argument.type + message: '#Row::(setColumn|deleteColumn)\(\) expects string, int given#' path: RecordBuilders/CustomVariables.php + count: 5 # This plugin supports Matomo 5 and 6, but the analysis only ever sees one of them, so the # version guard reads as decided. It is load-bearing on the other line. - - message: '#Ternary operator condition is always true#' + identifier: ternary.alwaysTrue path: RecordBuilders/CustomVariables.php + count: 1 bootstrapFiles: - ../../bootstrap-phpstan.php universalObjectCratesClasses: