Simplify Collection.

src/python/pants/base/specs.py

@@ -414,14 +414,12 @@ class FilesystemSpecs(Collection[FilesystemSpec]):
     @memoized_property
     def includes(self) -> Tuple[Union[FilesystemLiteralSpec, FilesystemGlobSpec], ...]:
         return tuple(
-            spec
-            for spec in self.dependencies
-            if isinstance(spec, (FilesystemGlobSpec, FilesystemLiteralSpec))
+            spec for spec in self if isinstance(spec, (FilesystemGlobSpec, FilesystemLiteralSpec))
         )
 
     @memoized_property
     def ignores(self) -> Tuple[FilesystemIgnoreSpec, ...]:
-        return tuple(spec for spec in self.dependencies if isinstance(spec, FilesystemIgnoreSpec))
+        return tuple(spec for spec in self if isinstance(spec, FilesystemIgnoreSpec))
 
     @staticmethod
     def _generate_path_globs(

src/python/pants/engine/collection.py

@@ -1,14 +1,14 @@
 # Copyright 2020 Pants project contributors (see CONTRIBUTORS.md).
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
-from typing import Any, ClassVar, Iterable, Sequence, TypeVar, Union, overload
+from typing import Any, ClassVar, Iterable, Tuple, TypeVar, Union, cast, overload
 
 from pants.util.ordered_set import FrozenOrderedSet
 
 T = TypeVar("T")
 
 
-class Collection(Sequence[T]):
+class Collection(Tuple[T, ...]):
     """A light newtype around immutable sequences for use with the V2 engine.
 
     This should be subclassed when you want to create a distinct collection type, such as:
@@ -19,12 +19,9 @@ class Example:
 
         class Examples(Collection[Example]):
             pass
-    """
 
-    def __init__(self, dependencies: Iterable[T] = ()) -> None:
-        # TODO: rename to `items`, `elements`, or even make this private. Python consumers should
-        #  not directly access this.
-        self.dependencies = tuple(dependencies)
+    N.B: Collection instances are only considered equal if both their types and contents are equal.
+    """
 
     @overload  # noqa: F811
     def __getitem__(self, index: int) -> T:
@@ -35,23 +32,26 @@ def __getitem__(self, index: slice) -> "Collection[T]":
         ...
 
     def __getitem__(self, index: Union[int, slice]) -> Union[T, "Collection[T]"]:  # noqa: F811
+        result = super().__getitem__(index)
         if isinstance(index, int):
-            return self.dependencies[index]
-        return self.__class__(self.dependencies[index])
+            return cast(T, result)
+        return self.__class__(cast(Tuple[T, ...], result))
 
-    def __len__(self) -> int:
-        return len(self.dependencies)
+    def __eq__(self, other: Any) -> bool:
+        return type(self) == type(other) and super().__eq__(other)
 
-    def __eq__(self, other: Union[Any, "Collection"]) -> bool:
-        if not isinstance(other, self.__class__):
-            return NotImplemented
-        return self.dependencies == other.dependencies
+    def __ne__(self, other: Any) -> bool:
+        # We must explicitly override to provide the inverse of _our_ __eq__ and not get the
+        # inverse of tuple.__eq__.
+        return not self == other
 
-    def __hash__(self) -> int:
-        return hash(self.dependencies)
+    # Unlike in Python 2 we must explicitly implement __hash__ since we explicitly implement __eq__
+    # per the Python 3 data model.
+    # See: https://docs.python.org/3/reference/datamodel.html#object.__hash__
+    __hash__ = Tuple.__hash__
 
     def __repr__(self) -> str:
-        return f"{self.__class__.__name__}({list(self.dependencies)})"
+        return f"{self.__class__.__name__}({list(self)})"
 
 
 # NB: This name is clunky. It would be more appropriate to call it `Set`, but that is claimed by

src/python/pants/engine/collection_test.py

@@ -46,6 +46,9 @@ def test_collection_reversed() -> None:
 
 
 def test_collection_equality() -> None:
+    assert () != Collection()
+    assert Collection() != ()
+
     assert Collection([]) == Collection([])
     c1 = Collection([1, 2, 3])
     assert c1 == Collection([1, 2, 3])

src/rust/engine/src/externs/interface.rs

@@ -1339,7 +1339,7 @@ fn capture_snapshots(
   session_ptr: PySession,
   path_globs_and_root_tuple_wrapper: PyObject,
 ) -> CPyResult<PyObject> {
-  let values = externs::project_multi(&path_globs_and_root_tuple_wrapper.into(), "dependencies");
+  let values = externs::project_iterable(&path_globs_and_root_tuple_wrapper.into());
   let path_globs_and_roots = values
     .iter()
     .map(|value| {
@@ -1398,7 +1398,7 @@ fn merge_directories(
   session_ptr: PySession,
   directories_value: PyObject,
 ) -> CPyResult<PyObject> {
-  let digests = externs::project_multi(&directories_value.into(), "dependencies")
+  let digests = externs::project_iterable(&directories_value.into())
     .iter()
     .map(|v| nodes::lift_digest(v))
     .collect::<Result<Vec<_>, _>>()

src/rust/engine/src/externs/mod.rs

@@ -164,7 +164,7 @@ pub fn store_bool(val: bool) -> Value {
 ///
 /// Gets an attribute of the given value as the given type.
 ///
-pub fn getattr<T>(value: &Value, field: &str) -> Result<T, String>
+fn getattr<T>(value: &Value, field: &str) -> Result<T, String>
 where
   for<'a> T: FromPyObject<'a>,
 {
@@ -184,13 +184,45 @@ where
     })
 }
 
+///
+/// Collect the Values contained within an outer Python Iterable Value.
+///
+fn collect_iterable(value: &Value) -> Result<Vec<Value>, String> {
+  let gil = Python::acquire_gil();
+  let py = gil.python();
+  match value.iter(py) {
+    Ok(py_iter) => py_iter
+      .enumerate()
+      .map(|(i, py_res)| {
+        py_res.map(Value::from).map_err(|py_err| {
+          format!(
+            "Could not iterate {}, failed to extract {}th item: {:?}",
+            val_to_str(value),
+            i,
+            py_err
+          )
+        })
+      })
+      .collect(),
+    Err(py_err) => Err(format!(
+      "Could not iterate {}: {:?}",
+      val_to_str(value),
+      py_err
+    )),
+  }
+}
+
 ///
 /// Pulls out the value specified by the field name from a given Value
 ///
 pub fn project_ignoring_type(value: &Value, field: &str) -> Value {
   getattr(value, field).unwrap()
 }
 
+pub fn project_iterable(value: &Value) -> Vec<Value> {
+  collect_iterable(value).unwrap()
+}
+
 pub fn project_multi(value: &Value, field: &str) -> Vec<Value> {
   getattr(value, field).unwrap()
 }

src/rust/engine/src/intrinsics.rs

@@ -314,7 +314,7 @@ fn create_digest_to_digest(
   context: Context,
   args: Vec<Value>,
 ) -> BoxFuture<'static, NodeResult<Value>> {
-  let file_values = externs::project_multi(&args[0], "dependencies");
+  let file_values = externs::project_iterable(&args[0]);
   let digests: Vec<_> = file_values
     .iter()
     .map(|file| {