Skip to content

Commit 8e84edf

Browse files
java-team-github-botError Prone Team
authored andcommitted
Allow the use of a configuration file to pass flags
Fix for public [issue #799](#799) \ Not `.json` as issue requested, but easier to edit and parse. This allows easier sharing of configurations between teams, build systems (cli, maven, gradle, bazel, IDEs, etc.), OSes. \ And bypasses the difficulties of wrapping long lines: end with `\` on a Linux shell, end with `^` in a Windows shell, bugs in Maven when running on Windows. PiperOrigin-RevId: 871403103
1 parent b48ff53 commit 8e84edf

2 files changed

Lines changed: 183 additions & 1 deletion

File tree

check_api/src/main/java/com/google/errorprone/ErrorProneOptions.java

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,11 @@
1616

1717
package com.google.errorprone;
1818

19+
import static com.google.common.collect.ImmutableList.toImmutableList;
20+
import static java.nio.charset.StandardCharsets.UTF_8;
21+
1922
import com.google.auto.value.AutoBuilder;
23+
import com.google.common.base.CharMatcher;
2024
import com.google.common.base.Optional;
2125
import com.google.common.base.Preconditions;
2226
import com.google.common.base.Splitter;
@@ -30,11 +34,13 @@
3034
import java.io.ObjectInputStream;
3135
import java.nio.file.FileSystems;
3236
import java.nio.file.Files;
37+
import java.nio.file.Path;
3338
import java.util.Arrays;
3439
import java.util.LinkedHashMap;
3540
import java.util.List;
3641
import java.util.Map;
3742
import java.util.regex.Pattern;
43+
import java.util.stream.Stream;
3844

3945
/**
4046
* Processes command-line options specific to error-prone.
@@ -63,6 +69,7 @@ public class ErrorProneOptions {
6369
"-XepDisableWarningsInGeneratedCode";
6470
private static final String COMPILING_TEST_ONLY_CODE = "-XepCompilingTestOnlyCode";
6571
private static final String COMPILING_PUBLICLY_VISIBLE_CODE = "-XepCompilingPubliclyVisibleCode";
72+
private static final String ARGUMENT_FILE_PREFIX = "@";
6673

6774
/** see {@link javax.tools.OptionChecker#isSupportedOption(String)} */
6875
public static int isSupportedOption(String option) {
@@ -381,6 +388,61 @@ public static ErrorProneOptions empty() {
381388
return EMPTY;
382389
}
383390

391+
/** Check if an argument looks like a argument file. */
392+
private static boolean isArgumentFile(String argument) {
393+
return argument.startsWith(ARGUMENT_FILE_PREFIX);
394+
}
395+
396+
/**
397+
* Read a CLI argument file, ignoring blank lines and full line comments. It fails if a line tries
398+
* to reference another argument file.
399+
*/
400+
private static ImmutableList<String> loadArgumentFileContent(String fileName) {
401+
Splitter splitter =
402+
Splitter.on(CharMatcher.breakingWhitespace()).trimResults().omitEmptyStrings();
403+
try (Stream<String> lines = Files.lines(Path.of(fileName), UTF_8)) {
404+
return lines
405+
// Remove comments. It also means it can damage file paths with `#`.
406+
.map(line -> line.replace("#.*", ""))
407+
// This might damage file paths with spaces.
408+
.flatMap(splitter::splitToStream)
409+
.filter(
410+
line -> {
411+
if (isArgumentFile(line)) {
412+
// This would in fact be quite tricky to get right.
413+
// Although `@a.cfg` seems pretty clear, relative path, when errorprone is
414+
// invoked from various tools (Maven, Gradle, IDEs), the concept becomes a lot
415+
// fuzzier. I'm in folder `x`, call `mvn -f y`, with arg
416+
// `@${project.basedir}/a.cfg`
417+
// where would `@b.cfg` invoked in `a.cfg` be?
418+
// Instead the user can explicitly specify the 2-3 config files they need.
419+
throw new InvalidCommandLineOptionException(
420+
"The parameter file cannot reference another parameter file " + line);
421+
}
422+
return true;
423+
})
424+
.collect(toImmutableList());
425+
} catch (IOException unused) {
426+
throw new InvalidCommandLineOptionException("Error loading parameters file " + fileName);
427+
}
428+
}
429+
430+
/**
431+
* Pre-processes an argument list, replacing arguments of the form {@code @filename} by the lines
432+
* read from the file.
433+
*/
434+
private static ImmutableList<String> preprocessArgs(Iterable<String> args) {
435+
ImmutableList.Builder<String> newArgs = ImmutableList.builder();
436+
for (String arg : args) {
437+
if (isArgumentFile(arg)) {
438+
newArgs.addAll(loadArgumentFileContent(arg.substring(ARGUMENT_FILE_PREFIX.length())));
439+
} else {
440+
newArgs.add(arg);
441+
}
442+
}
443+
return newArgs.build();
444+
}
445+
384446
/**
385447
* Given a list of command-line arguments, produce the corresponding {@link ErrorProneOptions}
386448
* instance.
@@ -403,7 +465,7 @@ public static ErrorProneOptions processArgs(Iterable<String> args) {
403465
boolean patchLocationSet = false;
404466
boolean patchCheckSet = false;
405467
Builder builder = new Builder();
406-
for (String arg : args) {
468+
for (String arg : preprocessArgs(args)) {
407469
switch (arg) {
408470
case IGNORE_SUPPRESSION_ANNOTATIONS -> builder.setIgnoreSuppressionAnnotations(true);
409471
case IGNORE_UNKNOWN_CHECKS_FLAG -> builder.setIgnoreUnknownChecks(true);

check_api/src/test/java/com/google/errorprone/ErrorProneOptionsTest.java

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,17 @@
1919
import static com.google.common.collect.ImmutableList.toImmutableList;
2020
import static com.google.common.collect.ImmutableMap.toImmutableMap;
2121
import static com.google.common.truth.Truth.assertThat;
22+
import static java.nio.charset.StandardCharsets.UTF_8;
2223
import static org.junit.Assert.assertThrows;
2324

2425
import com.google.common.collect.Collections2;
2526
import com.google.common.collect.ImmutableList;
2627
import com.google.common.collect.ImmutableMap;
2728
import com.google.errorprone.ErrorProneOptions.Severity;
2829
import com.google.errorprone.apply.ImportOrganizer;
30+
import java.io.File;
31+
import java.io.IOException;
32+
import java.nio.file.Files;
2933
import java.util.Arrays;
3034
import java.util.Collection;
3135
import java.util.List;
@@ -325,4 +329,120 @@ public void severityOrder() {
325329
assertThat(options.getSeverityMap()).containsExactlyEntriesIn(severityMap).inOrder();
326330
}
327331
}
332+
333+
@Test
334+
public void processArgumentsFileParsing() throws IOException {
335+
File source = File.createTempFile("ep_argfile_", ".cfg");
336+
source.deleteOnExit();
337+
Files.write(
338+
source.toPath(),
339+
ImmutableList.of(
340+
"# Line comment",
341+
"-Xep:UnicodeEscape:OFF",
342+
"-Xep:InvalidBlockTag:OFF # inline comment",
343+
"-Xep:JavaUtilDate:OFF",
344+
"# Several flags on a single line are acceptable:",
345+
"-Xep:LabelledBreakTarget:OFF \t -Xep:JUnit4TestNotRun:OFF"
346+
+ " -Xep:ComparableType:OFF",
347+
"-Xep:EqualsHashCode:WARN",
348+
"-Xep:ReturnValueIgnored:WARN",
349+
" # Indents and trailing, comment also indented:",
350+
"\t\t \t-Xep:ArrayToString:WARN",
351+
" -Xep:MisusedDayOfYear:WARN\t ",
352+
" -Xep:SelfComparison:WARN ",
353+
"-Xep:MisusedWeekYear:WARN"),
354+
UTF_8);
355+
356+
String[] args = {"@" + source.getAbsolutePath()};
357+
ImmutableMap<String, Severity> expectedSeverityMap =
358+
ImmutableMap.<String, Severity>builder()
359+
.put("UnicodeEscape", Severity.OFF)
360+
.put("InvalidBlockTag", Severity.OFF)
361+
.put("JavaUtilDate", Severity.OFF)
362+
.put("LabelledBreakTarget", Severity.OFF)
363+
.put("JUnit4TestNotRun", Severity.OFF)
364+
.put("ComparableType", Severity.OFF)
365+
.put("EqualsHashCode", Severity.WARN)
366+
.put("ReturnValueIgnored", Severity.WARN)
367+
.put("ArrayToString", Severity.WARN)
368+
.put("MisusedDayOfYear", Severity.WARN)
369+
.put("SelfComparison", Severity.WARN)
370+
.put("MisusedWeekYear", Severity.WARN)
371+
.buildOrThrow();
372+
373+
ErrorProneOptions options = ErrorProneOptions.processArgs(args);
374+
assertThat(options.getSeverityMap()).isEqualTo(expectedSeverityMap);
375+
}
376+
377+
@Test
378+
public void processArgumentsFileOverrides() throws IOException {
379+
File source1 = File.createTempFile("ep_argfile_", ".cfg");
380+
source1.deleteOnExit();
381+
File source2 = File.createTempFile("ep_argfile_", ".cfg");
382+
source2.deleteOnExit();
383+
384+
Files.write(
385+
source1.toPath(),
386+
ImmutableList.of(
387+
"-Xep:InvalidParam:WARN", "-Xep:JUnit4TestNotRun:WARN", "-Xep:EmptyBlockTag:WARN"),
388+
UTF_8);
389+
Files.write(
390+
source2.toPath(),
391+
ImmutableList.of(
392+
"-Xep:EffectivelyPrivate:OFF",
393+
"-Xep:ReturnValueIgnored:OFF",
394+
"-Xep:EmptyBlockTag:OFF",
395+
"-Xep:IntLongMath:OFF"),
396+
UTF_8);
397+
398+
String[] args = {
399+
"-Xep:InvalidParam:ERROR",
400+
"-Xep:EffectivelyPrivate:ERROR",
401+
"-Xep:UnicodeEscape:ERROR",
402+
"-Xep:JavaUtilDate:ERROR",
403+
"@" + source1.getAbsolutePath(),
404+
"-Xep:UnicodeEscape:ERROR",
405+
"-Xep:JUnit4TestNotRun:ERROR",
406+
"@" + source2.getAbsolutePath(),
407+
"-Xep:JavaUtilDate:ERROR",
408+
"-Xep:IntLongMath:ERROR"
409+
};
410+
411+
// To make the result easier to follow, the `args` set flags to ERROR,
412+
// the first config file uses WARN, and the second config file uses OFF.
413+
ImmutableMap<String, Severity> expectedSeverityMap =
414+
ImmutableMap.<String, Severity>builder()
415+
.put("InvalidParam", Severity.WARN)
416+
.put("EffectivelyPrivate", Severity.OFF)
417+
.put("UnicodeEscape", Severity.ERROR)
418+
.put("JavaUtilDate", Severity.ERROR)
419+
.put("JUnit4TestNotRun", Severity.ERROR)
420+
.put("EmptyBlockTag", Severity.OFF)
421+
.put("ReturnValueIgnored", Severity.OFF)
422+
.put("IntLongMath", Severity.ERROR)
423+
.buildOrThrow();
424+
425+
ErrorProneOptions options = ErrorProneOptions.processArgs(args);
426+
assertThat(options.getSeverityMap()).isEqualTo(expectedSeverityMap);
427+
}
428+
429+
@Test
430+
public void processArgumentsFileRefOtherConfig() throws IOException {
431+
File source = File.createTempFile("ep_argfile_", ".cfg");
432+
source.deleteOnExit();
433+
Files.write(
434+
source.toPath(),
435+
ImmutableList.of("-Xep:InvalidParam:WARN", "@other.cfg", "-Xep:EmptyBlockTag:WARN"),
436+
UTF_8);
437+
assertThrows(
438+
InvalidCommandLineOptionException.class,
439+
() -> ErrorProneOptions.processArgs(new String[] {"@" + source.getAbsolutePath()}));
440+
}
441+
442+
@Test
443+
public void processArgumentsFileMissing() {
444+
assertThrows(
445+
InvalidCommandLineOptionException.class,
446+
() -> ErrorProneOptions.processArgs(new String[] {"@test_cfg_is_missing.cfg"}));
447+
}
328448
}

0 commit comments

Comments
 (0)