fix(internet): narrow IGMP's id() to self, per owner's post-merge ruling

docs/source/contributing/conventions/protocol-family-grouping.rst

@@ -65,7 +65,11 @@ Member Shapes
    * - Narrow to self
      - the member's own name only
      - :class:`~pcapkit.protocols.internet.ah.AH` ``('AH',)``,
-       :class:`~pcapkit.protocols.internet.esp.ESP` ``('ESP',)``
+       :class:`~pcapkit.protocols.internet.esp.ESP` ``('ESP',)``;
+       :class:`IGMPv2 <pcapkit.protocols.internet.igmpv2.IGMP>` ``('IGMPv2',)`` --
+       :class:`IGMPv1 <pcapkit.protocols.internet.igmpv1.IGMP>` narrows the same way,
+       but to the RFC canonical name rather than a versioned one
+       (:ref:`protocol-family-canonical-names`)
    * - Family-inclusive
      - the member's label plus the family name
      - :class:`~pcapkit.protocols.link.c_tag.C_Tag` ``('C_Tag', 'VLAN')``,
@@ -108,11 +112,22 @@ preference on :pr:`2444`, not a settled rule.
 ``name`` of their own, so they report ``Hypertext Transfer Protocol``, which carries
 no version.
 
-**ICMPv4 is the case where the RFC's own name carries no version.** The owner's
-reply on :pr:`2446` proposes that ``ICMPv4`` keeps its versioned class and module name,
-that its ``id()`` should probably drop the version for the RFC canonical name, and
-that its ``name`` carries no "version 4", following RFC 792. It is a proposal, not a
-ruling.
+**ICMPv4 is the case where the RFC's own name carries no version.** The ruling on
+:pr:`2446`: ``ICMPv4`` keeps its versioned class and module name, but its ``id()``
+drops the version for the RFC canonical name (:rfc:`792` names it plain ``ICMP``),
+and its ``name`` carries no "version 4". ``ICMPv6`` keeps its own version, since
+:rfc:`4443` names it with one.
+
+**IGMPv1 follows the same shape, by the same reasoning** (:issue:`2439`, :pr:`2506`).
+:rfc:`1112` names IGMPv1's protocol plain ``IGMP``, so
+:meth:`IGMPv1.id() <pcapkit.protocols.internet.igmpv1.IGMP.id>` is ``('IGMP',)`` and
+its ``alias`` is ``'IGMP'``, while
+:meth:`IGMPv2.id() <pcapkit.protocols.internet.igmpv2.IGMP.id>` keeps its own version,
+``('IGMPv2',)``, since :rfc:`2236` names it with one. The family base's ``id()``
+follows: :meth:`IGMP.id() <pcapkit.protocols.internet.igmp.IGMP.id>` is
+``('IGMP', 'IGMPv2')``, the family-inclusive shape, because IGMPv1's canonical name
+and the family's own name are the same string -- the same shape
+:meth:`ICMP.id() <pcapkit.protocols.internet.icmp.ICMP.id>` takes, ``('ICMP', 'ICMPv6')``.
 
 .. _protocol-family-selectors:
 

pcapkit/protocols/internet/igmp.py

@@ -92,6 +92,11 @@ def length(self) -> 'Literal[8]':
     def id(cls) -> 'tuple[Literal["IGMP"], Literal["IGMPv1"], Literal["IGMPv2"], Literal["IGMPv3"]]':
         """Index ID of the protocol.
 
+        :rfc:`1112` names the protocol ``IGMP``, without a version, the same
+        shape :class:`~pcapkit.protocols.internet.icmp.ICMP` takes for
+        :rfc:`792` -- so the family base's own name doubles as IGMPv1's
+        canonical name. :rfc:`2236` names IGMPv2 with its own version.
+
         Returns:
             Index ID of the protocol, the family name and all three
             versions; the concrete IGMPv1, IGMPv2 and IGMPv3 classes narrow
@@ -120,7 +125,7 @@ def read(self, length: 'Optional[int]' = None, **kwargs: 'Any') -> '_PT':
         schema = cast('Schema_IGMP', self.__header__)
 
         if self._classify(schema.type, schema.max_resp_time, length) != self.version:
-            raise ProtocolError(f'{self.alias}: {schema.type!r} is not an {self.alias} '
+            raise ProtocolError(f'{self.alias}: {schema.type!r} is not an IGMPv{self.version} '
                                 f'message ({length} octets)')
 
         igmp = Data_IGMP(

pcapkit/protocols/internet/igmpv1.py

@@ -38,16 +38,26 @@ def version(self) -> 'Literal[1]':
         """IGMP version this class reads: ``1``."""
         return 1
 
+    @property
+    def alias(self) -> 'Literal["IGMP"]':
+        """Acronym of corresponding protocol."""
+        return 'IGMP'
+
     ##########################################################################
     # Methods.
     ##########################################################################
 
     @classmethod
-    def id(cls) -> 'tuple[Literal["IGMP"], Literal["IGMPv1"]]':  # type: ignore[override]
+    def id(cls) -> 'tuple[Literal["IGMP"]]':  # type: ignore[override]
         """Index ID of the protocol.
 
