Refactor options converter logic
Moved default converters from parser implementation to Converters. Moved other helpers to OptionsData. Also factored out new function getFieldSingularType.
--
PiperOrigin-RevId: 150473455
MOS_MIGRATED_REVID=150473455
GitOrigin-RevId: 097e64c412c6a4162a22880fd435ef4632878406
Change-Id: Ife5702b6f39415a7df3fd8b44c1867145a6ac466
diff --git a/java/com/google/devtools/common/options/Converters.java b/java/com/google/devtools/common/options/Converters.java
index c8b4d47..0d19029 100644
--- a/java/com/google/devtools/common/options/Converters.java
+++ b/java/com/google/devtools/common/options/Converters.java
@@ -16,7 +16,6 @@
import com.google.common.base.Splitter;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Maps;
-
import java.util.Iterator;
import java.util.List;
import java.util.Map;
@@ -30,6 +29,171 @@
*/
public final class Converters {
+ /** Standard converter for booleans. Accepts common shorthands/synonyms. */
+ public static class BooleanConverter implements Converter<Boolean> {
+ @Override
+ public Boolean convert(String input) throws OptionsParsingException {
+ if (input == null) {
+ return false;
+ }
+ input = input.toLowerCase();
+ if (input.equals("true")
+ || input.equals("1")
+ || input.equals("yes")
+ || input.equals("t")
+ || input.equals("y")) {
+ return true;
+ }
+ if (input.equals("false")
+ || input.equals("0")
+ || input.equals("no")
+ || input.equals("f")
+ || input.equals("n")) {
+ return false;
+ }
+ throw new OptionsParsingException("'" + input + "' is not a boolean");
+ }
+
+ @Override
+ public String getTypeDescription() {
+ return "a boolean";
+ }
+ }
+
+ /** Standard converter for Strings. */
+ public static class StringConverter implements Converter<String> {
+ @Override
+ public String convert(String input) {
+ return input;
+ }
+
+ @Override
+ public String getTypeDescription() {
+ return "a string";
+ }
+ }
+
+ /** Standard converter for integers. */
+ public static class IntegerConverter implements Converter<Integer> {
+ @Override
+ public Integer convert(String input) throws OptionsParsingException {
+ try {
+ return Integer.decode(input);
+ } catch (NumberFormatException e) {
+ throw new OptionsParsingException("'" + input + "' is not an int");
+ }
+ }
+
+ @Override
+ public String getTypeDescription() {
+ return "an integer";
+ }
+ }
+
+ /** Standard converter for longs. */
+ public static class LongConverter implements Converter<Long> {
+ @Override
+ public Long convert(String input) throws OptionsParsingException {
+ try {
+ return Long.decode(input);
+ } catch (NumberFormatException e) {
+ throw new OptionsParsingException("'" + input + "' is not a long");
+ }
+ }
+
+ @Override
+ public String getTypeDescription() {
+ return "a long integer";
+ }
+ }
+
+ /** Standard converter for doubles. */
+ public static class DoubleConverter implements Converter<Double> {
+ @Override
+ public Double convert(String input) throws OptionsParsingException {
+ try {
+ return Double.parseDouble(input);
+ } catch (NumberFormatException e) {
+ throw new OptionsParsingException("'" + input + "' is not a double");
+ }
+ }
+
+ @Override
+ public String getTypeDescription() {
+ return "a double";
+ }
+ }
+
+ /** Standard converter for TriState values. */
+ public static class TriStateConverter implements Converter<TriState> {
+ @Override
+ public TriState convert(String input) throws OptionsParsingException {
+ if (input == null) {
+ return TriState.AUTO;
+ }
+ input = input.toLowerCase();
+ if (input.equals("auto")) {
+ return TriState.AUTO;
+ }
+ if (input.equals("true")
+ || input.equals("1")
+ || input.equals("yes")
+ || input.equals("t")
+ || input.equals("y")) {
+ return TriState.YES;
+ }
+ if (input.equals("false")
+ || input.equals("0")
+ || input.equals("no")
+ || input.equals("f")
+ || input.equals("n")) {
+ return TriState.NO;
+ }
+ throw new OptionsParsingException("'" + input + "' is not a boolean");
+ }
+
+ @Override
+ public String getTypeDescription() {
+ return "a tri-state (auto, yes, no)";
+ }
+ }
+
+ /**
+ * Standard "converter" for Void. Should not actually be invoked. For instance, expansion flags
+ * are usually Void-typed and do not invoke the converter.
+ */
+ public static class VoidConverter implements Converter<Void> {
+ @Override
+ public Void convert(String input) throws OptionsParsingException {
+ if (input == null) {
+ return null; // expected input, return is unused so null is fine.
+ }
+ throw new OptionsParsingException("'" + input + "' unexpected");
+ }
+
+ @Override
+ public String getTypeDescription() {
+ return "";
+ }
+ }
+
+ /**
+ * The converters that are available to the options parser by default. These are used if the
+ * {@code @Option} annotation does not specify its own {@code converter}, and its type is one of
+ * the following.
+ */
+ static final Map<Class<?>, Converter<?>> DEFAULT_CONVERTERS = Maps.newHashMap();
+
+ static {
+ DEFAULT_CONVERTERS.put(String.class, new Converters.StringConverter());
+ DEFAULT_CONVERTERS.put(int.class, new Converters.IntegerConverter());
+ DEFAULT_CONVERTERS.put(long.class, new Converters.LongConverter());
+ DEFAULT_CONVERTERS.put(double.class, new Converters.DoubleConverter());
+ DEFAULT_CONVERTERS.put(boolean.class, new Converters.BooleanConverter());
+ DEFAULT_CONVERTERS.put(TriState.class, new Converters.TriStateConverter());
+ DEFAULT_CONVERTERS.put(Void.class, new Converters.VoidConverter());
+ }
+
/**
* Join a list of words as in English. Examples:
* "nothing"
@@ -92,7 +256,7 @@
public static class LogLevelConverter implements Converter<Level> {
- public static Level[] LEVELS = new Level[] {
+ public static final Level[] LEVELS = new Level[] {
Level.OFF, Level.SEVERE, Level.WARNING, Level.INFO, Level.FINE,
Level.FINER, Level.FINEST
};
@@ -295,32 +459,4 @@
}
}
- /**
- * A converter for boolean values. This is already one of the defaults, so clients
- * should not typically need to add this.
- */
- public static class BooleanConverter implements Converter<Boolean> {
- @Override
- public Boolean convert(String input) throws OptionsParsingException {
- if (input == null) {
- return false;
- }
- input = input.toLowerCase();
- if (input.equals("true") || input.equals("1") || input.equals("yes") ||
- input.equals("t") || input.equals("y")) {
- return true;
- }
- if (input.equals("false") || input.equals("0") || input.equals("no") ||
- input.equals("f") || input.equals("n")) {
- return false;
- }
- throw new OptionsParsingException("'" + input + "' is not a boolean");
- }
-
- @Override
- public String getTypeDescription() {
- return "a boolean";
- }
- }
-
}
diff --git a/java/com/google/devtools/common/options/OptionsData.java b/java/com/google/devtools/common/options/OptionsData.java
index ac23d63..ae315a4 100644
--- a/java/com/google/devtools/common/options/OptionsData.java
+++ b/java/com/google/devtools/common/options/OptionsData.java
@@ -28,7 +28,6 @@
import java.util.Collections;
import java.util.List;
import java.util.Map;
-
import javax.annotation.concurrent.Immutable;
/**
@@ -66,7 +65,7 @@
/**
* Mapping from each Option-annotated field to the proper converter.
*
- * @see OptionsParserImpl#findConverter
+ * @see #findConverter
*/
private final Map<Field, Converter<?>> converters;
@@ -130,6 +129,73 @@
return allowMultiple.get(field);
}
+ /**
+ * For an option that does not use {@link Option#allowMultiple}, returns its type. For an option
+ * 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()) {
+ // If the type isn't a List<T>, this is an error in the option's declaration.
+ if (!(fieldType instanceof ParameterizedType)) {
+ throw new AssertionError("Type of multiple occurrence option must be a List<...>");
+ }
+ ParameterizedType pfieldType = (ParameterizedType) fieldType;
+ if (pfieldType.getRawType() != List.class) {
+ throw new AssertionError("Type of multiple occurrence option must be a List<...>");
+ }
+ fieldType = pfieldType.getActualTypeArguments()[0];
+ }
+ return fieldType;
+ }
+
+ /**
+ * Returns whether a field should be considered as boolean.
+ *
+ * <p>Can be used for usage help and controlling whether the "no" prefix is allowed.
+ */
+ static boolean isBooleanField(Field field) {
+ return field.getType().equals(boolean.class)
+ || field.getType().equals(TriState.class)
+ || findConverter(field) instanceof BoolOrEnumConverter;
+ }
+
+ /** Returns whether a field has Void type. */
+ static boolean isVoidField(Field field) {
+ return field.getType().equals(Void.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.
+ */
+ static Converter<?> findConverter(Field optionField) {
+ Option annotation = optionField.getAnnotation(Option.class);
+ if (annotation.converter() == Converter.class) {
+ // No converter provided, use the default one.
+ Type type = getFieldSingularType(optionField, annotation);
+ Converter<?> converter = Converters.DEFAULT_CONVERTERS.get(type);
+ if (converter == null) {
+ throw new AssertionError(
+ "No converter found for "
+ + type
+ + "; possible fix: add "
+ + "converter=... to @Option annotation for "
+ + optionField.getName());
+ }
+ return converter;
+ }
+ try {
+ // Instantiate the given Converter class.
+ Class<?> converter = annotation.converter();
+ Constructor<?> constructor = converter.getConstructor(new Class<?>[0]);
+ return (Converter<?>) constructor.newInstance(new Object[0]);
+ } catch (Exception e) {
+ throw new AssertionError(e);
+ }
+ }
+
private static List<Field> getAllAnnotatedFields(Class<? extends OptionsBase> optionsClass) {
List<Field> allFields = Lists.newArrayList();
for (Field field : optionsClass.getFields()) {
@@ -144,7 +210,7 @@
}
private static Object retrieveDefaultFromAnnotation(Field optionField) {
- Converter<?> converter = OptionsParserImpl.findConverter(optionField);
+ Converter<?> converter = findConverter(optionField);
String defaultValueAsString = OptionsParserImpl.getDefaultOptionString(optionField);
// Special case for "null"
if (OptionsParserImpl.isSpecialNullDefault(defaultValueAsString, optionField)) {
@@ -194,27 +260,13 @@
for (Field field : fields) {
Option annotation = field.getAnnotation(Option.class);
- // Check that the field type is a List, and that the converter
- // type matches the element type of the list.
- Type fieldType = field.getGenericType();
- if (annotation.allowMultiple()) {
- if (!(fieldType instanceof ParameterizedType)) {
- throw new AssertionError("Type of multiple occurrence option must be a List<...>");
- }
- ParameterizedType pfieldType = (ParameterizedType) fieldType;
- if (pfieldType.getRawType() != List.class) {
- // Throw an assertion, because this indicates an undetected type
- // error in the code.
- throw new AssertionError("Type of multiple occurrence option must be a List<...>");
- }
- fieldType = pfieldType.getActualTypeArguments()[0];
- }
+ Type fieldType = getFieldSingularType(field, annotation);
// Get the converter return type.
@SuppressWarnings("rawtypes")
Class<? extends Converter> converter = annotation.converter();
if (converter == Converter.class) {
- Converter<?> actualConverter = OptionsParserImpl.DEFAULT_CONVERTERS.get(fieldType);
+ Converter<?> actualConverter = Converters.DEFAULT_CONVERTERS.get(fieldType);
if (actualConverter == null) {
throw new AssertionError("Cannot find converter for field of type "
+ field.getType() + " named " + field.getName()
@@ -282,7 +334,7 @@
optionDefaultsBuilder.put(field, retrieveDefaultFromAnnotation(field));
- convertersBuilder.put(field, OptionsParserImpl.findConverter(field));
+ convertersBuilder.put(field, findConverter(field));
allowMultipleBuilder.put(field, annotation.allowMultiple());
}
diff --git a/java/com/google/devtools/common/options/OptionsParserImpl.java b/java/com/google/devtools/common/options/OptionsParserImpl.java
index c15f927..93c2cdd 100644
--- a/java/com/google/devtools/common/options/OptionsParserImpl.java
+++ b/java/com/google/devtools/common/options/OptionsParserImpl.java
@@ -32,8 +32,6 @@
import com.google.devtools.common.options.OptionsParser.UnparsedOptionValueDescription;
import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
-import java.lang.reflect.ParameterizedType;
-import java.lang.reflect.Type;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
@@ -50,100 +48,6 @@
*/
class OptionsParserImpl {
- /**
- * A bunch of default converters in case the user doesn't specify a
- * different one in the field annotation.
- */
- static final Map<Class<?>, Converter<?>> DEFAULT_CONVERTERS = Maps.newHashMap();
-
- static {
- DEFAULT_CONVERTERS.put(String.class, new Converter<String>() {
- @Override
- public String convert(String input) {
- return input;
- }
- @Override
- public String getTypeDescription() {
- return "a string";
- }});
- DEFAULT_CONVERTERS.put(int.class, new Converter<Integer>() {
- @Override
- public Integer convert(String input) throws OptionsParsingException {
- try {
- return Integer.decode(input);
- } catch (NumberFormatException e) {
- throw new OptionsParsingException("'" + input + "' is not an int");
- }
- }
- @Override
- public String getTypeDescription() {
- return "an integer";
- }});
- DEFAULT_CONVERTERS.put(double.class, new Converter<Double>() {
- @Override
- public Double convert(String input) throws OptionsParsingException {
- try {
- return Double.parseDouble(input);
- } catch (NumberFormatException e) {
- throw new OptionsParsingException("'" + input + "' is not a double");
- }
- }
- @Override
- public String getTypeDescription() {
- return "a double";
- }});
- DEFAULT_CONVERTERS.put(boolean.class, new Converters.BooleanConverter());
- DEFAULT_CONVERTERS.put(TriState.class, new Converter<TriState>() {
- @Override
- public TriState convert(String input) throws OptionsParsingException {
- if (input == null) {
- return TriState.AUTO;
- }
- input = input.toLowerCase();
- if (input.equals("auto")) {
- return TriState.AUTO;
- }
- if (input.equals("true") || input.equals("1") || input.equals("yes") ||
- input.equals("t") || input.equals("y")) {
- return TriState.YES;
- }
- if (input.equals("false") || input.equals("0") || input.equals("no") ||
- input.equals("f") || input.equals("n")) {
- return TriState.NO;
- }
- throw new OptionsParsingException("'" + input + "' is not a boolean");
- }
- @Override
- public String getTypeDescription() {
- return "a tri-state (auto, yes, no)";
- }});
- DEFAULT_CONVERTERS.put(Void.class, new Converter<Void>() {
- @Override
- public Void convert(String input) throws OptionsParsingException {
- if (input == null) {
- return null; // expected input, return is unused so null is fine.
- }
- throw new OptionsParsingException("'" + input + "' unexpected");
- }
- @Override
- public String getTypeDescription() {
- return "";
- }});
- DEFAULT_CONVERTERS.put(long.class, new Converter<Long>() {
- @Override
- public Long convert(String input) throws OptionsParsingException {
- try {
- return Long.decode(input);
- } catch (NumberFormatException e) {
- throw new OptionsParsingException("'" + input + "' is not a long");
- }
- }
- @Override
- public String getTypeDescription() {
- return "a long integer";
- }});
- }
-
private final OptionsData optionsData;
/**
@@ -701,7 +605,7 @@
booleanValue = false;
if (field != null) {
// TODO(bazel-team): Add tests for these cases.
- if (!OptionsParserImpl.isBooleanField(field)) {
+ if (!OptionsData.isBooleanField(field)) {
throw new OptionsParsingException(
"Illegal use of 'no' prefix on non-boolean option: " + arg, arg);
}
@@ -725,7 +629,7 @@
if (value == null) {
// Special-case boolean to supply value based on presence of "no" prefix.
- if (OptionsParserImpl.isBooleanField(field)) {
+ if (OptionsData.isBooleanField(field)) {
value = booleanValue ? "1" : "0";
} else if (field.getType().equals(Void.class) && !option.wrapperOption()) {
// This is expected, Void type options have no args (unless they're wrapper options).
@@ -782,46 +686,7 @@
return annotation.defaultValue();
}
- static boolean isBooleanField(Field field) {
- return field.getType().equals(boolean.class)
- || field.getType().equals(TriState.class)
- || findConverter(field) instanceof BoolOrEnumConverter;
- }
-
- static boolean isVoidField(Field field) {
- return field.getType().equals(Void.class);
- }
-
static boolean isSpecialNullDefault(String defaultValueString, Field optionField) {
return defaultValueString.equals("null") && !optionField.getType().isPrimitive();
}
-
- static Converter<?> findConverter(Field optionField) {
- Option annotation = optionField.getAnnotation(Option.class);
- if (annotation.converter() == Converter.class) {
- Type type;
- if (annotation.allowMultiple()) {
- // The OptionParserImpl already checked that the type is List<T> for some T;
- // here we extract the type T.
- type = ((ParameterizedType) optionField.getGenericType()).getActualTypeArguments()[0];
- } else {
- type = optionField.getType();
- }
- Converter<?> converter = DEFAULT_CONVERTERS.get(type);
- if (converter == null) {
- throw new AssertionError("No converter found for "
- + type + "; possible fix: add "
- + "converter=... to @Option annotation for "
- + optionField.getName());
- }
- return converter;
- }
- try {
- Class<?> converter = annotation.converter();
- Constructor<?> constructor = converter.getConstructor(new Class<?>[0]);
- return (Converter<?>) constructor.newInstance(new Object[0]);
- } catch (Exception e) {
- throw new AssertionError(e);
- }
- }
}
diff --git a/java/com/google/devtools/common/options/OptionsUsage.java b/java/com/google/devtools/common/options/OptionsUsage.java
index b8c19df..f3ee4d3 100644
--- a/java/com/google/devtools/common/options/OptionsUsage.java
+++ b/java/com/google/devtools/common/options/OptionsUsage.java
@@ -13,14 +13,11 @@
// limitations under the License.
package com.google.devtools.common.options;
-import static com.google.devtools.common.options.OptionsParserImpl.findConverter;
-
import com.google.common.base.Joiner;
import com.google.common.base.Splitter;
import com.google.common.base.Strings;
import com.google.common.collect.Lists;
import com.google.common.escape.Escaper;
-
import java.lang.reflect.Field;
import java.text.BreakIterator;
import java.util.Collections;
@@ -138,8 +135,7 @@
Option annotation = optionField.getAnnotation(Option.class);
usage.append("<dt><code><a name=\"flag--").append(plainFlagName).append("\"></a>--");
usage.append(flagName);
- if (OptionsParserImpl.isBooleanField(optionField)
- || OptionsParserImpl.isVoidField(optionField)) {
+ if (OptionsData.isBooleanField(optionField) || OptionsData.isVoidField(optionField)) {
// Nothing for boolean, tristate, boolean_or_enum, or void options.
} else if (!valueDescription.isEmpty()) {
usage.append("=").append(escaper.escape(valueDescription));
@@ -157,7 +153,7 @@
} else {
// Don't call the annotation directly (we must allow overrides to certain defaults).
String defaultValueString = OptionsParserImpl.getDefaultOptionString(optionField);
- if (OptionsParserImpl.isVoidField(optionField)) {
+ if (OptionsData.isVoidField(optionField)) {
// Void options don't have a default.
} else if (OptionsParserImpl.isSpecialNullDefault(defaultValueString, optionField)) {
usage.append(" default: see description");
@@ -259,12 +255,12 @@
};
private static String getTypeDescription(Field optionsField) {
- return findConverter(optionsField).getTypeDescription();
+ return OptionsData.findConverter(optionsField).getTypeDescription();
}
static String getFlagName(Field field) {
String name = field.getAnnotation(Option.class).name();
- return OptionsParserImpl.isBooleanField(field) ? "[no]" + name : name;
+ return OptionsData.isBooleanField(field) ? "[no]" + name : name;
}
}
