Eliminate some middleman methods
Reduce spaghetti code by exposing the parser's OptionsData as package-private, rather than exposing individual methods ad hoc between OptionsParser and OptionsParserImpl.
Also change some calls from static constructors to diamond syntax.
RELNOTES: None
PiperOrigin-RevId: 153457442
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 3b55c74..9574a90 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsParser.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsParser.java
@@ -20,16 +20,14 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ListMultimap;
-import com.google.common.collect.Lists;
-import com.google.common.collect.Maps;
import com.google.common.escape.Escaper;
import java.lang.reflect.Field;
import java.nio.file.FileSystem;
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.List;
import java.util.Map;
import javax.annotation.Nullable;
@@ -93,7 +91,7 @@
* options classes on the classpath.
*/
private static final Map<ImmutableList<Class<? extends OptionsBase>>, OptionsData> optionsData =
- Maps.newHashMap();
+ new HashMap<>();
/**
* Returns {@link OpaqueOptionsData} suitable for passing along to {@link
@@ -104,32 +102,35 @@
* construct lots of different {@link OptionsParser} instances).
*/
public static OpaqueOptionsData getOptionsData(
- ImmutableList<Class<? extends OptionsBase>> optionsClasses) throws ConstructionException {
+ List<Class<? extends OptionsBase>> optionsClasses) throws ConstructionException {
return getOptionsDataInternal(optionsClasses);
}
- private static synchronized OptionsData getOptionsDataInternal(
- ImmutableList<Class<? extends OptionsBase>> optionsClasses) throws ConstructionException {
- OptionsData result = optionsData.get(optionsClasses);
+ /**
+ * Returns the {@link OptionsData} associated with the given list of options classes.
+ */
+ static synchronized OptionsData getOptionsDataInternal(
+ List<Class<? extends OptionsBase>> optionsClasses) throws ConstructionException {
+ ImmutableList<Class<? extends OptionsBase>> immutableOptionsClasses =
+ ImmutableList.copyOf(optionsClasses);
+ OptionsData result = optionsData.get(immutableOptionsClasses);
if (result == null) {
try {
- result = OptionsData.from(optionsClasses);
+ result = OptionsData.from(immutableOptionsClasses);
} catch (Exception e) {
throw new ConstructionException(e.getMessage(), e);
}
- optionsData.put(optionsClasses, result);
+ optionsData.put(immutableOptionsClasses, result);
}
return result;
}
/**
- * Returns all the annotated fields for the given class, including inherited
- * ones.
+ * Returns the {@link OptionsData} associated with the given options class.
*/
- static Collection<Field> getAllAnnotatedFields(Class<? extends OptionsBase> optionsClass) {
- OptionsData data = getOptionsDataInternal(
- ImmutableList.<Class<? extends OptionsBase>>of(optionsClass));
- return data.getFieldsForClass(optionsClass);
+ static OptionsData getOptionsDataInternal(Class<? extends OptionsBase> optionsClass)
+ throws ConstructionException {
+ return getOptionsDataInternal(ImmutableList.<Class<? extends OptionsBase>>of(optionsClass));
}
/**
@@ -328,7 +329,7 @@
// The generic type of the list is not known at runtime, so we can't
// use it here. It was already checked in the constructor, so this is
// type-safe.
- List result = Lists.newArrayList();
+ List result = new ArrayList<>();
ListMultimap realValue = (ListMultimap) value;
for (OptionPriority priority : OptionPriority.values()) {
// If there is no mapping for this key, this check avoids object creation (because
@@ -536,11 +537,12 @@
*/
public String describeOptions(
Map<String, String> categoryDescriptions, HelpVerbosity helpVerbosity) {
+ OptionsData data = impl.getOptionsData();
StringBuilder desc = new StringBuilder();
- if (!impl.getOptionsClasses().isEmpty()) {
- List<Field> allFields = Lists.newArrayList();
- for (Class<? extends OptionsBase> optionsClass : impl.getOptionsClasses()) {
- allFields.addAll(impl.getAnnotatedFieldsFor(optionsClass));
+ if (!data.getOptionsClasses().isEmpty()) {
+ List<Field> allFields = new ArrayList<>();
+ for (Class<? extends OptionsBase> optionsClass : data.getOptionsClasses()) {
+ allFields.addAll(data.getFieldsForClass(optionsClass));
}
Collections.sort(allFields, OptionsUsage.BY_CATEGORY);
String prevCategory = null;
@@ -579,11 +581,12 @@
* of the category.
*/
public String describeOptionsHtml(Map<String, String> categoryDescriptions, Escaper escaper) {
+ OptionsData data = impl.getOptionsData();
StringBuilder desc = new StringBuilder();
- if (!impl.getOptionsClasses().isEmpty()) {
- List<Field> allFields = Lists.newArrayList();
- for (Class<? extends OptionsBase> optionsClass : impl.getOptionsClasses()) {
- allFields.addAll(impl.getAnnotatedFieldsFor(optionsClass));
+ if (!data.getOptionsClasses().isEmpty()) {
+ List<Field> allFields = new ArrayList<>();
+ for (Class<? extends OptionsBase> optionsClass : data.getOptionsClasses()) {
+ allFields.addAll(data.getFieldsForClass(optionsClass));
}
Collections.sort(allFields, OptionsUsage.BY_CATEGORY);
String prevCategory = null;
@@ -620,12 +623,13 @@
* details on the format for the flag completion.
*/
public String getOptionsCompletion() {
+ OptionsData data = impl.getOptionsData();
StringBuilder desc = new StringBuilder();
// List all options
- List<Field> allFields = Lists.newArrayList();
- for (Class<? extends OptionsBase> optionsClass : impl.getOptionsClasses()) {
- allFields.addAll(impl.getAnnotatedFieldsFor(optionsClass));
+ 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>() {
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 918fef1..5c635fb 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java
@@ -25,7 +25,6 @@
import com.google.common.collect.Iterators;
import com.google.common.collect.LinkedHashMultimap;
import com.google.common.collect.Lists;
-import com.google.common.collect.Maps;
import com.google.common.collect.Multimap;
import com.google.devtools.common.options.OptionsParser.OptionDescription;
import com.google.devtools.common.options.OptionsParser.OptionUsageRestrictions;
@@ -33,10 +32,11 @@
import com.google.devtools.common.options.OptionsParser.UnparsedOptionValueDescription;
import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
+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.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
@@ -61,14 +61,14 @@
*
* This map is modified by repeated calls to {@link #parse(OptionPriority,Function,List)}.
*/
- private final Map<Field, OptionValueDescription> parsedValues = Maps.newHashMap();
+ private final Map<Field, OptionValueDescription> parsedValues = new HashMap<>();
/**
* We store the pre-parsed, explicit options for each priority in here.
* We use partially preparsed options, which can be different from the original
* representation, e.g. "--nofoo" becomes "--foo=0".
*/
- private final List<UnparsedOptionValueDescription> unparsedValues = Lists.newArrayList();
+ private final List<UnparsedOptionValueDescription> unparsedValues = new ArrayList<>();
/**
* Unparsed values for use with the canonicalize command are stored separately from
@@ -82,7 +82,7 @@
private final Multimap<Field, UnparsedOptionValueDescription> canonicalizeValues
= LinkedHashMultimap.create();
- private final List<String> warnings = Lists.newArrayList();
+ private final List<String> warnings = new ArrayList<>();
private boolean allowSingleDashLongOptions = false;
@@ -122,8 +122,10 @@
* The implementation of {@link OptionsBase#asMap}.
*/
static Map<String, Object> optionsAsMap(OptionsBase optionsInstance) {
- Map<String, Object> map = Maps.newHashMap();
- for (Field field : OptionsParser.getAllAnnotatedFields(optionsInstance.getClass())) {
+ Class<? extends OptionsBase> optionsClass = optionsInstance.getClass();
+ OptionsData data = OptionsParser.getOptionsDataInternal(optionsClass);
+ Map<String, Object> map = new HashMap<>();
+ for (Field field : data.getFieldsForClass(optionsClass)) {
try {
String name = field.getAnnotation(Option.class).name();
Object value = field.get(optionsInstance);
@@ -135,10 +137,6 @@
return map;
}
- List<Field> getAnnotatedFieldsFor(Class<? extends OptionsBase> clazz) {
- return optionsData.getFieldsForClass(clazz);
- }
-
/**
* Implements {@link OptionsParser#asListOfUnparsedOptions()}.
*/
@@ -200,7 +198,7 @@
}
});
- List<String> result = Lists.newArrayList();
+ List<String> result = new ArrayList<>();
for (UnparsedOptionValueDescription value : processed) {
// Ignore expansion options.
@@ -217,7 +215,7 @@
* Implements {@link OptionsParser#asListOfEffectiveOptions()}.
*/
List<OptionValueDescription> asListOfEffectiveOptions() {
- List<OptionValueDescription> result = Lists.newArrayList();
+ List<OptionValueDescription> result = new ArrayList<>();
for (Map.Entry<String, Field> mapEntry : optionsData.getAllNamedFields()) {
String fieldName = mapEntry.getKey();
Field field = mapEntry.getValue();
@@ -241,10 +239,6 @@
return result;
}
- Collection<Class<? extends OptionsBase>> getOptionsClasses() {
- return optionsData.getOptionsClasses();
- }
-
private void maybeAddDeprecationWarning(Field field) {
Option option = field.getAnnotation(Option.class);
// Continue to support the old behavior for @Deprecated options.
@@ -449,8 +443,8 @@
String expandedFrom,
List<String> args) throws OptionsParsingException {
- List<String> unparsedArgs = Lists.newArrayList();
- LinkedHashMap<String, List<String>> implicitRequirements = Maps.newLinkedHashMap();
+ List<String> unparsedArgs = new ArrayList<>();
+ LinkedHashMap<String, List<String>> implicitRequirements = new LinkedHashMap<>();
Iterator<String> argsIterator = argsPreProcessor.preProcess(args).iterator();
while (argsIterator.hasNext()) {
@@ -712,7 +706,7 @@
if (constructor == null) {
return null;
}
- optionsInstance = constructor.newInstance(new Object[0]);
+ optionsInstance = constructor.newInstance();
} catch (Exception e) {
throw new IllegalStateException(e);
}
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 b898ae6..1c9f519 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsUsage.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsUsage.java
@@ -16,10 +16,10 @@
import com.google.common.base.Joiner;
import com.google.common.base.Splitter;
import com.google.common.base.Strings;
-import com.google.common.collect.Lists;
import com.google.common.escape.Escaper;
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;
@@ -40,8 +40,8 @@
* OptionsBase} subclasses they depend on until a complete parser is constructed).
*/
static void getUsage(Class<? extends OptionsBase> optionsClass, StringBuilder usage) {
- List<Field> optionFields =
- Lists.newArrayList(OptionsParser.getAllAnnotatedFields(optionsClass));
+ OptionsData data = OptionsParser.getOptionsDataInternal(optionsClass);
+ List<Field> optionFields = new ArrayList<>(data.getFieldsForClass(optionsClass));
Collections.sort(optionFields, BY_NAME);
for (Field optionField : optionFields) {
getUsage(optionField, usage, OptionsParser.HelpVerbosity.LONG, null);
