Improve handling of parameters in @-files: add quote-handling for whitespaces in parameters.

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

@@ -14,34 +14,49 @@
 
 package com.google.googlejavaformat.java;
 
-import static java.nio.charset.StandardCharsets.UTF_8;
-
-import com.google.common.base.CharMatcher;
+import com.google.common.base.Preconditions;
 import com.google.common.base.Splitter;
 import com.google.common.collect.ImmutableRangeSet;
 import com.google.common.collect.Range;
 import java.io.IOException;
 import java.io.UncheckedIOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.nio.file.Paths;
+import java.util.ArrayDeque;
 import java.util.ArrayList;
+import java.util.Deque;
 import java.util.Iterator;
 import java.util.List;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 /** A parser for {@link CommandLineOptions}. */
 final class CommandLineOptionsParser {
 
   private static final Splitter COMMA_SPLITTER = Splitter.on(',');
   private static final Splitter COLON_SPLITTER = Splitter.on(':');
-  private static final Splitter ARG_SPLITTER =
-      Splitter.on(CharMatcher.breakingWhitespace()).omitEmptyStrings().trimResults();
+
+  /**
+   * Let's split arguments on whitespace (including tabulator and newline). Additionally allow quotes for arguments,
+   * such that they can contain whitespace that are kept in the argument without change.
+   *
+   * The regex matches either a quoted string (single or double quotes are allowed) or a plain unquoted string.
+   * It is possible to have double quotes within a single-quoted string and vice-versa. This is then kept 'as-is'.
+   * For simplicity, we do not handle escaped quotes.
+   */
+  private static final Pattern ARG_MATCHER = Pattern.compile(
+          "\"([^\"]*)(?:\"|$)" + // group 1: string in double quotes (or until EOF), with whitespace allowed
+          "|" + // OR
+          "'([^']*)(?:'|$)" + // group 2: string in single quotes (or until EOF), with whitespace allowed
+          "|" + // OR
+          "([^\\s\"']+)"  // group 3: unquoted string, without whitespace and without any quotes
+  );
 
   /** Parses {@link CommandLineOptions}. */
   static CommandLineOptions parse(Iterable<String> options) {
     CommandLineOptions.Builder optionsBuilder = CommandLineOptions.builder();
     List<String> expandedOptions = new ArrayList<>();
-    expandParamsFiles(options, expandedOptions);
+    expandParamsFiles(options, expandedOptions, new ArrayDeque<>());
     Iterator<String> it = expandedOptions.iterator();
     while (it.hasNext()) {
       String option = it.next();
@@ -186,7 +201,7 @@ private static Range<Integer> parseRange(String arg) {
    * Pre-processes an argument list, expanding arguments of the form {@code @filename} by reading
    * the content of the file and appending whitespace-delimited options to {@code arguments}.
    */
-  private static void expandParamsFiles(Iterable<String> args, List<String> expanded) {
+  private static void expandParamsFiles(Iterable<String> args, List<String> expanded, Deque<String> paramFilesStack) {
     for (String arg : args) {
       if (arg.isEmpty()) {
         continue;
@@ -196,14 +211,35 @@ private static void expandParamsFiles(Iterable<String> args, List<String> expand
       } else if (arg.startsWith("@@")) {
         expanded.add(arg.substring(1));
       } else {
-        Path path = Paths.get(arg.substring(1));
-        try {
-          String sequence = new String(Files.readAllBytes(path), UTF_8);
-          expandParamsFiles(ARG_SPLITTER.split(sequence), expanded);
-        } catch (IOException e) {
-          throw new UncheckedIOException(path + ": could not read file: " + e.getMessage(), e);
+        String filename = arg.substring(1);
+        if (paramFilesStack.contains(filename)) {
+          throw new IllegalArgumentException("parameter file was included recursively: " + filename);
+        }
+        paramFilesStack.push(filename);
+        expandParamsFiles(getParamsFromFile(filename), expanded, paramFilesStack);
+        String finishedFilename = paramFilesStack.pop();
+        Preconditions.checkState(filename.equals(finishedFilename));
+      }
+    }
+  }
+
+  /** Read parameters from file and handle quoted parameters. */
+  private static List<String> getParamsFromFile(String filename) {
+    String fileContent;
+    try {
+      fileContent = Files.readString(Path.of(filename));
+    } catch (IOException e) {
+      throw new UncheckedIOException(filename + ": could not read file: " + e.getMessage(), e);
+    }
+    List<String> paramsFromFile = new ArrayList<>();
+    Matcher m = ARG_MATCHER.matcher(fileContent);
+    while (m.find()) {
+      for (int i = 1; i <= m.groupCount(); i++) {
+        if (m.group(i) != null) { // only one group matches: double quote, single quotes or unquoted string.
+          paramsFromFile.add(m.group(i));
         }
       }
     }
+    return paramsFromFile;
   }
 }

core/src/test/java/com/google/googlejavaformat/java/CommandLineOptionsParserTest.java

@@ -18,6 +18,7 @@
 import static com.google.common.truth.Truth8.assertThat;
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.junit.Assert.fail;
+import static org.junit.Assert.assertThrows;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Range;
@@ -179,6 +180,60 @@ public void paramsFile() throws IOException {
     assertThat(options.files()).containsExactly("L", "M", "ℕ", "@O", "P", "Q");
   }
 
+  @Test
+  public void paramsFileWithNesting() throws IOException {
+    Path outer = testFolder.newFile("outer").toPath();
+    Path exit = testFolder.newFile("exit").toPath();
+    Path nested1 = testFolder.newFile("nested1").toPath();
+    Path nested2 = testFolder.newFile("nested2").toPath();
+    Path nested3 = testFolder.newFile("nested3").toPath();
+
+    String[] args = {"--dry-run", "@" + exit, "L", "@" + outer, "U"};
+
+    Files.write(exit, "--set-exit-if-changed".getBytes(UTF_8));
+    Files.write(outer, ("M\n@" + nested1.toAbsolutePath() + "\nT").getBytes(UTF_8));
+    Files.write(nested1, ("ℕ\n@" + nested2.toAbsolutePath() + "\nS").getBytes(UTF_8));
+    Files.write(nested2, ("O\n@" + nested3.toAbsolutePath() + "\nR").getBytes(UTF_8));
+    Files.write(nested3, "P\n\n   \n@@Q\n".getBytes(UTF_8));
+
+    CommandLineOptions options = CommandLineOptionsParser.parse(Arrays.asList(args));
+    assertThat(options.files()).containsExactly("L", "M", "ℕ", "O", "P", "@Q", "R", "S", "T", "U");
+  }
+
+  @Test
+  public void paramsFileWithRecursion() throws IOException {
+    Path outer = testFolder.newFile("outer").toPath();
+    Path exit = testFolder.newFile("exit").toPath();
+    Path nested1 = testFolder.newFile("nested1").toPath();
+    Path nested2 = testFolder.newFile("nested2").toPath();
+
+    String[] args = {"--dry-run", "@" + exit, "L", "@" + outer, "U"};
+
+    Files.write(exit, "--set-exit-if-changed".getBytes(UTF_8));
+    Files.write(outer, ("M\n@" + nested1.toAbsolutePath() + "\nT").getBytes(UTF_8));
+    Files.write(nested1, ("ℕ\n@" + nested2.toAbsolutePath() + "\nS").getBytes(UTF_8));
+    Files.write(nested2, ("O\n@" + nested1.toAbsolutePath() + "\nR").getBytes(UTF_8));
+
+    Exception exception = assertThrows(IllegalArgumentException.class, () -> CommandLineOptionsParser.parse(Arrays.asList(args)));
+    assertThat(exception.getMessage().startsWith("parameter file was included recursively: ")).isTrue();
+  }
+
+  @Test
+  public void paramsFileWithQuotesAndWhitespaces() throws IOException {
+    Path outer = testFolder.newFile("outer with whitespace").toPath();
+    Path exit = testFolder.newFile("exit with whitespace").toPath();
+    Path nested = testFolder.newFile("nested with whitespace").toPath();
+
+    String[] args = {"--dry-run", "@" + exit, "L +w", "@" + outer, "Q +w"};
+
+    Files.write(exit, "--set-exit-if-changed 'K +w".getBytes(UTF_8));
+    Files.write(outer, ("\"'M' +w\"\n\"@" + nested.toAbsolutePath() + "\"\n'\"P\" +w'").getBytes(UTF_8));
+    Files.write(nested, "\"ℕ +w\"\n\n   \n\"@@O +w".getBytes(UTF_8));
+
+    CommandLineOptions options = CommandLineOptionsParser.parse(Arrays.asList(args));
+    assertThat(options.files()).containsExactly("K +w", "L +w", "'M' +w", "ℕ +w", "@O +w", "\"P\" +w", "Q +w");
+  }
+
   @Test
   public void assumeFilename() {
     assertThat(