Delete the defaultMultipleValue field from options and refactor the l…

…ogic for retrieving

the default values of options.

The field defaultMultipleValue was introduced in commit 51a491b to allow defining a
default value for options that set allowMultiple. However due to the limitations of
the optionsParser end up being not useful since we cannot guarantee that an option
that allows multiple has a converter that returns a list of values.
Thus this CL deletes code that may confuse even more and clarifies the mechanism
that the options currently use to obtain their default values.

--
MOS_MIGRATED_REVID=120317261

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.
    *
-   * <p>The default value for flags that set allowMultiple to true should be set with
-   * {@link #defaultMultipleValue()}
+   * <p>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

src/main/java/com/google/devtools/common/options/OptionsData.java

@@ -144,36 +144,29 @@ private static List<Field> getAllAnnotatedFields(Class<? extends OptionsBase> 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<Object> 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<Class<? extends OptionsBase>> classes) {

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)

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<String> multiple;
-
-    @Option(name = "emptyMultiple",
-        defaultValue = "",
-        allowMultiple = true)
-    public List<String> emptyMultiple;
-  }
-
   public static class StringConverter implements Converter<String> {
     @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!