Skip to content

Commit d5a2236

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 marker assertion fails on the parent commit; the two reach guards pass either way, by design.
1 parent 7886d9f commit d5a2236

5 files changed

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

0 commit comments

Comments
 (0)