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 5cd6b73c2ce3ad..784ba640d41f7b 100644 --- a/src/main/java/com/google/devtools/common/options/Option.java +++ b/src/main/java/com/google/devtools/common/options/Option.java @@ -63,17 +63,11 @@ * "null" is only applicable when computing the default value; if specified * on the command-line, this string will have its usual literal meaning. * - *

The default value for flags that set allowMultiple to true should be set with - * {@link #defaultMultipleValue()} + *

The default value for flags that set allowMultiple is always the empty + * list and its default value is ignored. */ String defaultValue(); - /** - * This method is an extension of {@link #defaultValue()} and it enables setting default values - * for flags whose allowMultiple is true. In that case {@link #defaultValue()} is ignored. - */ - String[] defaultMultipleValue() default {}; - /** * A string describing the category of options that this belongs to. {@link * OptionsParser#describeOptions} prints options of the same category grouped 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 2683121062900d..0038cad95356b7 100644 --- a/src/main/java/com/google/devtools/common/options/OptionsData.java +++ b/src/main/java/com/google/devtools/common/options/OptionsData.java @@ -144,36 +144,29 @@ private static List getAllAnnotatedFields(Class op } private static Object retrieveDefaultFromAnnotation(Field optionField) { - Option annotation = optionField.getAnnotation(Option.class); - // If an option can be specified multiple times then get the default value from the - // defaultMultipleValue and ignore defaultValue - if (annotation.allowMultiple()) { - // Create a list with each value in defaultMultipleValue converted - String[] defaultMultipleValueString = - OptionsParserImpl.getDefaultMultipleOptionString(optionField); - ImmutableList.Builder builder = new ImmutableList.Builder<>(); - for (String element : defaultMultipleValueString) { - builder.add(convertDefaultValueFromAnnotation(element, optionField)); - } - return builder.build(); + Converter converter = OptionsParserImpl.findConverter(optionField); + String defaultValueAsString = OptionsParserImpl.getDefaultOptionString(optionField); + // Special case for "null" + if (OptionsParserImpl.isSpecialNullDefault(defaultValueAsString, optionField)) { + return null; } - // Otherwise convert the defaultValue - return convertDefaultValueFromAnnotation( - OptionsParserImpl.getDefaultOptionString(optionField), - optionField); - } - - private static Object convertDefaultValueFromAnnotation(String defaultValueString, - Field optionField) { + 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 { - return OptionsParserImpl.isSpecialNullDefault(defaultValueString, optionField) - ? null - : OptionsParserImpl.findConverter(optionField).convert(defaultValueString); + 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> classes) { 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 6f9ca021dc8489..96fb4963fc759d 100644 --- a/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java +++ b/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java @@ -774,11 +774,6 @@ static String getDefaultOptionString(Field optionField) { return annotation.defaultValue(); } - static String[] getDefaultMultipleOptionString(Field optionField) { - Option annotation = optionField.getAnnotation(Option.class); - return annotation.defaultMultipleValue(); - } - static boolean isBooleanField(Field field) { return field.getType().equals(boolean.class) || field.getType().equals(TriState.class) 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 96845a1ff6a3ac..a6ca76ed3d55de 100644 --- a/src/test/java/com/google/devtools/common/options/OptionsParserTest.java +++ b/src/test/java/com/google/devtools/common/options/OptionsParserTest.java @@ -105,22 +105,6 @@ public static class ExampleBoom extends OptionsBase { public String boom; } - /** - * Example with multiple default values - */ - public static class ExampleMultiple extends OptionsBase { - @Option(name = "multiple", - defaultValue = "", - defaultMultipleValue = {"a", "b"}, - allowMultiple = true) - public List multiple; - - @Option(name = "emptyMultiple", - defaultValue = "", - allowMultiple = true) - public List emptyMultiple; - } - public static class StringConverter implements Converter { @Override public String convert(String input) { @@ -239,22 +223,6 @@ public void parseAndOverrideWithEmptyStringToObtainNullValueInOption() assertNull(boom.boom); } - @Test - public void parseWithMultipleDefaultValues() throws OptionsParsingException { - OptionsParser parser = newOptionsParser(ExampleMultiple.class); - parser.parse(); - ExampleMultiple multiple = parser.getOptions(ExampleMultiple.class); - assertThat(multiple.multiple).containsExactly("a", "b"); - } - - @Test - public void parseWithEmptyMultipleDefaultValues() throws OptionsParsingException { - OptionsParser parser = newOptionsParser(ExampleMultiple.class); - parser.parse(); - ExampleMultiple multiple = parser.getOptions(ExampleMultiple.class); - assertThat(multiple.emptyMultiple).isEmpty(); - } - public static class CategoryTest extends OptionsBase { @Option(name = "swiss_bank_account_number", category = "undocumented", // Not printed in usage messages!