From 097e64c412c6a4162a22880fd435ef4632878406 Mon Sep 17 00:00:00 2001 From: Jon Brandvein Date: Fri, 17 Mar 2017 19:58:04 +0000 Subject: 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 --- .../google/devtools/common/options/Converters.java | 196 +++++++++++++++++---- .../devtools/common/options/OptionsData.java | 92 +++++++--- .../devtools/common/options/OptionsParserImpl.java | 139 +-------------- .../devtools/common/options/OptionsUsage.java | 12 +- 4 files changed, 244 insertions(+), 195 deletions(-) (limited to 'src') diff --git a/src/main/java/com/google/devtools/common/options/Converters.java b/src/main/java/com/google/devtools/common/options/Converters.java index c8b4d47bb8..0d1902933a 100644 --- a/src/main/java/com/google/devtools/common/options/Converters.java +++ b/src/main/java/com/google/devtools/common/options/Converters.java @@ -16,7 +16,6 @@ package com.google.devtools.common.options; 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 @@ import java.util.regex.PatternSyntaxException; */ public final class Converters { + /** Standard converter for booleans. Accepts common shorthands/synonyms. */ + public static class BooleanConverter implements Converter { + @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 { + @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 { + @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 { + @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 { + @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 { + @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 { + @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, 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 final class Converters { public static class LogLevelConverter implements Converter { - 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 @@ public final class Converters { } } - /** - * 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 { - @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/src/main/java/com/google/devtools/common/options/OptionsData.java b/src/main/java/com/google/devtools/common/options/OptionsData.java index ac23d639b2..ae315a4682 100644 --- a/src/main/java/com/google/devtools/common/options/OptionsData.java +++ b/src/main/java/com/google/devtools/common/options/OptionsData.java @@ -28,7 +28,6 @@ import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Map; - import javax.annotation.concurrent.Immutable; /** @@ -66,7 +65,7 @@ final class OptionsData extends OpaqueOptionsData { /** * Mapping from each Option-annotated field to the proper converter. * - * @see OptionsParserImpl#findConverter + * @see #findConverter */ private final Map> converters; @@ -130,6 +129,73 @@ final class OptionsData extends OpaqueOptionsData { 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} 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, 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. + * + *

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 getAllAnnotatedFields(Class optionsClass) { List allFields = Lists.newArrayList(); for (Field field : optionsClass.getFields()) { @@ -144,7 +210,7 @@ final class OptionsData extends OpaqueOptionsData { } 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 @@ final class OptionsData extends OpaqueOptionsData { 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 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 @@ final class OptionsData extends OpaqueOptionsData { optionDefaultsBuilder.put(field, retrieveDefaultFromAnnotation(field)); - convertersBuilder.put(field, OptionsParserImpl.findConverter(field)); + convertersBuilder.put(field, findConverter(field)); allowMultipleBuilder.put(field, annotation.allowMultiple()); } 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 c15f92757d..93c2cdd0cd 100644 --- a/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java +++ b/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java @@ -32,8 +32,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.lang.reflect.ParameterizedType; -import java.lang.reflect.Type; import java.util.Arrays; import java.util.Collection; import java.util.Collections; @@ -50,100 +48,6 @@ import java.util.Map; */ class OptionsParserImpl { - /** - * A bunch of default converters in case the user doesn't specify a - * different one in the field annotation. - */ - static final Map, Converter> DEFAULT_CONVERTERS = Maps.newHashMap(); - - static { - DEFAULT_CONVERTERS.put(String.class, new Converter() { - @Override - public String convert(String input) { - return input; - } - @Override - public String getTypeDescription() { - return "a string"; - }}); - DEFAULT_CONVERTERS.put(int.class, new Converter() { - @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() { - @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() { - @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() { - @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() { - @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 @@ class OptionsParserImpl { 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 @@ class OptionsParserImpl { 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 @@ class OptionsParserImpl { 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 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/src/main/java/com/google/devtools/common/options/OptionsUsage.java b/src/main/java/com/google/devtools/common/options/OptionsUsage.java index b8c19df800..f3ee4d3536 100644 --- a/src/main/java/com/google/devtools/common/options/OptionsUsage.java +++ b/src/main/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 @@ class OptionsUsage { Option annotation = optionField.getAnnotation(Option.class); usage.append("

--"); 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 @@ class OptionsUsage { } 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 @@ class OptionsUsage { }; 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; } } -- cgit v1.2.3