Various PMD refinements (#5435)

lefou · web-flow · commit b181fa45486a · 2025-07-03T11:53:32.000+02:00
Pull request: #5435
diff --git a/example/javalib/linting/6-pmd/build.mill b/example/javalib/linting/6-pmd/build.mill
@@ -3,53 +3,71 @@
 // PMD is a source code analyzer for Java that finds common programming flaws like unused variables,
 // empty catch blocks, unnecessary object creation, and more.
 //
-// To use this plugin in a Java module:
+// To customize this plugin in a Java module:
 //
 // 1. Extend JavaModule with PmdModule.
-// 2. [Optional] Define a PMD ruleset file(s) with `pmdRulesets` (default is `pmd-rules.xml` in root).
-// 3. [Optional] Define a version with `pmdVersion`.
-// 3. Run the `pmd` command.
+// 2. Define a PMD ruleset file(s) with `pmdRulesets` (default is `pmd-rules.xml` in root).
+// 3. Define the PMD version with `pmdVersion`.
 
 package build
 
 import mill._, javalib._
 import mill.javalib.pmd.PmdModule
 
-object `package` extends JavaModule with PmdModule {
+object `package` extends JavaModule, PmdModule {
   def pmdVersion = "7.15.0"
 }
 
-// Here's an example of simple PMD ruleset file, which is used to setup analysis rules.
-//
-// Be careful, rule formats can differ for different PMD versions.
+// Here's an example of a simple PMD ruleset file and a single Java source file.
+
 /** See Also: pmd-ruleset.xml */
+/** See Also: src/foo/EmptyCatchBlock.java */
+
+// CAUTION: Be careful, rule formats can differ between major PMD versions.
+
+// You can either run the `pmd` command on selected modules
+// or use the external module `mill.javalib.pmd/`.
 
 /** Usage
-> ./mill pmd -s true -v false # default format is text
-...src/EmptyCatchBlock.java:7:	EmptyCatchBlock:	Avoid empty catch blocks...
+
+> ./mill pmd -s true -v false # <1>
+Running PMD ...
+Writing PMD output to .../out/pmd.dest/pmd-output.text ...
+...src/foo/EmptyCatchBlock.java:7:	EmptyCatchBlock:	Avoid empty catch blocks...
 PMD found 1 violation(s)
-*/
 
+> ./mill mill.javalib.pmd/ # <2>
+error: Running PMD ...
+error: Writing PMD output to .../out/mill/javalib/pmd/PmdModule/pmd.dest/pmd-output.text ...
+error: 1 tasks failed
+error: mill.javalib.pmd.PmdModule.pmd PMD found 1 violation(s)
+
+*/
+// <1> `-s` shows the results on the console, `-v` does not fail the build
+// <2> Use the external `mill.javalib.pmd` module with default configuration
+//
 // The `pmd` command accepts several options to customize its behavior. Mill supports:
 //
-// * --stdout / -s : Enable output to stdout. False by default. (PMD will write output to a file regardless of this option).
+// * `--stdout` / `-s` : Enable output to stdout. False by default. (PMD will write output to a file regardless of this option).
 //
-// * --format / -f : Output format of the report (`text`, `xml`, `html`). Defaults to `text`.
+// * `--format` / `-f` : Output format of the report (`text`, `xml`, `html`). Defaults to `text`.
 //
-// * --fail-on-violation / -v : Fail if violations are found (true by default).
+// * `--fail-on-violation` / `-v` : Fail if violations are found (true by default).
 //
 // For a full list of PMD options, see the PMD documentation:
 // https://pmd.github.io/pmd/pmd_userdocs_cli_reference.html.
 //
 // Here's an example of producing HTML report:
+
 /** Usage
- > ./mill pmd -f "html" -s true -v false
+> ./mill pmd -f "html" -s true -v false
+Writing PMD output to .../out/pmd.dest/pmd-output.html ...
 ...<html><head><title>PMD</title></head><body>...
 ...<center><h3>PMD report</h3></center><center><h3>Problems found</h3></center><table align="center" cellspacing="0" cellpadding="3"><tr>...
 ...<th>#</th><th>File</th><th>Line</th><th>Problem</th></tr>...
 ...<tr bgcolor="lightgrey">...
 ...<td align="center">1</td>...
-...<td width="*%">...src/EmptyCatchBlock.java</td>...
+...<td width="*%">...src/foo/EmptyCatchBlock.java</td>...
 ...<td align="center" width="5%">7</td>...
 ...<td width="*"><a href="https://docs.pmd-code.org/pmd-doc-7.15.0/pmd_rules_java_errorprone.html#emptycatchblock">Avoid empty catch blocks</a></td>...
 ...</tr>...
diff --git a/example/javalib/linting/6-pmd/src/foo/EmptyCatchBlock.java b/example/javalib/linting/6-pmd/src/foo/EmptyCatchBlock.java
@@ -1,4 +1,4 @@
-package src;
+package foo;
 
 public class EmptyCatchBlock {
   public void bar() {
diff --git a/libs/jvmlib/src/mill/javalib/pmd/PmdModule.scala b/libs/jvmlib/src/mill/javalib/pmd/PmdModule.scala
@@ -5,32 +5,35 @@ import mill.api.{Discover, ExternalModule, TaskCtx}
 import mill.jvmlib.api.Versions
 import mill.scalalib.scalafmt.ScalafmtModule.sources
 import mill.scalalib.{CoursierModule, Dep, DepSyntax, OfflineSupportModule}
-import mill.util.Jvm
+import mill.util.{Jvm, Version}
 
 /**
  * Checks Java source files with PMD static code analyzer [[https://pmd.github.io/]].
  */
 trait PmdModule extends CoursierModule, OfflineSupportModule {
 
   /**
-   * Runs PMD and returns the number of violations found (exit code).
+   * Runs PMD.
    *
    * @note [[sources]] are processed when no [[PmdArgs.sources]] are specified.
    */
   def pmd(@mainargs.arg pmdArgs: PmdArgs): Command[(exitCode: Int, outputPath: PathRef)] =
     Task.Command {
-      val (outputPath, exitCode) = pmd0(pmdArgs.format, pmdArgs.sources)()
+      val res = pmd0(pmdArgs.format, pmdArgs.sources)()
       pmdHandleExitCode(
         pmdArgs.stdout,
         pmdArgs.failOnViolation,
-        exitCode,
-        outputPath,
+        res.exitCode,
+        res.outputPath,
         pmdArgs.format
       )
-      (exitCode, outputPath)
+      (res.exitCode, res.outputPath)
     }
 
-  protected def pmd0(format: String, leftover: mainargs.Leftover[String]) =
+  protected def pmd0(
+      format: String,
+      leftover: mainargs.Leftover[String]
+  ): Task[(outputPath: PathRef, exitCode: Int)] =
     Task.Anon {
       val output = Task.dest / s"pmd-output.$format"
       os.makeDir.all(output / os.up)
@@ -54,9 +57,9 @@ trait PmdModule extends CoursierModule, OfflineSupportModule {
         else "net.sourceforge.pmd.cli.PmdCli"
       val jvmArgs = pmdLanguage().map(lang => s"-Duser.language=$lang").toSeq
 
-      Task.log.info("Running PMD...")
+      Task.log.info("Running PMD ...")
       Task.log.debug(s"with $args")
-      Task.log.info(s"Writing PMD output to: $output...")
+      Task.log.info(s"Writing PMD output to $output ...")
 
       val exitCode = Jvm.callProcess(
         mainCls,
@@ -69,7 +72,7 @@ trait PmdModule extends CoursierModule, OfflineSupportModule {
         jvmArgs = jvmArgs
       ).exitCode
 
-      (PathRef(output), exitCode)
+      (outputPath = PathRef(output), exitCode = exitCode)
     }
 
   private def pmdHandleExitCode(
@@ -87,7 +90,7 @@ trait PmdModule extends CoursierModule, OfflineSupportModule {
         reportViolations(failOnViolation, countViolations(output, format, stdout))
       case 5 =>
         reportViolations(failOnViolation, countViolations(output, format, stdout))
-        throw new RuntimeException("At least one recoverable PMD error has occurred.")
+        Task.fail("At least one recoverable PMD error has occurred.")
       case x => Task.log.error(s"Unsupported PMD exit code: $x")
     exitCode
   }
@@ -127,10 +130,9 @@ trait PmdModule extends CoursierModule, OfflineSupportModule {
       failOnViolation: Boolean,
       violationCount: Option[Int]
   )(implicit ctx: TaskCtx): Unit = {
-    if (failOnViolation)
-      throw new RuntimeException(s"PMD found ${violationCount.getOrElse("some")} violation(s)")
-    else
-      Task.log.error(s"PMD found ${violationCount.getOrElse("some")} violation(s)")
+    val msg = s"PMD found ${violationCount.getOrElse("some")} violation(s)"
+    if (failOnViolation) Task.fail(msg)
+    else Task.log.error(msg)
   }
 
   /**
@@ -155,22 +157,19 @@ trait PmdModule extends CoursierModule, OfflineSupportModule {
   }
 
   /** Helper to check if the version is <= 6. False by default. */
-  private def isPmd6OrOlder(version: String): Boolean = {
-    version
-      .split(":").lastOption
-      .flatMap(_.takeWhile(_ != '.').toIntOption)
-      .exists(_ < 7)
-  }
+  private def isPmd6OrOlder(version: String): Boolean =
+    !Version.isAtLeast(version, "7")(using Version.IgnoreQualifierOrdering)
 
   /** PMD version. */
   def pmdVersion: T[String] = Task { Versions.pmdVersion }
 }
 
 /**
  * External module for PMD integration.
- * Allows usage via `import mill.javalib.pmd.PmdModule` in build.sc.
+ *
+ * Allows usage via `import mill.javalib.pmd/` in build.mill.
  */
-object PmdModule extends ExternalModule, PmdModule {
+object PmdModule extends ExternalModule, PmdModule, TaskModule {
   lazy val millDiscover: Discover = Discover[this.type]
-  def defaultCommandName() = "pmd"
+  override def defaultTask() = "pmd"
 }
diff --git a/libs/jvmlib/test/src/mill/javalib/pmd/PmdModuleTest.scala b/libs/jvmlib/test/src/mill/javalib/pmd/PmdModuleTest.scala
@@ -12,7 +12,7 @@ object PmdModuleTest extends TestSuite {
   val resources: os.Path = os.Path(sys.env("MILL_TEST_RESOURCE_DIR")) / "pmd"
   val modulePath: os.Path = resources / "example"
 
-  def runTest(module: TestRootModule with PmdModule with JavaModule): Unit = {
+  def runTest(module: TestRootModule & PmdModule & JavaModule): Unit = {
     UnitTester(module, modulePath).scoped { eval =>
       val format = "text"
       eval(module.pmd(PmdArgs(

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-package src;`
	`1`	`+package foo;`
`2`	`2`
`3`	`3`	`public class EmptyCatchBlock {`
`4`	`4`	`public void bar() {`