diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index cb4507dcac2336..867958884ee033 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 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 1581a19a4fd78a..be25cbdf67354d 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 4dcbc815cc6f25..685e4c20baeed7 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 325633eb010608..1fd8a743e2e04b 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 bca87ecd75100c..20af41430056cc 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: