ci: scope PMD/CPD/Checkstyle to changed modules in the lint lane

joaodinissf · claude · joaodinissf · commit f08c43c2e155 · 2026-07-16T19:18:24.000+02:00
compute-spotbugs-skip.sh becomes compute-analysis-skip.sh with a mode
argument: `spotbugs` injects spotbugs.skip as before, `lint` injects
pmd.skip, cpd.skip and checkstyle.skip, and each mode exports its
-pl/-am reactor scope args. The lint lane gains the scope step and
passes LINT_SCOPE_ARGS to both invocations; `compile` stays in the
PMD/Checkstyle invocation because PMD's type-resolving rules need
Tycho's aux-classpath (skip-injected -am dependencies compile but are
not analysed). Changes under ddk-configuration (rulesets, filters) now
also trigger the full-scan fail-safe in both lanes.

Code Scanning note: repo-wide alert state reflects the default branch,
which receives no lint/spotbugs analyses (verify runs on pull_request
only), so a scoped upload that omits unchanged modules can only affect
PR-context annotations — the same property the spotbugs category has
had since the per-module skip landed.

Co-Authored-By: Claude Fable 5 &lt;noreply@anthropic.com&gt;
diff --git a/.github/scripts/compute-analysis-skip.sh b/.github/scripts/compute-analysis-skip.sh
@@ -1,29 +1,40 @@
 #!/usr/bin/env bash
 #
-# Scope SpotBugs to a pull request's changed modules.
+# Scope static analysis (SpotBugs, or PMD/CPD/Checkstyle) to a pull request's
+# changed modules.
 #
-# Default is RUN (analyze). On a PR this injects <spotbugs.skip>true</spotbugs.skip>
-# into every UNCHANGED reactor module's pom, so spotbugs-maven-plugin skips the goal —
-# and therefore the per-module JVM fork (SpotBugsMojo gates on `skip` before forking) —
-# for those modules. The full-reactor compile is left intact (a changed module is still
-# analysed with its complete aux-classpath). Master/snapshot builds run a full scan;
-# this script is invoked on pull_request only.
+# Default is RUN (analyze). On a PR this injects <TOOL.skip>true</TOOL.skip>
+# properties into every UNCHANGED reactor module's pom, so the analysis mojos
+# skip those modules — for SpotBugs that also skips the per-module JVM fork
+# (SpotBugsMojo gates on `skip` before forking). A changed module is still
+# analysed with its complete aux-classpath: the -am-pulled unchanged
+# dependencies compile but are not analysed. Master/snapshot builds run a full
+# scan; this script is invoked on pull_request only.
 #
 # Why this and not -Dspotbugs.onlyAnalyze: onlyAnalyze is one clean flag, but SpotBugs
 # applies its class screener too late (after the per-module fork + class scan), so it
 # only trimmed ~17% of the goal vs ~88% for this per-module skip (measured on this
 # reactor). A small upstream SpotBugs early-exit (skip the run when no application class
 # matches the screener) would make onlyAnalyze competitive; if that ever lands, switch
-# to onlyAnalyze and delete this script (tracked in #1455 / spotbugs/spotbugs#3796).
+# to onlyAnalyze and delete the spotbugs mode here (tracked in #1455 /
+# spotbugs/spotbugs#3796).
 #
-# On top of the skips, the changed reactor modules are exported as SPOTBUGS_SCOPE_ARGS
-# ("-pl <changed> -am") so the lane builds only those modules plus their upstream
-# dependencies instead of the full reactor. The -am-pulled unchanged dependencies still
-# carry the injected skip: they compile (complete aux-classpath) but are not analysed.
+# On top of the skips, the changed reactor modules are exported as
+# SPOTBUGS_SCOPE_ARGS / LINT_SCOPE_ARGS ("-pl <changed> -am") so the lane builds
+# only those modules plus their upstream dependencies instead of the full reactor.
+# The lane's gate cross-checks <MODE>_KEPT / <MODE>_EXPECT_REPORTS so a build
+# failure swallowed by --fail-never can never pass as "nothing to scan".
 #
-# Run from the repository root.  Usage: compute-spotbugs-skip.sh <base-sha>
+# Run from the repository root.  Usage: compute-analysis-skip.sh <base-sha> <spotbugs|lint>
 set -euo pipefail
 base="${1:?base sha required}"
+mode="${2:?mode required: spotbugs|lint}"
+
+case "$mode" in
+  spotbugs) props="spotbugs.skip"; prefix="SPOTBUGS" ;;
+  lint)     props="pmd.skip cpd.skip checkstyle.skip"; prefix="LINT" ;;
+  *) echo "unknown mode: $mode" >&2; exit 2 ;;
+esac
 
 changed=$(git diff --name-only --diff-filter=ACMR "${base}...HEAD")
 
