From ca53c35eb32bcb4b7b6a760c7c31abfb0007db52 Mon Sep 17 00:00:00 2001 From: Jacob Ransom Date: Tue, 4 Aug 2026 10:15:28 +1200 Subject: [PATCH 1/8] Diff against the merge base with origin/5.x-dev in the pre-push hook [no_release] --- .git-hooks-matomo/pre-push | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/.git-hooks-matomo/pre-push b/.git-hooks-matomo/pre-push index c0e95b94..40ee12ba 100755 --- a/.git-hooks-matomo/pre-push +++ b/.git-hooks-matomo/pre-push @@ -60,12 +60,25 @@ STATUS=0 +### Resolve the base to diff against. ### + +# Use the merge base with the remote main branch: the local branch can be stale +# or missing, which silently widens the diff to files the push doesn't touch. +MAIN_BRANCH='5.x-dev' +DIFF_BASE=$(git merge-base HEAD "origin/${MAIN_BRANCH}" 2>/dev/null) +if [[ -z "$DIFF_BASE" ]]; then + echo "Could not resolve the merge base between HEAD and origin/${MAIN_BRANCH}." + echo "Run 'git fetch origin ${MAIN_BRANCH}' and push again." + exit 1 +fi + + + ### Run PHPStan on newly created files. ### PHPSTAN_CREATED_CONFIG=phpstan/phpstan.created.neon -MAIN_BRANCH='5.x-dev' if [[ -f "$PHPSTAN_CREATED_CONFIG" ]]; then - CHANGED_FILES=$(git diff --name-only ${MAIN_BRANCH} --diff-filter=A | grep '\.php$' || true) + CHANGED_FILES=$(git diff --name-only ${DIFF_BASE} --diff-filter=A | grep '\.php$' || true) if [ -z "$CHANGED_FILES" ]; then echo "No created PHP files" else @@ -80,7 +93,7 @@ fi ### Run PHPStan on modified files. ### PHPSTAN_MODIFIED_CONFIG=phpstan/phpstan.modified.neon if [[ -f "$PHPSTAN_MODIFIED_CONFIG" ]]; then - CHANGED_FILES=$(git diff --name-only ${MAIN_BRANCH} --diff-filter=CM | grep '\.php$' || true) + CHANGED_FILES=$(git diff --name-only ${DIFF_BASE} --diff-filter=CM | grep '\.php$' || true) if [ -z "$CHANGED_FILES" ]; then echo "No changed PHP files" else From 871d97ac2ac9e89d3b19caaef68167ddbf769234 Mon Sep 17 00:00:00 2001 From: Jacob Ransom Date: Tue, 4 Aug 2026 11:31:58 +1200 Subject: [PATCH 2/8] Align the modified-files hook level with the CI base level [no_release] A higher level in the pre-push hook than in CI blocks pushes on pre-existing errors in touched files that CI accepts. --- phpstan/phpstan.modified.neon | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/phpstan/phpstan.modified.neon b/phpstan/phpstan.modified.neon index fc9d1a45..a9e18cbb 100644 --- a/phpstan/phpstan.modified.neon +++ b/phpstan/phpstan.modified.neon @@ -1,5 +1,7 @@ includes: - ../phpstan.neon parameters: - level: 5 + # keep in sync with ../phpstan.neon: a higher level here makes the pre-push + # hook block on pre-existing errors in files a push merely touches + level: 1 tmpDir: /tmp/phpstan/GoogleAnalyticsImporter/modified From 5fc57b8a9749c73eb0ffeb5fc1a64a87680196f8 Mon Sep 17 00:00:00 2001 From: Jacob Ransom Date: Tue, 4 Aug 2026 11:47:11 +1200 Subject: [PATCH 3/8] Check newly created files at PHPStan level 9 [no_release] New files carry no pre-existing debt, so hold them to the strictest level. --- phpstan/phpstan.created.neon | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/phpstan/phpstan.created.neon b/phpstan/phpstan.created.neon index e9a7cc9b..1ddc9b14 100644 --- a/phpstan/phpstan.created.neon +++ b/phpstan/phpstan.created.neon @@ -1,5 +1,6 @@ includes: - ../phpstan.neon parameters: - level: 5 + # new files carry no pre-existing debt, so hold them to the strictest level + level: 9 tmpDir: /tmp/phpstan/GoogleAnalyticsImporter/created From b2fa97ac144c5adb2131bf14522aa165c8a88613 Mon Sep 17 00:00:00 2001 From: Jacob Ransom Date: Tue, 4 Aug 2026 12:04:19 +1200 Subject: [PATCH 4/8] Set the modified-files hook level to the plugin-wide goal of 5 [no_release] Touched files are ratcheted up to level 5 even where the CI base is lower; pre-existing findings in a touched file get fixed with the change that touches it. --- phpstan/phpstan.modified.neon | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/phpstan/phpstan.modified.neon b/phpstan/phpstan.modified.neon index a9e18cbb..c6524a25 100644 --- a/phpstan/phpstan.modified.neon +++ b/phpstan/phpstan.modified.neon @@ -1,7 +1,7 @@ includes: - ../phpstan.neon parameters: - # keep in sync with ../phpstan.neon: a higher level here makes the pre-push - # hook block on pre-existing errors in files a push merely touches - level: 1 + # touched files are ratcheted up to the plugin-wide goal of level 5, + # even where the CI base level is lower + level: 5 tmpDir: /tmp/phpstan/GoogleAnalyticsImporter/modified From 1fc1605e073c17ac3b19a541504fbff68ada4265 Mon Sep 17 00:00:00 2001 From: Jacob Ransom Date: Tue, 4 Aug 2026 13:52:06 +1200 Subject: [PATCH 5/8] Check pushed commits rather than the working tree in the pre-push hook Uncommitted local changes were analyzed and could block a push for files the push doesn't contain. Also corrects the hook's log label. --- .git-hooks-matomo/pre-push | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.git-hooks-matomo/pre-push b/.git-hooks-matomo/pre-push index 40ee12ba..bcbefd33 100755 --- a/.git-hooks-matomo/pre-push +++ b/.git-hooks-matomo/pre-push @@ -17,7 +17,7 @@ ### 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-commit hook in repo: $REPO_DIR" +echo "Running pre-push hook in repo: $REPO_DIR" if [[ "$REPO_DIR" =~ /plugins/(.*) ]]; then PLUGIN_PATH="plugins/${BASH_REMATCH[1]}/" @@ -78,7 +78,7 @@ fi PHPSTAN_CREATED_CONFIG=phpstan/phpstan.created.neon if [[ -f "$PHPSTAN_CREATED_CONFIG" ]]; then - CHANGED_FILES=$(git diff --name-only ${DIFF_BASE} --diff-filter=A | grep '\.php$' || true) + CHANGED_FILES=$(git diff --name-only ${DIFF_BASE} HEAD --diff-filter=A | grep '\.php$' || true) if [ -z "$CHANGED_FILES" ]; then echo "No created PHP files" else @@ -93,7 +93,7 @@ fi ### Run PHPStan on modified files. ### PHPSTAN_MODIFIED_CONFIG=phpstan/phpstan.modified.neon if [[ -f "$PHPSTAN_MODIFIED_CONFIG" ]]; then - CHANGED_FILES=$(git diff --name-only ${DIFF_BASE} --diff-filter=CM | grep '\.php$' || true) + CHANGED_FILES=$(git diff --name-only ${DIFF_BASE} HEAD --diff-filter=CM | grep '\.php$' || true) if [ -z "$CHANGED_FILES" ]; then echo "No changed PHP files" else From 690ac3ad1bff5cbd8c47eaec636d34a38f0a09d0 Mon Sep 17 00:00:00 2001 From: Jacob Ransom Date: Tue, 4 Aug 2026 14:12:38 +1200 Subject: [PATCH 6/8] Analyse the commits supplied on the hook's stdin rather than HEAD Pushing another local branch, or several refs at once, analysed the checked-out branch instead of what was actually pushed. --- .git-hooks-matomo/pre-push | 79 +++++++++++++++++++++----------------- 1 file changed, 43 insertions(+), 36 deletions(-) diff --git a/.git-hooks-matomo/pre-push b/.git-hooks-matomo/pre-push index bcbefd33..54bb7e5b 100755 --- a/.git-hooks-matomo/pre-push +++ b/.git-hooks-matomo/pre-push @@ -56,52 +56,59 @@ fi # Basic setup cd "$REPO_DIR" STATUS=0 - - - - -### Resolve the base to diff against. ### - -# Use the merge base with the remote main branch: the local branch can be stale -# or missing, which silently widens the diff to files the push doesn't touch. MAIN_BRANCH='5.x-dev' -DIFF_BASE=$(git merge-base HEAD "origin/${MAIN_BRANCH}" 2>/dev/null) -if [[ -z "$DIFF_BASE" ]]; then - echo "Could not resolve the merge base between HEAD and origin/${MAIN_BRANCH}." - echo "Run 'git fetch origin ${MAIN_BRANCH}' and push again." - exit 1 -fi +ZERO_OID='0000000000000000000000000000000000000000' +PHPSTAN_CREATED_CONFIG=phpstan/phpstan.created.neon +PHPSTAN_MODIFIED_CONFIG=phpstan/phpstan.modified.neon -### Run PHPStan on newly created files. ### +### Run PHPStan on the files a pushed commit adds or changes. ### -PHPSTAN_CREATED_CONFIG=phpstan/phpstan.created.neon -if [[ -f "$PHPSTAN_CREATED_CONFIG" ]]; then - CHANGED_FILES=$(git diff --name-only ${DIFF_BASE} HEAD --diff-filter=A | grep '\.php$' || true) - if [ -z "$CHANGED_FILES" ]; then - echo "No created PHP files" - else - echo "Running PHPstan at a very high level on new files" - CHANGED_FILES=`echo "$CHANGED_FILES" | sed -e 's/^\(.*\)$/"\1"/' | xargs -I{} echo "${PLUGIN_PATH}{}"` - echo "$CHANGED_FILES" | xargs $COMMAND analyse -c ${PLUGIN_PATH}${PHPSTAN_CREATED_CONFIG} || STATUS=1 +# $1 -- the pushed commit +# $2 -- git diff filter (A for created files, CM for modified files) +# $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 -fi + # Use the merge base with the remote main 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 + local changed_files + changed_files=$(git diff --name-only "$diff_base" "$commit" --diff-filter="$filter" | grep '\.php$' || true) + if [ -z "$changed_files" ]; then + echo "No ${label} PHP files" + return 0 + fi -### Run PHPStan on modified files. ### -PHPSTAN_MODIFIED_CONFIG=phpstan/phpstan.modified.neon -if [[ -f "$PHPSTAN_MODIFIED_CONFIG" ]]; then - CHANGED_FILES=$(git diff --name-only ${DIFF_BASE} HEAD --diff-filter=CM | grep '\.php$' || true) - if [ -z "$CHANGED_FILES" ]; then - echo "No changed PHP files" - else - echo "Running PHPstan on modified files" - CHANGED_FILES=`echo "$CHANGED_FILES" | sed -e 's/^\(.*\)$/"\1"/' | xargs -I{} echo "${PLUGIN_PATH}{}"` - echo "$CHANGED_FILES" | xargs $COMMAND analyse -c ${PLUGIN_PATH}${PHPSTAN_MODIFIED_CONFIG} || STATUS=1 + echo "Running PHPstan on ${label} files" + changed_files=`echo "$changed_files" | sed -e 's/^\(.*\)$/"\1"/' | xargs -I{} echo "${PLUGIN_PATH}{}"` + echo "$changed_files" | xargs $COMMAND analyse -c ${PLUGIN_PATH}${config} || return 1 +} + +# 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. +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 -fi + echo "Checking ${local_ref} (${local_oid})" + check_pushed_commit "$local_oid" A "$PHPSTAN_CREATED_CONFIG" "created" < /dev/null || STATUS=1 + check_pushed_commit "$local_oid" CM "$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. From d3c2e4d9285d07caf1268dc03a6ebea0717b36c9 Mon Sep 17 00:00:00 2001 From: Jacob Ransom Date: Tue, 4 Aug 2026 14:47:23 +1200 Subject: [PATCH 7/8] Skip the hook instead of rejecting pushes from standalone clones A plugin cloned outside a Matomo checkout cannot be checked, which is no reason to block its pushes. --- .git-hooks-matomo/pre-push | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.git-hooks-matomo/pre-push b/.git-hooks-matomo/pre-push index 54bb7e5b..eb24a2ca 100755 --- a/.git-hooks-matomo/pre-push +++ b/.git-hooks-matomo/pre-push @@ -22,8 +22,8 @@ echo "Running pre-push hook in repo: $REPO_DIR" if [[ "$REPO_DIR" =~ /plugins/(.*) ]]; then PLUGIN_PATH="plugins/${BASH_REMATCH[1]}/" else - echo "Not a plugin, not running any further checks" - exit 1 + echo "Not inside a Matomo checkout's plugins/ directory, skipping PHPStan checks" + exit 0 fi MATOMO_DIR=$(echo "$REPO_DIR" | sed -E 's|/plugins/.*$||') From 909de5adc1cbe24ad740493f23f60408a548fe0d Mon Sep 17 00:00:00 2001 From: Jacob Ransom Date: Thu, 27 Aug 2026 16:24:47 +1200 Subject: [PATCH 8/8] Call the shared PHPStan workflow The ~85-line workflow is now maintained centrally in matomo-org/plugin-ci-workflows, so each caller passes only its plugin name and any dependent plugins. The PHP version comes from the shared default (matomo6_min_php), which is what this branch needs now that it targets 6.x-dev. Where a path is excluded that the shared workflow no longer creates - github-action-tests/ lived inside the plugin only because the old workflow checked it out there - it is marked optional, because PHPStan treats a missing excludePath as a fatal config error rather than a no-op. --- .github/workflows/phpstan.yml | 86 +++-------------------------------- phpstan.neon | 2 +- 2 files changed, 7 insertions(+), 81 deletions(-) diff --git a/.github/workflows/phpstan.yml b/.github/workflows/phpstan.yml index 9762ff92..73b0b2f9 100644 --- a/.github/workflows/phpstan.yml +++ b/.github/workflows/phpstan.yml @@ -3,87 +3,13 @@ name: PHPStan 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 - -env: - PLUGIN_NAME: GoogleAnalyticsImporter - DEPENDENT_PLUGINS: matomo-org/plugin-MarketingCampaignsReporting innocraft/plugin-Funnels innocraft/plugin-ConnectAccounts jobs: phpstan: - name: PHPStan - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - with: - lfs: false - persist-credentials: false - - name: Setup PHP - uses: shivammathur/setup-php@v2 - with: - # 8.2 rather than 7.2: the ConnectAccounts dependency uses PHP 8 - # attribute syntax, which fatals when loaded on 7.2. The analysis - # level stays PHP 7.2 via phpVersion in phpstan.neon. - php-version: '8.2' - - - name: Check out github-action-tests repository - uses: actions/checkout@v4 - with: - repository: matomo-org/github-action-tests - ref: main - path: github-action-tests - persist-credentials: false - - - name: checkout matomo for plugin builds - shell: bash - run: ${{ github.workspace }}/github-action-tests/scripts/bash/checkout_matomo.sh - env: - PLUGIN_NAME: ${{ env.PLUGIN_NAME }} - WORKSPACE: ${{ github.workspace }} - ACTION_PATH: ${{ github.workspace }}/github-action-tests - MATOMO_TEST_TARGET: maximum_supported_matomo - - - name: prepare setup - shell: bash - run: | - cd ${{ github.workspace }}/matomo - echo -e "composer install" - composer install --ignore-platform-reqs - - - name: checkout additional plugins - if: ${{ env.DEPENDENT_PLUGINS != '' }} - shell: bash - working-directory: ${{ github.workspace }}/matomo - run: ${{ github.workspace }}/github-action-tests/scripts/bash/checkout_dependent_plugins.sh - - env: - DEPENDENT_PLUGINS: ${{ env.DEPENDENT_PLUGINS }} - GITHUB_USER_TOKEN: ${{ secrets.TESTS_ACCESS_TOKEN || secrets.GITHUB_TOKEN }} - - - name: "Restore result cache" - uses: actions/cache/restore@v4 - with: - path: /tmp/phpstan # same as in phpstan.neon - key: "phpstan-result-cache-${{ github.run_id }}" - restore-keys: | - phpstan-result-cache- - - - name: PHPStan whole repo - id: phpstan-all - run: cd ${{ github.workspace }}/matomo && composer run phpstan -- -vvv -c plugins/${{ env.PLUGIN_NAME }}/phpstan.neon - - - name: "Save result cache" - uses: actions/cache/save@v4 - if: ${{ !cancelled() }} - with: - path: /tmp/phpstan # same as in phpstan.neon - key: "phpstan-result-cache-${{ github.run_id }}" + uses: matomo-org/plugin-ci-workflows/.github/workflows/plugin-phpstan.yml@main + with: + plugin-name: GoogleAnalyticsImporter + dependent-plugins: 'matomo-org/plugin-MarketingCampaignsReporting innocraft/plugin-Funnels innocraft/plugin-ConnectAccounts' + secrets: + TESTS_ACCESS_TOKEN: ${{ secrets.TESTS_ACCESS_TOKEN }} diff --git a/phpstan.neon b/phpstan.neon index d7cae372..cdc521dc 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -12,7 +12,7 @@ parameters: # still scanned so the prefixed Google classes are discovered even # when the Matomo bootstrap cannot load plugin autoloaders (e.g. CI) analyse: - - vendor/ + - vendor/ (?) ignoreErrors: # WpMatomo and get_option exist only when running inside Matomo for WordPress - '#^Instantiated class WpMatomo\\Site not found\.$#'