Add OptionDefinition layer between the @Option annotation and its fields and the options parser.
Removes any direct reads of the annotation outside of OptionDefinition. This allows for fewer manual checks for the annotation's existence, unifies error wording, and paves the way for potentially generifying the OptionsParser to accept different @Option-equivalent annotations.
Also allows for cleanup of duplicate code by giving @Option-specific operations a clear home, such as sorts and default logic. In followup changes, we can eliminate some unnecessarily complex caching by instead memoizing values in the OptionDefinition. This will have the positive side effect of making sure reads come from the cached values.
RELNOTES: None.
PiperOrigin-RevId: 166019075
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/TransitiveOptionDetails.java b/src/main/java/com/google/devtools/build/lib/analysis/config/TransitiveOptionDetails.java
index f0cfae8..72f001f 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/TransitiveOptionDetails.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/TransitiveOptionDetails.java
@@ -16,9 +16,10 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
-import com.google.devtools.common.options.Option;
+import com.google.devtools.common.options.OptionDefinition;
import com.google.devtools.common.options.OptionMetadataTag;
import com.google.devtools.common.options.OptionsBase;
+import com.google.devtools.common.options.OptionsParser.ConstructionException;
import java.io.Serializable;
import java.lang.reflect.Field;
import java.util.Map;
@@ -43,24 +44,30 @@
try {
for (OptionsBase options : buildOptions) {
for (Field field : options.getClass().getFields()) {
- if (field.isAnnotationPresent(Option.class)) {
- Option option = field.getAnnotation(Option.class);
- if (ImmutableList.copyOf(option.metadataTags()).contains(OptionMetadataTag.INTERNAL)) {
+ OptionDefinition optionDefinition;
+ try {
+ optionDefinition = OptionDefinition.extractOptionDefinition(field);
+ } catch (ConstructionException e) {
+ // Skip non @Option fields.
+ continue;
+ }
+ if (ImmutableList.copyOf(optionDefinition.getOptionMetadataTags())
+ .contains(OptionMetadataTag.INTERNAL)) {
// ignore internal options
continue;
}
Object value = field.get(options);
if (value == null) {
- if (lateBoundDefaults.containsKey(option.name())) {
- value = lateBoundDefaults.get(option.name());
- } else if (!option.defaultValue().equals("null")) {
- // See {@link Option#defaultValue} for an explanation of default "null" strings.
- value = option.defaultValue();
+ if (lateBoundDefaults.containsKey(optionDefinition.getOptionName())) {
+ value = lateBoundDefaults.get(optionDefinition.getOptionName());
+ } else if (!optionDefinition.isSpecialNullDefault()) {
+ // See {@link Option#defaultValue} for an explanation of default "null" strings.
+ value = optionDefinition.getUnparsedDefaultValue();
}
}
- map.put(option.name(),
- new OptionDetails(options.getClass(), value, option.allowMultiple()));
- }
+ map.put(
+ optionDefinition.getOptionName(),
+ new OptionDetails(options.getClass(), value, optionDefinition.allowsMultiple()));
}
}
} catch (IllegalAccessException e) {
@@ -105,15 +112,15 @@
}
/**
- * Returns the {@link Option} class the defines the given option, null if the option isn't
+ * Returns the {@link OptionsBase} class the defines the given option, null if the option isn't
* recognized.
*
* <p>optionName is the name of the option as it appears on the command line e.g. {@link
- * Option#name}).
+ * OptionDefinition#getOptionName()}).
*/
public Class<? extends OptionsBase> getOptionClass(String optionName) {
- OptionDetails optionData = transitiveOptionsMap.get(optionName);
- return optionData == null ? null : optionData.optionsClass;
+ OptionDetails optionDetails = transitiveOptionsMap.get(optionName);
+ return optionDetails == null ? null : optionDetails.optionsClass;
}
/**
@@ -122,23 +129,23 @@
* distinguish between that and an unknown option.
*
* <p>optionName is the name of the option as it appears on the command line e.g. {@link
- * Option#name}).
+ * OptionDefinition#getOptionName()}).
*/
public Object getOptionValue(String optionName) {
- OptionDetails optionData = transitiveOptionsMap.get(optionName);
- return (optionData == null) ? null : optionData.value;
+ OptionDetails optionDetails = transitiveOptionsMap.get(optionName);
+ return (optionDetails == null) ? null : optionDetails.value;
}
/**
* Returns whether or not the given option supports multiple values at the command line (e.g.
- * "--myoption value1 --myOption value2 ..."). Returns false for unrecognized options. Use
- * {@link #getOptionClass} to distinguish between those and legitimate single-value options.
+ * "--myoption value1 --myOption value2 ..."). Returns false for unrecognized options. Use {@link
+ * #getOptionClass} to distinguish between those and legitimate single-value options.
*
- * <p>As declared in {@link Option#allowMultiple}, multi-value options are expected to be
- * of type {@code List<T>}.
+ * <p>As declared in {@link OptionDefinition#allowsMultiple()}, multi-value options are expected
+ * to be of type {@code List<T>}.
*/
public boolean allowsMultipleValues(String optionName) {
- OptionDetails optionData = transitiveOptionsMap.get(optionName);
- return (optionData == null) ? false : optionData.allowsMultiple;
+ OptionDetails optionDetails = transitiveOptionsMap.get(optionName);
+ return optionDetails != null && optionDetails.allowsMultiple;
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/AllIncompatibleChangesExpansion.java b/src/main/java/com/google/devtools/build/lib/runtime/AllIncompatibleChangesExpansion.java
index a23a4c5..6c42c20 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/AllIncompatibleChangesExpansion.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/AllIncompatibleChangesExpansion.java
@@ -18,12 +18,11 @@
import com.google.devtools.common.options.Converter;
import com.google.devtools.common.options.ExpansionContext;
import com.google.devtools.common.options.ExpansionFunction;
-import com.google.devtools.common.options.IsolatedOptionsData;
import com.google.devtools.common.options.Option;
+import com.google.devtools.common.options.OptionDefinition;
import com.google.devtools.common.options.OptionMetadataTag;
import com.google.devtools.common.options.OptionsBase;
import com.google.devtools.common.options.OptionsParser;
-import java.lang.reflect.Field;
import java.util.ArrayList;
import java.util.Map;
@@ -97,55 +96,55 @@
* <p>If any of these requirements are not satisfied, {@link IllegalArgumentException} is thrown,
* as this constitutes an internal error in the declaration of the option.
*/
- private static void validateIncompatibleChange(Field field, Option annotation) {
- String prefix = "Incompatible change option '--" + annotation.name() + "' ";
+ private static void validateIncompatibleChange(OptionDefinition optionDefinition) {
+ String prefix = "Incompatible change option '--" + optionDefinition.getOptionName() + "' ";
// To avoid ambiguity, and the suggestion of using .isEmpty().
String defaultString = "";
// Validate that disallowed fields aren't used. These will need updating if the default values
// in Option ever change, and perhaps if new fields are added.
- if (annotation.abbrev() != '\0') {
+ if (optionDefinition.getAbbreviation() != '\0') {
throw new IllegalArgumentException(prefix + "must not use the abbrev field");
}
- if (!annotation.valueHelp().equals(defaultString)) {
+ if (!optionDefinition.getValueTypeHelpText().equals(defaultString)) {
throw new IllegalArgumentException(prefix + "must not use the valueHelp field");
}
- if (annotation.converter() != Converter.class) {
+ if (optionDefinition.getProvidedConverter() != Converter.class) {
throw new IllegalArgumentException(prefix + "must not use the converter field");
}
- if (annotation.allowMultiple()) {
+ if (optionDefinition.allowsMultiple()) {
throw new IllegalArgumentException(prefix + "must not use the allowMultiple field");
}
- if (annotation.implicitRequirements().length > 0) {
+ if (optionDefinition.getImplicitRequirements().length > 0) {
throw new IllegalArgumentException(prefix + "must not use the implicitRequirements field");
}
- if (!annotation.oldName().equals(defaultString)) {
+ if (!optionDefinition.getOldOptionName().equals(defaultString)) {
throw new IllegalArgumentException(prefix + "must not use the oldName field");
}
- if (annotation.wrapperOption()) {
+ if (optionDefinition.isWrapperOption()) {
throw new IllegalArgumentException(prefix + "must not use the wrapperOption field");
}
// Validate the fields that are actually allowed.
- if (!annotation.name().startsWith(INCOMPATIBLE_NAME_PREFIX)) {
+ if (!optionDefinition.getOptionName().startsWith(INCOMPATIBLE_NAME_PREFIX)) {
throw new IllegalArgumentException(prefix + "must have name starting with \"incompatible_\"");
}
- if (!annotation.category().equals(INCOMPATIBLE_CATEGORY)) {
+ if (!optionDefinition.getOptionCategory().equals(INCOMPATIBLE_CATEGORY)) {
throw new IllegalArgumentException(prefix + "must have category \"incompatible changes\"");
}
- if (!ImmutableList.copyOf(annotation.metadataTags())
+ if (!ImmutableList.copyOf(optionDefinition.getOptionMetadataTags())
.contains(OptionMetadataTag.INCOMPATIBLE_CHANGE)) {
throw new IllegalArgumentException(
prefix + "must have metadata tag \"OptionMetadataTag.INCOMPATIBLE_CHANGE\"");
}
- if (!IsolatedOptionsData.isExpansionOption(annotation)) {
- if (!field.getType().equals(Boolean.TYPE)) {
+ if (!optionDefinition.isExpansionOption()) {
+ if (!optionDefinition.getType().equals(Boolean.TYPE)) {
throw new IllegalArgumentException(
prefix + "must have boolean type (unless it's an expansion option)");
}
}
- if (annotation.help().equals(defaultString)) {
+ if (optionDefinition.getHelpText().equals(defaultString)) {
throw new IllegalArgumentException(
prefix
+ "must have a \"help\" string that refers the user to "
@@ -158,13 +157,12 @@
// Grab all registered options that are identified as incompatible changes by either name or
// by category. Ensure they satisfy our requirements.
ArrayList<String> incompatibleChanges = new ArrayList<>();
- for (Map.Entry<String, Field> entry : context.getOptionsData().getAllNamedFields()) {
- Field field = entry.getValue();
- Option annotation = field.getAnnotation(Option.class);
- if (annotation.name().startsWith(INCOMPATIBLE_NAME_PREFIX)
- || annotation.category().equals(INCOMPATIBLE_CATEGORY)) {
- validateIncompatibleChange(field, annotation);
- incompatibleChanges.add("--" + annotation.name());
+ for (Map.Entry<String, OptionDefinition> entry : context.getOptionsData().getAllNamedFields()) {
+ OptionDefinition optionDefinition = entry.getValue();
+ if (optionDefinition.getOptionName().startsWith(INCOMPATIBLE_NAME_PREFIX)
+ || optionDefinition.getOptionCategory().equals(INCOMPATIBLE_CATEGORY)) {
+ validateIncompatibleChange(optionDefinition);
+ incompatibleChanges.add("--" + optionDefinition.getOptionName());
}
}
// Sort to get a deterministic canonical order. This probably isn't necessary because the
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
index e37a6ec..8d9f93c 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
@@ -15,7 +15,6 @@
package com.google.devtools.build.lib.runtime;
import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Function;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Lists;
@@ -76,11 +75,12 @@
import com.google.devtools.build.lib.windows.WindowsSubprocessFactory;
import com.google.devtools.common.options.CommandNameCache;
import com.google.devtools.common.options.InvocationPolicyParser;
-import com.google.devtools.common.options.Option;
+import com.google.devtools.common.options.OptionDefinition;
import com.google.devtools.common.options.OptionPriority;
import com.google.devtools.common.options.OptionsBase;
import com.google.devtools.common.options.OptionsClassProvider;
import com.google.devtools.common.options.OptionsParser;
+import com.google.devtools.common.options.OptionsParser.ConstructionException;
import com.google.devtools.common.options.OptionsParsingException;
import com.google.devtools.common.options.OptionsProvider;
import com.google.devtools.common.options.TriState;
@@ -101,6 +101,7 @@
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.Function;
import java.util.logging.Handler;
import java.util.logging.Level;
import java.util.logging.LogRecord;
@@ -652,11 +653,16 @@
}
for (Field field : startupFields) {
- if (field.isAnnotationPresent(Option.class)) {
- prefixes.add("--" + field.getAnnotation(Option.class).name());
+ try {
+ OptionDefinition optionDefinition = OptionDefinition.extractOptionDefinition(field);
+ prefixes.add("--" + optionDefinition.getOptionName());
if (field.getType() == boolean.class || field.getType() == TriState.class) {
- prefixes.add("--no" + field.getAnnotation(Option.class).name());
+ prefixes.add("--no" + optionDefinition.getOptionName());
}
+ } catch (ConstructionException e) {
+ // Do nothing, just ignore fields that are not actually options. OptionsBases technically
+ // shouldn't have fields that are not @Options, but this is a requirement that isn't yet
+ // being enforced, so this should not cause a failure here.
}
}
diff --git a/src/main/java/com/google/devtools/common/options/ExpansionContext.java b/src/main/java/com/google/devtools/common/options/ExpansionContext.java
index 74dac97..c6aecc7 100644
--- a/src/main/java/com/google/devtools/common/options/ExpansionContext.java
+++ b/src/main/java/com/google/devtools/common/options/ExpansionContext.java
@@ -26,13 +26,15 @@
@ThreadSafe
public final class ExpansionContext {
private final IsolatedOptionsData optionsData;
- private final Field field;
+ private final OptionDefinition optionDefinition;
@Nullable private final String unparsedValue;
public ExpansionContext(
- IsolatedOptionsData optionsData, Field field, @Nullable String unparsedValue) {
+ IsolatedOptionsData optionsData,
+ OptionDefinition optionDefinition,
+ @Nullable String unparsedValue) {
this.optionsData = optionsData;
- this.field = field;
+ this.optionDefinition = optionDefinition;
this.unparsedValue = unparsedValue;
}
@@ -42,8 +44,8 @@
}
/** {@link Field} object for option that is being expanded. */
- public Field getField() {
- return field;
+ public OptionDefinition getOptionDefinition() {
+ return optionDefinition;
}
/** Argument given to this flag during options parsing. Will be null if no argument was given. */
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 a66d93e..4be2b23 100644
--- a/src/main/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java
+++ b/src/main/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java
@@ -114,7 +114,7 @@
}
OptionDescription optionDescription = parser.getOptionDescription(flagName);
- // getOptionDescription() will return null if the option does not exist, however
+ // extractOptionDefinition() will return null if the option does not exist, however
// getOptionValueDescription() above would have thrown an IllegalArgumentException if that
// were the case.
Verify.verifyNotNull(optionDescription);
diff --git a/src/main/java/com/google/devtools/common/options/IsolatedOptionsData.java b/src/main/java/com/google/devtools/common/options/IsolatedOptionsData.java
index bd74b67..e087dbb 100644
--- a/src/main/java/com/google/devtools/common/options/IsolatedOptionsData.java
+++ b/src/main/java/com/google/devtools/common/options/IsolatedOptionsData.java
@@ -16,7 +16,6 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Ordering;
import com.google.devtools.common.options.OptionsParser.ConstructionException;
import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
@@ -24,13 +23,14 @@
import java.lang.reflect.Modifier;
import java.lang.reflect.ParameterizedType;
import java.lang.reflect.Type;
-import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
+import java.util.Objects;
import javax.annotation.concurrent.Immutable;
/**
@@ -57,20 +57,21 @@
private final ImmutableMap<Class<? extends OptionsBase>, Constructor<?>> optionsClasses;
/**
- * Mapping from option name to {@code @Option}-annotated field. Entries appear ordered first by
- * their options class (the order in which they were passed to {@link #from(Collection)}, and then
- * in alphabetic order within each options class.
+ * Mapping from option name to {@code OptionDefinition}. Entries appear ordered first by their
+ * options class (the order in which they were passed to {@link #from(Collection)}, and then in
+ * alphabetic order within each options class.
*/
- private final ImmutableMap<String, Field> nameToField;
+ private final ImmutableMap<String, OptionDefinition> nameToField;
- /** Mapping from option abbreviation to {@code Option}-annotated field (unordered). */
- private final ImmutableMap<Character, Field> abbrevToField;
+ /** Mapping from option abbreviation to {@code OptionDefinition} (unordered). */
+ private final ImmutableMap<Character, OptionDefinition> abbrevToField;
/**
- * Mapping from options class to a list of all {@code Option}-annotated fields in that class. The
- * map entries are unordered, but the fields in the lists are ordered alphabetically.
+ * Mapping from options class to a list of all {@code OptionFields} in that class. The map entries
+ * are unordered, but the fields in the lists are ordered alphabetically.
*/
- private final ImmutableMap<Class<? extends OptionsBase>, ImmutableList<Field>> allOptionsFields;
+ private final ImmutableMap<Class<? extends OptionsBase>, ImmutableList<OptionDefinition>>
+ allOptionsFields;
/**
* Mapping from each {@code Option}-annotated field to the default value for that field
@@ -79,20 +80,20 @@
* <p>(This is immutable like the others, but uses {@code Collections.unmodifiableMap} to support
* null values.)
*/
- private final Map<Field, Object> optionDefaults;
+ private final Map<OptionDefinition, Object> optionDefaults;
/**
* Mapping from each {@code Option}-annotated field to the proper converter (unordered).
*
* @see #findConverter
*/
- private final ImmutableMap<Field, Converter<?>> converters;
+ private final ImmutableMap<OptionDefinition, Converter<?>> converters;
/**
* Mapping from each {@code Option}-annotated field to a boolean for whether that field allows
* multiple values (unordered).
*/
- private final ImmutableMap<Field, Boolean> allowMultiple;
+ private final ImmutableMap<OptionDefinition, Boolean> allowMultiple;
/**
* Mapping from each options class to whether or not it has the {@link UsesOnlyCoreTypes}
@@ -105,14 +106,13 @@
"undocumented", "hidden", "internal");
private IsolatedOptionsData(
- Map<Class<? extends OptionsBase>,
- Constructor<?>> optionsClasses,
- Map<String, Field> nameToField,
- Map<Character, Field> abbrevToField,
- Map<Class<? extends OptionsBase>, ImmutableList<Field>> allOptionsFields,
- Map<Field, Object> optionDefaults,
- Map<Field, Converter<?>> converters,
- Map<Field, Boolean> allowMultiple,
+ Map<Class<? extends OptionsBase>, Constructor<?>> optionsClasses,
+ Map<String, OptionDefinition> nameToField,
+ Map<Character, OptionDefinition> abbrevToField,
+ Map<Class<? extends OptionsBase>, ImmutableList<OptionDefinition>> allOptionsFields,
+ Map<OptionDefinition, Object> optionDefaults,
+ Map<OptionDefinition, Converter<?>> converters,
+ Map<OptionDefinition, Boolean> allowMultiple,
Map<Class<? extends OptionsBase>, Boolean> usesOnlyCoreTypes) {
this.optionsClasses = ImmutableMap.copyOf(optionsClasses);
this.nameToField = ImmutableMap.copyOf(nameToField);
@@ -150,7 +150,7 @@
return (Constructor<T>) optionsClasses.get(clazz);
}
- public Field getFieldFromName(String name) {
+ public OptionDefinition getFieldFromName(String name) {
return nameToField.get(name);
}
@@ -159,11 +159,11 @@
* objects. Entries appear ordered first by their options class (the order in which they were
* passed to {@link #from(Collection)}, and then in alphabetic order within each options class.
*/
- public Iterable<Map.Entry<String, Field>> getAllNamedFields() {
+ public Iterable<Map.Entry<String, OptionDefinition>> getAllNamedFields() {
return nameToField.entrySet();
}
- public Field getFieldForAbbrev(char abbrev) {
+ public OptionDefinition getFieldForAbbrev(char abbrev) {
return abbrevToField.get(abbrev);
}
@@ -171,20 +171,21 @@
* Returns a list of all {@link Field} objects for options in the given options class, ordered
* alphabetically by option name.
*/
- public ImmutableList<Field> getFieldsForClass(Class<? extends OptionsBase> optionsClass) {
+ public ImmutableList<OptionDefinition> getOptionDefinitionsFromClass(
+ Class<? extends OptionsBase> optionsClass) {
return allOptionsFields.get(optionsClass);
}
- public Object getDefaultValue(Field field) {
- return optionDefaults.get(field);
+ public Object getDefaultValue(OptionDefinition optionDefinition) {
+ return optionDefaults.get(optionDefinition);
}
- public Converter<?> getConverter(Field field) {
- return converters.get(field);
+ public Converter<?> getConverter(OptionDefinition optionDefinition) {
+ return converters.get(optionDefinition);
}
- public boolean getAllowMultiple(Field field) {
- return allowMultiple.get(field);
+ public boolean getAllowMultiple(OptionDefinition optionDefinition) {
+ return allowMultiple.get(optionDefinition);
}
public boolean getUsesOnlyCoreTypes(Class<? extends OptionsBase> optionsClass) {
@@ -196,9 +197,9 @@
* that does use it, asserts that the type is a {@code List<T>} and returns its element type
* {@code T}.
*/
- private static Type getFieldSingularType(Field field, Option annotation) {
- Type fieldType = field.getGenericType();
- if (annotation.allowMultiple()) {
+ private static Type getFieldSingularType(OptionDefinition optionDefinition) {
+ Type fieldType = optionDefinition.getField().getGenericType();
+ if (optionDefinition.allowsMultiple()) {
// If the type isn't a List<T>, this is an error in the option's declaration.
if (!(fieldType instanceof ParameterizedType)) {
throw new ConstructionException("Type of multiple occurrence option must be a List<...>");
@@ -217,43 +218,24 @@
*
* <p>Can be used for usage help and controlling whether the "no" prefix is allowed.
*/
- boolean isBooleanField(Field field) {
- return isBooleanField(field, getConverter(field));
+ boolean isBooleanField(OptionDefinition optionDefinition) {
+ return isBooleanField(optionDefinition, getConverter(optionDefinition));
}
- private static boolean isBooleanField(Field field, Converter<?> converter) {
- return field.getType().equals(boolean.class)
- || field.getType().equals(TriState.class)
+ private static boolean isBooleanField(OptionDefinition optionDefinition, Converter<?> converter) {
+ return optionDefinition.getType().equals(boolean.class)
+ || optionDefinition.getType().equals(TriState.class)
|| converter instanceof BoolOrEnumConverter;
}
- /** Returns whether a field has Void type. */
- static boolean isVoidField(Field field) {
- return field.getType().equals(Void.class);
- }
-
- /** Returns whether the arg is an expansion option. */
- public static boolean isExpansionOption(Option annotation) {
- return (annotation.expansion().length > 0 || OptionsData.usesExpansionFunction(annotation));
- }
-
- /**
- * Returns whether the arg is an expansion option defined by an expansion function (and not a
- * constant expansion value).
- */
- static boolean usesExpansionFunction(Option annotation) {
- return annotation.expansionFunction() != ExpansionFunction.class;
- }
-
/**
* Given an {@code @Option}-annotated field, retrieves the {@link Converter} that will be used,
* taking into account the default converters if an explicit one is not specified.
*/
- private static Converter<?> findConverter(Field optionField) {
- Option annotation = optionField.getAnnotation(Option.class);
- if (annotation.converter() == Converter.class) {
+ private static Converter<?> findConverter(OptionDefinition optionDefinition) {
+ if (optionDefinition.getProvidedConverter() == Converter.class) {
// No converter provided, use the default one.
- Type type = getFieldSingularType(optionField, annotation);
+ Type type = getFieldSingularType(optionDefinition);
Converter<?> converter = Converters.DEFAULT_CONVERTERS.get(type);
if (converter == null) {
throw new ConstructionException(
@@ -261,13 +243,13 @@
+ type
+ "; possible fix: add "
+ "converter=... to @Option annotation for "
- + optionField.getName());
+ + optionDefinition.getField().getName());
}
return converter;
}
try {
// Instantiate the given Converter class.
- Class<?> converter = annotation.converter();
+ Class<?> converter = optionDefinition.getProvidedConverter();
Constructor<?> constructor = converter.getConstructor();
return (Converter<?>) constructor.newInstance();
} catch (Exception e) {
@@ -276,40 +258,32 @@
throw new ConstructionException(e);
}
}
-
- private static final Ordering<Field> fieldOrdering =
- new Ordering<Field>() {
- @Override
- public int compare(Field f1, Field f2) {
- String n1 = f1.getAnnotation(Option.class).name();
- String n2 = f2.getAnnotation(Option.class).name();
- return n1.compareTo(n2);
- }
- };
-
- /**
- * Return all {@code @Option}-annotated fields, alphabetically ordered by their option name (not
- * their field name).
- */
- private static ImmutableList<Field> getAllAnnotatedFieldsSorted(
+ /** Returns all {@code optionDefinitions}, ordered by their option name (not their field name). */
+ private static ImmutableList<OptionDefinition> getAllOptionDefinitionsSorted(
Class<? extends OptionsBase> optionsClass) {
- List<Field> unsortedFields = new ArrayList<>();
- for (Field field : optionsClass.getFields()) {
- if (field.isAnnotationPresent(Option.class)) {
- unsortedFields.add(field);
- }
- }
- return fieldOrdering.immutableSortedCopy(unsortedFields);
+ return Arrays.stream(optionsClass.getFields())
+ .map(field -> {
+ try {
+ return OptionDefinition.extractOptionDefinition(field);
+ } catch (ConstructionException e) {
+ // Ignore non-@Option annotated fields. Requiring all fields in the OptionsBase to be
+ // @Option-annotated requires a depot cleanup.
+ return null;
+ }
+ })
+ .filter(Objects::nonNull)
+ .sorted(OptionDefinition.BY_OPTION_NAME)
+ .collect(ImmutableList.toImmutableList());
}
- private static Object retrieveDefaultFromAnnotation(Field optionField) {
- Converter<?> converter = findConverter(optionField);
- String defaultValueAsString = OptionsParserImpl.getDefaultOptionString(optionField);
+ private static Object retrieveDefaultValue(OptionDefinition optionDefinition) {
+ Converter<?> converter = findConverter(optionDefinition);
+ String defaultValueAsString = optionDefinition.getUnparsedDefaultValue();
// Special case for "null"
- if (OptionsParserImpl.isSpecialNullDefault(defaultValueAsString, optionField)) {
+ if (optionDefinition.isSpecialNullDefault()) {
return null;
}
- boolean allowsMultiple = optionField.getAnnotation(Option.class).allowMultiple();
+ boolean allowsMultiple = optionDefinition.allowsMultiple();
// If the option allows multiple values then we intentionally return the empty list as
// the default value of this option since it is not always the case that an option
// that allows multiple values will have a converter that returns a list value.
@@ -321,17 +295,18 @@
try {
convertedValue = converter.convert(defaultValueAsString);
} catch (OptionsParsingException e) {
- throw new IllegalStateException("OptionsParsingException while "
- + "retrieving default for " + optionField.getName() + ": "
- + e.getMessage());
+ throw new IllegalStateException(
+ "OptionsParsingException while "
+ + "retrieving default for "
+ + optionDefinition.getField().getName()
+ + ": "
+ + e.getMessage());
}
return convertedValue;
}
private static <A> void checkForCollisions(
- Map<A, Field> aFieldMap,
- A optionName,
- String description) {
+ Map<A, OptionDefinition> aFieldMap, A optionName, String description) {
if (aFieldMap.containsKey(optionName)) {
throw new DuplicateOptionDeclarationException(
"Duplicate option name, due to " + description + ": --" + optionName);
@@ -354,7 +329,7 @@
}
private static void checkAndUpdateBooleanAliases(
- Map<String, Field> nameToFieldMap,
+ Map<String, OptionDefinition> nameToFieldMap,
Map<String, String> booleanAliasMap,
String optionName) {
// Check that the negating alias does not conflict with existing flags.
@@ -418,13 +393,13 @@
static IsolatedOptionsData from(Collection<Class<? extends OptionsBase>> classes) {
// Mind which fields have to preserve order.
Map<Class<? extends OptionsBase>, Constructor<?>> constructorBuilder = new LinkedHashMap<>();
- Map<Class<? extends OptionsBase>, ImmutableList<Field>> allOptionsFieldsBuilder =
+ Map<Class<? extends OptionsBase>, ImmutableList<OptionDefinition>> allOptionsFieldsBuilder =
new HashMap<>();
- Map<String, Field> nameToFieldBuilder = new LinkedHashMap<>();
- Map<Character, Field> abbrevToFieldBuilder = new HashMap<>();
- Map<Field, Object> optionDefaultsBuilder = new HashMap<>();
- Map<Field, Converter<?>> convertersBuilder = new HashMap<>();
- Map<Field, Boolean> allowMultipleBuilder = new HashMap<>();
+ Map<String, OptionDefinition> nameToFieldBuilder = new LinkedHashMap<>();
+ Map<Character, OptionDefinition> abbrevToFieldBuilder = new HashMap<>();
+ Map<OptionDefinition, Object> optionDefaultsBuilder = new HashMap<>();
+ Map<OptionDefinition, Converter<?>> convertersBuilder = new HashMap<>();
+ Map<OptionDefinition, Boolean> allowMultipleBuilder = new HashMap<>();
// Maps the negated boolean flag aliases to the original option name.
Map<String, String> booleanAliasMap = new HashMap<>();
@@ -441,44 +416,54 @@
throw new IllegalArgumentException(parsedOptionsClass
+ " lacks an accessible default constructor");
}
- ImmutableList<Field> fields = getAllAnnotatedFieldsSorted(parsedOptionsClass);
- allOptionsFieldsBuilder.put(parsedOptionsClass, fields);
+ ImmutableList<OptionDefinition> optionDefinitions =
+ getAllOptionDefinitionsSorted(parsedOptionsClass);
+ allOptionsFieldsBuilder.put(parsedOptionsClass, optionDefinitions);
- for (Field field : fields) {
- Option annotation = field.getAnnotation(Option.class);
- String optionName = annotation.name();
+ for (OptionDefinition optionDefinition : optionDefinitions) {
+ String optionName = optionDefinition.getOptionName();
+
+ // Check that the option makes sense on its own, as defined.
if (optionName == null) {
throw new ConstructionException("Option cannot have a null name");
}
- if (DEPRECATED_CATEGORIES.contains(annotation.category())) {
+ if (DEPRECATED_CATEGORIES.contains(optionDefinition.getOptionCategory())) {
throw new ConstructionException(
"Documentation level is no longer read from the option category. Category \""
- + annotation.category() + "\" in option \"" + optionName + "\" is disallowed.");
+ + optionDefinition.getOptionCategory()
+ + "\" in option \""
+ + optionName
+ + "\" is disallowed.");
}
- checkEffectTagRationality(optionName, annotation.effectTags());
+ checkEffectTagRationality(optionName, optionDefinition.getOptionEffectTags());
checkMetadataTagAndCategoryRationality(
- optionName, annotation.metadataTags(), annotation.documentationCategory());
-
- Type fieldType = getFieldSingularType(field, annotation);
+ optionName,
+ optionDefinition.getOptionMetadataTags(),
+ optionDefinition.getDocumentationCategory());
+ Type fieldType = getFieldSingularType(optionDefinition);
// For simple, static expansions, don't accept non-Void types.
- if (annotation.expansion().length != 0 && !isVoidField(field)) {
+ if (optionDefinition.getOptionExpansion().length != 0 && !optionDefinition.isVoidField()) {
throw new ConstructionException(
"Option "
- + optionName
+ + optionDefinition.getOptionName()
+ " is an expansion flag with a static expansion, but does not have Void type.");
}
- // Get the converter return type.
+ // Get the converter's return type to check that it matches the option type.
@SuppressWarnings("rawtypes")
- Class<? extends Converter> converterClass = annotation.converter();
+ Class<? extends Converter> converterClass = optionDefinition.getProvidedConverter();
if (converterClass == Converter.class) {
Converter<?> actualConverter = Converters.DEFAULT_CONVERTERS.get(fieldType);
if (actualConverter == null) {
- throw new ConstructionException("Cannot find converter for field of type "
- + field.getType() + " named " + field.getName()
- + " in class " + field.getDeclaringClass().getName());
+ throw new ConstructionException(
+ "Cannot find converter for field of type "
+ + optionDefinition.getType()
+ + " named "
+ + optionDefinition.getField().getName()
+ + " in class "
+ + optionDefinition.getField().getDeclaringClass().getName());
}
converterClass = actualConverter.getClass();
}
@@ -495,10 +480,10 @@
throw new ConstructionException(
"A known converter object doesn't implement the convert method");
}
- Converter<?> converter = findConverter(field);
- convertersBuilder.put(field, converter);
+ Converter<?> converter = findConverter(optionDefinition);
+ convertersBuilder.put(optionDefinition, converter);
- if (annotation.allowMultiple()) {
+ if (optionDefinition.allowsMultiple()) {
if (GenericTypeHelper.getRawType(converterResultType) == List.class) {
Type elementType =
((ParameterizedType) converterResultType).getActualTypeArguments()[0];
@@ -533,47 +518,51 @@
}
}
- if (isBooleanField(field, converter)) {
+ if (isBooleanField(optionDefinition, converter)) {
checkAndUpdateBooleanAliases(nameToFieldBuilder, booleanAliasMap, optionName);
}
checkForCollisions(nameToFieldBuilder, optionName, "option");
checkForBooleanAliasCollisions(booleanAliasMap, optionName, "option");
- nameToFieldBuilder.put(optionName, field);
+ nameToFieldBuilder.put(optionName, optionDefinition);
- if (!annotation.oldName().isEmpty()) {
- String oldName = annotation.oldName();
+ if (!optionDefinition.getOldOptionName().isEmpty()) {
+ String oldName = optionDefinition.getOldOptionName();
checkForCollisions(nameToFieldBuilder, oldName, "old option name");
checkForBooleanAliasCollisions(booleanAliasMap, oldName, "old option name");
- nameToFieldBuilder.put(annotation.oldName(), field);
+ nameToFieldBuilder.put(optionDefinition.getOldOptionName(), optionDefinition);
// If boolean, repeat the alias dance for the old name.
- if (isBooleanField(field, converter)) {
+ if (isBooleanField(optionDefinition, converter)) {
checkAndUpdateBooleanAliases(nameToFieldBuilder, booleanAliasMap, oldName);
}
}
- if (annotation.abbrev() != '\0') {
- checkForCollisions(abbrevToFieldBuilder, annotation.abbrev(), "option abbreviation");
- abbrevToFieldBuilder.put(annotation.abbrev(), field);
+ if (optionDefinition.getAbbreviation() != '\0') {
+ checkForCollisions(
+ abbrevToFieldBuilder, optionDefinition.getAbbreviation(), "option abbreviation");
+ abbrevToFieldBuilder.put(optionDefinition.getAbbreviation(), optionDefinition);
}
- optionDefaultsBuilder.put(field, retrieveDefaultFromAnnotation(field));
-
- allowMultipleBuilder.put(field, annotation.allowMultiple());
-
- }
+ optionDefaultsBuilder.put(optionDefinition, retrieveDefaultValue(optionDefinition));
+ allowMultipleBuilder.put(optionDefinition, optionDefinition.allowsMultiple());
+ }
boolean usesOnlyCoreTypes = parsedOptionsClass.isAnnotationPresent(UsesOnlyCoreTypes.class);
if (usesOnlyCoreTypes) {
// Validate that @UsesOnlyCoreTypes was used correctly.
- for (Field field : fields) {
+ for (OptionDefinition optionDefinition : optionDefinitions) {
// The classes in coreTypes are all final. But even if they weren't, we only want to check
// for exact matches; subclasses would not be considered core types.
- if (!UsesOnlyCoreTypes.CORE_TYPES.contains(field.getType())) {
+ if (!UsesOnlyCoreTypes.CORE_TYPES.contains(optionDefinition.getType())) {
throw new ConstructionException(
- "Options class '" + parsedOptionsClass.getName() + "' is marked as "
- + "@UsesOnlyCoreTypes, but field '" + field.getName()
- + "' has type '" + field.getType().getName() + "'");
+ "Options class '"
+ + parsedOptionsClass.getName()
+ + "' is marked as "
+ + "@UsesOnlyCoreTypes, but field '"
+ + optionDefinition.getField().getName()
+ + "' has type '"
+ + optionDefinition.getType().getName()
+ + "'");
}
}
}
diff --git a/src/main/java/com/google/devtools/common/options/Option.java b/src/main/java/com/google/devtools/common/options/Option.java
index 0c34fae..829f9e9 100644
--- a/src/main/java/com/google/devtools/common/options/Option.java
+++ b/src/main/java/com/google/devtools/common/options/Option.java
@@ -19,25 +19,21 @@
import java.lang.annotation.Target;
/**
- * An interface for annotating fields in classes (derived from OptionsBase)
- * that are options.
+ * An interface for annotating fields in classes (derived from OptionsBase) that are options.
+ *
+ * <p>The fields of this annotation have matching getters in {@link OptionDefinition}. Please do not
+ * access these fields directly, but instead go through that class.
*/
@Target(ElementType.FIELD)
@Retention(RetentionPolicy.RUNTIME)
public @interface Option {
- /**
- * The name of the option ("--name").
- */
+ /** The name of the option ("--name"). */
String name();
- /**
- * The single-character abbreviation of the option ("-abbrev").
- */
+ /** The single-character abbreviation of the option ("-a"). */
char abbrev() default '\0';
- /**
- * A help string for the usage information.
- */
+ /** A help string for the usage information. */
String help() default "";
/**
@@ -125,10 +121,10 @@
Class<? extends Converter> converter() default Converter.class;
/**
- * A flag indicating whether the option type should be allowed to occur multiple times in a single
- * option list.
+ * A boolean value indicating whether the option type should be allowed to occur multiple times in
+ * a single arg list.
*
- * <p>If the command can occur multiple times, then the attribute value <em>must</em> be a list
+ * <p>If the option can occur multiple times, then the attribute value <em>must</em> be a list
* type {@code List<T>}, and the result type of the converter for this option must either match
* the parameter {@code T} or {@code List<T>}. In the latter case the individual lists are
* concatenated to form the full options value.
diff --git a/src/main/java/com/google/devtools/common/options/OptionDefinition.java b/src/main/java/com/google/devtools/common/options/OptionDefinition.java
new file mode 100644
index 0000000..589208a
--- /dev/null
+++ b/src/main/java/com/google/devtools/common/options/OptionDefinition.java
@@ -0,0 +1,185 @@
+// Copyright 2017 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.common.options;
+
+import com.google.devtools.common.options.OptionsParser.ConstructionException;
+import java.lang.reflect.Field;
+import java.util.Comparator;
+
+/**
+ * Everything the {@link OptionsParser} needs to know about how an option is defined.
+ *
+ * <p>An {@code OptionDefinition} is effectively a wrapper around the {@link Option} annotation and
+ * the {@link Field} that is annotated, and should contain all logic about default settings and
+ * behavior.
+ */
+public final class OptionDefinition {
+
+ /**
+ * If the {@code field} is annotated with the appropriate @{@link Option} annotation, returns the
+ * {@code OptionDefinition} for that option. Otherwise, throws a {@link ConstructionException}.
+ */
+ public static OptionDefinition extractOptionDefinition(Field field) {
+ Option annotation = field == null ? null : field.getAnnotation(Option.class);
+ if (annotation == null) {
+ throw new ConstructionException(
+ "The field " + field + " does not have the right annotation to be considered an option.");
+ }
+ return new OptionDefinition(field, annotation);
+ }
+
+ private final Field field;
+ private final Option optionAnnotation;
+
+ private OptionDefinition(Field field, Option optionAnnotation) {
+ this.field = field;
+ this.optionAnnotation = optionAnnotation;
+ }
+
+ /** Returns the underlying {@code field} for this {@code OptionDefinition}. */
+ public Field getField() {
+ return field;
+ }
+
+ /**
+ * Returns the name of the option ("--name").
+ *
+ * <p>Labelled "Option" name to distinguish it from the field's name.
+ */
+ public String getOptionName() {
+ return optionAnnotation.name();
+ }
+
+ /** The single-character abbreviation of the option ("-a"). */
+ public char getAbbreviation() {
+ return optionAnnotation.abbrev();
+ }
+
+ /** {@link Option#help()} */
+ public String getHelpText() {
+ return optionAnnotation.help();
+ }
+
+ /** {@link Option#valueHelp()} */
+ public String getValueTypeHelpText() {
+ return optionAnnotation.valueHelp();
+ }
+
+ /** {@link Option#defaultValue()} */
+ public String getUnparsedDefaultValue() {
+ return optionAnnotation.defaultValue();
+ }
+
+ /** {@link Option#category()} */
+ public String getOptionCategory() {
+ return optionAnnotation.category();
+ }
+
+ /** {@link Option#documentationCategory()} */
+ public OptionDocumentationCategory getDocumentationCategory() {
+ return optionAnnotation.documentationCategory();
+ }
+
+ /** {@link Option#effectTags()} */
+ public OptionEffectTag[] getOptionEffectTags() {
+ return optionAnnotation.effectTags();
+ }
+
+ /** {@link Option#metadataTags()} */
+ public OptionMetadataTag[] getOptionMetadataTags() {
+ return optionAnnotation.metadataTags();
+ }
+
+ /** {@link Option#converter()} ()} */
+ @SuppressWarnings({"rawtypes"})
+ public Class<? extends Converter> getProvidedConverter() {
+ return optionAnnotation.converter();
+ }
+
+ /** {@link Option#allowMultiple()} */
+ public boolean allowsMultiple() {
+ return optionAnnotation.allowMultiple();
+ }
+
+ /** {@link Option#expansion()} */
+ public String[] getOptionExpansion() {
+ return optionAnnotation.expansion();
+ }
+
+ /** {@link Option#expansionFunction()} ()} */
+ Class<? extends ExpansionFunction> getExpansionFunction() {
+ return optionAnnotation.expansionFunction();
+ }
+
+ /** {@link Option#implicitRequirements()} ()} */
+ public String[] getImplicitRequirements() {
+ return optionAnnotation.implicitRequirements();
+ }
+
+ /** {@link Option#deprecationWarning()} ()} */
+ public String getDeprecationWarning() {
+ return optionAnnotation.deprecationWarning();
+ }
+
+ /** {@link Option#oldName()} ()} ()} */
+ public String getOldOptionName() {
+ return optionAnnotation.oldName();
+ }
+
+ /** {@link Option#wrapperOption()} ()} ()} */
+ public boolean isWrapperOption() {
+ return optionAnnotation.wrapperOption();
+ }
+
+ /** The type of the optionDefinition. */
+ public Class<?> getType() {
+ return field.getType();
+ }
+
+ /** Whether this field has type Void. */
+ boolean isVoidField() {
+ return getType().equals(Void.class);
+ }
+
+ public boolean isSpecialNullDefault() {
+ return getUnparsedDefaultValue().equals("null") && !getType().isPrimitive();
+ }
+
+ /** Returns whether the arg is an expansion option. */
+ public boolean isExpansionOption() {
+ return (getOptionExpansion().length > 0 || usesExpansionFunction());
+ }
+
+ /**
+ * Returns whether the arg is an expansion option defined by an expansion function (and not a
+ * constant expansion value).
+ */
+ public boolean usesExpansionFunction() {
+ return getExpansionFunction() != ExpansionFunction.class;
+ }
+
+ static final Comparator<OptionDefinition> BY_OPTION_NAME =
+ Comparator.comparing(OptionDefinition::getOptionName);
+
+ /**
+ * An ordering relation for option-field fields that first groups together options of the same
+ * category, then sorts by name within the category.
+ */
+ static final Comparator<OptionDefinition> BY_CATEGORY =
+ (left, right) -> {
+ int r = left.getOptionCategory().compareTo(right.getOptionCategory());
+ return r == 0 ? BY_OPTION_NAME.compare(left, right) : r;
+ };
+}
diff --git a/src/main/java/com/google/devtools/common/options/OptionsBase.java b/src/main/java/com/google/devtools/common/options/OptionsBase.java
index 315efe8..6b9f2f1 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsBase.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsBase.java
@@ -75,8 +75,8 @@
Map<String, Object> map = new LinkedHashMap<>();
for (Map.Entry<Field, Object> entry : OptionsParser.toMap(castClass, castThis).entrySet()) {
- String name = entry.getKey().getAnnotation(Option.class).name();
- map.put(name, entry.getValue());
+ OptionDefinition optionDefinition = OptionDefinition.extractOptionDefinition(entry.getKey());
+ map.put(optionDefinition.getOptionName(), entry.getValue());
}
return map;
}
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 c5fd9a9..eb20a9c 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsData.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsData.java
@@ -18,7 +18,6 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import java.lang.reflect.Constructor;
-import java.lang.reflect.Field;
import java.lang.reflect.Modifier;
import java.util.Collection;
import java.util.Map;
@@ -60,7 +59,7 @@
if (result == null) {
String valueString =
context.getUnparsedValue() != null ? context.getUnparsedValue() : "(null)";
- String name = context.getField().getAnnotation(Option.class).name();
+ String name = context.getOptionDefinition().getOptionName();
throw new OptionsParsingException(
"Error expanding option '"
+ name
@@ -83,10 +82,11 @@
* Mapping from each Option-annotated field with expansion information to the {@link
* ExpansionData} needed to caclulate it.
*/
- private final ImmutableMap<Field, ExpansionData> expansionDataForFields;
+ private final ImmutableMap<OptionDefinition, ExpansionData> expansionDataForFields;
/** Construct {@link OptionsData} by extending an {@link IsolatedOptionsData} with new info. */
- private OptionsData(IsolatedOptionsData base, Map<Field, ExpansionData> expansionDataForFields) {
+ private OptionsData(
+ IsolatedOptionsData base, Map<OptionDefinition, ExpansionData> expansionDataForFields) {
super(base);
this.expansionDataForFields = ImmutableMap.copyOf(expansionDataForFields);
}
@@ -99,18 +99,19 @@
* Option#expansion} or {@link Option#expansionFunction}. If the field is not an expansion option,
* returns an empty array.
*/
- public ImmutableList<String> getEvaluatedExpansion(Field field, @Nullable String unparsedValue)
+ public ImmutableList<String> getEvaluatedExpansion(
+ OptionDefinition optionDefinition, @Nullable String unparsedValue)
throws OptionsParsingException {
- ExpansionData expansionData = expansionDataForFields.get(field);
+ ExpansionData expansionData = expansionDataForFields.get(optionDefinition);
if (expansionData == null) {
return EMPTY_EXPANSION;
}
- return expansionData.getExpansion(new ExpansionContext(this, field, unparsedValue));
+ return expansionData.getExpansion(new ExpansionContext(this, optionDefinition, unparsedValue));
}
- ExpansionData getExpansionDataForField(Field field) {
- ExpansionData result = expansionDataForFields.get(field);
+ ExpansionData getExpansionDataForField(OptionDefinition optionDefinition) {
+ ExpansionData result = expansionDataForFields.get(optionDefinition);
return result != null ? result : EMPTY_EXPANSION_DATA;
}
@@ -125,19 +126,22 @@
IsolatedOptionsData isolatedData = IsolatedOptionsData.from(classes);
// All that's left is to compute expansions.
- 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);
+ ImmutableMap.Builder<OptionDefinition, ExpansionData> expansionDataBuilder =
+ ImmutableMap.<OptionDefinition, ExpansionData>builder();
+ for (Map.Entry<String, OptionDefinition> entry : isolatedData.getAllNamedFields()) {
+ OptionDefinition optionDefinition = entry.getValue();
// Determine either the hard-coded expansion, or the ExpansionFunction class.
- String[] constExpansion = annotation.expansion();
- Class<? extends ExpansionFunction> expansionFunctionClass = annotation.expansionFunction();
- if (constExpansion.length > 0 && usesExpansionFunction(annotation)) {
+ String[] constExpansion = optionDefinition.getOptionExpansion();
+ Class<? extends ExpansionFunction> expansionFunctionClass =
+ optionDefinition.getExpansionFunction();
+ if (constExpansion.length > 0 && optionDefinition.usesExpansionFunction()) {
throw new AssertionError(
- "Cannot set both expansion and expansionFunction for option --" + annotation.name());
+ "Cannot set both expansion and expansionFunction for option --"
+ + optionDefinition.getOptionName());
} else if (constExpansion.length > 0) {
- expansionDataBuilder.put(field, new ExpansionData(ImmutableList.copyOf(constExpansion)));
- } else if (usesExpansionFunction(annotation)) {
+ expansionDataBuilder.put(
+ optionDefinition, new ExpansionData(ImmutableList.copyOf(constExpansion)));
+ } else if (optionDefinition.usesExpansionFunction()) {
if (Modifier.isAbstract(expansionFunctionClass.getModifiers())) {
throw new AssertionError(
"The expansionFunction type " + expansionFunctionClass + " must be a concrete type");
@@ -155,22 +159,22 @@
ImmutableList<String> staticExpansion;
try {
- staticExpansion = instance.getExpansion(new ExpansionContext(isolatedData, field, null));
+ staticExpansion =
+ instance.getExpansion(new ExpansionContext(isolatedData, optionDefinition, null));
Preconditions.checkState(
staticExpansion != null,
"Error calling expansion function for option: %s",
- annotation.name());
- expansionDataBuilder.put(field, new ExpansionData(staticExpansion));
+ optionDefinition.getOptionName());
+ expansionDataBuilder.put(optionDefinition, new ExpansionData(staticExpansion));
} catch (ExpansionNeedsValueException e) {
// This expansion function needs data that isn't available yet. Save the instance and call
// it later.
- expansionDataBuilder.put(field, new ExpansionData(instance));
+ expansionDataBuilder.put(optionDefinition, new ExpansionData(instance));
} catch (OptionsParsingException e) {
throw new IllegalStateException("Error expanding void expansion function: ", e);
}
}
}
-
return new OptionsData(isolatedData, expansionDataBuilder.build());
}
}
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 d4779fe..d4e4305 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsParser.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsParser.java
@@ -14,8 +14,6 @@
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;
@@ -28,12 +26,15 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
+import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
+import java.util.Set;
import java.util.function.Function;
+import java.util.stream.Collectors;
import javax.annotation.Nullable;
/**
@@ -66,7 +67,7 @@
/**
* An unchecked exception thrown when there is a problem constructing a parser, e.g. an error
- * while validating an {@link Option} field in one of its {@link OptionsBase} subclasses.
+ * while validating an {@link OptionDefinition} in one of its {@link OptionsBase} subclasses.
*
* <p>This exception is unchecked because it generally indicates an internal error affecting all
* invocations of the program. I.e., any such error should be immediately obvious to the
@@ -225,9 +226,9 @@
}
}
- /**
- * The metadata about an option.
- */
+ // TODO(b/64904491) remove this once the converter and default information is in OptionDefinition
+ // and cached.
+ /** The metadata about an option. */
public static final class OptionDescription {
private final String name;
@@ -423,16 +424,21 @@
*/
public static class UnparsedOptionValueDescription {
private final String name;
- private final Field field;
+ private final OptionDefinition optionDefinition;
private final String unparsedValue;
private final OptionPriority priority;
private final String source;
private final boolean explicit;
- public UnparsedOptionValueDescription(String name, Field field, String unparsedValue,
- OptionPriority priority, String source, boolean explicit) {
+ public UnparsedOptionValueDescription(
+ String name,
+ OptionDefinition optionDefinition,
+ String unparsedValue,
+ OptionPriority priority,
+ String source,
+ boolean explicit) {
this.name = name;
- this.field = field;
+ this.optionDefinition = optionDefinition;
this.unparsedValue = unparsedValue;
this.priority = priority;
this.source = source;
@@ -443,20 +449,20 @@
return name;
}
- Field getField() {
- return field;
+ OptionDefinition getOptionDefinition() {
+ return optionDefinition;
}
public boolean isBooleanOption() {
- return field.getType().equals(boolean.class);
+ return optionDefinition.getType().equals(boolean.class);
}
private OptionDocumentationCategory documentationCategory() {
- return field.getAnnotation(Option.class).documentationCategory();
+ return optionDefinition.getDocumentationCategory();
}
private ImmutableList<OptionMetadataTag> metadataTags() {
- return ImmutableList.copyOf(field.getAnnotation(Option.class).metadataTags());
+ return ImmutableList.copyOf(optionDefinition.getOptionMetadataTags());
}
public boolean isDocumented() {
@@ -469,17 +475,15 @@
}
boolean isExpansion() {
- return OptionsData.isExpansionOption(field.getAnnotation(Option.class));
+ return optionDefinition.isExpansionOption();
}
boolean isImplicitRequirement() {
- Option option = field.getAnnotation(Option.class);
- return option.implicitRequirements().length > 0;
+ return optionDefinition.getImplicitRequirements().length > 0;
}
boolean allowMultiple() {
- Option option = field.getAnnotation(Option.class);
- return option.allowMultiple();
+ return optionDefinition.allowsMultiple();
}
public String getUnparsedValue() {
@@ -535,18 +539,18 @@
OptionsData data = impl.getOptionsData();
StringBuilder desc = new StringBuilder();
if (!data.getOptionsClasses().isEmpty()) {
- List<Field> allFields = new ArrayList<>();
+ List<OptionDefinition> allFields = new ArrayList<>();
for (Class<? extends OptionsBase> optionsClass : data.getOptionsClasses()) {
- allFields.addAll(data.getFieldsForClass(optionsClass));
+ allFields.addAll(data.getOptionDefinitionsFromClass(optionsClass));
}
- allFields.sort(OptionsUsage.BY_CATEGORY);
+ Collections.sort(allFields, OptionDefinition.BY_CATEGORY);
String prevCategory = null;
- for (Field optionField : allFields) {
- Option option = optionField.getAnnotation(Option.class);
- String category = option.category();
+ for (OptionDefinition optionDefinition : allFields) {
+ String category = optionDefinition.getOptionCategory();
if (!category.equals(prevCategory)
- && option.documentationCategory() != OptionDocumentationCategory.UNDOCUMENTED) {
+ && optionDefinition.getDocumentationCategory()
+ != OptionDocumentationCategory.UNDOCUMENTED) {
String description = categoryDescriptions.get(category);
if (description == null) {
description = "Options category '" + category + "'";
@@ -555,8 +559,9 @@
prevCategory = category;
}
- if (option.documentationCategory() != OptionDocumentationCategory.UNDOCUMENTED) {
- OptionsUsage.getUsage(optionField, desc, helpVerbosity, impl.getOptionsData());
+ if (optionDefinition.getDocumentationCategory()
+ != OptionDocumentationCategory.UNDOCUMENTED) {
+ OptionsUsage.getUsage(optionDefinition, desc, helpVerbosity, impl.getOptionsData());
}
}
}
@@ -578,18 +583,18 @@
OptionsData data = impl.getOptionsData();
StringBuilder desc = new StringBuilder();
if (!data.getOptionsClasses().isEmpty()) {
- List<Field> allFields = new ArrayList<>();
+ List<OptionDefinition> allFields = new ArrayList<>();
for (Class<? extends OptionsBase> optionsClass : data.getOptionsClasses()) {
- allFields.addAll(data.getFieldsForClass(optionsClass));
+ allFields.addAll(data.getOptionDefinitionsFromClass(optionsClass));
}
- allFields.sort(OptionsUsage.BY_CATEGORY);
+ Collections.sort(allFields, OptionDefinition.BY_CATEGORY);
String prevCategory = null;
- for (Field optionField : allFields) {
- Option option = optionField.getAnnotation(Option.class);
- String category = option.category();
+ for (OptionDefinition optionDefinition : allFields) {
+ String category = optionDefinition.getOptionCategory();
if (!category.equals(prevCategory)
- && option.documentationCategory() != OptionDocumentationCategory.UNDOCUMENTED) {
+ && optionDefinition.getDocumentationCategory()
+ != OptionDocumentationCategory.UNDOCUMENTED) {
String description = categoryDescriptions.get(category);
if (description == null) {
description = "Options category '" + category + "'";
@@ -602,8 +607,9 @@
prevCategory = category;
}
- if (option.documentationCategory() != OptionDocumentationCategory.UNDOCUMENTED) {
- OptionsUsage.getUsageHtml(optionField, desc, escaper, impl.getOptionsData());
+ if (optionDefinition.getDocumentationCategory()
+ != OptionDocumentationCategory.UNDOCUMENTED) {
+ OptionsUsage.getUsageHtml(optionDefinition, desc, escaper, impl.getOptionsData());
}
}
desc.append("</dl>\n");
@@ -613,8 +619,8 @@
/**
* Returns a string listing the possible flag completion for this command along with the command
- * completion if any. See {@link OptionsUsage#getCompletion(Field, StringBuilder)} for more
- * details on the format for the flag completion.
+ * completion if any. See {@link OptionsUsage#getCompletion(OptionDefinition, StringBuilder)} for
+ * more details on the format for the flag completion.
*/
public String getOptionsCompletion() {
OptionsData data = impl.getOptionsData();
@@ -623,14 +629,14 @@
data.getOptionsClasses()
// List all options
.stream()
- .flatMap(optionsClass -> data.getFieldsForClass(optionsClass).stream())
+ .flatMap(optionsClass -> data.getOptionDefinitionsFromClass(optionsClass).stream())
// Sort field for deterministic ordering
- .sorted(comparing(optionField -> optionField.getAnnotation(Option.class).name()))
+ .sorted(OptionDefinition.BY_OPTION_NAME)
.filter(
- optionField ->
- optionField.getAnnotation(Option.class).documentationCategory()
+ optionDefinition ->
+ optionDefinition.getDocumentationCategory()
!= OptionDocumentationCategory.UNDOCUMENTED)
- .forEach(optionField -> OptionsUsage.getCompletion(optionField, desc));
+ .forEach(optionDefinition -> OptionsUsage.getCompletion(optionDefinition, desc));
return desc.toString();
}
@@ -780,9 +786,9 @@
}
/** Returns all options fields of the given options class, in alphabetic order. */
- public static Collection<Field> getFields(Class<? extends OptionsBase> optionsClass) {
+ public static Collection<OptionDefinition> getFields(Class<? extends OptionsBase> optionsClass) {
OptionsData data = OptionsParser.getOptionsDataInternal(optionsClass);
- return data.getFieldsForClass(optionsClass);
+ return data.getOptionDefinitionsFromClass(optionsClass);
}
/**
@@ -798,21 +804,26 @@
* Returns a mapping from each option {@link Field} in {@code optionsClass} (including inherited
* ones) to its value in {@code options}.
*
- * <p>The map is a mutable copy; changing the map won't affect {@code options} and vice versa.
- * The map entries appear sorted alphabetically by option name.
+ * <p>To save space, the map directly stores {@code Fields} instead of the {@code
+ * OptionDefinitions}.
*
- * If {@code options} is an instance of a subclass of {@code optionsClass}, any options defined
- * by the subclass are not included in the map.
+ * <p>The map is a mutable copy; changing the map won't affect {@code options} and vice versa. The
+ * map entries appear sorted alphabetically by option name.
*
- * @throws IllegalArgumentException if {@code options} is not an instance of {@code optionsClass}
+ * <p>If {@code options} is an instance of a subclass of {@link OptionsBase}, any options defined
+ * by the subclass are not included in the map, only the options declared in the provided class
+ * are included.
+ *
+ * @throws IllegalArgumentException if {@code options} is not an instance of {@link OptionsBase}
*/
public static <O extends OptionsBase> Map<Field, Object> toMap(Class<O> optionsClass, O options) {
OptionsData data = getOptionsDataInternal(optionsClass);
- // Alphabetized due to getFieldsForClass()'s order.
+ // Alphabetized due to getOptionDefinitionsFromClass()'s order.
Map<Field, Object> map = new LinkedHashMap<>();
- for (Field field : data.getFieldsForClass(optionsClass)) {
+ for (OptionDefinition optionDefinition : data.getOptionDefinitionsFromClass(optionsClass)) {
try {
- map.put(field, field.get(options));
+ // Get the object value of the optionDefinition and place in map.
+ map.put(optionDefinition.getField(), optionDefinition.getField().get(options));
} catch (IllegalAccessException e) {
// All options fields of options classes should be public.
throw new IllegalStateException(e);
@@ -828,6 +839,9 @@
* Given a mapping as returned by {@link #toMap}, and the options class it that its entries
* correspond to, this constructs the corresponding instance of the options class.
*
+ * @param map Field to Object, expecting an entry for each field in the optionsClass. This
+ * directly refers to the Field, without wrapping it in an OptionDefinition, see {@link
+ * #toMap}.
* @throws IllegalArgumentException if {@code map} does not contain exactly the fields of {@code
* optionsClass}, with values of the appropriate type
*/
@@ -843,15 +857,15 @@
throw new IllegalStateException("Error while instantiating options class", e);
}
- List<Field> fields = data.getFieldsForClass(optionsClass);
+ List<OptionDefinition> optionDefinitions = data.getOptionDefinitionsFromClass(optionsClass);
// Ensure all fields are covered, no extraneous fields.
- validateFieldsSets(new LinkedHashSet<>(fields), new LinkedHashSet<>(map.keySet()));
+ validateFieldsSets(data, optionsClass, new LinkedHashSet<Field>(map.keySet()));
// Populate the instance.
- for (Field field : fields) {
+ for (OptionDefinition optionDefinition : optionDefinitions) {
// Non-null as per above check.
- Object value = map.get(field);
+ Object value = map.get(optionDefinition.getField());
try {
- field.set(optionsInstance, value);
+ optionDefinition.getField().set(optionsInstance, value);
} catch (IllegalAccessException e) {
throw new IllegalStateException(e);
}
@@ -861,41 +875,59 @@
}
/**
- * Raises a pretty {@link IllegalArgumentException} if the two sets of fields are not equal.
+ * Raises a pretty {@link IllegalArgumentException} if the provided set of fields is a complete
+ * set for the optionsClass.
*
* <p>The entries in {@code fieldsFromMap} may be ill formed by being null or lacking an {@link
- * Option} annotation. (This isn't done for {@code fieldsFromClass} because they come from an
- * {@link OptionsData} object.)
+ * Option} annotation.
*/
private static void validateFieldsSets(
- LinkedHashSet<Field> fieldsFromClass, LinkedHashSet<Field> fieldsFromMap) {
- if (!fieldsFromClass.equals(fieldsFromMap)) {
- List<String> extraNamesFromClass = new ArrayList<>();
- List<String> extraNamesFromMap = new ArrayList<>();
- for (Field field : fieldsFromClass) {
- if (!fieldsFromMap.contains(field)) {
- extraNamesFromClass.add("'" + field.getAnnotation(Option.class).name() + "'");
- }
+ OptionsData data,
+ Class<? extends OptionsBase> optionsClass,
+ LinkedHashSet<Field> fieldsFromMap) {
+ ImmutableList<OptionDefinition> optionFieldsFromClasses =
+ data.getOptionDefinitionsFromClass(optionsClass);
+ Set<Field> fieldsFromClass =
+ optionFieldsFromClasses
+ .stream()
+ .map(optionField -> optionField.getField())
+ .collect(Collectors.toSet());
+
+ if (fieldsFromClass.equals(fieldsFromMap)) {
+ // They are already equal, avoid additional checks.
+ return;
+ }
+
+ List<String> extraNamesFromClass = new ArrayList<>();
+ List<String> extraNamesFromMap = new ArrayList<>();
+ for (OptionDefinition optionDefinition : optionFieldsFromClasses) {
+ if (!fieldsFromMap.contains(optionDefinition.getField())) {
+ extraNamesFromClass.add("'" + optionDefinition.getOptionName() + "'");
}
- for (Field field : fieldsFromMap) {
- // Extra validation on the map keys since they don't come from OptionsData.
- if (!fieldsFromClass.contains(field)) {
- if (field == null) {
- extraNamesFromMap.add("<null field>");
- } else {
- Option annotation = field.getAnnotation(Option.class);
- if (annotation == null) {
- extraNamesFromMap.add("<non-Option field>");
- } else {
- extraNamesFromMap.add("'" + annotation.name() + "'");
- }
+ }
+ for (Field field : fieldsFromMap) {
+ // Extra validation on the map keys since they don't come from OptionsData.
+ if (!fieldsFromClass.contains(field)) {
+ if (field == null) {
+ extraNamesFromMap.add("<null field>");
+ } else {
+
+ OptionDefinition optionDefinition = null;
+ try {
+ optionDefinition = OptionDefinition.extractOptionDefinition(field);
+ extraNamesFromMap.add("'" + optionDefinition.getOptionName() + "'");
+ } catch (ConstructionException e) {
+ extraNamesFromMap.add("<non-Option field>");
}
}
}
- throw new IllegalArgumentException(
- "Map keys do not match fields of options class; extra map keys: {"
- + Joiner.on(", ").join(extraNamesFromMap) + "}; extra options class options: {"
- + Joiner.on(", ").join(extraNamesFromClass) + "}");
}
+ throw new IllegalArgumentException(
+ "Map keys do not match fields of options class; extra map keys: {"
+ + Joiner.on(", ").join(extraNamesFromMap)
+ + "}; extra options class options: {"
+ + Joiner.on(", ").join(extraNamesFromClass)
+ + "}");
}
}
+
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 49d1547..f8d6778 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java
@@ -28,7 +28,6 @@
import com.google.devtools.common.options.OptionsParser.OptionValueDescription;
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.HashMap;
@@ -52,13 +51,13 @@
* We store the results of parsing the arguments in here. It'll look like
*
* <pre>
- * Field("--host") -> "www.google.com"
- * Field("--port") -> 80
+ * OptionDefinition("--host") -> "www.google.com"
+ * OptionDefinition("--port") -> 80
* </pre>
*
* This map is modified by repeated calls to {@link #parse(OptionPriority,Function,List)}.
*/
- private final Map<Field, OptionValueDescription> parsedValues = new HashMap<>();
+ private final Map<OptionDefinition, OptionValueDescription> parsedValues = new HashMap<>();
/**
* We store the pre-parsed, explicit options for each priority in here.
@@ -68,16 +67,15 @@
private final List<UnparsedOptionValueDescription> unparsedValues = new ArrayList<>();
/**
- * Unparsed values for use with the canonicalize command are stored separately from
- * unparsedValues so that invocation policy can modify the values for canonicalization (e.g.
- * override user-specified values with default values) without corrupting the data used to
- * represent the user's original invocation for {@link #asListOfExplicitOptions()} and
- * {@link #asListOfUnparsedOptions()}. A LinkedHashMultimap is used so that canonicalization
- * happens in the correct order and multiple values can be stored for flags that allow multiple
- * values.
+ * Unparsed values for use with the canonicalize command are stored separately from unparsedValues
+ * so that invocation policy can modify the values for canonicalization (e.g. override
+ * user-specified values with default values) without corrupting the data used to represent the
+ * user's original invocation for {@link #asListOfExplicitOptions()} and {@link
+ * #asListOfUnparsedOptions()}. A LinkedHashMultimap is used so that canonicalization happens in
+ * the correct order and multiple values can be stored for flags that allow multiple values.
*/
- private final Multimap<Field, UnparsedOptionValueDescription> canonicalizeValues
- = LinkedHashMultimap.create();
+ private final Multimap<OptionDefinition, UnparsedOptionValueDescription> canonicalizeValues =
+ LinkedHashMultimap.create();
private final List<String> warnings = new ArrayList<>();
@@ -170,12 +168,12 @@
*/
List<OptionValueDescription> asListOfEffectiveOptions() {
List<OptionValueDescription> result = new ArrayList<>();
- for (Map.Entry<String, Field> mapEntry : optionsData.getAllNamedFields()) {
+ for (Map.Entry<String, OptionDefinition> mapEntry : optionsData.getAllNamedFields()) {
String fieldName = mapEntry.getKey();
- Field field = mapEntry.getValue();
- OptionValueDescription entry = parsedValues.get(field);
+ OptionDefinition optionDefinition = mapEntry.getValue();
+ OptionValueDescription entry = parsedValues.get(optionDefinition);
if (entry == null) {
- Object value = optionsData.getDefaultValue(field);
+ Object value = optionsData.getDefaultValue(optionDefinition);
result.add(
new OptionValueDescription(
fieldName,
@@ -193,12 +191,11 @@
return result;
}
- private void maybeAddDeprecationWarning(Field field) {
- Option option = field.getAnnotation(Option.class);
+ private void maybeAddDeprecationWarning(OptionDefinition optionDefinition) {
// Continue to support the old behavior for @Deprecated options.
- String warning = option.deprecationWarning();
- if (!warning.isEmpty() || (field.getAnnotation(Deprecated.class) != null)) {
- addDeprecationWarning(option.name(), warning);
+ String warning = optionDefinition.getDeprecationWarning();
+ if (!warning.isEmpty() || (optionDefinition.getField().isAnnotationPresent(Deprecated.class))) {
+ addDeprecationWarning(optionDefinition.getOptionName(), warning);
}
}
@@ -208,9 +205,15 @@
}
// Warnings should not end with a '.' because the internal reporter adds one automatically.
- private void setValue(Field field, String name, Object value,
- OptionPriority priority, String source, String implicitDependant, String expandedFrom) {
- OptionValueDescription entry = parsedValues.get(field);
+ private void setValue(
+ OptionDefinition optionDefinition,
+ String name,
+ Object value,
+ OptionPriority priority,
+ String source,
+ String implicitDependant,
+ String expandedFrom) {
+ OptionValueDescription entry = parsedValues.get(optionDefinition);
if (entry != null) {
// Override existing option if the new value has higher or equal priority.
if (priority.compareTo(entry.getPriority()) >= 0) {
@@ -258,22 +261,28 @@
// Record the new value:
parsedValues.put(
- field,
+ optionDefinition,
new OptionValueDescription(
name, null, value, priority, source, implicitDependant, expandedFrom, false));
}
} else {
parsedValues.put(
- field,
+ optionDefinition,
new OptionValueDescription(
name, null, value, priority, source, implicitDependant, expandedFrom, false));
- maybeAddDeprecationWarning(field);
+ maybeAddDeprecationWarning(optionDefinition);
}
}
- private void addListValue(Field field, String originalName, Object value, OptionPriority priority,
- String source, String implicitDependant, String expandedFrom) {
- OptionValueDescription entry = parsedValues.get(field);
+ private void addListValue(
+ OptionDefinition optionDefinition,
+ String originalName,
+ Object value,
+ OptionPriority priority,
+ String source,
+ String implicitDependant,
+ String expandedFrom) {
+ OptionValueDescription entry = parsedValues.get(optionDefinition);
if (entry == null) {
entry =
new OptionValueDescription(
@@ -285,47 +294,46 @@
implicitDependant,
expandedFrom,
true);
- parsedValues.put(field, entry);
- maybeAddDeprecationWarning(field);
+ parsedValues.put(optionDefinition, entry);
+ maybeAddDeprecationWarning(optionDefinition);
}
entry.addValue(priority, value);
}
OptionValueDescription clearValue(String optionName)
throws OptionsParsingException {
- Field field = optionsData.getFieldFromName(optionName);
- if (field == null) {
+ OptionDefinition optionDefinition = optionsData.getFieldFromName(optionName);
+ if (optionDefinition == null) {
throw new IllegalArgumentException("No such option '" + optionName + "'");
}
// Actually remove the value from various lists tracking effective options.
- canonicalizeValues.removeAll(field);
- return parsedValues.remove(field);
+ canonicalizeValues.removeAll(optionDefinition);
+ return parsedValues.remove(optionDefinition);
}
OptionValueDescription getOptionValueDescription(String name) {
- Field field = optionsData.getFieldFromName(name);
- if (field == null) {
+ OptionDefinition optionDefinition = optionsData.getFieldFromName(name);
+ if (optionDefinition == null) {
throw new IllegalArgumentException("No such option '" + name + "'");
}
- return parsedValues.get(field);
+ return parsedValues.get(optionDefinition);
}
OptionDescription getOptionDescription(String name) throws OptionsParsingException {
- Field field = optionsData.getFieldFromName(name);
- if (field == null) {
+ OptionDefinition optionDefinition = optionsData.getFieldFromName(name);
+ if (optionDefinition == null) {
return null;
}
- Option optionAnnotation = field.getAnnotation(Option.class);
return new OptionDescription(
name,
- optionsData.getDefaultValue(field),
- optionsData.getConverter(field),
- optionsData.getAllowMultiple(field),
- optionsData.getExpansionDataForField(field),
+ optionsData.getDefaultValue(optionDefinition),
+ optionsData.getConverter(optionDefinition),
+ optionsData.getAllowMultiple(optionDefinition),
+ optionsData.getExpansionDataForField(optionDefinition),
getImplicitDependantDescriptions(
- ImmutableList.copyOf(optionAnnotation.implicitRequirements()), name));
+ ImmutableList.copyOf(optionDefinition.getImplicitRequirements()), name));
}
/**
@@ -344,14 +352,14 @@
ParseOptionResult parseResult = parseOption(unparsedFlagExpression, optionsIterator);
builder.add(
new OptionValueDescription(
- parseResult.option.name(),
+ parseResult.optionDefinition.getOptionName(),
parseResult.value,
/* value */ null,
/* priority */ null,
/* source */ null,
implicitDependant,
/* expendedFrom */ null,
- optionsData.getAllowMultiple(parseResult.field)));
+ optionsData.getAllowMultiple(parseResult.optionDefinition)));
}
return builder.build();
}
@@ -365,9 +373,9 @@
ImmutableList<OptionValueDescription> getExpansionOptionValueDescriptions(
String flagName, @Nullable String flagValue) throws OptionsParsingException {
ImmutableList.Builder<OptionValueDescription> builder = ImmutableList.builder();
- Field field = optionsData.getFieldFromName(flagName);
+ OptionDefinition optionDefinition = optionsData.getFieldFromName(flagName);
- ImmutableList<String> options = optionsData.getEvaluatedExpansion(field, flagValue);
+ ImmutableList<String> options = optionsData.getEvaluatedExpansion(optionDefinition, flagValue);
Iterator<String> optionsIterator = options.iterator();
while (optionsIterator.hasNext()) {
@@ -375,24 +383,24 @@
ParseOptionResult parseResult = parseOption(unparsedFlagExpression, optionsIterator);
builder.add(
new OptionValueDescription(
- parseResult.option.name(),
+ parseResult.optionDefinition.getOptionName(),
parseResult.value,
/* value */ null,
/* priority */ null,
/* source */ null,
/* implicitDependant */ null,
flagName,
- optionsData.getAllowMultiple(parseResult.field)));
+ optionsData.getAllowMultiple(parseResult.optionDefinition)));
}
return builder.build();
}
boolean containsExplicitOption(String name) {
- Field field = optionsData.getFieldFromName(name);
- if (field == null) {
+ OptionDefinition optionDefinition = optionsData.getFieldFromName(name);
+ if (optionDefinition == null) {
throw new IllegalArgumentException("No such option '" + name + "'");
}
- return parsedValues.get(field) != null;
+ return parsedValues.get(optionDefinition) != null;
}
/**
@@ -440,13 +448,12 @@
}
ParseOptionResult parseOptionResult = parseOption(arg, argsIterator);
- Field field = parseOptionResult.field;
- Option option = parseOptionResult.option;
+ OptionDefinition optionDefinition = parseOptionResult.optionDefinition;
@Nullable String value = parseOptionResult.value;
- final String originalName = option.name();
+ final String originalName = optionDefinition.getOptionName();
- if (option.wrapperOption()) {
+ if (optionDefinition.isWrapperOption()) {
if (value.startsWith("-")) {
String sourceMessage = "Unwrapped from wrapper option --" + originalName;
List<String> unparsed =
@@ -483,29 +490,31 @@
UnparsedOptionValueDescription unparsedOptionValueDescription =
new UnparsedOptionValueDescription(
originalName,
- field,
+ optionDefinition,
value,
priority,
sourceFunction.apply(originalName),
expandedFrom == null);
unparsedValues.add(unparsedOptionValueDescription);
- if (option.allowMultiple()) {
- canonicalizeValues.put(field, unparsedOptionValueDescription);
+ if (optionDefinition.allowsMultiple()) {
+ canonicalizeValues.put(optionDefinition, unparsedOptionValueDescription);
} else {
- canonicalizeValues.replaceValues(field, ImmutableList.of(unparsedOptionValueDescription));
+ canonicalizeValues.replaceValues(
+ optionDefinition, ImmutableList.of(unparsedOptionValueDescription));
}
}
// Handle expansion options.
- if (OptionsData.isExpansionOption(field.getAnnotation(Option.class))) {
- ImmutableList<String> expansion = optionsData.getEvaluatedExpansion(field, value);
+ if (optionDefinition.isExpansionOption()) {
+ ImmutableList<String> expansion =
+ optionsData.getEvaluatedExpansion(optionDefinition, value);
String sourceMessage = "expanded from option --"
+ originalName
+ " from "
+ sourceFunction.apply(originalName);
Function<Object, String> expansionSourceFunction = o -> sourceMessage;
- maybeAddDeprecationWarning(field);
+ maybeAddDeprecationWarning(optionDefinition);
List<String> unparsed =
parse(priority, expansionSourceFunction, null, originalName, expansion);
if (!unparsed.isEmpty()) {
@@ -518,7 +527,7 @@
+ Joiner.on(' ').join(unparsed));
}
} else {
- Converter<?> converter = optionsData.getConverter(field);
+ Converter<?> converter = optionsData.getConverter(optionDefinition);
Object convertedValue;
try {
convertedValue = converter.convert(value);
@@ -531,23 +540,37 @@
// ...but allow duplicates of single-use options across separate calls to
// parse(); latest wins:
- if (!option.allowMultiple()) {
- setValue(field, originalName, convertedValue,
- priority, sourceFunction.apply(originalName), implicitDependent, expandedFrom);
+ if (!optionDefinition.allowsMultiple()) {
+ setValue(
+ optionDefinition,
+ originalName,
+ convertedValue,
+ priority,
+ sourceFunction.apply(originalName),
+ implicitDependent,
+ expandedFrom);
} else {
// But if it's a multiple-use option, then just accumulate the
// values, in the order in which they were seen.
// Note: The type of the list member is not known; Java introspection
// only makes it available in String form via the signature string
// for the field declaration.
- addListValue(field, originalName, convertedValue, priority,
- sourceFunction.apply(originalName), implicitDependent, expandedFrom);
+ addListValue(
+ optionDefinition,
+ originalName,
+ convertedValue,
+ priority,
+ sourceFunction.apply(originalName),
+ implicitDependent,
+ expandedFrom);
}
}
// Collect any implicit requirements.
- if (option.implicitRequirements().length > 0) {
- implicitRequirements.put(option.name(), Arrays.asList(option.implicitRequirements()));
+ if (optionDefinition.getImplicitRequirements().length > 0) {
+ implicitRequirements.put(
+ optionDefinition.getOptionName(),
+ Arrays.asList(optionDefinition.getImplicitRequirements()));
}
}
@@ -578,13 +601,11 @@
}
private static final class ParseOptionResult {
- final Field field;
- final Option option;
+ final OptionDefinition optionDefinition;
@Nullable final String value;
- ParseOptionResult(Field field, Option option, @Nullable String value) {
- this.field = field;
- this.option = option;
+ ParseOptionResult(OptionDefinition optionDefinition, @Nullable String value) {
+ this.optionDefinition = optionDefinition;
this.value = value;
}
}
@@ -593,15 +614,15 @@
throws OptionsParsingException {
String value = null;
- Field field;
+ OptionDefinition optionDefinition;
boolean booleanValue = true;
if (arg.length() == 2) { // -l (may be nullary or unary)
- field = optionsData.getFieldForAbbrev(arg.charAt(1));
+ optionDefinition = optionsData.getFieldForAbbrev(arg.charAt(1));
booleanValue = true;
} else if (arg.length() == 3 && arg.charAt(2) == '-') { // -l- (boolean)
- field = optionsData.getFieldForAbbrev(arg.charAt(1));
+ optionDefinition = optionsData.getFieldForAbbrev(arg.charAt(1));
booleanValue = false;
} else if (allowSingleDashLongOptions // -long_option
@@ -615,16 +636,16 @@
throw new OptionsParsingException("Invalid options syntax: " + arg, arg);
}
value = equalsAt == -1 ? null : arg.substring(equalsAt + 1);
- field = optionsData.getFieldFromName(name);
+ optionDefinition = optionsData.getFieldFromName(name);
// Look for a "no"-prefixed option name: "no<optionName>".
- if (field == null && name.startsWith("no")) {
+ if (optionDefinition == null && name.startsWith("no")) {
name = name.substring(2);
- field = optionsData.getFieldFromName(name);
+ optionDefinition = optionsData.getFieldFromName(name);
booleanValue = false;
- if (field != null) {
+ if (optionDefinition != null) {
// TODO(bazel-team): Add tests for these cases.
- if (!optionsData.isBooleanField(field)) {
+ if (!optionsData.isBooleanField(optionDefinition)) {
throw new OptionsParsingException(
"Illegal use of 'no' prefix on non-boolean option: " + arg, arg);
}
@@ -640,19 +661,19 @@
throw new OptionsParsingException("Invalid options syntax: " + arg, arg);
}
- Option option = field == null ? null : field.getAnnotation(Option.class);
-
- if (option == null
- || ImmutableList.copyOf(option.metadataTags()).contains(OptionMetadataTag.INTERNAL)) {
- // This also covers internal options, which are treated as if they did not exist.
+ if (optionDefinition == null
+ || ImmutableList.copyOf(optionDefinition.getOptionMetadataTags())
+ .contains(OptionMetadataTag.INTERNAL)) {
+ // Do not recognize internal options, which are treated as if they did not exist.
throw new OptionsParsingException("Unrecognized option: " + arg, arg);
}
if (value == null) {
// Special-case boolean to supply value based on presence of "no" prefix.
- if (optionsData.isBooleanField(field)) {
+ if (optionsData.isBooleanField(optionDefinition)) {
value = booleanValue ? "1" : "0";
- } else if (field.getType().equals(Void.class) && !option.wrapperOption()) {
+ } else if (optionDefinition.getType().equals(Void.class)
+ && !optionDefinition.isWrapperOption()) {
// This is expected, Void type options have no args (unless they're wrapper options).
} else if (nextArgs.hasNext()) {
value = nextArgs.next(); // "--flag value" form
@@ -661,7 +682,7 @@
}
}
- return new ParseOptionResult(field, option, value);
+ return new ParseOptionResult(optionDefinition, value);
}
/**
@@ -681,16 +702,17 @@
}
// Set the fields
- for (Field field : optionsData.getFieldsForClass(optionsClass)) {
+ for (OptionDefinition optionDefinition :
+ optionsData.getOptionDefinitionsFromClass(optionsClass)) {
Object value;
- OptionValueDescription entry = parsedValues.get(field);
+ OptionValueDescription entry = parsedValues.get(optionDefinition);
if (entry == null) {
- value = optionsData.getDefaultValue(field);
+ value = optionsData.getDefaultValue(optionDefinition);
} else {
value = entry.getValue();
}
try {
- field.set(optionsInstance, value);
+ optionDefinition.getField().set(optionsInstance, value);
} catch (IllegalAccessException e) {
throw new IllegalStateException(e);
}
@@ -701,13 +723,4 @@
List<String> getWarnings() {
return ImmutableList.copyOf(warnings);
}
-
- static String getDefaultOptionString(Field optionField) {
- Option annotation = optionField.getAnnotation(Option.class);
- return annotation.defaultValue();
- }
-
- static boolean isSpecialNullDefault(String defaultValueString, Field optionField) {
- return defaultValueString.equals("null") && !optionField.getType().isPrimitive();
- }
}
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 8e36c4a..51cc3c6 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsUsage.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsUsage.java
@@ -19,10 +19,8 @@
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.escape.Escaper;
-import java.lang.reflect.Field;
import java.text.BreakIterator;
import java.util.ArrayList;
-import java.util.Comparator;
import java.util.List;
import javax.annotation.Nullable;
@@ -40,10 +38,11 @@
*/
static void getUsage(Class<? extends OptionsBase> optionsClass, StringBuilder usage) {
OptionsData data = OptionsParser.getOptionsDataInternal(optionsClass);
- List<Field> optionFields = new ArrayList<>(data.getFieldsForClass(optionsClass));
- optionFields.sort(BY_NAME);
- for (Field optionField : optionFields) {
- getUsage(optionField, usage, OptionsParser.HelpVerbosity.LONG, data);
+ List<OptionDefinition> optionDefinitions =
+ new ArrayList<>(data.getOptionDefinitionsFromClass(optionsClass));
+ optionDefinitions.sort(OptionDefinition.BY_OPTION_NAME);
+ for (OptionDefinition optionDefinition : optionDefinitions) {
+ getUsage(optionDefinition, usage, OptionsParser.HelpVerbosity.LONG, data);
}
}
@@ -84,11 +83,10 @@
* or is statically declared in the annotation.
*/
private static @Nullable ImmutableList<String> getExpansionIfKnown(
- Field optionField, OptionsData optionsData) {
- Preconditions.checkNotNull(optionField);
- Preconditions.checkNotNull(optionsData);
+ OptionDefinition optionDefinition, OptionsData optionsData) {
+ Preconditions.checkNotNull(optionDefinition);
try {
- return optionsData.getEvaluatedExpansion(optionField, null);
+ return optionsData.getEvaluatedExpansion(optionDefinition, null);
} catch (ExpansionNeedsValueException e) {
return null;
} catch (OptionsParsingException e) {
@@ -99,29 +97,28 @@
/** Appends the usage message for a single option-field message to 'usage'. */
static void getUsage(
- Field optionField,
+ OptionDefinition optionDefinition,
StringBuilder usage,
OptionsParser.HelpVerbosity helpVerbosity,
OptionsData optionsData) {
- String flagName = getFlagName(optionField, optionsData);
- String typeDescription = getTypeDescription(optionField, optionsData);
- Option annotation = optionField.getAnnotation(Option.class);
+ String flagName = getFlagName(optionDefinition, optionsData);
+ String typeDescription = getTypeDescription(optionDefinition, optionsData);
usage.append(" --").append(flagName);
if (helpVerbosity == OptionsParser.HelpVerbosity.SHORT) { // just the name
usage.append('\n');
return;
}
- if (annotation.abbrev() != '\0') {
- usage.append(" [-").append(annotation.abbrev()).append(']');
+ if (optionDefinition.getAbbreviation() != '\0') {
+ usage.append(" [-").append(optionDefinition.getAbbreviation()).append(']');
}
if (!typeDescription.equals("")) {
usage.append(" (").append(typeDescription).append("; ");
- if (annotation.allowMultiple()) {
+ if (optionDefinition.allowsMultiple()) {
usage.append("may be used multiple times");
} else {
// Don't call the annotation directly (we must allow overrides to certain defaults)
- String defaultValueString = OptionsParserImpl.getDefaultOptionString(optionField);
- if (OptionsParserImpl.isSpecialNullDefault(defaultValueString, optionField)) {
+ String defaultValueString = optionDefinition.getUnparsedDefaultValue();
+ if (optionDefinition.isSpecialNullDefault()) {
usage.append("default: see description");
} else {
usage.append("default: \"").append(defaultValueString).append("\"");
@@ -133,11 +130,11 @@
if (helpVerbosity == OptionsParser.HelpVerbosity.MEDIUM) { // just the name and type.
return;
}
- if (!annotation.help().equals("")) {
- usage.append(paragraphFill(annotation.help(), /*indent=*/ 4, /*width=*/ 80));
+ if (!optionDefinition.getHelpText().isEmpty()) {
+ usage.append(paragraphFill(optionDefinition.getHelpText(), /*indent=*/ 4, /*width=*/ 80));
usage.append('\n');
}
- ImmutableList<String> expansion = getExpansionIfKnown(optionField, optionsData);
+ ImmutableList<String> expansion = getExpansionIfKnown(optionDefinition, optionsData);
if (expansion == null) {
usage.append(paragraphFill("Expands to unknown options.", /*indent=*/ 6, /*width=*/ 80));
usage.append('\n');
@@ -149,9 +146,9 @@
usage.append(paragraphFill(expandsMsg.toString(), /*indent=*/ 6, /*width=*/ 80));
usage.append('\n');
}
- if (annotation.implicitRequirements().length > 0) {
+ if (optionDefinition.getImplicitRequirements().length > 0) {
StringBuilder requiredMsg = new StringBuilder("Using this option will also add: ");
- for (String req : annotation.implicitRequirements()) {
+ for (String req : optionDefinition.getImplicitRequirements()) {
requiredMsg.append(req).append(" ");
}
usage.append(paragraphFill(requiredMsg.toString(), /*indent=*/ 6, /*width=*/ 80));
@@ -161,15 +158,17 @@
/** Append the usage message for a single option-field message to 'usage'. */
static void getUsageHtml(
- Field optionField, StringBuilder usage, Escaper escaper, OptionsData optionsData) {
- Option annotation = optionField.getAnnotation(Option.class);
- String plainFlagName = annotation.name();
- String flagName = getFlagName(optionField, optionsData);
- String valueDescription = annotation.valueHelp();
- String typeDescription = getTypeDescription(optionField, optionsData);
+ OptionDefinition optionDefinition,
+ StringBuilder usage,
+ Escaper escaper,
+ OptionsData optionsData) {
+ String plainFlagName = optionDefinition.getOptionName();
+ String flagName = getFlagName(optionDefinition, optionsData);
+ String valueDescription = optionDefinition.getValueTypeHelpText();
+ String typeDescription = getTypeDescription(optionDefinition, optionsData);
usage.append("<dt><code><a name=\"flag--").append(plainFlagName).append("\"></a>--");
usage.append(flagName);
- if (optionsData.isBooleanField(optionField) || OptionsData.isVoidField(optionField)) {
+ if (optionsData.isBooleanField(optionDefinition) || optionDefinition.isVoidField()) {
// Nothing for boolean, tristate, boolean_or_enum, or void options.
} else if (!valueDescription.isEmpty()) {
usage.append("=").append(escaper.escape(valueDescription));
@@ -178,18 +177,18 @@
usage.append("=<").append(escaper.escape(typeDescription)).append(">");
}
usage.append("</code>");
- if (annotation.abbrev() != '\0') {
- usage.append(" [<code>-").append(annotation.abbrev()).append("</code>]");
+ if (optionDefinition.getAbbreviation() != '\0') {
+ usage.append(" [<code>-").append(optionDefinition.getAbbreviation()).append("</code>]");
}
- if (annotation.allowMultiple()) {
+ if (optionDefinition.allowsMultiple()) {
// Allow-multiple options can't have a default value.
usage.append(" multiple uses are accumulated");
} else {
// Don't call the annotation directly (we must allow overrides to certain defaults).
- String defaultValueString = OptionsParserImpl.getDefaultOptionString(optionField);
- if (OptionsData.isVoidField(optionField)) {
+ String defaultValueString = optionDefinition.getUnparsedDefaultValue();
+ if (optionDefinition.isVoidField()) {
// Void options don't have a default.
- } else if (OptionsParserImpl.isSpecialNullDefault(defaultValueString, optionField)) {
+ } else if (optionDefinition.isSpecialNullDefault()) {
usage.append(" default: see description");
} else {
usage.append(" default: \"").append(escaper.escape(defaultValueString)).append("\"");
@@ -197,16 +196,18 @@
}
usage.append("</dt>\n");
usage.append("<dd>\n");
- if (!annotation.help().isEmpty()) {
- usage.append(paragraphFill(escaper.escape(annotation.help()), /*indent=*/ 0, /*width=*/ 80));
+ if (!optionDefinition.getHelpText().isEmpty()) {
+ usage.append(
+ paragraphFill(
+ escaper.escape(optionDefinition.getHelpText()), /*indent=*/ 0, /*width=*/ 80));
usage.append('\n');
}
- if (!optionsData.getExpansionDataForField(optionField).isEmpty()) {
+ if (!optionsData.getExpansionDataForField(optionDefinition).isEmpty()) {
// If this is an expansion option, list the expansion if known, or at least specify that we
// don't know.
usage.append("<br/>\n");
- ImmutableList<String> expansion = getExpansionIfKnown(optionField, optionsData);
+ ImmutableList<String> expansion = getExpansionIfKnown(optionDefinition, optionsData);
StringBuilder expandsMsg;
if (expansion == null) {
expandsMsg = new StringBuilder("Expands to unknown options.<br/>\n");
@@ -247,13 +248,13 @@
* --void_flag
* </pre>
*
- * @param field The field to return completion for
+ * @param optionDefinition The field to return completion for
* @param builder the string builder to store the completion values
*/
- static void getCompletion(Field field, StringBuilder builder) {
+ static void getCompletion(OptionDefinition optionDefinition, StringBuilder builder) {
// Return the list of possible completions for this option
- String flagName = field.getAnnotation(Option.class).name();
- Class<?> fieldType = field.getType();
+ String flagName = optionDefinition.getOptionName();
+ Class<?> fieldType = optionDefinition.getType();
builder.append("--").append(flagName);
if (fieldType.equals(boolean.class)) {
builder.append("\n");
@@ -278,34 +279,12 @@
}
}
- // TODO(brandjon): Should this use sorting by option name instead of field name?
- private static final Comparator<Field> BY_NAME = new Comparator<Field>() {
- @Override
- public int compare(Field left, Field right) {
- return left.getName().compareTo(right.getName());
- }
- };
-
- /**
- * An ordering relation for option-field fields that first groups together
- * options of the same category, then sorts by name within the category.
- */
- static final Comparator<Field> BY_CATEGORY = new Comparator<Field>() {
- @Override
- public int compare(Field left, Field right) {
- int r = left.getAnnotation(Option.class).category().compareTo(
- right.getAnnotation(Option.class).category());
- return r == 0 ? BY_NAME.compare(left, right) : r;
- }
- };
-
- private static String getTypeDescription(Field optionsField, OptionsData optionsData) {
+ private static String getTypeDescription(OptionDefinition optionsField, OptionsData optionsData) {
return optionsData.getConverter(optionsField).getTypeDescription();
}
- static String getFlagName(Field field, OptionsData optionsData) {
- String name = field.getAnnotation(Option.class).name();
- return optionsData.isBooleanField(field) ? "[no]" + name : name;
+ static String getFlagName(OptionDefinition optionDefinition, OptionsData optionsData) {
+ String name = optionDefinition.getOptionName();
+ return optionsData.isBooleanField(optionDefinition) ? "[no]" + name : name;
}
-
}
diff --git a/src/test/java/com/google/devtools/common/options/OptionsDataTest.java b/src/test/java/com/google/devtools/common/options/OptionsDataTest.java
index 5090fab..10e5a52 100644
--- a/src/test/java/com/google/devtools/common/options/OptionsDataTest.java
+++ b/src/test/java/com/google/devtools/common/options/OptionsDataTest.java
@@ -19,7 +19,6 @@
import com.google.common.collect.ImmutableList;
import com.google.devtools.common.options.OptionsParser.ConstructionException;
-import java.lang.reflect.Field;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
@@ -431,17 +430,17 @@
EndOfAlphabetOptions.class,
ReverseOrderedOptions.class);
ArrayList<String> names = new ArrayList<>();
- for (Map.Entry<String, Field> entry : data.getAllNamedFields()) {
+ for (Map.Entry<String, OptionDefinition> entry : data.getAllNamedFields()) {
names.add(entry.getKey());
}
assertThat(names).containsExactly(
"bar", "baz", "foo", "qux", "X", "Y", "A", "B", "C").inOrder();
}
- private List<String> getOptionNames(Iterable<Field> fields) {
+ private List<String> getOptionNames(Iterable<OptionDefinition> fields) {
ArrayList<String> result = new ArrayList<>();
- for (Field field : fields) {
- result.add(field.getAnnotation(Option.class).name());
+ for (OptionDefinition optionDefinition : fields) {
+ result.add(optionDefinition.getOptionName());
}
return result;
}
@@ -452,12 +451,15 @@
FieldNamesDifferOptions.class,
EndOfAlphabetOptions.class,
ReverseOrderedOptions.class);
- assertThat(getOptionNames(data.getFieldsForClass(FieldNamesDifferOptions.class)))
- .containsExactly("bar", "baz", "foo", "qux").inOrder();
- assertThat(getOptionNames(data.getFieldsForClass(EndOfAlphabetOptions.class)))
- .containsExactly("X", "Y").inOrder();
- assertThat(getOptionNames(data.getFieldsForClass(ReverseOrderedOptions.class)))
- .containsExactly("A", "B", "C").inOrder();
+ assertThat(getOptionNames(data.getOptionDefinitionsFromClass(FieldNamesDifferOptions.class)))
+ .containsExactly("bar", "baz", "foo", "qux")
+ .inOrder();
+ assertThat(getOptionNames(data.getOptionDefinitionsFromClass(EndOfAlphabetOptions.class)))
+ .containsExactly("X", "Y")
+ .inOrder();
+ assertThat(getOptionNames(data.getOptionDefinitionsFromClass(ReverseOrderedOptions.class)))
+ .containsExactly("A", "B", "C")
+ .inOrder();
}
/** Dummy options class. */
diff --git a/src/test/java/com/google/devtools/common/options/OptionsMapConversionTest.java b/src/test/java/com/google/devtools/common/options/OptionsMapConversionTest.java
index bed4886..30e115b 100644
--- a/src/test/java/com/google/devtools/common/options/OptionsMapConversionTest.java
+++ b/src/test/java/com/google/devtools/common/options/OptionsMapConversionTest.java
@@ -32,8 +32,8 @@
private static Map<String, Object> keysToStrings(Map<Field, Object> map) {
Map<String, Object> result = new LinkedHashMap<>();
for (Map.Entry<Field, Object> entry : map.entrySet()) {
- String name = entry.getKey().getAnnotation(Option.class).name();
- result.put(name, entry.getValue());
+ OptionDefinition optionDefinition = OptionDefinition.extractOptionDefinition(entry.getKey());
+ result.put(optionDefinition.getOptionName(), entry.getValue());
}
return result;
}
@@ -43,8 +43,8 @@
OptionsData data = OptionsParser.getOptionsDataInternal(optionsClass);
Map<Field, Object> result = new LinkedHashMap<>();
for (Map.Entry<String, Object> entry : map.entrySet()) {
- Field field = data.getFieldFromName(entry.getKey());
- result.put(field, entry.getValue());
+ OptionDefinition optionDefinition = data.getFieldFromName(entry.getKey());
+ result.put(optionDefinition.getField(), entry.getValue());
}
return result;
}
