Refactor formatter's main to use ExecutorCompletionService

PiperOrigin-RevId: 509606924

Author: cushon

core/src/main/java/com/google/googlejavaformat/java/FormatFileCallable.java

@@ -14,40 +14,73 @@
 
 package com.google.googlejavaformat.java;
 
+import com.google.auto.value.AutoValue;
 import com.google.common.collect.Range;
 import com.google.common.collect.RangeSet;
 import com.google.common.collect.TreeRangeSet;
+import java.nio.file.Path;
 import java.util.concurrent.Callable;
+import org.checkerframework.checker.nullness.qual.Nullable;
 
 /**
  * Encapsulates information about a file to be formatted, including which parts of the file to
  * format.
  */
-class FormatFileCallable implements Callable<String> {
+class FormatFileCallable implements Callable<FormatFileCallable.Result> {
+
+  @AutoValue
+  abstract static class Result {
+    abstract @Nullable Path path();
+
+    abstract String input();
+
+    abstract @Nullable String output();
+
+    boolean changed() {
+      return !input().equals(output());
+    }
+
+    abstract @Nullable FormatterException exception();
+
+    static Result create(
+        @Nullable Path path,
+        String input,
+        @Nullable String output,
+        @Nullable FormatterException exception) {
+      return new AutoValue_FormatFileCallable_Result(path, input, output, exception);
+    }
+  }
+
+  private final Path path;
   private final String input;
   private final CommandLineOptions parameters;
   private final JavaFormatterOptions options;
 
   public FormatFileCallable(
-      CommandLineOptions parameters, String input, JavaFormatterOptions options) {
+      CommandLineOptions parameters, Path path, String input, JavaFormatterOptions options) {
+    this.path = path;
     this.input = input;
     this.parameters = parameters;
     this.options = options;
   }
 
   @Override
-  public String call() throws FormatterException {
-    if (parameters.fixImportsOnly()) {
-      return fixImports(input);
-    }
+  public Result call() {
+    try {
+      if (parameters.fixImportsOnly()) {
+        return Result.create(path, input, fixImports(input), /* exception= */ null);
+      }
 
-    Formatter formatter = new Formatter(options);
-    String formatted = formatter.formatSource(input, characterRanges(input).asRanges());
-    formatted = fixImports(formatted);
-    if (parameters.reflowLongStrings()) {
-      formatted = StringWrapper.wrap(Formatter.MAX_LINE_LENGTH, formatted, formatter);
+      Formatter formatter = new Formatter(options);
+      String formatted = formatter.formatSource(input, characterRanges(input).asRanges());
+      formatted = fixImports(formatted);
+      if (parameters.reflowLongStrings()) {
+        formatted = StringWrapper.wrap(Formatter.MAX_LINE_LENGTH, formatted, formatter);
+      }
+      return Result.create(path, input, formatted, /* exception= */ null);
+    } catch (FormatterException e) {
+      return Result.create(path, input, /* output= */ null, e);
     }
-    return formatted;
   }
 
   private String fixImports(String input) throws FormatterException {

core/src/main/java/com/google/googlejavaformat/java/Main.java

@@ -16,6 +16,7 @@
 
 import static java.lang.Math.min;
 import static java.nio.charset.StandardCharsets.UTF_8;
+import static java.util.Comparator.comparing;
 
 import com.google.common.io.ByteStreams;
 import com.google.common.util.concurrent.MoreExecutors;
@@ -30,13 +31,14 @@
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.time.Duration;
+import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.LinkedHashMap;
-import java.util.Map;
+import java.util.Collections;
+import java.util.List;
 import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorCompletionService;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
-import java.util.concurrent.Future;
 
 /** The main class for the Java formatter CLI. */
 public final class Main {
@@ -123,50 +125,54 @@ private int formatFiles(CommandLineOptions parameters, JavaFormatterOptions opti
     int numThreads = min(MAX_THREADS, parameters.files().size());
     ExecutorService executorService = Executors.newFixedThreadPool(numThreads);
 
-    Map<Path, String> inputs = new LinkedHashMap<>();
-    Map<Path, Future<String>> results = new LinkedHashMap<>();
+    ExecutorCompletionService<FormatFileCallable.Result> cs =
+        new ExecutorCompletionService<>(executorService);
     boolean allOk = true;
 
+    int files = 0;
     for (String fileName : parameters.files()) {
       if (!fileName.endsWith(".java")) {
         errWriter.println("Skipping non-Java file: " + fileName);
         continue;
       }
       Path path = Paths.get(fileName);
-      String input;
       try {
-        input = new String(Files.readAllBytes(path), UTF_8);
-        inputs.put(path, input);
-        results.put(
-            path, executorService.submit(new FormatFileCallable(parameters, input, options)));
+        cs.submit(new FormatFileCallable(parameters, path, Files.readString(path), options));
+        files++;
       } catch (IOException e) {
         errWriter.println(fileName + ": could not read file: " + e.getMessage());
         allOk = false;
       }
     }
 
-    for (Map.Entry<Path, Future<String>> result : results.entrySet()) {
-      Path path = result.getKey();
-      String formatted;
+    List<FormatFileCallable.Result> results = new ArrayList<>();
+    while (files > 0) {
       try {
-        formatted = result.getValue().get();
+        files--;
+        results.add(cs.take().get());
       } catch (InterruptedException e) {
         errWriter.println(e.getMessage());
         allOk = false;
         continue;
       } catch (ExecutionException e) {
-        if (e.getCause() instanceof FormatterException) {
-          for (FormatterDiagnostic diagnostic : ((FormatterException) e.getCause()).diagnostics()) {
-            errWriter.println(path + ":" + diagnostic);
-          }
-        } else {
-          errWriter.println(path + ": error: " + e.getCause().getMessage());
-          e.getCause().printStackTrace(errWriter);
+        errWriter.println("error: " + e.getCause().getMessage());
+        e.getCause().printStackTrace(errWriter);
+        allOk = false;
+        continue;
+      }
+    }
+    Collections.sort(results, comparing(FormatFileCallable.Result::path));
+    for (FormatFileCallable.Result result : results) {
+      Path path = result.path();
+      if (result.exception() != null) {
+        for (FormatterDiagnostic diagnostic : result.exception().diagnostics()) {
+          errWriter.println(path + ":" + diagnostic);
         }
         allOk = false;
         continue;
       }
-      boolean changed = !formatted.equals(inputs.get(path));
+      String formatted = result.output();
+      boolean changed = result.changed();
       if (changed && parameters.setExitIfChanged()) {
         allOk = false;
       }
@@ -205,9 +211,16 @@ private int formatStdin(CommandLineOptions parameters, JavaFormatterOptions opti
     }
     String stdinFilename = parameters.assumeFilename().orElse(STDIN_FILENAME);
     boolean ok = true;
-    try {
-      String output = new FormatFileCallable(parameters, input, options).call();
-      boolean changed = !input.equals(output);
+    FormatFileCallable.Result result =
+        new FormatFileCallable(parameters, null, input, options).call();
+    if (result.exception() != null) {
+      for (FormatterDiagnostic diagnostic : result.exception().diagnostics()) {
+        errWriter.println(stdinFilename + ":" + diagnostic);
+      }
+      ok = false;
+    } else {
+      String output = result.output();
+      boolean changed = result.changed();
       if (changed && parameters.setExitIfChanged()) {
         ok = false;
       }
@@ -218,12 +231,6 @@ private int formatStdin(CommandLineOptions parameters, JavaFormatterOptions opti
       } else {
         outWriter.write(output);
       }
-    } catch (FormatterException e) {
-      for (FormatterDiagnostic diagnostic : e.diagnostics()) {
-        errWriter.println(stdinFilename + ":" + diagnostic);
-      }
-      ok = false;
-      // TODO(cpovirk): Catch other types of exception (as we do in the formatFiles case).
     }
     return ok ? 0 : 1;
   }