From 3d7a2955842f54467a569e4e5b74dd8ba9f5d67e Mon Sep 17 00:00:00 2001 From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Mon, 11 Feb 2019 22:02:21 -0800 Subject: [PATCH] add a TypedCollection type constraint to reduce boilerplate for datatype tuple fields (#7115) ### Problem *Resolves #6936.* There's been a [TODO in `pants.util.objects.Collection`](https://github.com/pantsbuild/pants/blob/c342fd3432aa0d73e402d2db7e013ecfcc76e9c8/src/python/pants/util/objects.py#L413) for a while to typecheck datatype tuple fields. #6936 has some thoughts on how to do this, but after realizing I could split out `TypeConstraint` into a base class and then introduce `BasicTypeConstraint` for type constraints which only act on the type, I think that ticket is invalidated as this solution is much cleaner. ### Solution - Split out logic for basic type checking (without looking at the object itself) into a `BasicTypeConstraint` class, which `Exactly` and friends inherit from. - Create the `TypedCollection` type constraint, which checks that its argument is iterable and then validates each element of the collection with a `BasicTypeConstraint` constructor argument. - Note that `TypedCollection` is a `TypeConstraint`, but not a `BasicTypeConstraint`, as it has to inspect the actual object object to determine whether each element matches the provided `BasicTypeConstraint`. - Move `pants.util.objects.Collection` into `src/python/pants/engine/objects.py`, as it is specifically for engine objects. - Use `TypedCollection` for the `dependencies` field of the datatype returned by `Collection.of()`. ### Result - `datatype` consumers and creators no longer have to have lots of boilerplate when using collections arguments, and those arguments can now be typechecked and made hashable for free! ### TODO in followup: `wrapper_type` See #7172. --- src/python/pants/engine/BUILD | 5 + src/python/pants/engine/addressable.py | 4 +- src/python/pants/engine/fs.py | 3 +- src/python/pants/engine/legacy/BUILD | 1 + src/python/pants/engine/legacy/graph.py | 3 +- src/python/pants/engine/objects.py | 41 ++++ src/python/pants/engine/scheduler.py | 3 +- src/python/pants/util/objects.py | 231 +++++++++++------- tests/python/pants_test/engine/BUILD | 10 + .../engine/legacy/test_graph_integration.py | 6 +- tests/python/pants_test/engine/test_engine.py | 12 +- tests/python/pants_test/engine/test_mapper.py | 2 +- .../python/pants_test/engine/test_objects.py | 33 +++ .../pants_test/engine/test_scheduler.py | 4 +- tests/python/pants_test/util/test_objects.py | 160 ++++++++++-- 15 files changed, 384 insertions(+), 134 deletions(-) create mode 100644 tests/python/pants_test/engine/test_objects.py diff --git a/src/python/pants/engine/BUILD b/src/python/pants/engine/BUILD index 168201a5719..cab6d0cbcee 100644 --- a/src/python/pants/engine/BUILD +++ b/src/python/pants/engine/BUILD @@ -50,6 +50,7 @@ python_library( dependencies=[ '3rdparty/python/twitter/commons:twitter.common.collections', '3rdparty/python:future', + ':objects', ':rules', ':selectors', 'src/python/pants/base:project_tree', @@ -121,7 +122,10 @@ python_library( name='objects', sources=['objects.py'], dependencies=[ + '3rdparty/python:future', 'src/python/pants/util:meta', + 'src/python/pants/util:memo', + 'src/python/pants/util:objects', ] ) @@ -172,6 +176,7 @@ python_library( ':isolated_process', ':native', ':nodes', + ':objects', ':rules', 'src/python/pants/base:exceptions', 'src/python/pants/base:specs', diff --git a/src/python/pants/engine/addressable.py b/src/python/pants/engine/addressable.py index 508a9471c61..25f4233c701 100644 --- a/src/python/pants/engine/addressable.py +++ b/src/python/pants/engine/addressable.py @@ -11,9 +11,9 @@ from future.utils import string_types from pants.build_graph.address import Address, BuildFileAddress -from pants.engine.objects import Resolvable, Serializable +from pants.engine.objects import Collection, Resolvable, Serializable from pants.util.collections_abc_backport import MutableMapping, MutableSequence -from pants.util.objects import Collection, TypeConstraintError +from pants.util.objects import TypeConstraintError Addresses = Collection.of(Address) diff --git a/src/python/pants/engine/fs.py b/src/python/pants/engine/fs.py index c43b41cf858..c290c87aadf 100644 --- a/src/python/pants/engine/fs.py +++ b/src/python/pants/engine/fs.py @@ -7,10 +7,11 @@ from future.utils import binary_type, text_type from pants.base.project_tree import Dir, File +from pants.engine.objects import Collection from pants.engine.rules import RootRule from pants.option.custom_types import GlobExpansionConjunction from pants.option.global_options import GlobMatchErrorBehavior -from pants.util.objects import Collection, datatype +from pants.util.objects import datatype class FileContent(datatype([('path', text_type), ('content', binary_type)])): diff --git a/src/python/pants/engine/legacy/BUILD b/src/python/pants/engine/legacy/BUILD index cb8086944d8..8db00aa36d4 100644 --- a/src/python/pants/engine/legacy/BUILD +++ b/src/python/pants/engine/legacy/BUILD @@ -75,6 +75,7 @@ python_library( 'src/python/pants/build_graph', 'src/python/pants/engine:build_files', 'src/python/pants/engine:mapper', + 'src/python/pants/engine:objects', 'src/python/pants/engine:parser', 'src/python/pants/engine:selectors', 'src/python/pants/option', diff --git a/src/python/pants/engine/legacy/graph.py b/src/python/pants/engine/legacy/graph.py index 72368f1a6db..7bd0efbeaf8 100644 --- a/src/python/pants/engine/legacy/graph.py +++ b/src/python/pants/engine/legacy/graph.py @@ -26,13 +26,14 @@ from pants.engine.legacy.address_mapper import LegacyAddressMapper from pants.engine.legacy.structs import BundleAdaptor, BundlesField, SourcesField, TargetAdaptor from pants.engine.mapper import AddressMapper +from pants.engine.objects import Collection from pants.engine.parser import SymbolTable, TargetAdaptorContainer from pants.engine.rules import RootRule, rule from pants.engine.selectors import Get, Select from pants.option.global_options import GlobMatchErrorBehavior from pants.source.filespec import any_matches_filespec from pants.source.wrapped_globs import EagerFilesetWithSpec, FilesetRelPathWrapper -from pants.util.objects import Collection, datatype +from pants.util.objects import datatype logger = logging.getLogger(__name__) diff --git a/src/python/pants/engine/objects.py b/src/python/pants/engine/objects.py index a0b0e784a7c..48b017d688d 100644 --- a/src/python/pants/engine/objects.py +++ b/src/python/pants/engine/objects.py @@ -5,10 +5,16 @@ from __future__ import absolute_import, division, print_function, unicode_literals import inspect +import sys from abc import abstractmethod, abstractproperty +from builtins import object from collections import namedtuple +from future.utils import PY2 + +from pants.util.memo import memoized_classmethod from pants.util.meta import AbstractClass +from pants.util.objects import Exactly, TypedCollection, datatype class SerializationError(Exception): @@ -146,3 +152,38 @@ def validate(self): :raises: :class:`ValidationError` if this object is invalid. """ + + +class Collection(object): + """Constructs classes representing collections of objects of a particular type. + + The produced class will expose its values under a field named dependencies - this is a stable API + 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 + def of(cls, *element_types): + union = '|'.join(element_type.__name__ for element_type in element_types) + type_name = '{}.of({})'.format(cls.__name__, union) + if PY2: + type_name = type_name.encode('utf-8') + type_checked_collection_class = datatype([ + # Create a datatype with a single field 'dependencies' which is type-checked on construction + # to be a collection containing elements of only the exact `element_types` specified. + ('dependencies', TypedCollection(Exactly(*element_types))) + ], superclass_name=cls.__name__) + supertypes = (cls, type_checked_collection_class) + properties = {'element_types': element_types} + collection_of_type = type(type_name, supertypes, properties) + + # Expose the custom class type at the module level to be pickle compatible. + setattr(sys.modules[cls.__module__], type_name, collection_of_type) + + return collection_of_type + + def __iter__(self): + return iter(self.dependencies) diff --git a/src/python/pants/engine/scheduler.py b/src/python/pants/engine/scheduler.py index 72d04e661f8..fa4688c64ab 100644 --- a/src/python/pants/engine/scheduler.py +++ b/src/python/pants/engine/scheduler.py @@ -19,12 +19,13 @@ from pants.engine.isolated_process import ExecuteProcessRequest, FallibleExecuteProcessResult from pants.engine.native import Function, TypeConstraint, TypeId from pants.engine.nodes import Return, Throw +from pants.engine.objects import Collection from pants.engine.rules import RuleIndex, SingletonRule, TaskRule from pants.engine.selectors import Params, Select, constraint_for from pants.rules.core.exceptions import GracefulTerminationException from pants.util.contextutil import temporary_file_path from pants.util.dirutil import check_no_overlapping_paths -from pants.util.objects import Collection, datatype +from pants.util.objects import datatype from pants.util.strutil import pluralize diff --git a/src/python/pants/util/objects.py b/src/python/pants/util/objects.py index ddc51f388fb..7c29fe1fefe 100644 --- a/src/python/pants/util/objects.py +++ b/src/python/pants/util/objects.py @@ -4,17 +4,15 @@ from __future__ import absolute_import, division, print_function, unicode_literals -import sys from abc import abstractmethod -from builtins import object, zip +from builtins import zip from collections import namedtuple -from future.utils import PY2 from twitter.common.collections import OrderedSet -from pants.util.collections_abc_backport import OrderedDict -from pants.util.memo import memoized, memoized_classproperty -from pants.util.meta import AbstractClass +from pants.util.collections_abc_backport import Iterable, OrderedDict +from pants.util.memo import memoized_classproperty +from pants.util.meta import AbstractClass, classproperty def datatype(field_decls, superclass_name=None, **kwargs): @@ -266,6 +264,7 @@ def __init__(self, type_name, msg, *args, **kwargs): type_name, formatted_msg, *args, **kwargs) +# TODO: make these members of the `TypeConstraint` class! class TypeConstraintError(TypeError): """Indicates a :class:`TypeConstraint` violation.""" @@ -273,43 +272,99 @@ 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`. """ - def __init__(self, *types, **kwargs): + 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. """ + self._description = description + + @abstractmethod + def satisfied_by(self, obj): + """Return `True` if the given object satisfies this type constraint. + + :rtype: bool + """ + + def make_type_constraint_error(self, obj, constraint): + return TypeConstraintError( + "value {!r} (with type {!r}) must satisfy this type constraint: {}." + .format(obj, type(obj).__name__, constraint)) + + # 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. + """ + + if self.satisfied_by(obj): + return obj + + raise self.make_type_constraint_error(obj, self) + + def __ne__(self, other): + return not (self == other) + + def __str__(self): + return self._description + + +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 + def _variance_symbol(cls): + """This is propagated to the the `TypeConstraint` constructor.""" + raise NotImplementedError('{} must implement the _variance_symbol classproperty!' + .format(cls.__name__)) + + def __init__(self, *types): + """Creates a type constraint based on some logic to match 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 types this constraint will match in some way. + """ + if not types: raise ValueError('Must supply at least one type') if any(not isinstance(t, type) for t in types): raise TypeError('Supplied types must be types. {!r}'.format(types)) - # NB: `types` is converted to tuple here because self.types's docstring says - # it returns a tuple. Does it matter what type this field is? + if len(types) == 1: + type_list = types[0].__name__ + else: + type_list = ' or '.join(t.__name__ for t in types) + description = '{}({})'.format(type(self).__name__, type_list) + + 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) - self._desc = kwargs.get('description', None) + # TODO(#7114): remove this after the engine is converted to use `TypeId` instead of + # `TypeConstraint`! @property def types(self): - """Return the subject types of this type constraint. - - :type: tuple of type - """ return self._types - def satisfied_by(self, obj): - """Return `True` if the given object satisfies this type constraint. - - :rtype: bool - """ - return self.satisfied_by_type(type(obj)) - @abstractmethod def satisfied_by_type(self, obj_type): """Return `True` if the given object satisfies this type constraint. @@ -317,18 +372,8 @@ def satisfied_by_type(self, obj_type): :rtype: bool """ - 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. - """ - - 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)) + def satisfied_by(self, obj): + return self.satisfied_by_type(type(obj)) def __hash__(self): return hash((type(self), self._types)) @@ -336,44 +381,23 @@ def __hash__(self): def __eq__(self, other): return type(self) == type(other) and self._types == other._types - def __ne__(self, other): - return not (self == other) - - def __str__(self): - if self._desc: - constrained_type = '({})'.format(self._desc) - else: - if len(self._types) == 1: - constrained_type = self._types[0].__name__ - else: - constrained_type = '({})'.format(', '.join(t.__name__ for t in self._types)) - return '{variance_symbol}{constrained_type}'.format(variance_symbol=self._variance_symbol, - constrained_type=constrained_type) - def __repr__(self): - if self._desc: - constrained_type = self._desc - else: - constrained_type = ', '.join(t.__name__ for t in self._types) + constrained_type = ', '.join(t.__name__ for t in self._types) return ('{type_constraint_type}({constrained_type})' .format(type_constraint_type=type(self).__name__, - constrained_type=constrained_type)) + constrained_type=constrained_type)) -class SuperclassesOf(TypeConstraint): +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(TypeConstraint): +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 @@ -384,41 +408,66 @@ def graph_str(self): return repr(self) -class SubclassesOf(TypeConstraint): +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 Collection(object): - """Constructs classes representing collections of objects of a particular type. +class TypedCollection(TypeConstraint): + """A `TypeConstraint` which accepts a TypeOnlyConstraint and validates a collection.""" - The produced class will expose its values under a field named dependencies - this is a stable API - which may be consumed e.g. over FFI from the engine. + _iterable_constraint = SubclassesOf(Iterable) - Python consumers of a Collection should prefer to use its standard iteration API. - """ - # TODO: could we check that the input is iterable in the ctor? - - @classmethod - @memoized - def of(cls, *element_types): - union = '|'.join(element_type.__name__ for element_type in element_types) - type_name = '{}.of({})'.format(cls.__name__, union) - if PY2: - type_name = type_name.encode('utf-8') - # TODO: could we allow type checking in the datatype() invocation here? - supertypes = (cls, datatype(['dependencies'], superclass_name='Collection')) - properties = {'element_types': element_types} - collection_of_type = type(type_name, supertypes, properties) - - # Expose the custom class type at the module level to be pickle compatible. - setattr(sys.modules[cls.__module__], type_name, collection_of_type) - - return collection_of_type - - def __iter__(self): - return iter(self.dependencies) + def __init__(self, constraint): + """Create a `TypeConstraint` which validates each member of a collection with `constraint`. + + :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, TypeOnlyConstraint): + raise TypeError("constraint for collection must be a {}! was: {}" + .format(TypeOnlyConstraint.__name__, constraint)) + + description = '{}({})'.format(type(self).__name__, constraint) + + self._constraint = constraint + + 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 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 {} matching iterable object {}: {}" + .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 {}: {}".format(self, base_iterable_error)) + + def __hash__(self): + return hash((type(self), self._constraint)) + + def __eq__(self, other): + return type(self) == type(other) and self._constraint == other._constraint + + def __repr__(self): + return ('{type_constraint_type}({constraint!r})' + .format(type_constraint_type=type(self).__name__, + constraint=self._constraint)) diff --git a/tests/python/pants_test/engine/BUILD b/tests/python/pants_test/engine/BUILD index c5849d62d7d..38b4d2cb4ad 100644 --- a/tests/python/pants_test/engine/BUILD +++ b/tests/python/pants_test/engine/BUILD @@ -156,6 +156,7 @@ python_tests( 'src/python/pants/build_graph', 'src/python/pants/engine:build_files', 'src/python/pants/engine:mapper', + 'src/python/pants/engine:objects', 'src/python/pants/engine:struct', 'src/python/pants/util:dirutil', 'tests/python/pants_test/engine/examples:mapper_test', @@ -232,3 +233,12 @@ python_library( 'src/python/pants/util:dirutil', ] ) + +python_tests( + name='objects', + sources=['test_objects.py'], + dependencies=[ + 'src/python/pants/engine:objects', + 'tests/python/pants_test:test_base', + ], +) diff --git a/tests/python/pants_test/engine/legacy/test_graph_integration.py b/tests/python/pants_test/engine/legacy/test_graph_integration.py index dc8751c068e..2f9b9b167d3 100644 --- a/tests/python/pants_test/engine/legacy/test_graph_integration.py +++ b/tests/python/pants_test/engine/legacy/test_graph_integration.py @@ -61,12 +61,12 @@ def _list_target_check_warnings_sources(self, target_name): _ERR_TARGETS = { 'testprojects/src/python/sources:some-missing-some-not': [ "globs('*.txt', '*.rs')", - "Snapshot(PathGlobs(include=({unicode_literal}\'testprojects/src/python/sources/*.txt\', {unicode_literal}\'testprojects/src/python/sources/*.rs\'), exclude=(), glob_match_error_behavior<=GlobMatchErrorBehavior>=GlobMatchErrorBehavior(failure_behavior=error), conjunction<=GlobExpansionConjunction>=GlobExpansionConjunction(conjunction=all_match)))".format(unicode_literal='u' if PY2 else ''), + "Snapshot(PathGlobs(include=({unicode_literal}\'testprojects/src/python/sources/*.txt\', {unicode_literal}\'testprojects/src/python/sources/*.rs\'), exclude=(), glob_match_error_behavior=GlobMatchErrorBehavior(failure_behavior=error), conjunction=GlobExpansionConjunction(conjunction=all_match)))".format(unicode_literal='u' if PY2 else ''), "Globs did not match. Excludes were: []. Unmatched globs were: [\"testprojects/src/python/sources/*.rs\"].", ], 'testprojects/src/python/sources:missing-sources': [ "*.scala", - "Snapshot(PathGlobs(include=({unicode_literal}\'testprojects/src/python/sources/*.scala\',), exclude=({unicode_literal}\'testprojects/src/python/sources/*Test.scala\', {unicode_literal}\'testprojects/src/python/sources/*Spec.scala\'), glob_match_error_behavior<=GlobMatchErrorBehavior>=GlobMatchErrorBehavior(failure_behavior=error), conjunction<=GlobExpansionConjunction>=GlobExpansionConjunction(conjunction=any_match)))".format(unicode_literal='u' if PY2 else ''), + "Snapshot(PathGlobs(include=({unicode_literal}\'testprojects/src/python/sources/*.scala\',), exclude=({unicode_literal}\'testprojects/src/python/sources/*Test.scala\', {unicode_literal}\'testprojects/src/python/sources/*Spec.scala\'), glob_match_error_behavior=GlobMatchErrorBehavior(failure_behavior=error), conjunction=GlobExpansionConjunction(conjunction=any_match)))".format(unicode_literal='u' if PY2 else ''), "Globs did not match. Excludes were: [\"testprojects/src/python/sources/*Test.scala\", \"testprojects/src/python/sources/*Spec.scala\"]. Unmatched globs were: [\"testprojects/src/python/sources/*.scala\"].", ], 'testprojects/src/java/org/pantsbuild/testproject/bundle:missing-bundle-fileset': [ @@ -75,7 +75,7 @@ def _list_target_check_warnings_sources(self, target_name): "Globs('*.aaaa')", "ZGlobs('**/*.abab')", "['file1.aaaa', 'file2.aaaa']", - "Snapshot(PathGlobs(include=({unicode_literal}\'testprojects/src/java/org/pantsbuild/testproject/bundle/*.aaaa\',), exclude=(), glob_match_error_behavior<=GlobMatchErrorBehavior>=GlobMatchErrorBehavior(failure_behavior=error), conjunction<=GlobExpansionConjunction>=GlobExpansionConjunction(conjunction=all_match)))".format(unicode_literal='u' if PY2 else ''), + "Snapshot(PathGlobs(include=({unicode_literal}\'testprojects/src/java/org/pantsbuild/testproject/bundle/*.aaaa\',), exclude=(), glob_match_error_behavior=GlobMatchErrorBehavior(failure_behavior=error), conjunction=GlobExpansionConjunction(conjunction=all_match)))".format(unicode_literal='u' if PY2 else ''), "Globs did not match. Excludes were: []. Unmatched globs were: [\"testprojects/src/java/org/pantsbuild/testproject/bundle/*.aaaa\"].", ] } diff --git a/tests/python/pants_test/engine/test_engine.py b/tests/python/pants_test/engine/test_engine.py index 1944c2e3260..9ad331b0aa9 100644 --- a/tests/python/pants_test/engine/test_engine.py +++ b/tests/python/pants_test/engine/test_engine.py @@ -123,8 +123,8 @@ def test_include_trace_error_raises_error_with_trace(self): self.assert_equal_with_printing(dedent(''' 1 Exception encountered: - Computing Select(, =A) - Computing Task(nested_raise, , =A, true) + Computing Select(, Exactly(A)) + Computing Task(nested_raise, , Exactly(A), true) Throw(An exception for B) Traceback (most recent call last): File LOCATION-INFO, in call @@ -175,8 +175,8 @@ def a_from_c_and_d(c, d): self.assert_equal_with_printing(dedent(''' 1 Exception encountered: - Computing Select(, =A) - Computing Task(a_from_c_and_d, , =A, true) + Computing Select(, Exactly(A)) + Computing Task(a_from_c_and_d, , Exactly(A), true) Computing Task(d_from_b_nested_raise, , =D, true) Throw(An exception for B) Traceback (most recent call last): @@ -189,8 +189,8 @@ def a_from_c_and_d(c, d): Exception: An exception for B - Computing Select(, =A) - Computing Task(a_from_c_and_d, , =A, true) + Computing Select(, Exactly(A)) + Computing Task(a_from_c_and_d, , Exactly(A), true) Computing Task(c_from_b_nested_raise, , =C, true) Throw(An exception for B) Traceback (most recent call last): diff --git a/tests/python/pants_test/engine/test_mapper.py b/tests/python/pants_test/engine/test_mapper.py index 3c8ce754d36..716d0bcd5ec 100644 --- a/tests/python/pants_test/engine/test_mapper.py +++ b/tests/python/pants_test/engine/test_mapper.py @@ -17,12 +17,12 @@ from pants.engine.fs import create_fs_rules from pants.engine.mapper import (AddressFamily, AddressMap, AddressMapper, DifferingFamiliesError, DuplicateNameError, UnaddressableObjectError) +from pants.engine.objects import Collection from pants.engine.parser import SymbolTable from pants.engine.rules import rule from pants.engine.selectors import Get, Select from pants.engine.struct import Struct from pants.util.dirutil import safe_open -from pants.util.objects import Collection from pants_test.engine.examples.parsers import JsonParser from pants_test.engine.scheduler_test_base import SchedulerTestBase from pants_test.engine.util import Target, TargetTable diff --git a/tests/python/pants_test/engine/test_objects.py b/tests/python/pants_test/engine/test_objects.py new file mode 100644 index 00000000000..0a194edeab8 --- /dev/null +++ b/tests/python/pants_test/engine/test_objects.py @@ -0,0 +1,33 @@ +# coding=utf-8 +# Copyright 2019 Pants project contributors (see CONTRIBUTORS.md). +# Licensed under the Apache License, Version 2.0 (see LICENSE). + +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 '{string_type}') must satisfy this type constraint: Exactly(int).""" + .format(u='' if PY3 else 'u', + string_type='str' if PY3 else 'unicode'))): + 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 or {string_type})) matching iterable object [()]: value () (with type 'tuple') must satisfy this type constraint: Exactly(int or {string_type}).""" + .format(string_type='str' if PY3 else 'unicode'))): + IntOrStringColl([()]) diff --git a/tests/python/pants_test/engine/test_scheduler.py b/tests/python/pants_test/engine/test_scheduler.py index 108d2bda5cc..586fbfb6e00 100644 --- a/tests/python/pants_test/engine/test_scheduler.py +++ b/tests/python/pants_test/engine/test_scheduler.py @@ -122,8 +122,8 @@ def test_trace_includes_rule_exception_traceback(self): trace = remove_locations_from_traceback(trace) assert_equal_with_printing(self, dedent(''' - Computing Select(, =A) - Computing Task(nested_raise, , =A, true) + Computing Select(, Exactly(A)) + Computing Task(nested_raise, , Exactly(A), true) Throw(An exception for B) Traceback (most recent call last): File LOCATION-INFO, in call diff --git a/tests/python/pants_test/util/test_objects.py b/tests/python/pants_test/util/test_objects.py index 4fb24ebbf34..9bf68ccb21b 100644 --- a/tests/python/pants_test/util/test_objects.py +++ b/tests/python/pants_test/util/test_objects.py @@ -12,19 +12,23 @@ from future.utils import PY2, PY3, text_type -from pants.util.objects import (Collection, Exactly, SubclassesOf, SuperclassesOf, TypeCheckError, +from pants.util.objects import (Exactly, SubclassesOf, SuperclassesOf, TypeCheckError, + TypeConstraintError, TypedCollection, TypedDatatypeInstanceConstructionError, datatype, enum) 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])]) - - class TypeConstraintTestBase(TestBase): class A(object): - pass + + def __repr__(self): + return '{}()'.format(type(self).__name__) + + def __str__(self): + return '(str form): {}'.format(repr(self)) + + def __eq__(self, other): + return type(self) == type(other) class B(A): pass @@ -41,9 +45,17 @@ def test_none(self): with self.assertRaises(ValueError): SubclassesOf() + def test_str_and_repr(self): + superclasses_of_b = SuperclassesOf(self.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("SuperclassesOf(A or B)", str(superclasses_of_multiple)) + self.assertEqual("SuperclassesOf(A, B)", repr(superclasses_of_multiple)) + def test_single(self): superclasses_of_b = SuperclassesOf(self.B) - self.assertEqual((self.B,), superclasses_of_b.types) self.assertTrue(superclasses_of_b.satisfied_by(self.A())) self.assertTrue(superclasses_of_b.satisfied_by(self.B())) self.assertFalse(superclasses_of_b.satisfied_by(self.BPrime())) @@ -51,12 +63,19 @@ def test_single(self): def test_multiple(self): superclasses_of_a_or_b = SuperclassesOf(self.A, self.B) - self.assertEqual((self.A, self.B), superclasses_of_a_or_b.types) self.assertTrue(superclasses_of_a_or_b.satisfied_by(self.A())) self.assertTrue(superclasses_of_a_or_b.satisfied_by(self.B())) self.assertFalse(superclasses_of_a_or_b.satisfied_by(self.BPrime())) self.assertFalse(superclasses_of_a_or_b.satisfied_by(self.C())) + def test_validate(self): + superclasses_of_a_or_b = SuperclassesOf(self.A, self.B) + self.assertEqual(self.A(), superclasses_of_a_or_b.validate_satisfied_by(self.A())) + self.assertEqual(self.B(), superclasses_of_a_or_b.validate_satisfied_by(self.B())) + with self.assertRaisesRegexp(TypeConstraintError, + re.escape("value C() (with type 'C') must satisfy this type constraint: SuperclassesOf(A or B).")): + superclasses_of_a_or_b.validate_satisfied_by(self.C()) + class ExactlyTest(TypeConstraintTestBase): def test_none(self): @@ -65,7 +84,6 @@ def test_none(self): def test_single(self): exactly_b = Exactly(self.B) - self.assertEqual((self.B,), exactly_b.types) self.assertFalse(exactly_b.satisfied_by(self.A())) self.assertTrue(exactly_b.satisfied_by(self.B())) self.assertFalse(exactly_b.satisfied_by(self.BPrime())) @@ -73,7 +91,6 @@ def test_single(self): def test_multiple(self): exactly_a_or_b = Exactly(self.A, self.B) - self.assertEqual((self.A, self.B), exactly_a_or_b.types) self.assertTrue(exactly_a_or_b.satisfied_by(self.A())) self.assertTrue(exactly_a_or_b.satisfied_by(self.B())) self.assertFalse(exactly_a_or_b.satisfied_by(self.BPrime())) @@ -84,31 +101,43 @@ def test_disallows_unsplatted_lists(self): Exactly([1]) def test_str_and_repr(self): - exactly_b_types = Exactly(self.B, description='B types') - self.assertEqual("=(B types)", str(exactly_b_types)) - self.assertEqual("Exactly(B types)", repr(exactly_b_types)) - 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): self.assertTrue(Exactly(self.B).satisfied_by_type(self.B)) self.assertFalse(Exactly(self.B).satisfied_by_type(self.C)) + def test_validate(self): + exactly_a_or_b = Exactly(self.A, self.B) + self.assertEqual(self.A(), exactly_a_or_b.validate_satisfied_by(self.A())) + self.assertEqual(self.B(), exactly_a_or_b.validate_satisfied_by(self.B())) + with self.assertRaisesRegexp(TypeConstraintError, + re.escape("value C() (with type 'C') must satisfy this type constraint: Exactly(A or B).")): + exactly_a_or_b.validate_satisfied_by(self.C()) + class SubclassesOfTest(TypeConstraintTestBase): def test_none(self): with self.assertRaises(ValueError): SubclassesOf() + def test_str_and_repr(self): + subclasses_of_b = SubclassesOf(self.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("SubclassesOf(A or B)", str(subclasses_of_multiple)) + self.assertEqual("SubclassesOf(A, B)", repr(subclasses_of_multiple)) + def test_single(self): subclasses_of_b = SubclassesOf(self.B) - self.assertEqual((self.B,), subclasses_of_b.types) self.assertFalse(subclasses_of_b.satisfied_by(self.A())) self.assertTrue(subclasses_of_b.satisfied_by(self.B())) self.assertFalse(subclasses_of_b.satisfied_by(self.BPrime())) @@ -116,12 +145,62 @@ def test_single(self): def test_multiple(self): subclasses_of_b_or_c = SubclassesOf(self.B, self.C) - self.assertEqual((self.B, self.C), subclasses_of_b_or_c.types) self.assertTrue(subclasses_of_b_or_c.satisfied_by(self.B())) self.assertTrue(subclasses_of_b_or_c.satisfied_by(self.C())) self.assertFalse(subclasses_of_b_or_c.satisfied_by(self.BPrime())) self.assertFalse(subclasses_of_b_or_c.satisfied_by(self.A())) + def test_validate(self): + subclasses_of_a_or_b = SubclassesOf(self.A, self.B) + self.assertEqual(self.A(), subclasses_of_a_or_b.validate_satisfied_by(self.A())) + self.assertEqual(self.B(), subclasses_of_a_or_b.validate_satisfied_by(self.B())) + self.assertEqual(self.C(), subclasses_of_a_or_b.validate_satisfied_by(self.C())) + with self.assertRaisesRegexp(TypeConstraintError, + re.escape("value 1 (with type 'int') must satisfy this type constraint: SubclassesOf(A or B).")): + subclasses_of_a_or_b.validate_satisfied_by(1) + + +class TypedCollectionTest(TypeConstraintTestBase): + def test_str_and_repr(self): + collection_of_exactly_b = TypedCollection(Exactly(self.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("TypedCollection(SubclassesOf(A or B))", + str(collection_of_multiple_subclasses)) + self.assertEqual("TypedCollection(SubclassesOf(A, B))", + repr(collection_of_multiple_subclasses)) + + def test_collection_single(self): + collection_constraint = TypedCollection(Exactly(self.A)) + self.assertTrue(collection_constraint.satisfied_by([self.A()])) + self.assertFalse(collection_constraint.satisfied_by([self.A(), self.B()])) + self.assertTrue(collection_constraint.satisfied_by([self.A(), self.A()])) + + def test_collection_multiple(self): + collection_constraint = TypedCollection(SubclassesOf(self.B, self.BPrime)) + self.assertTrue(collection_constraint.satisfied_by([self.B(), self.C(), self.BPrime()])) + self.assertFalse(collection_constraint.satisfied_by([self.B(), self.A()])) + + 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 TypeOnlyConstraint! was: {}".format(sub_collection))): + TypedCollection(sub_collection) + + def test_validate(self): + collection_exactly_a_or_b = TypedCollection(Exactly(self.A, self.B)) + self.assertEqual([self.A()], collection_exactly_a_or_b.validate_satisfied_by([self.A()])) + self.assertEqual([self.B()], collection_exactly_a_or_b.validate_satisfied_by([self.B()])) + with self.assertRaisesRegexp(TypeConstraintError, + re.escape("in wrapped constraint TypedCollection(Exactly(A or B)): value A() (with type 'A') must satisfy this type constraint: SubclassesOf(Iterable).")): + collection_exactly_a_or_b.validate_satisfied_by(self.A()) + with self.assertRaisesRegexp(TypeConstraintError, + re.escape("in wrapped constraint TypedCollection(Exactly(A or B)) matching iterable object [C()]: value C() (with type 'C') must satisfy this type constraint: Exactly(A or B).")): + collection_exactly_a_or_b.validate_satisfied_by([self.C()]) + class ExportedDatatype(datatype(['val'])): pass @@ -175,6 +254,12 @@ def __repr__(self): class WithSubclassTypeConstraint(datatype([('some_value', SubclassesOf(SomeBaseClass))])): pass +class WithCollectionTypeConstraint(datatype([ + ('dependencies', TypedCollection(Exactly(int))), +])): + pass + + class NonNegativeInt(datatype([('an_int', int)])): """Example of overriding __new__() to perform deeper argument checking.""" @@ -392,7 +477,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') @@ -402,7 +487,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') @@ -414,7 +499,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 @@ -424,7 +509,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) @@ -433,7 +518,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') @@ -448,7 +533,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 ')) @@ -457,7 +542,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') @@ -468,6 +553,14 @@ def compare_str(unicode_type_name): self.assertEqual(obj_with_mixin.as_str(), ' asdf ') self.assertEqual(obj_with_mixin.stripped(), 'asdf') + 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=[3])", + str(obj_with_collection)) + self.assertEqual("WithCollectionTypeConstraint(dependencies=[3])", + repr(obj_with_collection)) + def test_instance_construction_errors(self): with self.assertRaises(TypeError) as cm: SomeTypedDatatype(something=3) @@ -574,6 +667,21 @@ def compare_str(unicode_type_name, include_unicode=False): 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.assertRaises(TypeCheckError) as cm: + WithCollectionTypeConstraint(3) + expected_msg = """\ +error: in constructor of type WithCollectionTypeConstraint: type check error: +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: in wrapped constraint TypedCollection(Exactly(int)) matching iterable object [3, {u}'asdf']: value {u}'asdf' (with type '{string_type}') must satisfy this type constraint: Exactly(int).\ +""".format(u='u' if PY2 else '', string_type='unicode' if PY2 else 'str') + self.assertEqual(str(cm.exception), expected_msg) + def test_copy(self): obj = AnotherTypedDatatype(string='some_string', elements=[1, 2, 3]) new_obj = obj.copy(string='another_string')