From 9149b7b29a326f69035e0c334da9c30d1b8acb52 Mon Sep 17 00:00:00 2001 From: Matt <1169490+caddoo@users.noreply.github.com> Date: Thu, 9 Jul 2026 16:27:12 +1200 Subject: [PATCH 1/7] DEV-20451: sync Live.getLastVisitsDetails fixture for accurate time-spent metric --- .../System/expected/test___Live.getLastVisitsDetails_day.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/System/expected/test___Live.getLastVisitsDetails_day.xml b/tests/System/expected/test___Live.getLastVisitsDetails_day.xml index 87aa078b..70ed0932 100644 --- a/tests/System/expected/test___Live.getLastVisitsDetails_day.xml +++ b/tests/System/expected/test___Live.getLastVisitsDetails_day.xml @@ -16,8 +16,8 @@ 1 - 721 - 12 min 1s + 0 + 0s 1 Viewing homepage http://example.com/sub/page From 493ddbf070aed0c5ee3839a9623c32ec7c25f74e Mon Sep 17 00:00:00 2001 From: Matt <1169490+caddoo@users.noreply.github.com> Date: Wed, 22 Jul 2026 13:08:58 +1200 Subject: [PATCH 2/7] DEV-20451: sync Live.getLastVisitsDetails fixture for accurate time-spent metric Under the new PageViewTimeWriter the first pageview in the fixture is closed at the next in-visit action (t+6min), so the visitor log now shows timeSpent=360s rather than the legacy 0s (walk-forward never found a follow-up with time_spent_ref_action for this cache-focused scenario). --- .../System/expected/test___Live.getLastVisitsDetails_day.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/System/expected/test___Live.getLastVisitsDetails_day.xml b/tests/System/expected/test___Live.getLastVisitsDetails_day.xml index 70ed0932..0e225a6a 100644 --- a/tests/System/expected/test___Live.getLastVisitsDetails_day.xml +++ b/tests/System/expected/test___Live.getLastVisitsDetails_day.xml @@ -16,8 +16,8 @@ 1 - 0 - 0s + 360 + 6 min 0s 1 Viewing homepage http://example.com/sub/page From cb53de7fc67e5b7d51c9e0d8e4c6d80ed0315dae Mon Sep 17 00:00:00 2001 From: Stefan Giehl Date: Mon, 10 Aug 2026 08:53:06 +0200 Subject: [PATCH 3/7] Prepare for Matomo 6 (#90) * Prepare for Matomo 6 Bump plugin version to 6.0.0 and require Matomo >=6.0.0-b1,<7.0.0-b1. * Run tests on same PHP versions as core (8.1, 8.5) * Run plugin tests against MySQL 8.0 and MariaDB 10.6 (#89) * Run plugin tests against MySQL 8.0 and MariaDB 10.6 The workflow inherited the shared action's MySQL 5.7 default, which is below Matomo's new minimum. Pin the database-backed jobs to the supported floor: PluginTests now runs a MySQL 8.0 + MariaDB 10.6 matrix, and the UI/JS jobs run on MySQL 8.0. * Only upload plugin test artifacts for the MySQL matrix leg PluginTests runs a MySQL + MariaDB matrix; the upload-artifacts condition matched both legs and uploaded twice. Restrict the upload to the MySQL leg. * Add changelog entry for Matomo 6 compatibility [ignore_release] * Use the matomo6 PHP version aliases in the test workflow [ignore_release] --- .github/workflows/matomo-tests.yml | 11 ++++++++--- .github/workflows/phpcs.yml | 2 +- CHANGELOG.md | 3 +++ plugin.json | 4 ++-- 4 files changed, 14 insertions(+), 6 deletions(-) diff --git a/.github/workflows/matomo-tests.yml b/.github/workflows/matomo-tests.yml index cae01543..07fb8a6b 100644 --- a/.github/workflows/matomo-tests.yml +++ b/.github/workflows/matomo-tests.yml @@ -1,7 +1,7 @@ # Action for running tests # This file has been automatically created. # To recreate it you can run this command -# ./console generate:test-action --plugin="DeviceDetectorCache" --php-versions="matomo5_min_php,matomo5_max_php" --schedule-cron="15 3 * * 6" +# ./console generate:test-action --plugin="DeviceDetectorCache" --php-versions="matomo6_min_php,matomo6_max_php" --schedule-cron="15 3 * * 6" name: Plugin DeviceDetectorCache Tests @@ -37,8 +37,11 @@ jobs: strategy: fail-fast: false matrix: - php: [ 'matomo5_min_php', 'matomo5_max_php' ] + php: [ 'matomo6_min_php', 'matomo6_max_php' ] target: ['minimum_required_matomo', 'maximum_supported_matomo'] + database: + - { engine: 'Mysql', version: '8.0' } + - { engine: 'Mariadb', version: '10.6' } steps: - uses: actions/checkout@v3 with: @@ -52,6 +55,8 @@ jobs: plugin-name: 'DeviceDetectorCache' php-version: ${{ matrix.php }} test-type: 'PluginTests' + mysql-engine: ${{ matrix.database.engine }} + mysql-version: ${{ matrix.database.version }} matomo-test-branch: ${{ matrix.target }} artifacts-pass: ${{ secrets.ARTIFACTS_PASS }} - upload-artifacts: ${{ matrix.php == 'matomo5_min_php' && matrix.target == 'maximum_supported_matomo' }} + upload-artifacts: ${{ matrix.php == 'matomo6_min_php' && matrix.target == 'maximum_supported_matomo' && matrix.database.engine == 'Mysql' }} diff --git a/.github/workflows/phpcs.yml b/.github/workflows/phpcs.yml index e41da57b..7cef1a5c 100644 --- a/.github/workflows/phpcs.yml +++ b/.github/workflows/phpcs.yml @@ -26,7 +26,7 @@ jobs: - name: Setup PHP uses: shivammathur/setup-php@v2 with: - php-version: '7.4' + php-version: '8.1' tools: cs2pr - name: Install dependencies run: diff --git a/CHANGELOG.md b/CHANGELOG.md index a45c0b9c..fc498567 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ ## Changelog +6.0.0 +- Compatibility with Matomo 6.X + 5.0.3 - Added plugin category for Marketplace diff --git a/plugin.json b/plugin.json index 1ead9b7d..c9ad8a2f 100644 --- a/plugin.json +++ b/plugin.json @@ -1,10 +1,10 @@ { "name": "DeviceDetectorCache", "description": "Makes tracking faster by detecting many devices, operating systems, bots, and browsers from a cache.", - "version": "5.0.3", + "version": "6.0.0", "theme": false, "require": { - "matomo": ">=5.0.0-b1,<6.0.0-b1" + "matomo": ">=6.0.0-b1,<7.0.0-b1" }, "authors": [ { From 2e3c9e272aefeeec57275b88377536c41cc44045 Mon Sep 17 00:00:00 2001 From: "Weblate (bot)" Date: Mon, 24 Aug 2026 23:22:58 +0200 Subject: [PATCH 4/7] Translated using Weblate (Chinese (Traditional Han script)) (#91) Currently translated at 100.0% (1 of 1 strings) Translation: Matomo/Plugin DeviceDetectorCache Translate-URL: https://hosted.weblate.org/projects/matomo/plugin-devicedetectorcache/zh_Hant/ [ci skip] Co-authored-by: Lay --- lang/zh-tw.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lang/zh-tw.json b/lang/zh-tw.json index efe0e302..c013ff04 100644 --- a/lang/zh-tw.json +++ b/lang/zh-tw.json @@ -1,5 +1,5 @@ { "DeviceDetectorCache": { - "PluginDescription": "從快取內刪除一些裝置、作業系統、機器人、瀏覽器，以便加速追蹤。" + "PluginDescription": "透過快取偵測多種裝置、作業系統、機器人與瀏覽器，加快追蹤速度。" } -} \ No newline at end of file +} From 4e3ee4652dfc5ab0899581a2dcb0ceb0f687acb6 Mon Sep 17 00:00:00 2001 From: Jacob R Date: Fri, 11 Sep 2026 20:48:46 +1200 Subject: [PATCH 5/7] Move the checks onto the shared umbrella and build the other dev branch weekly [ignore_release] (#92) * Put the pull request checks behind the shared umbrella [ignore_release] The callers this replaces were already thin wrappers around the same shared workflows, so one caller means a check added centrally reaches this plugin without a pull request here. It also drops the concurrency group the checklist workflow carried, which would span both of the umbrella's lanes and defeat them. * Build the non-default dev branch weekly [ignore_release] GitHub runs `schedule` only from the default branch's copy of a workflow, so the other maintained branch has had no weekly build of its own. The cron keeps this plugin's own minute and hour, moved to Sunday, so the fleet stays staggered. --- .github/workflows/ci.yml | 23 ++++++++++++ .github/workflows/matomo-ai-checklist.yml | 32 ----------------- .github/workflows/phpcs.yml | 43 ----------------------- .github/workflows/weekly-branch-sweep.yml | 25 +++++++++++++ 4 files changed, 48 insertions(+), 75 deletions(-) create mode 100644 .github/workflows/ci.yml delete mode 100644 .github/workflows/matomo-ai-checklist.yml delete mode 100644 .github/workflows/phpcs.yml create mode 100644 .github/workflows/weekly-branch-sweep.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 00000000..1074ce56 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,23 @@ +name: Plugins CI + +on: + pull_request: + types: [opened, synchronize, reopened, edited] + push: + branches: + - '**.x-dev' + workflow_dispatch: + +permissions: + actions: read + contents: read + pull-requests: read + +jobs: + ci: + uses: matomo-org/plugin-ci-workflows/.github/workflows/plugin-ci.yml@main + with: + plugin-name: DeviceDetectorCache + # 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/.github/workflows/matomo-ai-checklist.yml b/.github/workflows/matomo-ai-checklist.yml deleted file mode 100644 index cc2c022d..00000000 --- a/.github/workflows/matomo-ai-checklist.yml +++ /dev/null @@ -1,32 +0,0 @@ -# Action for running tests - -name: AI Checklist - -on: - pull_request: - types: [opened, synchronize, reopened, edited] - -permissions: - actions: read - checks: none - contents: none - deployments: none - issues: none - packages: none - pull-requests: read - repository-projects: none - security-events: none - statuses: none - -concurrency: - group: ${{ github.workflow }}-${{ github.ref }} - cancel-in-progress: true - -jobs: - AiChecklist: - runs-on: ubuntu-24.04 - strategy: - fail-fast: false - steps: - - name: Run tests - uses: matomo-org/github-action-checklist-gate@main diff --git a/.github/workflows/phpcs.yml b/.github/workflows/phpcs.yml deleted file mode 100644 index 7cef1a5c..00000000 --- a/.github/workflows/phpcs.yml +++ /dev/null @@ -1,43 +0,0 @@ -name: PHPCS check - -on: pull_request - -permissions: - actions: read - checks: read - contents: read - deployments: none - issues: read - packages: none - pull-requests: read - repository-projects: none - security-events: none - statuses: read - -jobs: - phpcs: - name: PHPCS - runs-on: ubuntu-24.04 - steps: - - uses: actions/checkout@v4 - with: - lfs: false - persist-credentials: false - - name: Setup PHP - uses: shivammathur/setup-php@v2 - with: - php-version: '8.1' - tools: cs2pr - - name: Install dependencies - run: - composer init --name=matomo/devicedetectorcache --quiet; - composer --no-plugins config allow-plugins.dealerdirect/phpcodesniffer-composer-installer true -n; - composer config repositories.matomo-coding-standards vcs https://github.com/matomo-org/matomo-coding-standards -n; - composer require matomo-org/matomo-coding-standards:dev-master; - composer install --dev --prefer-dist --no-progress --no-suggest - - name: Check PHP code styles - id: phpcs - run: ./vendor/bin/phpcs --report-full --standard=phpcs.xml --report-checkstyle=./phpcs-report.xml - - name: Show PHPCS results in PR - if: ${{ always() && steps.phpcs.outcome == 'failure' }} - run: cs2pr ./phpcs-report.xml --prepend-filename diff --git a/.github/workflows/weekly-branch-sweep.yml b/.github/workflows/weekly-branch-sweep.yml new file mode 100644 index 00000000..b126cf22 --- /dev/null +++ b/.github/workflows/weekly-branch-sweep.yml @@ -0,0 +1,25 @@ +# Dispatches this plugin's build for each maintained branch that is not the default one, because +# GitHub only ever runs `schedule` from the default branch's copy of a workflow file. The logic +# lives in matomo-org/plugin-ci-workflows; see the "Branch sweep" section of its README.md for +# why this dispatches rather than building another branch's source here. + +name: Weekly branch sweep + +on: + schedule: + # Sunday, so it does not compete with this plugin's own Saturday build. Keeps the minute and + # hour of matomo-tests.yml's cron, so the fleet stays staggered across the window. + - cron: '15 3 * * 0' + workflow_dispatch: + +permissions: {} + +jobs: + sweep: + # Granted by the caller because permissions can only be maintained or reduced down a call + # chain, never elevated: the called workflow declares these too, but that can only cap them, + # not supply them, so without this block the dispatch is unauthorised. + permissions: + actions: write + contents: read + uses: matomo-org/plugin-ci-workflows/.github/workflows/plugin-branch-sweep.yml@main From da0723e9499cfc2fca6ce0e0217ff4df0047bbff Mon Sep 17 00:00:00 2001 From: Jacob R Date: Mon, 14 Sep 2026 22:23:57 +1200 Subject: [PATCH 6/7] Run PHPStan at level 5 [ignore_release] (#94) * Correct two return annotations and drop a stale argument [ignore_release] getRegexMatchEntry() and getNumEntriesToCache() are annotated @return string but cast to int; the annotation also made the subtraction in WarmDeviceDetectorCache read as int minus string. writeToCache() has taken one parameter since 0c205c38, yet every call site still passed a second empty array, which PHP discarded. The constructor takes an array of client-hint headers and converts it with ClientHints::factory(), so declaring it array stops the parent signature being inherited as ?ClientHints. * Run PHPStan at level 5 [ignore_release] The findings that would have failed it are fixed in the previous commit. --- .github/workflows/ci.yml | 3 --- CachedEntry.php | 2 +- Commands/WarmDeviceDetectorCache.php | 2 +- Configuration.php | 4 ++-- phpstan.neon | 19 +++++++++++++++++++ tests/Integration/CachedEntryTest.php | 26 +++++++++++++------------- 6 files changed, 36 insertions(+), 20 deletions(-) create mode 100644 phpstan.neon diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1074ce56..fdb9b3bf 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: DeviceDetectorCache - # 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/CachedEntry.php b/CachedEntry.php index 5291e71a..e3ae19d2 100644 --- a/CachedEntry.php +++ b/CachedEntry.php @@ -23,7 +23,7 @@ class CachedEntry extends DeviceDetector private static $CACHE_DIR = ''; private static $customCache = null; - public function __construct(string $userAgent, $clientHints, array $values) + public function __construct(string $userAgent, array $clientHints, array $values) { $clientHints = $clientHints ? ClientHints::factory($clientHints) : null; parent::__construct($userAgent, $clientHints); diff --git a/Commands/WarmDeviceDetectorCache.php b/Commands/WarmDeviceDetectorCache.php index 336d9c49..681589f7 100644 --- a/Commands/WarmDeviceDetectorCache.php +++ b/Commands/WarmDeviceDetectorCache.php @@ -164,7 +164,7 @@ protected function doExecute(): int if ($i <= 10) { $this->log('Found user agent ' . $agent . ' count: ' . $val); } - CachedEntry::writeToCache($agent, []); + CachedEntry::writeToCache($agent); // sleep 2ms to let CPU do something else // this will make things about 10m slower for 200K entries but at least sudden CPU increase for instance // can be prevented when there are only few CPUs available diff --git a/Configuration.php b/Configuration.php index c95cb1a5..be117dec 100644 --- a/Configuration.php +++ b/Configuration.php @@ -78,7 +78,7 @@ public function getAccessLogRegex() } /** - * @return string + * @return int */ public function getRegexMatchEntry() { @@ -86,7 +86,7 @@ public function getRegexMatchEntry() } /** - * @return string + * @return int */ public function getNumEntriesToCache() { diff --git a/phpstan.neon b/phpstan.neon new file mode 100644 index 00000000..6058b356 --- /dev/null +++ b/phpstan.neon @@ -0,0 +1,19 @@ +parameters: + level: 5 + phpVersion: 80100 + tmpDir: /tmp/phpstan/DeviceDetectorCache/main + paths: + - . + excludePaths: + - 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/* + bootstrapFiles: + - ../../bootstrap-phpstan.php + universalObjectCratesClasses: + - Piwik\Config + - Piwik\View + - Piwik\ViewDataTable\Config + scanDirectories: + - ../../ diff --git a/tests/Integration/CachedEntryTest.php b/tests/Integration/CachedEntryTest.php index c2e29043..32393a2d 100644 --- a/tests/Integration/CachedEntryTest.php +++ b/tests/Integration/CachedEntryTest.php @@ -165,11 +165,11 @@ public function testGetNumCacheFiles_noneCached() public function testGetNumCacheFiles() { - CachedEntry::writeToCache('foo', []); + CachedEntry::writeToCache('foo'); $this->assertEquals(1, CachedEntry::getNumEntriesInCacheDir()); - CachedEntry::writeToCache('bar', []); + CachedEntry::writeToCache('bar'); $this->assertEquals(2, CachedEntry::getNumEntriesInCacheDir()); - CachedEntry::writeToCache('baz', []); + CachedEntry::writeToCache('baz'); $this->assertEquals(3, CachedEntry::getNumEntriesInCacheDir()); } @@ -180,7 +180,7 @@ public function testGetCached_noEntry() public function test_writeToCache_GetCached() { - CachedEntry::writeToCache('foo', []); + CachedEntry::writeToCache('foo'); $cacheEntry = CachedEntry::getCached('foo', []); $this->assertEquals( [ @@ -212,7 +212,7 @@ public function test_getCacheDir() public function test_deleteLeastAccessedFiles_nothingToDelete() { - $filePath = CachedEntry::writeToCache('file', []); + $filePath = CachedEntry::writeToCache('file'); $this->assertFileExists($filePath); CachedEntry::deleteLeastAccessedFiles(-1); @@ -223,13 +223,13 @@ public function test_deleteLeastAccessedFiles_nothingToDelete() public function test_deleteLeastAccessedFiles_deletesOnlyOldest() { - $filePath1 = CachedEntry::writeToCache('file', []); + $filePath1 = CachedEntry::writeToCache('file'); sleep(1); // otherwise without sleep the sorting won't work properly - $filePath2 = CachedEntry::writeToCache('bar', []); + $filePath2 = CachedEntry::writeToCache('bar'); sleep(1); - $filePath3 = CachedEntry::writeToCache('baz', []); + $filePath3 = CachedEntry::writeToCache('baz'); sleep(1); - $filePath4 = CachedEntry::writeToCache('foo', []); + $filePath4 = CachedEntry::writeToCache('foo'); sleep(1); CachedEntry::deleteLeastAccessedFiles(2); @@ -242,13 +242,13 @@ public function test_deleteLeastAccessedFiles_deletesOnlyOldest() public function test_deleteLeastAccessedFiles_deletesOnlyOldest2() { - $filePath1 = CachedEntry::writeToCache('file', []); + $filePath1 = CachedEntry::writeToCache('file'); sleep(1); - $filePath2 = CachedEntry::writeToCache('bar', []); + $filePath2 = CachedEntry::writeToCache('bar'); sleep(1); - $filePath3 = CachedEntry::writeToCache('baz', []); + $filePath3 = CachedEntry::writeToCache('baz'); sleep(1); - $filePath4 = CachedEntry::writeToCache('foo', []); + $filePath4 = CachedEntry::writeToCache('foo'); sleep(1); touch($filePath1); From 01cb5b432747286ec43b7c89855625c137105d53 Mon Sep 17 00:00:00 2001 From: Jacob R Date: Thu, 17 Sep 2026 13:18:03 +1200 Subject: [PATCH 7/7] Ship the shared pre-push hook [ignore_release] (#95) Byte-identical to hooks/pre-push in matomo-org/plugin-ci-workflows, with verify-hook turned on so the copy cannot drift from it silently. The hook dispatches on phpstan/phpstan.created.neon and phpstan.modified.neon and silently analyses nothing without them, so both ship alongside it. --- .git-hooks-matomo/pre-push | 346 ++++++++++++++++++++++++++++++++++ .github/workflows/ci.yml | 1 + phpstan/phpstan.created.neon | 6 + phpstan/phpstan.modified.neon | 5 + 4 files changed, 358 insertions(+) create mode 100755 .git-hooks-matomo/pre-push create mode 100644 phpstan/phpstan.created.neon create mode 100644 phpstan/phpstan.modified.neon diff --git a/.git-hooks-matomo/pre-push b/.git-hooks-matomo/pre-push new file mode 100755 index 00000000..7d06e117 --- /dev/null +++ b/.git-hooks-matomo/pre-push @@ -0,0 +1,346 @@ +#!/bin/bash + +# This hook is called with the following parameters: +# +# $1 -- Name of the remote to which the push is being done +# $2 -- URL to which the push is being done +# +# If pushing without using a named remote those arguments will be equal. +# +# Information about the commits which are being pushed is supplied as lines to +# the standard input in the form: +# +# + + + +### Check we're running in the context of a plugin and get helpful dir variables ### + +REPO_DIR="$(git rev-parse --show-toplevel)" +echo "Running pre-push hook in repo: $REPO_DIR" + +if [[ "$REPO_DIR" =~ /plugins/(.*) ]]; then + PLUGIN_PATH="plugins/${BASH_REMATCH[1]}/" +else + echo "Not inside a Matomo checkout's plugins/ directory, skipping PHPStan checks" + exit 0 +fi +MATOMO_DIR=$(echo "$REPO_DIR" | sed -E 's|/plugins/.*$||') + + + +### Figure out how to run PHPStan - ddev or not. ### + +COMMAND=() +# Use local PHP if setup +if command -v php >/dev/null 2>&1 && [ -f "${MATOMO_DIR}/vendor/bin/phpstan" ]; then + COMMAND=("${MATOMO_DIR}/vendor/bin/phpstan") + PLUGIN_PATH='' +elif command -v ddev >/dev/null 2>&1; then + # Fall back to ddev when there is no local PHPStan. Local takes priority: it is faster, + # and it is what the elif above actually encodes. + if [ -d "$MATOMO_DIR/.ddev" ]; then + cd "$MATOMO_DIR" || exit 1 + # `ddev status` exits 0 for a stopped project, so its exit code says nothing about whether + # the containers are up. `ddev describe -j` reports the real state. + if [[ "$(ddev describe -j 2>/dev/null | sed -n 's/.*"status":"\([a-z]*\)".*/\1/p' | head -1)" == "running" ]]; then + COMMAND=(ddev exec phpstan) + else + DDEV_STOPPED=1 + fi + fi +fi +# If no command, exit +if [[ ${#COMMAND[@]} -eq 0 ]]; then + if [[ "${DDEV_STOPPED:-0}" -eq 1 ]]; then + # The tooling exists and simply is not started. Blocking the push here teaches people to + # reach for --no-verify, which is worse than skipping one check. + echo "ddev is not running, so PHPStan was skipped. Run 'ddev start' to check before pushing." + exit 0 + fi + echo "No way to run phpstan found." + exit 1 +fi + + + +# Basic setup +cd "$REPO_DIR" || exit 1 +STATUS=0 +ZERO_OID='0000000000000000000000000000000000000000' +PHPSTAN_CREATED_CONFIG=phpstan/phpstan.created.neon +PHPSTAN_MODIFIED_CONFIG=phpstan/phpstan.modified.neon + + + +### Work out what a pushed commit should be compared against. ### + +# The nearest origin/.x-dev, measured in commits between the merge base and the push. +# origin/HEAD is wrong twice over: git records it at clone time and never refreshes it, so a clone +# made while the default was 5.x-dev still names 5.x-dev long after the plugin moved to 6.x-dev; +# and the default branch is not the base of a backport branch in any case. Both mistakes widen the +# diff to files the push never touched. Distance needs no network and no naming convention. +# +# Assigns MAIN_BRANCH and BASE_BRANCH_TIED instead of echoing: reading stdout needs a command +# substitution, and the subshell would throw the tie flag away. Tied means two majors are equally +# near -- the branch predates their divergence, so the merge base, and with it the file list, is +# the same either way and only the label is a guess. +# +# $1 -- the pushed commit +resolve_base_branch() { + local commit="$1" + local ref merge_base distance best_branch='' best_distance='' + + BASE_BRANCH_TIED=0 + for ref in $(git for-each-ref --format='%(refname:short)' 'refs/remotes/origin/*.x-dev'); do + merge_base=$(git merge-base "$commit" "$ref" 2>/dev/null) || continue + distance=$(git rev-list --count "${merge_base}..${commit}") + if [[ -z "$best_distance" || "$distance" -lt "$best_distance" ]]; then + best_distance=$distance + best_branch=${ref#origin/} + # A strictly closer candidate settles it, including over an earlier tie between two + # branches that both just lost. + BASE_BRANCH_TIED=0 + elif [[ "$distance" -eq "$best_distance" ]]; then + # for-each-ref sorts ascending, so taking the later ref means the highest major wins a tie. + best_branch=${ref#origin/} + BASE_BRANCH_TIED=1 + fi + done + + if [[ -n "$best_branch" ]]; then + MAIN_BRANCH=$best_branch + return 0 + fi + + # No .x-dev refs at all -- a single-branch clone, or a fork. Fall back to the remote's + # default branch, then to a fixed name for a clone that can reach neither. Neither says anything + # about the target major, so treat it as tied. + local fallback + fallback=$(git symbolic-ref --quiet --short refs/remotes/origin/HEAD 2>/dev/null | sed 's|^origin/||') + if [[ -z "$fallback" ]]; then + fallback=$(git remote show origin 2>/dev/null | sed -n 's/.*HEAD branch: //p') + fi + BASE_BRANCH_TIED=1 + MAIN_BRANCH=${fallback:-5.x-dev} +} + +# PHPStan analyses the plugin against whichever Matomo checkout happens to contain it, which is not +# necessarily the major the push is based on. A 6.x-dev branch sitting in a Matomo 5 checkout is +# analysed against Matomo 5, and the findings look entirely real -- correct files, correct line +# numbers -- for signatures that simply differ between the majors. Warn rather than fail: the +# mismatch is sometimes deliberate, and a hard failure on a guess is what teaches --no-verify. +# +# $1 -- the base branch resolved for the push +WARNED_BASE_BRANCHES='' +warn_on_core_major_mismatch() { + local base_branch="$1" + local core_version_file="$MATOMO_DIR/core/Version.php" + local core_major branch_major + + [ -f "$core_version_file" ] || return 0 + # A tied resolution did not establish a target major, so there is nothing to compare against. + [ "${BASE_BRANCH_TIED:-0}" -eq 0 ] || return 0 + # Once per base branch, not once per pushed ref. + case " $WARNED_BASE_BRANCHES " in *" $base_branch "*) return 0 ;; esac + WARNED_BASE_BRANCHES="$WARNED_BASE_BRANCHES $base_branch" + + core_major=$(sed -n "s/.*const VERSION = '\([0-9]\{1,\}\)\..*/\1/p" "$core_version_file" | head -1) + # Only `.x-dev` says anything about the target major; any other branch name is left alone. + branch_major=$(printf '%s' "$base_branch" | sed -n 's/^\([0-9]\{1,\}\)\.x-dev$/\1/p') + [ -n "$core_major" ] && [ -n "$branch_major" ] && [ "$core_major" != "$branch_major" ] || return 0 + + echo + echo "WARNING: analysing against Matomo ${core_major}.x in $MATOMO_DIR, but this push is based" + echo " on $base_branch. Findings below may not match CI, which analyses against Matomo" + echo " ${branch_major}.x. Check a finding against a Matomo ${branch_major}.x checkout" + echo " before acting on it." + echo +} + + + +### Run PHPStan on the files a pushed commit adds or changes. ### + +# $1 -- the pushed commit +# $2 -- git diff filter (A for created files, CMR for modified files; R matters because a +# renamed-and-modified file has status R and would otherwise skip the check) +# $3 -- the phpstan config to use +# $4 -- log label for the file kind +check_pushed_commit() { + local commit="$1" filter="$2" config="$3" label="$4" + + if [[ ! -f "$config" ]]; then + return 0 + fi + + # Use the merge base with the remote base branch: the local branch can be stale + # or missing, which silently widens the diff to files the push doesn't touch. + local diff_base + diff_base=$(git merge-base "$commit" "origin/${MAIN_BRANCH}" 2>/dev/null) + if [[ -z "$diff_base" ]]; then + echo "Could not resolve the merge base between ${commit} and origin/${MAIN_BRANCH}." + echo "Run 'git fetch origin ${MAIN_BRANCH}' and push again." + return 1 + fi + + # Read NUL-delimited so a path containing a space stays one argument. Quoting the paths and + # piping through xargs does not: xargs strips the quotes it was given, then splits on the space. + local changed_files=() + local file + while IFS= read -r -d '' file; do + [[ "$file" == *.php ]] && changed_files+=("${PLUGIN_PATH}${file}") + done < <(git diff --name-only -z "$diff_base" "$commit" --diff-filter="$filter") + + if [[ ${#changed_files[@]} -eq 0 ]]; then + echo "No ${label} PHP files" + return 0 + fi + + echo "Running PHPstan on ${label} files" + + local out_file err_file status + out_file=$(mktemp) || { echo "Could not create a temporary file to capture the analysis" >&2; return 1; } + err_file=$(mktemp) || { rm -f "$out_file"; echo "Could not create a temporary file to capture the analysis" >&2; return 1; } + + # The exemption below matches a line of stderr, so the analyser must not be allowed to reshape it. + # All three of these were verified against 2.2.9 to break the match and block the push this exists + # to let through: + # --no-ansi PHPStan decorates its output whenever MSYSTEM and TERM=xterm are set (Git Bash + # sets both), even into a file, wrapping the line in colour codes. + # COLUMNS=120 Symfony wraps its error block to the terminal width, and an exported COLUMNS + # below ~36 splits the line in two. Only the local-PHP path inherits this, which + # is also the only path that can inherit a small COLUMNS in the first place. + # --no-progress both streams are captured, so the bar can never render live; without this the + # run ends by dumping a dead progress bar into the output. + COLUMNS=120 "${COMMAND[@]}" analyse --no-ansi --no-progress -c "${PLUGIN_PATH}${config}" \ + "${changed_files[@]}" > "$out_file" 2> "$err_file" + status=$? + + # PHPStan reports "nothing to analyse" on stderr and leaves stdout empty, while a run that + # analysed anything writes its result table to stdout. That pair is the only signal it offers: + # the exit code is 1 either way, and --error-format=json still emits this one as plain text + # (checked on 2.2.9). Matching stderr alone would accept a real failure whose own message + # happened to contain the phrase, which a custom rule is free to produce. + # + # Any other sign of failure on stderr withdraws the exemption too. That test is keyed to the + # shapes below rather than to stderr being otherwise empty, because Xdebug notices and PHP's own + # deprecation output land on stderr in ordinary dev environments -- treating those as failures + # would re-block precisely the pushes this exemption exists to let through. + # + # [ERROR] is the block PHPStan actually emits. [FATAL] and PHP's own fatals have not been seen + # alongside the no-files line, but a process that died is never a clean "nothing to analyse", and + # a fatal -- unlike a deprecation -- is never benign, so matching them cannot cost a false block. + local no_files_re='^[[:space:]]*(\[ERROR\][[:space:]]+)?No files found to analyse\.?[[:space:]]*$' + local failure_re='^[[:space:]]*(\[(ERROR|FATAL)\]|(PHP )?(Fatal|Parse) error:)' + local other_diagnostics + other_diagnostics=$(grep -E "$failure_re" "$err_file" | grep -vE "$no_files_re") + + if [[ "$status" -ne 0 ]] && [[ ! -s "$out_file" ]] \ + && grep -qE "$no_files_re" "$err_file" \ + && [[ -z "$other_diagnostics" ]] + then + # Reporting an [ERROR] on a push being allowed through is how a hook teaches people to stop + # reading its output, so the one line the message below restates in plain English is dropped -- + # along with the blank lines Symfony pads its block with, which would otherwise be all that + # survives on a quiet run. + # ddev wraps a non-zero exit in its own coloured "Failed to execute command ...: exit status 1", + # which --no-ansi cannot reach because it is ddev's line rather than PHPStan's. On an exempt run + # that is the failure being deliberately overridden. + # + # The escape is a shell literal rather than \x1b inside the sed script, because BSD and busybox + # sed leave \x1b unexpanded and Linux CI cannot catch that regression. LC_ALL=C keeps the + # substitution byte-oriented, so stderr carrying a non-UTF-8 path byte cannot abort it. + local esc=$'\033' + LC_ALL=C sed "s/${esc}\\[[0-9;]*m//g" "$err_file" \ + | grep -vE "$no_files_re" \ + | grep -vE '^Failed to execute command .*: exit status [0-9]+$' \ + | grep -v '^[[:space:]]*$' >&2 + # Name the files: an excludePaths that accidentally matches everything otherwise retires the + # hook as silently as the unset core.hooksPath the sibling audit reports. + echo "Every ${label} file is excluded by ${config}, so there is nothing to analyse: ${changed_files[*]}" + rm -f "$out_file" "$err_file" + return 0 + fi + + cat "$err_file" >&2 + cat "$out_file" + rm -f "$out_file" "$err_file" + return "$status" +} + +# Check the commits actually being pushed, as supplied on stdin: HEAD is wrong +# when pushing another local branch or several refs at once. The inner commands +# read /dev/null so they cannot consume the remaining stdin lines. +# shellcheck disable=SC2034 # remote_ref/remote_oid consume git's 4-field pre-push line +while read -r local_ref local_oid remote_ref remote_oid; do + if [[ "$local_oid" == "$ZERO_OID" ]]; then + continue # deleting the remote ref, nothing is pushed + fi + # Resolved per ref: one push can carry branches based on different majors. + resolve_base_branch "$local_oid" + warn_on_core_major_mismatch "$MAIN_BRANCH" + echo "Checking ${local_ref} (${local_oid}) against origin/${MAIN_BRANCH}" + check_pushed_commit "$local_oid" A "$PHPSTAN_CREATED_CONFIG" "created" < /dev/null || STATUS=1 + # CMR, not CM: a renamed-and-modified PHP file has status R and would otherwise skip the check. + check_pushed_commit "$local_oid" CMR "$PHPSTAN_MODIFIED_CONFIG" "modified" < /dev/null || STATUS=1 +done + +# Don't bother running the full check, as we check changes files already, and +# can assume that the unchanged files don't need rechecking. +# +# Github will check this anyway. +# +# PHPSTAN_BASE_CONFIG=phpstan.neon +# if [[ -f "$PHPSTAN_BASE_CONFIG" ]]; then +# echo "Running PHPstan at a base level on all plugin files" +# $COMMAND analyse -c ${PLUGIN_PATH}/${PHPSTAN_BASE_CONFIG} || STATUS=1 +# fi + +# A plugin whose core.hooksPath is unset has this file and no way to reach it, and nothing runs to +# say so -- which is exactly why four plugins went a year without the check ever firing. A hook that +# does run can see its siblings, so the working ones report the silent ones. +# +# Only plugins that ship the file are considered: a plugin without one has no hook to activate, and +# pointing core.hooksPath at a directory that does not exist would be worse than leaving it alone -- +# git then runs no hook at all, including anything the repository keeps in .git/hooks, and says +# nothing about it. +# +# Advisory, and at most once a day. Someone else's configuration is not grounds to fail a push. +audit_sibling_plugins() { + local plugins_dir="${MATOMO_DIR}/plugins" + local marker="${MATOMO_DIR}/tmp/.matomo-hook-audit" + local dir inactive=() + + [ -d "$plugins_dir" ] || return 0 + + # `find -mmin` rather than `stat`, whose format flags differ between GNU and BSD. + if [ -f "$marker" ] && [ -z "$(find "$marker" -mmin +1440 2>/dev/null)" ]; then + return 0 + fi + if mkdir -p "${MATOMO_DIR}/tmp" 2>/dev/null; then + : > "$marker" 2>/dev/null || true + fi + + for dir in "$plugins_dir"/*/; do + [ -f "${dir}.git-hooks-matomo/pre-push" ] || continue + git -C "$dir" rev-parse --git-dir >/dev/null 2>&1 || continue + [ -n "$(git -C "$dir" config --get core.hooksPath 2>/dev/null)" ] && continue + inactive+=("$(basename "$dir")") + done + + [ ${#inactive[@]} -eq 0 ] && return 0 + + echo + echo "NOTE: ${#inactive[@]} plugin(s) ship a pre-push hook that never runs, because" + echo " core.hooksPath is not set in them: ${inactive[*]}" + echo " Activate with add-git-hooks-to-plugins.sh from matomo-developer-tools." + echo +} + +# Only on a push that is going through: a rejected push's output should stay about the rejection. +if [[ $STATUS -eq 0 ]]; then + audit_sibling_plugins +fi + +exit $STATUS diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fdb9b3bf..d72ace73 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,3 +18,4 @@ jobs: uses: matomo-org/plugin-ci-workflows/.github/workflows/plugin-ci.yml@main with: plugin-name: DeviceDetectorCache + verify-hook: true diff --git a/phpstan/phpstan.created.neon b/phpstan/phpstan.created.neon new file mode 100644 index 00000000..c4a28c90 --- /dev/null +++ b/phpstan/phpstan.created.neon @@ -0,0 +1,6 @@ +includes: + - ../phpstan.neon +parameters: + # new files carry no pre-existing debt, so hold them to the strictest level + level: 9 + tmpDir: /tmp/phpstan/DeviceDetectorCache/created diff --git a/phpstan/phpstan.modified.neon b/phpstan/phpstan.modified.neon new file mode 100644 index 00000000..0143d67f --- /dev/null +++ b/phpstan/phpstan.modified.neon @@ -0,0 +1,5 @@ +includes: + - ../phpstan.neon +parameters: + level: 5 + tmpDir: /tmp/phpstan/DeviceDetectorCache/modified