Skip to content

Commit 9b05395

Browse files
gh-107570: Argument Clinic: report errors on the offending line (GH-155250)
Errors raised while the docstring is checked were reported on the line which ends the clinic block, and errors raised while the code is generated were reported without a file name and a line number at all. Functions and parameters now record the line on which they are declared, and the function docstring records where it starts, so that such errors point at the offending line.
1 parent 61818b6 commit 9b05395

6 files changed

Lines changed: 79 additions & 17 deletions

File tree

Lib/test/test_clinic.py

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,7 @@ def test_ambiguous_group_and_optional_parameters(self):
347347
/
348348
[clinic start generated code]*/
349349
"""
350-
self.expect_failure(block, err)
350+
self.expect_failure(block, err, lineno=2)
351351

352352
def test_star_after_vararg(self):
353353
err = "'my_test_func' uses '*' more than once."
@@ -3063,9 +3063,22 @@ def test_state_func_docstring_no_summary(self):
30633063
m.func
30643064
docstring1
30653065
docstring2
3066+
docstring3
30663067
"""
3068+
# The line which should have been left blank.
30673069
self.expect_failure(block, err, lineno=3)
30683070

3071+
def test_state_func_docstring_long_summary(self):
3072+
err = "Summary line for 'm.func' is too long!"
3073+
block = f"""
3074+
module m
3075+
m.func
3076+
{'x' * 100}
3077+
3078+
Body.
3079+
"""
3080+
self.expect_failure(block, err, lineno=2)
3081+
30693082
def test_state_func_docstring_only_one_param_template(self):
30703083
err = "You may not specify {parameters} more than once in a docstring!"
30713084
block = """
@@ -3077,6 +3090,7 @@ def test_state_func_docstring_only_one_param_template(self):
30773090
{parameters}
30783091
these are the params again:
30793092
{parameters}
3093+
and this is the end of the docstring
30803094
"""
30813095
self.expect_failure(block, err, lineno=7)
30823096

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Argument Clinic: report errors on the offending line.
2+
Errors in a docstring were reported on the line which ends the block, and
3+
errors detected when generating the code were reported without any position.

Tools/clinic/libclinic/clanguage.py

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,10 @@ def render(
9292
for o in signatures:
9393
if isinstance(o, Function):
9494
if function:
95-
fail("You may specify at most one function per block.\nFound a block containing at least two:\n\t" + repr(function) + " and " + repr(o))
95+
fail("You may specify at most one function per block.\n"
96+
"Found a block containing at least two:\n\t"
97+
+ repr(function) + " and " + repr(o),
98+
line_number=o.line_number)
9699
function = o
97100
return self.render_function(clinic, function)
98101

@@ -337,7 +340,8 @@ def render_option_group_parsing(
337340
if count in subsets:
338341
fail(f"Function {f.full_name!r} has an ambiguous group "
339342
f"configuration: a call with {count} argument(s) "
340-
f"can be parsed in more than one way.")
343+
f"can be parsed in more than one way.",
344+
line_number=f.line_number)
341345
subsets[count] = subset
342346

343347
if limited_capi:
@@ -462,7 +466,8 @@ def render_function(
462466

463467
if has_option_groups and (not positional):
464468
fail("You cannot use optional groups ('[' and ']') "
465-
"unless all parameters are positional-only ('/').")
469+
"unless all parameters are positional-only ('/').",
470+
line_number=f.line_number)
466471

467472
# HACK
468473
# when we're METH_O, but have a custom return converter,

Tools/clinic/libclinic/dsl_parser.py

Lines changed: 45 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,8 @@ class DSLParser:
263263
critical_section: bool
264264
target_critical_section: list[str]
265265
disable_fastcall: bool
266+
# Line of the file which is being parsed.
267+
line_number: int | None
266268
from_version_re = re.compile(r'([*/]) +\[from +(.+)\]')
267269
permit_long_summary = False
268270
permit_long_docstring_body = False
@@ -286,6 +288,7 @@ def __init__(self, clinic: Clinic) -> None:
286288

287289
def reset(self) -> None:
288290
self.function = None
291+
self.line_number = None
289292
self.state = self.state_dsl_start
290293
self.expecting_parameters = True
291294
self.keyword_only = False
@@ -509,6 +512,7 @@ def parse(self, block: Block) -> None:
509512
if '\t' in line:
510513
fail(f'Tab characters are illegal in the Clinic DSL: {line!r}',
511514
line_number=block_start)
515+
self.line_number = line_number
512516
try:
513517
self.state(line)
514518
except ClinicError as exc:
@@ -517,7 +521,14 @@ def parse(self, block: Block) -> None:
517521
raise
518522

519523
self.do_post_block_processing_cleanup(line_number)
520-
block.output.extend(self.clinic.language.render(self.clinic, block.signatures))
524+
try:
525+
block.output.extend(
526+
self.clinic.language.render(self.clinic, block.signatures))
527+
except ClinicError as exc:
528+
if exc.lineno is None:
529+
exc.lineno = line_number
530+
exc.filename = self.clinic.filename
531+
raise
521532

522533
if self.preserve_output:
523534
if block.output:
@@ -666,6 +677,8 @@ def parse_cloned_function(self, names: FunctionNames, existing: str) -> None:
666677
"cls": cls,
667678
"c_basename": c_basename,
668679
"docstring": "",
680+
"docstring_line_number": None,
681+
"line_number": self.line_number,
669682
}
670683
if not (existing_function.kind is self.kind and
671684
existing_function.coexist == self.coexist):
@@ -735,7 +748,8 @@ def state_modulename_name(self, line: str) -> None:
735748
critical_section=self.critical_section,
736749
disable_fastcall=self.disable_fastcall,
737750
target_critical_section=self.target_critical_section,
738-
forced_text_signature=self.forced_text_signature
751+
forced_text_signature=self.forced_text_signature,
752+
line_number=self.line_number,
739753
)
740754
self.add_function(func)
741755

