Skip to content

Commit 0891a80

Browse files
committed
chore(vendor): HIP Group and Packet crawlers write #: in _missing_, not # (#2331)
Problem: for an unassigned range, Group.process and Packet.process wrote a plain # comment, where every other crawler -- the shared Vendor.process and the HIP Parameter and EdDSACurve crawlers -- writes #:, the form Sphinx autodoc renders as an attribute comment. Fix: both now write #:, and the generated const modules are regenerated against the live IANA registry (the diff is exactly the four marker lines, confirming no unrelated registry drift). Test: tests/vendor/test_vendor_missing_comment_marker_2331_unit.py walks every crawler's miss-comment lines and every const module's matching range-guarded shape, asserting #: throughout. Each assertion fails on the parent commit.
1 parent 7886d9f commit 0891a80

5 files changed

Lines changed: 273 additions & 6 deletions

File tree

‎pcapkit/const/hip/group.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,6 @@ def _missing_(cls, value: 'int') -> 'Group':
9595
if not (isinstance(value, int) and 0 <= value <= 255):
9696
raise ValueError('%r is not a valid %s' % (value, cls.__name__))
9797
if 12 <= value <= 255:
98-
# Unassigned
98+
#: Unassigned
9999
return cls._unregistered_member(value, 'Unassigned')
100100
return super()._missing_(value)

‎pcapkit/const/hip/packet.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,12 +92,12 @@ def _missing_(cls, value: 'int') -> 'Packet':
9292
if not (isinstance(value, int) and 0 <= value <= 127):
9393
raise ValueError('%r is not a valid %s' % (value, cls.__name__))
9494
if 5 <= value <= 15:
95-
# Unassigned
95+
#: Unassigned
9696
return cls._unregistered_member(value, 'Unassigned')
9797
if 21 <= value <= 31:
98-
# Unassigned
98+
#: Unassigned
9999
return cls._unregistered_member(value, 'Unassigned')
100100
if 33 <= value <= 127:
101-
# Unassigned
101+
#: Unassigned
102102
return cls._unregistered_member(value, 'Unassigned')
103103
return super()._missing_(value)

‎pcapkit/vendor/hip/group.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ def process(self, data: 'list[str]') -> 'tuple[list[str], list[str]]':
7878
start, stop = item[0].split('-')
7979

8080
miss.append(f'if {start} <= value <= {stop}:')
81-
miss.append(f' # {desc}')
81+
miss.append(f' #: {desc}')
8282
miss.append(f" return cls._unregistered_member(value, '{self.safe_name(name)}')")
8383
return enum, miss
8484

‎pcapkit/vendor/hip/packet.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ def process(self, data: 'list[str]') -> 'tuple[list[str], list[str]]':
7979
start, stop = item[0].split('-')
8080

8181
miss.append(f'if {start} <= value <= {stop}:')
82-
miss.append(f' # {desc}')
82+
miss.append(f' #: {desc}')
8383
miss.append(f" return cls._unregistered_member(value, '{self.safe_name(name)}')")
8484
return enum, miss
8585