+        :rfc:`1112` names the protocol ``IGMP``, without a version.
+
+        String ``==`` compares against ``(cls, *id())``, so ``== 'IGMPv1'`` is
+        :data:`False`, while ``'IGMPv1' in`` matches by class.
+
         Returns:
-            Index ID of the protocol, the family name and the version.
+            Index ID of the protocol.
 
         """
-        return ('IGMP', 'IGMPv1')
+        return ('IGMP',)

pcapkit/protocols/internet/igmpv2.py

@@ -38,16 +38,23 @@ def version(self) -> 'Literal[2]':
         """IGMP version this class reads: ``2``."""
         return 2
 
+    @property
+    def alias(self) -> 'Literal["IGMPv2"]':
+        """Acronym of corresponding protocol."""
+        return 'IGMPv2'
+
     ##########################################################################
     # Methods.
     ##########################################################################
 
     @classmethod
-    def id(cls) -> 'tuple[Literal["IGMP"], Literal["IGMPv2"]]':  # type: ignore[override]
+    def id(cls) -> 'tuple[Literal["IGMPv2"]]':  # type: ignore[override]
         """Index ID of the protocol.
 
+        :rfc:`2236` names the protocol ``IGMPv2``, carrying its own version.
+
         Returns:
-            Index ID of the protocol, the family name and the version.
+            Index ID of the protocol.
 
         """
-        return ('IGMP', 'IGMPv2')
+        return ('IGMPv2',)

tests/protocols/internet/test_igmp_unit.py

@@ -112,6 +112,16 @@ def igmp_class(version: 'str'):
     return {'IGMPv1': igmpv1.IGMP, 'IGMPv2': igmpv2.IGMP, 'IGMPv3': igmpv3.IGMP}[version]
 
 
+def expected_alias(version: 'str') -> 'str':
+    """The alias a parsed layer reports for ``version``.
+
+    :rfc:`1112` names the protocol ``IGMP``, without a version, so IGMPv1's
+    alias is the bare family name; :rfc:`2236` names IGMPv2 with its own
+    version, so IGMPv2's alias carries it.
+    """
+    return 'IGMP' if version == 'IGMPv1' else version
+
+
 #: One fixture per message type IGMPv1 and IGMPv2 define, each a correctly
 #: checksummed 8-octet message, with the version it is read as. Type octets
 #: per :rfc:`1112#appendix-I` (``0x11``/``0x12``, v1) and :rfc:`2236#section-2.1`
@@ -323,7 +333,7 @@ def test_alias_names_the_version_read(self) -> None:
             with self.subTest(packet=name):
                 parsed = igmp_class(version)(io.BytesIO(packet), len(packet))
                 self.assertIsInstance(parsed, IGMPBase)
-                self.assertEqual(parsed.alias, version)
+                self.assertEqual(parsed.alias, expected_alias(version))
 
     def test_refusals_name_the_version_refused(self) -> None:
         from pcapkit.protocols.internet.igmpv1 import IGMP as IGMPv1
@@ -333,7 +343,7 @@ def test_refusals_name_the_version_refused(self) -> None:
 
         v1_report, v2_report = IGMP_PACKETS['v1-report'][0], IGMP_PACKETS['v2-report'][0]
         v2_query = IGMP_PACKETS['v2-query-general'][0]
-        with self.assertRaisesRegex(ProtocolError, r'^IGMPv1: .+ is not an IGMPv1 message \(8 octets\)'):
+        with self.assertRaisesRegex(ProtocolError, r'^IGMP: .+ is not an IGMPv1 message \(8 octets\)'):
             IGMPv1(io.BytesIO(v2_report), len(v2_report))
         with self.assertRaisesRegex(ProtocolError, r'^IGMPv2: .+ is not an IGMPv2 message \(8 octets\)'):
             IGMPv2(io.BytesIO(v1_report), len(v1_report))
@@ -350,7 +360,7 @@ def test_membership_query_max_resp_time_zero_is_v1(self) -> None:
 
         packet = igmp_packet(0x11, 0, '0.0.0.0')
         parsed = IGMP(io.BytesIO(packet), len(packet))
-        self.assertEqual(parsed.alias, 'IGMPv1')
+        self.assertEqual(parsed.alias, 'IGMP')
 
     def test_membership_query_max_resp_time_nonzero_is_v2(self) -> None:
         from pcapkit.protocols.internet.igmpv2 import IGMP
@@ -412,8 +422,8 @@ def test_id_and_version(self) -> None:
         from pcapkit.protocols.internet.igmpv2 import IGMP as IGMPv2
         from pcapkit.protocols.internet.igmpv3 import IGMP as IGMPv3
 
-        self.assertEqual(IGMPv1.id(), ('IGMP', 'IGMPv1'))
-        self.assertEqual(IGMPv2.id(), ('IGMP', 'IGMPv2'))
+        self.assertEqual(IGMPv1.id(), ('IGMP',))
+        self.assertEqual(IGMPv2.id(), ('IGMPv2',))
         self.assertEqual(IGMPv3.id(), ('IGMP', 'IGMPv3'))
         self.assertEqual(object.__new__(IGMPv1).version, 1)
         self.assertEqual(object.__new__(IGMPv2).version, 2)
@@ -426,8 +436,8 @@ def test_base_id_covers_all_three_versions(self) -> None:
 
     def test_each_version_narrows_the_base_id(self) -> None:
         v1, v2, v3 = igmp_class('IGMPv1'), igmp_class('IGMPv2'), igmp_class('IGMPv3')
-        self.assertEqual(v1.id(), ('IGMP', 'IGMPv1'))
-        self.assertEqual(v2.id(), ('IGMP', 'IGMPv2'))
+        self.assertEqual(v1.id(), ('IGMP',))
+        self.assertEqual(v2.id(), ('IGMPv2',))
         self.assertEqual(v3.id(), ('IGMP', 'IGMPv3'))
         self.assertNotIn('IGMPv2', v1.id())
         self.assertNotIn('IGMPv3', v1.id())
@@ -443,7 +453,7 @@ def test_igmpv1_parses_its_own_messages_directly(self) -> None:
             packet, _version = IGMP_PACKETS[name]
             with self.subTest(packet=name):
                 parsed = IGMPv1(io.BytesIO(packet), len(packet))
-                self.assertEqual(parsed.alias, 'IGMPv1')
+                self.assertEqual(parsed.alias, 'IGMP')
 
     def test_igmpv1_rejects_a_v2_only_message(self) -> None:
         from pcapkit.protocols.internet.igmpv1 import IGMP as IGMPv1
@@ -552,7 +562,7 @@ def test_versions_take_the_length_from_the_data_when_none_is_given(self) -> None
             packet, version = IGMP_PACKETS[name]
             with self.subTest(version=version):
                 parsed = cls(packet)
-                self.assertEqual(parsed.alias, version)
+                self.assertEqual(parsed.alias, expected_alias(version))
                 self.assertEqual(len(parsed), 8)
 
                 with warnings.catch_warnings():
@@ -681,7 +691,7 @@ def test_max_resp_time_bounds_round_trip(self) -> None:
                 parsed = cls(io.BytesIO(packet), len(packet))
                 self.assertEqual(packet[1], value)
                 self.assertEqual(parsed.info.max_resp_time, value)
-                self.assertEqual(parsed.alias, version)
+                self.assertEqual(parsed.alias, expected_alias(version))
                 self.assertEqual(cls.from_data(parsed.info).data, packet)
 
 
@@ -1126,7 +1136,7 @@ def test_ipv4_dispatches_igmp_to_each_version(self) -> None:
                     warnings.simplefilter('ignore')
                     parsed = IPv4(io.BytesIO(outer), len(outer))
                 self.assertIsInstance(parsed.payload, expected_classes[version])
-                self.assertEqual(parsed.payload.alias, version)
+                self.assertEqual(parsed.payload.alias, expected_alias(version))
 
 
 if __name__ == '__main__':