Merge pull request #235 from derek73/fix/issue-225-iterator-cursor

Fix HumanName acting as its own iterator with a stored cursor

Author: derek73

docs/release_log.rst

@@ -1,6 +1,7 @@
 Release Log
 ===========
 * 1.3.0 - Unreleased
+    - Fix ``HumanName`` acting as its own iterator with a stored cursor: breaking out of a loop, iterating in nested loops, or calling ``len(name)`` mid-loop corrupted subsequent iteration; ``iter(name)`` now returns a fresh independent iterator each time. ``next(name)`` on the instance itself (undocumented) now raises ``TypeError`` — call ``next(iter(name))`` instead (closes #225)
     - Fix parsing writing back into the ``Constants`` it reads (usually the shared module-level ``CONSTANTS``): pieces derived while parsing a name — period-joined titles/suffixes like ``"Lt.Gov."`` and conjunction-joined pieces like ``"Mr. and Mrs."`` or ``"von und zu"`` — are now tracked per parse instead of being permanently ``add()``-ed to the config, so parse results no longer depend on which names were parsed earlier in the process and parsing no longer mutates shared state across threads
     - Remove the vestigial ``unparsable`` attribute: the guard that was meant to set it has been unreachable since 2013 (v0.2.9), so it has reported ``False`` for every parsed name for over a decade; check ``len(name) == 0`` to detect an empty parse
     - Fix ``__hash__`` to lowercase the name like ``__eq__`` does, so equal ``HumanName`` instances hash equal and behave correctly in sets and dicts

nameparser/parser.py

@@ -80,7 +80,6 @@ class HumanName:
     The original string, untouched by the parser.
     """
 
-    _count = 0
     _members = ['title', 'first', 'middle', 'last', 'suffix', 'nickname', 'maiden']
     _full_name = ''
 
@@ -164,13 +163,11 @@ def __setstate__(self, state: dict) -> None:
             self.__dict__.setdefault(attr, set())
 
     def __iter__(self) -> Iterator[str]:
-        return self
+        return (value for member in self._members
+                if (value := getattr(self, member)))
 
     def __len__(self) -> int:
-        l = 0
-        for x in self:
-            l += 1
-        return l
+        return sum(1 for member in self._members if getattr(self, member))
 
     def __eq__(self, other: object) -> bool:
         """
@@ -195,15 +192,6 @@ def __setitem__(self, key: str, value: str) -> None:
         else:
             raise KeyError("Not a valid HumanName attribute", key)
 
-    def __next__(self) -> str:
-        if self._count >= len(self._members):
-            self._count = 0
-            raise StopIteration
-        else:
-            c = self._count
-            self._count = c + 1
-            return getattr(self, self._members[c]) or next(self)
-
     def __str__(self) -> str:
         if self.string_format is not None:
             # string_format = "{title} {first} {middle} {last} {suffix} ({nickname})"

tests/test_python_api.py

@@ -40,6 +40,48 @@ def test_len(self) -> None:
         # documented emptiness check (see usage.rst)
         self.assertEqual(len(HumanName("")), 0)
 
+    def test_iteration_restarts_after_break(self) -> None:
+        hn = HumanName("John Doe")
+        for _ in hn:
+            break
+        # a plain loop, not list(hn): list() presizes via __len__, which
+        # under the old shared-cursor implementation reset the cursor and
+        # masked this bug
+        collected = []
+        for part in hn:
+            collected.append(part)
+        self.assertEqual(collected, ["John", "Doe"])
+
+    def test_iterators_are_independent(self) -> None:
+        hn = HumanName("John Doe")
+        it1 = iter(hn)
+        it2 = iter(hn)
+        self.assertEqual(next(it1), "John")
+        self.assertEqual(next(it2), "John")
+        self.assertEqual(next(it1), "Doe")
+        self.assertEqual(next(it2), "Doe")
+
+    def test_len_during_iteration(self) -> None:
+        hn = HumanName("John Doe")
+        it = iter(hn)
+        self.assertEqual(next(it), "John")
+        # len() must count all members and leave the live iterator intact
+        self.assertEqual(len(hn), 2)
+        self.assertEqual(next(it), "Doe")
+
+    def test_instance_is_not_its_own_iterator(self) -> None:
+        # iterator state must never live on the instance; see release log
+        # for the next(name) -> next(iter(name)) migration
+        hn = HumanName("John Doe")
+        with pytest.raises(TypeError):
+            next(hn)  # type: ignore[call-overload]
+
+    def test_iterating_empty_name_yields_nothing(self) -> None:
+        collected = []
+        for part in HumanName(""):
+            collected.append(part)
+        self.assertEqual(collected, [])
+
     @pytest.mark.skipif(not dill, reason="requires python-dill module to test pickling")
     def test_config_pickle(self) -> None:
         constants = Constants()