diff --git a/src/python/pants/engine/objects.py b/src/python/pants/engine/objects.py index b8b24f1347cb..48b017d688d7 100644 --- a/src/python/pants/engine/objects.py +++ b/src/python/pants/engine/objects.py @@ -161,6 +161,8 @@ class Collection(object): which may be consumed e.g. over FFI from the engine. Python consumers of a Collection should prefer to use its standard iteration API. + + Note that elements of a Collection are type-checked upon construction. """ @memoized_classmethod diff --git a/src/python/pants/util/objects.py b/src/python/pants/util/objects.py index de913206372b..f82a708203e6 100644 --- a/src/python/pants/util/objects.py +++ b/src/python/pants/util/objects.py @@ -276,16 +276,14 @@ class TypeConstraint(AbstractClass): :class:`SubclassesOf`. """ - def __init__(self, variance_symbol, description): + def __init__(self, description): """Creates a type constraint centered around the given types. The type constraint is satisfied as a whole if satisfied for at least one of the given types. - :param type *types: The focus of this type constraint. - :param str description: A description for this constraint if the list of types is too long. + :param str description: A concise, readable description of what the type constraint represents. + Used directly as the __str__ implementation. """ - assert(variance_symbol) - self._variance_symbol = variance_symbol self._description = description @abstractmethod @@ -295,10 +293,15 @@ def satisfied_by(self, obj): :rtype: bool """ - def validate_satisfied_by(self, obj): - """Return some version of `obj` if the object satisfies this type constraint, or raise. + def make_type_constraint_error(self, obj, constraint): + return TypeConstraintError( + "value {!r} (with type {!r}) must satisfy this type constraint: {!r}." + .format(obj, type(obj).__name__, constraint)) - # TODO: consider disallowing overriding this too? + # TODO: disallow overriding this method with some form of mixin/decorator along with datatype + # __eq__! + def validate_satisfied_by(self, obj): + """Return `obj` if the object satisfies this type constraint, or raise. :raises: `TypeConstraintError` if `obj` does not satisfy the constraint. """ @@ -306,19 +309,23 @@ def validate_satisfied_by(self, obj): if self.satisfied_by(obj): return obj - raise TypeConstraintError( - "value {!r} (with type {!r}) must satisfy this type constraint: {!r}." - .format(obj, type(obj).__name__, self)) + raise self.make_type_constraint_error(obj, self) def __ne__(self, other): return not (self == other) def __str__(self): - return '{}{}'.format(self._variance_symbol, self._description) + return self._description -class BasicTypeConstraint(TypeConstraint): - """A `TypeConstraint` predicated only on the object's type.""" +class TypeOnlyConstraint(TypeConstraint): + """A `TypeConstraint` predicated only on the object's type. + + `TypeConstraint` subclasses may override `.satisfied_by()` to perform arbitrary validation on the + object itself -- however, this class implements `.satisfied_by()` with a guarantee that it will + only act on the object's `type` via `.satisfied_by_type()`. This kind of type checking is faster + and easier to understand than the more complex validation allowed by `.satisfied_by()`. + """ # TODO: make an @abstract_classproperty decorator to do this boilerplate! @classproperty @@ -328,12 +335,12 @@ def _variance_symbol(cls): .format(cls.__name__)) def __init__(self, *types): - """Creates a type constraint centered around the given types. + """Creates a type constraint based on some logic to match the given types. - The type constraint is satisfied as a whole if satisfied for at least one of the given types. + NB: A `TypeOnlyConstraint` implementation should ensure that the type constraint is satisfied as + a whole if satisfied for at least one of the given `types`. - :param type *types: The focus of this type constraint. - :param str description: A description for this constraint if the list of types is too long. + :param type *types: The types this constraint will match in some way. """ if not types: @@ -342,13 +349,12 @@ def __init__(self, *types): raise TypeError('Supplied types must be types. {!r}'.format(types)) if len(types) == 1: - constrained_type = types[0].__name__ + type_list = types[0].__name__ else: - constrained_type = '({})'.format(', '.join(t.__name__ for t in types)) + type_list = ' or '.join(t.__name__ for t in types) + description = '{}({})'.format(type(self).__name__, type_list) - super(BasicTypeConstraint, self).__init__( - variance_symbol=self._variance_symbol, - description=constrained_type) + super(TypeOnlyConstraint, self).__init__(description=description) # NB: This is made into a tuple so that we can use self._types in issubclass() and others! self._types = tuple(types) @@ -382,20 +388,16 @@ def __repr__(self): constrained_type=constrained_type)) -class SuperclassesOf(BasicTypeConstraint): +class SuperclassesOf(TypeOnlyConstraint): """Objects of the exact type as well as any super-types are allowed.""" - _variance_symbol = '-' - def satisfied_by_type(self, obj_type): return any(issubclass(t, obj_type) for t in self._types) -class Exactly(BasicTypeConstraint): +class Exactly(TypeOnlyConstraint): """Only objects of the exact type are allowed.""" - _variance_symbol = '=' - def satisfied_by_type(self, obj_type): return obj_type in self._types @@ -406,44 +408,59 @@ def graph_str(self): return repr(self) -class SubclassesOf(BasicTypeConstraint): +class SubclassesOf(TypeOnlyConstraint): """Objects of the exact type as well as any sub-types are allowed.""" - _variance_symbol = '+' - def satisfied_by_type(self, obj_type): return issubclass(obj_type, self._types) class TypedCollection(TypeConstraint): - """A `TypeConstraint` which accepts a BasicTypeConstraint and validates a collection.""" + """A `TypeConstraint` which accepts a TypeOnlyConstraint and validates a collection.""" - @classmethod - def _generate_variance_symbol(cls, constraint): - return '[{}]'.format(constraint._variance_symbol) + _iterable_constraint = SubclassesOf(Iterable) def __init__(self, constraint): - """Create a TypeConstraint which validates each member of a collection with `constraint`. + """Create a `TypeConstraint` which validates each member of a collection with `constraint`. - :param BasicTypeConstraint constraint: the TypeConstraint to apply to each element. This is - currently required to be a BasicTypeConstraint to avoid - complex prototypal type relationships. + :param TypeOnlyConstraint constraint: the `TypeConstraint` to apply to each element. This is + currently required to be a `TypeOnlyConstraint` to avoid + complex prototypal type relationships. """ - if not isinstance(constraint, BasicTypeConstraint): + if not isinstance(constraint, TypeOnlyConstraint): raise TypeError("constraint for collection must be a {}! was: {}" - .format(BasicTypeConstraint.__name__, constraint)) + .format(TypeOnlyConstraint.__name__, constraint)) + + description = '{}({})'.format(type(self).__name__, constraint) + self._constraint = constraint - super(TypedCollection, self).__init__( - variance_symbol=self._generate_variance_symbol(constraint), - description=constraint._description) + super(TypedCollection, self).__init__(description=description) + # TODO: consider making this a private method of TypeConstraint, as it now duplicates the logic in + # self.validate_satisfied_by()! def satisfied_by(self, obj): - if isinstance(obj, Iterable): + if self._iterable_constraint.satisfied_by(obj): return all(self._constraint.satisfied_by(el) for el in obj) return False + def make_collection_type_constraint_error(self, base_obj, el): + base_error = self.make_type_constraint_error(el, self._constraint) + return TypeConstraintError("in wrapped constraint {!r} matching iterable object {!r}: {}" + .format(self, base_obj, base_error)) + + def validate_satisfied_by(self, obj): + if self._iterable_constraint.satisfied_by(obj): + for el in obj: + if not self._constraint.satisfied_by(el): + raise self.make_collection_type_constraint_error(obj, el) + return obj + + base_iterable_error = self.make_type_constraint_error(obj, self._iterable_constraint) + raise TypeConstraintError( + "in wrapped constraint {!r}: {}".format(self, base_iterable_error)) + def __hash__(self): return hash((type(self), self._constraint)) diff --git a/tests/python/pants_test/engine/test_objects.py b/tests/python/pants_test/engine/test_objects.py index 664e89dfd70b..fcaaa867f154 100644 --- a/tests/python/pants_test/engine/test_objects.py +++ b/tests/python/pants_test/engine/test_objects.py @@ -4,10 +4,28 @@ from __future__ import absolute_import, division, print_function, unicode_literals +import re + +from future.utils import PY3, text_type + from pants.engine.objects import Collection +from pants.util.objects import TypeCheckError from pants_test.test_base import TestBase class CollectionTest(TestBase): def test_collection_iteration(self): self.assertEqual([1, 2], [x for x in Collection.of(int)([1, 2])]) + + def test_element_typechecking(self): + IntColl = Collection.of(int) + with self.assertRaisesRegexp(TypeCheckError, re.escape("""\ +field 'dependencies' was invalid: in wrapped constraint TypedCollection(Exactly(int)) matching iterable object [3, {u}'hello']: value {u}'hello' (with type 'unicode') must satisfy this type constraint: Exactly(int).""" + .format(u='' if PY3 else 'u'))): + IntColl([3, "hello"]) + + IntOrStringColl = Collection.of(int, text_type) + self.assertEqual([3, "hello"], [x for x in IntOrStringColl([3, "hello"])]) + with self.assertRaisesRegexp(TypeCheckError, re.escape("""\ +field 'dependencies' was invalid: in wrapped constraint TypedCollection(Exactly(int, unicode)) matching iterable object [{}]: value {} (with type 'dict') must satisfy this type constraint: Exactly(int, unicode).""")): + IntOrStringColl([{}]) diff --git a/tests/python/pants_test/util/test_objects.py b/tests/python/pants_test/util/test_objects.py index 4925d9e4b1f7..7af3717073bf 100644 --- a/tests/python/pants_test/util/test_objects.py +++ b/tests/python/pants_test/util/test_objects.py @@ -41,11 +41,11 @@ def test_none(self): def test_str_and_repr(self): superclasses_of_b = SuperclassesOf(self.B) - self.assertEqual("-B", str(superclasses_of_b)) + self.assertEqual("SuperclassesOf(B)", str(superclasses_of_b)) self.assertEqual("SuperclassesOf(B)", repr(superclasses_of_b)) superclasses_of_multiple = SuperclassesOf(self.A, self.B) - self.assertEqual("-(A, B)", str(superclasses_of_multiple)) + self.assertEqual("SuperclassesOf(A or B)", str(superclasses_of_multiple)) self.assertEqual("SuperclassesOf(A, B)", repr(superclasses_of_multiple)) def test_single(self): @@ -88,11 +88,11 @@ def test_disallows_unsplatted_lists(self): def test_str_and_repr(self): exactly_b = Exactly(self.B) - self.assertEqual("=B", str(exactly_b)) + self.assertEqual("Exactly(B)", str(exactly_b)) self.assertEqual("Exactly(B)", repr(exactly_b)) exactly_multiple = Exactly(self.A, self.B) - self.assertEqual("=(A, B)", str(exactly_multiple)) + self.assertEqual("Exactly(A or B)", str(exactly_multiple)) self.assertEqual("Exactly(A, B)", repr(exactly_multiple)) def test_checking_via_bare_type(self): @@ -107,11 +107,11 @@ def test_none(self): def test_str_and_repr(self): subclasses_of_b = SubclassesOf(self.B) - self.assertEqual("+B", str(subclasses_of_b)) + self.assertEqual("SubclassesOf(B)", str(subclasses_of_b)) self.assertEqual("SubclassesOf(B)", repr(subclasses_of_b)) subclasses_of_multiple = SubclassesOf(self.A, self.B) - self.assertEqual("+(A, B)", str(subclasses_of_multiple)) + self.assertEqual("SubclassesOf(A or B)", str(subclasses_of_multiple)) self.assertEqual("SubclassesOf(A, B)", repr(subclasses_of_multiple)) def test_single(self): @@ -132,13 +132,13 @@ def test_multiple(self): class TypedCollectionTest(TypeConstraintTestBase): def test_str_and_repr(self): collection_of_exactly_b = TypedCollection(Exactly(self.B)) - self.assertEqual("[=]B", str(collection_of_exactly_b)) - self.assertEqual("TypedCollection(Exactly(B))", - repr(collection_of_exactly_b)) + self.assertEqual("TypedCollection(Exactly(B))", str(collection_of_exactly_b)) + self.assertEqual("TypedCollection(Exactly(B))", repr(collection_of_exactly_b)) collection_of_multiple_subclasses = TypedCollection( SubclassesOf(self.A, self.B)) - self.assertEqual("[+](A, B)", str(collection_of_multiple_subclasses)) + self.assertEqual("TypedCollection(SubclassesOf(A or B))", + str(collection_of_multiple_subclasses)) self.assertEqual("TypedCollection(SubclassesOf(A, B))", repr(collection_of_multiple_subclasses)) @@ -153,20 +153,11 @@ def test_collection_multiple(self): self.assertTrue(collection_constraint.satisfied_by([self.B(), self.C(), self.BPrime()])) self.assertFalse(collection_constraint.satisfied_by([self.B(), self.A()])) - def collection_generator(): - yield self.A() - yield self.C() - yield self.A() - - collection_constraint = TypedCollection(Exactly(self.A, self.C)) - self.assertEquals( - (self.A(), self.C(), self.A()), - collection_constraint.validate_satisfied_by(tuple(collection_generator()))) - - def test_construction(self): + def test_no_complex_sub_constraint(self): + sub_collection = TypedCollection(Exactly(self.A)) with self.assertRaisesRegexp(TypeError, re.escape( - "constraint for collection must be a BasicTypeConstraint! was: {}".format(self.A))): - TypedCollection(self.A) + "constraint for collection must be a TypeOnlyConstraint! was: {}".format(sub_collection))): + TypedCollection(sub_collection) class ExportedDatatype(datatype(['val'])): @@ -444,7 +435,7 @@ def test_instance_construction_by_repr(self): some_val = SomeTypedDatatype(3) self.assertEqual(3, some_val.val) self.assertEqual(repr(some_val), "SomeTypedDatatype(val=3)") - self.assertEqual(str(some_val), "SomeTypedDatatype(val<=int>=3)") + self.assertEqual(str(some_val), "SomeTypedDatatype(val=3)") some_object = WithExplicitTypeConstraint(text_type('asdf'), 45) self.assertEqual(some_object.a_string, 'asdf') @@ -454,7 +445,7 @@ def compare_repr(include_unicode = False): .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) + expected_message = "WithExplicitTypeConstraint(a_string=asdf, an_int=45)".format(unicode_type_name) self.assertEqual(str(some_object), expected_message) if PY2: compare_str('unicode') @@ -466,7 +457,7 @@ def compare_str(unicode_type_name): some_nonneg_int = NonNegativeInt(an_int=3) self.assertEqual(3, some_nonneg_int.an_int) self.assertEqual(repr(some_nonneg_int), "NonNegativeInt(an_int=3)") - self.assertEqual(str(some_nonneg_int), "NonNegativeInt(an_int<=int>=3)") + self.assertEqual(str(some_nonneg_int), "NonNegativeInt(an_int=3)") wrapped_nonneg_int = CamelCaseWrapper(NonNegativeInt(45)) # test attribute naming for camel-cased types @@ -476,7 +467,7 @@ def compare_str(unicode_type_name): "CamelCaseWrapper(nonneg_int=NonNegativeInt(an_int=45))") self.assertEqual( str(wrapped_nonneg_int), - "CamelCaseWrapper(nonneg_int<=NonNegativeInt>=NonNegativeInt(an_int<=int>=45))") + "CamelCaseWrapper(nonneg_int=NonNegativeInt(an_int=45))") mixed_type_obj = MixedTyping(value=3, name=text_type('asdf')) self.assertEqual(3, mixed_type_obj.value) @@ -485,7 +476,7 @@ def compare_repr(include_unicode = False): .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) + expected_message = "MixedTyping(value=3, name=asdf)".format(unicode_type_name) self.assertEqual(str(mixed_type_obj), expected_message) if PY2: compare_str('unicode') @@ -500,7 +491,7 @@ def compare_str(unicode_type_name): "WithSubclassTypeConstraint(some_value=SomeDatatypeClass())") self.assertEqual( str(subclass_constraint_obj), - "WithSubclassTypeConstraint(some_value<+SomeBaseClass>=SomeDatatypeClass())") + "WithSubclassTypeConstraint(some_value=SomeDatatypeClass())") def test_mixin_type_construction(self): obj_with_mixin = TypedWithMixin(text_type(' asdf ')) @@ -509,7 +500,7 @@ def compare_repr(include_unicode = False): .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) + expected_message = "TypedWithMixin(val= asdf )".format(unicode_type_name) self.assertEqual(str(obj_with_mixin), expected_message) if PY2: compare_str('unicode') @@ -523,7 +514,7 @@ def compare_str(unicode_type_name): def test_instance_with_collection_construction_str_repr(self): # TODO: convert the type of the input collection using a `wrapper_type` argument! obj_with_collection = WithCollectionTypeConstraint([3]) - self.assertEqual("WithCollectionTypeConstraint(dependencies<[=]int>=[3])", + self.assertEqual("WithCollectionTypeConstraint(dependencies=[3])", str(obj_with_collection)) self.assertEqual("WithCollectionTypeConstraint(dependencies=[3])", repr(obj_with_collection)) @@ -638,14 +629,14 @@ def compare_str(unicode_type_name, include_unicode=False): WithCollectionTypeConstraint(3) expected_msg = """\ error: in constructor of type WithCollectionTypeConstraint: type check error: -field 'dependencies' was invalid: value 3 (with type 'int') must satisfy this type constraint: TypedCollection(Exactly(int)).""" +field 'dependencies' was invalid: in wrapped constraint TypedCollection(Exactly(int)): value 3 (with type 'int') must satisfy this type constraint: SubclassesOf(Iterable).""" self.assertEqual(str(cm.exception), expected_msg) with self.assertRaises(TypeCheckError) as cm: WithCollectionTypeConstraint([3, "asdf"]) expected_msg = """\ error: in constructor of type WithCollectionTypeConstraint: type check error: -field 'dependencies' was invalid: value [3, {}'asdf'] (with type 'list') must satisfy this type constraint: TypedCollection(Exactly(int)).""".format('u' if PY2 else '') +field 'dependencies' was invalid: in wrapped constraint TypedCollection(Exactly(int)) matching iterable object [3, {u}'asdf']: value {u}'asdf' (with type 'unicode') must satisfy this type constraint: Exactly(int).""".format(u='u' if PY2 else '') self.assertEqual(str(cm.exception), expected_msg) def test_copy(self):