Add a reusable PHPStan workflow and the canonical pre-push hook, PG-4897

.github/workflows/plugin-phpstan.yml

@@ -0,0 +1,120 @@
+name: PHPStan check
+
+on:
+  workflow_call:
+    inputs:
+      plugin-name:
+        description: "Name of the plugin, e.g. LoginLdap"
+        required: true
+        type: string
+      dependent-plugins:
+        description: "Space-separated repository slugs to check out, e.g. 'innocraft/plugin-Funnels'"
+        required: false
+        type: string
+        default: ''
+      php-version:
+        description: "PHP version the job runs on; the analysis level and phpVersion come from the plugin's phpstan.neon"
+        required: false
+        type: string
+        default: '7.2'
+      scripts-ref:
+        description: "Ref of matomo-org/github-action-tests to take the helper scripts from. When pinning the workflow to a SHA, pass the same SHA here."
+        required: false
+        type: string
+        default: 'main'
+      verify-hook:
+        description: "Fail when the plugin's .git-hooks-matomo/pre-push differs from the canonical copy in this repository"
+        required: false
+        type: boolean
+        default: true
+    secrets:
+      TESTS_ACCESS_TOKEN:
+        required: false
+
+# The job only reads the two repositories; everything else stays at none so any
+# caller with default workflow permissions can use this without granting scopes.
+permissions:
+  contents: read
+
+jobs:
+  phpstan:
+    name: PHPStan
+    runs-on: ubuntu-latest
+    env:
+      PLUGIN_NAME: ${{ inputs.plugin-name }}
+      DEPENDENT_PLUGINS: ${{ inputs.dependent-plugins }}
+    steps:
+      - name: Validate inputs
+        run: |
+          if [[ ! "$PLUGIN_NAME" =~ ^[A-Za-z0-9_]+$ ]]; then
+            echo "Invalid plugin-name: $PLUGIN_NAME"
+            exit 1
+          fi
+
+      - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0
+        with:
+          lfs: false
+          persist-credentials: false
+      - name: Setup PHP
+        uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # 2.37.2
+        with:
+          php-version: ${{ inputs.php-version }}
+
+      # No GitHub context identifies the called workflow's own commit (verified
+      # empirically: github.job_workflow_sha evaluates to empty), so pinning the
+      # scripts is the caller's job via scripts-ref.
+      - name: Check out github-action-tests repository
+        uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0
+        with:
+          repository: matomo-org/github-action-tests
+          ref: ${{ inputs.scripts-ref }}
+          path: github-action-tests
+          persist-credentials: false
+
+      - name: Check the pre-push hook matches the canonical copy
+        if: ${{ inputs.verify-hook }}
+        shell: bash
+        run: diff -u github-action-tests/hooks/pre-push .git-hooks-matomo/pre-push
+
+      - name: checkout matomo for plugin builds
+        shell: bash
+        run: ${{ github.workspace }}/github-action-tests/scripts/bash/checkout_matomo.sh
+        env:
+          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: ${{ inputs.dependent-plugins != '' }}
+        shell: bash
+        working-directory: ${{ github.workspace }}/matomo
+        run: ${{ github.workspace }}/github-action-tests/scripts/bash/checkout_dependent_plugins.sh
+        env:
+          GITHUB_USER_TOKEN: ${{ secrets.TESTS_ACCESS_TOKEN || github.token }}
+          TARGET_BRANCH: ${{ github.base_ref || github.ref_name }}
+
+      - name: "Restore result cache"
+        uses: actions/cache/restore@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0
+        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/${PLUGIN_NAME}/phpstan.neon"
+
+      - name: "Save result cache"
+        uses: actions/cache/save@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0
+        if: ${{ !cancelled() }}
+        with:
+          path: /tmp/phpstan # same as in phpstan.neon
+          key: "phpstan-result-cache-${{ github.run_id }}"

README.md

