diff --git a/core/src/main/java/com/google/googlejavaformat/java/CommandLineOptionsParser.java b/core/src/main/java/com/google/googlejavaformat/java/CommandLineOptionsParser.java index f7c3dec95..5ecee8649 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/CommandLineOptionsParser.java +++ b/core/src/main/java/com/google/googlejavaformat/java/CommandLineOptionsParser.java @@ -14,9 +14,7 @@ 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; @@ -24,24 +22,41 @@ 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 options) { CommandLineOptions.Builder optionsBuilder = CommandLineOptions.builder(); List expandedOptions = new ArrayList<>(); - expandParamsFiles(options, expandedOptions); + expandParamsFiles(options, expandedOptions, new ArrayDeque<>()); Iterator it = expandedOptions.iterator(); while (it.hasNext()) { String option = it.next(); @@ -186,7 +201,7 @@ private static Range 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 args, List expanded) { + private static void expandParamsFiles(Iterable args, List expanded, Deque paramFilesStack) { for (String arg : args) { if (arg.isEmpty()) { continue; @@ -196,14 +211,35 @@ private static void expandParamsFiles(Iterable args, List 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 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 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; } } diff --git a/core/src/test/java/com/google/googlejavaformat/java/CommandLineOptionsParserTest.java b/core/src/test/java/com/google/googlejavaformat/java/CommandLineOptionsParserTest.java index 1a4ed09b4..b2c831f4f 100644 --- a/core/src/test/java/com/google/googlejavaformat/java/CommandLineOptionsParserTest.java +++ b/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(