@@ -45,8 +56,7 @@ ${module_dirs}
 EOF
 
 # 1) A change to shared build/config can affect any module -> full scan (skip nothing).
-#    ddk-configuration holds the analyzers' rulesets and filters (e.g. the SpotBugs
-#    exclusion-filter), so a change there must re-scan everything, not skip silently.
+#    ddk-configuration holds the analyzers' rulesets and filters, so it counts too.
 #    ddk-target defines the target platform every module resolves against.
 #    Fail safe: the worst case here is "analyse everything", never "analyse nothing".
 while IFS= read -r f; do
@@ -60,10 +70,10 @@ while IFS= read -r f; do
       # full scan analysed all of them (not just >=1) — a mojo death swallowed by
       # --fail-never can't pass as long as one sibling reported.
       if [ -n "${GITHUB_ENV:-}" ]; then
-        echo "SPOTBUGS_KEPT=all" >> "$GITHUB_ENV"
-        echo "SPOTBUGS_EXPECT_REPORTS=${all_source_modules}" >> "$GITHUB_ENV"
+        echo "${prefix}_KEPT=all" >> "$GITHUB_ENV"
+        echo "${prefix}_EXPECT_REPORTS=${all_source_modules}" >> "$GITHUB_ENV"
       fi
-      echo "Build/config change ($f) -> full SpotBugs scan (no skips)."
+      echo "Build/config change ($f) -> full ${mode} scan (no skips)."
       exit 0
       ;;
   esac
@@ -76,18 +86,20 @@ EOF
 #    grep's no-match exit would otherwise kill the script under pipefail.
 changed_mods=$(printf '%s\n' "${changed}" | { grep '/' || true; } | cut -d/ -f1 | sort -u)
 
-# 3) Idempotently inject the skip property; handle poms with and without <properties>.
+# 3) Idempotently inject the skip properties; handle poms with and without <properties>.
 #    sed -i.bak + rm is portable across GNU (CI) and BSD (local) sed.
 inject_skip() {
-  local pom="$1/pom.xml"
+  local pom="$1/pom.xml" prop
   [ -f "$pom" ] || return 0
-  if grep -q '<spotbugs\.skip>' "$pom"; then return 0; fi
-  if grep -q '<properties>' "$pom"; then
-    sed -i.bak 's#<properties>#<properties>\n    <spotbugs.skip>true</spotbugs.skip>#' "$pom"
-  else
-    sed -i.bak 's#</project>#  <properties>\n    <spotbugs.skip>true</spotbugs.skip>\n  </properties>\n</project>#' "$pom"
-  fi
-  rm -f "$pom.bak"
+  for prop in $props; do
+    if grep -q "<${prop//./\\.}>" "$pom"; then continue; fi
+    if grep -q '<properties>' "$pom"; then
+      sed -i.bak "s#<properties>#<properties>\n    <${prop}>true</${prop}>#" "$pom"
+    else
+      sed -i.bak "s#</project>#  <properties>\n    <${prop}>true</${prop}>\n  </properties>\n</project>#" "$pom"
+    fi
+    rm -f "$pom.bak"
+  done
 }
 
 # 4) Skip every reactor module that was not touched by this PR. Kept modules with a
@@ -103,7 +115,7 @@ while IFS= read -r mod; do
     kept=$((kept + 1))
     kept_pl="${kept_pl:+${kept_pl},}../${mod}"
     # Only bundles with sources reliably emit a report (a source-less bundle,
-    # e.g. pure branding, has nothing for the analyzer to write a SARIF about).
+    # e.g. pure branding, has nothing for PMD to write a SARIF about).
     if [ -f "${mod}/META-INF/MANIFEST.MF" ] && [ -d "${mod}/src" ]; then
       expect_reports="${expect_reports:+${expect_reports} }${mod}"
     fi
@@ -115,6 +127,13 @@ done <<EOF
 ${module_dirs}
 EOF
 
+# 5) Scope the reactor to the changed modules + their upstream dependencies. ddk-target
+#    is always kept in the -pl list: the target-definition artifact is referenced by
+#    target-platform-configuration, not by any MANIFEST, so -am never pulls it — without
+#    it in the reactor Tycho falls back to a local-repository copy, which fails on a
+#    cold cache and can silently resolve a stale target definition on a warm one.
+#    With no analysable changed module (docs-only, or source-less-only) no scope args
+#    are exported; the workflow skips the lane's Maven step(s) entirely on <MODE>_KEPT=0.
 # The gate's presence check needs to distinguish "all modules skip-injected"
 # (zero reports is the expected state) from "the analysis silently died".
 # If the only changed modules are source-less (feature / target / repository — nothing
@@ -127,25 +146,18 @@ else
   effective_kept=$kept
 fi
 if [ -n "${GITHUB_ENV:-}" ]; then
