From 4dca1f2ef4671706bddea37cc01f16011bcc66c0 Mon Sep 17 00:00:00 2001
From: Mark Dickinson <mdickinson@enthought.com>
Date: Tue, 2 Jun 2020 09:52:58 +0100
Subject: [PATCH] Ensure length check on unconstrained list always passes
 (#1172)

* Ensure length check on unconstrained list always passes

* Add changelog entry

* Same fix for __delitem__
---
 CHANGES.rst                            |  2 +-
 traits/tests/test_trait_list_object.py | 19 +++++++++++++++++++
 traits/trait_list_object.py            |  6 +++---
 3 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/CHANGES.rst b/CHANGES.rst
index c0ec7609d..606d92c2b 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -164,7 +164,7 @@ Features
 * Add ``Map`` and ``PrefixMap`` trait types. (#886, #953, #956, #970, #1139)
 * Add ``TraitList`` as the base list object that can perform validation
   and emit change notifications. (#912, #981, #984, #989, #999, #1003, #1011,
-  #1026, #1009, #1040, #1173)
+  #1026, #1009, #1040, #1172, #1173)
 * Add ``TraitDict`` as the base dict object that can perform validation and
   emit change notifications. (#913)
 * Add ``TraitSet`` as the base set object that can perform validation and
diff --git a/traits/tests/test_trait_list_object.py b/traits/tests/test_trait_list_object.py
index 50358706c..8af740cb4 100644
--- a/traits/tests/test_trait_list_object.py
+++ b/traits/tests/test_trait_list_object.py
@@ -1150,6 +1150,8 @@ class HasLengthConstrainedLists(HasTraits):
 
     at_most_five = List(Int, maxlen=5)
 
+    unconstrained = List(Int)
+
 
 class TestTraitListObject(unittest.TestCase):
     def test_list_of_lists_pickle_with_notifier(self):
@@ -1214,6 +1216,11 @@ def test_delitem_slice_too_small(self):
             del foo.at_least_two[:]
         self.assertEqual(foo.at_least_two, [1, 2])
 
+    def test_delitem_from_empty(self):
+        foo = HasLengthConstrainedLists()
+        with self.assertRaises(IndexError):
+            del foo.unconstrained[0]
+
     def test_iadd(self):
         foo = HasLengthConstrainedLists(at_most_five=[1, 2])
         foo.at_most_five += [6, 7, 8]
@@ -1381,6 +1388,13 @@ def test_pop_too_small(self):
             foo.at_least_two.pop(10)
         self.assertEqual(foo.at_least_two, [1, 2])
 
+    def test_pop_from_empty(self):
+        foo = HasLengthConstrainedLists()
+        with self.assertRaises(IndexError):
+            foo.unconstrained.pop()
+        with self.assertRaises(IndexError):
+            foo.unconstrained.pop(10)
+
     def test_remove(self):
         foo = HasLengthConstrainedLists(at_least_two=[1, 2, 6, 4])
         foo.at_least_two.remove(2)
@@ -1398,6 +1412,11 @@ def test_remove_too_small(self):
             foo.at_least_two.remove(10)
         self.assertEqual(foo.at_least_two, [1, 2])
 
+    def test_remove_from_empty(self):
+        foo = HasLengthConstrainedLists()
+        with self.assertRaises(ValueError):
+            foo.unconstrained.remove(35)
+
     def test_length_violation_error_message(self):
         # Regression test for enthought/traits#1170
         foo = HasLengthConstrainedLists(at_least_two=[1, 2])
diff --git a/traits/trait_list_object.py b/traits/trait_list_object.py
index d94c03bac..b4309503b 100644
--- a/traits/trait_list_object.py
+++ b/traits/trait_list_object.py
@@ -634,7 +634,7 @@ def __delitem__(self, key):
         """
 
         removed_count = len(self[key]) if isinstance(key, slice) else 1
-        self._validate_length(len(self) - removed_count)
+        self._validate_length(max(len(self) - removed_count, 0))
         super().__delitem__(key)
 
     def __iadd__(self, value):
@@ -776,7 +776,7 @@ def pop(self, index=-1):
             If list is empty or index is out of range.
         """
 
-        self._validate_length(len(self) - 1)
+        self._validate_length(max(len(self) - 1, 0))
         return super().pop(index)
 
     def remove(self, value):
@@ -797,7 +797,7 @@ def remove(self, value):
             If the value is not present.
         """
 
-        self._validate_length(len(self) - 1)
+        self._validate_length(max(len(self) - 1, 0))
         super().remove(value)
 
     # -- pickle and copy support ----------------------------------------------