Allow updating dict option values instead of replacing them.

Similarly to list options, this is done by preceding the dict
literal with a '+'.

This allows, e.g., adding new Go matcher patterns in pants.ini
without having to copy the default ones.

Note that this has the side effect of making the implicit default
value the empty dict, instead of None, which is something we've
wanted for a while. So now code using dict-valued options won't
need to check for None.

This implementation is simpler than that of list options, because
lists have the added complication of interpreting --foo=bar as
--foo=+[bar], which is irrelevant here.

Testing Done:
CI passes: http://jenkins.pantsbuild.org/job/pantsbuild/job/pants/branch/PR-3449/4/

Reviewed at https://rbcommons.com/s/twitter/r/3896/

src/python/pants/help/help_info_extracter.py

@@ -73,8 +73,11 @@ def compute_default(kwargs):
     if typ == list:
       default_str = '[{}]'.format(','.join(["'{}'".format(s) for s in default]))
     elif typ == dict:
-      default_str = '{{ {} }}'.format(
-        ','.join(["'{}':'{}'".format(k, v) for k, v in default.items()]))
+      if default:
+        default_str = '{{ {} }}'.format(
+          ','.join(["'{}':'{}'".format(k, v) for k, v in default.items()]))
+      else:
+        default_str = '{}'
     elif typ == str:
       default_str = "'{}'".format(default).replace('\n', ' ')
     else:

src/python/pants/option/custom_types.py

@@ -19,7 +19,7 @@ def dict_option(s):
 
   :API: public
   """
-  return _convert(s, (dict,))
+  return DictValueComponent.create(s)
 
 
 def list_option(s):
@@ -128,8 +128,8 @@ def create(cls, value):
     Note that we accept tuple literals, but the internal value is always a list.
 
     :param value: The value to convert.  Can be an instance of ListValueComponent, a list, a tuple,
-           a string representation (possibly prefixed by +) of a list or tuple, or any allowed
-           member_type.
+                  a string representation (possibly prefixed by +) of a list or tuple, or any
+                  allowed member_type.
     :rtype: `ListValueComponent`
     """
     if isinstance(value, six.string_types):
@@ -156,3 +156,72 @@ def create(cls, value):
 
   def __repr__(self):
     return b'{} {}'.format(self.action, self.val)
+
+
+class DictValueComponent(object):
+  """A component of the value of a dict-typed option.
+
+  One or more instances of this class can be merged to form a dict value.
+
+  Each component may either replace or extend the preceding component.  So that, e.g., a config
+  file can extend the default value of a dict, instead of having to repeat it.
+  """
+  REPLACE = 'REPLACE'
+  EXTEND = 'EXTEND'
+
+  @classmethod
+  def merge(cls, components):
+    """Merges components into a single component, applying their actions appropriately.
+
+    This operation is associative:  M(M(a, b), c) == M(a, M(b, c)) == M(a, b, c).
+
+    :param list components: an iterable of instances of DictValueComponent.
+    :return: An instance representing the result of merging the components.
+    :rtype: `DictValueComponent`
+    """
+    # Note that action of the merged component is EXTEND until the first REPLACE is encountered.
+    # This guarantees associativity.
+    action = cls.EXTEND
+    val = {}
+    for component in components:
+      if component.action is cls.REPLACE:
+        val = component.val
+        action = cls.REPLACE
+      elif component.action is cls.EXTEND:
+        val.update(component.val)
+      else:
+        raise ParseError('Unknown action for dict value: {}'.format(component.action))
+    return cls(action, val)
+
+  def __init__(self, action, val):
+    self.action = action
+    self.val = val
+
+  @classmethod
+  def create(cls, value):
+    """Interpret value as either a dict or something to extend another dict with.
+
+    :param value: The value to convert.  Can be an instance of DictValueComponent, a dict,
+                  or a string representation (possibly prefixed by +) of a dict.
+    :rtype: `DictValueComponent`
+    """
+    if isinstance(value, six.string_types):
+      value = ensure_text(value)
+    if isinstance(value, cls):  # Ensure idempotency.
+      action = value.action
+      val = value.val
+    elif isinstance(value, dict):  # Ensure we can handle dict-typed default values.
+      action = cls.REPLACE
+      val = value
+    elif value.startswith('{'):
+      action = cls.REPLACE
+      val = _convert(value, dict)
+    elif value.startswith('+{'):
+      action = cls.EXTEND
+      val = _convert(value[1:], dict)
+    else:
+      raise ParseError('Invalid dict value: {}'.format(value))
+    return cls(action, dict(val))
+
+  def __repr__(self):
+    return b'{} {}'.format(self.action, self.val)

src/python/pants/option/option_util.py

@@ -11,3 +11,7 @@
 def is_list_option(kwargs):
   return (kwargs.get('action') == 'append' or kwargs.get('type') == list or
           kwargs.get('type') == list_option)
+
+
+def is_dict_option(kwargs):
+  return kwargs.get('type') == dict

src/python/pants/option/parser.py

@@ -15,14 +15,14 @@
 
 from pants.base.deprecated import validate_removal_semver, warn_or_error
 from pants.option.arg_splitter import GLOBAL_SCOPE, GLOBAL_SCOPE_CONFIG_SECTION
