refactor(vendor): finish the %-to-f-string sweep on the four bespoke templates

pcapkit/const/ftp/command.py

@@ -48,7 +48,7 @@ def _missing_(cls, value: 'str') -> 'FEATCode':
 
         """
         if not isinstance(value, str):
-            raise ValueError('%r is not a valid %s' % (value, cls.__name__))
+            raise ValueError(f'{value!r} is not a valid {cls.__name__}')
         return extend_enum(cls, value.upper(), value)
 
 
@@ -73,7 +73,7 @@ def _missing_(cls, value: 'int') -> 'CommandType':
 
         """
         if not (isinstance(value, int) and 0 <= value <= 0x07):
-            raise ValueError('%r is not a valid %s' % (value, cls.__name__))
+            raise ValueError(f'{value!r} is not a valid {cls.__name__}')
         return super()._missing_(value)
 
 
@@ -324,7 +324,7 @@ def _missing_(cls, value: 'str') -> 'Command':
 
         """
         if not isinstance(value, str):
-            raise ValueError('%r is not a valid %s' % (value, cls.__name__))
+            raise ValueError(f'{value!r} is not a valid {cls.__name__}')
         name = value.upper()
         if name in cls._member_map_:
             return cls._member_map_[name]  # type: ignore[return-value]

pcapkit/const/http/method.py

@@ -188,7 +188,7 @@ def _missing_(cls, value: 'str') -> 'Method':
 
         """
         if not isinstance(value, str):
-            raise ValueError('%r is not a valid %s' % (value, cls.__name__))
+            raise ValueError(f'{value!r} is not a valid {cls.__name__}')
         name = value.upper()
         if name in cls._member_map_:
             return cls._member_map_[name]  # type: ignore[return-value]

pcapkit/const/tcp/flags.py

@@ -1,5 +1,5 @@
 # -*- coding: utf-8 -*-
-# pylint: disable=line-too-long,consider-using-f-string
+# pylint: disable=line-too-long
 """TCP Header Flags
 ======================
 
@@ -88,5 +88,5 @@ def _missing_(cls, value: 'int') -> 'Flags':
 
         """
         if not (isinstance(value, int) and 0 <= value <= 0xFFFF):
-            raise ValueError('%r is not a valid %s' % (value, cls.__name__))
+            raise ValueError(f'{value!r} is not a valid {cls.__name__}')
         return super()._missing_(value)

pcapkit/vendor/ftp/command.py

@@ -88,7 +88,7 @@ def _missing_(cls, value: 'str') -> 'FEATCode':
 
         """
         if not isinstance(value, str):
-            raise ValueError('%r is not a valid %s' % (value, cls.__name__))
+            raise ValueError(f'{{value!r}} is not a valid {{cls.__name__}}')
         return extend_enum(cls, value.upper(), value)
 
 
@@ -113,7 +113,7 @@ def _missing_(cls, value: 'int') -> 'CommandType':
 
         """
         if not (isinstance(value, int) and 0 <= value <= 0x07):
-            raise ValueError('%r is not a valid %s' % (value, cls.__name__))
+            raise ValueError(f'{{value!r}} is not a valid {{cls.__name__}}')
         return super()._missing_(value)
 
 
@@ -186,7 +186,7 @@ def _missing_(cls, value: 'str') -> '{NAME}':
 
         """
         if not isinstance(value, str):
-            raise ValueError('%r is not a valid %s' % (value, cls.__name__))
+            raise ValueError(f'{{value!r}} is not a valid {{cls.__name__}}')
         name = value.upper()
         if name in cls._member_map_:
             return cls._member_map_[name]  # type: ignore[return-value]

pcapkit/vendor/http/method.py

@@ -93,7 +93,7 @@ def _missing_(cls, value: 'str') -> '{NAME}':
 
         """
         if not isinstance(value, str):
-            raise ValueError('%r is not a valid %s' % (value, cls.__name__))
+            raise ValueError(f'{{value!r}} is not a valid {{cls.__name__}}')
         name = value.upper()
         if name in cls._member_map_:
             return cls._member_map_[name]  # type: ignore[return-value]

pcapkit/vendor/reg/apptype/apptype.py

