Normalize SetManager constructor elements and validate element types

Review follow-up on the operand-normalization commit: the constructor
still stored raw elements, which that commit turned from a latent
inconsistency into active misfires — SetManager(['Dr.']) & ['Dr.']
returned empty and - silently no-opped against the exact spelling
stored in the set, Constants(titles=[...]) remained the last
silently-dead config path (raw 'Chemistry' can never match the
parser's lc() lookups), and titles vs the suffixes_prefixes_titles
union disagreed about the same token.

Fold the operand helper into _normalized_elements, shared by __init__
and all operators, so every entry is stored in the form lookups expect.
Junk elements now raise a curated TypeError instead of failing
cryptically inside lc() (bytes, int) or being silently transmuted
(None became '').

Also scope the docstring claim that operators normalize "the same way
add() does" (add() additionally decodes bytes), correct "strips
periods" to leading/trailing in prose, and pin __rxor__ separately.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: derek73

docs/release_log.rst

@@ -2,7 +2,7 @@ Release Log
 ===========
 * 1.3.0 - Unreleased
     - Fix a bare string passed to a set-backed ``Constants`` argument (e.g. ``Constants(titles='dr')``), to ``SetManager``, or as a ``SetManager`` set-operator operand (e.g. ``constants.titles |= 'esq'``) being silently split into single characters, replacing or polluting the set and producing wrong parses with no error; it now raises ``TypeError`` with the suggested fix — wrap strings in a list, decode ``bytes`` first (closes #238)
-    - Fix ``SetManager`` set operators skipping the lowercase/strip-periods normalization that ``add()`` applies: ``constants.titles |= ['Esq.']`` kept a raw ``'Esq.'`` the parser's lookups could never match, and ``titles & ['Dr.']`` missed ``'dr'``; operator operands are now normalized, so operator results behave like ``add()``-built sets
+    - Fix ``SetManager`` set operators and the constructor skipping the lowercase/strip-edge-periods normalization that ``add()`` applies: ``constants.titles |= ['Esq.']`` kept a raw ``'Esq.'`` the parser's lookups could never match, ``titles & ['Dr.']`` missed ``'dr'``, and ``Constants(titles=[...])`` stored raw elements that silently never matched; elements and operands are now normalized everywhere, and non-``str`` elements (``bytes``, ``None``, numbers) raise ``TypeError`` instead of crashing cryptically or being coerced
     - Fix the ``constants`` constructor argument silently discarding ``Constants`` *subclass* instances: the exact-type check replaced them with fresh defaults, throwing away the caller's configuration. Subclass instances are now used as given; anything that is neither ``None`` nor a ``Constants`` instance now raises ``TypeError`` instead of being silently swapped for defaults (closes #226)
     - Fix ``IndexError`` in ``initials()``/``initials_list()`` when a ``*_list`` attribute was assigned directly with an element containing unnormalized whitespace (e.g. ``name.middle_list = ['Q  R']``), bypassing the parser's whitespace normalization (closes #232)
     - 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)

nameparser/config/__init__.py

@@ -57,13 +57,14 @@ class SetManager(Set):
     ``collections.abc.Set``.
 
     Special functionality beyond that provided by set() is to normalize
-    constants for comparison (lower case, no periods) when they are add()ed
-    and remove()d, and to allow passing multiple string arguments to the
-    :py:func:`add()` and :py:func:`remove()` methods. The constructor and
-    the set operators also reject a bare string with ``TypeError``, since
-    e.g. ``set('dr')`` would silently build a set of single characters, and
-    the set operators normalize their operands the same way :py:func:`add()`
-    does, so operator results keep the normalized-elements invariant.
+    constants for comparison (lowercase, leading/trailing periods stripped)
+    when they are add()ed and remove()d, and to allow passing multiple
+    string arguments to the :py:func:`add()` and :py:func:`remove()`
+    methods. The constructor and the set operators apply the same
+    normalization to their elements and operands, so every entry is stored
+    in the form the parser's lookups expect, and they reject a bare string
+    with ``TypeError``, since e.g. ``set('dr')`` would silently build a set
+    of single characters.
 
     '''
 
@@ -85,9 +86,28 @@ def _reject_bare_string(value: object) -> None:
                 f"wrap it in a list: [{value!r}]"
             )
 
+    @classmethod
+    def _normalized_elements(cls, elements: Iterable[str]) -> set[str]:
+        # apply the same lc() normalization (lowercase, strip leading/
+        # trailing periods) that add() applies, and reject junk elements:
+        # lc() on bytes or int crashes without naming the culprit, and
+        # lc(None) silently transmutes to ''
+        cls._reject_bare_string(elements)
+        normalized = set()
+        for s in elements:
+            if isinstance(s, bytes):
+                raise TypeError(
+                    f"expected str elements, got bytes; decode it first: {s!r}.decode()"
+                )
+            if not isinstance(s, str):
+                raise TypeError(
+                    f"expected str elements, got {type(s).__name__}: {s!r}"
+                )
+            normalized.add(lc(s))
+        return normalized
+
     def __init__(self, elements: Iterable[str]) -> None:
-        self._reject_bare_string(elements)
-        self.elements = set(elements)
+        self.elements = self._normalized_elements(elements)
         # Optional invalidation hook, wired by an owning Constants so that
         # in-place add()/remove() can clear its cached suffixes_prefixes_titles
         # union. None when the manager is used standalone.
@@ -111,41 +131,36 @@ def __contains__(self, value: object) -> bool:
     def __len__(self) -> int:
         return len(self.elements)
 
-    # Set's mixin __or__/__and__ hand _from_iterable a generator, so the
-    # __init__ guard never sees a bare-string operand (c.titles |= 'esq'
-    # would silently add 'e', 's', 'q'); __sub__/__xor__ raised only by
-    # accident of constructing from the operand. Check all four uniformly.
-    # Operands are also normalized the way add() normalizes elements: the
-    # mixins build results from raw operand elements and test them against
-    # the stored (normalized) ones, so without this (titles | ['Esq.'])
-    # keeps a raw 'Esq.' the parser's lc() lookups can never match, and
-    # (titles & ['Dr.']) misses 'dr' — silently broken config either way.
-    @classmethod
-    def _normalized_operand(cls, other: Iterable[str]) -> set[str]:
-        cls._reject_bare_string(other)
-        return {lc(s) for s in other}
-
+    # Set's mixin operators compare operand and stored elements without
+    # normalizing (and __or__/__and__ hand _from_iterable a generator, so
+    # the __init__ guard alone never sees a bare-string operand: c.titles
+    # |= 'esq' would silently add 'e', 's', 'q'). Normalize every operand
+    # through _normalized_elements so operator results behave like
+    # add()-built sets: without it (titles | ['Esq.']) keeps a raw 'Esq.'
+    # the parser's lc() lookups can never match, and (titles & ['Dr.'])
+    # misses the stored 'dr'.
+    #
     # the runtime ABC accepts any Iterable operand, so annotate honestly and
     # ignore typeshed's narrower AbstractSet declarations
     def __or__(self, other: Iterable[str]) -> 'SetManager':  # type: ignore[override]
-        return super().__or__(self._normalized_operand(other))  # type: ignore[operator, return-value]
+        return super().__or__(self._normalized_elements(other))  # type: ignore[operator, return-value]
 
     __ror__ = __or__
 
     def __and__(self, other: Iterable[str]) -> 'SetManager':  # type: ignore[override]
-        return super().__and__(self._normalized_operand(other))  # type: ignore[operator, return-value]
+        return super().__and__(self._normalized_elements(other))  # type: ignore[operator, return-value]
 
     __rand__ = __and__
 
     def __sub__(self, other: Iterable[str]) -> 'SetManager':  # type: ignore[override]
-        return super().__sub__(self._normalized_operand(other))  # type: ignore[operator, return-value]
+        return super().__sub__(self._normalized_elements(other))  # type: ignore[operator, return-value]
 
     def __rsub__(self, other: Iterable[str]) -> 'SetManager':  # type: ignore[override]
         # typeshed omits Set.__rsub__, but the runtime ABC defines it
-        return super().__rsub__(self._normalized_operand(other))  # type: ignore[misc, operator, return-value]
+        return super().__rsub__(self._normalized_elements(other))  # type: ignore[misc, operator, return-value]
 
     def __xor__(self, other: Iterable[str]) -> 'SetManager':  # type: ignore[override]
-        return super().__xor__(self._normalized_operand(other))  # type: ignore[operator, return-value]
+        return super().__xor__(self._normalized_elements(other))  # type: ignore[operator, return-value]
 
     __rxor__ = __xor__
 

tests/test_constants.py

@@ -86,10 +86,11 @@ def test_set_manager_operators_accept_lists(self) -> None:
         self.m(hn.title, "Esq", hn)
 
     def test_set_manager_operators_normalize_like_add(self) -> None:
-        # add() lowercases and strips periods; without the same normalization
-        # of operator operands, (titles | ['Esq.']) contains raw 'Esq.', which
-        # the parser's lc()-based lookups can never match — silently broken
-        # config, same failure family as the bare-string shredding (#238)
+        # add() lowercases and strips leading/trailing periods; without the
+        # same normalization of operator operands, (titles | ['Esq.']) keeps
+        # a raw 'Esq.', which the parser's lc()-based lookups can never match
+        # — silently broken config, same failure family as the bare-string
+        # shredding (#238)
         sm = SetManager(['dr', 'mr'])
         self.assertEqual((sm | ['Esq.']).elements, {'dr', 'mr', 'esq'})
         self.assertEqual((['Esq.', 'Dr.'] | sm).elements, {'dr', 'mr', 'esq'})
@@ -98,6 +99,34 @@ def test_set_manager_operators_normalize_like_add(self) -> None:
         self.assertEqual((sm - ['Dr.']).elements, {'mr'})
         self.assertEqual((['Dr.', 'Esq.'] - sm).elements, {'esq'})
         self.assertEqual((sm ^ ['Dr.', 'Esq.']).elements, {'mr', 'esq'})
+        # pins __rxor__ separately in case it ever stops aliasing __xor__
+        self.assertEqual((['Dr.', 'Esq.'] ^ sm).elements, {'mr', 'esq'})
+
+    def test_set_manager_constructor_normalizes_like_add(self) -> None:
+        # without constructor normalization the operators misfire against
+        # the exact spelling visibly stored in the set: & returns empty
+        # and - silently no-ops
+        sm = SetManager(['Dr.', 'MR'])
+        self.assertEqual(sm.elements, {'dr', 'mr'})
+        self.assertEqual((sm & ['Dr.']).elements, {'dr'})
+        self.assertEqual((sm - ['Dr.']).elements, {'mr'})
+
+    def test_constants_kwarg_elements_are_normalized(self) -> None:
+        # Constants(titles=[...]) was the last silently-dead config path:
+        # a raw 'Chemistry' element can never match the parser's lc() lookups
+        c = Constants(titles=['chancellor', 'Chemistry'])
+        hn = HumanName("Chemistry Jane Smith", constants=c)
+        self.m(hn.title, "Chemistry", hn)
+
+    def test_set_manager_non_str_elements_raise_typeerror(self) -> None:
+        # lc() on junk elements either crashes context-free (bytes, int) or
+        # silently transmutes None into '' — raise a curated error instead
+        with pytest.raises(TypeError, match=r"decode it first"):
+            SetManager([b'dr'])  # type: ignore[list-item]
+        with pytest.raises(TypeError, match=r"expected str elements"):
+            SetManager([None])  # type: ignore[list-item]
+        with pytest.raises(TypeError, match=r"expected str elements"):
+            SetManager(['dr']) | [1]  # type: ignore[list-item]
 
     def test_set_manager_operator_result_parses_when_wired_into_constants(self) -> None:
         # end-to-end: an operator-built set must behave like an add()-built