@@ -154,3 +154,37 @@ This action is able to run certain test suites for Matomo or any Matomo plugin.
           dependent-plugins: 'slug/plugin-AdditionalPlugin'
           github-token: ${{ secrets.TESTS_ACCESS_TOKEN || secrets.GITHUB_TOKEN }}
 ```
+
+## Reusable workflows
+
+### PHPStan (`.github/workflows/plugin-phpstan.yml`)
+
+Runs the plugin's own `phpstan.neon` against a full Matomo checkout. A plugin repository's
+`.github/workflows/phpstan.yml` reduces to:
+
+```yaml
+name: PHPStan check
+on: pull_request
+jobs:
+  phpstan:
+    uses: matomo-org/github-action-tests/.github/workflows/plugin-phpstan.yml@main
+    with:
+      plugin-name: MyPlugin
+      # dependent-plugins: 'innocraft/plugin-Funnels'
+      # php-version: '8.2'
+    secrets:
+      TESTS_ACCESS_TOKEN: ${{ secrets.TESTS_ACCESS_TOKEN }}
+```
+
+The example uses `@main` to match how plugin repositories currently consume this repository's
+scripts. For immutability, pin the `uses:` reference to a full commit SHA — release tags stay
+mutable unless the repository enforces immutable releases — and pass the same SHA as
+`scripts-ref`, which the workflow uses to check out its helper scripts (default: `main`).
+
+## Git hooks (`hooks/`)
+
+`hooks/pre-push` is the canonical copy of the PHPStan pre-push hook that plugin repositories
+carry as `.git-hooks-matomo/pre-push` (developers opt in with
+`git config core.hooksPath .git-hooks-matomo`). Hooks must exist as local files, so plugins keep
+a copy; the reusable PHPStan workflow fails when a plugin's copy drifts from the canonical one
+(disable with `verify-hook: false`).

hooks/pre-push

@@ -0,0 +1,124 @@
+#!/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:
+#
+#   <local ref> <local oid> <remote ref> <remote oid>
+
+
+
+### 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
+  # Use ddev if setup (overridding local setup)
+  if [ -d "$MATOMO_DIR/.ddev" ]; then
+    cd "$MATOMO_DIR" || exit 1
+    if ddev status 2>&1 > /dev/null; then
+      COMMAND="ddev exec phpstan"
+    fi
+  fi
+fi
+# If no command, exit
+if [[ -z "$COMMAND" ]]; then
+  echo "No way to run phpstan found."
+  exit 1
+fi
+
+
+
+# Basic setup
+cd "$REPO_DIR"
+STATUS=0
+MAIN_BRANCH='5.x-dev'
+ZERO_OID='0000000000000000000000000000000000000000'
+PHPSTAN_CREATED_CONFIG=phpstan/phpstan.created.neon
+PHPSTAN_MODIFIED_CONFIG=phpstan/phpstan.modified.neon
+
+
+
+### Run PHPStan on the files a pushed commit adds or changes. ###
+
+# $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
+
+  # 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
+
+  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
+  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.
+#
+# 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
+
+exit $STATUS

scripts/bash/checkout_dependent_plugins.sh

@@ -9,11 +9,13 @@ else
   echo ""
   PLUGINS=($DEPENDENT_PLUGINS)
   for pluginSlug in ${PLUGINS[@]}; do
-    dependentPluginName=$(echo "$pluginSlug" | sed -E 's/[a-zA-Z0-9_-]+\/[a-zA-Z0-9_]+-(.*)/\1/')
-
-    if [ "$dependentPluginName" == "" ]; then
+    # Both documented repository forms: owner/PluginName and owner/plugin-PluginName.
+    # The strict plugin-name charset keeps the derived path inside plugins/ below.
+    if [[ ! "$pluginSlug" =~ ^[A-Za-z0-9_.-]+/(plugin-)?([A-Za-z0-9_]+)$ ]]; then
+        echo "Skipping invalid dependent plugin slug: $pluginSlug"
         continue
     fi
+    dependentPluginName="${BASH_REMATCH[2]}"
 
     echo "Cloning $pluginSlug into plugins/$dependentPluginName..."