-  echo "SPOTBUGS_KEPT=${effective_kept}" >> "$GITHUB_ENV"
+  echo "${prefix}_KEPT=${effective_kept}" >> "$GITHUB_ENV"
 fi
 
-# 5) Scope the reactor to the changed modules + their upstream dependencies. ddk-target
-#    is always kept in the -pl list: the target-definition artifact is referenced by
-#    target-platform-configuration, not by any MANIFEST, so -am never pulls it — without
-#    it in the reactor Tycho falls back to a local-repository copy, which fails on a
-#    cold cache and can silently resolve a stale target definition on a warm one.
-#    With no analysable changed module (docs-only, or source-less-only) no scope args
-#    are exported; the workflow skips the Maven step entirely on SPOTBUGS_KEPT=0.
 if [ "$effective_kept" -gt 0 ] && [ -n "${GITHUB_ENV:-}" ]; then
-  echo "SPOTBUGS_SCOPE_ARGS=-pl ../ddk-target,${kept_pl} -am" >> "$GITHUB_ENV"
-  echo "SPOTBUGS_EXPECT_REPORTS=${expect_reports}" >> "$GITHUB_ENV"
+  echo "${prefix}_SCOPE_ARGS=-pl ../ddk-target,${kept_pl} -am" >> "$GITHUB_ENV"
+  echo "${prefix}_EXPECT_REPORTS=${expect_reports}" >> "$GITHUB_ENV"
 fi
 
-echo "SpotBugs scope: scanning ${effective_kept} changed module(s), skipping ${skipped} unchanged."
+echo "${mode} scope: scanning ${effective_kept} changed module(s), skipping ${skipped} unchanged."
 echo "Changed modules: ${changed_mods:-<none>}"
 if [ "$kept" -gt 0 ] && [ "$effective_kept" -eq 0 ]; then
-  echo "Only source-less modules changed (no analysable sources) -> no-op (KEPT=0)."
+  echo "Only source-less modules changed (no analysable sources) -> no-op (${prefix}_KEPT=0)."
 fi
 if [ "$effective_kept" -gt 0 ]; then
   echo "Reactor scope args: -pl ../ddk-target,${kept_pl} -am"
diff --git a/.github/workflows/verify.yml b/.github/workflows/verify.yml
@@ -28,6 +28,8 @@ jobs:
     runs-on: ubuntu-24.04
     steps:
       - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0  # v7.0.0
+        with:
+          fetch-depth: 0  # need the PR base commit to diff the changed modules
       - uses: actions/setup-java@0f481fcb613427c0f801b606911222b5b6f3083a  # v5
         with:
           distribution: 'temurin'
@@ -43,17 +45,33 @@ jobs:
           key: ${{ runner.os }}-maven-publish-${{ hashFiles('**/pom.xml', '**/*.target') }}
           restore-keys: ${{ runner.os }}-maven-publish-
 
+      - name: Scope static analysis to the PR's changed modules
+        # Injects pmd/cpd/checkstyle skip properties into unchanged module poms and
+        # exports LINT_SCOPE_ARGS (-pl <changed> -am) so only the changed modules and
+        # their upstream deps build (skip-injected deps compile for PMD's type
+        # resolution but are not analysed). Build/config change -> full scan, full
+        # reactor. pull_request only; master/snapshot run a full scan.
+        run: bash .github/scripts/compute-analysis-skip.sh "${{ github.event.pull_request.base.sha }}" lint
+
       - name: PMD + Checkstyle reports (SARIF)
         # PMD: SarifRenderer FQCN — emits pmd.sarif.json AND keeps pmd.xml.
         # Checkstyle: output.format=sarif — SARIF content in checkstyle-result.xml.
         # CPD is excluded here: the global -Dformat flag uses PMD's Renderer
         # hierarchy and would ClassCastException CPD's CPDReportRenderer.
+        # `compile` stays: PMD's type-resolving rules need Tycho's aux-classpath.
+        # jgit.dirtyWorkingTree=ignore: the scope step edits poms (see the spotbugs
+        # lane for the rationale; this job releases nothing).
+        # Skipped entirely when the scope step kept no modules (e.g. a docs-only
+        # PR): every module would carry the skip properties, so the compile
+        # output would be unused. The gate below relaxes on the same condition.
+        if: env.LINT_KEPT != '0'
         run: |
-          mvn -T 2C -f ./ddk-parent/pom.xml --batch-mode --fail-never \
+          mvn -T 2C -f ./ddk-parent/pom.xml ${LINT_SCOPE_ARGS:-} --batch-mode --fail-never \
             compile \
             pmd:pmd checkstyle:checkstyle \
             -Dformat=net.sourceforge.pmd.renderers.SarifRenderer \
-            -Dcheckstyle.output.format=sarif
+            -Dcheckstyle.output.format=sarif \
+            -Djgit.dirtyWorkingTree=ignore
 
       - name: CPD report (separate invocation — no SARIF support)
         # CPD has no SARIF renderer; emits cpd.xml only. Run standalone so the
@@ -63,8 +81,11 @@ jobs:
         # compile pass (verified at the current token threshold and at 100).
         # NOTE: the CPD token threshold is governed by pmd.cpd.min in
         # ddk-parent/pom.xml (#1339; tuned to 100 by #1397).
+        # No jgit flag needed: a direct goal invocation runs no lifecycle, so the
+        # build-qualifier's dirty-tree check never executes here.
+        if: env.LINT_KEPT != '0'
         run: |
-          mvn -T 2C -f ./ddk-parent/pom.xml --batch-mode --fail-never \
+          mvn -T 2C -f ./ddk-parent/pom.xml ${LINT_SCOPE_ARGS:-} --batch-mode --fail-never \
             pmd:cpd-check
 
       - name: Merge per-module SARIFs (PMD + Checkstyle)
@@ -101,8 +122,29 @@ jobs:
         # merge() only writes its output when it found at least one valid input,
         # so a missing merged file means that analyzer silently died (e.g. a
         # plugin bump broke a renderer flag) — never a clean pass.
+        # Exception: the scope step skip-injected every module (no reactor module
+        # changed), where zero reports is the expected state. Each scanned
+        # source-bearing module must additionally have produced its own PMD
+        # SARIF, cpd.xml, and Checkstyle SARIF, so a partially-dead scoped
+        # build cannot hide either.
         run: |
           set -eu
+          if [ "${LINT_KEPT:-}" = "0" ]; then
+            echo "All modules skip-injected (no reactor module changed) — nothing to lint."
+            exit 0
+          fi
+          for mod in ${LINT_EXPECT_REPORTS:-}; do
+            for rep in pmd.sarif.json cpd.xml; do
+              if [ ! -s "${mod}/target/${rep}" ]; then
+                echo "::error::${mod} was scanned but produced no ${rep} — a build failure was swallowed by --fail-never."
+                exit 1
+              fi
+            done
+            if ! jq -e . "${mod}/target/checkstyle-result.xml" >/dev/null 2>&1; then
+              echo "::error::${mod} was scanned but produced no valid Checkstyle SARIF — a build failure was swallowed by --fail-never."
+              exit 1
+            fi
+          done
           for f in .sarif-merged/pmd.sarif .sarif-merged/checkstyle.sarif; do
             if [ ! -s "$f" ]; then
               echo "::error::No valid SARIF input produced for ${f##*/} — the analysis silently failed."
@@ -116,7 +158,7 @@ jobs:
           sarif_total=$(jq '[.runs[].results[]?] | length' \
             .sarif-merged/pmd.sarif .sarif-merged/checkstyle.sarif 2>/dev/null \
             | awk '{s+=$1} END {print s+0}')
-          cpd_total=$(find . -name 'cpd.xml' -path '*/target/*' -exec grep -c '<duplication ' {} + 2>/dev/null \
+          cpd_total=$(find . -name 'cpd.xml' -path '*/target/*' -exec grep -cH '<duplication ' {} + 2>/dev/null \
             | awk -F: '{s+=$2} END {print s+0}')
           echo "PMD/Checkstyle SARIF violations: $sarif_total"
           echo "CPD duplications: $cpd_total"
@@ -126,7 +168,9 @@ jobs:
           fi
 
       - name: Upload PMD/Checkstyle SARIF to Code Scanning
-        if: always()
+        # Skip when nothing was scanned (e.g. a docs-only PR -> all modules skipped):
+        # an empty .sarif-merged would otherwise fail upload-sarif ("No SARIF files").
+        if: ${{ always() && hashFiles('.sarif-merged/pmd.sarif', '.sarif-merged/checkstyle.sarif') != '' }}
         # Annotation-only, never the gate: a fork PR gets a read-only token and
         # upload-sarif 403s, which must not red an otherwise-clean lane.
         continue-on-error: true
@@ -166,7 +210,7 @@ jobs:
         # changed modules and their upstream deps build at all (skip-injected deps
         # compile for the aux-classpath but are not analysed). A build/config change ->
         # full scan, full reactor. pull_request only; master/snapshot run a full scan.
-        run: bash .github/scripts/compute-spotbugs-skip.sh "${{ github.event.pull_request.base.sha }}"
+        run: bash .github/scripts/compute-analysis-skip.sh "${{ github.event.pull_request.base.sha }}" spotbugs
 
       - name: SpotBugs report (SARIF)
         # sarifOutput=true emits spotbugsSarif.json (also writes spotbugsXml.xml).
