Make PalantirFormat not fail when src folder doesn't exist (#3926)

Noticed this issue when working on #3919. Added a test case to the unit tests to cover it
com-lihaoyi · Nov 10, 2024 · 9f7450e · 9f7450e
1 parent 2d31114
commit 9f7450e
Show file tree

Hide file tree

Showing 5 changed files with 70 additions and 47 deletions.
diff --git a/docs/package.mill b/docs/package.mill
@@ -363,6 +363,7 @@ object `package` extends RootModule {
         .asScala
         .map(e => (e.toString, e.attr("href")))
         .toSeq
+        .filter(!_._2.startsWith("mailto:"))
         .partition{case (e, l) => l.startsWith("http://") || l.startsWith("https://")}
       (
         path,

diff --git a/scalalib/src/mill/javalib/palantirformat/PalantirFormatModule.scala b/scalalib/src/mill/javalib/palantirformat/PalantirFormatModule.scala
@@ -128,58 +128,69 @@ object PalantirFormatModule extends ExternalModule with PalantirFormatBaseModule
       ctx.log.info("formatting java sources ...")
     }
 
-    val mainArgs = palantirArgs(sources, check, options)
-
-    ctx.log.debug(s"running palantirformat with $mainArgs")
-
-    val exitCode = Jvm.callSubprocess(
-      mainClass = "com.palantir.javaformat.java.Main",
-      classPath = classPath.map(_.path),
-      jvmArgs = jvmArgs,
-      mainArgs = mainArgs,
-      workingDir = ctx.dest,
-      check = false
-    ).exitCode
-
-    if (check && exitCode != 0) {
-      ctx.log.error(
-        "palantirformat aborted due to format error(s) (or invalid plugin settings/palantirformat options)"
-      )
-      throw new RuntimeException(s"palantirformat exit($exitCode)")
+    palantirArgs(sources, check, options) match {
+      case None =>
+
+        ctx.log.debug("source folder not found, skipping")
+      case Some(mainArgs) =>
+
+        ctx.log.debug(s"running palantirformat with $mainArgs")
+
+        val exitCode = Jvm.callSubprocess(
+          mainClass = "com.palantir.javaformat.java.Main",
+          classPath = classPath.map(_.path),
+          jvmArgs = jvmArgs,
+          mainArgs = mainArgs,
+          workingDir = ctx.dest,
+          check = false
+        ).exitCode
+
+        if (check && exitCode != 0) {
+          ctx.log.error(
+            "palantirformat aborted due to format error(s) (or invalid plugin settings/palantirformat options)"
+          )
+          throw new RuntimeException(s"palantirformat exit($exitCode)")
+        }
     }
+
   }
 
   private def palantirArgs(
       sources: IterableOnce[PathRef],
       check: Boolean,
       options: PathRef
-  ): Seq[String] = {
-
-    val args = Seq.newBuilder[String]
-
-    // https://github.com/palantir/palantir-java-format/blob/dae9be4b84e2bd4d7ea346c6374fda47eee7118f/palantir-java-format/src/main/java/com/palantir/javaformat/java/CommandLineOptionsParser.java#L199
-    if (os.exists(options.path)) args += s"@${options.path}"
-
-    // https://github.com/palantir/palantir-java-format/blob/dae9be4b84e2bd4d7ea346c6374fda47eee7118f/palantir-java-format/src/main/java/com/palantir/javaformat/java/CommandLineOptions.java#L27
-    if (check) {
-      // do not overwrite files and exit(1) if formatting changes were detected
-      args += "--dry-run" += "--set-exit-if-changed"
-    } else {
-      // format in place
-      args += "--replace"
+  ): Option[Seq[String]] = {
+
+    val sourceArgs = sources
+      .iterator
+      .map(_.path)
+      .filter(os.exists(_))
+      .flatMap(os.walk(_, includeTarget = true))
+      .filter(os.isFile)
+      .filter(_.ext == "java")
+      .map(_.toString())
+      .toSeq
+
+    Option.when(sourceArgs.nonEmpty) {
+      val args = Seq.newBuilder[String]
+
+      // https://github.com/palantir/palantir-java-format/blob/dae9be4b84e2bd4d7ea346c6374fda47eee7118f/palantir-java-format/src/main/java/com/palantir/javaformat/java/CommandLineOptionsParser.java#L199
+      if (os.exists(options.path)) args += s"@${options.path}"
+
+      // https://github.com/palantir/palantir-java-format/blob/dae9be4b84e2bd4d7ea346c6374fda47eee7118f/palantir-java-format/src/main/java/com/palantir/javaformat/java/CommandLineOptions.java#L27
+      if (check) {
+        // do not overwrite files and exit(1) if formatting changes were detected
+        args += "--dry-run" += "--set-exit-if-changed"
+      } else {
+        // format in place
+        args += "--replace"
+      }
+
+      // https://github.com/palantir/palantir-java-format/blob/dae9be4b84e2bd4d7ea346c6374fda47eee7118f/palantir-java-format/src/main/java/com/palantir/javaformat/java/CommandLineOptionsParser.java#L49
+      args ++= sourceArgs
+
+      args.result()
     }
-
-    // https://github.com/palantir/palantir-java-format/blob/dae9be4b84e2bd4d7ea346c6374fda47eee7118f/palantir-java-format/src/main/java/com/palantir/javaformat/java/CommandLineOptionsParser.java#L49
-    args ++=
-      sources
-        .iterator
-        .map(_.path)
-        .flatMap(os.walk(_, includeTarget = true))
-        .filter(os.isFile)
-        .filter(_.ext == "java")
-        .map(_.toString())
-
-    args.result()
   }
 
   /**

diff --git a/scalalib/test/resources/javalib/palantirformat/after/empty/.keep b/scalalib/test/resources/javalib/palantirformat/after/empty/.keep
diff --git a/scalalib/test/resources/javalib/palantirformat/before/empty/.keep b/scalalib/test/resources/javalib/palantirformat/before/empty/.keep
diff --git a/scalalib/test/src/mill/javalib/palantirformat/PalantirFormatModuleTest.scala b/scalalib/test/src/mill/javalib/palantirformat/PalantirFormatModuleTest.scala
@@ -29,6 +29,10 @@ object PalantirFormatModuleTest extends TestSuite {
         checkState(
           afterFormat(before / "palantir", sources = Seq("src/Main.java")),
           after / "palantir"
+        ),
+        checkState(
+          afterFormat(before / "empty"),
+          after / "empty"
         )
       )
 
@@ -47,6 +51,10 @@ object PalantirFormatModuleTest extends TestSuite {
         checkState(
           afterFormatAll(before / "palantir"),
           after / "palantir"
+        ),
+        checkState(
+          afterFormatAll(before / "empty"),
+          after / "empty"
         )
       )
 
@@ -71,6 +79,7 @@ object PalantirFormatModuleTest extends TestSuite {
   }
 
   def checkState(actualFiles: Seq[os.Path], expectedRoot: os.Path): Boolean = {
+
     val expectedFiles = walkFiles(expectedRoot)
     actualFiles.length == expectedFiles.length &&
     actualFiles.iterator.zip(expectedFiles.iterator).forall {
@@ -122,11 +131,13 @@ object PalantirFormatModuleTest extends TestSuite {
       },
       { _ =>
         val Right(sources) = eval(module.sources)
-        sources.value.flatMap(ref => walkFiles(ref.path))
+        sources.value.map(_.path).flatMap(walkFiles(_))
       }
     )
   }
 
-  def walkFiles(root: os.Path): Seq[os.Path] =
-    os.walk(root).filter(os.isFile)
+  def walkFiles(root: os.Path): Seq[os.Path] = {
+    if (os.exists(root)) os.walk(root).filter(p => os.isFile(p) && p.last != ".keep")
+    else Nil
+  }
 }