2 files changed, 49 insertions, 20 deletions

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 48dac23b80..96fb4963fc 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java
@@ -244,7 +244,7 @@ class OptionsParserImpl {
       = LinkedHashMultimap.create();
 
   private final List<String> warnings = Lists.newArrayList();
-  
+
   private boolean allowSingleDashLongOptions = false;
 
   /**
@@ -261,7 +261,7 @@ class OptionsParserImpl {
   void setAllowSingleDashLongOptions(boolean allowSingleDashLongOptions) {
     this.allowSingleDashLongOptions = allowSingleDashLongOptions;
   }
-  
+
   /**
    * The implementation of {@link OptionsBase#asMap}.
    */
@@ -468,11 +468,6 @@ class OptionsParserImpl {
     }
   }
 
-  private Object getValue(Field field) {
-    ParsedOptionEntry entry = parsedValues.get(field);
-    return entry == null ? null : entry.getValue();
-  }
-
   OptionValueDescription getOptionValueDescription(String name) {
     Field field = optionsData.getFieldFromName(name);
     if (field == null) {
@@ -659,7 +654,7 @@ class OptionsParserImpl {
                 sourceFunction.apply(originalName),
                 expandedFrom == null);
         unparsedValues.add(unparsedOptionValueDescription);
-        if (option.allowMultiple()) {          
+        if (option.allowMultiple()) {
           canonicalizeValues.put(field, unparsedOptionValueDescription);
         } else {
           canonicalizeValues.replaceValues(field, ImmutableList.of(unparsedOptionValueDescription));
@@ -754,9 +749,12 @@ class OptionsParserImpl {
 
     // Set the fields
     for (Field field : optionsData.getFieldsForClass(optionsClass)) {
-      Object value = getValue(field);
-      if (value == null) {
+      Object value;
+      ParsedOptionEntry entry = parsedValues.get(field);
+      if (entry == null) {
         value = optionsData.getDefaultValue(field);
+      } else {
+        value = entry.getValue();
       }
       try {
         field.set(optionsInstance, value);
@@ -814,4 +812,4 @@ class OptionsParserImpl {
       throw new AssertionError(e);
     }
   }
-}
+}
\ No newline at end of file
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 659f77f690..0448471a2f 100644
--- a/src/test/java/com/google/devtools/common/options/OptionsParserTest.java
+++ b/src/test/java/com/google/devtools/common/options/OptionsParserTest.java
@@ -86,6 +86,16 @@ public class OptionsParserTest {
     public String baz;
   }
 
+  /**
+   * Example with empty to null string converter
+   */
+  public static class ExampleBoom extends OptionsBase {
+    @Option(name = "boom",
+            defaultValue = "defaultBoom",
+            converter = EmptyToNullStringConverter.class)
+    public String boom;
+  }
+
   public static class StringConverter implements Converter<String> {
     @Override
     public String convert(String input) {
@@ -97,6 +107,16 @@ public class OptionsParserTest {
     }
   }
 
+  /**
+   * A converter that defaults to null if the input is the empty string
+   */
+  public static class EmptyToNullStringConverter extends StringConverter {
+    @Override
+    public String convert(String input) {
+      return input.isEmpty() ? null : input;
+    }
+  }
+
   @Test
   public void parseWithMultipleOptionsInterfaces()
       throws OptionsParsingException {
@@ -170,6 +190,17 @@ public class OptionsParserTest {
     }
   }
 
+  @Test
+  public void parseAndOverrideWithEmptyStringToObtainNullValueInOption()
+      throws OptionsParsingException {
+    OptionsParser parser = newOptionsParser(ExampleBoom.class);
+    // Override --boom value to the empty string
+    parser.parse("--boom=");
+    ExampleBoom boom = parser.getOptions(ExampleBoom.class);
+    // The converted value is intentionally null since boom uses the EmptyToNullStringConverter
+    assertNull(boom.boom);
+  }
+
   public static class CategoryTest extends OptionsBase {
     @Option(name = "swiss_bank_account_number",
             category = "undocumented", // Not printed in usage messages!
@@ -1029,11 +1060,11 @@ public class OptionsParserTest {
     assertEquals(100, result.longval);
   }
 
-  public static class OldNameExample extends OptionsBase { 
+  public static class OldNameExample extends OptionsBase {
     @Option(name = "new_name",
             oldName = "old_name",
             defaultValue = "defaultValue")
-    public String flag; 
+    public String flag;
   }
 
   @Test
@@ -1057,13 +1088,13 @@ public class OptionsParserTest {
     assertEquals(
         Arrays.asList("--new_name=foo"), canonicalize(OldNameExample.class, "--old_name=foo"));
   }
-  
-  public static class OldNameConflictExample extends OptionsBase { 
+
+  public static class OldNameConflictExample extends OptionsBase {
     @Option(name = "new_name",
             oldName = "old_name",
             defaultValue = "defaultValue")
-    public String flag1; 
-    
+    public String flag1;
+
     @Option(name = "old_name",
             defaultValue = "defaultValue")
     public String flag2;
@@ -1078,8 +1109,8 @@ public class OptionsParserTest {
       // expected
     }
   }
-  
-  public static class WrapperOptionExample extends OptionsBase { 
+
+  public static class WrapperOptionExample extends OptionsBase {
     @Option(name = "wrapper",
             defaultValue = "null",
             wrapperOption = true)
@@ -1094,7 +1125,7 @@ public class OptionsParserTest {
     @Option(name = "flag3", defaultValue = "foo")
     public String flag3;
   }
-  
+
   @Test
   public void testWrapperOption() throws OptionsParsingException {
     OptionsParser parser = newOptionsParser(WrapperOptionExample.class);