gh-107570: Argument Clinic: report errors on the offending line (GH-155250)
serhiy-storchaka <[email protected]>
| Newsgroups | gmane.comp.python.cvs |
|---|---|
| Message-ID | <[email protected]> |
https://github.com/python/cpython/commit/9b053955cc7b8751474351e231481671e2f3b366 commit: 9b053955cc7b8751474351e231481671e2f3b366 branch: main author: Serhiy Storchaka <[email protected]> committer: serhiy-storchaka <[email protected]> date: 2026-08-19T11:37:30+03:00 summary: 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. files: A Misc/NEWS.d/next/Tools-Demos/2026-08-05-18-56-17.gh-issue-107570.wXxtw2.rst M Lib/test/test_clinic.py M Tools/clinic/libclinic/clanguage.py M Tools/clinic/libclinic/dsl_parser.py M Tools/clinic/libclinic/function.py M Tools/clinic/libclinic/parse_args.py diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index 94a69b6d7309df..3bf654371b7364 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." @@ -3063,9 +3063,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 = """ @@ -3077,6 +3090,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 00000000000000..c57a5cee1c0d49 --- /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 ab77e7ad6603cd..e586011a92c346 100644 --- a/Tools/clinic/libclinic/clanguage.py +++ b/Tools/clinic/libclinic/clanguage.py @@ -92,7 +92,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) @@ -337,7 +340,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: @@ -462,7 +466,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 a6b1d2bed5e5de..75924dcc05a750 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 @@ -509,6 +512,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: @@ -517,7 +521,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: @@ -666,6 +677,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): @@ -735,7 +748,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) @@ -1141,7 +1155,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:]: @@ -1338,6 +1353,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 @@ -1581,12 +1598,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 @@ -1598,28 +1622,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: @@ -1679,6 +1711,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 cad673045d1c26..58b61c6f822196 100644 --- a/Tools/clinic/libclinic/function.py +++ b/Tools/clinic/libclinic/function.py @@ -118,6 +118,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 @@ -220,6 +224,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 b08b949028205d..37d7cb7ffabe51 100644 --- a/Tools/clinic/libclinic/parse_args.py +++ b/Tools/clinic/libclinic/parse_args.py @@ -346,7 +346,8 @@ def select_prototypes(self) -> None: self.docstring_definition = GETSET_DOCSTRING_PROTOTYPE_STRVAR elif self.func.kind in SETTERS: 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 {parser_retval};" self.methoddef_define = SETTERDEF_PROTOTYPE_DEFINE else: _______________________________________________ Python-checkins mailing list -- [email protected] To unsubscribe send an email to [email protected] https://mail.python.org/mailman3//lists/python-checkins.python.org Member address: [email protected]