diff options
9 files changed, 875 insertions, 537 deletions
diff --git a/src/main/java/com/google/devtools/common/options/ExpansionFunction.java b/src/main/java/com/google/devtools/common/options/ExpansionFunction.java new file mode 100644 index 0000000000..ffab6e70fc --- /dev/null +++ b/src/main/java/com/google/devtools/common/options/ExpansionFunction.java @@ -0,0 +1,31 @@ +// 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; + +/** + * A function from an option parser's static setup (what flags it knows about) to an expansion + * String[] to use for one of its options. + */ +public interface ExpansionFunction { + + /** + * Compute the expansion for an option. May be called at any time during or after the {@link + * OptionsParser}'s construction, or not at all. + * + * @param optionsData the parser's indexed information about its own options, before expansion + * information is computed + * @return An expansion to use for all occurrences of this option in this parser + */ + public String[] getExpansion(IsolatedOptionsData optionsData); +} diff --git a/src/main/java/com/google/devtools/common/options/IsolatedOptionsData.java b/src/main/java/com/google/devtools/common/options/IsolatedOptionsData.java new file mode 100644 index 0000000000..27f42f48b1 --- /dev/null +++ b/src/main/java/com/google/devtools/common/options/IsolatedOptionsData.java @@ -0,0 +1,382 @@ +// Copyright 2014 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.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Lists; +import com.google.common.collect.Maps; +import java.lang.reflect.Constructor; +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.lang.reflect.ParameterizedType; +import java.lang.reflect.Type; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import javax.annotation.concurrent.Immutable; + +/** + * An immutable selection of options data corresponding to a set of options classes. The data is + * collected using reflection, which can be expensive. Therefore this class can be used internally + * to cache the results. + * + * <p>The data is isolated in the sense that it has not yet been processed to add inter-option- + * dependent information -- namely, the results of evaluating expansion functions. The {@link + * OptionsData} subclass stores this added information. The reason for the split is so that we can + * avoid exposing to expansion functions the effects of evaluating other expansion functions, to + * ensure that the order in which they run is not significant. + */ +// TODO(brandjon): This class is technically not necessarily immutable due to optionsDefault +// accepting Object values, and the List in allOptionsField should be ImmutableList. Either fix +// this or remove @Immutable. +@Immutable +class IsolatedOptionsData extends OpaqueOptionsData { + + /** + * These are the options-declaring classes which are annotated with {@link Option} annotations. + */ + private final ImmutableMap<Class<? extends OptionsBase>, Constructor<?>> optionsClasses; + + /** Maps option name to Option-annotated Field. */ + private final ImmutableMap<String, Field> nameToField; + + /** Maps option abbreviation to Option-annotated Field. */ + private final ImmutableMap<Character, Field> abbrevToField; + + /** For each options class, contains a list of all Option-annotated fields in that class. */ + private final ImmutableMap<Class<? extends OptionsBase>, List<Field>> allOptionsFields; + + /** Mapping from each Option-annotated field to the default value for that field. */ + // Immutable like the others, but uses Collections.unmodifiableMap because of null values. + private final Map<Field, Object> optionDefaults; + + /** + * Mapping from each Option-annotated field to the proper converter. + * + * @see #findConverter + */ + private final ImmutableMap<Field, Converter<?>> converters; + + /** + * Mapping from each Option-annotated field to a boolean for whether that field allows multiple + * values. + */ + private final ImmutableMap<Field, Boolean> allowMultiple; + + private IsolatedOptionsData( + Map<Class<? extends OptionsBase>, Constructor<?>> optionsClasses, + Map<String, Field> nameToField, + Map<Character, Field> abbrevToField, + Map<Class<? extends OptionsBase>, List<Field>> allOptionsFields, + Map<Field, Object> optionDefaults, + Map<Field, Converter<?>> converters, + Map<Field, Boolean> allowMultiple) { + this.optionsClasses = ImmutableMap.copyOf(optionsClasses); + this.nameToField = ImmutableMap.copyOf(nameToField); + this.abbrevToField = ImmutableMap.copyOf(abbrevToField); + this.allOptionsFields = ImmutableMap.copyOf(allOptionsFields); + // Can't use an ImmutableMap here because of null values. + this.optionDefaults = Collections.unmodifiableMap(optionDefaults); + this.converters = ImmutableMap.copyOf(converters); + this.allowMultiple = ImmutableMap.copyOf(allowMultiple); + } + + protected IsolatedOptionsData(IsolatedOptionsData other) { + this( + other.optionsClasses, + other.nameToField, + other.abbrevToField, + other.allOptionsFields, + other.optionDefaults, + other.converters, + other.allowMultiple); + } + + public Collection<Class<? extends OptionsBase>> getOptionsClasses() { + return optionsClasses.keySet(); + } + + @SuppressWarnings("unchecked") // The construction ensures that the case is always valid. + public <T extends OptionsBase> Constructor<T> getConstructor(Class<T> clazz) { + return (Constructor<T>) optionsClasses.get(clazz); + } + + public Field getFieldFromName(String name) { + return nameToField.get(name); + } + + public Iterable<Map.Entry<String, Field>> getAllNamedFields() { + return nameToField.entrySet(); + } + + public Field getFieldForAbbrev(char abbrev) { + return abbrevToField.get(abbrev); + } + + public List<Field> getFieldsForClass(Class<? extends OptionsBase> optionsClass) { + return allOptionsFields.get(optionsClass); + } + + public Object getDefaultValue(Field field) { + return optionDefaults.get(field); + } + + public Converter<?> getConverter(Field field) { + return converters.get(field); + } + + public boolean getAllowMultiple(Field field) { + 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); + } + + /** + * 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. + */ + 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(); + return (Converter<?>) constructor.newInstance(); + } catch (Exception e) { + // This indicates an error in the Converter, and should be discovered the first time it is + // used. + throw new AssertionError(e); + } + } + + private static List<Field> getAllAnnotatedFields(Class<? extends OptionsBase> optionsClass) { + List<Field> allFields = Lists.newArrayList(); + for (Field field : optionsClass.getFields()) { + if (field.isAnnotationPresent(Option.class)) { + allFields.add(field); + } + } + if (allFields.isEmpty()) { + throw new IllegalStateException(optionsClass + " has no public @Option-annotated fields"); + } + return ImmutableList.copyOf(allFields); + } + + private static Object retrieveDefaultFromAnnotation(Field optionField) { + Converter<?> converter = findConverter(optionField); + String defaultValueAsString = OptionsParserImpl.getDefaultOptionString(optionField); + // Special case for "null" + if (OptionsParserImpl.isSpecialNullDefault(defaultValueAsString, optionField)) { + return null; + } + boolean allowsMultiple = optionField.getAnnotation(Option.class).allowMultiple(); + // 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. + if (allowsMultiple) { + return Collections.emptyList(); + } + // Otherwise try to convert the default value using the converter + Object convertedValue; + try { + convertedValue = converter.convert(defaultValueAsString); + } catch (OptionsParsingException e) { + throw new IllegalStateException("OptionsParsingException while " + + "retrieving default for " + optionField.getName() + ": " + + e.getMessage()); + } + return convertedValue; + } + + /** + * Constructs an {@link IsolatedOptionsData} object for a parser that knows about the given + * {@link OptionsBase} classes. No inter-option analysis is done. Performs basic sanity checking + * on each option in isolation. + */ + static IsolatedOptionsData from(Collection<Class<? extends OptionsBase>> classes) { + Map<Class<? extends OptionsBase>, Constructor<?>> constructorBuilder = Maps.newHashMap(); + Map<Class<? extends OptionsBase>, List<Field>> allOptionsFieldsBuilder = Maps.newHashMap(); + Map<String, Field> nameToFieldBuilder = Maps.newHashMap(); + Map<Character, Field> abbrevToFieldBuilder = Maps.newHashMap(); + Map<Field, Object> optionDefaultsBuilder = Maps.newHashMap(); + Map<Field, Converter<?>> convertersBuilder = Maps.newHashMap(); + Map<Field, Boolean> allowMultipleBuilder = Maps.newHashMap(); + + // Read all Option annotations: + for (Class<? extends OptionsBase> parsedOptionsClass : classes) { + try { + Constructor<? extends OptionsBase> constructor = + parsedOptionsClass.getConstructor(); + constructorBuilder.put(parsedOptionsClass, constructor); + } catch (NoSuchMethodException e) { + throw new IllegalArgumentException(parsedOptionsClass + + " lacks an accessible default constructor"); + } + List<Field> fields = getAllAnnotatedFields(parsedOptionsClass); + allOptionsFieldsBuilder.put(parsedOptionsClass, fields); + + for (Field field : fields) { + Option annotation = field.getAnnotation(Option.class); + + if (annotation.name() == null) { + throw new AssertionError("Option cannot have a null name"); + } + + Type fieldType = getFieldSingularType(field, annotation); + + // Get the converter return type. + @SuppressWarnings("rawtypes") + Class<? extends Converter> converter = annotation.converter(); + if (converter == Converter.class) { + 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() + + " in class " + field.getDeclaringClass().getName()); + } + converter = actualConverter.getClass(); + } + if (Modifier.isAbstract(converter.getModifiers())) { + throw new AssertionError("The converter type " + converter + + " must be a concrete type"); + } + Type converterResultType; + try { + Method convertMethod = converter.getMethod("convert", String.class); + converterResultType = GenericTypeHelper.getActualReturnType(converter, convertMethod); + } catch (NoSuchMethodException e) { + throw new AssertionError("A known converter object doesn't implement the convert" + + " method"); + } + + if (annotation.allowMultiple()) { + if (GenericTypeHelper.getRawType(converterResultType) == List.class) { + Type elementType = + ((ParameterizedType) converterResultType).getActualTypeArguments()[0]; + if (!GenericTypeHelper.isAssignableFrom(fieldType, elementType)) { + throw new AssertionError("If the converter return type of a multiple occurance " + + "option is a list, then the type of list elements (" + fieldType + ") must be " + + "assignable from the converter list element type (" + elementType + ")"); + } + } else { + if (!GenericTypeHelper.isAssignableFrom(fieldType, converterResultType)) { + throw new AssertionError("Type of list elements (" + fieldType + + ") for multiple occurrence option must be assignable from the converter " + + "return type (" + converterResultType + ")"); + } + } + } else { + if (!GenericTypeHelper.isAssignableFrom(fieldType, converterResultType)) { + throw new AssertionError("Type of field (" + fieldType + + ") must be assignable from the converter " + + "return type (" + converterResultType + ")"); + } + } + + if (nameToFieldBuilder.put(annotation.name(), field) != null) { + throw new DuplicateOptionDeclarationException( + "Duplicate option name: --" + annotation.name()); + } + if (!annotation.oldName().isEmpty()) { + if (nameToFieldBuilder.put(annotation.oldName(), field) != null) { + throw new DuplicateOptionDeclarationException( + "Old option name duplicates option name: --" + annotation.oldName()); + } + } + if (annotation.abbrev() != '\0') { + if (abbrevToFieldBuilder.put(annotation.abbrev(), field) != null) { + throw new DuplicateOptionDeclarationException( + "Duplicate option abbrev: -" + annotation.abbrev()); + } + } + + optionDefaultsBuilder.put(field, retrieveDefaultFromAnnotation(field)); + + convertersBuilder.put(field, findConverter(field)); + + allowMultipleBuilder.put(field, annotation.allowMultiple()); + } + } + + return new IsolatedOptionsData( + constructorBuilder, + nameToFieldBuilder, + abbrevToFieldBuilder, + allOptionsFieldsBuilder, + optionDefaultsBuilder, + convertersBuilder, + allowMultipleBuilder); + } +} 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 1b2fb93f1b..249ee70fd0 100644 --- a/src/main/java/com/google/devtools/common/options/Option.java +++ b/src/main/java/com/google/devtools/common/options/Option.java @@ -47,143 +47,147 @@ public @interface Option { String valueHelp() default ""; /** - * The default value for the option. This method should only be invoked - * directly by the parser implementation. Any access to default values - * should go via the parser to allow for application specific defaults. + * The default value for the option. This method should only be invoked directly by the parser + * implementation. Any access to default values should go via the parser to allow for application + * specific defaults. * - * <p>There are two reasons this is a string. Firstly, it ensures that - * explicitly specifying this option at its default value (as printed in the - * usage message) has the same behavior as not specifying the option at all; - * this would be very hard to achieve if the default value was an instance of - * type T, since we'd need to ensure that {@link #toString()} and {@link - * #converter} were dual to each other. The second reason is more mundane - * but also more restrictive: annotation values must be compile-time - * constants. + * <p>There are two reasons this is a string. Firstly, it ensures that explicitly specifying this + * option at its default value (as printed in the usage message) has the same behavior as not + * specifying the option at all; this would be very hard to achieve if the default value was an + * instance of type T, since we'd need to ensure that {@link #toString()} and {@link #converter} + * were dual to each other. The second reason is more mundane but also more restrictive: + * annotation values must be compile-time constants. * - * <p>If an option's defaultValue() is the string "null", the option's - * converter will not be invoked to interpret it; a null reference will be - * used instead. (It would be nice if defaultValue could simply return null, - * but bizarrely, the Java Language Specification does not consider null to - * be a compile-time constant.) This special interpretation of the string - * "null" is only applicable when computing the default value; if specified - * on the command-line, this string will have its usual literal meaning. + * <p>If an option's defaultValue() is the string "null", the option's converter will not be + * invoked to interpret it; a null reference will be used instead. (It would be nice if + * defaultValue could simply return null, but bizarrely, the Java Language Specification does not + * consider null to be a compile-time constant.) This special interpretation of the string "null" + * is only applicable when computing the default value; if specified on the command-line, this + * string will have its usual literal meaning. * - * <p>The default value for flags that set allowMultiple is always the empty - * list and its default value is ignored. + * <p>The default value for flags that set allowMultiple is always the empty list and its default + * value is ignored. */ String defaultValue(); /** * A string describing the category of options that this belongs to. {@link - * OptionsParser#describeOptions} prints options of the same category grouped - * together. + * OptionsParser#describeOptions} prints options of the same category grouped together. * * <p>There are three special category values: + * * <ul> - * <li>{@code "undocumented"}: options which are useful for (some subset of) users, but not meant - * to be publicly advertised. For example, experimental options which are only meant to be - * used by specific testers or team members, but which should otherwise be treated normally. - * These options will not be listed in the usage info displayed for the {@code --help} option. - * They are otherwise normal - {@link OptionsParser.UnparsedOptionValueDescription#isHidden()} - * returns {@code false} for them, and they can be parsed normally from the command line or - * RC files. - * <li>{@code "hidden"}: options which users should not pass or know about, but which are used - * by the program (e.g., communication between a command-line client and a backend server). - * Like {@code "undocumented"} options, these options will not be listed in the usage info - * displayed for the {@code --help} option. However, in addition to this, calling - * {@link OptionsParser.UnparsedOptionValueDescription#isHidden()} on these options will - * return {@code true} - for example, this can be checked to strip out such secret options - * when logging or otherwise reporting the command line to the user. This category does not - * affect the option in any other way; it can still be parsed normally from the command line - * or an RC file. - * <li>{@code "internal"}: options which are purely for internal use within the JVM, and should - * never be shown to the user, nor ever need to be parsed by the options parser. - * Like {@code "hidden"} options, these options will not be listed in the usage info displayed - * for the --help option, and are considered hidden by - * {@link OptionsParser.UnparsedOptionValueDescription#isHidden()}. Unlike those, this type of - * option cannot be parsed by any call to {@link OptionsParser#parse} - it will be treated as - * if it was not defined. + * <li>{@code "undocumented"}: options which are useful for (some subset of) users, but not + * meant to be publicly advertised. For example, experimental options which are only meant + * to be used by specific testers or team members, but which should otherwise be treated + * normally. These options will not be listed in the usage info displayed for the {@code + * --help} option. They are otherwise normal - {@link + * OptionsParser.UnparsedOptionValueDescription#isHidden()} returns {@code false} for them, + * and they can be parsed normally from the command line or RC files. + * <li>{@code "hidden"}: options which users should not pass or know about, but which are used + * by the program (e.g., communication between a command-line client and a backend server). + * Like {@code "undocumented"} options, these options will not be listed in the usage info + * displayed for the {@code --help} option. However, in addition to this, calling {@link + * OptionsParser.UnparsedOptionValueDescription#isHidden()} on these options will return + * {@code true} - for example, this can be checked to strip out such secret options when + * logging or otherwise reporting the command line to the user. This category does not + * affect the option in any other way; it can still be parsed normally from the command line + * or an RC file. + * <li>{@code "internal"}: options which are purely for internal use within the JVM, and should + * never be shown to the user, nor ever need to be parsed by the options parser. Like {@code + * "hidden"} options, these options will not be listed in the usage info displayed for the + * --help option, and are considered hidden by {@link + * OptionsParser.UnparsedOptionValueDescription#isHidden()}. Unlike those, this type of + * option cannot be parsed by any call to {@link OptionsParser#parse} - it will be treated + * as if it was not defined. * </ul> */ String category() default "misc"; /** - * The converter that we'll use to convert this option into an object or - * a simple type. The default is to use the builtin converters. - * Custom converters must implement the {@link Converter} interface. + * The converter that we'll use to convert the string representation of this option's value into + * an object or a simple type. The default is to use the builtin converters ({@link + * Converters#DEFAULT_CONVERTERS}). Custom converters must implement the {@link Converter} + * interface. */ @SuppressWarnings({"unchecked", "rawtypes"}) // Can't figure out how to coerce Converter.class into Class<? extends Converter<?>> 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 flag indicating whether the option type should be allowed to occur multiple times in a single + * option list. * - * <p>If the command 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. + * <p>If the command 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. * - * <p>The {@link #defaultValue()} field of the annotation is ignored for repeatable - * flags and the default value will be the empty list. + * <p>The {@link #defaultValue()} field of the annotation is ignored for repeatable flags and the + * default value will be the empty list. */ boolean allowMultiple() default false; /** - * If the option is actually an abbreviation for other options, this field will - * contain the strings to expand this option into. The original option is dropped - * and the replacement used in its stead. It is recommended that such an option be - * of type {@link Void}. + * If the option is actually an abbreviation for other options, this field will contain the + * strings to expand this option into. The original option is dropped and the replacement used in + * its stead. It is recommended that such an option be of type {@link Void}. * - * An expanded option overrides previously specified options of the same name, - * even if it is explicitly specified. This is the original behavior and can - * be surprising if the user is not aware of it, which has led to several - * requests to change this behavior. This was discussed in the blaze team and - * it was decided that it is not a strong enough case to change the behavior. + * <p>An expanded option overrides previously specified options of the same name, even if it is + * explicitly specified. This is the original behavior and can be surprising if the user is not + * aware of it, which has led to several requests to change this behavior. This was discussed in + * the blaze team and it was decided that it is not a strong enough case to change the behavior. */ String[] expansion() default {}; /** - * If the option requires that additional options be implicitly appended, this field - * will contain the additional options. Implicit dependencies are parsed at the end - * of each {@link OptionsParser#parse} invocation, and override options specified in - * the same call. However, they can be overridden by options specified in a later - * call or by options with a higher priority. + * A mechanism for specifying an expansion that is a function of the parser's {@link + * IsolatedOptionsData}. This can be used to create an option that expands to different strings + * depending on what other options the parser knows about. + * + * <p>If provided (i.e. not {@link ExpansionFunction}{@code .class}), the {@code expansion} field + * must not be set. The mechanism of expansion is as if the {@code expansion} field were set to + * whatever the return value of this function is. + */ + Class<? extends ExpansionFunction> expansionFunction() default ExpansionFunction.class; + + /** + * If the option requires that additional options be implicitly appended, this field will contain + * the additional options. Implicit dependencies are parsed at the end of each {@link + * OptionsParser#parse} invocation, and override options specified in the same call. However, they + * can be overridden by options specified in a later call or by options with a higher priority. * * @see OptionPriority */ String[] implicitRequirements() default {}; /** - * If this field is a non-empty string, the option is deprecated, and a - * deprecation warning is added to the list of warnings when such an option - * is used. + * If this field is a non-empty string, the option is deprecated, and a deprecation warning is + * added to the list of warnings when such an option is used. */ String deprecationWarning() default ""; /** - * The old name for this option. If an option has a name "foo" and an old name "bar", - * --foo=baz and --bar=baz will be equivalent. If the old name is used, a warning will be printed - * indicating that the old name is deprecated and the new name should be used. + * The old name for this option. If an option has a name "foo" and an old name "bar", --foo=baz + * and --bar=baz will be equivalent. If the old name is used, a warning will be printed indicating + * that the old name is deprecated and the new name should be used. */ String oldName() default ""; /** - * Indicates that this option is a wrapper for other options, and will be unwrapped - * when parsed. For example, if foo is a wrapper option, then "--foo=--bar=baz" - * will be parsed as the flag "--bar=baz" (rather than --foo taking the value - * "--bar=baz"). A wrapper option should have the type {@link Void} (if it is something other - * than Void, the parser will not assign a value to it). The - * {@link Option#implicitRequirements()}, {@link Option#expansion()}, {@link Option#converter()} - * attributes will not be processed. Wrapper options are implicitly repeatable (i.e., as though - * {@link Option#allowMultiple()} is true regardless of its value in the annotation). + * Indicates that this option is a wrapper for other options, and will be unwrapped when parsed. + * For example, if foo is a wrapper option, then "--foo=--bar=baz" will be parsed as the flag + * "--bar=baz" (rather than --foo taking the value "--bar=baz"). A wrapper option should have the + * type {@link Void} (if it is something other than Void, the parser will not assign a value to + * it). The {@link Option#implicitRequirements()}, {@link Option#expansion()}, {@link + * Option#converter()} attributes will not be processed. Wrapper options are implicitly repeatable + * (i.e., as though {@link Option#allowMultiple()} is true regardless of its value in the + * annotation). * * <p>Wrapper options are provided only for transitioning flags which appear as values to other * flags, to top-level flags. Wrapper options should not be used in Invocation Policy, as - * expansion flags to other flags, or as implicit requirements to other flags. Use the inner - * flags instead. + * expansion flags to other flags, or as implicit requirements to other flags. Use the inner flags + * instead. */ boolean wrapperOption() default false; } 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 ae315a4682..e71321c23f 100644 --- a/src/main/java/com/google/devtools/common/options/OptionsData.java +++ b/src/main/java/com/google/devtools/common/options/OptionsData.java @@ -1,4 +1,4 @@ -// Copyright 2014 The Bazel Authors. All rights reserved. +// 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. @@ -14,332 +14,89 @@ package com.google.devtools.common.options; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import com.google.common.collect.Lists; import com.google.common.collect.Maps; import java.lang.reflect.Constructor; import java.lang.reflect.Field; -import java.lang.reflect.Method; import java.lang.reflect.Modifier; -import java.lang.reflect.ParameterizedType; -import java.lang.reflect.Type; import java.util.Collection; -import java.util.Collections; -import java.util.List; import java.util.Map; import javax.annotation.concurrent.Immutable; /** - * An immutable selection of options data corresponding to a set of options - * classes. The data is collected using reflection, which can be expensive. - * Therefore this class can be used internally to cache the results. + * This extends IsolatedOptionsData with information that can only be determined once all the {@link + * OptionsBase} subclasses for a parser are known. In particular, this includes expansion + * information. */ @Immutable -final class OptionsData extends OpaqueOptionsData { +final class OptionsData extends IsolatedOptionsData { /** - * These are the options-declaring classes which are annotated with - * {@link Option} annotations. + * Mapping from each Option-annotated field with a {@code String[]} expansion to that expansion. */ - private final Map<Class<? extends OptionsBase>, Constructor<?>> optionsClasses; + // TODO(brandjon): This is technically not necessarily immutable due to String[], and should use + // ImmutableList. Either fix this or remove @Immutable. + private final ImmutableMap<Field, String[]> evaluatedExpansions; - /** Maps option name to Option-annotated Field. */ - private final Map<String, Field> nameToField; - - /** Maps option abbreviation to Option-annotated Field. */ - private final Map<Character, Field> abbrevToField; - - /** - * For each options class, contains a list of all Option-annotated fields in - * that class. - */ - private final Map<Class<? extends OptionsBase>, List<Field>> allOptionsFields; - - /** - * Mapping from each Option-annotated field to the default value for that - * field. - */ - private final Map<Field, Object> optionDefaults; - - /** - * Mapping from each Option-annotated field to the proper converter. - * - * @see #findConverter - */ - private final Map<Field, Converter<?>> converters; - - /** - * Mapping from each Option-annotated field to a boolean for whether that field allows multiple - * values. - */ - private final Map<Field, Boolean> allowMultiple; - - private OptionsData(Map<Class<? extends OptionsBase>, Constructor<?>> optionsClasses, - Map<String, Field> nameToField, - Map<Character, Field> abbrevToField, - Map<Class<? extends OptionsBase>, List<Field>> allOptionsFields, - Map<Field, Object> optionDefaults, - Map<Field, Converter<?>> converters, - Map<Field, Boolean> allowMultiple) { - this.optionsClasses = ImmutableMap.copyOf(optionsClasses); - this.allOptionsFields = ImmutableMap.copyOf(allOptionsFields); - this.nameToField = ImmutableMap.copyOf(nameToField); - this.abbrevToField = ImmutableMap.copyOf(abbrevToField); - // Can't use an ImmutableMap here because of null values. - this.optionDefaults = Collections.unmodifiableMap(optionDefaults); - this.converters = ImmutableMap.copyOf(converters); - this.allowMultiple = ImmutableMap.copyOf(allowMultiple); + /** Construct {@link OptionsData} by extending an {@link IsolatedOptionsData} with new info. */ + private OptionsData(IsolatedOptionsData base, Map<Field, String[]> evaluatedExpansions) { + super(base); + this.evaluatedExpansions = ImmutableMap.copyOf(evaluatedExpansions); } - public Collection<Class<? extends OptionsBase>> getOptionsClasses() { - return optionsClasses.keySet(); - } - - @SuppressWarnings("unchecked") // The construction ensures that the case is always valid. - public <T extends OptionsBase> Constructor<T> getConstructor(Class<T> clazz) { - return (Constructor<T>) optionsClasses.get(clazz); - } - - public Field getFieldFromName(String name) { - return nameToField.get(name); - } - - public Iterable<Map.Entry<String, Field>> getAllNamedFields() { - return nameToField.entrySet(); - } - - public Field getFieldForAbbrev(char abbrev) { - return abbrevToField.get(abbrev); - } - - public List<Field> getFieldsForClass(Class<? extends OptionsBase> optionsClass) { - return allOptionsFields.get(optionsClass); - } - - public Object getDefaultValue(Field field) { - return optionDefaults.get(field); - } - - public Converter<?> getConverter(Field field) { - return converters.get(field); - } - - public boolean getAllowMultiple(Field field) { - 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; - } + private static final String[] EMPTY_EXPANSION = new String[] {}; /** - * Returns whether a field should be considered as boolean. - * - * <p>Can be used for usage help and controlling whether the "no" prefix is allowed. + * Returns the expansion of an options field, regardless of whether it was defined using {@link + * Option#expansion} or {@link Option#expansionFunction}. If the field is not an expansion option, + * returns an empty array. */ - 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); + public String[] getEvaluatedExpansion(Field field) { + String[] result = evaluatedExpansions.get(field); + return result != null ? result : EMPTY_EXPANSION; } /** - * 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. + * Constructs an {@link OptionsData} object for a parser that knows about the given {@link + * OptionsBase} classes. In addition to the work done to construct the {@link + * IsolatedOptionsData}, this also computes expansion information. */ - 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) { + public static OptionsData from(Collection<Class<? extends OptionsBase>> classes) { + IsolatedOptionsData isolatedData = IsolatedOptionsData.from(classes); + + // All that's left is to compute expansions. + Map<Field, String[]> evaluatedExpansionsBuilder = Maps.newHashMap(); + for (Map.Entry<String, Field> entry : isolatedData.getAllNamedFields()) { + Field field = entry.getValue(); + Option annotation = field.getAnnotation(Option.class); + // 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)) { 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()) { - if (field.isAnnotationPresent(Option.class)) { - allFields.add(field); - } - } - if (allFields.isEmpty()) { - throw new IllegalStateException(optionsClass + " has no public @Option-annotated fields"); - } - return ImmutableList.copyOf(allFields); - } - - private static Object retrieveDefaultFromAnnotation(Field optionField) { - Converter<?> converter = findConverter(optionField); - String defaultValueAsString = OptionsParserImpl.getDefaultOptionString(optionField); - // Special case for "null" - if (OptionsParserImpl.isSpecialNullDefault(defaultValueAsString, optionField)) { - return null; - } - boolean allowsMultiple = optionField.getAnnotation(Option.class).allowMultiple(); - // 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. - if (allowsMultiple) { - return Collections.emptyList(); - } - // Otherwise try to convert the default value using the converter - Object convertedValue; - try { - convertedValue = converter.convert(defaultValueAsString); - } catch (OptionsParsingException e) { - throw new IllegalStateException("OptionsParsingException while " - + "retrieving default for " + optionField.getName() + ": " - + e.getMessage()); - } - return convertedValue; - } - - static OptionsData of(Collection<Class<? extends OptionsBase>> classes) { - Map<Class<? extends OptionsBase>, Constructor<?>> constructorBuilder = Maps.newHashMap(); - Map<Class<? extends OptionsBase>, List<Field>> allOptionsFieldsBuilder = Maps.newHashMap(); - Map<String, Field> nameToFieldBuilder = Maps.newHashMap(); - Map<Character, Field> abbrevToFieldBuilder = Maps.newHashMap(); - Map<Field, Object> optionDefaultsBuilder = Maps.newHashMap(); - Map<Field, Converter<?>> convertersBuilder = Maps.newHashMap(); - Map<Field, Boolean> allowMultipleBuilder = Maps.newHashMap(); - - // Read all Option annotations: - for (Class<? extends OptionsBase> parsedOptionsClass : classes) { - try { - Constructor<? extends OptionsBase> constructor = - parsedOptionsClass.getConstructor(new Class[0]); - constructorBuilder.put(parsedOptionsClass, constructor); - } catch (NoSuchMethodException e) { - throw new IllegalArgumentException(parsedOptionsClass - + " lacks an accessible default constructor"); - } - List<Field> fields = getAllAnnotatedFields(parsedOptionsClass); - allOptionsFieldsBuilder.put(parsedOptionsClass, fields); - - for (Field field : fields) { - Option annotation = field.getAnnotation(Option.class); - - Type fieldType = getFieldSingularType(field, annotation); - - // Get the converter return type. - @SuppressWarnings("rawtypes") - Class<? extends Converter> converter = annotation.converter(); - if (converter == Converter.class) { - 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() - + " in class " + field.getDeclaringClass().getName()); - } - converter = actualConverter.getClass(); - } - if (Modifier.isAbstract(converter.getModifiers())) { - throw new AssertionError("The converter type (" + converter - + ") must be a concrete type"); - } - Type converterResultType; - try { - Method convertMethod = converter.getMethod("convert", String.class); - converterResultType = GenericTypeHelper.getActualReturnType(converter, convertMethod); - } catch (NoSuchMethodException e) { - throw new AssertionError("A known converter object doesn't implement the convert" - + " method"); - } - - if (annotation.allowMultiple()) { - if (GenericTypeHelper.getRawType(converterResultType) == List.class) { - Type elementType = - ((ParameterizedType) converterResultType).getActualTypeArguments()[0]; - if (!GenericTypeHelper.isAssignableFrom(fieldType, elementType)) { - throw new AssertionError("If the converter return type of a multiple occurance " + - "option is a list, then the type of list elements (" + fieldType + ") must be " + - "assignable from the converter list element type (" + elementType + ")"); - } - } else { - if (!GenericTypeHelper.isAssignableFrom(fieldType, converterResultType)) { - throw new AssertionError("Type of list elements (" + fieldType + - ") for multiple occurrence option must be assignable from the converter " + - "return type (" + converterResultType + ")"); - } - } - } else { - if (!GenericTypeHelper.isAssignableFrom(fieldType, converterResultType)) { - throw new AssertionError("Type of field (" + fieldType + - ") must be assignable from the converter " + - "return type (" + converterResultType + ")"); - } - } - - if (annotation.name() == null) { + "Cannot set both expansion and expansionFunction for option --" + annotation.name()); + } else if (constExpansion.length > 0) { + evaluatedExpansionsBuilder.put(field, constExpansion); + } else if (usesExpansionFunction(annotation)) { + if (Modifier.isAbstract(expansionFunctionClass.getModifiers())) { throw new AssertionError( - "Option cannot have a null name"); - } - if (nameToFieldBuilder.put(annotation.name(), field) != null) { - throw new DuplicateOptionDeclarationException( - "Duplicate option name: --" + annotation.name()); + "The expansionFunction type " + expansionFunctionClass + " must be a concrete type"); } - if (!annotation.oldName().isEmpty()) { - if (nameToFieldBuilder.put(annotation.oldName(), field) != null) { - throw new DuplicateOptionDeclarationException( - "Old option name duplicates option name: --" + annotation.oldName()); - } - } - if (annotation.abbrev() != '\0') { - if (abbrevToFieldBuilder.put(annotation.abbrev(), field) != null) { - throw new DuplicateOptionDeclarationException( - "Duplicate option abbrev: -" + annotation.abbrev()); - } + // Evaluate the ExpansionFunction. + ExpansionFunction instance; + try { + Constructor<?> constructor = expansionFunctionClass.getConstructor(); + instance = (ExpansionFunction) constructor.newInstance(); + } catch (Exception e) { + // This indicates an error in the ExpansionFunction, and should be discovered the first + // time it is used. + throw new AssertionError(e); } - - optionDefaultsBuilder.put(field, retrieveDefaultFromAnnotation(field)); - - convertersBuilder.put(field, findConverter(field)); - - allowMultipleBuilder.put(field, annotation.allowMultiple()); + String[] expansion = instance.getExpansion(isolatedData); + evaluatedExpansionsBuilder.put(field, expansion); } } - return new OptionsData(constructorBuilder, nameToFieldBuilder, abbrevToFieldBuilder, - allOptionsFieldsBuilder, optionDefaultsBuilder, convertersBuilder, allowMultipleBuilder); + + return new OptionsData(isolatedData, evaluatedExpansionsBuilder); } } 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 354e00a96e..1c4b2780a0 100644 --- a/src/main/java/com/google/devtools/common/options/OptionsParser.java +++ b/src/main/java/com/google/devtools/common/options/OptionsParser.java @@ -88,7 +88,7 @@ public class OptionsParser implements OptionsProvider { ImmutableList<Class<? extends OptionsBase>> optionsClasses) { OptionsData result = optionsData.get(optionsClasses); if (result == null) { - result = OptionsData.of(optionsClasses); + result = OptionsData.from(optionsClasses); optionsData.put(optionsClasses, result); } return result; @@ -140,10 +140,6 @@ public class OptionsParser implements OptionsProvider { private final List<String> residue = new ArrayList<String>(); private boolean allowResidue = true; - OptionsParser(Collection<Class<? extends OptionsBase>> optionsClasses) { - this(OptionsData.of(optionsClasses)); - } - OptionsParser(OptionsData optionsData) { impl = new OptionsParserImpl(optionsData); } @@ -401,7 +397,8 @@ public class OptionsParser implements OptionsProvider { boolean isExpansion() { Option option = field.getAnnotation(Option.class); - return option.expansion().length > 0; + return (option.expansion().length > 0 + || OptionsData.usesExpansionFunction(option)); } boolean isImplicitRequirement() { @@ -464,22 +461,19 @@ public class OptionsParser implements OptionsProvider { } /** - * Returns a description of all the options this parser can digest. - * In addition to {@link Option} annotations, this method also - * interprets {@link OptionsUsage} annotations which give an intuitive short - * description for the options. + * Returns a description of all the options this parser can digest. In addition to {@link Option} + * annotations, this method also interprets {@link OptionsUsage} annotations which give an + * intuitive short description for the options. Options of the same category (see {@link + * Option#category}) will be grouped together. * - * @param categoryDescriptions a mapping from category names to category - * descriptions. Options of the same category (see {@link - * Option#category}) will be grouped together, preceded by the description - * of the category. - * @param helpVerbosity if {@code long}, the options will be described - * verbosely, including their types, defaults and descriptions. If {@code - * medium}, the descriptions are omitted, and if {@code short}, the options - * are just enumerated. + * @param categoryDescriptions a mapping from category names to category descriptions. + * Descriptions are optional; if omitted, a string based on the category name will be used. + * @param helpVerbosity if {@code long}, the options will be described verbosely, including their + * types, defaults and descriptions. If {@code medium}, the descriptions are omitted, and if + * {@code short}, the options are just enumerated. */ - public String describeOptions(Map<String, String> categoryDescriptions, - HelpVerbosity helpVerbosity) { + public String describeOptions( + Map<String, String> categoryDescriptions, HelpVerbosity helpVerbosity) { StringBuilder desc = new StringBuilder(); if (!impl.getOptionsClasses().isEmpty()) { List<Field> allFields = Lists.newArrayList(); @@ -503,7 +497,7 @@ public class OptionsParser implements OptionsProvider { } if (documentationLevel(prevCategory) == DocumentationLevel.DOCUMENTED) { - OptionsUsage.getUsage(optionField, desc, helpVerbosity); + OptionsUsage.getUsage(optionField, desc, helpVerbosity, impl.getOptionsData()); } } } @@ -548,7 +542,7 @@ public class OptionsParser implements OptionsProvider { } if (level == DocumentationLevel.DOCUMENTED) { - OptionsUsage.getUsageHtml(optionField, desc, escaper); + OptionsUsage.getUsageHtml(optionField, desc, escaper, impl.getOptionsData()); } } desc.append("</dl>\n"); 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 adf875bf65..5c6498a36f 100644 --- a/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java +++ b/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java @@ -100,6 +100,10 @@ class OptionsParserImpl { this.optionsData = optionsData; } + OptionsData getOptionsData() { + return optionsData; + } + /** * Indicates whether or not the parser will allow long options with a * single-dash, instead of the usual double-dash, too, eg. -example instead of just --example. @@ -344,7 +348,8 @@ class OptionsParserImpl { // Recurse to remove any implicit or expansion flags that this flag may have added when // originally parsed. - for (String[] args : new String[][] {option.implicitRequirements(), option.expansion()}) { + String[] expansion = optionsData.getEvaluatedExpansion(field); + for (String[] args : new String[][] {option.implicitRequirements(), expansion}) { Iterator<String> argsIterator = Iterators.forArray(args); while (argsIterator.hasNext()) { String arg = argsIterator.next(); @@ -482,7 +487,8 @@ class OptionsParserImpl { } // Handle expansion options. - if (option.expansion().length > 0) { + String[] expansion = optionsData.getEvaluatedExpansion(field); + if (expansion.length > 0) { Function<Object, String> expansionSourceFunction = Functions.constant( "expanded from option --" @@ -491,7 +497,7 @@ class OptionsParserImpl { + sourceFunction.apply(originalName)); maybeAddDeprecationWarning(field); List<String> unparsed = parse(priority, expansionSourceFunction, null, originalName, - ImmutableList.copyOf(option.expansion())); + ImmutableList.copyOf(expansion)); if (!unparsed.isEmpty()) { // Throw an assertion, because this indicates an error in the code that specified the // expansion for the current option. 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 f3ee4d3536..aa48cb722f 100644 --- a/src/main/java/com/google/devtools/common/options/OptionsUsage.java +++ b/src/main/java/com/google/devtools/common/options/OptionsUsage.java @@ -23,6 +23,7 @@ import java.text.BreakIterator; import java.util.Collections; import java.util.Comparator; import java.util.List; +import javax.annotation.Nullable; /** * A renderer for usage messages. For now this is very simple. @@ -33,15 +34,17 @@ class OptionsUsage { private static final Joiner COMMA_JOINER = Joiner.on(","); /** - * Given an options class, render the usage string into the usage, - * which is passed in as an argument. + * Given an options class, render the usage string into the usage, which is passed in as an + * argument. This will not include information about expansions for options using expansion + * functions (it would be unsafe to report this as we cannot know what options from other {@link + * OptionsBase} subclasses they depend on until a complete parser is constructed). */ static void getUsage(Class<? extends OptionsBase> optionsClass, StringBuilder usage) { List<Field> optionFields = Lists.newArrayList(OptionsParser.getAllAnnotatedFields(optionsClass)); Collections.sort(optionFields, BY_NAME); for (Field optionField : optionFields) { - getUsage(optionField, usage, OptionsParser.HelpVerbosity.LONG); + getUsage(optionField, usage, OptionsParser.HelpVerbosity.LONG, null); } } @@ -76,10 +79,35 @@ class OptionsUsage { } /** - * Append the usage message for a single option-field message to 'usage'. + * Returns the expansion for an option, to the extent known. Precisely, if an {@link OptionsData} + * object is supplied, the expansion is read from that. Otherwise, the annotation is inspected: If + * the annotation uses {@link Option#expansion} it is returned, and if it uses {@link + * Option#expansionFunction} null is returned, indicating a lack of definite information. In all + * cases, when the option is not an expansion option, an empty array is returned. */ - static void getUsage(Field optionField, StringBuilder usage, - OptionsParser.HelpVerbosity helpVerbosity) { + private static @Nullable String[] getExpansionIfKnown( + Field optionField, Option annotation, @Nullable OptionsData optionsData) { + if (optionsData != null) { + return optionsData.getEvaluatedExpansion(optionField); + } else { + if (OptionsData.usesExpansionFunction(annotation)) { + return null; + } else { + // Empty array if it's not an expansion option. + return annotation.expansion(); + } + } + } + + /** + * Appends the usage message for a single option-field message to 'usage'. If {@code optionsData} + * is not supplied, options that use expansion functions won't be fully described. + */ + static void getUsage( + Field optionField, + StringBuilder usage, + OptionsParser.HelpVerbosity helpVerbosity, + @Nullable OptionsData optionsData) { String flagName = getFlagName(optionField); String typeDescription = getTypeDescription(optionField); Option annotation = optionField.getAnnotation(Option.class); @@ -114,9 +142,12 @@ class OptionsUsage { usage.append(paragraphFill(annotation.help(), 4, 80)); // (indent, width) usage.append('\n'); } - if (annotation.expansion().length > 0) { + String[] expansion = getExpansionIfKnown(optionField, annotation, optionsData); + if (expansion == null) { + usage.append(" Expands to unknown options.\n"); + } else if (expansion.length > 0) { StringBuilder expandsMsg = new StringBuilder("Expands to: "); - for (String exp : annotation.expansion()) { + for (String exp : expansion) { expandsMsg.append(exp).append(" "); } usage.append(paragraphFill(expandsMsg.toString(), 4, 80)); // (indent, width) @@ -125,9 +156,11 @@ class OptionsUsage { } /** - * Append the usage message for a single option-field message to 'usage'. + * Append the usage message for a single option-field message to 'usage'. If {@code optionsData} + * is not supplied, options that use expansion functions won't be fully described. */ - static void getUsageHtml(Field optionField, StringBuilder usage, Escaper escaper) { + static void getUsageHtml( + Field optionField, StringBuilder usage, Escaper escaper, @Nullable OptionsData optionsData) { String plainFlagName = optionField.getAnnotation(Option.class).name(); String flagName = getFlagName(optionField); String valueDescription = optionField.getAnnotation(Option.class).valueHelp(); @@ -167,10 +200,13 @@ class OptionsUsage { usage.append(paragraphFill(escaper.escape(annotation.help()), 0, 80)); // (indent, width) usage.append('\n'); } - if (annotation.expansion().length > 0) { + String[] expansion = getExpansionIfKnown(optionField, annotation, optionsData); + if (expansion == null) { + usage.append(" Expands to unknown options.<br>\n"); + } else if (expansion.length > 0) { usage.append("<br/>\n"); StringBuilder expandsMsg = new StringBuilder("Expands to:<br/>\n"); - for (String exp : annotation.expansion()) { + for (String exp : expansion) { // TODO(ulfjack): Can we link to the expanded flags here? expandsMsg .append(" <code>") diff --git a/src/test/java/com/google/devtools/common/options/OptionsParserTest.java b/src/test/java/com/google/devtools/common/options/OptionsParserTest.java index f8bcebd598..b8f669e1d5 100644 --- a/src/test/java/com/google/devtools/common/options/OptionsParserTest.java +++ b/src/test/java/com/google/devtools/common/options/OptionsParserTest.java @@ -838,32 +838,116 @@ public class OptionsParserTest { fail(); } + /** ConflictingExpansionOptions */ + public static class ConflictingExpansionsOptions extends OptionsBase { + + /** ExpFunc */ + public static class ExpFunc implements ExpansionFunction { + @Override + public String[] getExpansion(IsolatedOptionsData optionsData) { + return new String[] {"--yyy"}; + } + } + + @Option( + name = "badness", + expansion = {"--xxx"}, + expansionFunction = ExpFunc.class, + defaultValue = "null" + ) + public Void badness; + } + + @Test + public void conflictingExpansions() throws Exception { + try { + OptionsParser.newOptionsParser(ConflictingExpansionsOptions.class); + fail("Should have failed due to specifying both expansion and expansionFunction"); + } catch (AssertionError e) { + assertThat(e.getMessage()) + .contains("Cannot set both expansion and expansionFunction for " + "option --badness"); + } + } + + /** NullExpansionOptions */ + public static class NullExpansionsOptions extends OptionsBase { + + /** ExpFunc */ + public static class ExpFunc implements ExpansionFunction { + @Override + public String[] getExpansion(IsolatedOptionsData optionsData) { + return null; + } + } + + @Option(name = "badness", expansionFunction = ExpFunc.class, defaultValue = "null") + public Void badness; + } + + @Test + public void nullExpansions() throws Exception { + // Ensure that we get the NPE at the time of parser construction, not later when actually + // parsing. + try { + OptionsParser.newOptionsParser(NullExpansionsOptions.class); + fail("Should have failed due to null expansion function result"); + } catch (NullPointerException e) { + assertThat(e.getMessage()).contains("null value in entry"); + } + } + + /** ExpansionOptions */ public static class ExpansionOptions extends OptionsBase { - @Option(name = "first", - expansion = { "--second=first" }, - defaultValue = "null") - public Void first; + @Option(name = "underlying", defaultValue = "null") + public String underlying; + + @Option( + name = "expands", + expansion = {"--underlying=from_expansion"}, + defaultValue = "null" + ) + public Void expands; + + /** ExpFunc */ + public static class ExpFunc implements ExpansionFunction { + @Override + public String[] getExpansion(IsolatedOptionsData optionsData) { + return new String[] {"--expands"}; + } + } - @Option(name = "second", - defaultValue = "null") - public String second; + @Option(name = "expands_by_function", defaultValue = "null", expansionFunction = ExpFunc.class) + public Void expandsByFunction; + } + + @Test + public void describeOptionsWithExpansion() throws Exception { + // We have to test this here rather than in OptionsTest because expansion functions require + // that an options parser be constructed. + OptionsParser parser = OptionsParser.newOptionsParser(ExpansionOptions.class); + String usage = + parser.describeOptions(ImmutableMap.<String, String>of(), OptionsParser.HelpVerbosity.LONG); + assertThat(usage).contains(" --expands\n Expands to: --underlying=from_expansion"); + assertThat(usage).contains(" --expands_by_function\n Expands to: --expands"); } @Test public void overrideExpansionWithExplicit() throws Exception { OptionsParser parser = OptionsParser.newOptionsParser(ExpansionOptions.class); - parser.parse(OptionPriority.COMMAND_LINE, null, Arrays.asList("--first", "--second=second")); + parser.parse( + OptionPriority.COMMAND_LINE, null, Arrays.asList("--expands", "--underlying=direct_value")); ExpansionOptions options = parser.getOptions(ExpansionOptions.class); - assertEquals("second", options.second); + assertEquals("direct_value", options.underlying); assertEquals(0, parser.getWarnings().size()); } @Test public void overrideExplicitWithExpansion() throws Exception { OptionsParser parser = OptionsParser.newOptionsParser(ExpansionOptions.class); - parser.parse(OptionPriority.COMMAND_LINE, null, Arrays.asList("--second=second", "--first")); + parser.parse( + OptionPriority.COMMAND_LINE, null, Arrays.asList("--underlying=direct_value", "--expands")); ExpansionOptions options = parser.getOptions(ExpansionOptions.class); - assertEquals("first", options.second); + assertEquals("from_expansion", options.underlying); } @Test diff --git a/src/test/java/com/google/devtools/common/options/OptionsTest.java b/src/test/java/com/google/devtools/common/options/OptionsTest.java index fccc011ffb..21a8a067b3 100644 --- a/src/test/java/com/google/devtools/common/options/OptionsTest.java +++ b/src/test/java/com/google/devtools/common/options/OptionsTest.java @@ -16,15 +16,14 @@ package com.google.devtools.common.options; import static com.google.common.truth.Truth.assertThat; import static java.util.Arrays.asList; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import com.google.common.collect.Iterables; import com.google.common.testing.EqualsTester; import java.net.MalformedURLException; import java.net.URL; +import java.util.Map; +import java.util.TreeSet; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -66,6 +65,31 @@ public class OptionsTest { defaultValue = "null", expansion = { "--host=special.google.com", "--port=8080"}) public Void special; + + // Interestingly, the class needs to be public, or else the default constructor ends up not + // being public and the expander can't be instantiated. + /** SpecialExpansion */ + public static class SpecialExpansion implements ExpansionFunction { + @Override + public String[] getExpansion(IsolatedOptionsData optionsData) { + TreeSet<String> flags = new TreeSet<>(); + for (Map.Entry<String, ?> entry : optionsData.getAllNamedFields()) { + if (entry.getKey().startsWith("specialexp_")) { + flags.add("--" + entry.getKey()); + } + } + return Iterables.toArray(flags, String.class); + } + } + + @Option(name = "specialexp_foo", defaultValue = "false") + public boolean specialExpFoo; + + @Option(name = "specialexp_bar", defaultValue = "false") + public boolean specialExpBar; + + @Option(name = "specialexp", defaultValue = "null", expansionFunction = SpecialExpansion.class) + public Void specialExp; } @Test @@ -73,15 +97,14 @@ public class OptionsTest { // TODO(bazel-team): don't include trailing space after last word in line. String input = "The quick brown fox jumps over the lazy dog."; - assertEquals(" The quick \n brown fox \n jumps over \n the lazy \n" - + " dog.", - OptionsUsage.paragraphFill(input, 2, 13)); - assertEquals(" The quick brown \n fox jumps over \n the lazy dog.", - OptionsUsage.paragraphFill(input, 3, 19)); + assertThat(OptionsUsage.paragraphFill(input, 2, 13)) + .isEqualTo(" The quick \n brown fox \n jumps over \n the lazy \n" + " dog."); + assertThat(OptionsUsage.paragraphFill(input, 3, 19)) + .isEqualTo(" The quick brown \n fox jumps over \n the lazy dog."); String input2 = "The quick brown fox jumps\nAnother paragraph."; - assertEquals(" The quick brown fox \n jumps\n Another paragraph.", - OptionsUsage.paragraphFill(input2, 2, 23)); + assertThat(OptionsUsage.paragraphFill(input2, 2, 23)) + .isEqualTo(" The quick brown fox \n jumps\n Another paragraph."); } @Test @@ -90,11 +113,11 @@ public class OptionsTest { String[] remainingArgs = options.getRemainingArgs(); HttpOptions webFlags = options.getOptions(); - assertEquals("www.google.com", webFlags.host); - assertEquals(80, webFlags.port); - assertEquals(false, webFlags.isDebugging); - assertEquals(TriState.AUTO, webFlags.triState); - assertEquals(0, remainingArgs.length); + assertThat(webFlags.host).isEqualTo("www.google.com"); + assertThat(webFlags.port).isEqualTo(80); + assertThat(webFlags.isDebugging).isEqualTo(false); + assertThat(webFlags.triState).isEqualTo(TriState.AUTO); + assertThat(remainingArgs.length).isEqualTo(0); } @Test @@ -110,28 +133,27 @@ public class OptionsTest { String toString = left.toString(); // Don't rely on Set.toString iteration order: - assertTrue(toString.startsWith( - "com.google.devtools.common.options.OptionsTest" - + "$HttpOptions{")); - assertTrue(toString.contains("host=foo")); - assertTrue(toString.contains("port=80")); - assertTrue(toString.endsWith("}")); + assertThat(toString) + .startsWith("com.google.devtools.common.options.OptionsTest" + "$HttpOptions{"); + assertThat(toString).contains("host=foo"); + assertThat(toString).contains("port=80"); + assertThat(toString).endsWith("}"); new EqualsTester().addEqualityGroup(left).testEquals(); - assertTrue(left.toString().equals(likeLeft.toString())); - assertTrue(left.equals(likeLeft)); - assertTrue(likeLeft.equals(left)); - assertFalse(left.equals(right)); - assertFalse(right.equals(left)); - assertFalse(left.equals(null)); - assertFalse(likeLeft.equals(null)); - assertEquals(likeLeft.hashCode(), likeLeft.hashCode()); - assertEquals(left.hashCode(), likeLeft.hashCode()); + assertThat(left.toString()).isEqualTo(likeLeft.toString()); + assertThat(left).isEqualTo(likeLeft); + assertThat(likeLeft).isEqualTo(left); + assertThat(left).isNotEqualTo(right); + assertThat(right).isNotEqualTo(left); + assertThat(left).isNotEqualTo(null); + assertThat(likeLeft).isNotEqualTo(null); + assertThat(likeLeft.hashCode()).isEqualTo(likeLeft.hashCode()); + assertThat(likeLeft.hashCode()).isEqualTo(left.hashCode()); // Strictly speaking this is not required for hashCode to be correct, // but a good hashCode should be different at least for some values. So, // we're making sure that at least this particular pair of inputs yields // different values. - assertFalse(left.hashCode() == right.hashCode()); + assertThat(left.hashCode()).isNotEqualTo(right.hashCode()); } @Test @@ -141,11 +163,12 @@ public class OptionsTest { String[] args2 = { "-p", "80", "--host", "foo" }; HttpOptions options2 = Options.parse(HttpOptions.class, args2).getOptions(); - assertEquals("order/abbreviations shouldn't matter", options1, options2); + // Order/abbreviations shouldn't matter. + assertThat(options1).isEqualTo(options2); - assertEquals("explicitly setting a default shouldn't matter", - Options.parse(HttpOptions.class, "--port", "80").getOptions(), - Options.parse(HttpOptions.class).getOptions()); + // Explicitly setting a default shouldn't matter. + assertThat(Options.parse(HttpOptions.class, "--port", "80").getOptions()) + .isEqualTo(Options.parse(HttpOptions.class).getOptions()); assertThat(Options.parse(HttpOptions.class, "--port", "3").getOptions()) .isNotEqualTo(Options.parse(HttpOptions.class).getOptions()); @@ -161,10 +184,10 @@ public class OptionsTest { String[] remainingArgs = options.getRemainingArgs(); HttpOptions webFlags = options.getOptions(); - assertEquals("google.com", webFlags.host); - assertEquals(8080, webFlags.port); - assertEquals(true, webFlags.isDebugging); - assertEquals(0, remainingArgs.length); + assertThat(webFlags.host).isEqualTo("google.com"); + assertThat(webFlags.port).isEqualTo(8080); + assertThat(webFlags.isDebugging).isEqualTo(true); + assertThat(remainingArgs.length).isEqualTo(0); } @Test @@ -176,10 +199,10 @@ public class OptionsTest { String[] remainingArgs = options.getRemainingArgs(); HttpOptions webFlags = options.getOptions(); - assertEquals("google.com", webFlags.host); - assertEquals(8080, webFlags.port); - assertEquals(true, webFlags.isDebugging); - assertEquals(0, remainingArgs.length); + assertThat(webFlags.host).isEqualTo("google.com"); + assertThat(webFlags.port).isEqualTo(8080); + assertThat(webFlags.isDebugging).isEqualTo(true); + assertThat(remainingArgs.length).isEqualTo(0); } @Test @@ -187,8 +210,8 @@ public class OptionsTest { Options<HttpOptions> options = Options.parse(HttpOptions.class, new String[]{"--nodebug", "--notristate"}); HttpOptions webFlags = options.getOptions(); - assertEquals(false, webFlags.isDebugging); - assertEquals(TriState.NO, webFlags.triState); + assertThat(webFlags.isDebugging).isEqualTo(false); + assertThat(webFlags.triState).isEqualTo(TriState.NO); } @Test @@ -196,8 +219,8 @@ public class OptionsTest { Options<HttpOptions> options = Options.parse(HttpOptions.class, new String[]{"--no_debug", "--no_tristate"}); HttpOptions webFlags = options.getOptions(); - assertEquals(false, webFlags.isDebugging); - assertEquals(TriState.NO, webFlags.triState); + assertThat(webFlags.isDebugging).isEqualTo(false); + assertThat(webFlags.triState).isEqualTo(TriState.NO); } @Test @@ -205,8 +228,8 @@ public class OptionsTest { Options<HttpOptions> options = Options.parse(HttpOptions.class, new String[]{"-d-", "-t-"}); HttpOptions webFlags = options.getOptions(); - assertEquals(false, webFlags.isDebugging); - assertEquals(TriState.NO, webFlags.triState); + assertThat(webFlags.isDebugging).isEqualTo(false); + assertThat(webFlags.triState).isEqualTo(TriState.NO); } @Test @@ -214,8 +237,8 @@ public class OptionsTest { Options<HttpOptions> options = Options.parse(HttpOptions.class, new String[]{"--debug=0", "--tristate=0"}); HttpOptions webFlags = options.getOptions(); - assertEquals(false, webFlags.isDebugging); - assertEquals(TriState.NO, webFlags.triState); + assertThat(webFlags.isDebugging).isEqualTo(false); + assertThat(webFlags.triState).isEqualTo(TriState.NO); } @Test @@ -223,8 +246,8 @@ public class OptionsTest { Options<HttpOptions> options = Options.parse(HttpOptions.class, new String[]{"--debug=1", "--tristate=1"}); HttpOptions webFlags = options.getOptions(); - assertEquals(true, webFlags.isDebugging); - assertEquals(TriState.YES, webFlags.triState); + assertThat(webFlags.isDebugging).isEqualTo(true); + assertThat(webFlags.triState).isEqualTo(TriState.YES); } @Test @@ -232,7 +255,7 @@ public class OptionsTest { String[] args = {"these", "aint", "options"}; Options<HttpOptions> options = Options.parse(HttpOptions.class, args); String[] remainingArgs = options.getRemainingArgs(); - assertEquals(asList(args), asList(remainingArgs)); + assertThat(asList(remainingArgs)).isEqualTo(asList(args)); } @Test @@ -246,7 +269,7 @@ public class OptionsTest { String[] notoptions = {"notta", "option" }; Options<HttpOptions> options = Options.parse(HttpOptions.class, args); String[] remainingArgs = options.getRemainingArgs(); - assertEquals(asList(notoptions), asList(remainingArgs)); + assertThat(asList(remainingArgs)).isEqualTo(asList(notoptions)); } @Test @@ -256,7 +279,7 @@ public class OptionsTest { Options.parse(HttpOptions.class, args); fail(); } catch (OptionsParsingException e) { - assertEquals("Unrecognized option: --unknown", e.getMessage()); + assertThat(e.getMessage()).isEqualTo("Unrecognized option: --unknown"); } } @@ -267,7 +290,7 @@ public class OptionsTest { Options.parse(HttpOptions.class, args); fail(); } catch (OptionsParsingException e) { - assertEquals("Expected value after --port", e.getMessage()); + assertThat(e.getMessage()).isEqualTo("Expected value after --port"); } } @@ -276,7 +299,7 @@ public class OptionsTest { String[] args = {"--port=80", "--port", "81"}; Options<HttpOptions> options = Options.parse(HttpOptions.class, args); HttpOptions webFlags = options.getOptions(); - assertEquals(81, webFlags.port); + assertThat(webFlags.port).isEqualTo(81); } @Test @@ -284,7 +307,7 @@ public class OptionsTest { String[] args = {"--port=80", "-p", "81"}; Options<HttpOptions> options = Options.parse(HttpOptions.class, args); HttpOptions webFlags = options.getOptions(); - assertEquals(81, webFlags.port); + assertThat(webFlags.port).isEqualTo(81); } @Test @@ -300,8 +323,10 @@ public class OptionsTest { Options.parse(HttpOptions.class, new String[]{"--debug=not_a_boolean"}); fail(); } catch (OptionsParsingException e) { - assertEquals("While parsing option --debug=not_a_boolean: " - + "\'not_a_boolean\' is not a boolean", e.getMessage()); + assertThat(e.getMessage()) + .isEqualTo( + "While parsing option --debug=not_a_boolean: " + + "\'not_a_boolean\' is not a boolean"); } } @@ -311,29 +336,38 @@ public class OptionsTest { Options.parse(HttpOptions.class, new String[]{"--nodebug=1"}); fail(); } catch (OptionsParsingException e) { - assertEquals("Unexpected value after boolean option: --nodebug=1", e.getMessage()); + assertThat(e.getMessage()).isEqualTo("Unexpected value after boolean option: --nodebug=1"); } } @Test - public void usageForBuiltinTypes() { + public void usageForBuiltinTypesNoExpansion() { String usage = Options.getUsage(HttpOptions.class); // We can't rely on the option ordering. - assertTrue(usage.contains( - " --[no]debug [-d] (a boolean; default: \"false\")\n" + - " debug")); - assertTrue(usage.contains( - " --host (a string; default: \"www.google.com\")\n" + - " The URL at which the server will be running.")); - assertTrue(usage.contains( - " --port [-p] (an integer; default: \"80\")\n" + - " The port at which the server will be running.")); - assertTrue(usage.contains( - " --special\n" + - " Expands to: --host=special.google.com --port=8080")); - assertTrue(usage.contains( - " --[no]tristate [-t] (a tri-state (auto, yes, no); default: \"auto\")\n" + - " tri-state option returning auto by default")); + assertThat(usage) + .contains(" --[no]debug [-d] (a boolean; default: \"false\")\n" + " debug"); + assertThat(usage) + .contains( + " --host (a string; default: \"www.google.com\")\n" + + " The URL at which the server will be running."); + assertThat(usage) + .contains( + " --port [-p] (an integer; default: \"80\")\n" + + " The port at which the server will be running."); + assertThat(usage) + .contains( + " --[no]tristate [-t] (a tri-state (auto, yes, no); default: \"auto\")\n" + + " tri-state option returning auto by default"); + } + + @Test + public void usageForExpansion() { + String usage = Options.getUsage(HttpOptions.class); + assertThat(usage) + .contains(" --special\n Expands to: --host=special.google.com --port=8080"); + // Expansion functions aren't evaluated since we're just grabbing the usage for an OptionsBase + // subclass and not for a completed parser. The completed case is covered in OptionsParserTest. + assertThat(usage).contains(" --specialexp\n Expands to unknown options."); } public static class NullTestOptions extends OptionsBase { @@ -352,13 +386,13 @@ public class OptionsTest { @Test public void usageForNullDefault() { String usage = Options.getUsage(NullTestOptions.class); - assertTrue(usage.contains( - " --host (a string; default: see description)\n" + - " The URL at which the server will be running.")); - assertTrue(usage.contains( - " --none\n" + - " An expanded option.\n" + - " Expands to: --host=www.google.com")); + assertThat(usage) + .contains( + " --host (a string; default: see description)\n" + + " The URL at which the server will be running."); + assertThat(usage) + .contains( + " --none\n" + " An expanded option.\n" + " Expands to: --host=www.google.com"); } public static class MyURLConverter implements Converter<URL> { @@ -394,7 +428,7 @@ public class OptionsTest { Options<UsesCustomConverter> options = Options.parse(UsesCustomConverter.class, new String[0]); URL expected = new URL("http://www.google.com/"); - assertEquals(expected, options.getOptions().url); + assertThat(options.getOptions().url).isEqualTo(expected); } @Test @@ -404,17 +438,18 @@ public class OptionsTest { Options.parse(UsesCustomConverter.class, args); fail(); } catch (OptionsParsingException e) { - assertEquals("While parsing option --url=a_malformed:url: " - + "Could not convert 'a_malformed:url': " - + "no protocol: a_malformed:url", e.getMessage()); + assertThat(e.getMessage()) + .isEqualTo( + "While parsing option --url=a_malformed:url: " + + "Could not convert 'a_malformed:url': " + + "no protocol: a_malformed:url"); } } @Test public void usageWithCustomConverter() { - assertEquals( - " --url (a url; default: \"http://www.google.com/\")\n", - Options.getUsage(UsesCustomConverter.class)); + assertThat(Options.getUsage(UsesCustomConverter.class)) + .isEqualTo(" --url (a url; default: \"http://www.google.com/\")\n"); } @Test @@ -423,7 +458,7 @@ public class OptionsTest { Options.parse(HttpOptions.class, new String[]{"--no-debug"}); fail(); } catch (OptionsParsingException e) { - assertEquals("Unrecognized option: --no-debug", e.getMessage()); + assertThat(e.getMessage()).isEqualTo("Unrecognized option: --no-debug"); } } @@ -434,7 +469,7 @@ public class OptionsTest { @Test public void nullDefaultForReferenceTypeOption() throws Exception { J options = Options.parse(J.class, NO_ARGS).getOptions(); - assertNull(options.string); + assertThat(options.string).isNull(); } public static class K extends OptionsBase { @@ -451,9 +486,10 @@ public class OptionsTest { Options.parse(K.class, NO_ARGS).getOptions(); fail(); } catch (IllegalStateException e) { - assertEquals("OptionsParsingException while retrieving default for " - + "int1: 'null' is not an int", - e.getMessage()); + assertThat(e.getMessage()) + .isEqualTo( + "OptionsParsingException while retrieving default for " + + "int1: 'null' is not an int"); } } @@ -463,30 +499,30 @@ public class OptionsTest { HttpOptions options = Options.parse(HttpOptions.class, new String[] { "--host", "null" }).getOptions(); - assertEquals("null", options.host); + assertThat(options.host).isEqualTo("null"); } @Test public void nonDecimalRadicesForIntegerOptions() throws Exception { Options<HttpOptions> options = Options.parse(HttpOptions.class, new String[] { "--port", "0x51"}); - assertEquals(81, options.getOptions().port); + assertThat(options.getOptions().port).isEqualTo(81); } @Test public void expansionOptionSimple() throws Exception { Options<HttpOptions> options = Options.parse(HttpOptions.class, new String[] {"--special"}); - assertEquals("special.google.com", options.getOptions().host); - assertEquals(8080, options.getOptions().port); + assertThat(options.getOptions().host).isEqualTo("special.google.com"); + assertThat(options.getOptions().port).isEqualTo(8080); } @Test public void expansionOptionOverride() throws Exception { Options<HttpOptions> options = Options.parse(HttpOptions.class, new String[] {"--port=90", "--special", "--host=foo"}); - assertEquals("foo", options.getOptions().host); - assertEquals(8080, options.getOptions().port); + assertThat(options.getOptions().host).isEqualTo("foo"); + assertThat(options.getOptions().port).isEqualTo(8080); } @Test @@ -495,6 +531,14 @@ public class OptionsTest { Options.parse(HttpOptions.class, new String[] { "--host=special.google.com", "--port=8080"}); Options<HttpOptions> options2 = Options.parse(HttpOptions.class, new String[] { "--special" }); - assertEquals(options1.getOptions(), options2.getOptions()); + assertThat(options1.getOptions()).isEqualTo(options2.getOptions()); + } + + @Test + public void expansionFunction() throws Exception { + Options<HttpOptions> options1 = Options.parse(HttpOptions.class, new String[] {"--specialexp"}); + Options<HttpOptions> options2 = + Options.parse(HttpOptions.class, new String[] {"--specialexp_foo", "--specialexp_bar"}); + assertThat(options1.getOptions()).isEqualTo(options2.getOptions()); } } |