add forkMode parameter

spotify · Mar 8, 2022 · 05f6b33 · 05f6b33
1 parent cfd85b3
commit 05f6b33
Show file tree

Hide file tree

Showing 3 changed files with 71 additions and 24 deletions.
diff --git a/src/main/java/com/spotify/fmt/AbstractFMT.java b/src/main/java/com/spotify/fmt/AbstractFMT.java
@@ -1,5 +1,6 @@
 package com.spotify.fmt;
 
+import com.google.common.annotations.VisibleForTesting;
 import java.io.File;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -56,8 +57,20 @@ public abstract class AbstractFMT extends AbstractMojo {
   @Parameter(property = "plugin.artifactMap", required = true, readonly = true)
   private Map<String, Artifact> pluginArtifactMap;
 
-  @Parameter(defaultValue = "false", property = "forkWithDefaultClasspath")
-  boolean forkWithDefaultClasspath;
+  /**
+   * Option to specify whether to run google-java-format in a fork or in-process. Can be {@code default}, {@code never} and {@code always.
+   * The {@code default} (which is the default) will fork when JDK 16+ is detected.
+   * The {@code never} will never fork and instead run in-process, regardless of JDK version.
+   * The {@code always} will always fork, regardless of JDK version.<br>
+   */
+  @Parameter(defaultValue = "default", property = "fmt.forkMode")
+  String forkMode;
+
+  /**
+   * Whether to use the classpath from the java.class.path property when forking. Only intended for
+   * use by unit tests.
+   */
+  @VisibleForTesting boolean useDefaultClasspathWhenForking;
 
   private FormattingResult result;
 
@@ -97,6 +110,7 @@ public void execute() throws MojoFailureException {
 
     FormattingConfiguration configuration =
         FormattingConfiguration.builder()
+            .debug(getLog().isDebugEnabled())
             .directoriesToFormat(directoriesToFormat)
             .style(style)
             .filesNamePattern(filesNamePattern)
@@ -107,33 +121,47 @@ public void execute() throws MojoFailureException {
             .processingLabel(getProcessingLabel())
             .build();
 
-    final boolean debugLoggingEnabled = getLog().isDebugEnabled();
-    final List<String> classpath =
-        pluginArtifactMap.values().stream()
-            .map(a -> a.getFile().getAbsolutePath())
-            .collect(Collectors.toList());
-
-    try (ForkingExecutor executor =
-        new ForkingExecutor(getLog())
-            .javaArgs(javaArgs())
-            .classpath(classpath)
-            .withDefaultClasspath(forkWithDefaultClasspath)) {
-      try {
-        result =
-            executor.execute(
-                () -> {
-                  Logging.configure(debugLoggingEnabled);
-                  Formatter formatter = new Formatter(configuration);
-                  return formatter.format();
-                });
-      } catch (Exception e) {
-        throw new MojoFailureException(e);
+    FormattingCallable formattingCallable = new FormattingCallable(configuration);
+
+    try {
+      if (shouldFork()) {
+        final List<String> classpath =
+            pluginArtifactMap.values().stream()
+                .map(a -> a.getFile().getAbsolutePath())
+                .collect(Collectors.toList());
+
+        try (ForkingExecutor executor =
+            new ForkingExecutor(getLog())
+                .javaArgs(javaArgs())
+                .classpath(classpath)
+                .withDefaultClasspath(useDefaultClasspathWhenForking)) {
+          result = executor.execute(formattingCallable);
+        }
+
+      } else {
+        result = formattingCallable.call();
       }
+    } catch (Exception e) {
+      throw new MojoFailureException(e);
     }
 
     postExecute(result);
   }
 
+  private boolean shouldFork() {
+    switch (forkMode) {
+      case "never":
+        return false;
+      case "default":
+        return hasModuleSystem();
+      case "always":
+        return true;
+      default:
+        throw new IllegalArgumentException(
+            "Invalid forkMode: " + forkMode + ", must be `default`, `never` or `always`");
+    }
+  }
+
   /**
    * Post Execute action. It is called at the end of the execute method. Subclasses can add extra
    * checks.
@@ -172,6 +200,7 @@ private void handleMissingDirectory(String directoryDisplayName, File directory)
    */
   protected abstract String getProcessingLabel();
 
+  /** Is this JDK 16+? */
   private boolean hasModuleSystem() {
     try {
       Class.forName("java.lang.Module");
@@ -193,4 +222,20 @@ private List<String> javaArgs() {
         "--add-exports", "jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED",
         "--add-exports", "jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED");
   }
+
+  private static class FormattingCallable implements SerializableCallable<FormattingResult> {
+
+    private final FormattingConfiguration configuration;
+
+    FormattingCallable(FormattingConfiguration configuration) {
+      this.configuration = configuration;
+    }
+
+    @Override
+    public FormattingResult call() {
+      Logging.configure(configuration.debug());
+      Formatter formatter = new Formatter(configuration);
+      return formatter.format();
+    }
+  }
 }
diff --git a/src/main/java/com/spotify/fmt/FormattingConfiguration.java b/src/main/java/com/spotify/fmt/FormattingConfiguration.java
@@ -8,6 +8,8 @@
 @AutoMatter
 interface FormattingConfiguration extends Serializable {
 
+  boolean debug();
+
   String style();
 
   List<File> directoriesToFormat();

diff --git a/src/test/java/com/spotify/fmt/FMTTest.java b/src/test/java/com/spotify/fmt/FMTTest.java
@@ -172,7 +172,7 @@ private <T extends AbstractFMT> T loadMojo(Class<T> cls, String pomFilePath, Str
     File pomFile = loadPom(pomFilePath);
     T fmt = (T) mojoRule.lookupConfiguredMojo(pomFile, goal);
     // Required for forking to work in unit tests where ${plugin.artifactMap} is not populated.
-    fmt.forkWithDefaultClasspath = true;
+    fmt.useDefaultClasspathWhenForking = true;
     return fmt;
   }