refactor: remove two unreachable branches, commented-out raises and unused test helpers (#2318)

Problem: two guards could never run, because something earlier had already
raised -- Header.read's FileError on an unknown magic number, which the
schema's magic_number_callback beats to it; and _read_opt_calipso's two
length guards, which calipso_pad_len beats to it while the option is
unpacked. Beside them sat commented-out raise lines next to the warn that
replaced them, a repeated pylint: disable=unused-argument, and three names
nothing read: dispatch.py's _internet_enum, a FAKE_GH query no caller sends,
and GATING.

A third candidate, _read_opt_qs's unknown QS function arm, looked
unreachable the same way but is not: the unassigned-option dispatch it
depends on is by schema class, decided from the func a parse's own registry
resolves it against, not from the func held in a directly-constructed
schema's flags. A QuickStartRequestOption or QuickStartReportOption built
directly with an unassigned func reaches this method's own check regardless,
and a parse reaches it too once a caller registers a schema against that
func, so it stays, with a Note on each of the three readers explaining why.

Fix: the two genuinely unreachable guards are deleted, with the prose that
described them corrected -- Header.read documents ProtocolError, and
_read_opt_calipso documents what checked the length instead. The three
fabricated-schema stanzas that reached the deleted guards by calling a
private reader directly are replaced by the real parse paths, which raise as
before.

Test: tests/protocols/test_dead_branches_2318_unit.py parses what was
supposed to reach each deleted branch and pins what happens instead, pins
the restored QS guard against both schema kinds across all three protocols
through the public construction path and through a parse once a schema is
registered against the func, and pins the source as holding no such deleted
branch, no commented-out raise beside a warn, and no repeated pragma;
tests/project/test_dead_helpers_2318_unit.py pins the rule that made each
helper dead. 8 of the 17 test methods across the two files fail on the
parent commit; the other 9 pin unchanged behaviour by design.

Author: JarryShaw

examples/generators/dispatch.py

@@ -135,13 +135,16 @@ def _resolve(entry: 'Any') -> 'Any':
 def code_name(code: 'Any', enum: 'Optional[Any]' = None) -> 'str':
     """Render a registry key as a name usable in a test id.
 
-    Most ``__proto__`` tables are keyed entirely by enumeration members, whose
-    own ``.name`` is used directly. ``Link.__proto__`` holds an entry keyed by
-    a bare :class:`int` instead (0x8137 for IPX) -- :func:`_resolve` does not
-    care, since dict lookup does not distinguish an :class:`~enum.IntEnum`
-    member from the plain :class:`int` it equals, but a case label needs a
-    name. ``enum`` is consulted for that one, so the label reads
-    ``Novell_Inc_0x8137`` rather than the opaque ``33079``.
+    Many ``__proto__`` tables are keyed entirely by enumeration members, whose
+    own ``.name`` is used directly. ``Link.__proto__`` and the port tables hold
+    bare :class:`int` keys as well -- :func:`_resolve` does not care, since
+    dict lookup does not distinguish an :class:`~enum.IntEnum` member from the
+    plain :class:`int` it equals, but a case label needs a name. ``enum`` is
+    consulted where an enumeration member equals such a key, which is
+    ``Link.__proto__``'s 0x8137 for IPX, so that label reads
+    ``Novell_Inc_0x8137`` rather than the opaque ``33079``. A port number has
+    no enumeration to consult, so it keeps its plain string form and its family
+    is given no ``enum``.
 
     Args:
         code: Registry key.
@@ -580,16 +583,11 @@ def _link_enum() -> 'Any':
     return EtherType
 
 
-def _internet_enum() -> 'Any':
-    from pcapkit.const.reg.transtype import TransType
-    return TransType
-
-
 #: Every family this module exercises, one per ``__proto__`` table named in
 #: GitHub issue #496, and the loopback table #1574 added.
 FAMILIES = (
     Family('link', _link_registry, _link_build, _link_enum),
-    Family('internet', _internet_registry, _internet_build, _internet_enum),
+    Family('internet', _internet_registry, _internet_build),
     Family('tcp', _tcp_registry, _tcp_build),
     Family('udp', _udp_registry, _udp_build),
     Family('sctp', _sctp_registry, _sctp_build),

pcapkit/protocols/application/httpv2.py

@@ -575,7 +575,6 @@ def _read_http_none(self, schema: 'Schema_UnassignedFrame', *,
 
         """
         if any(header.flags.values()):
-            #raise ProtocolError(f'HTTP/2: [Type {frame}] invalid format')
             warn(f'HTTP/2: [Type {header.type}] invalid format', ProtocolWarning)
 
         data = Data_UnassignedFrame(

pcapkit/protocols/internet/hip.py

@@ -1508,7 +1508,6 @@ def _read_param_hip_cipher(self, schema: 'Schema_HIPCipherParameter', *, version
         ciphers = schema.ciphers
         if len(ciphers) > 6:
             warn(f'HIPv{version}: [ParamNo {schema.type}] invalid format', ProtocolWarning)
-            # raise ProtocolError(f'HIPv{version}: [ParamNo {schema.type}] invalid format')
             ciphers = ciphers[:6]
 
         hip_cipher = Data_HIPCipherParameter(

pcapkit/protocols/internet/hopopt.py

@@ -769,17 +769,14 @@ def _read_opt_calipso(self, schema: 'Schema_CALIPSOOption', *, options: 'Option'
         Returns:
             Parsed option data.
 
-        Raises:
-            ProtocolError: If the option is malformed.
+        Note:
+            ``Cmpt Length`` counts 32-bit words and may be odd
+            [:rfc:`5570#section-5.2`]; it only has to fit inside ``Opt Data
+            Len``, which
+            :func:`~pcapkit.protocols.schema.internet.ipv6_option.calipso_pad_len`
+            has already checked while the option was unpacked.
 
         """
-        if schema.len < 8 and schema.len % 8 != 0:
-            raise ProtocolError(f'{self.alias}: [OptNo {schema.type}] invalid format')
-        # ``Cmpt Length`` counts 32-bit words and may be odd [:rfc:`5570#section-5.2`];
-        # it only has to fit inside ``Opt Data Len``
-        if schema.len < 8 + schema.cmpt_len * 4:
-            raise ProtocolError(f'{self.alias}: [OptNo {schema.type}] invalid format')
-
         opt = Data_CALIPSOOption(
             type=schema.type,
             action=Enum_OptionAction.get(schema.type >> 6),
@@ -930,7 +927,7 @@ def _read_opt_pdm(self, schema: 'Schema_PDMOption', *, options: 'Option') -> 'Da
         )
         return opt
 
-    def _read_opt_qs(self, schema: 'Schema_QuickStartOption', *, options: 'Option') -> 'Data_QuickStartOption':  # pylint: disable=unused-argument  # pylint: disable=unused-argument
+    def _read_opt_qs(self, schema: 'Schema_QuickStartOption', *, options: 'Option') -> 'Data_QuickStartOption':  # pylint: disable=unused-argument
         """Read HOPOPT Quick Start option.
 
         Structure of HOPOPT Quick-Start option [:rfc:`4782`]:
@@ -974,7 +971,13 @@ def _read_opt_qs(self, schema: 'Schema_QuickStartOption', *, options: 'Option')
         Note:
             An option whose QS function is neither of the two above has no
             defined layout, so the schema keeps it as an unassigned option and
-            it is parsed as one, see :meth:`_read_opt_none`.
+            it is parsed as one, see :meth:`_read_opt_none`. That dispatch is
+            by the schema's own class, decided from the octets a file read
+            hands it; a :class:`~pcapkit.protocols.schema.internet.hopopt.QuickStartRequestOption`
+            or :class:`~pcapkit.protocols.schema.internet.hopopt.QuickStartReportOption`
+            built directly with an unassigned ``func`` reaches this method
+            regardless, which is why it still raises below rather than
+            returning early.
 
         """
         if isinstance(schema, Schema_UnassignedOption):
@@ -1533,7 +1536,6 @@ def _make_opt_pad(self, code: 'Enum_Option', opt: 'Optional[Data_PadOption]' = N
             pad = getattr(opt, 'pad', b'')
 
         if code == Enum_Option.Pad1 and length != 0:
-            #raise ProtocolError(f'{self.alias}: [OptNo {code}] invalid format')
             warn(f'{self.alias}: [OptNo {code}] invalid format', ProtocolWarning)
             code = Enum_Option.PadN  # type: ignore[assignment]
 

pcapkit/protocols/internet/ipv4.py

@@ -883,7 +883,6 @@ def _read_opt_sec(self, schema: 'Schema_SECOption', *, options: 'Option') -> 'Da
                         flags.append(authority)
 
                 if byte & 0x01 == 0 and base < schema.length - 4:
-                    #raise ProtocolError(f'{self.alias}: [OptNo {kind}] invalid format: remaining data')
                     warn(f'{self.alias}: [OptNo {schema.type}] invalid format: remaining data', ProtocolWarning)
 
             if schema.data[-1] & 0x01 == 1:
@@ -1319,7 +1318,14 @@ def _read_opt_qs(self, schema: 'Schema_QSOption', *, options: 'Option') -> 'Data
         Note:
             An option whose QS function is neither of the two above has no
             defined layout, so the schema keeps it as an unassigned option and
-            it is parsed as one, see :meth:`_read_opt_unassigned`.
+            it is parsed as one, see :meth:`_read_opt_unassigned`. That
+            dispatch is by the schema's own class, decided from the octets a
+            file read hands it; a
+            :class:`~pcapkit.protocols.schema.internet.ipv4.QuickStartRequestOption`
+            or :class:`~pcapkit.protocols.schema.internet.ipv4.QuickStartReportOption`
+            built directly with an unassigned ``func`` reaches this method
+            regardless, which is why it still raises below rather than
+            returning early.
 
         """
         if isinstance(schema, Schema_UnassignedOption):

pcapkit/protocols/internet/ipv6_opts.py

@@ -757,17 +757,14 @@ def _read_opt_calipso(self, schema: 'Schema_CALIPSOOption', *, options: 'Option'
         Returns:
             Parsed option data.
 
-        Raises:
-            ProtocolError: If the option is malformed.
+        Note:
+            ``Cmpt Length`` counts 32-bit words and may be odd
+            [:rfc:`5570#section-5.2`]; it only has to fit inside ``Opt Data
+            Len``, which
+            :func:`~pcapkit.protocols.schema.internet.ipv6_option.calipso_pad_len`
+            has already checked while the option was unpacked.
 
         """
-        if schema.len < 8 and schema.len % 8 != 0:
-            raise ProtocolError(f'{self.alias}: [OptNo {schema.type}] invalid format')
-        # ``Cmpt Length`` counts 32-bit words and may be odd [:rfc:`5570#section-5.2`];
-        # it only has to fit inside ``Opt Data Len``
-        if schema.len < 8 + schema.cmpt_len * 4:
-            raise ProtocolError(f'{self.alias}: [OptNo {schema.type}] invalid format')
-
         opt = Data_CALIPSOOption(
             type=schema.type,
             action=Enum_OptionAction.get(schema.type >> 6),
@@ -918,7 +915,7 @@ def _read_opt_pdm(self, schema: 'Schema_PDMOption', *, options: 'Option') -> 'Da
         )
         return opt
 
-    def _read_opt_qs(self, schema: 'Schema_QuickStartOption', *, options: 'Option') -> 'Data_QuickStartOption':  # pylint: disable=unused-argument  # pylint: disable=unused-argument
+    def _read_opt_qs(self, schema: 'Schema_QuickStartOption', *, options: 'Option') -> 'Data_QuickStartOption':  # pylint: disable=unused-argument
         """Read IPv6-Opts Quick Start option.
 
         Structure of IPv6-Opts Quick-Start option [:rfc:`4782`]:
@@ -962,7 +959,13 @@ def _read_opt_qs(self, schema: 'Schema_QuickStartOption', *, options: 'Option')
         Note:
             An option whose QS function is neither of the two above has no
             defined layout, so the schema keeps it as an unassigned option and
-            it is parsed as one, see :meth:`_read_opt_none`.
+            it is parsed as one, see :meth:`_read_opt_none`. That dispatch is
+            by the schema's own class, decided from the octets a file read
+            hands it; a :class:`~pcapkit.protocols.schema.internet.ipv6_opts.QuickStartRequestOption`
+            or :class:`~pcapkit.protocols.schema.internet.ipv6_opts.QuickStartReportOption`
+            built directly with an unassigned ``func`` reaches this method
+            regardless, which is why it still raises below rather than
+            returning early.
 
         """
         if isinstance(schema, Schema_UnassignedOption):
@@ -1521,7 +1524,6 @@ def _make_opt_pad(self, code: 'Enum_Option', opt: 'Optional[Data_PadOption]' = N
             pad = getattr(opt, 'pad', b'')
 
         if code == Enum_Option.Pad1 and length != 0:
-            #raise ProtocolError(f'{self.alias}: [OptNo {code}] invalid format')
             warn(f'{self.alias}: [OptNo {code}] invalid format', ProtocolWarning)
             code = Enum_Option.PadN  # type: ignore[assignment]
 

pcapkit/protocols/internet/mh.py

@@ -7921,7 +7921,6 @@ def _make_opt_pad(self, type: 'Enum_Option', option: 'Optional[Data_PadOption]'
             data = getattr(option, 'data', b'')
 
         if type == Enum_Option.Pad1 and length != 0:
-            # raise ProtocolError(f'{self.alias}: [OptNo {type}] invalid format')
             warn(f'{self.alias}: [OptNo {type}] invalid format', ProtocolWarning)
             type = Enum_Option.PadN  # type: ignore[assignment]
 

pcapkit/protocols/misc/pcap/header.py

@@ -42,7 +42,7 @@
 from pcapkit.protocols.schema.misc.pcap.header import GLOBAL_HEADER_LEN, MAGIC_NUMBER
 from pcapkit.protocols.schema.misc.pcap.header import Header as Schema_Header
 from pcapkit.protocols.schema.schema import keep_short_read, replay_short_read
-from pcapkit.utilities.exceptions import EndianError, FileError, UnsupportedCall
+from pcapkit.utilities.exceptions import EndianError, UnsupportedCall
 
 if TYPE_CHECKING:
     from enum import IntEnum as StdlibEnum
@@ -141,11 +141,10 @@ def read(self, length: 'Optional[int]' = None, **kwargs: 'Any') -> 'Data_Header'
             Parsed packet data.
 
         Raises:
-            FileError: If the magic number is invalid. Parsing does not get
+            ProtocolError: If the magic number is invalid. Parsing does not get
                 this far with one: the schema's
                 :func:`~pcapkit.protocols.schema.misc.pcap.header.magic_number_callback`
-                raises :exc:`~pcapkit.utilities.exceptions.ProtocolError` for it
-                while the header is unpacked.
+                raises it while the header is unpacked.
 
         """
         if TYPE_CHECKING:
@@ -154,12 +153,10 @@ def read(self, length: 'Optional[int]' = None, **kwargs: 'Any') -> 'Data_Header'
 
         _magn = schema.magic_number
         # NOTE: looked up as :obj:`bytes`, so that a :obj:`bytearray` magic
-        # number is matched rather than rejected as unhashable.
-        _decl = (MAGIC_NUMBER.get(bytes(_magn))
-                 if isinstance(_magn, (bytes, bytearray, memoryview)) else None)
-        if _decl is None:
-            raise FileError(5, 'Unknown file format', self._file.name)  # pylint: disable=no-member
-        self._byte, self._nsec = _decl
+        # number is matched rather than rejected as unhashable. The lookup
+        # cannot miss: ``magic_number_callback`` has already raised for a magic
+        # number this mapping does not hold.
+        self._byte, self._nsec = MAGIC_NUMBER[bytes(_magn)]
 
         header = Data_Header(
             magic_number=Data_MagicNumber(

pcapkit/protocols/misc/pcapng.py

@@ -989,7 +989,6 @@ def nanosecond(self) -> 'bool':
     def ts_resolution(self) -> 'int':
         """Timestamp resolution of the current block, in units per second."""
         if self._ctx is None:
-            #raise UnsupportedCall(f"'{self.__class__.__name__}' object has no attribute 'ts_resolution'")
             warn(f"'{self.__class__.__name__}' object has no attribute 'ts_resolution'",
                  AttributeWarning, stacklevel=stacklevel())
             return DEFAULT_RESOLUTION
@@ -1001,7 +1000,6 @@ def ts_resolution(self) -> 'int':
     def ts_offset(self) -> 'int':
         """Timestamp offset of the current block, in seconds."""
         if self._ctx is None:
-            #raise UnsupportedCall(f"'{self.__class__.__name__}' object has no attribute 'ts_offset'")
             warn(f"'{self.__class__.__name__}' object has no attribute 'ts_offset'",
                  AttributeWarning, stacklevel=stacklevel())
             return 0
@@ -1018,7 +1016,6 @@ def ts_timezone(self) -> 'timezone':
 
         """
         if self._ctx is None:
-            #raise UnsupportedCall(f"'{self.__class__.__name__}' object has no attribute 'ts_timezone'")
             warn(f"'{self.__class__.__name__}' object has no attribute 'ts_timezone'",
                  AttributeWarning, stacklevel=stacklevel())
             return datetime.timezone.utc
@@ -1786,7 +1783,6 @@ def _get_resolution(self, interface_id: 'int' = 0) -> 'int':
 
         """
         if self._ctx is None:
-            # raise UnsupportedCall(f"'{self.__class__.__name__}' object has no attribute '_get_resolution'")
             warn(f"'{self.__class__.__name__}' object has no attribute '_get_resolution'",
                  AttributeWarning, stacklevel=stacklevel())
             return DEFAULT_RESOLUTION
@@ -1809,7 +1805,6 @@ def _get_offset(self, interface_id: 'int' = 0) -> 'int':
 
         """
         if self._ctx is None:
-            # raise UnsupportedCall(f"'{self.__class__.__name__}' object has no attribute '_get_offset'")
             warn(f"'{self.__class__.__name__}' object has no attribute '_get_offset'",
                  AttributeWarning, stacklevel=stacklevel())
             return 0
@@ -1841,7 +1836,6 @@ def _get_timezone(self, interface_id: 'int' = 0) -> 'timezone':
         # and ``_get_offset`` fall back to the format's own defaults in this
         # situation; this is the matching one.
         if self._ctx is None:
-            # raise UnsupportedCall(f"'{self.__class__.__name__}' object has no attribute '_get_timezone'")
             warn(f"'{self.__class__.__name__}' object has no attribute '_get_timezone'",
                  AttributeWarning, stacklevel=stacklevel())
             return datetime.timezone.utc

pcapkit/protocols/schema/internet/hip.py

@@ -918,8 +918,6 @@ def pre_unpack(cls, packet: 'dict[str, Any]') -> 'None':
             if not cipher_list:
                 warn(f'HIP: [ParamNo {Enum_Parameter.ENCRYPTED}] '
                     'missing HIP_CIPHER parameter', ProtocolWarning)
-                # raise ProtocolError(f'HIPv{version}: [ParamNo {schema.type}] invalid format')
-
                 cipher_id = Enum_Cipher(0xffff)
             else:
                 cipher_ids = []  # type: list[Enum_Cipher]
@@ -933,8 +931,6 @@ def pre_unpack(cls, packet: 'dict[str, Any]') -> 'None':
                 if encrypted_index >= len(cipher_ids):
                     warn(f'HIP: [ParamNo {Enum_Parameter.ENCRYPTED}] '
                         'too many ENCRYPTED parameters', ProtocolWarning)
-                    # raise ProtocolError(f'HIPv{version}: [ParamNo {schema.type}] invalid format')
-
                     cipher_id = Enum_Cipher(0xfffe)
                 else:
                     cipher_id = cipher_ids[encrypted_index]