Skip to content

Commit 04da192

Browse files
gh-107570: Argument Clinic: report errors on the offending line
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 1d90627 commit 04da192

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."
@@ -2925,9 +2925,22 @@ def test_state_func_docstring_no_summary(self):
29252925
m.func
29262926
docstring1
29272927
docstring2
2928+
docstring3
29282929
"""
2930+
# The line which should have been left blank.
29292931
self.expect_failure(block, err, lineno=3)
29302932

2933+
def test_state_func_docstring_long_summary(self):
2934+
err = "Summary line for 'm.func' is too long!"
2935+
block = f"""
2936+
module m
2937+
m.func
2938+
{'x' * 100}
2939+
2940+
Body.
2941+
"""
2942+
self.expect_failure(block, err, lineno=2)
2943+
29312944
def test_state_func_docstring_only_one_param_template(self):
29322945
err = "You may not specify {parameters} more than once in a docstring!"
29332946
block = """
@@ -2939,6 +2952,7 @@ def test_state_func_docstring_only_one_param_template(self):
29392952
{parameters}
29402953
these are the params again:
29412954
{parameters}
2955+
and this is the end of the docstring
29422956
"""
29432957
self.expect_failure(block, err, lineno=7)
29442958

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
@@ -91,7 +91,10 @@ def render(
9191
for o in signatures:
9292
if isinstance(o, Function):
9393
if function:
94-
fail("You may specify at most one function per block.\nFound a block containing at least two:\n\t" + repr(function) + " and " + repr(o))
94+
fail("You may specify at most one function per block.\n"
95+
"Found a block containing at least two:\n\t"
96+
+ repr(function) + " and " + repr(o),
97+
line_number=o.line_number)
9598
function = o
9699
return self.render_function(clinic, function)
97100

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

342346
if limited_capi:
@@ -461,7 +465,8 @@ def render_function(
461465

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

466471
# HACK
467472
# 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
@@ -499,6 +502,7 @@ def parse(self, block: Block) -> None:
499502
if '\t' in line:
500503
fail(f'Tab characters are illegal in the Clinic DSL: {line!r}',
501504
line_number=block_start)
505+
self.line_number = line_number
502506
try:
503507
self.state(line)
504508
except ClinicError as exc:
@@ -507,7 +511,14 @@ def parse(self, block: Block) -> None:
507511
raise
508512

509513
self.do_post_block_processing_cleanup(line_number)
510-
block.output.extend(self.clinic.language.render(self.clinic, block.signatures))
514+
try:
515+
block.output.extend(
516+
self.clinic.language.render(self.clinic, block.signatures))
517+
except ClinicError as exc:
518+
if exc.lineno is None:
519+
exc.lineno = line_number
520+
exc.filename = self.clinic.filename
521+
raise
511522

512523
if self.preserve_output:
513524
if block.output:
@@ -656,6 +667,8 @@ def parse_cloned_function(self, names: FunctionNames, existing: str) -> None:
656667
"cls": cls,
657668
"c_basename": c_basename,
658669
"docstring": "",
670+
"docstring_line_number": None,
671+
"line_number": self.line_number,
659672
}
660673
if not (existing_function.kind is self.kind and
661674
existing_function.coexist == self.coexist):
@@ -725,7 +738,8 @@ def state_modulename_name(self, line: str) -> None:
725738
critical_section=self.critical_section,
726739
disable_fastcall=self.disable_fastcall,
727740
target_critical_section=self.target_critical_section,
728-
forced_text_signature=self.forced_text_signature
741+
forced_text_signature=self.forced_text_signature,
742+
line_number=self.line_number,
729743
)
730744
self.add_function(func)
731745

@@ -1116,7 +1130,8 @@ def bad_node(self, node: ast.AST) -> None:
11161130
converter=converter, default=value,
11171131
group=self.group_stack[-1] if self.group_stack else 0,
11181132
group_depth=len(self.group_stack),
1119-
deprecated_positional=self.deprecated_positional)
1133+
deprecated_positional=self.deprecated_positional,
1134+
line_number=self.line_number)
11201135

11211136
names = [k.name for k in self.function.parameters.values()]
11221137
if parameter_name in names[1:]:
@@ -1313,6 +1328,8 @@ def docstring_append(self, obj: Function | Parameter, line: str) -> None:
13131328
docstring = obj.docstring
13141329
if docstring:
13151330
docstring += "\n"
1331+
elif isinstance(obj, Function) and line.rstrip():
1332+
obj.docstring_line_number = self.line_number
13161333
if stripped := line.rstrip():
13171334
docstring += self.indent.dedent(stripped)
13181335
obj.docstring = docstring
@@ -1556,12 +1573,19 @@ def format_docstring(self) -> str:
15561573
# Guido said Clinic should enforce this:
15571574
# http://mail.python.org/pipermail/python-dev/2013-June/127110.html
15581575

1576+
def docstring_line(index: int) -> int | None:
1577+
"""Return the line of the file which holds the index-th line."""
1578+
if f.docstring_line_number is None:
1579+
return None
1580+
return f.docstring_line_number + index
1581+
15591582
lines = f.docstring.split('\n')
15601583
if len(lines) >= 2:
15611584
if lines[1]:
15621585
fail(f"Docstring for {f.full_name!r} does not have a summary line!\n"
15631586
"Every non-blank function docstring must start with "
1564-
"a single line summary followed by an empty line.")
1587+
"a single line summary followed by an empty line.",
1588+
line_number=docstring_line(1))
15651589
elif len(lines) == 1:
15661590
# the docstring is only one line right now--the summary line.
15671591
# add an empty line after the summary line so we have space
@@ -1573,28 +1597,36 @@ def format_docstring(self) -> str:
15731597
# Existing violations are recorded in OVERLONG_{SUMMARY,BODY}.
15741598
max_width = f.docstring_line_width
15751599
summary_len = len(lines[0])
1576-
max_body = max(map(len, lines[1:]))
1600+
long_body = [i for i, line in enumerate(lines)
1601+
if i and len(line) > max_width]
15771602
if summary_len > max_width:
15781603
if not self.permit_long_summary:
15791604
fail(f"Summary line for {f.full_name!r} is too long!\n"
1580-
f"The summary line must be no longer than {max_width} characters.")
1605+
f"The summary line must be no longer than {max_width} characters.",
1606+
line_number=docstring_line(0))
15811607
else:
15821608
if self.permit_long_summary:
15831609
warn("Remove the @permit_long_summary decorator from "
1584-
f"{f.full_name!r}!\n")
1610+
f"{f.full_name!r}!\n", filename=self.clinic.filename,
1611+
line_number=f.line_number)
15851612

1586-
if max_body > max_width:
1613+
if long_body:
15871614
if not self.permit_long_docstring_body:
15881615
warn(f"Docstring lines for {f.full_name!r} are too long!\n"
1589-
f"Lines should be no longer than {max_width} characters.")
1616+
f"Lines should be no longer than {max_width} characters.",
1617+
filename=self.clinic.filename,
1618+
line_number=docstring_line(long_body[0]))
15901619
else:
15911620
if self.permit_long_docstring_body:
15921621
warn("Remove the @permit_long_docstring_body decorator from "
1593-
f"{f.full_name!r}!\n")
1622+
f"{f.full_name!r}!\n", filename=self.clinic.filename,
1623+
line_number=f.line_number)
15941624

1625+
markers = [i for i, line in enumerate(lines) if '{parameters}' in line]
15951626
parameters_marker_count = len(f.docstring.split('{parameters}')) - 1
15961627
if parameters_marker_count > 1:
1597-
fail('You may not specify {parameters} more than once in a docstring!')
1628+
fail('You may not specify {parameters} more than once in a docstring!',
1629+
line_number=docstring_line(markers[-1]))
15981630

15991631
# insert signature at front and params after the summary line
16001632
if not parameters_marker_count:
@@ -1654,6 +1686,7 @@ def do_post_block_processing_cleanup(self, lineno: int) -> None:
16541686
try:
16551687
self.function.docstring = self.format_docstring()
16561688
except ClinicError as exc:
1657-
exc.lineno = lineno
1689+
if exc.lineno is None:
1690+
exc.lineno = lineno
16581691
exc.filename = self.clinic.filename
16591692
raise

Tools/clinic/libclinic/function.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,10 @@ class Function:
111111
critical_section: bool = False
112112
disable_fastcall: bool = False
113113
target_critical_section: list[str] = dc.field(default_factory=list)
114+
# Line of the file on which the function is declared.
115+
line_number: int | None = None
116+
# Line on which the docstring starts (`None` if there is no docstring).
117+
docstring_line_number: int | None = None
114118

115119
def __post_init__(self) -> None:
116120
self.parent = self.cls or self.module
@@ -213,6 +217,8 @@ class Parameter:
213217
# (`None` signifies that there is no deprecation)
214218
deprecated_positional: VersionTuple | None = None
215219
deprecated_keyword: VersionTuple | None = None
220+
# Line of the file on which the parameter is declared.
221+
line_number: int | None = None
216222
right_bracket_count: int = dc.field(init=False, default=0)
217223

218224
def __repr__(self) -> str:

Tools/clinic/libclinic/parse_args.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,8 @@ def select_prototypes(self) -> None:
329329
self.docstring_definition = GETSET_DOCSTRING_PROTOTYPE_STRVAR
330330
elif self.func.kind is SETTER:
331331
if self.func.docstring:
332-
fail("docstrings are only supported for @getter, not @setter")
332+
fail("docstrings are only supported for @getter, not @setter",
333+
line_number=self.func.line_number)
333334
self.return_value_declaration = "int {return_value};"
334335
self.methoddef_define = SETTERDEF_PROTOTYPE_DEFINE
335336
else:

0 commit comments

Comments
 (0)