Merge pull request #266 from mkurz/plugin-reorder-modifiers-cli

Support `reorderModifiers = false` via google-java-format CLI

Author: mkurz

.github/workflows/ci.yml

@@ -65,11 +65,11 @@ jobs:
     - name: Build and test (sbt 2.x, Java 21)
       if: ${{ matrix.jobtype == 3 }}
       shell: bash
-      run: sbt -v '++ 3.x' clean scalafmtSbtCheck scalafmtCheckAll test 'scripted sbt-java-formatter/*' 'scripted java-runtime/compatible-21'
+      run: sbt -v '++ 3.x' clean scalafmtSbtCheck scalafmtCheckAll test 'scripted sbt-java-formatter/*' 'scripted java-runtime/compatible-21' 'scripted java-runtime/reorder-modifiers'
     - name: Build and test (sbt 2.x, Java 25)
       if: ${{ matrix.jobtype == 4 }}
       shell: bash
-      run: sbt -v '++ 3.x' clean scalafmtSbtCheck scalafmtCheckAll test 'scripted java-runtime/compatible-21'
+      run: sbt -v '++ 3.x' clean scalafmtSbtCheck scalafmtCheckAll test 'scripted java-runtime/compatible-21' 'scripted java-runtime/reorder-modifiers'
     - name: Build and test (sbt 1.x, Java 11 with formatter on Java 25)
       if: ${{ matrix.jobtype == 5 }}
       shell: bash
@@ -78,4 +78,4 @@ jobs:
         SBT_JAVAFMT_JAVA_HOME: ${{ env.JAVA_HOME_25_X64 }}
       run: |
         export PATH="${JAVA_HOME}/bin:$PATH"
-        sbt -v '++ 2.12.x' clean scalafmtSbtCheck scalafmtCheckAll test 'scripted java-runtime/compatible-21'
+        sbt -v '++ 2.12.x' clean scalafmtSbtCheck scalafmtCheckAll test 'scripted java-runtime/compatible-21' 'scripted java-runtime/reorder-modifiers'

README.md

@@ -38,6 +38,7 @@ For available versions see [releases](https://github.com/sbt/sbt-java-formatter/
 * The `javafmtRemoveUnusedImports` setting controls whether unused imports are removed (`true` by default).
 * The `javafmtReflowLongStrings` setting controls whether long string literals are reflowed (`true` by default).
 * The `javafmtFormatJavadoc` setting controls whether Javadoc comments are reformatted (`true` by default).
+* The `javafmtReorderModifiers` setting controls whether modifiers are reordered into JLS order (`true` by default). Disabling it requires `javafmtFormatterCompatibleJavaVersion := 21`.
 * The `javafmtFormatterCompatibleJavaVersion` setting selects which `google-java-format` runtime line to use (`21` by default).
 * The `javafmtJavaMaxHeap` setting controls the maximum heap passed to the forked `google-java-format` JVM (`Some("256m")` by default).
 
@@ -85,10 +86,11 @@ SBT_JAVAFMT_JAVA_HOME=/path/to/jdk-17 sbt javafmt
 > [!NOTE]
 > Many projects use Java 17 as their baseline today. If that is true for your build, `ThisBuild / javafmtFormatterCompatibleJavaVersion := 17` is often the simplest setup.
 >
-> The main Java language support changes in `google-java-format` [after `v1.28.0`](https://github.com/google/google-java-format/compare/v1.28.0...v1.35.0) are:
+> The main Java language support changes in `google-java-format` [after `v1.28.0`](https://github.com/google/google-java-format/compare/v1.28.0...v1.36.0) are:
 >
 > - [Initial support for import module in google-java-format](https://github.com/google/google-java-format/commit/6afe380707ec16884ec2761763ccec998de403d1)
 > - [Support Instance Main Methods in google-java-format](https://github.com/google/google-java-format/commit/737b0032b3a18eb6e458271ea440098c166f6c2d)
+> - [Initial support for Markdown documentation comments (`///`)](https://github.com/google/google-java-format/commit/53e2a763436abf00268f25ab59e481ca1d99cd29)
 >
 > If you do not use these language features, setting `ThisBuild / javafmtFormatterCompatibleJavaVersion := 17` lets sbt-java-formatter work out of the box on CI servers and developer machines that run sbt on Java 17.
 
@@ -114,6 +116,7 @@ ThisBuild / javafmtSortImports := true
 ThisBuild / javafmtRemoveUnusedImports := true
 ThisBuild / javafmtReflowLongStrings := true
 ThisBuild / javafmtFormatJavadoc := true
+ThisBuild / javafmtReorderModifiers := true
 ```
 
 Set any of them to `false` to pass the corresponding `--skip-...` flag to `google-java-format`.
@@ -122,18 +125,14 @@ Set any of them to `false` to pass the corresponding `--skip-...` flag to `googl
 
 - `11` -> `google-java-format 1.24.0`
 - `17` -> `google-java-format 1.28.0`
-- `21` -> `google-java-format 1.35.0` (default)
+- `21` -> `google-java-format 1.36.0` (default)
 
 If the selected formatter runtime is newer than the Java used to launch the formatter JVM, either:
 
 - lower `ThisBuild / javafmtFormatterCompatibleJavaVersion`
 - or point the formatter to a newer JDK via `SBT_JAVAFMT_JAVA_HOME` or `-Dsbt-javafmt.java.home=...`
 
-`javafmtOptions` is still available for compatibility, but the preferred sbt-facing configuration is through the dedicated `javafmt...` settings above.
-
-`JavaFormatterOptions.reorderModifiers()` currently has no effect in this plugin.
-
-The plugin now runs `google-java-format` via its CLI in a forked JVM, and the released `google-java-format` CLI used here [does not yet support a corresponding `--skip-reordering-modifiers` flag](https://github.com/google/google-java-format/pull/1373).
+`javafmtOptions` is still available for compatibility with upstream `JavaFormatterOptions`, but the preferred sbt-facing configuration is through the dedicated `javafmt...` settings above.
 
 If you want to tweak the format, take a minute to consider whether it is really worth it, and have a look at the motivations in the [Google Java Style Guide](https://google.github.io/styleguide/javaguide.html).
 If you decide you really need more flexibility, you could consider other plugins such as the [sbt-checkstyle-plugin](https://github.com/etsy/sbt-checkstyle-plugin)

plugin/src/main/scala/com/github/sbt/JavaFormatterPlugin.scala

@@ -82,8 +82,10 @@ object JavaFormatterPlugin extends AutoPlugin {
       settingKey[Boolean]("Whether google-java-format should reflow long string literals. Enabled by default.")
     val javafmtFormatJavadoc =
       settingKey[Boolean]("Whether google-java-format should format Javadoc comments. Enabled by default.")
+    val javafmtReorderModifiers =
+      settingKey[Boolean]("Whether google-java-format should reorder modifiers into JLS order. Enabled by default.")
     val javafmtOptions = settingKey[JavaFormatterOptions](
-      "Compatibility setting for upstream JavaFormatterOptions. Prefer the dedicated javafmt... settings; reorderModifiers() currently has no effect with the released google-java-format CLI used by this plugin.")
+      "Compatibility setting for upstream JavaFormatterOptions. Prefer the dedicated javafmt... settings where available.")
   }
 
   import autoImport._
@@ -121,7 +123,8 @@ object JavaFormatterPlugin extends AutoPlugin {
       javafmtSortImports := true,
       javafmtRemoveUnusedImports := true,
       javafmtReflowLongStrings := true,
-      javafmtFormatJavadoc := true)
+      javafmtFormatJavadoc := true,
+      javafmtReorderModifiers := true)
 
   def toBeScopedSettings: Seq[Setting[?]] =
     List(
@@ -130,6 +133,7 @@ object JavaFormatterPlugin extends AutoPlugin {
         .builder()
         .style(javafmtStyle.value)
         .formatJavadoc(javafmtFormatJavadoc.value)
+        .reorderModifiers(javafmtReorderModifiers.value)
         .build(),
       javafmt := {
         val streamz = streams.value
@@ -139,7 +143,8 @@ object JavaFormatterPlugin extends AutoPlugin {
         val iF = (javafmt / includeFilter).value
         val eF = (javafmt / excludeFilter).value
         val cache = streamz.cacheStoreFactory
-        val options = javafmtOptions.value
+        val options =
+          validateOptions(javafmtOptions.value, javafmtFormatterCompatibleJavaVersion.value)
         val formatterClasspath = javafmtFormatterClasspath.value.toVector
         val javaMaxHeap = javafmtJavaMaxHeap.value
         val sortImports = javafmtSortImports.value
@@ -169,7 +174,8 @@ object JavaFormatterPlugin extends AutoPlugin {
         val iF = (javafmt / includeFilter).value
         val eF = (javafmt / excludeFilter).value
         val cache = (javafmt / streams).value.cacheStoreFactory
-        val options = javafmtOptions.value
+        val options =
+          validateOptions(javafmtOptions.value, javafmtFormatterCompatibleJavaVersion.value)
         val formatterClasspath = javafmtFormatterClasspath.value.toVector
         val javaMaxHeap = javafmtJavaMaxHeap.value
         val sortImports = javafmtSortImports.value
@@ -199,7 +205,8 @@ object JavaFormatterPlugin extends AutoPlugin {
         val iF = (javafmt / includeFilter).value
         val eF = (javafmt / excludeFilter).value
         val cache = streamz.cacheStoreFactory
-        val options = javafmtOptions.value
+        val options =
+          validateOptions(javafmtOptions.value, javafmtFormatterCompatibleJavaVersion.value)
         val formatterClasspath = javafmtFormatterClasspath.value.toVector
         val javaMaxHeap = javafmtJavaMaxHeap.value
         val sortImports = javafmtSortImports.value
@@ -229,7 +236,8 @@ object JavaFormatterPlugin extends AutoPlugin {
         val iF = (javafmt / includeFilter).value
         val eF = (javafmt / excludeFilter).value
         val cache = (javafmt / streams).value.cacheStoreFactory
-        val options = javafmtOptions.value
+        val options =
+          validateOptions(javafmtOptions.value, javafmtFormatterCompatibleJavaVersion.value)
         val formatterClasspath = javafmtFormatterClasspath.value.toVector
         val javaMaxHeap = javafmtJavaMaxHeap.value
         val sortImports = javafmtSortImports.value
@@ -279,12 +287,23 @@ object JavaFormatterPlugin extends AutoPlugin {
     compatibleJavaVersion match {
       case 11    => "1.24.0"
       case 17    => "1.28.0"
-      case 21    => "1.35.0"
+      case 21    => "1.36.0"
       case other =>
         throw new MessageOnlyException(
           s"Unsupported javafmtFormatterCompatibleJavaVersion: $other. Expected one of: 11, 17, 21.")
     }
 
+  private def validateOptions(options: JavaFormatterOptions, compatibleJavaVersion: Int): JavaFormatterOptions = {
+    if (!options.reorderModifiers() && compatibleJavaVersion != 21) {
+      throw new MessageOnlyException(
+        "Disabling modifier reordering requires " +
+        "ThisBuild / javafmtFormatterCompatibleJavaVersion := 21 " +
+        "because the Java 11 and Java 17 formatter runtime lines do not support " +
+        "--skip-reordering-modifiers.")
+    }
+    options
+  }
+
   @transient
   private val javafmtDoFormatOnCompile =
     taskKey[Unit]("Format Java source files if javafmtOnCompile is on.")

plugin/src/main/scala/com/github/sbt/javaformatter/JavaFormatter.scala

@@ -363,17 +363,15 @@ object JavaFormatter {
       sortImports: Boolean,
       removeUnusedImports: Boolean,
       reflowLongStrings: Boolean): Seq[String] = {
-    if (!options.reorderModifiers()) {
-      throw new MessageOnlyException(
-        "The forked google-java-format CLI does not support reorderModifiers = false. " +
-        "Please use the default reorderModifiers setting.")
-    }
     val styleFlags =
       if (options.style() == JavaFormatterOptions.Style.AOSP) Seq("--aosp")
       else Nil
     val javadocFlags =
       if (options.formatJavadoc()) Nil
       else Seq("--skip-javadoc-formatting")
+    val reorderModifiersFlags =
+      if (options.reorderModifiers()) Nil
+      else Seq("--skip-reordering-modifiers")
     val fixImportsOnlyFlags =
       if (fixImportsOnly) Seq("--fix-imports-only")
       else Nil
@@ -386,7 +384,7 @@ object JavaFormatter {
     val reflowLongStringsFlags =
       if (reflowLongStrings) Nil
       else Seq("--skip-reflowing-long-strings")
-    styleFlags ++ javadocFlags ++ fixImportsOnlyFlags ++ sortImportsFlags ++ removeUnusedImportsFlags ++ reflowLongStringsFlags
+    styleFlags ++ javadocFlags ++ reorderModifiersFlags ++ fixImportsOnlyFlags ++ sortImportsFlags ++ removeUnusedImportsFlags ++ reflowLongStringsFlags
   }
 
   private case class CliResult(exitCode: Int, stdout: Vector[String], stderr: Vector[String])

plugin/src/sbt-test/java-runtime/reorder-modifiers/build.sbt

@@ -0,0 +1,2 @@
+ThisBuild / javafmtFormatterCompatibleJavaVersion := 21
+ThisBuild / javafmtReorderModifiers := false

…odifiers-unsupported/project/plugins.sbt …me/reorder-modifiers/project/plugins.sbtplugin/src/sbt-test/sbt-java-formatter/reorder-modifiers-unsupported/project/plugins.sbt renamed to plugin/src/sbt-test/java-runtime/reorder-modifiers/project/plugins.sbt plugin/src/sbt-test/sbt-java-formatter/reorder-modifiers-unsupported/project/plugins.sbt renamed to plugin/src/sbt-test/java-runtime/reorder-modifiers/project/plugins.sbt

@@ -0,0 +1,5 @@
+package com.lightbend;
+
+final public class BadFormatting {
+  static public void main(String[] args) {}
+}

plugin/src/sbt-test/java-runtime/reorder-modifiers/src/main/java-expected/com/lightbend/BadFormatting.java

@@ -0,0 +1,5 @@
+package com.lightbend;
+
+final      public class BadFormatting{
+  static   public void main(String[] args) {}
+}

plugin/src/sbt-test/java-runtime/reorder-modifiers/src/main/java/com/lightbend/BadFormatting.java

@@ -0,0 +1,8 @@
+-> javafmtCheck
+> javafmt
+> javafmtCheck
+
+$ exec diff src/main/java/com/lightbend/BadFormatting.java src/main/java-expected/com/lightbend/BadFormatting.java
+
+> set ThisBuild / javafmtFormatterCompatibleJavaVersion := 17
+-> javafmtCheck