Polishing
- Use Java 8 idioms more consistently.
- Use newer Guava idioms more consistently.
- Apply some IntelliJ IDEA refactoring suggestions.
- Other changes made for readability and/or brevity.
Closes #3462.
PiperOrigin-RevId: 164700946
diff --git a/src/main/java/com/google/devtools/common/options/ArgsPreProcessor.java b/src/main/java/com/google/devtools/common/options/ArgsPreProcessor.java
index 516ff1f..00e8033 100644
--- a/src/main/java/com/google/devtools/common/options/ArgsPreProcessor.java
+++ b/src/main/java/com/google/devtools/common/options/ArgsPreProcessor.java
@@ -16,6 +16,7 @@
import java.util.List;
/** Defines a preprocessing service for the "args" string list that is executed before parsing. */
+@FunctionalInterface
interface ArgsPreProcessor {
List<String> preProcess(List<String> args) throws OptionsParsingException;
}
diff --git a/src/main/java/com/google/devtools/common/options/CommandNameCache.java b/src/main/java/com/google/devtools/common/options/CommandNameCache.java
index 13c85b7..622efff 100644
--- a/src/main/java/com/google/devtools/common/options/CommandNameCache.java
+++ b/src/main/java/com/google/devtools/common/options/CommandNameCache.java
@@ -16,6 +16,7 @@
import com.google.common.collect.ImmutableSet;
/** Cache mapping a command to the names of all commands it inherits from, including itself. */
+@FunctionalInterface
public interface CommandNameCache {
/** Class that exists only to expose a static instance variable that can be set and retrieved. */
class CommandNameCacheInstance implements CommandNameCache {
diff --git a/src/main/java/com/google/devtools/common/options/Converters.java b/src/main/java/com/google/devtools/common/options/Converters.java
index aacc64b..cf26e71 100644
--- a/src/main/java/com/google/devtools/common/options/Converters.java
+++ b/src/main/java/com/google/devtools/common/options/Converters.java
@@ -275,9 +275,7 @@
@Override
public List<String> convert(String input) {
- return input.equals("")
- ? ImmutableList.<String>of()
- : ImmutableList.copyOf(splitter.split(input));
+ return input.isEmpty() ? ImmutableList.of() : ImmutableList.copyOf(splitter.split(input));
}
@Override
@@ -311,9 +309,7 @@
try {
int level = Integer.parseInt(input);
return LEVELS[level];
- } catch (NumberFormatException e) {
- throw new OptionsParsingException("Not a log level: " + input);
- } catch (ArrayIndexOutOfBoundsException e) {
+ } catch (NumberFormatException | ArrayIndexOutOfBoundsException e) {
throw new OptionsParsingException("Not a log level: " + input);
}
}
diff --git a/src/main/java/com/google/devtools/common/options/ExpansionFunction.java b/src/main/java/com/google/devtools/common/options/ExpansionFunction.java
index 1031125..09119b2 100644
--- a/src/main/java/com/google/devtools/common/options/ExpansionFunction.java
+++ b/src/main/java/com/google/devtools/common/options/ExpansionFunction.java
@@ -19,6 +19,7 @@
* A function from an option parser's static setup (what flags it knows about) to a list of
* expansion Strings to use for one of its options.
*/
+@FunctionalInterface
public interface ExpansionFunction {
/**
diff --git a/src/main/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java b/src/main/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java
index 9d8c13a..a66d93e 100644
--- a/src/main/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java
+++ b/src/main/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java
@@ -19,7 +19,6 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Multimap;
-import com.google.common.collect.Sets;
import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.AllowValues;
import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.DisallowValues;
import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.FlagPolicy;
@@ -30,9 +29,9 @@
import com.google.devtools.common.options.OptionsParser.OptionDescription;
import com.google.devtools.common.options.OptionsParser.OptionValueDescription;
import java.util.ArrayList;
-import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -197,7 +196,7 @@
ImmutableSet<String> commandAndParentCommands =
command == null
- ? ImmutableSet.<String>of()
+ ? ImmutableSet.of()
: CommandNameCache.CommandNameCacheInstance.INSTANCE.get(command);
// Expand all policies to transfer policies on expansion flags to policies on the child flags.
@@ -242,8 +241,7 @@
String expansionFlagName = expansionPolicy.getFlagName();
- ImmutableList.Builder<OptionValueDescription> resultsBuilder =
- ImmutableList.<OptionValueDescription>builder();
+ ImmutableList.Builder<OptionValueDescription> resultsBuilder = ImmutableList.builder();
switch (expansionPolicy.getOperationCase()) {
case SET_VALUE:
{
@@ -307,7 +305,7 @@
ImmutableList<OptionValueDescription> expansions =
getExpansionsFromFlagPolicy(originalPolicy, originalOptionDescription, parser);
- ImmutableList.Builder<OptionValueDescription> subflagBuilder = new ImmutableList.Builder<>();
+ ImmutableList.Builder<OptionValueDescription> subflagBuilder = ImmutableList.builder();
ImmutableList<OptionValueDescription> subflags =
subflagBuilder
.addAll(originalOptionDescription.getImplicitRequirements())
@@ -640,7 +638,7 @@
// of string comparison. For example, "--foo=0", "--foo=false", "--nofoo", and "-f-"
// (if the option has an abbreviation) are all equal for boolean flags. Plus converters
// can be arbitrarily complex.
- Set<Object> convertedPolicyValues = Sets.newHashSet();
+ Set<Object> convertedPolicyValues = new HashSet<>();
for (String value : policyValues) {
Object convertedValue = optionDescription.getConverter().convert(value);
// Some converters return lists, and if the flag is a repeatable flag, the items in the
@@ -807,7 +805,7 @@
parser.parseWithSourceFunction(
OptionPriority.INVOCATION_POLICY,
INVOCATION_POLICY_SOURCE,
- Arrays.asList(String.format("--%s=%s", flagName, flagValue)));
+ ImmutableList.of(String.format("--%s=%s", flagName, flagValue)));
}
}
diff --git a/src/main/java/com/google/devtools/common/options/Options.java b/src/main/java/com/google/devtools/common/options/Options.java
index 88cf5f8..c52e395 100644
--- a/src/main/java/com/google/devtools/common/options/Options.java
+++ b/src/main/java/com/google/devtools/common/options/Options.java
@@ -52,8 +52,7 @@
OptionsParser parser = OptionsParser.newOptionsParser(optionsClass);
parser.parse(OptionPriority.COMMAND_LINE, null, Arrays.asList(args));
List<String> remainingArgs = parser.getResidue();
- return new Options<O>(parser.getOptions(optionsClass),
- remainingArgs.toArray(new String[0]));
+ return new Options<>(parser.getOptions(optionsClass), remainingArgs.toArray(new String[0]));
}
/**
diff --git a/src/main/java/com/google/devtools/common/options/OptionsData.java b/src/main/java/com/google/devtools/common/options/OptionsData.java
index 48f47ff..c5fd9a9 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsData.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsData.java
@@ -125,8 +125,7 @@
IsolatedOptionsData isolatedData = IsolatedOptionsData.from(classes);
// All that's left is to compute expansions.
- ImmutableMap.Builder<Field, ExpansionData> expansionDataBuilder =
- ImmutableMap.<Field, ExpansionData>builder();
+ ImmutableMap.Builder<Field, ExpansionData> expansionDataBuilder = ImmutableMap.builder();
for (Map.Entry<String, Field> entry : isolatedData.getAllNamedFields()) {
Field field = entry.getValue();
Option annotation = field.getAnnotation(Option.class);
@@ -154,7 +153,7 @@
throw new AssertionError(e);
}
- ImmutableList<String> staticExpansion = null;
+ ImmutableList<String> staticExpansion;
try {
staticExpansion = instance.getExpansion(new ExpansionContext(isolatedData, field, null));
Preconditions.checkState(
diff --git a/src/main/java/com/google/devtools/common/options/OptionsParser.java b/src/main/java/com/google/devtools/common/options/OptionsParser.java
index b507785..d4779fe 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsParser.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsParser.java
@@ -14,9 +14,12 @@
package com.google.devtools.common.options;
+import static java.util.Comparator.comparing;
+
import com.google.common.base.Joiner;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ListMultimap;
import com.google.common.escape.Escaper;
import java.lang.reflect.Constructor;
@@ -25,8 +28,6 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
-import java.util.Collections;
-import java.util.Comparator;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
@@ -133,7 +134,7 @@
*/
static OptionsData getOptionsDataInternal(Class<? extends OptionsBase> optionsClass)
throws ConstructionException {
- return getOptionsDataInternal(ImmutableList.<Class<? extends OptionsBase>>of(optionsClass));
+ return getOptionsDataInternal(ImmutableList.of(optionsClass));
}
/**
@@ -157,8 +158,7 @@
public static OptionsParser newOptionsParser(
Iterable<? extends Class<? extends OptionsBase>> optionsClasses)
throws ConstructionException {
- return newOptionsParser(
- getOptionsDataInternal(ImmutableList.<Class<? extends OptionsBase>>copyOf(optionsClasses)));
+ return newOptionsParser(getOptionsDataInternal(ImmutableList.copyOf(optionsClasses)));
}
/**
@@ -212,8 +212,7 @@
public void parseAndExitUponError(OptionPriority priority, String source, String[] args) {
for (String arg : args) {
if (arg.equals("--help")) {
- System.out.println(describeOptions(Collections.<String, String>emptyMap(),
- HelpVerbosity.LONG));
+ System.out.println(describeOptions(ImmutableMap.of(), HelpVerbosity.LONG));
System.exit(0);
}
}
@@ -540,7 +539,7 @@
for (Class<? extends OptionsBase> optionsClass : data.getOptionsClasses()) {
allFields.addAll(data.getFieldsForClass(optionsClass));
}
- Collections.sort(allFields, OptionsUsage.BY_CATEGORY);
+ allFields.sort(OptionsUsage.BY_CATEGORY);
String prevCategory = null;
for (Field optionField : allFields) {
@@ -583,7 +582,7 @@
for (Class<? extends OptionsBase> optionsClass : data.getOptionsClasses()) {
allFields.addAll(data.getFieldsForClass(optionsClass));
}
- Collections.sort(allFields, OptionsUsage.BY_CATEGORY);
+ allFields.sort(OptionsUsage.BY_CATEGORY);
String prevCategory = null;
for (Field optionField : allFields) {
@@ -621,26 +620,17 @@
OptionsData data = impl.getOptionsData();
StringBuilder desc = new StringBuilder();
- // List all options
- List<Field> allFields = new ArrayList<>();
- for (Class<? extends OptionsBase> optionsClass : data.getOptionsClasses()) {
- allFields.addAll(data.getFieldsForClass(optionsClass));
- }
- // Sort field for deterministic ordering
- Collections.sort(allFields, new Comparator<Field>() {
- @Override
- public int compare(Field f1, Field f2) {
- String name1 = f1.getAnnotation(Option.class).name();
- String name2 = f2.getAnnotation(Option.class).name();
- return name1.compareTo(name2);
- }
- });
- for (Field optionField : allFields) {
- Option option = optionField.getAnnotation(Option.class);
- if (option.documentationCategory() != OptionDocumentationCategory.UNDOCUMENTED) {
- OptionsUsage.getCompletion(optionField, desc);
- }
- }
+ data.getOptionsClasses()
+ // List all options
+ .stream()
+ .flatMap(optionsClass -> data.getFieldsForClass(optionsClass).stream())
+ // Sort field for deterministic ordering
+ .sorted(comparing(optionField -> optionField.getAnnotation(Option.class).name()))
+ .filter(
+ optionField ->
+ optionField.getAnnotation(Option.class).documentationCategory()
+ != OptionDocumentationCategory.UNDOCUMENTED)
+ .forEach(optionField -> OptionsUsage.getCompletion(optionField, desc));
return desc.toString();
}
@@ -744,8 +734,7 @@
*/
public OptionValueDescription clearValue(String optionName)
throws OptionsParsingException {
- OptionValueDescription clearedValue = impl.clearValue(optionName);
- return clearedValue;
+ return impl.clearValue(optionName);
}
@Override
diff --git a/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java b/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java
index d49d5c4..5ebd4a5 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java
@@ -14,15 +14,15 @@
package com.google.devtools.common.options;
+import static java.util.Comparator.comparing;
+import static java.util.stream.Collectors.toCollection;
+
import com.google.common.base.Joiner;
import com.google.common.base.Preconditions;
-import com.google.common.base.Predicate;
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.Iterables;
import com.google.common.collect.Iterators;
import com.google.common.collect.LinkedHashMultimap;
-import com.google.common.collect.Lists;
import com.google.common.collect.Multimap;
import com.google.devtools.common.options.OptionsParser.OptionDescription;
import com.google.devtools.common.options.OptionsParser.OptionValueDescription;
@@ -31,8 +31,6 @@
import java.lang.reflect.Field;
import java.util.ArrayList;
import java.util.Arrays;
-import java.util.Collections;
-import java.util.Comparator;
import java.util.HashMap;
import java.util.Iterator;
import java.util.LinkedHashMap;
@@ -121,74 +119,50 @@
* Implements {@link OptionsParser#asListOfUnparsedOptions()}.
*/
List<UnparsedOptionValueDescription> asListOfUnparsedOptions() {
- List<UnparsedOptionValueDescription> result = Lists.newArrayList(unparsedValues);
- // It is vital that this sort is stable so that options on the same priority are not reordered.
- Collections.sort(result, new Comparator<UnparsedOptionValueDescription>() {
- @Override
- public int compare(UnparsedOptionValueDescription o1,
- UnparsedOptionValueDescription o2) {
- return o1.getPriority().compareTo(o2.getPriority());
- }
- });
- return result;
+ return unparsedValues
+ .stream()
+ // It is vital that this sort is stable so that options on the same priority are not
+ // reordered.
+ .sorted(comparing(UnparsedOptionValueDescription::getPriority))
+ .collect(toCollection(ArrayList::new));
}
/**
* Implements {@link OptionsParser#asListOfExplicitOptions()}.
*/
List<UnparsedOptionValueDescription> asListOfExplicitOptions() {
- List<UnparsedOptionValueDescription> result = Lists.newArrayList(Iterables.filter(
- unparsedValues,
- new Predicate<UnparsedOptionValueDescription>() {
- @Override
- public boolean apply(UnparsedOptionValueDescription input) {
- return input.isExplicit();
- }
- }));
- // It is vital that this sort is stable so that options on the same priority are not reordered.
- Collections.sort(result, new Comparator<UnparsedOptionValueDescription>() {
- @Override
- public int compare(UnparsedOptionValueDescription o1,
- UnparsedOptionValueDescription o2) {
- return o1.getPriority().compareTo(o2.getPriority());
- }
- });
- return result;
+ return unparsedValues
+ .stream()
+ .filter(UnparsedOptionValueDescription::isExplicit)
+ // It is vital that this sort is stable so that options on the same priority are not
+ // reordered.
+ .sorted(comparing(UnparsedOptionValueDescription::getPriority))
+ .collect(toCollection(ArrayList::new));
}
/**
* Implements {@link OptionsParser#canonicalize}.
*/
List<String> asCanonicalizedList() {
-
- List<UnparsedOptionValueDescription> processed = Lists.newArrayList(
- canonicalizeValues.values());
- // Sort implicit requirement options to the end, keeping their existing order, and sort the
- // other options alphabetically.
- Collections.sort(processed, new Comparator<UnparsedOptionValueDescription>() {
- @Override
- public int compare(UnparsedOptionValueDescription o1, UnparsedOptionValueDescription o2) {
- if (o1.isImplicitRequirement()) {
- return o2.isImplicitRequirement() ? 0 : 1;
- }
- if (o2.isImplicitRequirement()) {
- return -1;
- }
- return o1.getName().compareTo(o2.getName());
- }
- });
-
- List<String> result = new ArrayList<>();
- for (UnparsedOptionValueDescription value : processed) {
-
- // Ignore expansion options.
- if (value.isExpansion()) {
- continue;
- }
-
- result.add("--" + value.getName() + "=" + value.getUnparsedValue());
- }
- return result;
+ return canonicalizeValues
+ .values()
+ .stream()
+ // Sort implicit requirement options to the end, keeping their existing order, and sort
+ // the other options alphabetically.
+ .sorted(
+ (v1, v2) -> {
+ if (v1.isImplicitRequirement()) {
+ return v2.isImplicitRequirement() ? 0 : 1;
+ }
+ if (v2.isImplicitRequirement()) {
+ return -1;
+ }
+ return v1.getName().compareTo(v2.getName());
+ })
+ // Ignore expansion options.
+ .filter(value -> !value.isExpansion())
+ .map(value -> "--" + value.getName() + "=" + value.getUnparsedValue())
+ .collect(toCollection(ArrayList::new));
}
/**
diff --git a/src/main/java/com/google/devtools/common/options/OptionsUsage.java b/src/main/java/com/google/devtools/common/options/OptionsUsage.java
index 5f7c48a..6971e27 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsUsage.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsUsage.java
@@ -21,7 +21,6 @@
import java.lang.reflect.Field;
import java.text.BreakIterator;
import java.util.ArrayList;
-import java.util.Collections;
import java.util.Comparator;
import java.util.List;
import javax.annotation.Nullable;
@@ -43,7 +42,7 @@
static void getUsage(Class<? extends OptionsBase> optionsClass, StringBuilder usage) {
OptionsData data = OptionsParser.getOptionsDataInternal(optionsClass);
List<Field> optionFields = new ArrayList<>(data.getFieldsForClass(optionsClass));
- Collections.sort(optionFields, BY_NAME);
+ optionFields.sort(BY_NAME);
for (Field optionField : optionFields) {
getUsage(optionField, usage, OptionsParser.HelpVerbosity.LONG, null);
}
@@ -119,7 +118,7 @@
String flagName = getFlagName(optionField);
String typeDescription = getTypeDescription(optionField);
Option annotation = optionField.getAnnotation(Option.class);
- usage.append(" --" + flagName);
+ usage.append(" --").append(flagName);
if (helpVerbosity == OptionsParser.HelpVerbosity.SHORT) { // just the name
usage.append('\n');
return;
@@ -128,7 +127,7 @@
usage.append(" [-").append(annotation.abbrev()).append(']');
}
if (!typeDescription.equals("")) {
- usage.append(" (" + typeDescription + "; ");
+ usage.append(" (").append(typeDescription).append("; ");
if (annotation.allowMultiple()) {
usage.append("may be used multiple times");
} else {
@@ -137,7 +136,7 @@
if (OptionsParserImpl.isSpecialNullDefault(defaultValueString, optionField)) {
usage.append("default: see description");
} else {
- usage.append("default: \"" + defaultValueString + "\"");
+ usage.append("default: \"").append(defaultValueString).append("\"");
}
}
usage.append(")");
diff --git a/src/main/java/com/google/devtools/common/options/ParamsFilePreProcessor.java b/src/main/java/com/google/devtools/common/options/ParamsFilePreProcessor.java
index bd1c5a9..30b67ed 100644
--- a/src/main/java/com/google/devtools/common/options/ParamsFilePreProcessor.java
+++ b/src/main/java/com/google/devtools/common/options/ParamsFilePreProcessor.java
@@ -152,11 +152,7 @@
}
// check to see if the current position is escaped
- if (lastChar == '\\') {
- escaped = true;
- } else {
- escaped = false;
- }
+ escaped = (lastChar == '\\');
if (!escaped && current == '\'') {
singleQuoteStart = singleQuoteStart == -1 ? readerPosition : -1;
diff --git a/src/main/java/com/google/devtools/common/options/testing/ConverterTester.java b/src/main/java/com/google/devtools/common/options/testing/ConverterTester.java
index 7d0b8d3..86f87f3 100644
--- a/src/main/java/com/google/devtools/common/options/testing/ConverterTester.java
+++ b/src/main/java/com/google/devtools/common/options/testing/ConverterTester.java
@@ -88,7 +88,7 @@
* @see EqualsTester#addEqualityGroup
*/
public ConverterTester addEqualityGroup(String... inputs) {
- ImmutableList.Builder<WrappedItem> wrapped = new ImmutableList.Builder<>();
+ ImmutableList.Builder<WrappedItem> wrapped = ImmutableList.builder();
ImmutableList<String> inputList = ImmutableList.copyOf(inputs);
inputLists.add(inputList);
for (String input : inputList) {
diff --git a/src/main/java/com/google/devtools/common/options/testing/ConverterTesterMap.java b/src/main/java/com/google/devtools/common/options/testing/ConverterTesterMap.java
index afa0231..cf028b9 100644
--- a/src/main/java/com/google/devtools/common/options/testing/ConverterTesterMap.java
+++ b/src/main/java/com/google/devtools/common/options/testing/ConverterTesterMap.java
@@ -44,7 +44,7 @@
private final ImmutableMap.Builder<Class<? extends Converter<?>>, ConverterTester> delegate;
public Builder() {
- this.delegate = new ImmutableMap.Builder<>();
+ this.delegate = ImmutableMap.builder();
}
/**
@@ -71,9 +71,7 @@
* permitted; duplicates will cause {@link #build} to fail.
*/
public Builder addAll(Iterable<ConverterTester> items) {
- for (ConverterTester item : items) {
- add(item);
- }
+ items.forEach(this::add);
return this;
}
diff --git a/src/main/java/com/google/devtools/common/options/testing/OptionsTester.java b/src/main/java/com/google/devtools/common/options/testing/OptionsTester.java
index fa58462..9ecfdc5 100644
--- a/src/main/java/com/google/devtools/common/options/testing/OptionsTester.java
+++ b/src/main/java/com/google/devtools/common/options/testing/OptionsTester.java
@@ -38,7 +38,7 @@
}
private static ImmutableList<Field> getAllFields(Class<? extends OptionsBase> optionsClass) {
- ImmutableList.Builder<Field> builder = new ImmutableList.Builder<>();
+ ImmutableList.Builder<Field> builder = ImmutableList.builder();
Class<? extends OptionsBase> current = optionsClass;
while (!OptionsBase.class.equals(current)) {
builder.add(current.getDeclaredFields());
@@ -103,7 +103,7 @@
*/
public OptionsTester testAllDefaultValuesTestedBy(ConverterTesterMap testers) {
ImmutableListMultimap.Builder<Class<? extends Converter<?>>, Field> converterClassesBuilder =
- new ImmutableListMultimap.Builder<>();
+ ImmutableListMultimap.builder();
for (Field field : getAllFields(optionsClass)) {
Option option = field.getAnnotation(Option.class);
if (option != null && !Converter.class.equals(option.converter())) {
