Ability to filter list options.

Generalizes the "EXTEND" functionality of ListValueComponent to "MODIFY".
Each ListValueComponent now stores a list of appends and a list of filters,
and these get applied when the final `val` is requested.

Introduces two new forms of syntax for values of list options:

1. -['foo', 'bar'] filters those values from the running list computation.
2. ...,...,... splits the value on the commas and merges the results of evaluating the ....

The latter allows you to both append to and filter a list in, say, pants.ini:

```
list_option: -[1,2,3],+[4,5,6]

```

Testing Done:
Added tests of the new syntax and functionality. All options tests pass.
Full CI passes here: http://jenkins.pantsbuild.org/job/pantsbuild/job/pants/branch/PR-3583/

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

src/docs/options.md

@@ -75,20 +75,35 @@ Surround strings with single or double quotes if they contain embedded spaces: `
 
 ### List Options
 
-List options can be appended to, as well as overridden. For example, for an option `--foo`
-whose default value is `[1, 2]`, then in `pants.ini`:
+List options can be appended to and filtered, as well as overridden.
+For example, for an option `--foo` whose default value is `[1, 2]`, then in `pants.ini`:
 
 + `foo: 3` will yield `[1, 2, 3]`.
 + `foo: +[3, 4]` will yield `[1, 2, 3, 4]`.
++ `foo: -[1]` will yield `[2]`.
 + `foo: [3, 4]` will yield `[3, 4]`.
 
-On the command line you can append multiple times:
+Multiple append and filter expressions may be delimited with commas,
+allowing you to append and filter simultaneously:
+
++ `foo: +[3,4],-[1]` will yield `[2, 3, 4]`.
+
+On the command line you can append single values multiple times:
 
 + `--foo=3 --foo=4` will yield the value `[1, 2, 3, 4]`.
 
-Note that the command line values will be appended to the value determined from the defaults
+Note that these command line values will be appended to the value determined from the defaults
 plus the values in `pants.ini`. To override the value, use `--foo=[3, 4]`.
 
+Filters apply to the entire list constructed so far, and will filter all appearances of the value:
+
++ `--foo=1 --foo=1 --foo=2 --foo=-[1]` will yield `[2, 2]`.
+
+Filters take precedence over appends, so you cannot "add something back in":
+
++ `--foo=-[2] --foo=2` will yield `[1]`.
+
+
 ### Dict Options
 
 Dict option values are Python-style dict literals: `--foo={"a":1,"b":2}`.

src/python/pants/option/BUILD

@@ -11,6 +11,7 @@ python_library(
     'src/python/pants/base:build_environment',
     'src/python/pants/base:deprecated',
     'src/python/pants/util:eval',
+    'src/python/pants/util:memo',
     'src/python/pants/util:meta',
     'src/python/pants/util:strutil',
   ]

src/python/pants/option/custom_types.py

@@ -5,10 +5,13 @@
 from __future__ import (absolute_import, division, generators, nested_scopes, print_function,
                         unicode_literals, with_statement)
 
+import re
+
 import six
 
 from pants.option.errors import ParseError
 from pants.util.eval import parse_expression
+from pants.util.memo import memoized_method
 from pants.util.strutil import ensure_text
 
 
@@ -98,11 +101,36 @@ class ListValueComponent(object):
 
   One or more instances of this class can be merged to form a list value.
 
-  Each component may either replace or extend the preceding component.  So that, e.g., a cmd-line
-  flag can append to the value specified in the config file, instead of having to repeat it.
+  A component consists of values to append and values to filter while constructing the final list.
+
+  Each component may either replace or modify the preceding component.  So that, e.g., a config
+  file can append to and/or filter the default value list, instead of having to repeat most
+  of the contents of the default value list.
   """
   REPLACE = 'REPLACE'
-  EXTEND = 'EXTEND'
+  MODIFY = 'MODIFY'
+
+  # We use a regex to parse the comma-separated lists of modifier expressions (each of which is
+  # a list or tuple literal preceded by a + or a -).  Note that these expressions are technically
+  # a context-free grammar, but in practice using this regex as a heuristic will work fine. The
+  # values that could defeat it are extremely unlikely to be encountered in practice.
+  # If we do ever encounter them, we'll have to replace this with a real parser.
+  @classmethod
+  @memoized_method
+  def _get_modifier_expr_re(cls):
+    # Note that the regex consists of a positive lookbehind assertion for a ] or a ),
+    # followed by a comma (possibly surrounded by whitespace), followed by a
+    # positive lookahead assertion for [ or (.  The lookahead/lookbehind assertions mean that
+    # the bracket/paren characters don't get consumed in the split.
+    return re.compile(r'(?<=\]|\))\s*,\s*(?=[+-](?:\[|\())')
+
+  @classmethod
+  def _split_modifier_expr(cls, s):
+    # This check ensures that the first expression (before the first split point) is a modification.
+    if s.startswith('+') or s.startswith('-'):
+      return cls._get_modifier_expr_re().split(s)
+    else:
+      return [s]
 
   @classmethod
   def merge(cls, components):
@@ -114,23 +142,35 @@ def merge(cls, components):
     :return: An instance representing the result of merging the components.
     :rtype: `ListValueComponent`
     """
-    # Note that action of the merged component is EXTEND until the first REPLACE is encountered.
+    # Note that action of the merged component is MODIFY until the first REPLACE is encountered.
     # This guarantees associativity.
-    action = cls.EXTEND
-    val = []
+    action = cls.MODIFY
+    appends = []
+    filters = []
     for component in components:
-      if component.action is cls.REPLACE:
-        val = component.val
+      if component._action is cls.REPLACE:
+        appends = component._appends
+        filters = component._filters
         action = cls.REPLACE
-      elif component.action is cls.EXTEND:
-        val.extend(component.val)
+      elif component._action is cls.MODIFY:
+        appends.extend(component._appends)
+        filters.extend(component._filters)
       else:
         raise ParseError('Unknown action for list value: {}'.format(component.action))
-    return cls(action, val)
+    return cls(action, appends, filters)
 
-  def __init__(self, action, val):
-    self.action = action
-    self.val = val
+  def __init__(self, action, appends, filters):
+    self._action = action
+    self._appends = appends
+    self._filters = filters
+
+  @property
+  def val(self):
+    ret = list(self._appends)
+    for x in self._filters:
+      # Note: can't do ret.remove(x) because that only removes the first instance of x.
+      ret = [y for y in ret if y != x]
+    return ret
 
   @classmethod
   def create(cls, value):
@@ -139,34 +179,42 @@ 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 of a list or tuple (possibly prefixed by + or -
+                  indicating modification instead of replacement), or any allowed member_type.
+                  May also be a comma-separated sequence of modifications.
     :rtype: `ListValueComponent`
     """
     if isinstance(value, six.string_types):
       value = ensure_text(value)
+      comma_separated_exprs = cls._split_modifier_expr(value)
+      if len(comma_separated_exprs) > 1:
+        return cls.merge([cls.create(x) for x in comma_separated_exprs])
+
+    action = cls.MODIFY
+    appends = []
+    filters = []
     if isinstance(value, cls):  # Ensure idempotency.
-      action = value.action
-      val = value.val
+      action = value._action
+      appends = value._appends
+      filters = value._filters
     elif isinstance(value, (list, tuple)):  # Ensure we can handle list-typed default values.
       action = cls.REPLACE
-      val = value
+      appends = value
     elif value.startswith('[') or value.startswith('('):
       action = cls.REPLACE
-      val = _convert(value, (list, tuple))
+      appends = _convert(value, (list, tuple))
     elif value.startswith('+[') or value.startswith('+('):
-      action = cls.EXTEND
-      val = _convert(value[1:], (list, tuple))
+      appends = _convert(value[1:], (list, tuple))
+    elif value.startswith('-[') or value.startswith('-('):
+      filters = _convert(value[1:], (list, tuple))
     elif isinstance(value, six.string_types):
-      action = cls.EXTEND
-      val = [value]
+      appends = [value]
     else:
-      action = cls.EXTEND
-      val = _convert('[{}]'.format(value), list)
-    return cls(action, list(val))
+      appends = _convert('[{}]'.format(value), list)
+    return cls(action, list(appends), list(filters))
 
   def __repr__(self):
-    return b'{} {}'.format(self.action, self.val)
+    return b'{} +{} -{}'.format(self._action, self._appends, self._filters)
 
 
 class DictValueComponent(object):

tests/python/pants_test/option/test_custom_types.py

@@ -8,7 +8,9 @@
 import unittest
 from textwrap import dedent
 
-from pants.option.custom_types import dict_option, list_option
+import pytest
+
+from pants.option.custom_types import ListValueComponent, dict_option, list_option
 from pants.option.errors import ParseError
 from pants.util.strutil import ensure_binary
 
@@ -29,6 +31,9 @@ def _do_test_dict_error(self, s):
     with self.assertRaises(ParseError):
       self._do_test({}, s)
 
+  def _do_split(self, expr, expected):
+    self.assertEqual(expected, ListValueComponent._split_modifier_expr(expr))
+
   def test_dict(self):
     self._do_test({}, '{}')
     self._do_test({'a': 'b'}, '{ "a": "b" }')
@@ -57,6 +62,48 @@ def test_list(self):
     self._do_test([1, 2], '+[1,2]')
     self._do_test(['\\'], '\\')
 
+  def test_split_list_modifier_expressions(self):
+    self._do_split('1', ['1'])
+    self._do_split('foo', ['foo'])
+    self._do_split('1,2', ['1,2'])
+    self._do_split('[1,2]', ['[1,2]'])
+    self._do_split('[1,2],[3,4]', ['[1,2],[3,4]'])
+    self._do_split('+[1,2],[3,4]', ['+[1,2],[3,4]'])
+    self._do_split('[1,2],-[3,4]', ['[1,2],-[3,4]'])
+    self._do_split('+[1,2],foo', ['+[1,2],foo'])
+
+    self._do_split('+[1,2],-[3,4]', ['+[1,2]', '-[3,4]'])
+    self._do_split('-[1,2],+[3,4]', ['-[1,2]', '+[3,4]'])
+    self._do_split('-[1,2],+[3,4],-[5,6],+[7,8]', ['-[1,2]', '+[3,4]', '-[5,6]', '+[7,8]'])
+    self._do_split('+[-1,-2],-[-3,-4]', ['+[-1,-2]', '-[-3,-4]'])
+    self._do_split('+["-"],-["+"]', ['+["-"]', '-["+"]'])
+    self._do_split('+["+[3,4]"],-["-[4,5]"]', ['+["+[3,4]"]', '-["-[4,5]"]'])
+
+    # Spot-check that this works with literal tuples as well as lists.
+    self._do_split('+(1,2),-(3,4)', ['+(1,2)', '-(3,4)'])
+    self._do_split('-[1,2],+[3,4],-(5,6),+[7,8]', ['-[1,2]', '+[3,4]', '-(5,6)', '+[7,8]'])
+    self._do_split('+(-1,-2),-[-3,-4]', ['+(-1,-2)', '-[-3,-4]'])
+    self._do_split('+("+(3,4)"),-("-(4,5)")', ['+("+(3,4)")', '-("-(4,5)")'])
+
+    # Check that whitespace around the comma is OK.
+    self._do_split('+[1,2] , -[3,4]', ['+[1,2]', '-[3,4]'])
+    self._do_split('+[1,2]    ,-[3,4]', ['+[1,2]', '-[3,4]'])
+    self._do_split('+[1,2] ,     -[3,4]', ['+[1,2]', '-[3,4]'])
+
+    # We will split some invalid expressions, but that's OK, we'll error out later on the
+    # broken components.
+    self._do_split('+1,2],-[3,4', ['+1,2]','-[3,4'])
+    self._do_split('+(1,2],-[3,4)', ['+(1,2]', '-[3,4)'])
+
+  @pytest.mark.xfail(reason='The heuristic list modifier expression splitter cannot handle '
+                            'certain very unlikely cases.')
+  def test_split_unlikely_list_modifier_expression(self):
+    # Example of the kind of (unlikely) values that will defeat our heuristic, regex-based
+    # splitter of list modifier expressions.
+    funky_string = '],+['
+    self._do_split('+["{}"],-["foo"]'.format(funky_string),
+                   ['+["{}"]'.format(funky_string), '-["foo"]'])
+
   def test_unicode_comments(self):
     """We had a bug where unicode characters in comments would cause the option parser to fail.
 

tests/python/pants_test/option/test_options.py

@@ -339,6 +339,9 @@ def check(expected, args_str, env=None, config=None):
     check([1, 2, 3, 4, 5], './pants --listy=4 --listy=5')
     check([1, 2, 3, 4, 5], './pants --listy=+[4,5]')
 
+    # Filtering from the default.
+    check([1, 3], './pants --listy=-[2]')
+
     # Replacing the default.
     check([4, 5], './pants --listy=[4,5]')
 
@@ -347,8 +350,17 @@ def check(expected, args_str, env=None, config=None):
           env={'PANTS_GLOBAL_LISTY': '+[6,7]'},
           config={'GLOBAL': {'listy': '+[4,5]'}})
 
-    # Overwriting from env, then appending.
-    check([6, 7, 8, 9], './pants --listy=+[8,9]',
+    # Appending and filtering across env, config and flags (in the right order).
+    check([2, 3, 4, 7], './pants --listy=-[1,5,6]',
+          env={'PANTS_GLOBAL_LISTY': '+[6,7]'},
+          config={'GLOBAL': {'listy': '+[4,5]'}})
+
+    check([1, 2, 8, 9], './pants --listy=+[8,9]',
+          env={'PANTS_GLOBAL_LISTY': '-[4,5]'},
+          config={'GLOBAL': {'listy': '+[4,5],-[3]'}})
+
+    # Overwriting from env, then appending and filtering.
+    check([7, 8, 9], './pants --listy=+[8,9],-[6]',
           env={'PANTS_GLOBAL_LISTY': '[6,7]'},
           config={'GLOBAL': {'listy': '+[4,5]'}})
 
@@ -360,7 +372,26 @@ def check(expected, args_str, env=None, config=None):
     # Overwriting from flags.
     check([8, 9], './pants --listy=[8,9]',
           env={'PANTS_GLOBAL_LISTY': '+[6,7]'},
-          config={'GLOBAL': {'listy': '[4,5]'}})
+          config={'GLOBAL': {'listy': '+[4,5],-[8]'}})
+
+    # Filtering all instances of repeated values.
+    check([1, 2, 3, 4, 6], './pants --listy=-[5]',
+          config={'GLOBAL': {'listy': '[1, 2, 5, 3, 4, 5, 6, 5, 5]'}})
+
+    # Filtering a value even though it was appended again at a higher rank.
+    check([1, 2, 3, 5], './pants --listy=+[4]',
+          env={'PANTS_GLOBAL_LISTY': '-[4]'},
+          config={'GLOBAL': {'listy': '+[4, 5]'}})
+
+    # Filtering a value even though it was appended again at the same rank.
+    check([1, 2, 3, 5], './pants',
+          env={'PANTS_GLOBAL_LISTY': '-[4],+[4]'},
+          config={'GLOBAL': {'listy': '+[4, 5]'}})
+
+    # Overwriting cancels filters.
+    check([4], './pants',
+          env={'PANTS_GLOBAL_LISTY': '[4]'},
+          config={'GLOBAL': {'listy': '-[4]'}})
 
   def test_dict_list_option(self):
     def check(expected, args_str, env=None, config=None):