gh-107614: Normalise Argument Clinic error messages (#107615)

- always wrap the offending line, token, or name in quotes
- in most cases, put the entire error message on one line

Added tests for uncovered branches that were touched by this PR.

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
This commit is contained in:
Erlend E. Aasland 2023-08-04 14:13:10 +02:00 committed by GitHub
parent 2ba7c7f7b1
commit ac7605ed19
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 179 additions and 122 deletions

View File

@ -161,11 +161,8 @@ class ClinicWholeFileTest(TestCase):
[clinic start generated code]*/ [clinic start generated code]*/
/*[clinic end generated code: output=0123456789abcdef input=fedcba9876543210]*/ /*[clinic end generated code: output=0123456789abcdef input=fedcba9876543210]*/
""" """
err = ( err = ("Checksum mismatch! "
'Checksum mismatch!\n' "Expected '0123456789abcdef', computed 'da39a3ee5e6b4b0d'")
'Expected: 0123456789abcdef\n'
'Computed: da39a3ee5e6b4b0d\n'
)
self.expect_failure(raw, err, filename="test.c", lineno=3) self.expect_failure(raw, err, filename="test.c", lineno=3)
def test_garbage_after_stop_line(self): def test_garbage_after_stop_line(self):
@ -403,7 +400,7 @@ class ClinicWholeFileTest(TestCase):
self.expect_failure(block, err, lineno=9) self.expect_failure(block, err, lineno=9)
def test_badly_formed_return_annotation(self): def test_badly_formed_return_annotation(self):
err = "Badly formed annotation for m.f: 'Custom'" err = "Badly formed annotation for 'm.f': 'Custom'"
block = """ block = """
/*[python input] /*[python input]
class Custom_return_converter(CReturnConverter): class Custom_return_converter(CReturnConverter):
@ -509,6 +506,15 @@ class ClinicWholeFileTest(TestCase):
self.assertIn(expected_warning, stdout.getvalue()) self.assertIn(expected_warning, stdout.getvalue())
self.assertEqual(generated, expected_generated) self.assertEqual(generated, expected_generated)
def test_dest_clear(self):
err = "Can't clear destination 'file': it's not of type 'buffer'"
block = """
/*[clinic input]
destination file clear
[clinic start generated code]*/
"""
self.expect_failure(block, err, lineno=2)
def test_directive_set_misuse(self): def test_directive_set_misuse(self):
err = "unknown variable 'ets'" err = "unknown variable 'ets'"
block = """ block = """
@ -598,7 +604,7 @@ class ClinicWholeFileTest(TestCase):
self.assertEqual(generated, expected) self.assertEqual(generated, expected)
def test_directive_preserve_twice(self): def test_directive_preserve_twice(self):
err = "Can't have preserve twice in one block!" err = "Can't have 'preserve' twice in one block!"
block = """ block = """
/*[clinic input] /*[clinic input]
preserve preserve
@ -637,13 +643,24 @@ class ClinicWholeFileTest(TestCase):
self.assertEqual(generated, block) self.assertEqual(generated, block)
def test_directive_output_invalid_command(self): def test_directive_output_invalid_command(self):
err = ( err = dedent("""
"Invalid command / destination name 'cmd', must be one of:\n" Invalid command or destination name 'cmd'. Must be one of:
" preset push pop print everything cpp_if docstring_prototype " - 'preset'
"docstring_definition methoddef_define impl_prototype " - 'push'
"parser_prototype parser_definition cpp_endif methoddef_ifndef " - 'pop'
"impl_definition" - 'print'
) - 'everything'
- 'cpp_if'
- 'docstring_prototype'
- 'docstring_definition'
- 'methoddef_define'
- 'impl_prototype'
- 'parser_prototype'
- 'parser_definition'
- 'cpp_endif'
- 'methoddef_ifndef'
- 'impl_definition'
""").strip()
block = """ block = """
/*[clinic input] /*[clinic input]
output cmd buffer output cmd buffer
@ -919,7 +936,7 @@ class ClinicParserTest(TestCase):
self.assertEqual("MAXSIZE", p.converter.c_default) self.assertEqual("MAXSIZE", p.converter.c_default)
err = ( err = (
"When you specify a named constant ('sys.maxsize') as your default value,\n" "When you specify a named constant ('sys.maxsize') as your default value, "
"you MUST specify a valid c_default." "you MUST specify a valid c_default."
) )
block = """ block = """
@ -931,7 +948,7 @@ class ClinicParserTest(TestCase):
def test_param_default_expr_binop(self): def test_param_default_expr_binop(self):
err = ( err = (
"When you specify an expression ('a + b') as your default value,\n" "When you specify an expression ('a + b') as your default value, "
"you MUST specify a valid c_default." "you MUST specify a valid c_default."
) )
block = """ block = """
@ -954,7 +971,7 @@ class ClinicParserTest(TestCase):
def test_param_default_parameters_out_of_order(self): def test_param_default_parameters_out_of_order(self):
err = ( err = (
"Can't have a parameter without a default ('something_else')\n" "Can't have a parameter without a default ('something_else') "
"after a parameter with a default!" "after a parameter with a default!"
) )
block = """ block = """
@ -1088,7 +1105,7 @@ class ClinicParserTest(TestCase):
module os module os
os.stat -> invalid syntax os.stat -> invalid syntax
""" """
err = "Badly formed annotation for os.stat: 'invalid syntax'" err = "Badly formed annotation for 'os.stat': 'invalid syntax'"
self.expect_failure(block, err) self.expect_failure(block, err)
def test_legacy_converter_disallowed_in_return_annotation(self): def test_legacy_converter_disallowed_in_return_annotation(self):
@ -1252,8 +1269,8 @@ class ClinicParserTest(TestCase):
def test_disallowed_grouping__two_top_groups_on_left(self): def test_disallowed_grouping__two_top_groups_on_left(self):
err = ( err = (
'Function two_top_groups_on_left has an unsupported group ' "Function 'two_top_groups_on_left' has an unsupported group "
'configuration. (Unexpected state 2.b)' "configuration. (Unexpected state 2.b)"
) )
block = """ block = """
module foo module foo
@ -1281,7 +1298,7 @@ class ClinicParserTest(TestCase):
] ]
""" """
err = ( err = (
"Function two_top_groups_on_right has an unsupported group " "Function 'two_top_groups_on_right' has an unsupported group "
"configuration. (Unexpected state 6.b)" "configuration. (Unexpected state 6.b)"
) )
self.expect_failure(block, err) self.expect_failure(block, err)
@ -1317,7 +1334,7 @@ class ClinicParserTest(TestCase):
param: int param: int
""" """
err = ( err = (
"Function group_after_parameter_on_left has an unsupported group " "Function 'group_after_parameter_on_left' has an unsupported group "
"configuration. (Unexpected state 2.b)" "configuration. (Unexpected state 2.b)"
) )
self.expect_failure(block, err) self.expect_failure(block, err)
@ -1334,7 +1351,7 @@ class ClinicParserTest(TestCase):
param: int param: int
""" """
err = ( err = (
"Function empty_group has an empty group.\n" "Function 'empty_group' has an empty group. "
"All groups must contain at least one parameter." "All groups must contain at least one parameter."
) )
self.expect_failure(block, err) self.expect_failure(block, err)
@ -1351,7 +1368,7 @@ class ClinicParserTest(TestCase):
] ]
""" """
err = ( err = (
"Function empty_group has an empty group.\n" "Function 'empty_group' has an empty group. "
"All groups must contain at least one parameter." "All groups must contain at least one parameter."
) )
self.expect_failure(block, err) self.expect_failure(block, err)
@ -1365,7 +1382,7 @@ class ClinicParserTest(TestCase):
group2: int group2: int
] ]
""" """
err = "Function empty_group has a ] without a matching [." err = "Function 'empty_group' has a ']' without a matching '['"
self.expect_failure(block, err) self.expect_failure(block, err)
def test_no_parameters(self): def test_no_parameters(self):
@ -1400,7 +1417,7 @@ class ClinicParserTest(TestCase):
foo.bar => int foo.bar => int
/ /
""" """
err = "Illegal function name: foo.bar => int" err = "Illegal function name: 'foo.bar => int'"
self.expect_failure(block, err) self.expect_failure(block, err)
def test_illegal_c_basename(self): def test_illegal_c_basename(self):
@ -1409,7 +1426,7 @@ class ClinicParserTest(TestCase):
foo.bar as 935 foo.bar as 935
/ /
""" """
err = "Illegal C basename: 935" err = "Illegal C basename: '935'"
self.expect_failure(block, err) self.expect_failure(block, err)
def test_single_star(self): def test_single_star(self):
@ -1419,7 +1436,7 @@ class ClinicParserTest(TestCase):
* *
* *
""" """
err = "Function bar uses '*' more than once." err = "Function 'bar' uses '*' more than once."
self.expect_failure(block, err) self.expect_failure(block, err)
def test_parameters_required_after_star(self): def test_parameters_required_after_star(self):
@ -1429,7 +1446,7 @@ class ClinicParserTest(TestCase):
"module foo\nfoo.bar\n this: int\n *", "module foo\nfoo.bar\n this: int\n *",
"module foo\nfoo.bar\n this: int\n *\nDocstring.", "module foo\nfoo.bar\n this: int\n *\nDocstring.",
) )
err = "Function bar specifies '*' without any parameters afterwards." err = "Function 'bar' specifies '*' without any parameters afterwards."
for block in dataset: for block in dataset:
with self.subTest(block=block): with self.subTest(block=block):
self.expect_failure(block, err) self.expect_failure(block, err)
@ -1442,7 +1459,7 @@ class ClinicParserTest(TestCase):
/ /
""" """
err = ( err = (
"Function bar has an unsupported group configuration. " "Function 'bar' has an unsupported group configuration. "
"(Unexpected state 0.d)" "(Unexpected state 0.d)"
) )
self.expect_failure(block, err) self.expect_failure(block, err)
@ -1456,7 +1473,7 @@ class ClinicParserTest(TestCase):
b: int b: int
/ /
""" """
err = "Function bar uses '/' more than once." err = "Function 'bar' uses '/' more than once."
self.expect_failure(block, err) self.expect_failure(block, err)
def test_mix_star_and_slash(self): def test_mix_star_and_slash(self):
@ -1470,7 +1487,7 @@ class ClinicParserTest(TestCase):
/ /
""" """
err = ( err = (
"Function bar mixes keyword-only and positional-only parameters, " "Function 'bar' mixes keyword-only and positional-only parameters, "
"which is unsupported." "which is unsupported."
) )
self.expect_failure(block, err) self.expect_failure(block, err)
@ -1483,7 +1500,7 @@ class ClinicParserTest(TestCase):
x: int x: int
""" """
err = ( err = (
"Function bar has an unsupported group configuration. " "Function 'bar' has an unsupported group configuration. "
"(Unexpected state 0.d)" "(Unexpected state 0.d)"
) )
self.expect_failure(block, err) self.expect_failure(block, err)
@ -1582,7 +1599,8 @@ class ClinicParserTest(TestCase):
*vararg1: object *vararg1: object
\t*vararg2: object \t*vararg2: object
""" """
err = "Tab characters are illegal in the Clinic DSL." err = ("Tab characters are illegal in the Clinic DSL: "
r"'\t*vararg2: object'")
self.expect_failure(block, err) self.expect_failure(block, err)
def test_indent_stack_illegal_outdent(self): def test_indent_stack_illegal_outdent(self):
@ -1841,8 +1859,17 @@ class ClinicParserTest(TestCase):
""" """
self.expect_failure(block, err, lineno=1) self.expect_failure(block, err, lineno=1)
def test_default_is_not_of_correct_type(self):
err = ("int_converter: default value 2.5 for field 'a' "
"is not of type 'int'")
block = """
fn
a: int = 2.5
"""
self.expect_failure(block, err, lineno=1)
def test_invalid_legacy_converter(self): def test_invalid_legacy_converter(self):
err = "fhi is not a valid legacy converter" err = "'fhi' is not a valid legacy converter"
block = """ block = """
fn fn
a: 'fhi' a: 'fhi'
@ -1850,7 +1877,7 @@ class ClinicParserTest(TestCase):
self.expect_failure(block, err, lineno=1) self.expect_failure(block, err, lineno=1)
def test_parent_class_or_module_does_not_exist(self): def test_parent_class_or_module_does_not_exist(self):
err = "Parent class or module z does not exist" err = "Parent class or module 'z' does not exist"
block = """ block = """
module m module m
z.func z.func
@ -1877,7 +1904,7 @@ class ClinicParserTest(TestCase):
self.expect_failure(block, err, lineno=2) self.expect_failure(block, err, lineno=2)
def test_state_func_docstring_assert_no_group(self): def test_state_func_docstring_assert_no_group(self):
err = "Function func has a ] without a matching [." err = "Function 'func' has a ']' without a matching '['"
block = """ block = """
module m module m
m.func m.func
@ -1887,7 +1914,7 @@ class ClinicParserTest(TestCase):
self.expect_failure(block, err, lineno=2) self.expect_failure(block, err, lineno=2)
def test_state_func_docstring_no_summary(self): def test_state_func_docstring_no_summary(self):
err = "Docstring for m.func does not have a summary line!" err = "Docstring for 'm.func' does not have a summary line!"
block = """ block = """
module m module m
m.func m.func
@ -1983,13 +2010,11 @@ class ClinicExternalTest(TestCase):
const char *hand_edited = "output block is overwritten"; const char *hand_edited = "output block is overwritten";
/*[clinic end generated code: output=bogus input=bogus]*/ /*[clinic end generated code: output=bogus input=bogus]*/
""") """)
fail_msg = dedent(""" fail_msg = (
Checksum mismatch! "Checksum mismatch! Expected 'bogus', computed '2ed19'. "
Expected: bogus "Suggested fix: remove all generated code including the end marker, "
Computed: 2ed19 "or use the '-f' option.\n"
Suggested fix: remove all generated code including the end marker, )
or use the '-f' option.
""")
with os_helper.temp_dir() as tmp_dir: with os_helper.temp_dir() as tmp_dir:
fn = os.path.join(tmp_dir, "test.c") fn = os.path.join(tmp_dir, "test.c")
with open(fn, "w", encoding="utf-8") as f: with open(fn, "w", encoding="utf-8") as f:
@ -2214,7 +2239,7 @@ class ClinicExternalTest(TestCase):
f.write("") # Write an empty output file! f.write("") # Write an empty output file!
# Clinic should complain about the empty output file. # Clinic should complain about the empty output file.
_, err = self.expect_failure(in_fn) _, err = self.expect_failure(in_fn)
expected_err = (f"Modified destination file {out_fn!r}, " expected_err = (f"Modified destination file {out_fn!r}; "
"not overwriting!") "not overwriting!")
self.assertIn(expected_err, err) self.assertIn(expected_err, err)
# Run clinic again, this time with the -f option. # Run clinic again, this time with the -f option.

View File

@ -290,9 +290,11 @@ def linear_format(s: str, **kwargs: str) -> str:
continue continue
if trailing: if trailing:
fail("Text found after {" + name + "} block marker! It must be on a line by itself.") fail(f"Text found after {{{name}}} block marker! "
"It must be on a line by itself.")
if indent.strip(): if indent.strip():
fail("Non-whitespace characters found before {" + name + "} block marker! It must be on a line by itself.") fail(f"Non-whitespace characters found before {{{name}}} block marker! "
"It must be on a line by itself.")
value = kwargs[name] value = kwargs[name]
if not value: if not value:
@ -1534,7 +1536,8 @@ class CLanguage(Language):
c.render(p, data) c.render(p, data)
if has_option_groups and (not positional): if has_option_groups and (not positional):
fail("You cannot use optional groups ('[' and ']')\nunless all parameters are positional-only ('/').") fail("You cannot use optional groups ('[' and ']') "
"unless all parameters are positional-only ('/').")
# HACK # HACK
# when we're METH_O, but have a custom return converter, # when we're METH_O, but have a custom return converter,
@ -1871,7 +1874,7 @@ class BlockParser:
for field in shlex.split(arguments): for field in shlex.split(arguments):
name, equals, value = field.partition('=') name, equals, value = field.partition('=')
if not equals: if not equals:
fail("Mangled Argument Clinic marker line:", repr(line)) fail(f"Mangled Argument Clinic marker line: {line!r}")
d[name.strip()] = value.strip() d[name.strip()] = value.strip()
if self.verify: if self.verify:
@ -1882,11 +1885,10 @@ class BlockParser:
computed = compute_checksum(output, len(checksum)) computed = compute_checksum(output, len(checksum))
if checksum != computed: if checksum != computed:
fail("Checksum mismatch!\nExpected: {}\nComputed: {}\n" fail("Checksum mismatch! "
f"Expected {checksum!r}, computed {computed!r}. "
"Suggested fix: remove all generated code including " "Suggested fix: remove all generated code including "
"the end marker,\n" "the end marker, or use the '-f' option.")
"or use the '-f' option."
.format(checksum, computed))
else: else:
# put back output # put back output
output_lines = output.splitlines(keepends=True) output_lines = output.splitlines(keepends=True)
@ -2017,9 +2019,10 @@ class Destination:
) )
extra_arguments = 1 if self.type == "file" else 0 extra_arguments = 1 if self.type == "file" else 0
if len(args) < extra_arguments: if len(args) < extra_arguments:
fail(f"Not enough arguments for destination {self.name} new {self.type}") fail(f"Not enough arguments for destination "
f"{self.name!r} new {self.type!r}")
if len(args) > extra_arguments: if len(args) > extra_arguments:
fail(f"Too many arguments for destination {self.name} new {self.type}") fail(f"Too many arguments for destination {self.name!r} new {self.type!r}")
if self.type =='file': if self.type =='file':
d = {} d = {}
filename = self.clinic.filename filename = self.clinic.filename
@ -2041,7 +2044,7 @@ class Destination:
def clear(self) -> None: def clear(self) -> None:
if self.type != 'buffer': if self.type != 'buffer':
fail("Can't clear destination" + self.name + " , it's not of type buffer") fail(f"Can't clear destination {self.name!r}: it's not of type 'buffer'")
self.buffers.clear() self.buffers.clear()
def dump(self) -> str: def dump(self) -> str:
@ -2220,13 +2223,13 @@ impl_definition block
*args: str *args: str
) -> None: ) -> None:
if name in self.destinations: if name in self.destinations:
fail("Destination already exists: " + repr(name)) fail(f"Destination already exists: {name!r}")
self.destinations[name] = Destination(name, type, self, args) self.destinations[name] = Destination(name, type, self, args)
def get_destination(self, name: str) -> Destination: def get_destination(self, name: str) -> Destination:
d = self.destinations.get(name) d = self.destinations.get(name)
if not d: if not d:
fail("Destination does not exist: " + repr(name)) fail(f"Destination does not exist: {name!r}")
return d return d
def get_destination_buffer( def get_destination_buffer(
@ -2273,15 +2276,16 @@ impl_definition block
os.makedirs(dirname) os.makedirs(dirname)
except FileExistsError: except FileExistsError:
if not os.path.isdir(dirname): if not os.path.isdir(dirname):
fail("Can't write to destination {}, " fail(f"Can't write to destination "
"can't make directory {}!".format( f"{destination.filename!r}; "
destination.filename, dirname)) f"can't make directory {dirname!r}!")
if self.verify: if self.verify:
with open(destination.filename) as f: with open(destination.filename) as f:
parser_2 = BlockParser(f.read(), language=self.language) parser_2 = BlockParser(f.read(), language=self.language)
blocks = list(parser_2) blocks = list(parser_2)
if (len(blocks) != 1) or (blocks[0].input != 'preserve\n'): if (len(blocks) != 1) or (blocks[0].input != 'preserve\n'):
fail("Modified destination file " + repr(destination.filename) + ", not overwriting!") fail(f"Modified destination file "
f"{destination.filename!r}; not overwriting!")
except FileNotFoundError: except FileNotFoundError:
pass pass
@ -2322,7 +2326,8 @@ impl_definition block
return module, cls return module, cls
child = parent.classes.get(field) child = parent.classes.get(field)
if not child: if not child:
fail('Parent class or module ' + '.'.join(so_far) + " does not exist.") fullname = ".".join(so_far)
fail(f"Parent class or module {fullname!r} does not exist.")
cls = parent = child cls = parent = child
return module, cls return module, cls
@ -2339,12 +2344,12 @@ def parse_file(
extension = os.path.splitext(filename)[1][1:] extension = os.path.splitext(filename)[1][1:]
if not extension: if not extension:
fail("Can't extract file type for file " + repr(filename)) fail(f"Can't extract file type for file {filename!r}")
try: try:
language = extensions[extension](filename) language = extensions[extension](filename)
except KeyError: except KeyError:
fail("Can't identify file type for file " + repr(filename)) fail(f"Can't identify file type for file {filename!r}")
with open(filename, encoding="utf-8") as f: with open(filename, encoding="utf-8") as f:
raw = f.read() raw = f.read()
@ -2823,8 +2828,9 @@ class CConverter(metaclass=CConverterAutoRegister):
else: else:
names = [cls.__name__ for cls in self.default_type] names = [cls.__name__ for cls in self.default_type]
types_str = ', '.join(names) types_str = ', '.join(names)
fail("{}: default value {!r} for field {} is not of type {}".format( cls_name = self.__class__.__name__
self.__class__.__name__, default, name, types_str)) fail(f"{cls_name}: default value {default!r} for field "
f"{name!r} is not of type {types_str!r}")
self.default = default self.default = default
if c_default: if c_default:
@ -3146,7 +3152,7 @@ class bool_converter(CConverter):
if accept == {int}: if accept == {int}:
self.format_unit = 'i' self.format_unit = 'i'
elif accept != {object}: elif accept != {object}:
fail("bool_converter: illegal 'accept' argument " + repr(accept)) fail(f"bool_converter: illegal 'accept' argument {accept!r}")
if self.default is not unspecified: if self.default is not unspecified:
self.default = bool(self.default) self.default = bool(self.default)
self.c_default = str(int(self.default)) self.c_default = str(int(self.default))
@ -3196,7 +3202,7 @@ class char_converter(CConverter):
def converter_init(self) -> None: def converter_init(self) -> None:
if isinstance(self.default, self.default_type): if isinstance(self.default, self.default_type):
if len(self.default) != 1: if len(self.default) != 1:
fail("char_converter: illegal default value " + repr(self.default)) fail(f"char_converter: illegal default value {self.default!r}")
self.c_default = repr(bytes(self.default))[1:] self.c_default = repr(bytes(self.default))[1:]
if self.c_default == '"\'"': if self.c_default == '"\'"':
@ -3335,7 +3341,7 @@ class int_converter(CConverter):
if accept == {str}: if accept == {str}:
self.format_unit = 'C' self.format_unit = 'C'
elif accept != {int}: elif accept != {int}:
fail("int_converter: illegal 'accept' argument " + repr(accept)) fail(f"int_converter: illegal 'accept' argument {accept!r}")
if type is not None: if type is not None:
self.type = type self.type = type
@ -3472,7 +3478,7 @@ class Py_ssize_t_converter(CConverter):
elif accept == {int, NoneType}: elif accept == {int, NoneType}:
self.converter = '_Py_convert_optional_to_ssize_t' self.converter = '_Py_convert_optional_to_ssize_t'
else: else:
fail("Py_ssize_t_converter: illegal 'accept' argument " + repr(accept)) fail(f"Py_ssize_t_converter: illegal 'accept' argument {accept!r}")
def parse_arg(self, argname: str, displayname: str) -> str | None: def parse_arg(self, argname: str, displayname: str) -> str | None:
if self.format_unit == 'n': if self.format_unit == 'n':
@ -3502,7 +3508,7 @@ class slice_index_converter(CConverter):
elif accept == {int, NoneType}: elif accept == {int, NoneType}:
self.converter = '_PyEval_SliceIndex' self.converter = '_PyEval_SliceIndex'
else: else:
fail("slice_index_converter: illegal 'accept' argument " + repr(accept)) fail(f"slice_index_converter: illegal 'accept' argument {accept!r}")
class size_t_converter(CConverter): class size_t_converter(CConverter):
type = 'size_t' type = 'size_t'
@ -3850,7 +3856,7 @@ class Py_UNICODE_converter(CConverter):
elif accept == {str, NoneType}: elif accept == {str, NoneType}:
self.converter = '_PyUnicode_WideCharString_Opt_Converter' self.converter = '_PyUnicode_WideCharString_Opt_Converter'
else: else:
fail("Py_UNICODE_converter: illegal 'accept' argument " + repr(accept)) fail(f"Py_UNICODE_converter: illegal 'accept' argument {accept!r}")
self.c_default = "NULL" self.c_default = "NULL"
def cleanup(self) -> str: def cleanup(self) -> str:
@ -4378,7 +4384,7 @@ class IndentStack:
margin = self.margin margin = self.margin
indent = self.indents[-1] indent = self.indents[-1]
if not line.startswith(margin): if not line.startswith(margin):
fail('Cannot dedent, line does not start with the previous margin:') fail('Cannot dedent; line does not start with the previous margin.')
return line[indent:] return line[indent:]
@ -4464,7 +4470,9 @@ class DSLParser:
def directive_version(self, required: str) -> None: def directive_version(self, required: str) -> None:
global version global version
if version_comparitor(version, required) < 0: if version_comparitor(version, required) < 0:
fail("Insufficient Clinic version!\n Version: " + version + "\n Required: " + required) fail("Insufficient Clinic version!\n"
f" Version: {version}\n"
f" Required: {required}")
def directive_module(self, name: str) -> None: def directive_module(self, name: str) -> None:
fields = name.split('.')[:-1] fields = name.split('.')[:-1]
@ -4473,7 +4481,7 @@ class DSLParser:
fail("Can't nest a module inside a class!") fail("Can't nest a module inside a class!")
if name in module.modules: if name in module.modules:
fail("Already defined module " + repr(name) + "!") fail(f"Already defined module {name!r}!")
m = Module(name, module) m = Module(name, module)
module.modules[name] = m module.modules[name] = m
@ -4491,7 +4499,7 @@ class DSLParser:
parent = cls or module parent = cls or module
if name in parent.classes: if name in parent.classes:
fail("Already defined class " + repr(name) + "!") fail(f"Already defined class {name!r}!")
c = Class(name, module, cls, typedef, type_object) c = Class(name, module, cls, typedef, type_object)
parent.classes[name] = c parent.classes[name] = c
@ -4499,7 +4507,7 @@ class DSLParser:
def directive_set(self, name: str, value: str) -> None: def directive_set(self, name: str, value: str) -> None:
if name not in ("line_prefix", "line_suffix"): if name not in ("line_prefix", "line_suffix"):
fail("unknown variable", repr(name)) fail(f"unknown variable {name!r}")
value = value.format_map({ value = value.format_map({
'block comment start': '/*', 'block comment start': '/*',
@ -4520,7 +4528,7 @@ class DSLParser:
case "clear": case "clear":
self.clinic.get_destination(name).clear() self.clinic.get_destination(name).clear()
case _: case _:
fail("unknown destination command", repr(command)) fail(f"unknown destination command {command!r}")
def directive_output( def directive_output(
@ -4533,7 +4541,7 @@ class DSLParser:
if command_or_name == "preset": if command_or_name == "preset":
preset = self.clinic.presets.get(destination) preset = self.clinic.presets.get(destination)
if not preset: if not preset:
fail("Unknown preset " + repr(destination) + "!") fail(f"Unknown preset {destination!r}!")
fd.update(preset) fd.update(preset)
return return
@ -4562,7 +4570,11 @@ class DSLParser:
return return
if command_or_name not in fd: if command_or_name not in fd:
fail("Invalid command / destination name " + repr(command_or_name) + ", must be one of:\n preset push pop print everything " + " ".join(fd)) allowed = ["preset", "push", "pop", "print", "everything"]
allowed.extend(fd)
fail(f"Invalid command or destination name {command_or_name!r}. "
"Must be one of:\n -",
"\n - ".join([repr(word) for word in allowed]))
fd[command_or_name] = d fd[command_or_name] = d
def directive_dump(self, name: str) -> None: def directive_dump(self, name: str) -> None:
@ -4574,7 +4586,7 @@ class DSLParser:
def directive_preserve(self) -> None: def directive_preserve(self) -> None:
if self.preserve_output: if self.preserve_output:
fail("Can't have preserve twice in one block!") fail("Can't have 'preserve' twice in one block!")
self.preserve_output = True self.preserve_output = True
def at_classmethod(self) -> None: def at_classmethod(self) -> None:
@ -4601,7 +4613,8 @@ class DSLParser:
lines = block.input.split('\n') lines = block.input.split('\n')
for line_number, line in enumerate(lines, self.clinic.block_parser.block_start_line_number): for line_number, line in enumerate(lines, self.clinic.block_parser.block_start_line_number):
if '\t' in line: if '\t' in line:
fail('Tab characters are illegal in the Clinic DSL.\n\t' + repr(line), line_number=block_start) fail(f'Tab characters are illegal in the Clinic DSL: {line!r}',
line_number=block_start)
try: try:
self.state(line) self.state(line)
except ClinicError as exc: except ClinicError as exc:
@ -4712,7 +4725,8 @@ class DSLParser:
module, cls = self.clinic._module_and_class(fields) module, cls = self.clinic._module_and_class(fields)
if not (existing_function.kind is self.kind and existing_function.coexist == self.coexist): if not (existing_function.kind is self.kind and existing_function.coexist == self.coexist):
fail("'kind' of function and cloned function don't match! (@classmethod/@staticmethod/@coexist)") fail("'kind' of function and cloned function don't match! "
"(@classmethod/@staticmethod/@coexist)")
function = existing_function.copy( function = existing_function.copy(
name=function_name, full_name=full_name, module=module, name=function_name, full_name=full_name, module=module,
cls=cls, c_basename=c_basename, docstring='' cls=cls, c_basename=c_basename, docstring=''
@ -4731,9 +4745,9 @@ class DSLParser:
c_basename = c_basename.strip() or None c_basename = c_basename.strip() or None
if not is_legal_py_identifier(full_name): if not is_legal_py_identifier(full_name):
fail("Illegal function name:", full_name) fail(f"Illegal function name: {full_name!r}")
if c_basename and not is_legal_c_identifier(c_basename): if c_basename and not is_legal_c_identifier(c_basename):
fail("Illegal C basename:", c_basename) fail(f"Illegal C basename: {c_basename!r}")
return_converter = None return_converter = None
if returns: if returns:
@ -4741,7 +4755,7 @@ class DSLParser:
try: try:
module_node = ast.parse(ast_input) module_node = ast.parse(ast_input)
except SyntaxError: except SyntaxError:
fail(f"Badly formed annotation for {full_name}: {returns!r}") fail(f"Badly formed annotation for {full_name!r}: {returns!r}")
function_node = module_node.body[0] function_node = module_node.body[0]
assert isinstance(function_node, ast.FunctionDef) assert isinstance(function_node, ast.FunctionDef)
try: try:
@ -4752,7 +4766,7 @@ class DSLParser:
fail(f"No available return converter called {name!r}") fail(f"No available return converter called {name!r}")
return_converter = return_converters[name](**kwargs) return_converter = return_converters[name](**kwargs)
except ValueError: except ValueError:
fail(f"Badly formed annotation for {full_name}: {returns!r}") fail(f"Badly formed annotation for {full_name!r}: {returns!r}")
fields = [x.strip() for x in full_name.split('.')] fields = [x.strip() for x in full_name.split('.')]
function_name = fields.pop() function_name = fields.pop()
@ -4777,7 +4791,7 @@ class DSLParser:
return_converter = CReturnConverter() return_converter = CReturnConverter()
if not module: if not module:
fail("Undefined module used in declaration of " + repr(full_name.strip()) + ".") fail("Undefined module used in declaration of {full_name.strip()!r}.")
self.function = Function(name=function_name, full_name=full_name, module=module, cls=cls, c_basename=c_basename, self.function = Function(name=function_name, full_name=full_name, module=module, cls=cls, c_basename=c_basename,
return_converter=return_converter, kind=self.kind, coexist=self.coexist) return_converter=return_converter, kind=self.kind, coexist=self.coexist)
self.block.signatures.append(self.function) self.block.signatures.append(self.function)
@ -4963,18 +4977,22 @@ class DSLParser:
except SyntaxError: except SyntaxError:
pass pass
if not module: if not module:
fail("Function " + self.function.name + " has an invalid parameter declaration:\n\t" + line) fail(f"Function {self.function.name!r} has an invalid parameter declaration:\n\t",
repr(line))
function = module.body[0] function = module.body[0]
assert isinstance(function, ast.FunctionDef) assert isinstance(function, ast.FunctionDef)
function_args = function.args function_args = function.args
if len(function_args.args) > 1: if len(function_args.args) > 1:
fail("Function " + self.function.name + " has an invalid parameter declaration (comma?):\n\t" + line) fail(f"Function {self.function.name!r} has an "
f"invalid parameter declaration (comma?): {line!r}")
if function_args.defaults or function_args.kw_defaults: if function_args.defaults or function_args.kw_defaults:
fail("Function " + self.function.name + " has an invalid parameter declaration (default value?):\n\t" + line) fail(f"Function {self.function.name!r} has an "
f"invalid parameter declaration (default value?): {line!r}")
if function_args.kwarg: if function_args.kwarg:
fail("Function " + self.function.name + " has an invalid parameter declaration (**kwargs?):\n\t" + line) fail(f"Function {self.function.name!r} has an "
f"invalid parameter declaration (**kwargs?): {line!r}")
if function_args.vararg: if function_args.vararg:
is_vararg = True is_vararg = True
@ -4988,7 +5006,7 @@ class DSLParser:
if not default: if not default:
if self.parameter_state is ParamState.OPTIONAL: if self.parameter_state is ParamState.OPTIONAL:
fail(f"Can't have a parameter without a default ({parameter_name!r})\n" fail(f"Can't have a parameter without a default ({parameter_name!r}) "
"after a parameter with a default!") "after a parameter with a default!")
value: Sentinels | Null value: Sentinels | Null
if is_vararg: if is_vararg:
@ -5048,10 +5066,10 @@ class DSLParser:
except NameError: except NameError:
pass # probably a named constant pass # probably a named constant
except Exception as e: except Exception as e:
fail("Malformed expression given as default value\n" fail("Malformed expression given as default value "
"{!r} caused {!r}".format(default, e)) f"{default!r} caused {e!r}")
if bad: if bad:
fail("Unsupported expression as default value: " + repr(default)) fail(f"Unsupported expression as default value: {default!r}")
assignment = module.body[0] assignment = module.body[0]
assert isinstance(assignment, ast.Assign) assert isinstance(assignment, ast.Assign)
@ -5069,7 +5087,10 @@ class DSLParser:
)): )):
c_default = kwargs.get("c_default") c_default = kwargs.get("c_default")
if not (isinstance(c_default, str) and c_default): if not (isinstance(c_default, str) and c_default):
fail("When you specify an expression (" + repr(default) + ") as your default value,\nyou MUST specify a valid c_default." + ast.dump(expr)) fail(f"When you specify an expression ({default!r}) "
f"as your default value, "
f"you MUST specify a valid c_default.",
ast.dump(expr))
py_default = default py_default = default
value = unknown value = unknown
elif isinstance(expr, ast.Attribute): elif isinstance(expr, ast.Attribute):
@ -5079,13 +5100,16 @@ class DSLParser:
a.append(n.attr) a.append(n.attr)
n = n.value n = n.value
if not isinstance(n, ast.Name): if not isinstance(n, ast.Name):
fail("Unsupported default value " + repr(default) + " (looked like a Python constant)") fail(f"Unsupported default value {default!r} "
"(looked like a Python constant)")
a.append(n.id) a.append(n.id)
py_default = ".".join(reversed(a)) py_default = ".".join(reversed(a))
c_default = kwargs.get("c_default") c_default = kwargs.get("c_default")
if not (isinstance(c_default, str) and c_default): if not (isinstance(c_default, str) and c_default):
fail("When you specify a named constant (" + repr(py_default) + ") as your default value,\nyou MUST specify a valid c_default.") fail(f"When you specify a named constant ({py_default!r}) "
"as your default value, "
"you MUST specify a valid c_default.")
try: try:
value = eval(py_default) value = eval(py_default)
@ -5102,13 +5126,15 @@ class DSLParser:
c_default = py_default c_default = py_default
except SyntaxError as e: except SyntaxError as e:
fail("Syntax error: " + repr(e.text)) fail(f"Syntax error: {e.text!r}")
except (ValueError, AttributeError): except (ValueError, AttributeError):
value = unknown value = unknown
c_default = kwargs.get("c_default") c_default = kwargs.get("c_default")
py_default = default py_default = default
if not (isinstance(c_default, str) and c_default): if not (isinstance(c_default, str) and c_default):
fail("When you specify a named constant (" + repr(py_default) + ") as your default value,\nyou MUST specify a valid c_default.") fail("When you specify a named constant "
f"({py_default!r}) as your default value, "
"you MUST specify a valid c_default.")
kwargs.setdefault('c_default', c_default) kwargs.setdefault('c_default', c_default)
kwargs.setdefault('py_default', py_default) kwargs.setdefault('py_default', py_default)
@ -5116,7 +5142,7 @@ class DSLParser:
dict = legacy_converters if legacy else converters dict = legacy_converters if legacy else converters
legacy_str = "legacy " if legacy else "" legacy_str = "legacy " if legacy else ""
if name not in dict: if name not in dict:
fail(f'{name} is not a valid {legacy_str}converter') fail(f'{name!r} is not a valid {legacy_str}converter')
# if you use a c_name for the parameter, we just give that name to the converter # if you use a c_name for the parameter, we just give that name to the converter
# but the parameter object gets the python name # but the parameter object gets the python name
converter = dict[name](c_name or parameter_name, parameter_name, self.function, value, **kwargs) converter = dict[name](c_name or parameter_name, parameter_name, self.function, value, **kwargs)
@ -5141,7 +5167,8 @@ class DSLParser:
self.parameter_state = ParamState.START self.parameter_state = ParamState.START
self.function.parameters.clear() self.function.parameters.clear()
else: else:
fail("A 'self' parameter, if specified, must be the very first thing in the parameter block.") fail("A 'self' parameter, if specified, must be the "
"very first thing in the parameter block.")
if isinstance(converter, defining_class_converter): if isinstance(converter, defining_class_converter):
_lp = len(self.function.parameters) _lp = len(self.function.parameters)
@ -5153,16 +5180,18 @@ class DSLParser:
if self.group: if self.group:
fail("A 'defining_class' parameter cannot be in an optional group.") fail("A 'defining_class' parameter cannot be in an optional group.")
else: else:
fail("A 'defining_class' parameter, if specified, must either be the first thing in the parameter block, or come just after 'self'.") fail("A 'defining_class' parameter, if specified, must either "
"be the first thing in the parameter block, or come just "
"after 'self'.")
p = Parameter(parameter_name, kind, function=self.function, converter=converter, default=value, group=self.group) p = Parameter(parameter_name, kind, function=self.function, converter=converter, default=value, group=self.group)
names = [k.name for k in self.function.parameters.values()] names = [k.name for k in self.function.parameters.values()]
if parameter_name in names[1:]: if parameter_name in names[1:]:
fail("You can't have two parameters named " + repr(parameter_name) + "!") fail(f"You can't have two parameters named {parameter_name!r}!")
elif names and parameter_name == names[0] and c_name is None: elif names and parameter_name == names[0] and c_name is None:
fail(f"Parameter '{parameter_name}' requires a custom C name") fail(f"Parameter {parameter_name!r} requires a custom C name")
key = f"{parameter_name}_as_{c_name}" if c_name else parameter_name key = f"{parameter_name}_as_{c_name}" if c_name else parameter_name
self.function.parameters[key] = p self.function.parameters[key] = p
@ -5192,7 +5221,7 @@ class DSLParser:
def parse_star(self, function: Function) -> None: def parse_star(self, function: Function) -> None:
"""Parse keyword-only parameter marker '*'.""" """Parse keyword-only parameter marker '*'."""
if self.keyword_only: if self.keyword_only:
fail(f"Function {function.name} uses '*' more than once.") fail(f"Function {function.name!r} uses '*' more than once.")
self.keyword_only = True self.keyword_only = True
def parse_opening_square_bracket(self, function: Function) -> None: def parse_opening_square_bracket(self, function: Function) -> None:
@ -5203,7 +5232,8 @@ class DSLParser:
case ParamState.REQUIRED | ParamState.GROUP_AFTER: case ParamState.REQUIRED | ParamState.GROUP_AFTER:
self.parameter_state = ParamState.GROUP_AFTER self.parameter_state = ParamState.GROUP_AFTER
case st: case st:
fail(f"Function {function.name} has an unsupported group configuration. " fail(f"Function {function.name!r} "
f"has an unsupported group configuration. "
f"(Unexpected state {st}.b)") f"(Unexpected state {st}.b)")
self.group += 1 self.group += 1
function.docstring_only = True function.docstring_only = True
@ -5211,9 +5241,9 @@ class DSLParser:
def parse_closing_square_bracket(self, function: Function) -> None: def parse_closing_square_bracket(self, function: Function) -> None:
"""Parse closing parameter group symbol ']'.""" """Parse closing parameter group symbol ']'."""
if not self.group: if not self.group:
fail(f"Function {function.name} has a ] without a matching [.") fail(f"Function {function.name!r} has a ']' without a matching '['.")
if not any(p.group == self.group for p in function.parameters.values()): if not any(p.group == self.group for p in function.parameters.values()):
fail(f"Function {function.name} has an empty group.\n" fail(f"Function {function.name!r} has an empty group. "
"All groups must contain at least one parameter.") "All groups must contain at least one parameter.")
self.group -= 1 self.group -= 1
match self.parameter_state: match self.parameter_state:
@ -5222,13 +5252,14 @@ class DSLParser:
case ParamState.GROUP_AFTER | ParamState.RIGHT_SQUARE_AFTER: case ParamState.GROUP_AFTER | ParamState.RIGHT_SQUARE_AFTER:
self.parameter_state = ParamState.RIGHT_SQUARE_AFTER self.parameter_state = ParamState.RIGHT_SQUARE_AFTER
case st: case st:
fail(f"Function {function.name} has an unsupported group configuration. " fail(f"Function {function.name!r} "
f"has an unsupported group configuration. "
f"(Unexpected state {st}.c)") f"(Unexpected state {st}.c)")
def parse_slash(self, function: Function) -> None: def parse_slash(self, function: Function) -> None:
"""Parse positional-only parameter marker '/'.""" """Parse positional-only parameter marker '/'."""
if self.positional_only: if self.positional_only:
fail(f"Function {function.name} uses '/' more than once.") fail(f"Function {function.name!r} uses '/' more than once.")
self.positional_only = True self.positional_only = True
# REQUIRED and OPTIONAL are allowed here, that allows positional-only # REQUIRED and OPTIONAL are allowed here, that allows positional-only
# without option groups to work (and have default values!) # without option groups to work (and have default values!)
@ -5239,10 +5270,10 @@ class DSLParser:
ParamState.GROUP_BEFORE, ParamState.GROUP_BEFORE,
} }
if (self.parameter_state not in allowed) or self.group: if (self.parameter_state not in allowed) or self.group:
fail(f"Function {function.name} has an unsupported group configuration. " fail(f"Function {function.name!r} has an unsupported group configuration. "
f"(Unexpected state {self.parameter_state}.d)") f"(Unexpected state {self.parameter_state}.d)")
if self.keyword_only: if self.keyword_only:
fail(f"Function {function.name} mixes keyword-only and " fail(f"Function {function.name!r} mixes keyword-only and "
"positional-only parameters, which is unsupported.") "positional-only parameters, which is unsupported.")
# fixup preceding parameters # fixup preceding parameters
for p in function.parameters.values(): for p in function.parameters.values():
@ -5251,7 +5282,7 @@ class DSLParser:
if (p.kind is not inspect.Parameter.POSITIONAL_OR_KEYWORD and if (p.kind is not inspect.Parameter.POSITIONAL_OR_KEYWORD and
not isinstance(p.converter, self_converter) not isinstance(p.converter, self_converter)
): ):
fail(f"Function {function.name} mixes keyword-only and " fail(f"Function {function.name!r} mixes keyword-only and "
"positional-only parameters, which is unsupported.") "positional-only parameters, which is unsupported.")
p.kind = inspect.Parameter.POSITIONAL_ONLY p.kind = inspect.Parameter.POSITIONAL_ONLY
@ -5301,7 +5332,7 @@ class DSLParser:
assert self.function is not None assert self.function is not None
if self.group: if self.group:
fail("Function " + self.function.name + " has a ] without a matching [.") fail(f"Function {self.function.name!r} has a ']' without a matching '['.")
if not self.valid_line(line): if not self.valid_line(line):
return return
@ -5528,9 +5559,9 @@ class DSLParser:
if len(lines) >= 2: if len(lines) >= 2:
if lines[1]: if lines[1]:
fail("Docstring for " + f.full_name + " does not have a summary line!\n" + fail(f"Docstring for {f.full_name!r} does not have a summary line!\n"
"Every non-blank function docstring must start with\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.")
elif len(lines) == 1: elif len(lines) == 1:
# the docstring is only one line right now--the summary line. # the docstring is only one line right now--the summary line.
# add an empty line after the summary line so we have space # add an empty line after the summary line so we have space
@ -5573,7 +5604,8 @@ class DSLParser:
last_parameter = next(reversed(list(values))) last_parameter = next(reversed(list(values)))
no_parameter_after_star = last_parameter.kind != inspect.Parameter.KEYWORD_ONLY no_parameter_after_star = last_parameter.kind != inspect.Parameter.KEYWORD_ONLY
if no_parameter_after_star: if no_parameter_after_star:
fail("Function " + self.function.name + " specifies '*' without any parameters afterwards.") fail(f"Function {self.function.name!r} specifies '*' "
"without any parameters afterwards.")
self.function.docstring = self.format_docstring() self.function.docstring = self.format_docstring()