Make TraitListObject use the CTrait to validate items, not the handler (

#1625)

* Use the CTrait's validation instead of the handler validation in TraitListObject and friends

* Add regression tests

* Add changelog note highlighting the possibility of backwards incompatibility with Tuple

CHANGES.rst

@@ -1,6 +1,30 @@
 Traits CHANGELOG
 ================
 
+Release 7.0.0
+-------------
+
+TBD Release summary
+
+Released: XXXX-XX-XX
+
+Migrating from earlier versions of Traits
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Traits 7.0 should be largely backwards compatible with earlier versions
+of Traits, but there are some things to watch out for.
+
+* Validation of items within a container (e.g., ``foos = List(MyTraitType)``)
+  now always matches the validation used for the item trait at top level (e.g.,
+  ``foo = MyTraitType``). Previously, the validation methods used could differ,
+  thanks to a bug in the container implementations. For most trait types this
+  will make no difference, but for the ``Tuple`` trait type this change has the
+  consequence that lists will no longer be accepted as valid for ``Tuple``
+  traits inside list items. See issue #1619 and PR #1625 for more information.
+
+TBD Release details
+
+
 Release 6.3.2
 -------------
 

traits/tests/test_trait_dict_object.py

@@ -14,7 +14,7 @@
 import unittest
 from unittest import mock
 
-from traits.api import HasTraits
+from traits.api import DefaultValue, HasTraits, TraitType, ValidateTrait
 from traits.trait_dict_object import TraitDict, TraitDictEvent, TraitDictObject
 from traits.trait_errors import TraitError
 from traits.trait_types import Dict, Int, Str
@@ -34,6 +34,18 @@ def int_validator(value):
         raise TraitError
 
 
+class RangeInstance(TraitType):
+    """
+    Dummy custom trait type for use in validation tests.
+    """
+
+    default_value_type = DefaultValue.constant
+
+    default_value = range(10)
+
+    fast_validate = ValidateTrait.coerce, range
+
+
 class TestTraitDict(unittest.TestCase):
 
     def setUp(self):
@@ -434,6 +446,46 @@ def test_trait_dict_object_pickle(self):
         tdo_unpickled.value_validator(1)
         tdo_unpickled.value_validator(True)
 
+    def test_disconnected_dict(self):
+        # Objects that are disconnected from their HasTraits "owner" can arise
+        # as a result of clone_traits operations, or of serialization and
+        # deserialization.
+        disconnected = TraitDictObject(
+            trait=Dict(Str, Str),
+            object=None,
+            name="foo",
+            value={},
+        )
+        self.assertEqual(disconnected.object(), None)
+
+    def test_key_validation_uses_ctrait(self):
+        # Regression test for enthought/traits#1619
+
+        class HasRanges(HasTraits):
+            ranges = Dict(RangeInstance(), Int())
+
+        obj = HasRanges()
+
+        with self.assertRaises(TraitError):
+            obj.ranges[3] = 27
+
+        obj.ranges[range(10, 20)] = 3
+        self.assertEqual(obj.ranges, {range(10, 20): 3})
+
+    def test_value_validation_uses_ctrait(self):
+        # Regression test for enthought/traits#1619
+
+        class HasRanges(HasTraits):
+            ranges = Dict(Int(), RangeInstance())
+
+        obj = HasRanges()
+
+        with self.assertRaises(TraitError):
+            obj.ranges[3] = 27
+
+        obj.ranges[3] = range(10, 20)
+        self.assertEqual(obj.ranges, {3: range(10, 20)})
+
 
 class TestTraitDictEvent(unittest.TestCase):
 
@@ -455,15 +507,3 @@ class DifferentName(TraitDictEvent):
         differnt_name_subclass = DifferentName()
         self.assertEqual(desired_repr, str(differnt_name_subclass))
         self.assertEqual(desired_repr, repr(differnt_name_subclass))
-
-    def test_disconnected_dict(self):
-        # Objects that are disconnected from their HasTraits "owner" can arise
-        # as a result of clone_traits operations, or of serialization and
-        # deserialization.
-        disconnected = TraitDictObject(
-            trait=Dict(Str, Str),
-            object=None,
-            name="foo",
-            value={},
-        )
-        self.assertEqual(disconnected.object(), None)

traits/tests/test_trait_list_object.py

@@ -13,7 +13,14 @@
 import pickle
 import unittest.mock
 
-from traits.api import HasTraits, Int, List
+from traits.api import (
+    DefaultValue,
+    HasTraits,
+    Int,
+    List,
+    TraitType,
+    ValidateTrait,
+)
 from traits.testing.optional_dependencies import numpy, requires_numpy
 from traits.trait_base import _validate_everything
 from traits.trait_errors import TraitError
@@ -1469,3 +1476,24 @@ def test_disconnected_list(self):
             value=[1, 2, 3],
         )
         self.assertEqual(disconnected.object(), None)
