From ac7605ed197e8b2336d44c8ac8aeae6faa90a768 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Fri, 4 Aug 2023 14:13:10 +0200 Subject: [PATCH] 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 --- Lib/test/test_clinic.py | 117 +++++++++++++++---------- Tools/clinic/clinic.py | 184 +++++++++++++++++++++++----------------- 2 files changed, 179 insertions(+), 122 deletions(-) diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index 562c66a37e3..dd17d02519b 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -161,11 +161,8 @@ class ClinicWholeFileTest(TestCase): [clinic start generated code]*/ /*[clinic end generated code: output=0123456789abcdef input=fedcba9876543210]*/ """ - err = ( - 'Checksum mismatch!\n' - 'Expected: 0123456789abcdef\n' - 'Computed: da39a3ee5e6b4b0d\n' - ) + err = ("Checksum mismatch! " + "Expected '0123456789abcdef', computed 'da39a3ee5e6b4b0d'") self.expect_failure(raw, err, filename="test.c", lineno=3) def test_garbage_after_stop_line(self): @@ -403,7 +400,7 @@ class ClinicWholeFileTest(TestCase): self.expect_failure(block, err, lineno=9) def test_badly_formed_return_annotation(self): - err = "Badly formed annotation for m.f: 'Custom'" + err = "Badly formed annotation for 'm.f': 'Custom'" block = """ /*[python input] class Custom_return_converter(CReturnConverter): @@ -509,6 +506,15 @@ class ClinicWholeFileTest(TestCase): self.assertIn(expected_warning, stdout.getvalue()) 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): err = "unknown variable 'ets'" block = """ @@ -598,7 +604,7 @@ class ClinicWholeFileTest(TestCase): self.assertEqual(generated, expected) 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 = """ /*[clinic input] preserve @@ -637,13 +643,24 @@ class ClinicWholeFileTest(TestCase): self.assertEqual(generated, block) def test_directive_output_invalid_command(self): - err = ( - "Invalid command / destination name 'cmd', must be one of:\n" - " preset push pop print everything cpp_if docstring_prototype " - "docstring_definition methoddef_define impl_prototype " - "parser_prototype parser_definition cpp_endif methoddef_ifndef " - "impl_definition" - ) + err = dedent(""" + Invalid command or destination name 'cmd'. Must be one of: + - 'preset' + - 'push' + - 'pop' + - 'print' + - 'everything' + - 'cpp_if' + - 'docstring_prototype' + - 'docstring_definition' + - 'methoddef_define' + - 'impl_prototype' + - 'parser_prototype' + - 'parser_definition' + - 'cpp_endif' + - 'methoddef_ifndef' + - 'impl_definition' + """).strip() block = """ /*[clinic input] output cmd buffer @@ -919,7 +936,7 @@ class ClinicParserTest(TestCase): self.assertEqual("MAXSIZE", p.converter.c_default) 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." ) block = """ @@ -931,7 +948,7 @@ class ClinicParserTest(TestCase): def test_param_default_expr_binop(self): 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." ) block = """ @@ -954,7 +971,7 @@ class ClinicParserTest(TestCase): def test_param_default_parameters_out_of_order(self): 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!" ) block = """ @@ -1088,7 +1105,7 @@ class ClinicParserTest(TestCase): module os 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) 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): err = ( - 'Function two_top_groups_on_left has an unsupported group ' - 'configuration. (Unexpected state 2.b)' + "Function 'two_top_groups_on_left' has an unsupported group " + "configuration. (Unexpected state 2.b)" ) block = """ module foo @@ -1281,7 +1298,7 @@ class ClinicParserTest(TestCase): ] """ 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)" ) self.expect_failure(block, err) @@ -1317,7 +1334,7 @@ class ClinicParserTest(TestCase): param: int """ 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)" ) self.expect_failure(block, err) @@ -1334,7 +1351,7 @@ class ClinicParserTest(TestCase): param: int """ err = ( - "Function empty_group has an empty group.\n" + "Function 'empty_group' has an empty group. " "All groups must contain at least one parameter." ) self.expect_failure(block, err) @@ -1351,7 +1368,7 @@ class ClinicParserTest(TestCase): ] """ err = ( - "Function empty_group has an empty group.\n" + "Function 'empty_group' has an empty group. " "All groups must contain at least one parameter." ) self.expect_failure(block, err) @@ -1365,7 +1382,7 @@ class ClinicParserTest(TestCase): 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) def test_no_parameters(self): @@ -1400,7 +1417,7 @@ class ClinicParserTest(TestCase): foo.bar => int / """ - err = "Illegal function name: foo.bar => int" + err = "Illegal function name: 'foo.bar => int'" self.expect_failure(block, err) def test_illegal_c_basename(self): @@ -1409,7 +1426,7 @@ class ClinicParserTest(TestCase): foo.bar as 935 / """ - err = "Illegal C basename: 935" + err = "Illegal C basename: '935'" self.expect_failure(block, err) 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) 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 *\nDocstring.", ) - err = "Function bar specifies '*' without any parameters afterwards." + err = "Function 'bar' specifies '*' without any parameters afterwards." for block in dataset: with self.subTest(block=block): self.expect_failure(block, err) @@ -1442,7 +1459,7 @@ class ClinicParserTest(TestCase): / """ err = ( - "Function bar has an unsupported group configuration. " + "Function 'bar' has an unsupported group configuration. " "(Unexpected state 0.d)" ) self.expect_failure(block, err) @@ -1456,7 +1473,7 @@ class ClinicParserTest(TestCase): b: int / """ - err = "Function bar uses '/' more than once." + err = "Function 'bar' uses '/' more than once." self.expect_failure(block, err) def test_mix_star_and_slash(self): @@ -1470,7 +1487,7 @@ class ClinicParserTest(TestCase): / """ err = ( - "Function bar mixes keyword-only and positional-only parameters, " + "Function 'bar' mixes keyword-only and positional-only parameters, " "which is unsupported." ) self.expect_failure(block, err) @@ -1483,7 +1500,7 @@ class ClinicParserTest(TestCase): x: int """ err = ( - "Function bar has an unsupported group configuration. " + "Function 'bar' has an unsupported group configuration. " "(Unexpected state 0.d)" ) self.expect_failure(block, err) @@ -1582,7 +1599,8 @@ class ClinicParserTest(TestCase): *vararg1: 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) def test_indent_stack_illegal_outdent(self): @@ -1841,8 +1859,17 @@ class ClinicParserTest(TestCase): """ 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): - err = "fhi is not a valid legacy converter" + err = "'fhi' is not a valid legacy converter" block = """ fn a: 'fhi' @@ -1850,7 +1877,7 @@ class ClinicParserTest(TestCase): self.expect_failure(block, err, lineno=1) 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 = """ module m z.func @@ -1877,7 +1904,7 @@ class ClinicParserTest(TestCase): self.expect_failure(block, err, lineno=2) 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 = """ module m m.func @@ -1887,7 +1914,7 @@ class ClinicParserTest(TestCase): self.expect_failure(block, err, lineno=2) 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 = """ module m m.func @@ -1983,13 +2010,11 @@ class ClinicExternalTest(TestCase): const char *hand_edited = "output block is overwritten"; /*[clinic end generated code: output=bogus input=bogus]*/ """) - fail_msg = dedent(""" - Checksum mismatch! - Expected: bogus - Computed: 2ed19 - Suggested fix: remove all generated code including the end marker, - or use the '-f' option. - """) + fail_msg = ( + "Checksum mismatch! Expected 'bogus', computed '2ed19'. " + "Suggested fix: remove all generated code including the end marker, " + "or use the '-f' option.\n" + ) with os_helper.temp_dir() as tmp_dir: fn = os.path.join(tmp_dir, "test.c") with open(fn, "w", encoding="utf-8") as f: @@ -2214,7 +2239,7 @@ class ClinicExternalTest(TestCase): f.write("") # Write an empty output file! # Clinic should complain about the empty output file. _, 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!") self.assertIn(expected_err, err) # Run clinic again, this time with the -f option. diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 7525c1ca524..ee5e0ef70f2 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -290,9 +290,11 @@ def linear_format(s: str, **kwargs: str) -> str: continue 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(): - 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] if not value: @@ -1534,7 +1536,8 @@ class CLanguage(Language): c.render(p, data) 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 # when we're METH_O, but have a custom return converter, @@ -1871,7 +1874,7 @@ class BlockParser: for field in shlex.split(arguments): name, equals, value = field.partition('=') 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() if self.verify: @@ -1882,11 +1885,10 @@ class BlockParser: computed = compute_checksum(output, len(checksum)) 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 " - "the end marker,\n" - "or use the '-f' option." - .format(checksum, computed)) + "the end marker, or use the '-f' option.") else: # put back output output_lines = output.splitlines(keepends=True) @@ -2017,9 +2019,10 @@ class Destination: ) extra_arguments = 1 if self.type == "file" else 0 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: - 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': d = {} filename = self.clinic.filename @@ -2041,7 +2044,7 @@ class Destination: def clear(self) -> None: 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() def dump(self) -> str: @@ -2220,13 +2223,13 @@ impl_definition block *args: str ) -> None: 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) def get_destination(self, name: str) -> Destination: d = self.destinations.get(name) if not d: - fail("Destination does not exist: " + repr(name)) + fail(f"Destination does not exist: {name!r}") return d def get_destination_buffer( @@ -2273,15 +2276,16 @@ impl_definition block os.makedirs(dirname) except FileExistsError: if not os.path.isdir(dirname): - fail("Can't write to destination {}, " - "can't make directory {}!".format( - destination.filename, dirname)) + fail(f"Can't write to destination " + f"{destination.filename!r}; " + f"can't make directory {dirname!r}!") if self.verify: with open(destination.filename) as f: parser_2 = BlockParser(f.read(), language=self.language) blocks = list(parser_2) 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: pass @@ -2322,7 +2326,8 @@ impl_definition block return module, cls child = parent.classes.get(field) 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 return module, cls @@ -2339,12 +2344,12 @@ def parse_file( extension = os.path.splitext(filename)[1][1:] 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: language = extensions[extension](filename) 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: raw = f.read() @@ -2823,8 +2828,9 @@ class CConverter(metaclass=CConverterAutoRegister): else: names = [cls.__name__ for cls in self.default_type] types_str = ', '.join(names) - fail("{}: default value {!r} for field {} is not of type {}".format( - self.__class__.__name__, default, name, types_str)) + cls_name = self.__class__.__name__ + fail(f"{cls_name}: default value {default!r} for field " + f"{name!r} is not of type {types_str!r}") self.default = default if c_default: @@ -3146,7 +3152,7 @@ class bool_converter(CConverter): if accept == {int}: self.format_unit = 'i' 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: self.default = bool(self.default) self.c_default = str(int(self.default)) @@ -3196,7 +3202,7 @@ class char_converter(CConverter): def converter_init(self) -> None: if isinstance(self.default, self.default_type): 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:] if self.c_default == '"\'"': @@ -3335,7 +3341,7 @@ class int_converter(CConverter): if accept == {str}: self.format_unit = 'C' 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: self.type = type @@ -3472,7 +3478,7 @@ class Py_ssize_t_converter(CConverter): elif accept == {int, NoneType}: self.converter = '_Py_convert_optional_to_ssize_t' 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: if self.format_unit == 'n': @@ -3502,7 +3508,7 @@ class slice_index_converter(CConverter): elif accept == {int, NoneType}: self.converter = '_PyEval_SliceIndex' 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): type = 'size_t' @@ -3850,7 +3856,7 @@ class Py_UNICODE_converter(CConverter): elif accept == {str, NoneType}: self.converter = '_PyUnicode_WideCharString_Opt_Converter' else: - fail("Py_UNICODE_converter: illegal 'accept' argument " + repr(accept)) + fail(f"Py_UNICODE_converter: illegal 'accept' argument {accept!r}") self.c_default = "NULL" def cleanup(self) -> str: @@ -4378,7 +4384,7 @@ class IndentStack: margin = self.margin indent = self.indents[-1] 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:] @@ -4464,7 +4470,9 @@ class DSLParser: def directive_version(self, required: str) -> None: global version 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: fields = name.split('.')[:-1] @@ -4473,7 +4481,7 @@ class DSLParser: fail("Can't nest a module inside a class!") if name in module.modules: - fail("Already defined module " + repr(name) + "!") + fail(f"Already defined module {name!r}!") m = Module(name, module) module.modules[name] = m @@ -4491,7 +4499,7 @@ class DSLParser: parent = cls or module 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) parent.classes[name] = c @@ -4499,7 +4507,7 @@ class DSLParser: def directive_set(self, name: str, value: str) -> None: if name not in ("line_prefix", "line_suffix"): - fail("unknown variable", repr(name)) + fail(f"unknown variable {name!r}") value = value.format_map({ 'block comment start': '/*', @@ -4520,7 +4528,7 @@ class DSLParser: case "clear": self.clinic.get_destination(name).clear() case _: - fail("unknown destination command", repr(command)) + fail(f"unknown destination command {command!r}") def directive_output( @@ -4533,7 +4541,7 @@ class DSLParser: if command_or_name == "preset": preset = self.clinic.presets.get(destination) if not preset: - fail("Unknown preset " + repr(destination) + "!") + fail(f"Unknown preset {destination!r}!") fd.update(preset) return @@ -4562,7 +4570,11 @@ class DSLParser: return 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 def directive_dump(self, name: str) -> None: @@ -4574,7 +4586,7 @@ class DSLParser: def directive_preserve(self) -> None: 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 def at_classmethod(self) -> None: @@ -4601,7 +4613,8 @@ class DSLParser: lines = block.input.split('\n') for line_number, line in enumerate(lines, self.clinic.block_parser.block_start_line_number): 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: self.state(line) except ClinicError as exc: @@ -4712,7 +4725,8 @@ class DSLParser: module, cls = self.clinic._module_and_class(fields) 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( name=function_name, full_name=full_name, module=module, cls=cls, c_basename=c_basename, docstring='' @@ -4731,9 +4745,9 @@ class DSLParser: c_basename = c_basename.strip() or None 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): - fail("Illegal C basename:", c_basename) + fail(f"Illegal C basename: {c_basename!r}") return_converter = None if returns: @@ -4741,7 +4755,7 @@ class DSLParser: try: module_node = ast.parse(ast_input) 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] assert isinstance(function_node, ast.FunctionDef) try: @@ -4752,7 +4766,7 @@ class DSLParser: fail(f"No available return converter called {name!r}") return_converter = return_converters[name](**kwargs) 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('.')] function_name = fields.pop() @@ -4777,7 +4791,7 @@ class DSLParser: return_converter = CReturnConverter() 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, return_converter=return_converter, kind=self.kind, coexist=self.coexist) self.block.signatures.append(self.function) @@ -4963,18 +4977,22 @@ class DSLParser: except SyntaxError: pass 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] assert isinstance(function, ast.FunctionDef) function_args = function.args 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: - 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: - 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: is_vararg = True @@ -4988,7 +5006,7 @@ class DSLParser: if not default: 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!") value: Sentinels | Null if is_vararg: @@ -5048,10 +5066,10 @@ class DSLParser: except NameError: pass # probably a named constant except Exception as e: - fail("Malformed expression given as default value\n" - "{!r} caused {!r}".format(default, e)) + fail("Malformed expression given as default value " + f"{default!r} caused {e!r}") if bad: - fail("Unsupported expression as default value: " + repr(default)) + fail(f"Unsupported expression as default value: {default!r}") assignment = module.body[0] assert isinstance(assignment, ast.Assign) @@ -5069,7 +5087,10 @@ class DSLParser: )): c_default = kwargs.get("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 value = unknown elif isinstance(expr, ast.Attribute): @@ -5079,13 +5100,16 @@ class DSLParser: a.append(n.attr) n = n.value 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) py_default = ".".join(reversed(a)) c_default = kwargs.get("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: value = eval(py_default) @@ -5102,13 +5126,15 @@ class DSLParser: c_default = py_default except SyntaxError as e: - fail("Syntax error: " + repr(e.text)) + fail(f"Syntax error: {e.text!r}") except (ValueError, AttributeError): value = unknown c_default = kwargs.get("c_default") py_default = 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('py_default', py_default) @@ -5116,7 +5142,7 @@ class DSLParser: dict = legacy_converters if legacy else converters legacy_str = "legacy " if legacy else "" 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 # but the parameter object gets the python name 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.function.parameters.clear() 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): _lp = len(self.function.parameters) @@ -5153,16 +5180,18 @@ class DSLParser: if self.group: fail("A 'defining_class' parameter cannot be in an optional group.") 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) names = [k.name for k in self.function.parameters.values()] 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: - 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 self.function.parameters[key] = p @@ -5192,7 +5221,7 @@ class DSLParser: def parse_star(self, function: Function) -> None: """Parse keyword-only parameter marker '*'.""" 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 def parse_opening_square_bracket(self, function: Function) -> None: @@ -5203,7 +5232,8 @@ class DSLParser: case ParamState.REQUIRED | ParamState.GROUP_AFTER: self.parameter_state = ParamState.GROUP_AFTER 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)") self.group += 1 function.docstring_only = True @@ -5211,9 +5241,9 @@ class DSLParser: def parse_closing_square_bracket(self, function: Function) -> None: """Parse closing parameter group symbol ']'.""" 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()): - 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.") self.group -= 1 match self.parameter_state: @@ -5222,13 +5252,14 @@ class DSLParser: case ParamState.GROUP_AFTER | ParamState.RIGHT_SQUARE_AFTER: self.parameter_state = ParamState.RIGHT_SQUARE_AFTER 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)") def parse_slash(self, function: Function) -> None: """Parse positional-only parameter marker '/'.""" 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 # REQUIRED and OPTIONAL are allowed here, that allows positional-only # without option groups to work (and have default values!) @@ -5239,10 +5270,10 @@ class DSLParser: ParamState.GROUP_BEFORE, } 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)") 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.") # fixup preceding parameters for p in function.parameters.values(): @@ -5251,7 +5282,7 @@ class DSLParser: if (p.kind is not inspect.Parameter.POSITIONAL_OR_KEYWORD and 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.") p.kind = inspect.Parameter.POSITIONAL_ONLY @@ -5301,7 +5332,7 @@ class DSLParser: assert self.function is not None 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): return @@ -5528,9 +5559,9 @@ class DSLParser: if len(lines) >= 2: if lines[1]: - fail("Docstring for " + f.full_name + " does not have a summary line!\n" + - "Every non-blank function docstring must start with\n" + - "a single line summary followed by an empty line.") + 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.") 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 @@ -5573,7 +5604,8 @@ class DSLParser: last_parameter = next(reversed(list(values))) no_parameter_after_star = last_parameter.kind != inspect.Parameter.KEYWORD_ONLY 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()