Review follow-ups: guard set operators and fix the bytes hint

The Set mixin's __or__/__and__ hand _from_iterable a generator, so the
new __init__ guard never saw a bare-string operand: c.titles |= 'esq'
still silently added 'e', 's', 'q' as titles, while __sub__/__xor__
raised only by accident of constructing from the operand. Route all
four operators (and reflections) through a shared bare-string check.
typeshed declares narrower AbstractSet operands and omits the runtime
ABC's __rsub__, hence the targeted type ignores.

The bytes branch of the error hint was itself a trap: following "wrap
it in a list: [b'dr']" produces a set whose bytes elements never match
parsed str tokens — another silent failure. bytes now get "decode it
first: [b'dr'.decode()]".

Also correct the SetManager class docstring's "Only special
functionality" claim (the guards are user-visible API now) and drop
add()'s false "Can pass a list of strings" (that raises
AttributeError).

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

Author: derek73

docs/release_log.rst

@@ -1,7 +1,7 @@
 Release Log
 ===========
 * 1.3.0 - Unreleased
-    - Fix a bare string passed to a set-backed ``Constants`` argument (e.g. ``Constants(titles='dr')``) or to ``SetManager`` being silently split into single characters, replacing the default set and producing wrong parses with no error; it now raises ``TypeError`` with the suggested fix (wrap it in a list) (closes #238)
+    - 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 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

@@ -56,23 +56,35 @@ class SetManager(Set):
     Easily add and remove config variables per module or instance. Subclass of
     ``collections.abc.Set``.
 
-    Only 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 allow passing multiple 
-    string arguments to the :py:func:`add()` and :py:func:`remove()` methods.
+    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.
 
     '''
 
     _on_change: Callable[[], None] | None
 
-    def __init__(self, elements: Iterable[str]) -> None:
-        # a bare string is an iterable of its characters, so set(elements)
-        # would silently build a set of single characters (#238)
-        if isinstance(elements, (str, bytes)):
+    @staticmethod
+    def _reject_bare_string(value: object) -> None:
+        # a bare string is an iterable of its characters, so set(value)
+        # would silently build a set of single characters — and bytes
+        # iterates to ints, which can never match parsed tokens (#238)
+        if isinstance(value, bytes):
+            raise TypeError(
+                "expected an iterable of strings, got a single bytes; "
+                f"decode it first: [{value!r}.decode()]"
+            )
+        if isinstance(value, str):
             raise TypeError(
-                "expected an iterable of strings, got a single "
-                f"{type(elements).__name__}; wrap it in a list: [{elements!r}]"
+                "expected an iterable of strings, got a single str; "
+                f"wrap it in a list: [{value!r}]"
             )
+
+    def __init__(self, elements: Iterable[str]) -> None:
+        self._reject_bare_string(elements)
         self.elements = set(elements)
         # Optional invalidation hook, wired by an owning Constants so that
         # in-place add()/remove() can clear its cached suffixes_prefixes_titles
@@ -97,6 +109,39 @@ 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.
+    # 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]
+        self._reject_bare_string(other)
+        return super().__or__(other)  # type: ignore[operator, return-value]
+
+    __ror__ = __or__
+
+    def __and__(self, other: Iterable[str]) -> 'SetManager':  # type: ignore[override]
+        self._reject_bare_string(other)
+        return super().__and__(other)  # type: ignore[operator, return-value]
+
+    __rand__ = __and__
+
+    def __sub__(self, other: Iterable[str]) -> 'SetManager':  # type: ignore[override]
+        self._reject_bare_string(other)
+        return super().__sub__(other)  # type: ignore[operator, return-value]
+
+    def __rsub__(self, other: Iterable[str]) -> 'SetManager':  # type: ignore[override]
+        self._reject_bare_string(other)
+        # typeshed omits Set.__rsub__, but the runtime ABC defines it
+        return super().__rsub__(other)  # type: ignore[misc, operator, return-value]
+
+    def __xor__(self, other: Iterable[str]) -> 'SetManager':  # type: ignore[override]
+        self._reject_bare_string(other)
+        return super().__xor__(other)  # type: ignore[operator, return-value]
+
+    __rxor__ = __xor__
+
     def add_with_encoding(self, s: str, encoding: str | None = None) -> None:
         """
         Add the lowercased, leading/trailing-periods-stripped version of the string to the set. Pass an
@@ -118,7 +163,7 @@ def add_with_encoding(self, s: str, encoding: str | None = None) -> None:
     def add(self, *strings: str) -> Self:
         """
         Add the lowercased, leading/trailing-periods-stripped version of the string arguments to the set.
-        Can pass a list of strings. Returns ``self`` for chaining.
+        Returns ``self`` for chaining.
         """
         for s in strings:
             self.add_with_encoding(s)

tests/test_constants.py

@@ -55,9 +55,36 @@ def test_constants_bare_string_kwarg_raises_typeerror(self) -> None:
     def test_set_manager_bare_string_raises_typeerror(self) -> None:
         with pytest.raises(TypeError, match=r"wrap it in a list"):
             SetManager('dr')
-        with pytest.raises(TypeError, match=r"wrap it in a list"):
+
+    def test_set_manager_bytes_raises_with_decode_hint(self) -> None:
+        # a "wrap it in a list" hint would be a trap for bytes: [b'dr'] is
+        # accepted but its elements never match parsed str tokens
+        with pytest.raises(TypeError, match=r"decode it first"):
             SetManager(b'dr')  # type: ignore[arg-type]
 
+    def test_set_manager_bare_string_operand_raises_typeerror(self) -> None:
+        # Set's mixin __or__/__and__ hand _from_iterable a generator, so the
+        # constructor guard alone never sees a bare-string operand; without
+        # an operand check, c.titles |= 'esq' silently adds 'e', 's', 'q'
+        sm = SetManager(['dr', 'mr'])
+        for op in (lambda: sm | 'abc',
+                   lambda: 'abc' | sm,
+                   lambda: sm & 'abc',
+                   lambda: 'abc' & sm,
+                   lambda: sm - 'abc',
+                   lambda: sm ^ 'abc'):
+            with pytest.raises(TypeError, match=r"wrap it in a list"):
+                op()
+
+    def test_set_manager_operators_accept_lists(self) -> None:
+        c = Constants()
+        with pytest.raises(TypeError, match=r"wrap it in a list"):
+            c.titles |= 'esq'
+        c.titles |= ['esq']
+        self.assertIn('esq', c.titles)
+        hn = HumanName("Esq Jane Smith", constants=c)
+        self.m(hn.title, "Esq", hn)
+
     def test_remove_title(self) -> None:
         hn = HumanName("Hon Solo", constants=None)
         start_len = len(hn.C.titles)