fixup! Use set instead of list for block level elements

Author: pawamoy

markdown/util.py

@@ -79,7 +79,9 @@ def __delitem__(self, index: int, /) -> None:
         )
         element = self._list[index]
         del self._list[index]
-        self._set.remove(element)
+        # Only remove from set if absent from list.
+        if element not in self._list:
+            self._set.remove(element)
 
     def __getitem__(self, index: int, /) -> str:
         warnings.warn(
@@ -163,7 +165,9 @@ def __setitem__(self, index: int, value: str, /) -> None:
         # In-place item-setting should update both list and set.
         old = self._list[index]
         self._list[index] = value
-        self._set.discard(old)
+        # Only remove from set if absent from list.
+        if old not in self._list:
+            self._set.discard(old)
         self._set.add(value)
 
     def __str__(self) -> str:
@@ -262,16 +266,34 @@ def issubset(self, other: set[str], /) -> bool:
     def issuperset(self, other: set[str], /) -> bool:
         return self._set.issuperset(other)
 
-    def pop(self, index: int = -1, /) -> str:
+    def pop(self, index: int | None = None, /) -> str:
         # In-place pop should update both list and set.
+        if index is None:
+            index = -1
+        else:
+            warnings.warn(
+                "Using block level elements as a list is deprecated, use it as a set instead.",
+                DeprecationWarning,
+            )
         element = self._list.pop(index)
-        self._set.remove(element)
+        # Only remove from set if absent from list.
+        if element not in self._list:
+            self._set.remove(element)
         return element
 
     def remove(self, element: str, /) -> None:
         # In-place removal should update both list and set.
-        self._list.remove(element)
-        self._set.remove(element)
+        # We give precedence to set behavior, so we remove all occurrences from the list.
+        while True:
+            try:
+                self._list.remove(element)
+            except ValueError:
+                break
+        # We raise ValueError for backwards compatibility.
+        try:
+            self._set.remove(element)
+        except KeyError:
+            raise ValueError(f"{element!r} not in list") from None
 
     def reverse(self) -> None:
         warnings.warn(

tests/test_block_level_elements.py

@@ -43,13 +43,25 @@ def test__init__(self):
         self.assertEqual(ble._list, [])
         self.assertEqual(ble._set, set())
 
+    def test__init__duplicates(self):
+        ble = _BlockLevelElements(["a", "a", "b"])
+        self.assertEqual(ble._list, ["a", "a", "b"])
+        self.assertEqual(ble._set, {"a", "b"})
+
     def test___add__(self):
-        ble = _BlockLevelElements(["a", "b", ])
+        ble = _BlockLevelElements(["a", "b"])
         with self.assertWarns(DeprecationWarning):
             ble2 = ble + ["c", "d"]
         self.assertIsInstance(ble2, list)
         self.assertEqual(ble2, ["a", "b", "c", "d"])
 
+    def test___add__duplicates(self):
+        ble = _BlockLevelElements(["a", "b"])
+        with self.assertWarns(DeprecationWarning):
+            ble2 = ble + ["a", "b"]
+        self.assertIsInstance(ble2, list)
+        self.assertEqual(ble2, ["a", "b", "a", "b"])
+
     def test___and__(self):
         ble = _BlockLevelElements(["a", "b"])
         ble2 = ble & {"b", "c"}
@@ -74,6 +86,13 @@ def test___delitem__(self):
         with self.assertWarns(DeprecationWarning):
             self.assertRaises(IndexError, ble.__delitem__, 10)
 
+    def test___delitem__duplicates(self):
+        ble = _BlockLevelElements(["a", "a", "b"])
+        with self.assertWarns(DeprecationWarning):
+            del ble[0]
+        self.assertEqual(ble._list, ["a", "b"])
+        self.assertEqual(ble._set, {"a", "b"})
+
     def test___getitem__(self):
         ble = _BlockLevelElements(["a", "b", "c"])
         with self.assertWarns(DeprecationWarning):
@@ -82,13 +101,27 @@ def test___getitem__(self):
             self.assertEqual(ble[2], "c")
             self.assertRaises(IndexError, ble.__getitem__, 10)
 
+    def test___getitem__duplicates(self):
+        ble = _BlockLevelElements(["a", "a", "b"])
+        with self.assertWarns(DeprecationWarning):
+            self.assertEqual(ble[0], "a")
+            self.assertEqual(ble[1], "a")
+            self.assertEqual(ble[2], "b")
+
     def test___iadd__(self):
         ble = _BlockLevelElements(["a", "b"])
         with self.assertWarns(DeprecationWarning):
             ble += ["c", "d"]
         self.assertEqual(ble._list, ["a", "b", "c", "d"])
         self.assertEqual(ble._set, {"a", "b", "c", "d"})
 
+    def test___iadd__duplicates(self):
+        ble = _BlockLevelElements(["a", "b"])
+        with self.assertWarns(DeprecationWarning):
+            ble += ["a", "b"]
+        self.assertEqual(ble._list, ["a", "b", "a", "b"])
+        self.assertEqual(ble._set, {"a", "b"})
+
     def test___iand__(self):
         ble = _BlockLevelElements(["a", "b"])
         ble &= {"b", "c"}
@@ -105,11 +138,19 @@ def test___iter__(self):
         ble = _BlockLevelElements(["a", "b", "c"])
         self.assertEqual(tuple(ble), ("a", "b", "c"))
 
+    def test___iter__duplicates(self):
+        ble = _BlockLevelElements(["a", "a", "b"])
+        self.assertEqual(tuple(ble), ("a", "a", "b"))
+
     def test___len__(self):
         self.assertEqual(len(_BlockLevelElements([])), 0)
-        self.assertEqual(len(_BlockLevelElements(["a", "a"])), 2)
+        self.assertEqual(len(_BlockLevelElements(["a", "b"])), 2)
         self.assertEqual(len(_BlockLevelElements(["a", "b", "c"])), 3)
 
+    def test___len__duplicates(self):
+        self.assertEqual(len(_BlockLevelElements(["a", "a"])), 2)
+        self.assertEqual(len(_BlockLevelElements(["a", "a", "b"])), 3)
+
     def test___or__(self):
         ble = _BlockLevelElements(["a", "b"])
         ble2 = ble | {"b", "c"}
@@ -157,6 +198,11 @@ def test___reversed__(self):
         with self.assertWarns(DeprecationWarning):
             self.assertEqual(tuple(reversed(ble)), ("c", "b", "a"))
 
+    def test___reversed__duplicates(self):
+        ble = _BlockLevelElements(["a", "a", "b"])
+        with self.assertWarns(DeprecationWarning):
+            self.assertEqual(tuple(reversed(ble)), ("b", "a", "a"))
+
     def test___setitem__(self):
         ble = _BlockLevelElements(["a", "b", "c"])
         with self.assertWarns(DeprecationWarning):
@@ -170,6 +216,17 @@ def test___setitem__(self):
         with self.assertWarns(DeprecationWarning):
             self.assertRaises(IndexError, ble.__setitem__, 10, "f")
 
+    def test___setitem__duplicates(self):
+        ble = _BlockLevelElements(["a", "a", "b"])
+        with self.assertWarns(DeprecationWarning):
+            ble[0] = "b"
+        self.assertEqual(ble._list, ["b", "a", "b"])
+        self.assertEqual(ble._set, {"a", "b"})
+        with self.assertWarns(DeprecationWarning):
+            ble[1] = "b"
+        self.assertEqual(ble._list, ["b", "b", "b"])
+        self.assertEqual(ble._set, {"b"})
+
     def test___str__(self):
         ble = _BlockLevelElements(["a"])
         self.assertEqual(str(ble), "{'a'}")
@@ -180,13 +237,26 @@ def test_add(self):
         self.assertEqual(ble._list, ["a", "b", "c"])
         self.assertEqual(ble._set, {"a", "b", "c"})
 
+    def test_add_duplicates(self):
+        ble = _BlockLevelElements(["a", "b"])
+        ble.add("a")
+        self.assertEqual(ble._list, ["a", "b", "a"])
+        self.assertEqual(ble._set, {"a", "b"})
+
     def test_append(self):
         ble = _BlockLevelElements(["a", "b"])
         with self.assertWarns(DeprecationWarning):
             ble.append("c")
         self.assertEqual(ble._list, ["a", "b", "c"])
         self.assertEqual(ble._set, {"a", "b", "c"})
 
+    def test_append_duplicates(self):
+        ble = _BlockLevelElements(["a", "b"])
+        with self.assertWarns(DeprecationWarning):
+            ble.append("a")
+        self.assertEqual(ble._list, ["a", "b", "a"])
+        self.assertEqual(ble._set, {"a", "b"})
+
     def test_clear(self):
         ble = _BlockLevelElements(["a", "b"])
         ble.clear()
@@ -200,13 +270,25 @@ def test_copy(self):
         self.assertEqual(ble2._list, ["a", "b"])
         self.assertEqual(ble2._set, {"a", "b"})
 
+    def test_copy_duplicates(self):
+        ble = _BlockLevelElements(["a", "a"])
+        ble2 = ble.copy()
+        self.assertIsNot(ble2, ble)
+        self.assertEqual(ble2._list, ["a", "a"])
+        self.assertEqual(ble2._set, {"a"})
+
     def test_count(self):
-        ble = _BlockLevelElements(["a", "b", "a"])
+        ble = _BlockLevelElements(["a", "b"])
         with self.assertWarns(DeprecationWarning):
-            self.assertEqual(ble.count("a"), 2)
+            self.assertEqual(ble.count("a"), 1)
             self.assertEqual(ble.count("b"), 1)
             self.assertEqual(ble.count("c"), 0)
 
+    def test_count_duplicates(self):
+        ble = _BlockLevelElements(["a", "a"])
+        with self.assertWarns(DeprecationWarning):
+            self.assertEqual(ble.count("a"), 2)
+
     def test_difference(self):
         ble = _BlockLevelElements(["a", "b"])
         ble2 = ble.difference({"b", "c"})
@@ -222,6 +304,7 @@ def test_difference_update(self):
     def test_discard(self):
         ble = _BlockLevelElements(["a", "b"])
         ble.discard("b")
+        ble.discard("b")  # Assert no error.
         self.assertEqual(ble._list, ["a"])
         self.assertEqual(ble._set, {"a"})
 
@@ -232,6 +315,13 @@ def test_extend(self):
         self.assertEqual(ble._list, ["a", "b", "c", "d"])
         self.assertEqual(ble._set, {"a", "b", "c", "d"})
 
+    def test_extend_duplicates(self):
+        ble = _BlockLevelElements(["a", "b"])
+        with self.assertWarns(DeprecationWarning):
+            ble.extend(["a", "b"])
+        self.assertEqual(ble._list, ["a", "b", "a", "b"])
+        self.assertEqual(ble._set, {"a", "b"})
+
     def test_index(self):
         ble = _BlockLevelElements(["a", "b", "c"])
         with self.assertWarns(DeprecationWarning):
@@ -240,6 +330,13 @@ def test_index(self):
             self.assertEqual(ble.index("c"), 2)
             self.assertRaises(ValueError, ble.index, "d")
 
+    def test_index_duplicates(self):
+        ble = _BlockLevelElements(["a", "b", "b"])
+        with self.assertWarns(DeprecationWarning):
+            self.assertEqual(ble.index("b"), 1)
+        with self.assertWarns(DeprecationWarning):
+            self.assertEqual(ble.index("b", 2), 2)
+
     def test_insert(self):
         ble = _BlockLevelElements(["a", "b", "c"])
         with self.assertWarns(DeprecationWarning):
@@ -250,6 +347,13 @@ def test_insert(self):
             ble.insert(100, "e")
         self.assertIn("e", ble)
 
+    def test_insert_duplicates(self):
+        ble = _BlockLevelElements(["a", "a", "b"])
+        with self.assertWarns(DeprecationWarning):
+            ble.insert(1, "b")
+        self.assertEqual(ble._list, ["a", "b", "a", "b"])
+        self.assertEqual(ble._set, {"a", "b"})
+
     def test_intersection(self):
         ble = _BlockLevelElements(["a", "b"])
         ble2 = ble.intersection({"b", "c"})
@@ -288,27 +392,59 @@ def test_pop(self):
         self.assertEqual(ble._set, {"b"})
         self.assertRaises(IndexError, ble.pop, 10)
 
+    def test_pop_duplicates(self):
+        ble = _BlockLevelElements(["a", "a", "b", "b"])
+        self.assertEqual(ble.pop(), "b")
+        self.assertEqual(ble._list, ["a", "a", "b"])
+        self.assertEqual(ble._set, {"a", "b"})
+        self.assertEqual(ble.pop(0), "a")
+        self.assertEqual(ble._list, ["a", "b"])
+        self.assertEqual(ble._set, {"a", "b"})
+        self.assertEqual(ble.pop(), "b")
+        self.assertEqual(ble._list, ["a"])
+        self.assertEqual(ble._set, {"a"})
+
     def test_remove(self):
         ble = _BlockLevelElements(["a", "b", "c"])
         ble.remove("b")
         self.assertEqual(ble._list, ["a", "c"])
         self.assertEqual(ble._set, {"a", "c"})
         self.assertRaises(ValueError, ble.remove, "d")
 
+    def test_remove_duplicates(self):
+        ble = _BlockLevelElements(["a", "a", "b"])
+        ble.remove("a")
+        self.assertEqual(ble._list, ["b"])
+        self.assertEqual(ble._set, {"b"})
+
     def test_reverse(self):
         ble = _BlockLevelElements(["a", "b", "c"])
         with self.assertWarns(DeprecationWarning):
             ble.reverse()
         self.assertEqual(ble._list, ["c", "b", "a"])
         self.assertEqual(ble._set, {"a", "b", "c"})
 
+    def test_reverse_duplicates(self):
+        ble = _BlockLevelElements(["a", "a", "b"])
+        with self.assertWarns(DeprecationWarning):
+            ble.reverse()
+        self.assertEqual(ble._list, ["b", "a", "a"])
+        self.assertEqual(ble._set, {"a", "b"})
+
     def test_sort(self):
         ble = _BlockLevelElements(["c", "a", "b"])
         with self.assertWarns(DeprecationWarning):
             ble.sort()
         self.assertEqual(ble._list, ["a", "b", "c"])
         self.assertEqual(ble._set, {"a", "b", "c"})
 
+    def test_sort_duplicates(self):
+        ble = _BlockLevelElements(["b", "a", "b"])
+        with self.assertWarns(DeprecationWarning):
+            ble.sort()
+        self.assertEqual(ble._list, ["a", "b", "b"])
+        self.assertEqual(ble._set, {"a", "b"})
+
     def test_symmetric_difference(self):
         ble = _BlockLevelElements(["a", "b"])
         ble2 = ble.symmetric_difference({"b", "c"})