@@ -92,7 +92,7 @@
 #: lookup, and no members at all.
 BASE = lambda NAME, DOCS, FLAG, TABLE, MISS, MODL: f'''\
 # -*- coding: utf-8 -*-
-# pylint: disable=line-too-long,consider-using-f-string
+# pylint: disable=line-too-long
 """{(name := DOCS.split(' [', maxsplit=1)[0])}
 {'=' * (len(name) + 6)}
 
@@ -248,7 +248,7 @@ class {NAME}(StrEnum):
 
     def __new__(cls, value: 'int', name: 'str' = '<null>',
                 proto: 'TransportProtocol' = TransportProtocol.undefined) -> 'Type[{NAME}]':
-        temp = '%s [%d - %s]' % (name, value, proto.name)
+        temp = f'{{name}} [{{value}} - {{proto.name}}]'
 
         obj = str.__new__(cls, temp)
         obj._value_ = temp
@@ -268,10 +268,10 @@ def __new__(cls, value: 'int', name: 'str' = '<null>',
         return obj
 
     def __repr__(self) -> 'str':
-        return "<%s.%s: %d [%s]>" % (self.__class__.__name__, self.svc, self.port, self.proto.name)
+        return f'<{{self.__class__.__name__}}.{{self.svc}}: {{self.port}} [{{self.proto.name}}]>'
 
     def __str__(self) -> 'str':
-        return '%s [%d - %s]' % (self.svc, self.port, self.proto.name)
+        return f'{{self.svc}} [{{self.port}} - {{self.proto.name}}]'
 
     def __int__(self) -> 'int':
         return self.port
@@ -325,7 +325,7 @@ def _dispatch(cls, key: 'int', proto: 'TransportProtocol | str') -> 'Type[{NAME}
         # protocol, so a name never matched and the miss path minted a brand-new
         # member with port -1 -- GitHub issue #734's silent junk. Rejecting a
         # non-port outright is the honest answer, and it has to happen before the
-        # miss path, which formats ``key`` with ``%d``.
+        # miss path, which formats ``key`` into an f-string.
         if not isinstance(key, int):
             raise ValueError(f'{{key!r}} is not a valid port number for {{cls.__name__}}')
         if cls.__registry__ is not None:
@@ -436,7 +436,7 @@ def get(cls, key: 'int', *,
         # propagates, so both entry points answer an out-of-range port identically.
         ret = owner._missing_(key)
         if ret is None:
-            ret = extend_enum(owner, 'PORT_%d_%s' % (key, owner.__transport__.name),
+            ret = extend_enum(owner, f'PORT_{{key}}_{{owner.__transport__.name}}',
                               key, 'unknown', owner.__transport__)
         return ret
 
@@ -776,7 +776,7 @@ def records(self, data: 'list[str]') -> 'tuple[OrderedDict[str, Record], list[st
 
                 miss.append(f'if {start} <= value <= {stop}{claim}:')
                 miss.append(f'    #: {cmmt}')
-                miss.append(f"    return extend_enum(cls, '{self.safe_name(svc)}_%d' % value, "
+                miss.append(f"    return extend_enum(cls, f'{self.safe_name(svc)}_{{value}}', "
                             f"value, {svc!r}, {flag})")
 
         return line, miss

pcapkit/vendor/tcp/flags.py

@@ -36,7 +36,7 @@
 #: Default constant template of enumerate registry from IANA CSV.
 LINE = lambda NAME, DOCS, FLAG, ENUM, MODL: f'''\
 # -*- coding: utf-8 -*-
-# pylint: disable=line-too-long,consider-using-f-string
+# pylint: disable=line-too-long
 """{(name := DOCS.split(' [', maxsplit=1)[0])}
 {'=' * (len(name) + 6)}
 
@@ -91,7 +91,7 @@ def _missing_(cls, value: 'int') -> '{NAME}':
 
         """
         if not ({FLAG}):
