Skip to content

Commit 9372e1b

Browse files
timsaucerclaude
andcommitted
test: match the hook table against dispatch sites, not raw text
`test_hook_reference_table_lists_every_hook` scanned every byte of `crates/**.rs` and `python/datafusion/**.py` for `__datafusion_*__`, so comments and docstrings counted as evidence a hook exists. A doc-comment contrasting a hook with one that was removed, or naming a hypothetical, would have had to be deleted or added to the guide's table, and neither is right. Count sites instead. On the Rust side a site is a string literal holding nothing but the hook name -- what `hasattr`, `getattr`, and `call_capsule_getter` are handed -- or a `fn` of that name, which is a hook the host implements itself. Error strings that merely embed a name no longer count; each already sits beside a real lookup. On the Python side, read the syntax tree rather than the text: a method being defined, an attribute being accessed, or a string standing alone. A docstring is one string node holding the whole docstring, so prose drops out without a rule of its own. The dispatched set is unchanged at 18, still matching the table exactly. Verified in both directions: a comment naming a removed hook now passes, while adding a real lookup for an undocumented name still fails. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 5a1bfeb commit 9372e1b

1 file changed

Lines changed: 58 additions & 10 deletions

File tree

python/tests/test_docstrings.py

Lines changed: 58 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,26 @@
4444
# The hook reference in the extension guide is a hand-written table of every
4545
# `__datafusion_*__` name. Nothing about adding a hook forces it to be updated,
4646
# so the table is compared against the names the package actually dispatches.
47+
#
48+
# "Dispatches" means a *site*, not an occurrence: the name spelled where the
49+
# host looks the hook up or defines its own, not everywhere the name is
50+
# written. Scanning raw text would make the table's contents depend on prose —
51+
# a doc-comment contrasting a hook with one that was removed, or naming a
52+
# hypothetical, would have to be either deleted or added to the table, and
53+
# neither is right. Structure answers the question text cannot.
4754
HOOK_REFERENCE = REPO_ROOT / "docs" / "source" / "extension-guide" / "index.md"
48-
HOOK_NAME = re.compile(r"__datafusion_[a-z_]+__")
55+
HOOK_NAME = re.compile(r"^__datafusion_[a-z_]+__$")
4956
HOOK_TABLE_ROW = re.compile(r"^\| `(__datafusion_[a-z_]+__)`")
5057

58+
# A Rust dispatch site is a string literal holding nothing but the hook name —
59+
# what `hasattr`, `getattr`, and `call_capsule_getter` are handed — or a `fn`
60+
# of that name, which is a hook the host itself implements. An error message
61+
# that merely embeds the name (`"__datafusion_scalar_udf__ does not exist"`)
62+
# is prose and does not count; every such message sits beside a real lookup.
63+
RUST_HOOK_SITE = re.compile(
64+
r'"(__datafusion_[a-z_]+__)"|\bfn\s+(__datafusion_[a-z_]+__)\b'
65+
)
66+
5167
# Above this, a docstring has stopped being a contract and become a
5268
# narrative. Move the argument to a guide page under `docs/source/` and leave
5369
# a one-line pointer. Raising this is not the fix.
@@ -139,18 +155,46 @@ def test_extension_api_has_a_doctest(name: str, obj: object) -> None:
139155
)
140156

141157

158+
def _rust_hook_sites() -> set[str]:
159+
"""Hook names Rust looks up or defines. See :data:`RUST_HOOK_SITE`."""
160+
found: set[str] = set()
161+
for path in sorted((REPO_ROOT / "crates").rglob("*.rs")):
162+
for match in RUST_HOOK_SITE.finditer(path.read_text()):
163+
found.add(match.group(1) or match.group(2))
164+
return found
165+
166+
167+
def _python_hook_sites() -> set[str]:
168+
"""Hook names the Python wrappers call, declare, or look up by string.
169+
170+
Read off the syntax tree rather than the text, so a name is counted when
171+
it is an attribute being accessed (``extension.__datafusion_x__(ctx)``), a
172+
method being defined (the protocol stubs), or a string standing alone (a
173+
``getattr`` argument) — and not when it merely appears inside a docstring.
174+
"""
175+
found: set[str] = set()
176+
for path in sorted(PACKAGE_ROOT.rglob("*.py")):
177+
tree = ast.parse(path.read_text(), filename=str(path))
178+
for node in ast.walk(tree):
179+
if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)):
180+
name = node.name
181+
elif isinstance(node, ast.Attribute):
182+
name = node.attr
183+
elif isinstance(node, ast.Constant) and isinstance(node.value, str):
184+
name = node.value
185+
else:
186+
continue
187+
if HOOK_NAME.match(name):
188+
found.add(name)
189+
return found
190+
191+
142192
def test_hook_reference_table_lists_every_hook() -> None:
143193
"""The guide's hook table matches the hooks the package dispatches."""
144194
if not HOOK_REFERENCE.is_file():
145195
pytest.skip("running against an installed wheel, without docs/ or crates/")
146196

147-
dispatched: set[str] = set()
148-
for directory, suffix in (
149-
(REPO_ROOT / "crates", "*.rs"),
150-
(PACKAGE_ROOT, "*.py"),
151-
):
152-
for path in sorted(directory.rglob(suffix)):
153-
dispatched.update(HOOK_NAME.findall(path.read_text()))
197+
dispatched = _rust_hook_sites() | _python_hook_sites()
154198

155199
documented = {
156200
match.group(1)
@@ -162,12 +206,16 @@ def test_hook_reference_table_lists_every_hook() -> None:
162206
f" dispatched but not in the table: {name}"
163207
for name in sorted(dispatched - documented)
164208
] + [
165-
f" in the table but nowhere in the source: {name}"
209+
f" in the table but dispatched from nowhere: {name}"
166210
for name in sorted(documented - dispatched)
167211
]
168212
assert not problems, (
169213
f"{HOOK_REFERENCE.relative_to(REPO_ROOT)} is out of sync with the "
170-
"hooks in crates/ and python/datafusion/:\n" + "\n".join(problems)
214+
"hooks in crates/ and python/datafusion/:\n"
215+
+ "\n".join(problems)
216+
+ "\n\nOnly dispatch sites count — a name looked up by string, an "
217+
"attribute accessed, or a method defined. Naming a hook in prose does "
218+
"not put it in this set, and does not belong in the table either."
171219
)
172220

173221

0 commit comments

Comments
 (0)