From 9acfaf9462e3e7f128dbcac68714fb9c60a2890c Mon Sep 17 00:00:00 2001
From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com>
Date: Sat, 8 Sep 2018 15:05:37 -0700
Subject: [PATCH 1/6] implement default values for datatypes

---
 src/python/pants/util/objects.py | 321 ++++++++++++++++++++++++++-----
 1 file changed, 274 insertions(+), 47 deletions(-)

diff --git a/src/python/pants/util/objects.py b/src/python/pants/util/objects.py
index a55dc5e1360..d5c7cd89158 100644
--- a/src/python/pants/util/objects.py
+++ b/src/python/pants/util/objects.py
@@ -7,15 +7,18 @@
 import sys
 from abc import abstractmethod
 from builtins import object, zip
-from collections import OrderedDict, namedtuple
+from collections import OrderedDict, deque, namedtuple
 
-from future.utils import PY2
+from future.utils import PY2, text_type
 from twitter.common.collections import OrderedSet
 
-from pants.util.memo import memoized, memoized_classproperty
+from pants.util.memo import memoized, memoized_classmethod, memoized_classproperty
 from pants.util.meta import AbstractClass
 
 
+# TODO: when we can restrict the python version to >= 3.6 in our python 3 shard, we can use the
+# backported dataclasses library as a backend to take advantage of cool python 3 things like type
+# hints (https://github.com/ericvsmith/dataclasses). Python 3.7+ provides dataclasses in the stdlib.
 def datatype(field_decls, superclass_name=None, **kwargs):
   """A wrapper for `namedtuple` that accounts for the type of the object in equality.
 
@@ -30,62 +33,87 @@ def datatype(field_decls, superclass_name=None, **kwargs):
   :return: A type object which can then be subclassed.
   :raises: :class:`TypeError`
   """
-  field_names = []
-  fields_with_constraints = OrderedDict()
+  parsed_field_list = []
+
+  default_values = []
   for maybe_decl in field_decls:
-    # ('field_name', type)
-    if isinstance(maybe_decl, tuple):
-      field_name, type_spec = maybe_decl
-      if isinstance(type_spec, type):
-        type_constraint = Exactly(type_spec)
-      elif isinstance(type_spec, TypeConstraint):
-        type_constraint = type_spec
+    parsed_decl = DatatypeFieldDecl.parse(maybe_decl)
+    if default_values:
+      if not parsed_decl.has_default_value:
+        raise DatatypeFieldDecl.FieldDeclarationError(
+          "datatype field declaration {!r} (parsed into {!r}) must have a default value, "
+          "because it follows a declaration with a default value in the field declarations "
+          "{!r} (the preceding parsed arguments were: {!r})."
+          .format(maybe_decl, parsed_decl, field_decls, parsed_field_list))
       else:
-        raise TypeError(
-          "type spec for field '{}' was not a type or TypeConstraint: was {!r} (type {!r})."
-          .format(field_name, type_spec, type(type_spec).__name__))
-      fields_with_constraints[field_name] = type_constraint
-    else:
-      # interpret it as a field name without a type to check
-      field_name = maybe_decl
-    # namedtuple() already checks field uniqueness
-    field_names.append(field_name)
+        default_values.append(parsed_decl.default_value)
+    elif parsed_decl.has_default_value:
+      # This is the first field declaring a default value.
+      default_values.append(parsed_decl.default_value)
+      # namedtuple() already checks field name uniqueness, so we defer to it checking that here.
+    parsed_field_list.append(parsed_decl)
 
   if not superclass_name:
     superclass_name = '_anonymous_namedtuple_subclass'
 
-  namedtuple_cls = namedtuple(superclass_name, field_names, **kwargs)
+  all_fields_unchecked_names = [p.field_name for p in parsed_field_list]
+  namedtuple_cls = namedtuple(superclass_name, all_fields_unchecked_names, **kwargs)
+
+  # NB: We use `parsed_field_list` above so that namedtuple() can check for duplicated field names,
+  # but we will use `parsed_field_dict` as the source of truth from now on.
+  parsed_field_dict = OrderedDict((p.field_name, p) for p in parsed_field_list)
+
+  # Python makes it very easy to add default values for arguments -- these defaults will apply
+  # regardless of whether the arguments to __new__() are specified positionally or by keyword.
+  if default_values:
+    namedtuple_cls.__new__.__defaults__ = tuple(default_values)
 
   class DataType(namedtuple_cls):
     @classmethod
     def make_type_error(cls, msg, *args, **kwargs):
       return TypeCheckError(cls.__name__, msg, *args, **kwargs)
 
-    def __new__(cls, *args, **kwargs):
-      # TODO: Ideally we could execute this exactly once per `cls` but it should be a
-      # relatively cheap check.
+    @classmethod
+    def _validate_fields(cls, this_object):
+      """Validate the fields of the object satisfy any declared type constraints."""
+      arg_check_error_messages = []
+      for p in parsed_field_dict.values():
+        cur_field_constraint = p.type_constraint
+        if cur_field_constraint is None:
+          continue
+
+        cur_field_name = p.field_name
+        cur_field_value = getattr(this_object, cur_field_name)
+
+        try:
+          cur_field_constraint.validate_satisfied_by(cur_field_value)
+        except TypeError as e:
+          arg_check_error_messages.append(
+            "field '{name}' was invalid: {err}"
+            .format(name=cur_field_name, err=str(e)))
+
+      # TODO(cosmicexplorer): Make this kind of exception pattern (filter for
+      # errors then display them all at once) more ergonomic.
+      if arg_check_error_messages:
+        raise cls.make_type_error('\n'.join(arg_check_error_messages))
+
+      return this_object
+
+    @memoized_classmethod
+    def _eq_canary_check(cls):
+      """Check whether __eq__ has been overridden, memoized to once per `cls`."""
       if not hasattr(cls.__eq__, '_eq_override_canary'):
         raise cls.make_type_error('Should not override __eq__.')
 
+    def __new__(cls, *args, **kwargs):
+      cls._eq_canary_check()
+
       try:
         this_object = super(DataType, cls).__new__(cls, *args, **kwargs)
       except TypeError as e:
         raise cls.make_type_error(e)
 
-      # TODO(cosmicexplorer): Make this kind of exception pattern (filter for
-      # errors then display them all at once) more ergonomic.
-      type_failure_msgs = []
-      for field_name, field_constraint in fields_with_constraints.items():
-        field_value = getattr(this_object, field_name)
-        try:
-          field_constraint.validate_satisfied_by(field_value)
-        except TypeConstraintError as e:
-          type_failure_msgs.append(
-            "field '{}' was invalid: {}".format(field_name, e))
-      if type_failure_msgs:
-        raise cls.make_type_error('\n'.join(type_failure_msgs))
-
-      return this_object
+      return cls._validate_fields(this_object)
 
     def __eq__(self, other):
       if self is other:
@@ -104,7 +132,17 @@ def __ne__(self, other):
       return not (self == other)
 
     def __hash__(self):
-      return super(DataType, self).__hash__()
+      try:
+        return super(DataType, self).__hash__()
+      except TypeError as e:
+        # This gives at least a little more context into what failed. If a datatype() is intended to
+        # be hashable, it needs to ensure its fields are `tuple` or another hashable collection
+        # instead of e.g. `list`. This wrapped error message can help to find the specific
+        # datatype() instance which needs to ensure its fields are hashable.
+        raise self.make_type_error(
+          "Hash failed for object {!r}: {}"
+          .format(self, e),
+          e)
 
     # NB: As datatype is not iterable, we need to override both __iter__ and all of the
     # namedtuple methods that expect self to be iterable.
