mirror of
https://github.com/safishamsi/graphify.git
synced 2026-10-13 23:49:14 +00:00
Fix module docstring extraction when a leading comment precedes it
_get_docstring's loop checked only the FIRST child of a module/class/
function body for a docstring expression_statement, then unconditionally
broke -- so a leading comment (the #! shebang essentially every
executable Python script starts with, a coding declaration, a license
header) sat in that first-child slot instead of the docstring, and the
real docstring one position later was never seen. No warning, no
partial-extraction report, exit 0 -- module rationale for an entire
class of file (any shebang-led or comment-led script) was silently
absent from the graph.
Comments are not statements: a docstring must be the first STATEMENT
in the body, so the fix skips leading `comment` children before
checking whether the first real statement is a docstring. The same
shared helper also serves class/function bodies, so a comment as the
first line of a function/class body is fixed too, not just the
module-level case.
Fixes #3312.
(cherry picked from commit 87d4cb49b7)
This commit is contained in:
1 parent
937e59a547
commit
749206102d
2 files changed
+106
No files matched your search
@@ -1205,9 +1205,22 @@ def _extract_python_rationale(path: Path, result: dict) -> None:
|
||||
file_nid = _make_id(str(path))
|
||||
|
||||
def _get_docstring(body_node) -> tuple[str, int] | None:
|
||||
"""A docstring is the first STATEMENT in a module/class/function body.
|
||||
|
||||
A leading `comment` node — the shebang line essentially every
|
||||
executable script starts with, a coding-declaration or license
|
||||
header, or any ordinary comment — is not a statement: tree-sitter
|
||||
still parses it as a sibling child of the body, but Python's own
|
||||
docstring rule skips right over it. The old unconditional `break`
|
||||
after the first loop iteration stopped at that comment instead of
|
||||
looking past it, so a module docstring behind a shebang (or any
|
||||
leading comment) was silently never found (#3312).
|
||||
"""
|
||||
if not body_node:
|
||||
return None
|
||||
for child in body_node.children:
|
||||
if child.type == "comment":
|
||||
continue
|
||||
if child.type == "expression_statement":
|
||||
for sub in child.children:
|
||||
if sub.type in ("string", "concatenated_string"):
|
||||
|
||||
@@ -45,6 +45,99 @@ def test_class_docstring_extracted(tmp_path):
|
||||
assert any("Redis" in n["label"] for n in rationale)
|
||||
|
||||
|
||||
def test_module_docstring_extracted_behind_shebang(tmp_path):
|
||||
"""#3312: a leading `#!` shebang comment must not hide the module docstring
|
||||
that follows it — essentially every executable Python script starts with
|
||||
one, so this silently dropped module rationale for that whole class of
|
||||
file."""
|
||||
p = tmp_path / "sample.py"
|
||||
p.write_text(
|
||||
"#!/usr/bin/env python3\n"
|
||||
'"""This module handles authentication because legacy sessions were insecure."""\n'
|
||||
"def login(): pass\n"
|
||||
)
|
||||
result = extract_python(p)
|
||||
rationale = [n for n in result["nodes"] if n.get("file_type") == "rationale"]
|
||||
assert any("authentication" in n["label"] for n in rationale)
|
||||
module_ds = next(n for n in rationale if "authentication" in n["label"])
|
||||
assert module_ds["source_location"] == "L2"
|
||||
|
||||
|
||||
def test_module_docstring_extracted_behind_plain_comment(tmp_path):
|
||||
"""#3312: same bug, non-shebang trigger — any ordinary leading comment
|
||||
(license header, coding declaration, ...) hid the module docstring."""
|
||||
p = tmp_path / "sample.py"
|
||||
p.write_text(
|
||||
"# an ordinary leading comment, not a shebang\n"
|
||||
'"""This module handles authentication because legacy sessions were insecure."""\n'
|
||||
"def login(): pass\n"
|
||||
)
|
||||
result = extract_python(p)
|
||||
rationale = [n for n in result["nodes"] if n.get("file_type") == "rationale"]
|
||||
assert any("authentication" in n["label"] for n in rationale)
|
||||
|
||||
|
||||
def test_module_docstring_extracted_behind_multiple_leading_comments(tmp_path):
|
||||
"""#3312: a multi-line header (shebang + coding declaration + license) —
|
||||
every leading comment must be skipped, not just the first one."""
|
||||
p = tmp_path / "sample.py"
|
||||
p.write_text(
|
||||
"#!/usr/bin/env python3\n"
|
||||
"# -*- coding: utf-8 -*-\n"
|
||||
"# Copyright 2026 Someone. All rights reserved.\n"
|
||||
'"""This module handles authentication because legacy sessions were insecure."""\n'
|
||||
"def login(): pass\n"
|
||||
)
|
||||
result = extract_python(p)
|
||||
rationale = [n for n in result["nodes"] if n.get("file_type") == "rationale"]
|
||||
assert any("authentication" in n["label"] for n in rationale)
|
||||
|
||||
|
||||
def test_function_docstring_extracted_behind_leading_comment(tmp_path):
|
||||
"""#3312: the same _get_docstring helper serves function/class bodies too —
|
||||
a comment as the first line of a function body must not hide its
|
||||
docstring either."""
|
||||
path = _write_py(tmp_path, '''
|
||||
def process():
|
||||
# a leading comment inside the function body
|
||||
"""We use chunked processing here because the full dataset exceeds RAM."""
|
||||
pass
|
||||
''')
|
||||
result = extract_python(path)
|
||||
rationale = [n for n in result["nodes"] if n.get("file_type") == "rationale"]
|
||||
assert any("chunked" in n["label"] for n in rationale)
|
||||
|
||||
|
||||
def test_no_docstring_after_comment_extracts_nothing(tmp_path):
|
||||
"""#3312 follow-up: a leading comment with NO docstring after it must not
|
||||
fabricate a rationale node from something else in the body."""
|
||||
p = tmp_path / "sample.py"
|
||||
p.write_text(
|
||||
"#!/usr/bin/env python3\n"
|
||||
"# just a comment, no docstring at all\n"
|
||||
"import os\n"
|
||||
"x = 1\n"
|
||||
)
|
||||
result = extract_python(p)
|
||||
rationale = [n for n in result["nodes"] if n.get("file_type") == "rationale"]
|
||||
assert rationale == []
|
||||
|
||||
|
||||
def test_non_first_statement_string_not_treated_as_docstring(tmp_path):
|
||||
"""#3312 follow-up: a bare string literal that is NOT the first statement
|
||||
(because an assignment precedes it) is not a docstring and must still be
|
||||
correctly rejected after the leading-comment skip is added."""
|
||||
p = tmp_path / "sample.py"
|
||||
p.write_text(
|
||||
"#!/usr/bin/env python3\n"
|
||||
"x = 1\n"
|
||||
'"""This string is not a docstring since it is not the first statement, long enough."""\n'
|
||||
)
|
||||
result = extract_python(p)
|
||||
rationale = [n for n in result["nodes"] if n.get("file_type") == "rationale"]
|
||||
assert rationale == []
|
||||
|
||||
|
||||
def test_rationale_comment_extracted(tmp_path):
|
||||
path = _write_py(tmp_path, '''
|
||||
def build():
|
||||
|
||||
Reference in new issue
Block a user