-            raise ValueError('%r is not a valid %s' % (value, cls.__name__))
+            raise ValueError(f'{{value!r}} is not a valid {{cls.__name__}}')
         return super()._missing_(value)
 '''.strip()  # type: Callable[[str, str, str, str, str], str]
 

tests/const/test_const_apptype_split_unit.py

@@ -835,6 +835,66 @@ def test_no_lookup_call_site_in_the_library_builds_a_composite_proto(self) -> No
         self.assertIn('proto = code.proto', source)
         self.assertIn('if test not in proto:', source)
 
+    def test_the_new_dunders_are_byte_identical_to_the_percent_form(self) -> None:
+        """GitHub issue #798: ``AppType``'s ``__new__``/``__repr__``/``__str__`` moved
+        from ``%`` formatting to f-strings, and ``__new__``'s format sets every
+        real member's underlying :class:`~aenum.StrEnum` value -- a far larger
+        blast radius than an error path, so this is checked member by member
+        rather than spot-checked.
+
+        Swept over all 12,391 real members (TCP 6147, UDP 6143, SCTP 91,
+        DCCP 10, matching the population GitHub issue #783 measured), each
+        compared against what the pre-#798 ``%``-style formula would have
+        produced for that same member's own ``svc``/``port``/``proto``. This
+        is an invariance check -- it is true either side of #798's fix by
+        construction, since both formulas render the same text for the same
+        inputs -- rather than a regression test that fails on stock ``main``.
+
+        Runs against a freshly imported, purged-and-restored ``pcapkit`` tree
+        (as :class:`~tests.const.test_const_enum_builtin_parity.ConstEnumBuiltinParityTests`
+        does) rather than whatever module instance an earlier test in this
+        file left behind: several sibling tests here mint throwaway members
+        via :func:`~aenum.extend_enum` and clean up with
+        :meth:`~unittest.TestCase.addCleanup`, but the population counts below
+        are only meaningful against a registry no other test has touched.
+        """
+        from tests._support import (ISOLATED_PREFIXES, purge_modules, restore_modules,
+                                    snapshot_modules)
+
+        snapshot = snapshot_modules(ISOLATED_PREFIXES)
+        purge_modules(['pcapkit'])
+        self.addCleanup(restore_modules, snapshot, ISOLATED_PREFIXES)
+
+        from pcapkit.const.reg.apptype import DCCP, SCTP, TCP, UDP
+
+        registries = {'TCP': TCP, 'UDP': UDP, 'SCTP': SCTP, 'DCCP': DCCP}
+        expected_counts = {'TCP': 6147, 'UDP': 6143, 'SCTP': 91, 'DCCP': 10}
+        total = 0
+
+        for name, cls in registries.items():
+            count = 0
+            for member in cls:
+                count += 1
+                total += 1
+                svc, port, proto = member.svc, member.port, member.proto
+
+                with self.subTest(registry=name, member=member.name, check='value'):
+                    old_value = '%s [%d - %s]' % (svc, port, proto.name)  # pylint: disable=consider-using-f-string
+                    self.assertEqual(str(member._value_), old_value)  # type: ignore[attr-defined]
+
+                with self.subTest(registry=name, member=member.name, check='repr'):
+                    old_repr = "<%s.%s: %d [%s]>" % (  # pylint: disable=consider-using-f-string
+                        member.__class__.__name__, svc, port, proto.name)
+                    self.assertEqual(repr(member), old_repr)
+
+                with self.subTest(registry=name, member=member.name, check='str'):
+                    old_str = '%s [%d - %s]' % (svc, port, proto.name)  # pylint: disable=consider-using-f-string
+                    self.assertEqual(str(member), old_str)
+
+            self.assertEqual(count, expected_counts[name], f'{name} population changed')
+
+        self.assertEqual(total, 12391)
+
     @staticmethod
     def _purge_member(cls: type, name: str, port: int) -> None:
         """Undo an :func:`~aenum.extend_enum` so the registry is left as found.

tests/const/test_const_enum_builtin_parity.py

@@ -100,20 +100,19 @@
 #: Each was rendering a registry that GitHub issue #647 found unguarded.
 #:
 #: Keyed to each template's own guard text rather than one literal shared by
-#: all four: GitHub issue #792 moved ``pcapkit.vendor.reg.apptype.apptype``'s
-#: copy to an f-string, following the library-wide convention GitHub issue
-#: #783 settled, while the other three still raise with ``%`` -- #792
-#: deliberately left them alone so the ``const`` diff stayed reviewable, and
-#: #798 tracks sweeping them, along with the ``%``-formatted dunders and
-#: dropping the f-string disable. One shared literal can no longer pin all four;
-#: what #647 actually needs pinned is that each template still carries *a*
-#: guard rejecting an invalid value, in whatever form that template's own
-#: raise takes, not that the four agree on a formatting style the library is
-#: moving away from.
+#: all four, kept as a dict rather than collapsed back to one shared literal:
+#: GitHub issue #792 moved ``pcapkit.vendor.reg.apptype.apptype``'s copy to an
+#: f-string first, deliberately leaving the other three on ``%`` so its own
+#: diff over a 12,391-member file stayed reviewable, and GitHub issue #798
+#: swept the remaining three onto the same f-string form. All four now agree,
+#: but the dict stays: a future bespoke guard is not guaranteed to match this
+#: one's shape, and what #647 actually needs pinned is that each template
+#: still carries *a* guard rejecting an invalid value, not that every template
+#: shares one literal.
 BESPOKE_TEMPLATES = {
-    'pcapkit.vendor.tcp.flags': "raise ValueError('%r is not a valid %s' % (value, cls.__name__))",
-    'pcapkit.vendor.ftp.command': "raise ValueError('%r is not a valid %s' % (value, cls.__name__))",
-    'pcapkit.vendor.http.method': "raise ValueError('%r is not a valid %s' % (value, cls.__name__))",
+    'pcapkit.vendor.tcp.flags': "raise ValueError(f'{{value!r}} is not a valid {{cls.__name__}}')",
+    'pcapkit.vendor.ftp.command': "raise ValueError(f'{{value!r}} is not a valid {{cls.__name__}}')",
+    'pcapkit.vendor.http.method': "raise ValueError(f'{{value!r}} is not a valid {{cls.__name__}}')",
     'pcapkit.vendor.reg.apptype.apptype': "raise ValueError(f'{{value!r}} is not a valid {{cls.__name__}}')",
 }
 
@@ -589,9 +588,9 @@ class ConstEnumGuardTemplateTests(unittest.TestCase):
     agree with it -- the next crawl would simply revert them. The four templates
     below each carry their own copy of the guard rather than inheriting the one
     in :mod:`pcapkit.vendor.default`, which is why all four had to be edited and
-    why all four are checked -- each against its own guard text now that
-    GitHub issue #792 moved one of them off ``%`` formatting, per
-    :data:`BESPOKE_TEMPLATES`.
+    why all four are checked -- each against its own guard text in
+    :data:`BESPOKE_TEMPLATES`, now that GitHub issue #798 finished moving all
+    four off ``%`` formatting (GitHub issue #792 did the first).
     """
 
     def setUp(self) -> None:
@@ -607,6 +606,61 @@ def test_every_bespoke_template_carries_the_guard(self) -> None:
                     guard, source, f'{module_name} no longer emits its guard; '
                                    f'see GitHub issue #647')
 
+    def test_no_bespoke_const_module_still_uses_percent_formatting_for_its_guard(self) -> None:
+        """The committed side of :data:`BESPOKE_TEMPLATES`, needing no ``requests``.
+
+        GitHub issue #798: the three vendor templates the previous test's
+        ``skipUnless`` can silently skip -- exactly how #792's regression
+        reached seven CI legs -- so this checks the same guards through the
+        committed :mod:`pcapkit.const` modules instead, which need no network
+        dependency to import. Asserts the *old* ``%``-style literal is gone
+        rather than merely that a guard exists, so a half-converted template
+        (old and new form both present) would still fail this.
+        """
+        old_guard = "raise ValueError('%r is not a valid %s' % (value, cls.__name__))"
+        const_modules = (
+            'pcapkit.const.tcp.flags',
+            'pcapkit.const.ftp.command',
+            'pcapkit.const.http.method',
+            'pcapkit.const.reg.apptype.apptype',
+        )
+        for module_name in const_modules:
+            with self.subTest(const=module_name):
+                source = inspect.getsource(importlib.import_module(module_name))
+                self.assertNotIn(
+                    old_guard, source, f'{module_name} still raises its guard with '
+                                        f'%% formatting; see GitHub issue #798')
+
+    def test_the_disable_drops_only_where_nothing_else_needs_percent_formatting(self) -> None:
+        """GitHub issue #798's third part: the disable is earned, not blanket-dropped.
+
+        ``pcapkit.const.tcp.flags`` and ``pcapkit.const.reg.apptype.apptype``
+        carry no other %-formatted code once their guards (and, for the
+        latter, its three dunders and its span-handling tail) are converted,
+        so their module-level ``consider-using-f-string`` disable comes off.
+        ``pcapkit.const.ftp.command`` and ``pcapkit.const.http.method`` each
+        still render a ``__repr__`` using ``%`` -- out of #798's stated scope,
+        which named only the ``AppType`` template's dunders -- so their
+        disable has to stay; this pins *why* rather than letting a future
+        sweep assume the omission was an oversight.
+        """
+        dropped = ('pcapkit.const.tcp.flags', 'pcapkit.const.reg.apptype.apptype')
+        for module_name in dropped:
+            with self.subTest(const=module_name):
+                source = inspect.getsource(importlib.import_module(module_name))
+                self.assertNotIn('consider-using-f-string', source)
+                self.assertNotIn('%', source)
+
+        retained = ('pcapkit.const.ftp.command', 'pcapkit.const.http.method')
+        for module_name in retained:
+            with self.subTest(const=module_name):
+                source = inspect.getsource(importlib.import_module(module_name))
+                self.assertIn('consider-using-f-string', source)
+                # The __repr__ still using %-formatting is the reason the
+                # disable is retained; the guard itself no longer needs it.
+                self.assertIn('def __repr__', source)
+                self.assertIn('%', source)
+
     @unittest.skipUnless(importlib.util.find_spec('requests') is not None,
                          'pcapkit.vendor needs requests')
     def test_the_tcp_flags_template_renders_the_committed_module(self) -> None: