run formatter in fork - run on java 16+ without --add-exports flags

README.md

@@ -97,7 +97,9 @@ For example, you may prefer that the `check` goal is performed in an earlier pha
 
 `skipSortingImports` is whether the plugin should skip sorting imports.
 
-`style` sets the formatter style to be _google_ or _aosp_. By default this is 'google'. Projects using Android conventions may prefer `aosp`.
+`style` sets the formatter style to be `google` or `aosp`. By default this is `google`. Projects using Android conventions may prefer `aosp`.
+
+`forkMode` lets you specify whether to run google-java-format in a fork or in-process. Also adds JVM arguments when needed. Value `default` will fork when JDK 16+ is detected, `never` runs in-process, regardless of JDK version and `always` will always fork.
 
 example:
 ```xml
@@ -199,14 +201,6 @@ You can pass parameters via standard `-D` syntax.
 
 `-Dfmt.skip` is whether the plugin should skip the operation.
 
-### Using with Java 16+ and Maven
-
-Since the JDK is more restrictive since version 16 you need to pass some [parameters](https://github.com/google/google-java-format#jdk-16) to the JVM to run the Google Java Formatter. To do so add the file  `.mvn/jvm.config` under your project's root directory with the following contents:
-
-```
---add-exports jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED --add-exports jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED --add-exports jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED --add-exports jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED --add-exports jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED 
-```
-
 ### Using with Java 8
 
 Starting from version 1.8, Google Java Formatter requires Java 11 to run. Incidently, all versions of this plugin starting from 2.10 inclusively also require this Java version to properly function. The 2.9.x release branch is the most up-to-date version that still runs on Java 8.

pom.xml

@@ -86,6 +86,13 @@
             <version>1.14.0</version>
         </dependency>
 
+        <dependency>
+            <groupId>io.norberg</groupId>
+            <artifactId>auto-matter</artifactId>
+            <version>0.25.1</version>
+            <scope>provided</scope>
+        </dependency>
+
         <dependency>
             <groupId>junit</groupId>
             <artifactId>junit</artifactId>
@@ -168,6 +175,9 @@
                 <groupId>com.spotify.fmt</groupId>
                 <artifactId>fmt-maven-plugin</artifactId>
                 <version>2.15</version>
+                <configuration>
+                    <skip>true</skip>
+                </configuration>
                 <executions>
                     <execution>
                         <goals>
@@ -181,8 +191,8 @@
                 <artifactId>maven-compiler-plugin</artifactId>
                 <version>3.7.0</version>
                 <configuration>
-                    <source>1.8</source>
-                    <target>1.8</target>
+                    <source>9</source>
+                    <target>9</target>
                 </configuration>
             </plugin>
         </plugins>
@@ -236,15 +246,5 @@
                 </plugins>
             </build>
         </profile>
-        <profile>
-            <id>jdk17+</id>
-            <activation>
-                <jdk>[17,)</jdk>
-            </activation>
-            <properties>
-                <!-- TODO: Remove this once google-java-fmt supports 17 or the plugin supports forking and run with these args built-in -->
-                <argLine> --add-exports jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED --add-exports jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED --add-exports jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED --add-exports jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED --add-exports jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED</argLine>
-            </properties>
-        </profile>
     </profiles>
 </project>

src/main/java/com/spotify/fmt/AbstractFMT.java

@@ -1,21 +1,15 @@
 package com.spotify.fmt;
 
-import com.google.common.base.Charsets;
-import com.google.common.io.CharSource;
-import com.google.googlejavaformat.java.*;
+import com.google.common.annotations.VisibleForTesting;
 import java.io.File;
-import java.io.FileFilter;
-import java.io.IOException;
-import java.nio.file.Files;
-import java.nio.file.Path;
-import java.nio.file.Paths;
 import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
-import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.Map;
 import java.util.stream.Collectors;
-import java.util.stream.Stream;
+import org.apache.maven.artifact.Artifact;
 import org.apache.maven.plugin.AbstractMojo;
-import org.apache.maven.plugin.MojoExecutionException;
 import org.apache.maven.plugin.MojoFailureException;
 import org.apache.maven.plugins.annotations.Parameter;
 
@@ -60,16 +54,31 @@ public abstract class AbstractFMT extends AbstractMojo {
   @Parameter(defaultValue = "google", property = "style")
   private String style;
 
-  private List<String> filesProcessed = new CopyOnWriteArrayList<>();
-  private int nonComplyingFiles;
-
   /**
-   * execute.
+   * Option to specify whether to run google-java-format in a fork or in-process. Can be {@code
+   * default}, {@code never} and {@code always}. Also adds JVM arguments when needed.
    *
-   * @throws org.apache.maven.plugin.MojoExecutionException if any.
+   * <p>Specifying {@code default} (which is the default) will fork when JDK 16+ is detected.
+   * Specifying {@code never} will never fork and instead run in-process, regardless of JDK version.
+   * Specifying {@code always} will always fork, regardless of JDK version.<br>
+   */
+  @Parameter(defaultValue = "default", property = "fmt.forkMode")
+  private String forkMode;
+
+  @Parameter(property = "plugin.artifactMap", required = true, readonly = true)
+  private Map<String, Artifact> pluginArtifactMap;
+
+  /**
+   * 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;
+
+  /** execute. */
   @Override
-  public void execute() throws MojoExecutionException, MojoFailureException {
+  public void execute() throws MojoFailureException {
     if (skip) {
       getLog().info("Skipping format check");
       return;
@@ -101,133 +110,70 @@ public void execute() throws MojoExecutionException, MojoFailureException {
       }
     }
 
-    JavaFormatterOptions.Style style = style();
-    Formatter formatter = getFormatter(style);
-
-    for (File directoryToFormat : directoriesToFormat) {
-      formatSourceFilesInDirectory(directoryToFormat, formatter, style);
-    }
-
-    logNumberOfFilesProcessed();
-    postExecute(this.filesProcessed, this.nonComplyingFiles);
-  }
-
-  /**
-   * Post Execute action. It is called at the end of the execute method. Subclasses can add extra
-   * checks.
-   *
-   * @param filesProcessed the list of processed files by the formatter
-   * @param nonComplyingFiles the number of files that are not compliant
-   * @throws MojoFailureException if there is an exception
-   */
-  protected void postExecute(List<String> filesProcessed, int nonComplyingFiles)
-      throws MojoFailureException {}
+    FormattingConfiguration configuration =
+        FormattingConfiguration.builder()
+            .debug(getLog().isDebugEnabled())
+            .directoriesToFormat(directoriesToFormat)
+            .style(style)
+            .filesNamePattern(filesNamePattern)
+            .filesPathPattern(filesPathPattern)
+            .verbose(verbose)
+            .skipSortingImports(skipSortingImports)
+            .writeReformattedFiles(shouldWriteReformattedFiles())
+            .processingLabel(getProcessingLabel())
+            .build();
 
-  /**
-   * Getter for the field <code>filesProcessed</code>.
-   *
-   * @return a {@link java.util.List} object.
-   */
-  public List<String> getFilesProcessed() {
-    return filesProcessed;
-  }
+    FormattingCallable formattingCallable = new FormattingCallable(configuration);
 
-  public void formatSourceFilesInDirectory(
-      File directory, Formatter formatter, JavaFormatterOptions.Style style)
-      throws MojoFailureException {
-    if (!directory.isDirectory()) {
-      getLog().info("Directory '" + directory + "' is not a directory. Skipping.");
-      return;
-    }
+    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);
+        }
 
-    try (Stream<Path> paths = Files.walk(Paths.get(directory.getPath()))) {
-      FileFilter fileNameFilter = getFileNameFilter();
-      FileFilter pathFilter = getPathFilter();
-      long failures =
-          paths.collect(Collectors.toList()).parallelStream()
-              .filter(p -> p.toFile().exists())
-              .map(Path::toFile)
-              .filter(fileNameFilter::accept)
-              .filter(pathFilter::accept)
-              .map(file -> formatSourceFile(file, formatter, style))
-              .filter(r -> !r)
-              .count();
-
-      if (failures > 0) {
-        throw new MojoFailureException(
-            "There were errors when formatting files. Error count: " + failures);
+      } else {
+        result = formattingCallable.call();
       }
-    } catch (IOException exception) {
-      throw new MojoFailureException(exception.getMessage());
-    }
-  }
-
-  private JavaFormatterOptions.Style style() throws MojoFailureException {
-    if ("aosp".equalsIgnoreCase(style)) {
-      getLog().debug("Using AOSP style");
-      return JavaFormatterOptions.Style.AOSP;
-    }
-    if ("google".equalsIgnoreCase(style)) {
-      getLog().debug("Using Google style");
-      return JavaFormatterOptions.Style.GOOGLE;
+    } catch (Exception e) {
+      throw new MojoFailureException(e);
     }
-    String message = "Unknown style '" + style + "'. Expected 'google' or 'aosp'.";
-    getLog().error(message);
-    throw new MojoFailureException(message);
-  }
-
-  private Formatter getFormatter(JavaFormatterOptions.Style style) throws MojoFailureException {
-    return new Formatter(JavaFormatterOptions.builder().style(style).build());
-  }
 
-  private FileFilter getFileNameFilter() {
-    if (verbose) {
-      getLog().debug("Filter files on '" + filesNamePattern + "'.");
-    }
-    return pathname -> pathname.isDirectory() || pathname.getName().matches(filesNamePattern);
+    postExecute(result);
   }
 
-  private FileFilter getPathFilter() {
-    if (verbose) {
-      getLog().debug("Filter paths on '" + filesPathPattern + "'.");
+  private boolean shouldFork() {
+    switch (forkMode) {
+      case "default":
+        return javaRuntimeStronglyEncapsulatesByDefault();
+      case "never":
+        return false;
+      case "always":
+        return true;
+      default:
+        throw new IllegalArgumentException(
+            "Invalid forkMode: " + forkMode + ", must be `default`, `never` or `always`");
     }
-    return pathname -> pathname.isDirectory() || pathname.getPath().matches(filesPathPattern);
   }
 
-  private boolean formatSourceFile(
-      File file, Formatter formatter, JavaFormatterOptions.Style style) {
-    if (file.isDirectory()) {
-      if (verbose) {
-        getLog().debug("File '" + file + "' is a directory. Skipping.");
-      }
-      return true;
-    }
-
-    if (verbose) {
-      getLog().debug("Formatting '" + file + "'.");
-    }
+  /**
+   * Post Execute action. It is called at the end of the execute method. Subclasses can add extra
+   * checks.
+   *
+   * @param result The formatting result
+   */
+  protected void postExecute(FormattingResult result) throws MojoFailureException {}
 
-    CharSource source = com.google.common.io.Files.asCharSource(file, Charsets.UTF_8);
-    try {
-      String input = source.read();
-      String formatted = formatter.formatSource(input);
-      formatted = RemoveUnusedImports.removeUnusedImports(formatted);
-      if (!skipSortingImports) {
-        formatted = ImportOrderer.reorderImports(formatted, style);
-      }
-      if (!input.equals(formatted)) {
-        onNonComplyingFile(file, formatted);
-        nonComplyingFiles += 1;
-      }
-      filesProcessed.add(file.getAbsolutePath());
-      if (filesProcessed.size() % 100 == 0) {
-        logNumberOfFilesProcessed();
-      }
-    } catch (FormatterException | IOException e) {
-      getLog().error("Failed to format file '" + file + "'.", e);
-      return false;
-    }
-    return true;
+  public FormattingResult getResult() {
+    return result;
   }
 
   private void handleMissingDirectory(String directoryDisplayName, File directory)
@@ -246,27 +192,53 @@ private void handleMissingDirectory(String directoryDisplayName, File directory)
     }
   }
 
-  protected void logNumberOfFilesProcessed() {
-    getLog()
-        .info(
-            String.format(
-                "Processed %d files (%d %s).",
-                filesProcessed.size(), nonComplyingFiles, getProcessingLabel()));
-  }
-
-  /**
-   * Hook called when the processd file is not compliant with the formatter.
-   *
-   * @param file the file that is not compliant
-   * @param formatted the corresponding formatted of the file.
-   * @throws IOException on any
-   */
-  protected abstract void onNonComplyingFile(File file, String formatted) throws IOException;
+  /** Whether to write reformatted files to disk. */
+  protected abstract boolean shouldWriteReformattedFiles();
 
   /**
    * Provides the name of the label used when a non-formatted file is found.
    *
    * @return the label to use in the log
    */
   protected abstract String getProcessingLabel();
+
+  @VisibleForTesting
+  static boolean javaRuntimeStronglyEncapsulatesByDefault() {
+    return Runtime.version().compareTo(Runtime.Version.parse("16")) >= 0;
+  }
+
+  private List<String> javaArgs() {
+    if (!javaRuntimeStronglyEncapsulatesByDefault()) {
+      return Collections.emptyList();
+    }
+
+    // https://github.com/google/google-java-format/blame/13ca73ebbfa86f6aca5f86be16e6829de6d5014c/pom.xml#L238
+    return Arrays.asList(
+        "--add-exports", "jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED",
+        "--add-exports", "jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED",
+        "--add-exports", "jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED",
+        "--add-exports", "jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED",
+        "--add-exports", "jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED",
+        "--add-exports", "jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED",
+        "--add-exports", "jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED",
+        "--add-exports", "jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED",
+        "--add-opens", "jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED",
+        "--add-opens", "jdk.compiler/com.sun.tools.javac.comp=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();
+    }
+  }
 }