-from pants.option.custom_types import (ListValueComponent, dict_option, file_option, list_option,
-                                       target_option)
+from pants.option.custom_types import (DictValueComponent, ListValueComponent, dict_option,
+                                       file_option, list_option, target_option)
 from pants.option.errors import (BooleanOptionNameWithNo, FrozenRegistration, ImplicitValIsNone,
                                  InvalidKwarg, InvalidMemberType, MemberTypeNotAllowed,
                                  NoOptionNames, OptionAlreadyRegistered, OptionNameDash,
                                  OptionNameDoubleDash, ParseError, RecursiveSubsystemOption,
                                  Shadowing)
-from pants.option.option_util import is_list_option
+from pants.option.option_util import is_dict_option, is_list_option
 from pants.option.ranked_value import RankedValue
 from pants.option.scope import ScopeInfo
 
@@ -390,6 +390,13 @@ def _wrap_type(t):
     else:
       return t
 
+  @staticmethod
+  def _convert_member_type(t, x):
+    if t == dict:
+      return dict_option(x).val
+    else:
+      return t(x)
+
   def _compute_value(self, dest, kwargs, flag_val_strs):
     """Compute the value to use for an option.
 
@@ -460,6 +467,10 @@ def expand(val_str):
       # Note: It's important to set flag_val to None if no flags were specified, so we can
       # distinguish between no flags set vs. explicit setting of the value to [].
       flag_val = ListValueComponent.merge(flag_vals) if flag_vals else None
+    elif is_dict_option(kwargs):
+      # Note: It's important to set flag_val to None if no flags were specified, so we can
+      # distinguish between no flags set vs. explicit setting of the value to {}.
+      flag_val = DictValueComponent.merge(flag_vals) if flag_vals else None
     elif len(flag_vals) > 1:
       raise ParseError('Multiple cmd line flags specified for option {} in {}'.format(
           dest, self._scope_str()))
@@ -510,7 +521,14 @@ def check(val):
       merged_rank = ranked_vals[-1].rank
       merged_val = ListValueComponent.merge(
           [rv.value for rv in ranked_vals if rv.value is not None]).val
-      merged_val = [self._wrap_type(kwargs.get('member_type', str))(x) for x in merged_val]
+      merged_val = [self._convert_member_type(kwargs.get('member_type', str), x)
+                    for x in merged_val]
+      map(check, merged_val)
+      ret = RankedValue(merged_rank, merged_val)
+    elif is_dict_option(kwargs):
+      merged_rank = ranked_vals[-1].rank
+      merged_val = DictValueComponent.merge(
+          [rv.value for rv in ranked_vals if rv.value is not None]).val
       map(check, merged_val)
       ret = RankedValue(merged_rank, merged_val)
     else:

tests/python/pants_test/help/test_help_info_extracter.py

@@ -91,8 +91,7 @@ def do_test(args, kwargs, expected_default):
     do_test(['--foo'], {'type': int}, 'None')
     do_test(['--foo'], {'type': int, 'default': 42}, '42')
     do_test(['--foo'], {'type': list}, '[]')
-    # TODO: Change this if we switch the implicit default to {}.
-    do_test(['--foo'], {'type': dict}, 'None')
+    do_test(['--foo'], {'type': dict}, '{}')
 
   def test_deprecated(self):
     kwargs = {'removal_version': '999.99.9', 'removal_hint': 'do not use this'}

tests/python/pants_test/option/test_custom_types.py

@@ -17,7 +17,7 @@ class CustomTypesTest(unittest.TestCase):
 
   def _do_test(self, expected_val, s):
     if isinstance(expected_val, dict):
-      val = dict_option(s)
+      val = dict_option(s).val
     elif isinstance(expected_val, (list, tuple)):
       val = list_option(s).val
     else:

tests/python/pants_test/option/test_options.py

@@ -428,6 +428,25 @@ def check(expected, args_str, env=None, config=None):
     check(['//:c', '//:d'],
           './pants', config={'GLOBAL': {'target_listy': '["//:c", "//:d"]'} })
 
+  def test_dict_option(self):
+    def check(expected, args_str, env=None, config=None):
+      options = self._parse(args_str=args_str, env=env, config=config)
+      self.assertEqual(expected, options.for_global_scope().dicty)
+
+    check({'a': 'b'}, './pants')
+    check({'c': 'd'}, './pants --dicty=\'{"c": "d"}\'')
+    check({'a': 'b', 'c': 'd'}, './pants --dicty=\'+{"c": "d"}\'')
+
+    check({'c': 'd'}, './pants', config={'GLOBAL': {'dicty': '{"c": "d"}'}})
+    check({'a': 'b', 'c': 'd'}, './pants', config={'GLOBAL': {'dicty': '+{"c": "d"}'}})
+    check({'a': 'b', 'c': 'd', 'e': 'f'}, './pants --dicty=\'+{"e": "f"}\'',
+          config={'GLOBAL': {'dicty': '+{"c": "d"}'}})
+
+    # Check that highest rank wins if we have multiple values for the same key.
+    check({'a': 'b+', 'c': 'd'}, './pants', config={'GLOBAL': {'dicty': '+{"a": "b+", "c": "d"}'}})
+    check({'a': 'b++', 'c': 'd'}, './pants --dicty=\'+{"a": "b++"}\'',
+          config={'GLOBAL': {'dicty': '+{"a": "b+", "c": "d"}'}})
+
   def test_defaults(self):
     # Hard-coded defaults.
     options = self._parse('./pants compile.java -n33')