+
+    def test_item_validation_uses_ctrait(self):
+        # Regression test for enthought/traits#1619
+
+        class RangeInstance(TraitType):
+            default_value_type = DefaultValue.constant
+
+            default_value = range(10)
+
+            fast_validate = ValidateTrait.coerce, range
+
+        class HasRanges(HasTraits):
+            ranges = List(RangeInstance())
+
+        obj = HasRanges()
+
+        with self.assertRaises(TraitError):
+            obj.ranges.append(23)
+
+        obj.ranges.append(range(10, 20))
+        self.assertEqual(obj.ranges, [range(10, 20)])

traits/tests/test_trait_set_object.py

@@ -11,7 +11,14 @@
 import unittest
 from unittest import mock
 
-from traits.api import HasTraits, Set, Str
+from traits.api import (
+    DefaultValue,
+    HasTraits,
+    Set,
+    Str,
+    TraitType,
+    ValidateTrait,
+)
 from traits.trait_base import _validate_everything
 from traits.trait_errors import TraitError
 from traits.trait_set_object import TraitSet, TraitSetEvent, TraitSetObject
@@ -529,6 +536,27 @@ def test_disconnected_set(self):
         )
         self.assertEqual(disconnected.object(), None)
 
+    def test_item_validation_uses_ctrait(self):
+        # Regression test for enthought/traits#1619
+
+        class RangeInstance(TraitType):
+            default_value_type = DefaultValue.constant
+
+            default_value = range(10)
+
+            fast_validate = ValidateTrait.coerce, range
+
+        class HasRanges(HasTraits):
+            ranges = Set(RangeInstance())
+
+        obj = HasRanges()
+
+        with self.assertRaises(TraitError):
+            obj.ranges.add(23)
+
+        obj.ranges.add(range(10, 20))
+        self.assertEqual(obj.ranges, {range(10, 20)})
+
 
 class TestTraitSetEvent(unittest.TestCase):
 

traits/trait_dict_object.py

@@ -450,7 +450,7 @@ def __init__(self, trait, object, name, value):
                          notifiers=[self.notifier])
 
     def _key_validator(self, key):
-        """ Calls the trait's key_trait.handler.validate.
+        """ Key validator based on the Dict's key_trait.
 
         Parameters
         ----------
@@ -476,7 +476,7 @@ def _key_validator(self, key):
         if trait is None or object is None:
             return key
 
-        validate = trait.key_trait.handler.validate
+        validate = trait.key_trait.validate
         if validate is None:
             return key
 
@@ -487,7 +487,7 @@ def _key_validator(self, key):
             raise excep
 
     def _value_validator(self, value):
-        """ Calls the trait's value_handler.validate
+        """ Value validator based on the Dict's value_trait.
 
         Parameters
         ----------
@@ -513,7 +513,7 @@ def _value_validator(self, value):
         if trait is None or object is None:
             return value
 
-        validate = trait.value_handler.validate
+        validate = trait.value_trait.validate
         if validate is None:
             return value
 

traits/trait_list_object.py

@@ -859,7 +859,7 @@ def _item_validator(self, value):
         if object is None:
             return value
 
-        trait_validator = self.trait.item_trait.handler.validate
+        trait_validator = self.trait.item_trait.validate
         if trait_validator is None:
             return value
 

traits/trait_set_object.py

@@ -511,7 +511,7 @@ def _validator(self, value):
         object = object_ref()
 
         # validate the new value(s)
-        validate = trait.item_trait.handler.validate
+        validate = trait.item_trait.validate
 
         if validate is None:
             return value

traits/trait_types.py

@@ -3077,6 +3077,8 @@ def __init__(
         if (handler is not None) and handler.has_items:
             handler = handler.clone()
             handler.has_items = False
+        # This attribute isn't actually used for anything, but we keep it
+        # for backwards compatibility.
         self.value_handler = handler
 
         super().__init__(value, **metadata)