Skip to content
Permalink
Browse files

add a TypedCollection type constraint to reduce boilerplate for datat…

…ype tuple fields (pantsbuild#7115)

### Problem

*Resolves pantsbuild#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.

pantsbuild#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 pantsbuild#7172.
  • Loading branch information...
cosmicexplorer committed Feb 12, 2019
1 parent 874ce34 commit 3d7a2955842f54467a569e4e5b74dd8ba9f5d67e
@@ -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',
@@ -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)
@@ -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)])):
@@ -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',
@@ -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__)
@@ -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)
@@ -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


Oops, something went wrong.

0 comments on commit 3d7a295

Please sign in to comment.
You can’t perform that action at this time.