From 04da192db5d88384fdfb5778eabd2648f86c1d03 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Wed, 5 Aug 2026 18:56:36 +0300 Subject: [PATCH] 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. --- Lib/test/test_clinic.py | 16 +++++- ...-08-05-18-56-17.gh-issue-107570.wXxtw2.rst | 3 + Tools/clinic/libclinic/clanguage.py | 11 +++- Tools/clinic/libclinic/dsl_parser.py | 57 +++++++++++++++---- Tools/clinic/libclinic/function.py | 6 ++ Tools/clinic/libclinic/parse_args.py | 3 +- 6 files changed, 79 insertions(+), 17 deletions(-) create mode 100644 Misc/NEWS.d/next/Tools-Demos/2026-08-05-18-56-17.gh-issue-107570.wXxtw2.rst diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index cb4507dcac2336d..867958884ee0334 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -347,7 +347,7 @@ def test_ambiguous_group_and_optional_parameters(self): / [clinic start generated code]*/ """ - self.expect_failure(block, err) + self.expect_failure(block, err, lineno=2) def test_star_after_vararg(self): err = "'my_test_func' uses '*' more than once." @@ -2925,9 +2925,22 @@ def test_state_func_docstring_no_summary(self): m.func docstring1 docstring2 + docstring3 """ + # The line which should have been left blank. self.expect_failure(block, err, lineno=3) + def test_state_func_docstring_long_summary(self): + err = "Summary line for 'm.func' is too long!" + block = f""" + module m + m.func + {'x' * 100} + + Body. + """ + self.expect_failure(block, err, lineno=2) + def test_state_func_docstring_only_one_param_template(self): err = "You may not specify {parameters} more than once in a docstring!" block = """ @@ -2939,6 +2952,7 @@ def test_state_func_docstring_only_one_param_template(self): {parameters} these are the params again: {parameters} + and this is the end of the docstring """ self.expect_failure(block, err, lineno=7) diff --git a/Misc/NEWS.d/next/Tools-Demos/2026-08-05-18-56-17.gh-issue-107570.wXxtw2.rst b/Misc/NEWS.d/next/Tools-Demos/2026-08-05-18-56-17.gh-issue-107570.wXxtw2.rst new file mode 100644 index 000000000000000..c57a5cee1c0d49d --- /dev/null +++ b/Misc/NEWS.d/next/Tools-Demos/2026-08-05-18-56-17.gh-issue-107570.wXxtw2.rst @@ -0,0 +1,3 @@ +Argument Clinic: report errors on the offending line. +Errors in a docstring were reported on the line which ends the block, and +errors detected when generating the code were reported without any position. diff --git a/Tools/clinic/libclinic/clanguage.py b/Tools/clinic/libclinic/clanguage.py index 1581a19a4fd78ab..be25cbdf67354df 100644 --- a/Tools/clinic/libclinic/clanguage.py +++ b/Tools/clinic/libclinic/clanguage.py @@ -91,7 +91,10 @@ def render( for o in signatures: if isinstance(o, Function): if function: - fail("You may specify at most one function per block.\nFound a block containing at least two:\n\t" + repr(function) + " and " + repr(o)) + fail("You may specify at most one function per block.\n" + "Found a block containing at least two:\n\t" + + repr(function) + " and " + repr(o), + line_number=o.line_number) function = o return self.render_function(clinic, function) @@ -336,7 +339,8 @@ def render_option_group_parsing( if count in subsets: fail(f"Function {f.full_name!r} has an ambiguous group " f"configuration: a call with {count} argument(s) " - f"can be parsed in more than one way.") + f"can be parsed in more than one way.", + line_number=f.line_number) subsets[count] = subset if limited_capi: @@ -461,7 +465,8 @@ def render_function( if has_option_groups and (not positional): fail("You cannot use optional groups ('[' and ']') " - "unless all parameters are positional-only ('/').") + "unless all parameters are positional-only ('/').", + line_number=f.line_number) # HACK # when we're METH_O, but have a custom return converter, diff --git a/Tools/clinic/libclinic/dsl_parser.py b/Tools/clinic/libclinic/dsl_parser.py index 4dcbc815cc6f25b..685e4c20baeed77 100644 --- a/Tools/clinic/libclinic/dsl_parser.py +++ b/Tools/clinic/libclinic/dsl_parser.py @@ -263,6 +263,8 @@ class DSLParser: critical_section: bool target_critical_section: list[str] disable_fastcall: bool + # Line of the file which is being parsed. + line_number: int | None from_version_re = re.compile(r'([*/]) +\[from +(.+)\]') permit_long_summary = False permit_long_docstring_body = False @@ -286,6 +288,7 @@ def __init__(self, clinic: Clinic) -> None: def reset(self) -> None: self.function = None + self.line_number = None self.state = self.state_dsl_start self.expecting_parameters = True self.keyword_only = False @@ -499,6 +502,7 @@ def parse(self, block: Block) -> None: if '\t' in line: fail(f'Tab characters are illegal in the Clinic DSL: {line!r}', line_number=block_start) + self.line_number = line_number try: self.state(line) except ClinicError as exc: @@ -507,7 +511,14 @@ def parse(self, block: Block) -> None: raise self.do_post_block_processing_cleanup(line_number) - block.output.extend(self.clinic.language.render(self.clinic, block.signatures)) + try: + block.output.extend( + self.clinic.language.render(self.clinic, block.signatures)) + except ClinicError as exc: + if exc.lineno is None: + exc.lineno = line_number + exc.filename = self.clinic.filename + raise if self.preserve_output: if block.output: @@ -656,6 +667,8 @@ def parse_cloned_function(self, names: FunctionNames, existing: str) -> None: "cls": cls, "c_basename": c_basename, "docstring": "", + "docstring_line_number": None, + "line_number": self.line_number, } if not (existing_function.kind is self.kind and existing_function.coexist == self.coexist): @@ -725,7 +738,8 @@ def state_modulename_name(self, line: str) -> None: critical_section=self.critical_section, disable_fastcall=self.disable_fastcall, target_critical_section=self.target_critical_section, - forced_text_signature=self.forced_text_signature + forced_text_signature=self.forced_text_signature, + line_number=self.line_number, ) self.add_function(func) @@ -1116,7 +1130,8 @@ def bad_node(self, node: ast.AST) -> None: converter=converter, default=value, group=self.group_stack[-1] if self.group_stack else 0, group_depth=len(self.group_stack), - deprecated_positional=self.deprecated_positional) + deprecated_positional=self.deprecated_positional, + line_number=self.line_number) names = [k.name for k in self.function.parameters.values()] if parameter_name in names[1:]: @@ -1313,6 +1328,8 @@ def docstring_append(self, obj: Function | Parameter, line: str) -> None: docstring = obj.docstring if docstring: docstring += "\n" + elif isinstance(obj, Function) and line.rstrip(): + obj.docstring_line_number = self.line_number if stripped := line.rstrip(): docstring += self.indent.dedent(stripped) obj.docstring = docstring @@ -1556,12 +1573,19 @@ def format_docstring(self) -> str: # Guido said Clinic should enforce this: # http://mail.python.org/pipermail/python-dev/2013-June/127110.html + def docstring_line(index: int) -> int | None: + """Return the line of the file which holds the index-th line.""" + if f.docstring_line_number is None: + return None + return f.docstring_line_number + index + lines = f.docstring.split('\n') if len(lines) >= 2: if lines[1]: fail(f"Docstring for {f.full_name!r} does not have a summary line!\n" "Every non-blank function docstring must start with " - "a single line summary followed by an empty line.") + "a single line summary followed by an empty line.", + line_number=docstring_line(1)) elif len(lines) == 1: # the docstring is only one line right now--the summary line. # add an empty line after the summary line so we have space @@ -1573,28 +1597,36 @@ def format_docstring(self) -> str: # Existing violations are recorded in OVERLONG_{SUMMARY,BODY}. max_width = f.docstring_line_width summary_len = len(lines[0]) - max_body = max(map(len, lines[1:])) + long_body = [i for i, line in enumerate(lines) + if i and len(line) > max_width] if summary_len > max_width: if not self.permit_long_summary: fail(f"Summary line for {f.full_name!r} is too long!\n" - f"The summary line must be no longer than {max_width} characters.") + f"The summary line must be no longer than {max_width} characters.", + line_number=docstring_line(0)) else: if self.permit_long_summary: warn("Remove the @permit_long_summary decorator from " - f"{f.full_name!r}!\n") + f"{f.full_name!r}!\n", filename=self.clinic.filename, + line_number=f.line_number) - if max_body > max_width: + if long_body: if not self.permit_long_docstring_body: warn(f"Docstring lines for {f.full_name!r} are too long!\n" - f"Lines should be no longer than {max_width} characters.") + f"Lines should be no longer than {max_width} characters.", + filename=self.clinic.filename, + line_number=docstring_line(long_body[0])) else: if self.permit_long_docstring_body: warn("Remove the @permit_long_docstring_body decorator from " - f"{f.full_name!r}!\n") + f"{f.full_name!r}!\n", filename=self.clinic.filename, + line_number=f.line_number) + markers = [i for i, line in enumerate(lines) if '{parameters}' in line] parameters_marker_count = len(f.docstring.split('{parameters}')) - 1 if parameters_marker_count > 1: - fail('You may not specify {parameters} more than once in a docstring!') + fail('You may not specify {parameters} more than once in a docstring!', + line_number=docstring_line(markers[-1])) # insert signature at front and params after the summary line if not parameters_marker_count: @@ -1654,6 +1686,7 @@ def do_post_block_processing_cleanup(self, lineno: int) -> None: try: self.function.docstring = self.format_docstring() except ClinicError as exc: - exc.lineno = lineno + if exc.lineno is None: + exc.lineno = lineno exc.filename = self.clinic.filename raise diff --git a/Tools/clinic/libclinic/function.py b/Tools/clinic/libclinic/function.py index 325633eb010608f..1fd8a743e2e04b8 100644 --- a/Tools/clinic/libclinic/function.py +++ b/Tools/clinic/libclinic/function.py @@ -111,6 +111,10 @@ class Function: critical_section: bool = False disable_fastcall: bool = False target_critical_section: list[str] = dc.field(default_factory=list) + # Line of the file on which the function is declared. + line_number: int | None = None + # Line on which the docstring starts (`None` if there is no docstring). + docstring_line_number: int | None = None def __post_init__(self) -> None: self.parent = self.cls or self.module @@ -213,6 +217,8 @@ class Parameter: # (`None` signifies that there is no deprecation) deprecated_positional: VersionTuple | None = None deprecated_keyword: VersionTuple | None = None + # Line of the file on which the parameter is declared. + line_number: int | None = None right_bracket_count: int = dc.field(init=False, default=0) def __repr__(self) -> str: diff --git a/Tools/clinic/libclinic/parse_args.py b/Tools/clinic/libclinic/parse_args.py index bca87ecd75100ce..20af41430056cc8 100644 --- a/Tools/clinic/libclinic/parse_args.py +++ b/Tools/clinic/libclinic/parse_args.py @@ -329,7 +329,8 @@ def select_prototypes(self) -> None: self.docstring_definition = GETSET_DOCSTRING_PROTOTYPE_STRVAR elif self.func.kind is SETTER: if self.func.docstring: - fail("docstrings are only supported for @getter, not @setter") + fail("docstrings are only supported for @getter, not @setter", + line_number=self.func.line_number) self.return_value_declaration = "int {return_value};" self.methoddef_define = SETTERDEF_PROTOTYPE_DEFINE else: