ci: make PMD a zero-findings gate (#6119)

* ci: make PMD a zero-findings gate

The PMD gate failed only on a fixed list of rules, so an unlisted one
(AvoidUsingHardCodedIP in OtlpTestCollector, from #6114) reached master
while the report showed it in red. Any PMD finding now fails the build,
matching SpotBugs; the one finding gets a NOPMD with its reason.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* unity-compat: mark the C#-shaped PMD findings the gate now enforces

ClassNamingConventions (Action_1, Int32_, TMP_Text: names the translator
binds to), AbstractClassWithoutAnyMethod (abstract in C#) and
ExtendsObject (UnityEngine.Object, which PMD mistakes for java.lang.Object).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Drop the remaining references to the old PMD forbidden list

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

Author: shai-almog

.github/scripts/generate-quality-report.py

@@ -485,7 +485,7 @@ def parse_pmd() -> Optional[AnalysisReport]:
             root = ET.parse(report_path).getroot()
         except ET.ParseError:
             # Unreadable is not clean. Reported as a finding of its own so the
-            # forbidden-rule gate below cannot mistake it for an empty report.
+            # PMD gate below cannot mistake it for an empty report.
             findings.append(Finding(severity="P1", location=str(report_path),
                                     message="PMD report could not be parsed",
                                     rule="UnparseableReport"))
@@ -950,62 +950,15 @@ def main() -> None:
 
     pmd = parse_pmd()
     if pmd:
-        forbidden_pmd_rules = {
-            "UnparseableReport",
-            "ClassWithOnlyPrivateConstructorsShouldBeFinal",
-            "CompareObjectsWithEquals",
-            "FormalParameterNamingConventions",
-            "LiteralsFirstInComparisons",
-            "LocalVariableNamingConventions",
-            "MethodNamingConventions",
-            "OneDeclarationPerLine",
-            "MissingOverride",
-            "ControlStatementBraces",
-            "UnnecessaryFullyQualifiedName",
-            "UseCollectionIsEmpty",
-            "UnusedPrivateField",
-            "UnnecessaryLocalBeforeReturn",
-            "PreserveStackTrace",
-            "CloseResource",
-            "AssignmentInOperand",
-            "EmptyControlStatement",
-            "ForLoopCanBeForeach",
-            "UnusedFormalParameter",
-            "SwitchStmtsShouldHaveDefault",
-            "UnconditionalIfStatement",
-            "SimplifyConditional",
-            "UselessOverridingMethod",
-            "UselessQualifiedThis",
-            "AvoidBranchingStatementAsLastInLoop",
-            "AvoidUsingVolatile",
-            "DoNotCallGarbageCollectionExplicitly",
-            "SuspiciousEqualsMethodName",
-            "UseUtilityClass",
-            "EmptyCatchBlock",
-            "SimplifyBooleanReturns",
-            "UnusedLocalVariable",
-            "UnnecessaryConstructor",
-            "UnnecessaryImport",
-            "AvoidStringBufferField",
-            "LogicInversion",
-            "DefaultLabelNotLastInSwitchStmt",
-            "SingularField",
-            "InstantiationToGetClass",
-            "UnnecessaryModifier",
-            "FinalFieldCouldBeStatic",
-            "AvoidInstanceofChecksInCatchClause",
-            "AvoidUsingOctalValues",
-            "NonThreadSafeSingleton",
-            "ComparisonWithNaN",
-            "DontUseFloatTypeForLoopIndices",
-            "MissingStaticMethodInNonInstantiatableClass",
-            "OverrideBothEqualsAndHashcode",
-            "UnnecessarySemicolon"
-        }
-        violations = [f for f in pmd.findings if f.rule in forbidden_pmd_rules]
-        if violations:
-            print("\n❌ Build failed due to forbidden PMD violations:")
-            for v in violations:
+        # Zero findings, like SpotBugs: ANY rule fails the build. This used to
+        # be a list of forbidden rules, and a rule nobody had listed
+        # (AvoidUsingHardCodedIP) reached master while the report showed it
+        # in red. An intentional exception is a `//NOPMD <Rule> - <reason>`
+        # on the line, never a gap in this gate. UnparseableReport is a
+        # finding too, so a corrupt report also fails here.
+        if pmd.findings:
+            print("\n\u274c Build failed due to PMD findings (the gate is zero findings):")
+            for v in pmd.findings:
                 print(f"  - {v.rule}: {v.location} - {v.message}")
             sys.exit(1)
 

.github/workflows/pr.yml

@@ -782,7 +782,7 @@ jobs:
           # SpotBugs run that silently stops running is not mistaken for clean.
           QUALITY_REPORT_REQUIRED_SPOTBUGS: core-unittests:android:ios:ByteCodeTranslator:codenameone-maven-plugin:build-engine:project-model:codenameone-gradle-plugin:build-hint-catalog:build-hint-tools:backend:backend-test:javac:android-res-compiler:android-compat:swing-compat:javafx-compat:fxml-compiler:desktop-compat-runtime
           # PMD and Checkstyle run on the core and on the backend runtime, and each
-          # must produce its report: without one, the forbidden-rule gate below it
+          # must produce its report: without one, the PMD gate below it
           # has nothing to read and passes.
           QUALITY_REPORT_REQUIRED_PMD: core-unittests:backend:backend-test:backend-parparvm
           QUALITY_REPORT_REQUIRED_CHECKSTYLE: core-unittests:backend:backend-test

.github/workflows/unity-compat.yml

@@ -20,8 +20,8 @@ name: Unity compatibility
 #                              built by the application's own Maven build and
 #                              staged for every target, then converted to
 #                              Gradle and staged again: the same entries
-#   static analysis            SpotBugs (zero findings), PMD (the forbidden
-#                              list) and Checkstyle (zero errors) over both
+#   static analysis            SpotBugs (zero findings), PMD (zero
+#                              findings) and Checkstyle (zero errors) over both
 #                              modules, enforced by the script PR CI uses for
 #                              the rest of the tree, and the ClassCastException
 #                              gate over the runtime, which ParparVM translates
@@ -180,7 +180,7 @@ jobs:
           rm -f cil-translator/target/spotbugsXml.xml cil-translator/target/pmd.xml cil-translator/target/checkstyle-result.xml
           rm -f unity-compat/target/spotbugsXml.xml unity-compat/target/pmd.xml unity-compat/target/checkstyle-result.xml
           xvfb-run -a mvn -B -Plocal-dev-javase -Dunity-compat -DskipTests -pl cil-translator,unity-compat verify
-      # The same script, with the same forbidden PMD list, that judges the rest
+      # The same script, with the same zero-findings PMD gate, that judges the rest
       # of the tree in pr.yml. PMD only writes a report, so this step is what
       # enforces it; every module named must have produced each report.
       - name: Enforce the quality gates

CLAUDE.md

@@ -136,7 +136,10 @@ any of them cannot skip it.
   `maven/cil-translator/`, `maven/unity-compat/`), scoped to the
   class or method it applies to and with a comment explaining why. Keep the
   generated report at zero rather than tolerating known noise.
-- PMD and Checkstyle still gate on their own lists in the same script.
+- **PMD is a zero-findings gate too**, in the same script: any rule fails the
+  build. An intentional exception is a `//NOPMD <Rule> - <reason>` on the line.
+  It once gated a fixed list of rules, and an unlisted one reached master.
+- Checkstyle gates on errors in the same script.
 
 To reproduce the SpotBugs gate locally:
 
@@ -443,12 +446,11 @@ The backend is held to the client's gates:
 - `///` markdown comments only, no `package.html`
   (`validate-java25-markdown-docs.sh` covers `vm/backend/src` and `impl/`);
   `package-info.java` per package; no since markers.
-- SpotBugs (zero findings, `maven/backend/spotbugs-exclude.xml`), PMD (the core's
-  forbidden list) and Checkstyle (zero errors) run in `maven/backend`'s `verify`,
+- SpotBugs (zero findings, `maven/backend/spotbugs-exclude.xml`), PMD (zero findings) and Checkstyle (zero errors) run in `maven/backend`'s `verify`,
   using the core's own `pmd.xml` and `checkstyle.xml`. Only two of the three
   fail a local build: SpotBugs, and Checkstyle through its `check` execution
   (`checkstyle-zero-errors`). **PMD writes `maven/backend/target/pmd.xml` and
-  judges nothing** -- its forbidden list lives in
+  judges nothing** -- the zero-findings gate lives in
   `.github/scripts/generate-quality-report.py`, which PR CI runs afterwards. A
   green `mvn verify` therefore says nothing about PMD: read the report. Both
   reports once sat at seven and fifteen findings behind green local builds.
@@ -908,8 +910,11 @@ exceptions, so that the next finding is fixed rather than added to them:
   `get_position`, `GetComponent` and `Item1` are what translated bytecode
   calls, so SpotBugs' `NM_*` patterns are excluded for
   `com.codename1.unitycompat.{system,unityengine}` and each class with such
-  members carries `@SuppressWarnings("PMD.MethodNamingConventions")`. A new
-  runtime class needs that annotation; a Java-named helper never does.
+  members carries `@SuppressWarnings("PMD.MethodNamingConventions")` -- plus
+  `"PMD.ClassNamingConventions"` when the type itself has a C# name
+  (`Action_1`, `Int32_`, `TMP_Text`). A new runtime class needs that
+  annotation; a Java-named helper never does. An `extends Object` in
+  `unityengine` is `UnityEngine.Object`, so it carries `// NOPMD ExtendsObject`.
 - **The translated classes are left out of SpotBugs by name**
   (`UnityEngine.*`): they are generated bytecode in `target/classes` with no
   Java source. PMD and Checkstyle read sources and never see them. A finding

maven/unity-compat/src/main/java/com/codename1/unitycompat/system/Action_1.java

@@ -28,7 +28,7 @@
 /// class, which `$sameType` cannot tell apart; the closed type a delegate
 /// was made as travels in `Delegate.$typeKey`, and `Delegate.Combine`
 /// compares that as well.
-@SuppressWarnings("PMD.MethodNamingConventions") // C# member names: translated code binds to them by name
+@SuppressWarnings({"PMD.MethodNamingConventions", "PMD.ClassNamingConventions"}) // C# member and type names: translated code binds to them by name
 public abstract class Action_1 extends MulticastDelegate {
     public abstract void Invoke(Object a0);
 

maven/unity-compat/src/main/java/com/codename1/unitycompat/system/Action_2.java

@@ -25,7 +25,7 @@
 /// `System.Action`2`. Type arguments are erased, so every parameter is an
 /// `Object` and the generated subclass unboxes what the method it calls
 /// takes as a primitive.
-@SuppressWarnings("PMD.MethodNamingConventions") // C# member names: translated code binds to them by name
+@SuppressWarnings({"PMD.MethodNamingConventions", "PMD.ClassNamingConventions"}) // C# member and type names: translated code binds to them by name
 public abstract class Action_2 extends MulticastDelegate {
     public abstract void Invoke(Object a0, Object a1);
 

maven/unity-compat/src/main/java/com/codename1/unitycompat/system/Action_3.java

@@ -25,7 +25,7 @@
 /// `System.Action`3`. Type arguments are erased, so every parameter is an
 /// `Object` and the generated subclass unboxes what the method it calls
 /// takes as a primitive.
-@SuppressWarnings("PMD.MethodNamingConventions") // C# member names: translated code binds to them by name
+@SuppressWarnings({"PMD.MethodNamingConventions", "PMD.ClassNamingConventions"}) // C# member and type names: translated code binds to them by name
 public abstract class Action_3 extends MulticastDelegate {
     public abstract void Invoke(Object a0, Object a1, Object a2);
 

maven/unity-compat/src/main/java/com/codename1/unitycompat/system/Array_.java

@@ -25,7 +25,7 @@
 /// The members of `System.Array`, for the arrays that have no class of
 /// their own to put them on: a JVM array for one dimension, an [MdArray]
 /// for more. Which one an `Array` is, is only known when it gets here.
-@SuppressWarnings("PMD.MethodNamingConventions") // C# member names: translated code binds to them by name
+@SuppressWarnings({"PMD.MethodNamingConventions", "PMD.ClassNamingConventions"}) // C# member and type names: translated code binds to them by name
 public final class Array_ {
     private Array_() {
     }

maven/unity-compat/src/main/java/com/codename1/unitycompat/system/Boolean_.java

@@ -24,7 +24,7 @@
 
 /// The methods of `System.Boolean`, which is the primitive `boolean` here and so
 /// cannot carry them itself. The value comes first.
-@SuppressWarnings("PMD.MethodNamingConventions") // C# member names: translated code binds to them by name
+@SuppressWarnings({"PMD.MethodNamingConventions", "PMD.ClassNamingConventions"}) // C# member and type names: translated code binds to them by name
 public final class Boolean_ {
     private Boolean_() {
     }

maven/unity-compat/src/main/java/com/codename1/unitycompat/system/Byte_.java

@@ -24,7 +24,7 @@
 
 /// The methods of `System.Byte`. A C# `byte` is unsigned and a JVM `byte`
 /// is not, so the value is masked on the way in.
-@SuppressWarnings("PMD.MethodNamingConventions") // C# member names: translated code binds to them by name
+@SuppressWarnings({"PMD.MethodNamingConventions", "PMD.ClassNamingConventions"}) // C# member and type names: translated code binds to them by name
 public final class Byte_ {
     private Byte_() {
     }