@@ -1141,7 +1155,8 @@ def bad_node(self, node: ast.AST) -> None:
11411155
converter=converter, default=value,
11421156
group=self.group_stack[-1] if self.group_stack else 0,
11431157
group_depth=len(self.group_stack),
1144-
deprecated_positional=self.deprecated_positional)
1158+
deprecated_positional=self.deprecated_positional,
1159+
line_number=self.line_number)
11451160

11461161
names = [k.name for k in self.function.parameters.values()]
11471162
if parameter_name in names[1:]:
@@ -1338,6 +1353,8 @@ def docstring_append(self, obj: Function | Parameter, line: str) -> None:
13381353
docstring = obj.docstring
13391354
if docstring:
13401355
docstring += "\n"
1356+
elif isinstance(obj, Function) and line.rstrip():
1357+
obj.docstring_line_number = self.line_number
13411358
if stripped := line.rstrip():
13421359
docstring += self.indent.dedent(stripped)
13431360
obj.docstring = docstring
@@ -1581,12 +1598,19 @@ def format_docstring(self) -> str:
15811598
# Guido said Clinic should enforce this:
15821599
# http://mail.python.org/pipermail/python-dev/2013-June/127110.html
15831600

1601+
def docstring_line(index: int) -> int | None:
1602+
"""Return the line of the file which holds the index-th line."""
1603+
if f.docstring_line_number is None:
1604+
return None
1605+
return f.docstring_line_number + index
1606+
15841607
lines = f.docstring.split('\n')
15851608
if len(lines) >= 2:
15861609
if lines[1]:
15871610
fail(f"Docstring for {f.full_name!r} does not have a summary line!\n"
15881611
"Every non-blank function docstring must start with "
1589-
"a single line summary followed by an empty line.")
1612+
"a single line summary followed by an empty line.",
1613+
line_number=docstring_line(1))
15901614
elif len(lines) == 1:
15911615
# the docstring is only one line right now--the summary line.
15921616
# add an empty line after the summary line so we have space
@@ -1598,28 +1622,36 @@ def format_docstring(self) -> str:
15981622
# Existing violations are recorded in OVERLONG_{SUMMARY,BODY}.
15991623
max_width = f.docstring_line_width
16001624
summary_len = len(lines[0])
1601-
max_body = max(map(len, lines[1:]))
1625+
long_body = [i for i, line in enumerate(lines)
1626+
if i and len(line) > max_width]
16021627
if summary_len > max_width:
16031628
if not self.permit_long_summary:
16041629
fail(f"Summary line for {f.full_name!r} is too long!\n"
1605-
f"The summary line must be no longer than {max_width} characters.")
1630+
f"The summary line must be no longer than {max_width} characters.",
1631+
line_number=docstring_line(0))
16061632
else:
16071633
if self.permit_long_summary:
16081634
warn("Remove the @permit_long_summary decorator from "
1609-
f"{f.full_name!r}!\n")
1635+
f"{f.full_name!r}!\n", filename=self.clinic.filename,
1636+
line_number=f.line_number)
16101637