@@ -124,16 +162,28 @@ def _replace(_self, **kwds):
       field_dict.update(**kwds)
       return type(_self)(**field_dict)
 
-    copy = _replace
+    def copy(self, **kwargs):
+      """Return the result of `self._replace(**kwargs)`.
+
+      This method stub makes error messages provide this method's name, instead of pointing to the
+      private `_replace()`.
+
+      We intentionally accept only keyword arguments to make copy() calls in Python code consuming
+      the datatype remain valid if the datatype's definition is updated, unless a field is removed
+      (which fails loudly and quickly with an unknown keyword argument error).
+      """
+      return self._replace(**kwargs)
 
-    # NB: it is *not* recommended to rely on the ordering of the tuple returned by this method.
+    # NB: it is *not* recommended to rely on the ordering of the tuple returned by this
+    # method. Prefer to access fields by name, as well as construct them by name where that is more
+    # clear.
     def __getnewargs__(self):
       '''Return self as a plain tuple.  Used by copy and pickle.'''
       return tuple(self._super_iter())
 
     def __repr__(self):
       args_formatted = []
-      for field_name in field_names:
+      for field_name in parsed_field_dict.keys():
         field_value = getattr(self, field_name)
         args_formatted.append("{}={!r}".format(field_name, field_value))
       return '{class_name}({args_joined})'.format(
@@ -142,8 +192,8 @@ def __repr__(self):
 
     def __str__(self):
       elements_formatted = []
-      for field_name in field_names:
-        constraint_for_field = fields_with_constraints.get(field_name, None)
+      for field_name, parsed_decl in parsed_field_dict.items():
+        constraint_for_field = parsed_decl.type_constraint
         field_value = getattr(self, field_name)
         if not constraint_for_field:
           elements_formatted.append(
@@ -272,10 +322,23 @@ class TypeConstraintError(TypeError):
 class TypeConstraint(AbstractClass):
   """Represents a type constraint.
 
-  Not intended for direct use; instead, use one of :class:`SuperclassesOf`, :class:`Exact` or
+  Not intended for direct use; instead, use one of :class:`SuperclassesOf`, :class:`Exactly` or
   :class:`SubclassesOf`.
+
+  Override `has_default_value` and `default_value` to provide a default value for the type
+  constraint, as specified in the docstring for :class:`DatatypeFieldDecl`.
+
+  NB: If `has_default_value` is False, `default_value` should not be accessed.
   """
 
+  has_default_value = False
+
+  def default_value(self):
+    raise NotImplementedError(
+      "The type constraint {!r} with type '{}' did not define a `default_value` "
+      "-- is `has_default_value` True?"
+      .format(self, type(self).__name__))
+
   def __init__(self, *types, **kwargs):
     """Creates a type constraint centered around the given types.
 
@@ -316,6 +379,9 @@ def satisfied_by_type(self, obj_type):
     :rtype: bool
     """
 
+  # TODO: This method could be extended to allow for coercion -- we could change the interface to
+  # return None if the constraint is satified, raise on error, or return a new object if coercion
+  # occurs.
   def validate_satisfied_by(self, obj):
     """Return `obj` if the object satisfies this type constraint, or raise.
 
@@ -355,7 +421,7 @@ def __repr__(self):
     else:
       constrained_type = ', '.join(t.__name__ for t in self._types)
     return ('{type_constraint_type}({constrained_type})'
-      .format(type_constraint_type=type(self).__name__,
+            .format(type_constraint_type=type(self).__name__,
                     constrained_type=constrained_type))
 
 
@@ -392,6 +458,167 @@ def satisfied_by_type(self, obj_type):
     return issubclass(obj_type, self._types)
 
 
+class DatatypeFieldDecl(namedtuple('DatatypeFieldDecl', [
+    'field_name',
+    'type_constraint',
+    'default_value',
+    'has_default_value',
+])):
+  """Description of a field, used in calls to datatype().
+
+  All elements of the list passed to datatype() are parsed into instances of this class by the
+  parse() classmethod.
+
+  `default_value` is only relevant if `has_default_value` is True.
+  """
+
+  class FieldDeclarationError(TypeError): pass
+
+  def __new__(cls, field_name, type_constraint=None, **kwargs):
+    """Parse the arguments into a type constraint and a default value.
+
+    If the argument `default_value` is provided (only by keyword), `has_default_value` is set to
+    True, and the argument `default_value` is used as this field's `default_value`. If
+    `default_value` is not provided (as when parsing a field declaration from a tuple), but
+    `type_constraint` is a TypeConstraint with the property `has_default_value` evaluating to True,
+    then `has_default_value` is set to True, and the `type_constraint`'s `default_value` field is
+    used as this field's `default_value`.
+
+    If a `default_value` is provided by either of the above methods, its type is checked here with
+    `type_constraint.validate_satisfied_by(default_value)`, with any type checking errors wrapped in
+    :class:`DatatypeFieldDecl.FieldDeclarationError`. See the documentation for
+    `TypeConstraint.validate_satisfied_by()` for more information on how this is done.
+
+    :param str field_name: Name of the attribute to access the field at.
+    :param type_constraint: If None, the field's type is never checked. If `type_constraint` is a
+                            type, it is converted into a type constraint of `Exactly(<type>)`. If
+                            `type_constraint` is already a `TypeConstraint`, just use that.
+    :param default_value: This argument may only be provided by keyword. See above for details on
+                          how this argument is intepreted.
+    :raises: :class:`DatatypeFieldDecl.FieldDeclarationError` if the field declaration was invalid.
+    """
+    if not isinstance(field_name, text_type):
+      raise cls.FieldDeclarationError(
+        "field_name must be an instance of {!r}, but was instead {!r} (type {!r})."
+        .format(text_type, field_name, type(field_name).__name__))
+
+    # If `default_value` was provided as a keyword argument, get its value and set
+    # `has_default_value`.
+    if 'default_value' in kwargs:
+      has_default_value = True
+      default_value = kwargs.pop('default_value')
+    else:
+      has_default_value = False
+      default_value = None
+    if kwargs:
+      raise cls.FieldDeclarationError("Unrecognized keyword arguments: {!r}".format(kwargs))
+
+    # Parse the `type_constraint` field, and get a `default_value` from it if provided and if
+    # `default_value` was not provided as a keyword argument to this constructor.
+    if type_constraint is None:
+      pass
+    elif isinstance(type_constraint, TypeConstraint):
+      if not has_default_value and type_constraint.has_default_value:
+        has_default_value = True
+        default_value = type_constraint.default_value
+    elif isinstance(type_constraint, type):
+      type_constraint = Exactly(type_constraint)
+    else:
+      raise cls.FieldDeclarationError(
+        "type_constraint for field '{field}' must be an instance of `type` or `TypeConstraint`, "
+        "or else None, but was instead {value!r} (type {the_type!r})."
+        .format(field=field_name, value=type_constraint, the_type=type(type_constraint).__name__))
+
+    # The default value for the field must obey the field's type constraint, if both are
+    # provided. This will error at datatype class creation time if not.
+    if has_default_value and (type_constraint is not None):
+      try:
+        # NB: `TypeConstraint.validate_satisfied_by()` by can change the value of the object.
+        maybe_new_default_value = type_constraint.validate_satisfied_by(default_value)
+        if maybe_new_default_value is not None:
+          default_value = maybe_new_default_value
+      except TypeConstraintError as e:
+        raise cls.FieldDeclarationError(
+          "default_value {default_value!r} for the field '{field_name}' must satisfy the provided "
+          "type_constraint {tc!r}. {err}"
+          .format(default_value=default_value,
+                  field_name=field_name,
+                  tc=type_constraint,
+                  err=str(e)),
+          e)
+
+    return super(DatatypeFieldDecl, cls).__new__(
+      cls, field_name, type_constraint, default_value, has_default_value)
+
+  @classmethod
+  def _parse_tuple(cls, tuple_decl):
+    """Interpret the elements of a tuple (by position) into a field declaration.
+
+    Currently, we try to accept a syntax similar to `typing.NamedTuple` from Python 3 allows for
+    type-annotated fields, to allow for easy interoperability as we move to Python 3. Currently, we
+    accept a tuple with 1 or 2 elements, informally denoted by:
+
+    ('field_name': str, type?: (TypeConstraint | type))
+    """
+    type_spec = None
+    remaining_decl_elements = deque(tuple_decl)
+
+    if not bool(remaining_decl_elements):
+      raise ValueError("Empty tuple {!r} passed to datatype().".format(tuple_decl))
+
+    field_name = text_type(remaining_decl_elements.popleft())
+
+    # A type constraint may optionally be provided, either as a TypeConstraint instance, or as a
+    # type, which is shorthand for Exactly(<type>).
+    if bool(remaining_decl_elements):
+      type_spec = remaining_decl_elements.popleft()
+
+    if bool(remaining_decl_elements):
+      raise ValueError(
+        "There are too many elements of the tuple {!r} passed to datatype(). "
+        "The tuple must have between 1 and 2 elements. The remaining elements were: {!r}."
+        .format(tuple_decl, list(remaining_decl_elements)))
+
+    return cls(field_name=field_name, type_constraint=type_spec)
+
+  @classmethod
+  def parse(cls, maybe_decl):
+    """Interpret `maybe_decl` into a datatype field declaration.
+
+    `maybe_decl` may be:
+    1. 'field_name' => an untyped field named 'field_name'.
+    2. ('field_name',) => an untyped field named 'field_name'.
+    3. ('field_name', type_constraint) => a field named 'field_name' with the given type constraint.
+    4. an instance of this class => return `maybe_decl`.
+
+    The type of `maybe_decl` can be informally denoted by:
+
+    str | DatatypeFieldDecl | (field_name: str, type?: (TypeConstraint | type))
+
+    :raises: :class:`DatatypeFieldDecl.FieldDeclarationError`
+    """
+    if isinstance(maybe_decl, cls):
+      # If already a DatatypeFieldDecl instance, just return it.
+      parsed_decl = maybe_decl
+    elif isinstance(maybe_decl, text_type):
+      # A string alone is interpreted as an untyped field of that name.
+      parsed_decl = cls(field_name=maybe_decl)
+    elif isinstance(maybe_decl, tuple):
+      # A tuple may be provided, whose elements are interpreted into a DatatypeFieldDecl.
+      parsed_decl = cls._parse_tuple(maybe_decl)
+    else:
+      # Unrecognized input.
+      raise cls.FieldDeclarationError(
+        "The field declaration {value!r} must be a {str_type!r}, tuple, "
+        "or {this_type!r} instance, but its type was: {the_type!r}."
+        .format(value=maybe_decl,
+                str_type=text_type,
+                this_type=cls.__name__,
+                the_type=type(maybe_decl).__name__))
+
+    return parsed_decl
+
+
 class Collection(object):
   """Constructs classes representing collections of objects of a particular type."""
   # TODO: could we check that the input is iterable in the ctor?

From 7c64671f459d51ba38175ad4b3584665a0901a89 Mon Sep 17 00:00:00 2001
From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com>
Date: Sat, 8 Sep 2018 15:06:12 -0700
Subject: [PATCH 2/6] test the new default value feature

---
 tests/python/pants_test/util/BUILD           |   2 +-
 tests/python/pants_test/util/test_objects.py | 338 +++++++++++--------
 2 files changed, 189 insertions(+), 151 deletions(-)

diff --git a/tests/python/pants_test/util/BUILD b/tests/python/pants_test/util/BUILD
index 30479649f52..d603c68d1cd 100644
--- a/tests/python/pants_test/util/BUILD
+++ b/tests/python/pants_test/util/BUILD
@@ -110,7 +110,7 @@ python_tests(
   dependencies = [
     '3rdparty/python:future',
     'src/python/pants/util:objects',
-    'tests/python/pants_test:base_test',
+    'tests/python/pants_test:test_base',
   ]
 )
 
diff --git a/tests/python/pants_test/util/test_objects.py b/tests/python/pants_test/util/test_objects.py
index 0fa1c9b6b15..39be1efef0c 100644
--- a/tests/python/pants_test/util/test_objects.py
+++ b/tests/python/pants_test/util/test_objects.py
@@ -10,14 +10,14 @@
 from abc import abstractmethod
 from builtins import object, str
 
-from future.utils import PY2, PY3, text_type
+from future.utils import PY3, text_type
 
-from pants.util.objects import (Exactly, SubclassesOf, SuperclassesOf, TypeCheckError,
-                                TypedDatatypeInstanceConstructionError, datatype, enum)
-from pants_test.base_test import BaseTest
+from pants.util.objects import DatatypeFieldDecl as F
+from pants.util.objects import Exactly, SubclassesOf, SuperclassesOf, TypeCheckError, datatype, enum
+from pants_test.test_base import TestBase
 
 
-class TypeConstraintTestBase(BaseTest):
+class TypeConstraintTestBase(TestBase):
   class A(object):
     pass
 
@@ -138,6 +138,8 @@ def stripped(self):
     return self.as_str().strip()
 
 
+# These datatype() calls with strings or tuples for fields are interpreted into a DatatypeFieldDecl
+# by DatatypeFieldDecl.parse().
 class TypedWithMixin(datatype([('val', text_type)]), SomeMixin):
   """Example of using `datatype()` with a mixin."""
 
@@ -154,6 +156,17 @@ class WithExplicitTypeConstraint(datatype([('a_string', text_type), ('an_int', E
 class MixedTyping(datatype(['value', ('name', text_type)])): pass
 
 
+# `F` is what we imported `pants.util.objects.DatatypeFieldDecl` as.
+class WithDefaultValueTuple(datatype([
+    F('an_int', int, default_value=3),
+])): pass
+
+
+class WithJustDefaultValueExplicitFieldDecl(datatype([
+    F('a_bool', Exactly(bool), default_value=True),
+])): pass
+
+
 class SomeBaseClass(object):
   @abstractmethod
   def something(self): pass
@@ -199,7 +212,7 @@ def __eq__(self, other):
 class SomeEnum(enum('x', [1, 2])): pass
 
 
-class DatatypeTest(BaseTest):
+class DatatypeTest(TestBase):
 
   def test_eq_with_not_implemented_super(self):
     class DatatypeSuperNotImpl(datatype(['val']), ReturnsNotImplemented, tuple):
@@ -303,22 +316,19 @@ def test_unexpect_kwarg(self):
       bar(other=1)
 
 
-class TypedDatatypeTest(BaseTest):
+class TypedDatatypeTest(TestBase):
+
+  unicode_literal = '' if PY3 else 'u'
 
   def test_class_construction_errors(self):
     # NB: datatype subclasses declared at top level are the success cases
     # here by not failing on import.
 
-    # If the type_name can't be converted into a suitable identifier, throw a
-    # ValueError.
-    with self.assertRaises(ValueError) as cm:
+    # If the type_name can't be converted into a suitable identifier, raise a FieldDeclarationError.
+    expected_rx_str = re.escape(
+      "The field declaration <type 'int'> must be a <type 'unicode'>, tuple, or 'DatatypeFieldDecl' instance, but its type was: 'type'.")
+    with self.assertRaisesRegexp(F.FieldDeclarationError, expected_rx_str):
       class NonStrType(datatype([int])): pass
-    expected_msg = (
-      "Type names and field names must be valid identifiers: \"<class 'int'>\""
-      if PY3 else
-      "Type names and field names can only contain alphanumeric characters and underscores: \"<type 'int'>\""
-    )
-    self.assertEqual(str(cm.exception), expected_msg)
 
     # This raises a TypeError because it doesn't provide a required argument.
     with self.assertRaises(TypeError) as cm:
@@ -330,23 +340,17 @@ class NoFields(datatype()): pass
     )
     self.assertEqual(str(cm.exception), expected_msg)
 
-    with self.assertRaises(ValueError) as cm:
+    expected_rx_str = re.escape(
+      "The field declaration {text_t!r} must be a {text_t!r}, tuple, or 'DatatypeFieldDecl' instance, but its type was: 'type'."
+      .format(text_t=text_type))
+    with self.assertRaisesRegexp(F.FieldDeclarationError, expected_rx_str):
       class JustTypeField(datatype([text_type])): pass
-    expected_msg = (
-      "Type names and field names must be valid identifiers: \"<class 'str'>\""
-      if PY3 else
-      "Type names and field names can only contain alphanumeric characters and underscores: \"<type 'unicode'>\""
-    )
-    self.assertEqual(str(cm.exception), expected_msg)
 
-    with self.assertRaises(ValueError) as cm:
+    expected_rx_str = re.escape(
+      "The field declaration 3 must be a {text_t!r}, tuple, or 'DatatypeFieldDecl' instance, but its type was: 'int'."
+      .format(text_t=text_type))
+    with self.assertRaisesRegexp(F.FieldDeclarationError, expected_rx_str):
       class NonStringField(datatype([3])): pass
-    expected_msg = (
-      "Type names and field names must be valid identifiers: '3'"
-      if PY3 else
-      "Type names and field names cannot start with a number: '3'"
-    )
-    self.assertEqual(str(cm.exception), expected_msg)
 
     with self.assertRaises(ValueError) as cm:
       class NonStringTypeField(datatype([(32, int)])): pass
@@ -376,12 +380,79 @@ class MultipleSameNameWithType(datatype([
     expected_msg = "Encountered duplicate field name: 'field_a'"
     self.assertEqual(str(cm.exception), expected_msg)
 
-    with self.assertRaises(TypeError) as cm:
+    expected_rx_str = re.escape(
+      "type_constraint for field 'a_field' must be an instance of `type` or `TypeConstraint`, or else None, but was instead 2 (type 'int').")
+    with self.assertRaisesRegexp(TypeError, expected_rx_str):
       class InvalidTypeSpec(datatype([('a_field', 2)])): pass
-    expected_msg = (
-      "type spec for field 'a_field' was not a type or TypeConstraint: "
-      "was 2 (type 'int').")
-    self.assertEqual(str(cm.exception), expected_msg)
+
+  def test_class_construction_default_value(self):
+    with self.assertRaisesRegexp(ValueError, re.escape("Empty tuple () passed to datatype().")):
+      class WithEmptyTuple(datatype([()])): pass
+
+    class WithKnownDefaultValueFromTypeConstraint(datatype([F('x', int, default_value=0)])): pass
+    self.assertEqual(WithKnownDefaultValueFromTypeConstraint().x, 0)
+    self.assertEqual(WithKnownDefaultValueFromTypeConstraint(4).x, 4)
+    expected_rx_str = re.escape(
+      """error: in constructor of type WithKnownDefaultValueFromTypeConstraint: type check error:
+field 'x' was invalid: value None (with type 'NoneType') must satisfy this type constraint: Exactly(int).""")
+    with self.assertRaisesRegexp(TypeCheckError, expected_rx_str):
+      WithKnownDefaultValueFromTypeConstraint(None)
+
+    class BrokenTypeConstraint(Exactly):
+      has_default_value = True
+
+      def __init__(self, type_to_wrap, default_value):
+        super(BrokenTypeConstraint, self).__init__(type_to_wrap)
+        self.default_value = default_value
+
+      def __repr__(self):
+        return '{}({})'.format(type(self).__name__, self.types[0])
+
+    expected_rx_str = re.escape(
+      "default_value 3 for the field 'x' must satisfy the provided type_constraint "
+      "BrokenTypeConstraint({})."
+      .format(text_type))
+    with self.assertRaisesRegexp(TypeError, expected_rx_str):
+      class WithBrokenTypeConstraint(datatype([('x', BrokenTypeConstraint(text_type, 3))])): pass
+
+    # This could just be a tuple, but the keyword in the F constructor adds clarity. This works
+    # because of the expanded type constraint.
+    class WithCheckedDefaultValue(datatype([
+        F('x', Exactly(int, type(None)), default_value=None)]
+    )): pass
+    self.assertEqual(WithCheckedDefaultValue().x, None)
+    self.assertEqual(WithCheckedDefaultValue(3).x, 3)
+    self.assertEqual(WithCheckedDefaultValue(x=3).x, 3)
+
+    expected_rx_str = re.escape(
+      "There are too many elements of the tuple ({unicode_literal}'x', <{class_type} 'int'>, None) passed to datatype(). The tuple must have between 1 and 2 elements. The remaining elements were: [None]."
+      .format(unicode_literal=self.unicode_literal,
+              class_type=('class' if PY3 else 'type')))
+    with self.assertRaisesRegexp(ValueError, expected_rx_str):
+      class WithTooManyElementsTuple(datatype([('x', int, None)])): pass
+
+    expected_msg_ending = (
+      "__new__() takes from 2 to 3 positional arguments but 4 were given"
+      if PY3 else
+      "__new__() takes at most 3 arguments (4 given)"
+    )
+    with self.assertRaisesRegexp(TypeError, re.escape(expected_msg_ending)):
+      class WithTooManyPositionalArgsForFieldDecl(datatype([F('x', int, None)])): pass
+
+    expected_msg_ending = (
+      "__new__() takes from 2 to 3 positional arguments but 5 were given"
+      if PY3 else
+      "__new__() takes at most 3 arguments (5 given)"
+    )
+    with self.assertRaisesRegexp(TypeError, re.escape(expected_msg_ending)):
+      class WithUnknownKwargForFieldDecl(datatype([
+          F('x', int, None, should_have_default=False)
+      ])): pass
+
+  def test_instance_construction_default_value(self):
+    self.assertEqual(WithDefaultValueTuple().an_int, 3)
+    self.assertEqual(WithDefaultValueTuple(4).an_int, 4)
+    self.assertEqual(WithDefaultValueTuple(an_int=4).an_int, 4)
 
   def test_instance_construction_by_repr(self):
     some_val = SomeTypedDatatype(3)
@@ -392,19 +463,12 @@ def test_instance_construction_by_repr(self):
     some_object = WithExplicitTypeConstraint(text_type('asdf'), 45)
     self.assertEqual(some_object.a_string, 'asdf')
     self.assertEqual(some_object.an_int, 45)
-    def compare_repr(include_unicode = False):
-      expected_message = "WithExplicitTypeConstraint(a_string={unicode_literal}'asdf', an_int=45)"\
-        .format(unicode_literal='u' if include_unicode else '')
-      self.assertEqual(repr(some_object), expected_message)
-    def compare_str(unicode_type_name):
-      expected_message = "WithExplicitTypeConstraint(a_string<={}>=asdf, an_int<=int>=45)".format(unicode_type_name)
-      self.assertEqual(str(some_object), expected_message)
-    if PY2:
-      compare_str('unicode')
-      compare_repr(include_unicode=True)
-    else:
-      compare_str('str')
-      compare_repr()
+    expected_repr = ("WithExplicitTypeConstraint(a_string={unicode_literal}'asdf', an_int=45)"
+                     .format(unicode_literal=self.unicode_literal))
+    self.assertEqual(repr(some_object), expected_repr)
+    expected_str = ("WithExplicitTypeConstraint(a_string<={}>=asdf, an_int<=int>=45)"
+                    .format(text_type.__name__))
+    self.assertEqual(str(some_object), expected_str)
 
     some_nonneg_int = NonNegativeInt(an_int=3)
     self.assertEqual(3, some_nonneg_int.an_int)
@@ -423,19 +487,11 @@ def compare_str(unicode_type_name):
 
     mixed_type_obj = MixedTyping(value=3, name=text_type('asdf'))
     self.assertEqual(3, mixed_type_obj.value)
-    def compare_repr(include_unicode = False):
-      expected_message = "MixedTyping(value=3, name={unicode_literal}'asdf')" \
-        .format(unicode_literal='u' if include_unicode else '')
-      self.assertEqual(repr(mixed_type_obj), expected_message)
-    def compare_str(unicode_type_name):
-      expected_message = "MixedTyping(value=3, name<={}>=asdf)".format(unicode_type_name)
-      self.assertEqual(str(mixed_type_obj), expected_message)
-    if PY2:
-      compare_str('unicode')
-      compare_repr(include_unicode=True)
-    else:
-      compare_str('str')
-      compare_repr()
+    expected_repr = ("MixedTyping(value=3, name={unicode_literal}'asdf')"
+                     .format(unicode_literal=self.unicode_literal))
+    self.assertEqual(repr(mixed_type_obj), expected_repr)
+    expected_str = "MixedTyping(value=3, name<={}>=asdf)".format(text_type.__name__)
+    self.assertEqual(str(mixed_type_obj), expected_str)
 
     subclass_constraint_obj = WithSubclassTypeConstraint(SomeDatatypeClass())
     self.assertEqual('asdf', subclass_constraint_obj.some_value.something())
@@ -447,127 +503,103 @@ def compare_str(unicode_type_name):
 
   def test_mixin_type_construction(self):
     obj_with_mixin = TypedWithMixin(text_type(' asdf '))
-    def compare_repr(include_unicode = False):
-      expected_message = "TypedWithMixin(val={unicode_literal}' asdf ')" \
-        .format(unicode_literal='u' if include_unicode else '')
-      self.assertEqual(repr(obj_with_mixin), expected_message)
-    def compare_str(unicode_type_name):
-      expected_message = "TypedWithMixin(val<={}>= asdf )".format(unicode_type_name)
-      self.assertEqual(str(obj_with_mixin), expected_message)
-    if PY2:
-      compare_str('unicode')
-      compare_repr(include_unicode=True)
-    else:
-      compare_str('str')
-      compare_repr()
+    expected_repr = ("TypedWithMixin(val={unicode_literal}' asdf ')"
+                     .format(unicode_literal=self.unicode_literal))
+    self.assertEqual(repr(obj_with_mixin), expected_repr)
+
+    expected_str = "TypedWithMixin(val<={}>= asdf )".format(text_type.__name__)
+    self.assertEqual(str(obj_with_mixin), expected_str)
     self.assertEqual(obj_with_mixin.as_str(), ' asdf ')
     self.assertEqual(obj_with_mixin.stripped(), 'asdf')
 
   def test_instance_construction_errors(self):
-    with self.assertRaises(TypeError) as cm:
+    # test that kwargs with keywords that aren't field names fail the same way
+    expected_rx_str = re.escape(
+      """error: in constructor of type SomeTypedDatatype: type check error:
+__new__() got an unexpected keyword argument 'something'""")
+    with self.assertRaisesRegexp(TypeCheckError, expected_rx_str):
       SomeTypedDatatype(something=3)
-    expected_msg = "error: in constructor of type SomeTypedDatatype: type check error:\n__new__() got an unexpected keyword argument 'something'"
-    self.assertEqual(str(cm.exception), expected_msg)
 
     # not providing all the fields
-    with self.assertRaises(TypeError) as cm:
-      SomeTypedDatatype()
     expected_msg_ending = (
       "__new__() missing 1 required positional argument: 'val'"
       if PY3 else
       "__new__() takes exactly 2 arguments (1 given)"
     )
-    expected_msg = "error: in constructor of type SomeTypedDatatype: type check error:\n" + expected_msg_ending
-    self.assertEqual(str(cm.exception), expected_msg)
+    expected_rx_str = re.escape(
+      """error: in constructor of type SomeTypedDatatype: type check error:
+{}"""
+      .format(expected_msg_ending))
+    with self.assertRaisesRegexp(TypeCheckError, expected_rx_str):
+      SomeTypedDatatype()
 
-    # unrecognized fields
-    with self.assertRaises(TypeError) as cm:
-      SomeTypedDatatype(3, 4)
+    # test that too many positional args fails
     expected_msg_ending = (
       "__new__() takes 2 positional arguments but 3 were given"
       if PY3 else
       "__new__() takes exactly 2 arguments (3 given)"
     )
-    expected_msg = "error: in constructor of type SomeTypedDatatype: type check error:\n" + expected_msg_ending
-    self.assertEqual(str(cm.exception), expected_msg)
+    expected_rx_str = re.escape(
+      "error: in constructor of type SomeTypedDatatype: type check error:\n{}"
+      .format(expected_msg_ending))
+    with self.assertRaisesRegexp(TypeCheckError, expected_rx_str):
+      SomeTypedDatatype(3, 4)
 
-    with self.assertRaises(TypedDatatypeInstanceConstructionError) as cm:
-      CamelCaseWrapper(nonneg_int=3)
-    expected_msg = (
+    expected_rx_str = re.escape(
       """error: in constructor of type CamelCaseWrapper: type check error:
 field 'nonneg_int' was invalid: value 3 (with type 'int') must satisfy this type constraint: Exactly(NonNegativeInt).""")
-    self.assertEqual(str(cm.exception), expected_msg)
+    with self.assertRaisesRegexp(TypeCheckError, expected_rx_str):
+      CamelCaseWrapper(nonneg_int=3)
 
     # test that kwargs with keywords that aren't field names fail the same way
-    with self.assertRaises(TypeError) as cm:
+    expected_rx_str = re.escape(
+      "error: in constructor of type CamelCaseWrapper: type check error:\n__new__() got an unexpected keyword argument 'a'")
+    with self.assertRaisesRegexp(TypeError, expected_rx_str):
       CamelCaseWrapper(4, a=3)
-    expected_msg = "error: in constructor of type CamelCaseWrapper: type check error:\n__new__() got an unexpected keyword argument 'a'"
-    self.assertEqual(str(cm.exception), expected_msg)
 
   def test_type_check_errors(self):
     # single type checking failure
-    with self.assertRaises(TypeCheckError) as cm:
-      SomeTypedDatatype([])
-    expected_msg = (
+    expected_rx_str = re.escape(
       """error: in constructor of type SomeTypedDatatype: type check error:
 field 'val' was invalid: value [] (with type 'list') must satisfy this type constraint: Exactly(int).""")
-    self.assertEqual(str(cm.exception), expected_msg)
+    with self.assertRaisesRegexp(TypeError, expected_rx_str):
+      SomeTypedDatatype([])
 
     # type checking failure with multiple arguments (one is correct)
-    with self.assertRaises(TypeCheckError) as cm:
-      AnotherTypedDatatype(text_type('correct'), text_type('should be list'))
-    def compare_str(unicode_type_name, include_unicode=False):
-      expected_message = (
-        """error: in constructor of type AnotherTypedDatatype: type check error:
+    expected_rx_str = re.escape(
+      """error: in constructor of type AnotherTypedDatatype: type check error:
 field 'elements' was invalid: value {unicode_literal}'should be list' (with type '{type_name}') must satisfy this type constraint: Exactly(list)."""
-      .format(type_name=unicode_type_name, unicode_literal='u' if include_unicode else ''))
-      self.assertEqual(str(cm.exception), expected_message)
-    if PY2:
-      compare_str('unicode', include_unicode=True)
-    else:
-      compare_str('str')
+      .format(type_name=text_type.__name__, unicode_literal=self.unicode_literal))
+    with self.assertRaisesRegexp(TypeCheckError, expected_rx_str):
+      AnotherTypedDatatype(text_type('correct'), text_type('should be list'))
 
     # type checking failure on both arguments
-    with self.assertRaises(TypeCheckError) as cm:
-      AnotherTypedDatatype(3, text_type('should be list'))
-    def compare_str(unicode_type_name, include_unicode=False):
-      expected_message = (
-        """error: in constructor of type AnotherTypedDatatype: type check error:
+    expected_rx_str = re.escape(
+      """error: in constructor of type AnotherTypedDatatype: type check error:
 field 'string' was invalid: value 3 (with type 'int') must satisfy this type constraint: Exactly({type_name}).
 field 'elements' was invalid: value {unicode_literal}'should be list' (with type '{type_name}') must satisfy this type constraint: Exactly(list)."""
-          .format(type_name=unicode_type_name, unicode_literal='u' if include_unicode else ''))
-      self.assertEqual(str(cm.exception), expected_message)
-    if PY2:
-      compare_str('unicode', include_unicode=True)
-    else:
-      compare_str('str')
-
-    with self.assertRaises(TypeCheckError) as cm:
-      NonNegativeInt(text_type('asdf'))
-    def compare_str(unicode_type_name, include_unicode=False):
-      expected_message = (
-        """error: in constructor of type NonNegativeInt: type check error:
+      .format(type_name=text_type.__name__, unicode_literal=self.unicode_literal))
+    with self.assertRaisesRegexp(TypeCheckError, expected_rx_str):
+      AnotherTypedDatatype(3, text_type('should be list'))
+
+    expected_rx_str = re.escape(
+      """error: in constructor of type NonNegativeInt: type check error:
 field 'an_int' was invalid: value {unicode_literal}'asdf' (with type '{type_name}') must satisfy this type constraint: Exactly(int)."""
-          .format(type_name=unicode_type_name, unicode_literal='u' if include_unicode else ''))
-      self.assertEqual(str(cm.exception), expected_message)
-    if PY2:
-      compare_str('unicode', include_unicode=True)
-    else:
-      compare_str('str')
-
-    with self.assertRaises(TypeCheckError) as cm:
-      NonNegativeInt(-3)
-    expected_msg = (
+      .format(type_name=text_type.__name__, unicode_literal=self.unicode_literal))
+    with self.assertRaisesRegexp(TypeCheckError, expected_rx_str):
+      NonNegativeInt(text_type('asdf'))
+
+    expected_rx_str = re.escape(
       """error: in constructor of type NonNegativeInt: type check error:
 value is negative: -3.""")
-    self.assertEqual(str(cm.exception), expected_msg)
+    with self.assertRaisesRegexp(TypeCheckError, expected_rx_str):
+      NonNegativeInt(-3)
 
-    with self.assertRaises(TypeCheckError) as cm:
-      WithSubclassTypeConstraint(3)
-    expected_msg = (
+    expected_rx_str = re.escape(
       """error: in constructor of type WithSubclassTypeConstraint: type check error:
 field 'some_value' was invalid: value 3 (with type 'int') must satisfy this type constraint: SubclassesOf(SomeBaseClass).""")
-    self.assertEqual(str(cm.exception), expected_msg)
+    with self.assertRaisesRegexp(TypeCheckError, expected_rx_str):
+      WithSubclassTypeConstraint(3)
 
   def test_copy(self):
     obj = AnotherTypedDatatype(string='some_string', elements=[1, 2, 3])
@@ -580,19 +612,25 @@ def test_copy(self):
   def test_copy_failure(self):
     obj = AnotherTypedDatatype(string='some string', elements=[1,2,3])
 
-    with self.assertRaises(TypeCheckError) as cm:
-      obj.copy(nonexistent_field=3)
-    expected_msg = (
+    expected_rx_str = re.escape(
       """error: in constructor of type AnotherTypedDatatype: type check error:
 __new__() got an unexpected keyword argument 'nonexistent_field'""")
-    self.assertEqual(str(cm.exception), expected_msg)
+    with self.assertRaisesRegexp(TypeCheckError, expected_rx_str):
+      obj.copy(nonexistent_field=3)
 
-    with self.assertRaises(TypeCheckError) as cm:
-      obj.copy(elements=3)
-    expected_msg = (
+    expected_msg_ending = (
+      "copy() takes 1 positional argument but 2 were given"
+      if PY3 else
+      "copy() takes exactly 1 argument (2 given)"
+    )
+    with self.assertRaisesRegexp(TypeError, re.escape(expected_msg_ending)):
+      obj.copy(3)
+
+    expected_rx_str = re.escape(
       """error: in constructor of type AnotherTypedDatatype: type check error:
 field 'elements' was invalid: value 3 (with type 'int') must satisfy this type constraint: Exactly(list).""")
-    self.assertEqual(str(cm.exception), expected_msg)
+    with self.assertRaisesRegexp(TypeCheckError, expected_rx_str):
+      obj.copy(elements=3)
 
   def test_enum_class_creation_errors(self):
     expected_rx = re.escape(
@@ -618,7 +656,7 @@ def test_enum_instance_creation_errors(self):
       SomeEnum(x=3)
 
     expected_rx_falsy_value = re.escape(
-      "Value {}'' for 'x' must be one of: OrderedSet([1, 2])."
-      .format('u' if PY2 else ''))
+      "Value {unicode_literal}'' for 'x' must be one of: OrderedSet([1, 2])."
+      .format(unicode_literal=self.unicode_literal))
     with self.assertRaisesRegexp(TypeCheckError, expected_rx_falsy_value):
       SomeEnum(x='')

From aa003152b667ad0da5a850150e786e4f371016d9 Mon Sep 17 00:00:00 2001
From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com>
Date: Sat, 8 Sep 2018 15:37:21 -0700
Subject: [PATCH 3/6] re-implement _replace()

---
 src/python/pants/util/objects.py             | 58 +++++++++++++++++---
 tests/python/pants_test/util/test_objects.py |  6 +-
 2 files changed, 54 insertions(+), 10 deletions(-)

diff --git a/src/python/pants/util/objects.py b/src/python/pants/util/objects.py
index d5c7cd89158..aab60470d00 100644
--- a/src/python/pants/util/objects.py
+++ b/src/python/pants/util/objects.py
@@ -156,11 +156,53 @@ def _asdict(self):
       '''Return a new OrderedDict which maps field names to their values'''
       return OrderedDict(zip(self._fields, self._super_iter()))
 
-    def _replace(_self, **kwds):
-      '''Return a new datatype object replacing specified fields with new values'''
-      field_dict = _self._asdict()
-      field_dict.update(**kwds)
-      return type(_self)(**field_dict)
+    @memoized_classproperty
+    def _supertype_keyword_only_cached_constructor(cls):
+      """This method is less of an optimization and more to avoid mistakes calling super()."""
+      def ctor(**kw):
+        return super(DataType, cls).__new__(cls, **kw)
+      return ctor
+
+    def _replace(self, **kwds):
+      '''Return a new datatype object replacing specified fields with new values.
+
+      This method upholds 2 contracts:
+      1. If no keyword arguments are provided, return the original object.
+      2. Do not call __new__() -- the parent class's __new__() is used instead (skipping e.g. type
+         checks, which are done in this method by hand).
+
+      These two contracts allow it to be used in __new__() overrides without worrying about
+      unbounded recursion.
+      '''
+      field_dict = self._asdict()
+
+      arg_check_error_messages = []
+      for cur_field_name, cur_field_value in kwds.items():
+        if cur_field_name not in parsed_field_dict:
+          arg_check_error_messages.append(
+            "Field '{}' was not recognized."
+            .format(cur_field_name))
+          continue
+
+        cur_field_decl = parsed_field_dict[cur_field_name]
+        maybe_type_constraint = cur_field_decl.type_constraint
+
+        if maybe_type_constraint:
+          try:
+            maybe_type_constraint.validate_satisfied_by(cur_field_value)
+          except TypeError as e:
+            arg_check_error_messages.append(
+              "Type error for field '{}': {}"
+              .format(cur_field_name, str(e)))
+
+        field_dict[cur_field_name] = cur_field_value
+
+      if arg_check_error_messages:
+        raise self.make_type_error(
+          "Replacing fields {kw!r} of object {obj!r} failed:\n{errs}"
+          .format(kw=kwds, obj=self, errs='\n'.join(arg_check_error_messages)))
+
+      return self._supertype_keyword_only_cached_constructor(**field_dict)
 
     def copy(self, **kwargs):
       """Return the result of `self._replace(**kwargs)`.
@@ -168,9 +210,9 @@ def copy(self, **kwargs):
       This method stub makes error messages provide this method's name, instead of pointing to the
       private `_replace()`.
 
-      We intentionally accept only keyword arguments to make copy() calls in Python code consuming
-      the datatype remain valid if the datatype's definition is updated, unless a field is removed
-      (which fails loudly and quickly with an unknown keyword argument error).
+      NB: We intentionally accept only keyword arguments to make copy() calls in Python code
+      consuming the datatype remain valid if the datatype's definition is updated, unless a field is
+      removed (which fails loudly and quickly with an unknown keyword argument error).
       """
       return self._replace(**kwargs)
 
diff --git a/tests/python/pants_test/util/test_objects.py b/tests/python/pants_test/util/test_objects.py
index 39be1efef0c..473b3c60b00 100644
--- a/tests/python/pants_test/util/test_objects.py
+++ b/tests/python/pants_test/util/test_objects.py
@@ -614,7 +614,8 @@ def test_copy_failure(self):
 
     expected_rx_str = re.escape(
       """error: in constructor of type AnotherTypedDatatype: type check error:
-__new__() got an unexpected keyword argument 'nonexistent_field'""")
+Replacing fields {'nonexistent_field': 3} of object AnotherTypedDatatype(string=u'some string', elements=[1, 2, 3]) failed:
+Field 'nonexistent_field' was not recognized.""")
     with self.assertRaisesRegexp(TypeCheckError, expected_rx_str):
       obj.copy(nonexistent_field=3)
 
@@ -628,7 +629,8 @@ def test_copy_failure(self):
 
     expected_rx_str = re.escape(
       """error: in constructor of type AnotherTypedDatatype: type check error:
-field 'elements' was invalid: value 3 (with type 'int') must satisfy this type constraint: Exactly(list).""")
+Replacing fields {'elements': 3} of object AnotherTypedDatatype(string=u'some string', elements=[1, 2, 3]) failed:
+Type error for field 'elements': value 3 (with type 'int') must satisfy this type constraint: Exactly(list).""")
     with self.assertRaisesRegexp(TypeCheckError, expected_rx_str):
       obj.copy(elements=3)
 

From 2c94bcb9881d85dafe72ee95253deef5f06f0f02 Mon Sep 17 00:00:00 2001
From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com>
Date: Sat, 8 Sep 2018 15:38:04 -0700
Subject: [PATCH 4/6] remove or simplify a few __new__ overrides

---
 src/python/pants/base/project_tree.py         |  9 ----
 src/python/pants/engine/isolated_process.py   | 42 +++++++------------
 src/python/pants/option/scope.py              | 10 ++---
 .../engine/test_isolated_process.py           |  7 ++--
 4 files changed, 21 insertions(+), 47 deletions(-)

diff --git a/src/python/pants/base/project_tree.py b/src/python/pants/base/project_tree.py
index c14a685b027..09dbc02816e 100644
--- a/src/python/pants/base/project_tree.py
+++ b/src/python/pants/base/project_tree.py
@@ -212,19 +212,10 @@ def path(self):
 class File(datatype(['path']), Stat):
   """A file."""
 
-  def __new__(cls, path):
-    return super(File, cls).__new__(cls, path)
-
 
 class Dir(datatype(['path']), Stat):
   """A directory."""
 
-  def __new__(cls, path):
-    return super(Dir, cls).__new__(cls, path)
-
 
 class Link(datatype(['path']), Stat):
   """A symbolic link."""
-
-  def __new__(cls, path):
-    return super(Link, cls).__new__(cls, path)
diff --git a/src/python/pants/engine/isolated_process.py b/src/python/pants/engine/isolated_process.py
index 218321885f0..fb1cf8581ee 100644
--- a/src/python/pants/engine/isolated_process.py
+++ b/src/python/pants/engine/isolated_process.py
@@ -11,6 +11,7 @@
 from pants.engine.fs import DirectoryDigest
 from pants.engine.rules import RootRule, rule
 from pants.engine.selectors import Select
+from pants.util.objects import DatatypeFieldDecl as F
 from pants.util.objects import Exactly, TypeCheckError, datatype
 
 
@@ -23,28 +24,25 @@ class ExecuteProcessRequest(datatype([
   ('argv', tuple),
   ('input_files', DirectoryDigest),
   ('description', text_type),
-  ('env', tuple),
-  ('output_files', tuple),
-  ('output_directories', tuple),
+  # TODO: allow inferring a default value if a type is provided which has a 0-arg constructor.
+  F('env', Exactly(tuple, dict, type(None)), default_value=None),
+  F('output_files', tuple, default_value=()),
+  F('output_directories', tuple, default_value=()),
   # NB: timeout_seconds covers the whole remote operation including queuing and setup.
-  ('timeout_seconds', Exactly(float, int)),
-  ('jdk_home', Exactly(text_type, type(None))),
+  F('timeout_seconds', Exactly(float, int), default_value=_default_timeout_seconds),
+  F('jdk_home', Exactly(text_type, type(None)), default_value=None),
 ])):
   """Request for execution with args and snapshots to extract."""
 
-  def __new__(
-    cls,
-    argv,
-    input_files,
-    description,
-    env=None,
-    output_files=(),
-    output_directories=(),
-    timeout_seconds=_default_timeout_seconds,
-    jdk_home=None,
-  ):
+  def __new__(cls, *args, **kwargs):
+    this_object = super(ExecuteProcessRequest, cls).__new__(cls, *args, **kwargs)
+
+    env = this_object.env
+    # `env` is a tuple, a dict, or None.
     if env is None:
       env = ()
+    elif isinstance(env, tuple):
+      pass
     else:
       if not isinstance(env, dict):
         raise TypeCheckError(
@@ -56,17 +54,7 @@ def __new__(
         )
       env = tuple(item for pair in env.items() for item in pair)
 
-    return super(ExecuteProcessRequest, cls).__new__(
-      cls,
-      argv=argv,
-      env=env,
-      input_files=input_files,
-      description=description,
-      output_files=output_files,
-      output_directories=output_directories,
-      timeout_seconds=timeout_seconds,
-      jdk_home=jdk_home,
-    )
+    return this_object.copy(env=env)
 
 
 class ExecuteProcessResult(datatype([('stdout', binary_type),
diff --git a/src/python/pants/option/scope.py b/src/python/pants/option/scope.py
index c0a4c6f8eb4..88509153549 100644
--- a/src/python/pants/option/scope.py
+++ b/src/python/pants/option/scope.py
@@ -4,15 +4,15 @@
 
 from __future__ import absolute_import, division, print_function, unicode_literals
 
-from collections import namedtuple
+from pants.util.objects import DatatypeFieldDecl as F
+from pants.util.objects import datatype
 
 
 GLOBAL_SCOPE = ''
 GLOBAL_SCOPE_CONFIG_SECTION = 'GLOBAL'
 
 
-# FIXME: convert this to a datatype?
-class ScopeInfo(namedtuple('_ScopeInfo', ['scope', 'category', 'optionable_cls'])):
+class ScopeInfo(datatype(['scope', 'category', F('optionable_cls', default_value=None)])):
   """Information about a scope."""
 
   # Symbolic constants for different categories of scope.
@@ -36,7 +36,3 @@ def deprecated_scope_removal_version(self):
 
   def _optionable_cls_attr(self, name, default=None):
     return getattr(self.optionable_cls, name) if self.optionable_cls else default
-
-
-# Allow the optionable_cls to default to None.
-ScopeInfo.__new__.__defaults__ = (None, )
diff --git a/tests/python/pants_test/engine/test_isolated_process.py b/tests/python/pants_test/engine/test_isolated_process.py
index 2b734a9bee6..5ee21dbd0a7 100644
--- a/tests/python/pants_test/engine/test_isolated_process.py
+++ b/tests/python/pants_test/engine/test_isolated_process.py
@@ -5,6 +5,7 @@
 from __future__ import absolute_import, division, print_function, unicode_literals
 
 import os
+import re
 import unittest
 from builtins import str
 
@@ -217,13 +218,11 @@ def test_blows_up_on_invalid_args(self):
     with self.assertRaises(TypeCheckError):
       self._default_args_execute_process_request(argv=('1',), env=['foo', 'bar'])
 
-    # TODO(cosmicexplorer): we should probably check that the digest info in
-    # ExecuteProcessRequest is valid, beyond just checking if it's a string.
     with self.assertRaisesRegexp(TypeCheckError, "env"):
       ExecuteProcessRequest(
         argv=('1',),
-        env=(),
-        input_files='',
+        env=[],
+        input_files=EMPTY_DIRECTORY_DIGEST,
         output_files=(),
         output_directories=(),
         timeout_seconds=0.1,

From c622065ddecb31435abd30eb8e422285e4d989a9 Mon Sep 17 00:00:00 2001
From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com>
Date: Sat, 8 Sep 2018 15:49:05 -0700
Subject: [PATCH 5/6] remove a few __new__ overrides

---
 src/python/pants/base/specs.py       |  9 +++++----
 src/python/pants/engine/selectors.py | 22 ++++++++++------------
 2 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/src/python/pants/base/specs.py b/src/python/pants/base/specs.py
index c4247cc4b27..14445fcaf03 100644
--- a/src/python/pants/base/specs.py
+++ b/src/python/pants/base/specs.py
@@ -8,6 +8,7 @@
 from abc import abstractmethod
 
 from pants.util.meta import AbstractClass
+from pants.util.objects import DatatypeFieldDecl as F
 from pants.util.objects import datatype
 
 
@@ -60,11 +61,11 @@ def to_spec_string(self):
     return '{}^'.format(self.directory)
 
 
-class Specs(datatype(['dependencies', 'tags', ('exclude_patterns', tuple)])):
+class Specs(datatype([
+    'dependencies',
+    F('tags', tuple, default_value=()),
+    F('exclude_patterns', tuple, default_value=())])):
   """A collection of Specs representing Spec subclasses, tags and regex filters."""
 
-  def __new__(cls, dependencies, tags=tuple(), exclude_patterns=tuple()):
-    return super(Specs, cls).__new__(cls, dependencies, tags, exclude_patterns)
-
   def exclude_patterns_memo(self):
     return [re.compile(pattern) for pattern in set(self.exclude_patterns or [])]
diff --git a/src/python/pants/engine/selectors.py b/src/python/pants/engine/selectors.py
index d46e4b88a1a..ea185f60d1d 100644
--- a/src/python/pants/engine/selectors.py
+++ b/src/python/pants/engine/selectors.py
@@ -11,7 +11,8 @@
 import six
 
 from pants.util.meta import AbstractClass
-from pants.util.objects import Exactly, datatype
+from pants.util.objects import DatatypeFieldDecl as F
+from pants.util.objects import Exactly, SubclassesOf, datatype
 
 
 def type_or_constraint_repr(constraint):
@@ -96,23 +97,25 @@ def product(self):
     """The product that this selector produces."""
 
 
-class Select(datatype(['product', 'optional']), Selector):
+class Select(datatype([
+    'product',
+    F('optional', bool, default_value=False),
+]), Selector):
   """Selects the given Product for the Subject provided to the constructor.
 
   If optional=True and no matching product can be produced, will return None.
   """
 
-  def __new__(cls, product, optional=False):
-    obj = super(Select, cls).__new__(cls, product, optional)
-    return obj
-
   def __repr__(self):
     return '{}({}{})'.format(type(self).__name__,
                              type_or_constraint_repr(self.product),
                              ', optional=True' if self.optional else '')
 
 
-class SelectVariant(datatype(['product', 'variant_key']), Selector):
+class SelectVariant(datatype([
+    'product',
+    ('variant_key', SubclassesOf(*six.string_types)),
+]), Selector):
   """Selects the matching Product and variant name for the Subject provided to the constructor.
 
   For example: a SelectVariant with a variant_key of "thrift" and a product of type ApacheThrift
@@ -121,11 +124,6 @@ class SelectVariant(datatype(['product', 'variant_key']), Selector):
   """
   optional = False
 
-  def __new__(cls, product, variant_key):
-    if not isinstance(variant_key, six.string_types):
-      raise ValueError('Expected variant_key to be a string, but was {!r}'.format(variant_key))
-    return super(SelectVariant, cls).__new__(cls, product, variant_key)
-
   def __repr__(self):
     return '{}({}, {})'.format(type(self).__name__,
                                type_or_constraint_repr(self.product),

From 62153d8e07fb8b7e57acccd7a16fcaae6badbe31 Mon Sep 17 00:00:00 2001
From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com>
Date: Sat, 8 Sep 2018 16:22:42 -0700
Subject: [PATCH 6/6] remove unused import

---
 tests/python/pants_test/engine/test_isolated_process.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tests/python/pants_test/engine/test_isolated_process.py b/tests/python/pants_test/engine/test_isolated_process.py
index 5ee21dbd0a7..8a728a0452d 100644
--- a/tests/python/pants_test/engine/test_isolated_process.py
+++ b/tests/python/pants_test/engine/test_isolated_process.py
@@ -5,7 +5,6 @@
 from __future__ import absolute_import, division, print_function, unicode_literals
 
 import os
-import re
 import unittest
 from builtins import str