Lines changed: 267 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,267 @@
1+
# -*- coding: utf-8 -*-
2+
"""Every comment a crawler renders into ``_missing_`` is a ``#:`` one.
3+
4+
GitHub issue #2331. :meth:`pcapkit.vendor.default.Vendor.process` renders a
5+
range-guarded :meth:`~enum.Enum._missing_` branch as three lines -- an ``if``
6+
header, a comment naming the span, then an indented ``return``::
7+
8+
miss.append(f'if {start} <= value <= {stop}:')
9+
miss.append(f' #: {desc}')
10+
miss.append(f" return cls._unregistered_member(value, '{self.safe_name(name)}')")
11+
12+
``#:`` is Sphinx's attribute-doc marker, and every crawler rendered it bar
13+
:mod:`pcapkit.vendor.hip.group` and :mod:`pcapkit.vendor.hip.packet`, which
14+
rendered a plain ``#``. So their unassigned spans in
15+
:mod:`pcapkit.const.hip.group` and :mod:`pcapkit.const.hip.packet` read
16+
``# Unassigned`` where every other span across :mod:`pcapkit.const` reads
17+
``#: Unassigned``. The rendered documentation is not what the fix is about --
18+
autodoc reads a ``#:`` comment where an attribute is declared, not one inside a
19+
function body, as the issue itself notes -- it is that the generators emit one
20+
form rather than two.
21+
22+
Nothing observable changed: a comment is a comment to the interpreter either
23+
way, so no behavioural suite could have caught it, and nothing checked the
24+
marker. Hence the two static sweeps below, one per layer:
25+
26+
* :class:`VendorMissingCommentMarkerTests` reads the crawlers' own ASTs, so a
27+
crawler that starts emitting ``#`` is caught before anyone regenerates.
28+
* :class:`ConstMissingCommentMarkerTests` reads the committed registries, so a
29+
generated file that disagrees with the generator which will overwrite it is
30+
caught too -- the same two-layer split
31+
:file:`tests/vendor/test_vendor_missing_body_unit.py` makes for #866.
32+
33+
Neither needs the network, a regeneration, or an importable
34+
:mod:`pcapkit.vendor`: both read the checked-out tree as text.
35+
36+
"""
37+
from __future__ import annotations
38+
39+
import ast
40+
import pathlib
41+
import re
42+
import unittest
43+
44+
#: Test tier (#1958): reads the vendor templates and the committed constant modules
45+
#: as text, a question about the tree rather than about an interpreter.
46+
__tier__ = 'LATEST'
47+
48+
#: Repository root, i.e. the grandparent of the directory holding this file.
49+
ROOT = pathlib.Path(__file__).resolve().parents[2]
50+
51+
#: Root of the vendor crawler package, as a path rather than an import, so this
52+
#: module reads the same tree it is checked out in regardless of which
53+
#: ``pcapkit`` an editable install happens to resolve.
54+
VENDOR_ROOT = ROOT / 'pcapkit' / 'vendor'
55+
56+
#: Root of the generated constant package, read the same way and for the same
57+
#: reason.
58+
CONST_ROOT = ROOT / 'pcapkit' / 'const'
59+
60+
#: The range guard :meth:`~pcapkit.vendor.default.Vendor.process` opens an
61+
#: unassigned-span branch with. Anchors :func:`_const_span_comments` to the
62+
#: *generated* shape, so the prose ``#`` comments that
63+
#: :mod:`pcapkit.const.ftp.command` and :mod:`pcapkit.const.http.method`
64+
#: hand-write above their own ``_unregistered_member`` calls are not swept:
65+
#: those are explanatory notes, not attribute docs, and ``#`` is right for them.
66+
SPAN_GUARD = re.compile(r'^\s*if .*<= value <=.*:$')
67+
68+
#: The ``return`` such a branch closes on.
69+
SPAN_RETURN = re.compile(r'^\s*return cls\._unregistered_member\(')
70+
71+
#: Crawlers the AST walk must reach, so a walk that silently matched nothing --
72+
#: an ``ast`` change, a rename of the ``miss`` local -- fails loudly instead of
73+
#: sweeping an empty set. Named rather than counted: a count of what the tree
74+
#: holds drifts as soon as the tree does, which :ref:`documentation`'s *state no
75+
#: count* rules against (:issue:`1103`), and the vendor cron regenerates this
76+
#: tree on its own schedule.
77+
REQUIRED_CRAWLERS = (
78+
'pcapkit/vendor/default.py', # the shared template every crawler inherits
79+
'pcapkit/vendor/hip/group.py', # #2331's pair
80+
'pcapkit/vendor/hip/packet.py',
81+
'pcapkit/vendor/hip/parameter.py', # their already-correct HIP siblings
82+
'pcapkit/vendor/hip/eddsa_curve.py',
83+
)
84+
85+
#: Registries the text sweep must reach, for the same reason and named the same
86+
#: way.
87+
REQUIRED_REGISTRIES = (
88+
'pcapkit/const/hip/group.py',
89+
'pcapkit/const/hip/packet.py',
90+
)
91+
92+
93+
def _leading_text(node: 'ast.expr') -> 'str | None':
94+
"""Read the literal text a ``miss`` line starts with.
95+
96+
A crawler renders a line either as a plain :class:`str` constant or as an
97+
f-string whose first segment is one -- ``f' #: {desc}'``. Only that
98+
leading segment decides whether the line is a comment, so an f-string's
99+
interpolations are not inspected.
100+
101+
Args:
102+
node: Expression appended to, or listed in, ``miss``.
103+
104+
Returns:
105+
The leading literal text, or :obj:`None` when the line begins with
106+
something other than literal text, e.g. ``f'{prefix}...'``.
107+
108+
"""
109+
if isinstance(node, ast.Constant) and isinstance(node.value, str):
110+
return node.value
111+
if isinstance(node, ast.JoinedStr) and node.values:
112+
head = node.values[0]
113+
if isinstance(head, ast.Constant) and isinstance(head.value, str):
114+
return head.value
115+
return None
116+
117+
118+
def _vendor_comment_lines() -> 'list[tuple[str, str]]':
119+
"""Collect every comment line the crawlers render into ``_missing_``.
120+
121+
Walks each crawler's AST for a line added to ``miss``, whether by
122+
``miss.append(...)`` or as an element of a ``miss = [...]`` literal, and
123+
keeps the ones whose leading literal text is a comment.
124+
125+
Returns:
126+
Pairs of repository-relative crawler path and comment line.
127+
128+
"""
129+
found = [] # type: list[tuple[str, str]]
130+
for path in sorted(VENDOR_ROOT.rglob('*.py')):
131+
tree = ast.parse(path.read_text(encoding='utf-8'))
132+
lines = [] # type: list[ast.expr]
133+
for node in ast.walk(tree):
134+
if (isinstance(node, ast.Call)
135+
and isinstance(node.func, ast.Attribute) and node.func.attr == 'append'
136+
and isinstance(node.func.value, ast.Name) and node.func.value.id == 'miss'
137+
and node.args):
138+
lines.append(node.args[0])
139+
elif (isinstance(node, ast.Assign) and isinstance(node.value, ast.List)
140+
and any(isinstance(target, ast.Name) and target.id == 'miss'
141+
for target in node.targets)):
142+
lines.extend(node.value.elts)
143+
for line in lines:
144+
text = _leading_text(line)
145+
if text is not None and text.lstrip().startswith('#'):
146+
found.append((str(path.relative_to(ROOT)), text))
147+
return found
148+
149+
150+
def _const_span_comments() -> 'list[tuple[str, int, str]]':
151+
"""Collect every comment inside a generated unassigned-span branch.
152+
153+
A branch is recognised by its shape rather than by its text: a
154+
:data:`SPAN_GUARD` header, one or more comment lines, then a
155+
:data:`SPAN_RETURN`. A comment sitting anywhere else -- including one above
156+
an ``_unregistered_member`` call that no range guard opens -- is not swept.
157+
158+
Returns:
159+
Triples of repository-relative module path, 1-based line number and
160+
comment line.
161+
162+
"""
163+
found = [] # type: list[tuple[str, int, str]]
164+
for path in sorted(CONST_ROOT.rglob('*.py')):
165+
lines = path.read_text(encoding='utf-8').splitlines()
166+
for index, line in enumerate(lines):
167+
if not SPAN_GUARD.match(line):
168+
continue
169+
cursor = index + 1
170+
comments = [] # type: list[tuple[int, str]]
171+
while cursor < len(lines) and lines[cursor].lstrip().startswith('#'):
172+
comments.append((cursor + 1, lines[cursor]))
173+
cursor += 1
174+
if comments and cursor < len(lines) and SPAN_RETURN.match(lines[cursor]):
175+
found.extend((str(path.relative_to(ROOT)), number, text)
176+
for number, text in comments)
177+
return found
178+
179+
180+
class VendorMissingCommentMarkerTests(unittest.TestCase):
181+
"""The crawler layer: what the generators emit."""
182+
183+
@classmethod
184+
def setUpClass(cls) -> 'None':
185+
cls.comments = _vendor_comment_lines() # type: ignore[attr-defined]
186+
187+
def test_the_sweep_reaches_the_crawlers_it_is_about(self) -> 'None':
188+
"""A walk that matched nothing would pass every check below vacuously."""
189+
swept = {crawler for crawler, _ in self.comments} # type: ignore[attr-defined]
190+
for crawler in REQUIRED_CRAWLERS:
191+
with self.subTest(crawler=crawler):
192+
self.assertIn(crawler, swept)
193+
194+
def test_every_crawler_renders_a_sphinx_comment(self) -> 'None':
195+
"""One form of comment across the generators, which is ``#:``."""
196+
for crawler, text in self.comments: # type: ignore[attr-defined]
197+
with self.subTest(crawler=crawler, line=text):
198+
self.assertTrue(text.lstrip().startswith('#:'),
199+
f'{crawler} renders {text!r} into ``_missing_``, a plain '
200+
f'``#`` comment where every other crawler renders ``#:``; '
201+
f'see GitHub issue #2331')
202+
203+
def test_the_hip_group_and_packet_crawlers_render_sphinx_comments(self) -> 'None':
204+
"""The specific pair from #2331, pinned by name.
205+
206+
Kept alongside the general sweep because these are the crawlers the
207+
issue was filed against, and a named test says which file to look at
208+
without having to read a subTest label.
209+
210+
"""
211+
for crawler in ('pcapkit/vendor/hip/group.py', 'pcapkit/vendor/hip/packet.py'):
212+
with self.subTest(crawler=crawler):
213+
texts = [text for path, text in self.comments # type: ignore[attr-defined]
214+
if path == crawler]
215+
self.assertEqual(texts, [' #: '])
216+
217+
218+
class ConstMissingCommentMarkerTests(unittest.TestCase):
219+
"""The generated layer: what the committed registries carry."""
220+
221+
@classmethod
222+
def setUpClass(cls) -> 'None':
223+
cls.comments = _const_span_comments() # type: ignore[attr-defined]
224+
225+
def test_the_sweep_reaches_the_registries_it_is_about(self) -> 'None':
226+
"""A sweep that matched nothing would pass every check below vacuously."""
227+
swept = {module for module, _, _ in self.comments} # type: ignore[attr-defined]
228+
for module in REQUIRED_REGISTRIES:
229+
with self.subTest(module=module):
230+
self.assertIn(module, swept)
231+
232+
def test_every_registry_carries_a_sphinx_comment(self) -> 'None':
233+
"""A generated file must not disagree with the generator."""
234+
for module, number, text in self.comments: # type: ignore[attr-defined]
235+
with self.subTest(module=module, line=number):
236+
self.assertTrue(text.lstrip().startswith('#:'),
237+
f'{module}:{number} reads {text.strip()!r} inside an '
238+
f'unassigned-span branch, a plain ``#`` comment where the '
239+
f'crawlers render ``#:``; see GitHub issue #2331')
240+
241+
def test_the_hip_group_and_packet_registries_carry_sphinx_comments(self) -> 'None':
242+
"""The spans #2331 names, pinned by module rather than by tally.
243+
244+
How many unassigned spans each registry has is IANA's business and
245+
changes with each regeneration, so this checks what every one of them
246+
reads instead of how many there are.
247+
248+
"""
249+
for module in REQUIRED_REGISTRIES:
250+
with self.subTest(module=module):
251+
texts = {text.strip() for path, _, text
252+
in self.comments if path == module} # type: ignore[attr-defined]
253+
self.assertEqual(texts, {'#: Unassigned'})
254+
255+
def test_the_hip_group_and_packet_lookups_still_resolve(self) -> 'None':
256+
"""Only the comment text changed, so the lookups behave as before."""
257+
from pcapkit.const.hip.group import Group
258+
from pcapkit.const.hip.packet import Packet
259+
260+
self.assertEqual(Group(12).name, 'Unassigned')
261+
self.assertEqual(Packet(33).name, 'Unassigned')
262+
self.assertNotIn('Unassigned', Group.__members__)
263+
self.assertNotIn('Unassigned', Packet.__members__)
264+
265+
266+
if __name__ == '__main__':
267+
unittest.main()

0 commit comments

Comments
 (0)