From 0a6a85e37f634fa870c920997d9270dca319cc03 Mon Sep 17 00:00:00 2001 From: Liam Miller-Cushon Date: Tue, 14 Feb 2023 12:33:08 -0800 Subject: [PATCH] Refactor formatter's main to use `ExecutorCompletionService` PiperOrigin-RevId: 509606924 --- .../java/FormatFileCallable.java | 57 +++++++++++---- .../google/googlejavaformat/java/Main.java | 69 ++++++++++--------- 2 files changed, 83 insertions(+), 43 deletions(-) diff --git a/core/src/main/java/com/google/googlejavaformat/java/FormatFileCallable.java b/core/src/main/java/com/google/googlejavaformat/java/FormatFileCallable.java index 9d8ae41ca..3d68a23f3 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/FormatFileCallable.java +++ b/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 { +class FormatFileCallable implements Callable { + + @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 { diff --git a/core/src/main/java/com/google/googlejavaformat/java/Main.java b/core/src/main/java/com/google/googlejavaformat/java/Main.java index 628c8bb24..9c60fcb12 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/Main.java +++ b/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 inputs = new LinkedHashMap<>(); - Map> results = new LinkedHashMap<>(); + ExecutorCompletionService 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> result : results.entrySet()) { - Path path = result.getKey(); - String formatted; + List 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; }