1611-
if max_body > max_width:
1638+
if long_body:
16121639
if not self.permit_long_docstring_body:
16131640
warn(f"Docstring lines for {f.full_name!r} are too long!\n"
1614-
f"Lines should be no longer than {max_width} characters.")
1641+
f"Lines should be no longer than {max_width} characters.",
1642+
filename=self.clinic.filename,
1643+
line_number=docstring_line(long_body[0]))
16151644
else:
16161645
if self.permit_long_docstring_body:
16171646
warn("Remove the @permit_long_docstring_body decorator from "
1618-
f"{f.full_name!r}!\n")
1647+
f"{f.full_name!r}!\n", filename=self.clinic.filename,
1648+
line_number=f.line_number)
16191649

1650+
markers = [i for i, line in enumerate(lines) if '{parameters}' in line]
16201651
parameters_marker_count = len(f.docstring.split('{parameters}')) - 1
16211652
if parameters_marker_count > 1:
1622-
fail('You may not specify {parameters} more than once in a docstring!')
1653+
fail('You may not specify {parameters} more than once in a docstring!',
1654+
line_number=docstring_line(markers[-1]))
16231655

16241656
# insert signature at front and params after the summary line
16251657
if not parameters_marker_count:
@@ -1679,6 +1711,7 @@ def do_post_block_processing_cleanup(self, lineno: int) -> None:
16791711
try:
16801712
self.function.docstring = self.format_docstring()
16811713
except ClinicError as exc:
1682-
exc.lineno = lineno
1714+
if exc.lineno is None:
1715+
exc.lineno = lineno
16831716
exc.filename = self.clinic.filename
16841717
raise

Tools/clinic/libclinic/function.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,10 @@ class Function:
118118
critical_section: bool = False
119119
disable_fastcall: bool = False
120120
target_critical_section: list[str] = dc.field(default_factory=list)
121+
# Line of the file on which the function is declared.
122+
line_number: int | None = None
123+
# Line on which the docstring starts (`None` if there is no docstring).
124+
docstring_line_number: int | None = None
121125

122126
def __post_init__(self) -> None:
123127
self.parent = self.cls or self.module
@@ -220,6 +224,8 @@ class Parameter:
220224
# (`None` signifies that there is no deprecation)
221225
deprecated_positional: VersionTuple | None = None
222226
deprecated_keyword: VersionTuple | None = None
227+
# Line of the file on which the parameter is declared.
228+
line_number: int | None = None
223229
right_bracket_count: int = dc.field(init=False, default=0)
224230

225231
def __repr__(self) -> str:

Tools/clinic/libclinic/parse_args.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,8 @@ def select_prototypes(self) -> None:
346346
self.docstring_definition = GETSET_DOCSTRING_PROTOTYPE_STRVAR
347347
elif self.func.kind in SETTERS:
348348
if self.func.docstring:
349-
fail("docstrings are only supported for @getter, not @setter")
349+
fail("docstrings are only supported for @getter, not @setter",
350+
line_number=self.func.line_number)
350351
self.return_value_declaration = "int {parser_retval};"
351352
self.methoddef_define = SETTERDEF_PROTOTYPE_DEFINE
352353
else:

0 commit comments

Comments
 (0)