From 36bac55a73b717b4028e85c508129e0f24f21081 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Sun, 30 Jul 2023 00:33:34 +0200 Subject: [PATCH 01/24] Rework error handling - Introduce ClinicError and ClinicWarning - fail() raises ClinicError - warn() uses warnings.warn(msg, ClinicWarning) - the CLI runs main(), catches ClinicError, formats the error message prints to stderr and exits with an error - adapt the test suite to work with ClinicError --- Lib/test/test_clinic.py | 495 +++++++++++++++++----------------------- Tools/clinic/clinic.py | 96 +++++--- 2 files changed, 273 insertions(+), 318 deletions(-) diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index 3ce27d1dd6b487..17484e3ade7d23 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -21,17 +21,14 @@ from clinic import DSLParser -class _ParserBase(TestCase): - maxDiff = None - - def expect_parser_failure(self, parser, _input): - with support.captured_stdout() as stdout: - with self.assertRaises(SystemExit): - parser(_input) - return stdout.getvalue() - - def parse_function_should_fail(self, _input): - return self.expect_parser_failure(self.parse_function, _input) +def _expect_failure(tc, parser, code, errmsg, *, filename=None, lineno=None): + _code = dedent(code).strip() + with tc.assertRaisesRegex(clinic.ClinicError, errmsg) as cm: + parser(_code) + if filename is not None: + tc.assertEqual(cm.exception.filename, filename) + if lineno is not None: + tc.assertEqual(cm.exception.lineno, lineno) class FakeConverter: @@ -109,14 +106,15 @@ def __repr__(self): return "" -class ClinicWholeFileTest(_ParserBase): +class ClinicWholeFileTest(TestCase): + + def expect_failure(self, raw, errmsg, *, filename=None, lineno=None): + _expect_failure(self, self.clinic.parse, raw, errmsg, + filename=filename, lineno=lineno) + def setUp(self): self.clinic = clinic.Clinic(clinic.CLanguage(None), filename="test.c") - def expect_failure(self, raw): - _input = dedent(raw).strip() - return self.expect_parser_failure(self.clinic.parse, _input) - def test_eol(self): # regression test: # clinic's block parser didn't recognize @@ -140,12 +138,11 @@ def test_mangled_marker_line(self): [clinic start generated code]*/ /*[clinic end generated code: foo]*/ """ - msg = ( - 'Error in file "test.c" on line 3:\n' - "Mangled Argument Clinic marker line: '/*[clinic end generated code: foo]*/'\n" + err = ( + "Mangled Argument Clinic marker line: " + "'\/\*\[clinic end generated code: foo\]\*\/'" ) - out = self.expect_failure(raw) - self.assertEqual(out, msg) + self.expect_failure(raw, err, filename="test.c", lineno=3) def test_checksum_mismatch(self): raw = """ @@ -153,38 +150,31 @@ def test_checksum_mismatch(self): [clinic start generated code]*/ /*[clinic end generated code: output=0123456789abcdef input=fedcba9876543210]*/ """ - msg = ( - 'Error in file "test.c" on line 3:\n' + err = ( 'Checksum mismatch!\n' 'Expected: 0123456789abcdef\n' 'Computed: da39a3ee5e6b4b0d\n' ) - out = self.expect_failure(raw) - self.assertIn(msg, out) + self.expect_failure(raw, err, filename="test.c", lineno=3) def test_garbage_after_stop_line(self): raw = """ /*[clinic input] [clinic start generated code]*/foobarfoobar! """ - msg = ( - 'Error in file "test.c" on line 2:\n' - "Garbage after stop line: 'foobarfoobar!'\n" - ) - out = self.expect_failure(raw) - self.assertEqual(out, msg) + err = "Garbage after stop line: 'foobarfoobar!'" + self.expect_failure(raw, err, filename="test.c", lineno=2) def test_whitespace_before_stop_line(self): raw = """ /*[clinic input] [clinic start generated code]*/ """ - msg = ( - 'Error in file "test.c" on line 2:\n' - "Whitespace is not allowed before the stop line: ' [clinic start generated code]*/'\n" + err = ( + "Whitespace is not allowed before the stop line: " + "' \[clinic start generated code\]\*\/'" ) - out = self.expect_failure(raw) - self.assertEqual(out, msg) + self.expect_failure(raw, err, filename="test.c", lineno=2) def test_parse_with_body_prefix(self): clang = clinic.CLanguage(None) @@ -214,12 +204,8 @@ def test_cpp_monitor_fail_nested_block_comment(self): */ */ """ - msg = ( - 'Error in file "test.c" on line 2:\n' - 'Nested block comment!\n' - ) - out = self.expect_failure(raw) - self.assertEqual(out, msg) + err = 'Nested block comment!' + self.expect_failure(raw, err, filename="test.c", lineno=2) def test_cpp_monitor_fail_invalid_format_noarg(self): raw = """ @@ -227,12 +213,8 @@ def test_cpp_monitor_fail_invalid_format_noarg(self): a() #endif """ - msg = ( - 'Error in file "test.c" on line 1:\n' - 'Invalid format for #if line: no argument!\n' - ) - out = self.expect_failure(raw) - self.assertEqual(out, msg) + err = 'Invalid format for #if line: no argument!' + self.expect_failure(raw, err, filename="test.c", lineno=1) def test_cpp_monitor_fail_invalid_format_toomanyargs(self): raw = """ @@ -240,39 +222,31 @@ def test_cpp_monitor_fail_invalid_format_toomanyargs(self): a() #endif """ - msg = ( - 'Error in file "test.c" on line 1:\n' - 'Invalid format for #ifdef line: should be exactly one argument!\n' - ) - out = self.expect_failure(raw) - self.assertEqual(out, msg) + err = 'Invalid format for #ifdef line: should be exactly one argument!' + self.expect_failure(raw, err, filename="test.c", lineno=1) def test_cpp_monitor_fail_no_matching_if(self): raw = '#else' - msg = ( - 'Error in file "test.c" on line 1:\n' - '#else without matching #if / #ifdef / #ifndef!\n' - ) - out = self.expect_failure(raw) - self.assertEqual(out, msg) + err = '#else without matching #if / #ifdef / #ifndef!' + self.expect_failure(raw, err, filename="test.c", lineno=1) def test_directive_output_unknown_preset(self): - out = self.expect_failure(""" + raw = """ /*[clinic input] output preset nosuchpreset [clinic start generated code]*/ - """) - msg = "Unknown preset 'nosuchpreset'" - self.assertIn(msg, out) + """ + err = "Unknown preset 'nosuchpreset'" + self.expect_failure(raw, err) def test_directive_output_cant_pop(self): - out = self.expect_failure(""" + raw = """ /*[clinic input] output pop [clinic start generated code]*/ - """) - msg = "Can't 'output pop', stack is empty" - self.assertIn(msg, out) + """ + err = "Can't 'output pop', stack is empty" + self.expect_failure(raw, err) def test_directive_output_print(self): raw = dedent(""" @@ -310,13 +284,13 @@ def test_directive_output_print(self): ) def test_unknown_destination_command(self): - out = self.expect_failure(""" + raw = """ /*[clinic input] destination buffer nosuchcommand [clinic start generated code]*/ - """) - msg = "unknown destination command 'nosuchcommand'" - self.assertIn(msg, out) + """ + err = "unknown destination command 'nosuchcommand'" + self.expect_failure(raw, err) class ClinicGroupPermuterTest(TestCase): @@ -495,7 +469,27 @@ def test_clinic_1(self): """) -class ClinicParserTest(_ParserBase): +class ClinicParserTest(TestCase): + + def parse(self, text): + c = FakeClinic() + parser = DSLParser(c) + block = clinic.Block(text) + parser.parse(block) + return block + + def parse_function(self, text, signatures_in_block=2, function_index=1): + block = self.parse(text) + s = block.signatures + self.assertEqual(len(s), signatures_in_block) + assert isinstance(s[0], clinic.Module) + assert isinstance(s[function_index], clinic.Function) + return s[function_index] + + def expect_failure(self, block, err, *, filename=None, lineno=None): + _expect_failure(self, self.parse_function, block, err, + filename=filename, lineno=lineno) + def checkDocstring(self, fn, expected): self.assertTrue(hasattr(fn, "docstring")) self.assertEqual(fn.docstring.strip(), @@ -565,17 +559,16 @@ def test_param_default_expression(self): self.assertEqual(sys.maxsize, p.default) self.assertEqual("MAXSIZE", p.converter.c_default) - expected_msg = ( - "Error on line 0:\n" - "When you specify a named constant ('sys.maxsize') as your default value,\n" - "you MUST specify a valid c_default.\n" + err = ( + "When you specify a named constant \('sys.maxsize'\) as your default value,\n" + "you MUST specify a valid c_default." ) - out = self.parse_function_should_fail(""" + block = """ module os os.access follow_symlinks: int = sys.maxsize - """) - self.assertEqual(out, expected_msg) + """ + self.expect_failure(block, err, lineno=0) def test_param_no_docstring(self): function = self.parse_function(""" @@ -590,17 +583,17 @@ def test_param_no_docstring(self): self.assertIsInstance(conv, clinic.str_converter) def test_param_default_parameters_out_of_order(self): - expected_msg = ( - "Error on line 0:\n" - "Can't have a parameter without a default ('something_else')\n" - "after a parameter with a default!\n" + err = ( + "Can't have a parameter without a default \('something_else'\)\n" + "after a parameter with a default!" ) - out = self.parse_function_should_fail(""" + block = """ module os os.access follow_symlinks: bool = True - something_else: str""") - self.assertEqual(out, expected_msg) + something_else: str + """ + self.expect_failure(block, err, lineno=0) def disabled_test_converter_arguments(self): function = self.parse_function(""" @@ -677,17 +670,12 @@ def test_c_name(self): self.assertEqual("os_stat_fn", function.c_basename) def test_cloning_nonexistent_function_correctly_fails(self): - stdout = self.parse_function_should_fail(""" - cloned = fooooooooooooooooooooooo + block = """ + cloned = fooooooooooooooooo This is trying to clone a nonexistent function!! - """) - expected_error = """\ -cls=None, module=, existing='fooooooooooooooooooooooo' -(cls or module).functions=[] -Error on line 0: -Couldn't find existing function 'fooooooooooooooooooooooo'! -""" - self.assertEqual(expected_error, stdout) + """ + err = "Couldn't find existing function 'fooooooooooooooooo'!" + self.expect_failure(block, err, lineno=0) def test_return_converter(self): function = self.parse_function(""" @@ -697,30 +685,28 @@ def test_return_converter(self): self.assertIsInstance(function.return_converter, clinic.int_return_converter) def test_return_converter_invalid_syntax(self): - stdout = self.parse_function_should_fail(""" + block = """ module os os.stat -> invalid syntax - """) - expected_error = "Badly formed annotation for os.stat: 'invalid syntax'" - self.assertIn(expected_error, stdout) + """ + err = "Badly formed annotation for os.stat: 'invalid syntax'" + self.expect_failure(block, err) def test_legacy_converter_disallowed_in_return_annotation(self): - stdout = self.parse_function_should_fail(""" + block = """ module os os.stat -> "s" - """) - expected_error = "Legacy converter 's' not allowed as a return converter" - self.assertIn(expected_error, stdout) + """ + err = "Legacy converter 's' not allowed as a return converter" + self.expect_failure(block, err) def test_unknown_return_converter(self): - stdout = self.parse_function_should_fail(""" + block = """ module os - os.stat -> foooooooooooooooooooooooo - """) - expected_error = ( - "No available return converter called 'foooooooooooooooooooooooo'" - ) - self.assertIn(expected_error, stdout) + os.stat -> fooooooooooooooooooooooo + """ + err = "No available return converter called 'fooooooooooooooooooooooo'" + self.expect_failure(block, err) def test_star(self): function = self.parse_function(""" @@ -865,19 +851,12 @@ def test_nested_groups(self): Attributes for the character. """) - def parse_function_should_fail(self, s): - with support.captured_stdout() as stdout: - with self.assertRaises(SystemExit): - self.parse_function(s) - return stdout.getvalue() - def test_disallowed_grouping__two_top_groups_on_left(self): - expected_msg = ( - 'Error on line 0:\n' + err = ( 'Function two_top_groups_on_left has an unsupported group ' - 'configuration. (Unexpected state 2.b)\n' + 'configuration. \(Unexpected state 2.b\)' ) - out = self.parse_function_should_fail(""" + block = """ module foo foo.two_top_groups_on_left [ @@ -887,11 +866,11 @@ def test_disallowed_grouping__two_top_groups_on_left(self): group2 : int ] param: int - """) - self.assertEqual(out, expected_msg) + """ + self.expect_failure(block, err, lineno=0) def test_disallowed_grouping__two_top_groups_on_right(self): - out = self.parse_function_should_fail(""" + block = """ module foo foo.two_top_groups_on_right param: int @@ -901,15 +880,15 @@ def test_disallowed_grouping__two_top_groups_on_right(self): [ group2 : int ] - """) - msg = ( + """ + err = ( "Function two_top_groups_on_right has an unsupported group " - "configuration. (Unexpected state 6.b)" + "configuration. \(Unexpected state 6.b\)" ) - self.assertIn(msg, out) + self.expect_failure(block, err) def test_disallowed_grouping__parameter_after_group_on_right(self): - out = self.parse_function_should_fail(""" + block = """ module foo foo.parameter_after_group_on_right param: int @@ -919,15 +898,15 @@ def test_disallowed_grouping__parameter_after_group_on_right(self): ] group2 : int ] - """) - msg = ( + """ + err = ( "Function parameter_after_group_on_right has an unsupported group " - "configuration. (Unexpected state 6.a)" + "configuration. \(Unexpected state 6.a\)" ) - self.assertIn(msg, out) + self.expect_failure(block, err) def test_disallowed_grouping__group_after_parameter_on_left(self): - out = self.parse_function_should_fail(""" + block = """ module foo foo.group_after_parameter_on_left [ @@ -937,15 +916,15 @@ def test_disallowed_grouping__group_after_parameter_on_left(self): ] ] param: int - """) - msg = ( + """ + err = ( "Function group_after_parameter_on_left has an unsupported group " - "configuration. (Unexpected state 2.b)" + "configuration. \(Unexpected state 2.b\)" ) - self.assertIn(msg, out) + self.expect_failure(block, err) def test_disallowed_grouping__empty_group_on_left(self): - out = self.parse_function_should_fail(""" + block = """ module foo foo.empty_group [ @@ -954,15 +933,15 @@ def test_disallowed_grouping__empty_group_on_left(self): group2 : int ] param: int - """) - msg = ( + """ + err = ( "Function empty_group has an empty group.\n" "All groups must contain at least one parameter." ) - self.assertIn(msg, out) + self.expect_failure(block, err) def test_disallowed_grouping__empty_group_on_right(self): - out = self.parse_function_should_fail(""" + block = """ module foo foo.empty_group param: int @@ -971,24 +950,24 @@ def test_disallowed_grouping__empty_group_on_right(self): ] group2 : int ] - """) - msg = ( + """ + err = ( "Function empty_group has an empty group.\n" "All groups must contain at least one parameter." ) - self.assertIn(msg, out) + self.expect_failure(block, err) def test_disallowed_grouping__no_matching_bracket(self): - out = self.parse_function_should_fail(""" + block = """ module foo foo.empty_group param: int ] group2: int ] - """) - msg = "Function empty_group has a ] without a matching [." - self.assertIn(msg, out) + """ + err = "Function empty_group has a \] without a matching \[." + self.expect_failure(block, err) def test_no_parameters(self): function = self.parse_function(""" @@ -1017,31 +996,32 @@ class foo.Bar "unused" "notneeded" self.assertEqual(1, len(function.parameters)) def test_illegal_module_line(self): - out = self.parse_function_should_fail(""" + block = """ module foo foo.bar => int / - """) - msg = "Illegal function name: foo.bar => int" - self.assertIn(msg, out) + """ + err = "Illegal function name: foo.bar => int" + self.expect_failure(block, err) def test_illegal_c_basename(self): - out = self.parse_function_should_fail(""" + block = """ module foo foo.bar as 935 / - """) - msg = "Illegal C basename: 935" - self.assertIn(msg, out) + """ + err = "Illegal C basename: 935" + self.expect_failure(block, err) def test_single_star(self): - out = self.parse_function_should_fail(""" + block = """ module foo foo.bar * * - """) - self.assertIn("Function bar uses '*' more than once.", out) + """ + err = "Function bar uses '\*' more than once." + self.expect_failure(block, err) def test_parameters_required_after_star(self): dataset = ( @@ -1050,39 +1030,38 @@ def test_parameters_required_after_star(self): "module foo\nfoo.bar\n this: int\n *", "module foo\nfoo.bar\n this: int\n *\nDocstring.", ) - msg = "Function bar specifies '*' without any parameters afterwards." + err = "Function bar specifies '\*' without any parameters afterwards." for block in dataset: with self.subTest(block=block): - out = self.parse_function_should_fail(block) - self.assertIn(msg, out) + self.expect_failure(block, err) def test_single_slash(self): - out = self.parse_function_should_fail(""" + block = """ module foo foo.bar / / - """) - msg = ( + """ + err = ( "Function bar has an unsupported group configuration. " - "(Unexpected state 0.d)" + "\(Unexpected state 0.d\)" ) - self.assertIn(msg, out) + self.expect_failure(block, err) def test_double_slash(self): - out = self.parse_function_should_fail(""" + block = """ module foo foo.bar a: int / b: int / - """) - msg = "Function bar uses '/' more than once." - self.assertIn(msg, out) + """ + err = "Function bar uses '/' more than once." + self.expect_failure(block, err) def test_mix_star_and_slash(self): - out = self.parse_function_should_fail(""" + block = """ module foo foo.bar x: int @@ -1090,38 +1069,35 @@ def test_mix_star_and_slash(self): * z: int / - """) - msg = ( + """ + err = ( "Function bar mixes keyword-only and positional-only parameters, " "which is unsupported." ) - self.assertIn(msg, out) + self.expect_failure(block, err) def test_parameters_not_permitted_after_slash_for_now(self): - out = self.parse_function_should_fail(""" + block = """ module foo foo.bar / x: int - """) - msg = ( + """ + err = ( "Function bar has an unsupported group configuration. " - "(Unexpected state 0.d)" + "\(Unexpected state 0.d\)" ) - self.assertIn(msg, out) + self.expect_failure(block, err) def test_parameters_no_more_than_one_vararg(self): - expected_msg = ( - "Error on line 0:\n" - "Too many var args\n" - ) - out = self.parse_function_should_fail(""" + err = "Too many var args" + block = """ module foo foo.bar *vararg1: object *vararg2: object - """) - self.assertEqual(out, expected_msg) + """ + self.expect_failure(block, err, lineno=0) def test_function_not_at_column_0(self): function = self.parse_function(""" @@ -1144,23 +1120,24 @@ def test_function_not_at_column_0(self): """) def test_indent_stack_no_tabs(self): - out = self.parse_function_should_fail(""" + block = """ module foo foo.bar *vararg1: object \t*vararg2: object - """) - msg = "Tab characters are illegal in the Clinic DSL." - self.assertIn(msg, out) + """ + err = "Tab characters are illegal in the Clinic DSL." + self.expect_failure(block, err) def test_indent_stack_illegal_outdent(self): - out = self.parse_function_should_fail(""" + block = """ module foo foo.bar a: object b: object - """) - self.assertIn("Illegal outdent", out) + """ + err = "Illegal outdent" + self.expect_failure(block, err) def test_directive(self): c = FakeClinic() @@ -1178,10 +1155,7 @@ def test_legacy_converters(self): self.assertIsInstance(conv, clinic.str_converter) def test_legacy_converters_non_string_constant_annotation(self): - expected_failure_message = ( - "Error on line 0:\n" - "Annotations must be either a name, a function call, or a string.\n" - ) + err = "Annotations must be either a name, a function call, or a string" dataset = ( 'module os\nos.access\n path: 42', 'module os\nos.access\n path: 42.42', @@ -1190,14 +1164,10 @@ def test_legacy_converters_non_string_constant_annotation(self): ) for block in dataset: with self.subTest(block=block): - out = self.parse_function_should_fail(block) - self.assertEqual(out, expected_failure_message) + self.expect_failure(block, err, lineno=0) def test_other_bizarre_things_in_annotations_fail(self): - expected_failure_message = ( - "Error on line 0:\n" - "Annotations must be either a name, a function call, or a string.\n" - ) + err = "Annotations must be either a name, a function call, or a string" dataset = ( 'module os\nos.access\n path: {"some": "dictionary"}', 'module os\nos.access\n path: ["list", "of", "strings"]', @@ -1205,30 +1175,24 @@ def test_other_bizarre_things_in_annotations_fail(self): ) for block in dataset: with self.subTest(block=block): - out = self.parse_function_should_fail(block) - self.assertEqual(out, expected_failure_message) + self.expect_failure(block, err, lineno=0) def test_kwarg_splats_disallowed_in_function_call_annotations(self): - expected_error_msg = ( - "Error on line 0:\n" - "Cannot use a kwarg splat in a function-call annotation\n" - ) + err = "Cannot use a kwarg splat in a function-call annotation" dataset = ( 'module fo\nfo.barbaz\n o: bool(**{None: "bang!"})', 'module fo\nfo.barbaz -> bool(**{None: "bang!"})', 'module fo\nfo.barbaz -> bool(**{"bang": 42})', 'module fo\nfo.barbaz\n o: bool(**{"bang": None})', ) - for fn in dataset: - with self.subTest(fn=fn): - out = self.parse_function_should_fail(fn) - self.assertEqual(out, expected_error_msg) + for block in dataset: + with self.subTest(block=block): + self.expect_failure(block, err, lineno=0) def test_self_param_placement(self): expected_error_msg = ( - "Error on line 0:\n" "A 'self' parameter, if specified, must be the very first thing " - "in the parameter block.\n" + "in the parameter block." ) block = """ module foo @@ -1236,27 +1200,21 @@ def test_self_param_placement(self): a: int self: self(type="PyObject *") """ - out = self.parse_function_should_fail(block) - self.assertEqual(out, expected_error_msg) + self.expect_failure(block, expected_error_msg, lineno=0) def test_self_param_cannot_be_optional(self): - expected_error_msg = ( - "Error on line 0:\n" - "A 'self' parameter cannot be marked optional.\n" - ) + err = "A 'self' parameter cannot be marked optional." block = """ module foo foo.func self: self(type="PyObject *") = None """ - out = self.parse_function_should_fail(block) - self.assertEqual(out, expected_error_msg) + self.expect_failure(block, err, lineno=0) def test_defining_class_param_placement(self): expected_error_msg = ( - "Error on line 0:\n" "A 'defining_class' parameter, if specified, must either be the " - "first thing in the parameter block, or come just after 'self'.\n" + "first thing in the parameter block, or come just after 'self'." ) block = """ module foo @@ -1265,21 +1223,16 @@ def test_defining_class_param_placement(self): a: int cls: defining_class """ - out = self.parse_function_should_fail(block) - self.assertEqual(out, expected_error_msg) + self.expect_failure(block, expected_error_msg, lineno=0) def test_defining_class_param_cannot_be_optional(self): - expected_error_msg = ( - "Error on line 0:\n" - "A 'defining_class' parameter cannot be marked optional.\n" - ) + err = "A 'defining_class' parameter cannot be marked optional." block = """ module foo foo.func cls: defining_class(type="PyObject *") = None """ - out = self.parse_function_should_fail(block) - self.assertEqual(out, expected_error_msg) + self.expect_failure(block, err, lineno=0) def test_slot_methods_cannot_access_defining_class(self): block = """ @@ -1289,34 +1242,28 @@ class Foo "" "" cls: defining_class a: object """ - msg = "Slot methods cannot access their defining class." - with self.assertRaisesRegex(ValueError, msg): + err = "Slot methods cannot access their defining class." + with self.assertRaisesRegex(ValueError, err): self.parse_function(block) def test_new_must_be_a_class_method(self): - expected_error_msg = ( - "Error on line 0:\n" - "__new__ must be a class method!\n" - ) - out = self.parse_function_should_fail(""" + err = "__new__ must be a class method!" + block = """ module foo class Foo "" "" Foo.__new__ - """) - self.assertEqual(out, expected_error_msg) + """ + self.expect_failure(block, err, lineno=0) def test_init_must_be_a_normal_method(self): - expected_error_msg = ( - "Error on line 0:\n" - "__init__ must be a normal method, not a class or static method!\n" - ) - out = self.parse_function_should_fail(""" + err = "__init__ must be a normal method, not a class or static method!" + block = """ module foo class Foo "" "" @classmethod Foo.__init__ - """) - self.assertEqual(out, expected_error_msg) + """ + self.expect_failure(block, err, lineno=0) def test_unused_param(self): block = self.parse(""" @@ -1356,21 +1303,6 @@ def test_unused_param(self): parser_decl = p.simple_declaration(in_parser=True) self.assertNotIn("Py_UNUSED", parser_decl) - def parse(self, text): - c = FakeClinic() - parser = DSLParser(c) - block = clinic.Block(text) - parser.parse(block) - return block - - def parse_function(self, text, signatures_in_block=2, function_index=1): - block = self.parse(text) - s = block.signatures - self.assertEqual(len(s), signatures_in_block) - assert isinstance(s[0], clinic.Module) - assert isinstance(s[function_index], clinic.Function) - return s[function_index] - def test_scaffolding(self): # test repr on special values self.assertEqual(repr(clinic.unspecified), '') @@ -1382,11 +1314,12 @@ def test_scaffolding(self): 'The igloos are melting!\n' ) with support.captured_stdout() as stdout: - with self.assertRaises(SystemExit): - clinic.fail('The igloos are melting!', - filename='clown.txt', line_number=69) - actual = stdout.getvalue() - self.assertEqual(actual, expected) + errmsg = 'The igloos are melting' + with self.assertRaisesRegex(clinic.ClinicError, errmsg) as cm: + clinic.fail(errmsg, filename='clown.txt', line_number=69) + exc = cm.exception + self.assertEqual(exc.filename, 'clown.txt') + self.assertEqual(exc.lineno, 69) class ClinicExternalTest(TestCase): @@ -1406,9 +1339,11 @@ def _do_test(self, *args, expect_success=True): self.fail("".join([*proc.stdout, *proc.stderr])) stdout = proc.stdout.read() stderr = proc.stderr.read() - # Clinic never writes to stderr. - self.assertEqual(stderr, "") - return stdout + if expect_success: + self.assertEqual(stderr, "") + return stdout + else: + return stderr def expect_success(self, *args): return self._do_test(*args) @@ -1470,7 +1405,7 @@ def test_cli_force(self): Computed: 2ed19 Suggested fix: remove all generated code including the end marker, or use the '-f' option. - """) + """).strip() 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: @@ -1630,7 +1565,7 @@ def test_cli_fail_converters_and_filename(self): def test_cli_fail_no_filename(self): out = self.expect_failure() - self.assertIn("usage: clinic.py", out) + self.assertIn("no input files", out) def test_cli_fail_output_and_multiple_files(self): out = self.expect_failure("-o", "out.c", "input.c", "moreinput.c") @@ -2104,8 +2039,8 @@ def test_gh_99233_refcount(self): self.assertEqual(arg_refcount_origin, arg_refcount_after) def test_gh_99240_double_free(self): - expected_error = r'gh_99240_double_free\(\) argument 2 must be encoded string without null bytes, not str' - with self.assertRaisesRegex(TypeError, expected_error): + err = r'gh_99240_double_free\(\) argument 2 must be encoded string without null bytes, not str' + with self.assertRaisesRegex(TypeError, err): ac_tester.gh_99240_double_free('a', '\0b') def test_cloned_func_exception_message(self): diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index defe703b825b16..ae40cd14f8379f 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -7,6 +7,7 @@ from __future__ import annotations import abc +import argparse import ast import builtins as bltns import collections @@ -28,6 +29,7 @@ import sys import textwrap import traceback +import warnings from collections.abc import ( Callable, @@ -136,6 +138,24 @@ def text_accumulator() -> TextAccumulator: text, append, output = _text_accumulator() return TextAccumulator(append, output) + +class ClinicError(Exception): + def __init__( + self, + message: str, + /, + *, + lineno: int | None = None, + filename: str | None = None + ): + super().__init__(message) + self.lineno = lineno + self.filename = filename + + +class ClinicWarning(Warning): ... + + @overload def warn_or_fail( *args: object, @@ -159,25 +179,21 @@ def warn_or_fail( line_number: int | None = None, ) -> None: joined = " ".join([str(a) for a in args]) - add, output = text_accumulator() - if fail: - add("Error") - else: - add("Warning") if clinic: if filename is None: filename = clinic.filename if getattr(clinic, 'block_parser', None) and (line_number is None): line_number = clinic.block_parser.line_number - if filename is not None: - add(' in file "' + filename + '"') - if line_number is not None: - add(" on line " + str(line_number)) - add(':\n') - add(joined) - print(output()) if fail: - sys.exit(-1) + raise ClinicError(joined, lineno=line_number, filename=filename) + else: + msg = "Warning" + if filename is not None: + msg += f" in file {filename!r}" + if line_number is not None: + msg += f" on line {line_number}" + msg += f": {joined}" + warnings.warn(msg, ClinicWarning) def warn( @@ -5620,9 +5636,10 @@ def state_terminal(self, line: str | None) -> None: clinic = None -def main(argv: list[str]) -> None: - import sys - import argparse +class CLIError(Exception): ... + + +def create_cli() -> argparse.ArgumentParser: cmdline = argparse.ArgumentParser( description="""Preprocessor for CPython C files. @@ -5646,14 +5663,13 @@ def main(argv: list[str]) -> None: help="the directory tree to walk in --make mode") cmdline.add_argument("filename", metavar="FILE", type=str, nargs="*", help="the list of files to process") - ns = cmdline.parse_args(argv) + return cmdline + +def main(ns: argparse.Namespace) -> None: if ns.converters: if ns.filename: - print("Usage error: can't specify --converters and a filename at the same time.") - print() - cmdline.print_usage() - sys.exit(-1) + raise CLIError("can't specify --converters and a filename at the same time.") converters: list[tuple[str, str]] = [] return_converters: list[tuple[str, str]] = [] ignored = set(""" @@ -5707,19 +5723,13 @@ def main(argv: list[str]) -> None: print() print("All converters also accept (c_default=None, py_default=None, annotation=None).") print("All return converters also accept (py_default=None).") - sys.exit(0) + return if ns.make: if ns.output or ns.filename: - print("Usage error: can't use -o or filenames with --make.") - print() - cmdline.print_usage() - sys.exit(-1) + raise CLIError("can't use -o or filenames with --make.") if not ns.srcdir: - print("Usage error: --srcdir must not be empty with --make.") - print() - cmdline.print_usage() - sys.exit(-1) + raise CLIError("--srcdir must not be empty with --make.") for root, dirs, files in os.walk(ns.srcdir): for rcs_dir in ('.svn', '.git', '.hg', 'build', 'externals'): if rcs_dir in dirs: @@ -5735,14 +5745,10 @@ def main(argv: list[str]) -> None: return if not ns.filename: - cmdline.print_usage() - sys.exit(-1) + raise CLIError("no input files") if ns.output and len(ns.filename) > 1: - print("Usage error: can't use -o with multiple filenames.") - print() - cmdline.print_usage() - sys.exit(-1) + raise CLIError("can't use -o with multiple filenames.") for filename in ns.filename: if ns.verbose: @@ -5751,5 +5757,19 @@ def main(argv: list[str]) -> None: if __name__ == "__main__": - main(sys.argv[1:]) - sys.exit(0) + cli = create_cli() + try: + args = cli.parse_args() + main(args) + except CLIError as exc: + if msg := str(exc): + sys.stderr.write(f"Usage error: {msg}\n") + cli.print_usage() + sys.exit(1) + except ClinicError as exc: + msg = textwrap.dedent(f"""\ + Error in file {exc.filename!r} on line {exc.lineno}: + {exc} + """) + sys.stderr.write(str(exc)) + sys.exit(1) From 789f5ee39c5bc14e9227849ed48d1c2e330ef5d9 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 31 Jul 2023 14:31:05 +0200 Subject: [PATCH 02/24] Adapt new test --- Lib/test/test_clinic.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index 29bec2f8aa6da3..6f52722ac0c95b 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -293,7 +293,7 @@ def test_unknown_destination_command(self): self.expect_failure(raw, err) def test_no_access_to_members_in_converter_init(self): - out = self.expect_failure(""" + raw = """ /*[python input] class Custom_converter(CConverter): converter = "some_c_function" @@ -305,12 +305,12 @@ def converter_init(self): test.fn a: Custom [clinic start generated code]*/ - """) - msg = ( + """ + err = ( "Stepped on a land mine, trying to access attribute 'noaccess':\n" "Don't access members of self.function inside converter_init!" ) - self.assertIn(msg, out) + self.expect_failure(raw, err) class ClinicGroupPermuterTest(TestCase): From 4350eccd01f6933972adf0489818dad077f8dbf5 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Tue, 1 Aug 2023 23:58:27 +0200 Subject: [PATCH 03/24] Align with Alex's changes --- Tools/clinic/clinic.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index e6ce332262b5e8..66656fdf1caa0f 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -5749,7 +5749,6 @@ def main(argv: list[str] | None = None) -> NoReturn: args = parser.parse_args(argv) try: run_clinic(parser, args) - sys.exit(0) except ClinicError as exc: msg = textwrap.dedent(f"""\ Error in file {exc.filename!r} on line {exc.lineno}: @@ -5757,6 +5756,8 @@ def main(argv: list[str] | None = None) -> NoReturn: """) sys.stderr.write(str(exc)) sys.exit(1) + else: + sys.exit(0) if __name__ == "__main__": From ad5181aedfeb68091f492607561acd5afee0de2e Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 2 Aug 2023 00:59:31 +0200 Subject: [PATCH 04/24] Address review: pass iso. ellipse --- Tools/clinic/clinic.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 66656fdf1caa0f..0345798e94cdd3 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -153,7 +153,8 @@ def __init__( self.filename = filename -class ClinicWarning(Warning): ... +class ClinicWarning(Warning): + pass @overload From 545e7b641e4fbd3c4697f8f2d02e3045fab00b68 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 2 Aug 2023 01:01:35 +0200 Subject: [PATCH 05/24] Address review: add return annotation --- Tools/clinic/clinic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 0345798e94cdd3..bdc06a11199ade 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -147,7 +147,7 @@ def __init__( *, lineno: int | None = None, filename: str | None = None - ): + ) -> None: super().__init__(message) self.lineno = lineno self.filename = filename From c978a6d5513e8f4226e453737348cbc7d42b10d4 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 2 Aug 2023 01:06:08 +0200 Subject: [PATCH 06/24] Address review: re.escape --- Lib/test/test_clinic.py | 48 ++++++++++++++++++++++------------------- 1 file changed, 26 insertions(+), 22 deletions(-) diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index ad1d2080f1f591..8a76f424beda98 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -10,6 +10,7 @@ import collections import inspect import os.path +import re import sys import unittest @@ -136,9 +137,9 @@ def test_mangled_marker_line(self): [clinic start generated code]*/ /*[clinic end generated code: foo]*/ """ - err = ( + err = re.escape( "Mangled Argument Clinic marker line: " - "'\/\*\[clinic end generated code: foo\]\*\/'" + "'/*[clinic end generated code: foo]*/'" ) self.expect_failure(raw, err, filename="test.c", lineno=3) @@ -168,9 +169,9 @@ def test_whitespace_before_stop_line(self): /*[clinic input] [clinic start generated code]*/ """ - err = ( + err = re.escape( "Whitespace is not allowed before the stop line: " - "' \[clinic start generated code\]\*\/'" + "' [clinic start generated code]*/'" ) self.expect_failure(raw, err, filename="test.c", lineno=2) @@ -576,8 +577,8 @@ def test_param_default_expression(self): self.assertEqual(sys.maxsize, p.default) self.assertEqual("MAXSIZE", p.converter.c_default) - err = ( - "When you specify a named constant \('sys.maxsize'\) as your default value,\n" + err = re.escape( + "When you specify a named constant ('sys.maxsize') as your default value,\n" "you MUST specify a valid c_default." ) block = """ @@ -600,8 +601,8 @@ def test_param_no_docstring(self): self.assertIsInstance(conv, clinic.str_converter) def test_param_default_parameters_out_of_order(self): - err = ( - "Can't have a parameter without a default \('something_else'\)\n" + err = re.escape( + "Can't have a parameter without a default ('something_else')\n" "after a parameter with a default!" ) block = """ @@ -869,9 +870,9 @@ def test_nested_groups(self): """) def test_disallowed_grouping__two_top_groups_on_left(self): - err = ( + err = re.escape( 'Function two_top_groups_on_left has an unsupported group ' - 'configuration. \(Unexpected state 2.b\)' + 'configuration. (Unexpected state 2.b)' ) block = """ module foo @@ -898,9 +899,9 @@ def test_disallowed_grouping__two_top_groups_on_right(self): group2 : int ] """ - err = ( + err = re.escape( "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) @@ -916,9 +917,9 @@ def test_disallowed_grouping__parameter_after_group_on_right(self): group2 : int ] """ - err = ( + err = re.escape( "Function parameter_after_group_on_right has an unsupported group " - "configuration. \(Unexpected state 6.a\)" + "configuration. (Unexpected state 6.a)" ) self.expect_failure(block, err) @@ -934,9 +935,9 @@ def test_disallowed_grouping__group_after_parameter_on_left(self): ] param: int """ - err = ( + err = re.escape( "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) @@ -983,7 +984,7 @@ def test_disallowed_grouping__no_matching_bracket(self): group2: int ] """ - err = "Function empty_group has a \] without a matching \[." + err = re.escape("Function empty_group has a ] without a matching [.") self.expect_failure(block, err) def test_no_parameters(self): @@ -1059,9 +1060,9 @@ def test_single_slash(self): / / """ - err = ( + err = re.escape( "Function bar has an unsupported group configuration. " - "\(Unexpected state 0.d\)" + "(Unexpected state 0.d)" ) self.expect_failure(block, err) @@ -1100,9 +1101,9 @@ def test_parameters_not_permitted_after_slash_for_now(self): / x: int """ - err = ( + err = re.escape( "Function bar has an unsupported group configuration. " - "\(Unexpected state 0.d\)" + "(Unexpected state 0.d)" ) self.expect_failure(block, err) @@ -2048,7 +2049,10 @@ def test_gh_99233_refcount(self): self.assertEqual(arg_refcount_origin, arg_refcount_after) def test_gh_99240_double_free(self): - err = r'gh_99240_double_free\(\) argument 2 must be encoded string without null bytes, not str' + err = re.escape( + "gh_99240_double_free() argument 2 must be encoded string " + "without null bytes, not str" + ) with self.assertRaisesRegex(TypeError, err): ac_tester.gh_99240_double_free('a', '\0b') From 33b038c13964fc2e0ee98250e59e44f919d3d07a Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 2 Aug 2023 01:13:55 +0200 Subject: [PATCH 07/24] Address review: fix test --- Lib/test/test_clinic.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index 8a76f424beda98..25edd2c1ec8dd4 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -693,7 +693,14 @@ def test_cloning_nonexistent_function_correctly_fails(self): This is trying to clone a nonexistent function!! """ err = "Couldn't find existing function 'fooooooooooooooooo'!" - self.expect_failure(block, err, lineno=0) + with support.captured_stdout() as out: + self.expect_failure(block, err, lineno=0) + expected_debug_print = dedent("""\ + cls=None, module=, existing='fooooooooooooooooo' + (cls or module).functions=[] + """) + out = out.getvalue() + self.assertIn(expected_debug_print, out) def test_return_converter(self): function = self.parse_function(""" From 7b631a32d4144a4373d450ab6e6174799aaa563b Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 2 Aug 2023 01:46:05 +0200 Subject: [PATCH 08/24] Refactor: extract method parse_parameter_name() --- Tools/clinic/clinic.py | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index bdc06a11199ade..4d3b6266c0f20b 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -4927,6 +4927,24 @@ def state_parameter(self, line: str) -> None: case param: self.parse_parameter(param) + def parse_parameter_name(self, line: str) -> tuple[str, str, str]: + c_name = None + # Handle custom parameter names. + name, have_as_token, trailing = line.partition(' as ') + if have_as_token: + name = name.strip() + if ' ' not in name: + fields = trailing.strip().split(' ') + if not fields: + fail("Invalid 'as' clause!") + c_name = fields[0] + if c_name.endswith(':'): + name += ':' + c_name = c_name[:-1] + fields[0] = name + line = ' '.join(fields) + return c_name, name, line + def parse_parameter(self, line: str) -> None: assert self.function is not None @@ -4943,21 +4961,7 @@ def parse_parameter(self, line: str) -> None: case st: fail(f"Function {self.function.name} has an unsupported group configuration. (Unexpected state {st}.a)") - # handle "as" for parameters too - c_name = None - name, have_as_token, trailing = line.partition(' as ') - if have_as_token: - name = name.strip() - if ' ' not in name: - fields = trailing.strip().split(' ') - if not fields: - fail("Invalid 'as' clause!") - c_name = fields[0] - if c_name.endswith(':'): - name += ':' - c_name = c_name[:-1] - fields[0] = name - line = ' '.join(fields) + c_name, name, line = self.parse_parameter_name(line) default: str | None base, equals, default = line.rpartition('=') From 553cb6eb6c1e3cc9642771e9cb8a509b0da34a16 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 2 Aug 2023 02:03:56 +0200 Subject: [PATCH 09/24] Address review: re.escape better --- Lib/test/test_clinic.py | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index 25edd2c1ec8dd4..2a402671d7ea24 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -21,9 +21,10 @@ def _expect_failure(tc, parser, code, errmsg, *, filename=None, lineno=None): - _code = dedent(code).strip() + code = dedent(code).strip() + errmsg = re.escape(errmsg) with tc.assertRaisesRegex(clinic.ClinicError, errmsg) as cm: - parser(_code) + parser(code) if filename is not None: tc.assertEqual(cm.exception.filename, filename) if lineno is not None: @@ -137,7 +138,7 @@ def test_mangled_marker_line(self): [clinic start generated code]*/ /*[clinic end generated code: foo]*/ """ - err = re.escape( + err = ( "Mangled Argument Clinic marker line: " "'/*[clinic end generated code: foo]*/'" ) @@ -169,7 +170,7 @@ def test_whitespace_before_stop_line(self): /*[clinic input] [clinic start generated code]*/ """ - err = re.escape( + err = ( "Whitespace is not allowed before the stop line: " "' [clinic start generated code]*/'" ) @@ -577,7 +578,7 @@ def test_param_default_expression(self): self.assertEqual(sys.maxsize, p.default) self.assertEqual("MAXSIZE", p.converter.c_default) - err = re.escape( + err = ( "When you specify a named constant ('sys.maxsize') as your default value,\n" "you MUST specify a valid c_default." ) @@ -601,7 +602,7 @@ def test_param_no_docstring(self): self.assertIsInstance(conv, clinic.str_converter) def test_param_default_parameters_out_of_order(self): - err = re.escape( + err = ( "Can't have a parameter without a default ('something_else')\n" "after a parameter with a default!" ) @@ -877,7 +878,7 @@ def test_nested_groups(self): """) def test_disallowed_grouping__two_top_groups_on_left(self): - err = re.escape( + err = ( 'Function two_top_groups_on_left has an unsupported group ' 'configuration. (Unexpected state 2.b)' ) @@ -906,7 +907,7 @@ def test_disallowed_grouping__two_top_groups_on_right(self): group2 : int ] """ - err = re.escape( + err = ( "Function two_top_groups_on_right has an unsupported group " "configuration. (Unexpected state 6.b)" ) @@ -924,7 +925,7 @@ def test_disallowed_grouping__parameter_after_group_on_right(self): group2 : int ] """ - err = re.escape( + err = ( "Function parameter_after_group_on_right has an unsupported group " "configuration. (Unexpected state 6.a)" ) @@ -942,7 +943,7 @@ def test_disallowed_grouping__group_after_parameter_on_left(self): ] param: int """ - err = re.escape( + err = ( "Function group_after_parameter_on_left has an unsupported group " "configuration. (Unexpected state 2.b)" ) @@ -991,7 +992,7 @@ def test_disallowed_grouping__no_matching_bracket(self): group2: int ] """ - err = re.escape("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): @@ -1045,7 +1046,7 @@ def test_single_star(self): * * """ - 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): @@ -1055,7 +1056,7 @@ def test_parameters_required_after_star(self): "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) @@ -1067,7 +1068,7 @@ def test_single_slash(self): / / """ - err = re.escape( + err = ( "Function bar has an unsupported group configuration. " "(Unexpected state 0.d)" ) @@ -1108,7 +1109,7 @@ def test_parameters_not_permitted_after_slash_for_now(self): / x: int """ - err = re.escape( + err = ( "Function bar has an unsupported group configuration. " "(Unexpected state 0.d)" ) From 1514f631f37f5f51d53f2bb094bb11fbda085686 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 2 Aug 2023 02:06:38 +0200 Subject: [PATCH 10/24] Address review: UserWarning iso. Warning --- Tools/clinic/clinic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 4d3b6266c0f20b..3525e75e230c16 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -153,7 +153,7 @@ def __init__( self.filename = filename -class ClinicWarning(Warning): +class ClinicWarning(UserWarning): pass From d3daef7633fa07967be001c4dfdffcc292771cc0 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 2 Aug 2023 02:09:09 +0200 Subject: [PATCH 11/24] Revert "Refactor: extract method parse_parameter_name()" This reverts commit 7b631a32d4144a4373d450ab6e6174799aaa563b. --- Tools/clinic/clinic.py | 34 +++++++++++++++------------------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 3525e75e230c16..2837eab70dec02 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -4927,24 +4927,6 @@ def state_parameter(self, line: str) -> None: case param: self.parse_parameter(param) - def parse_parameter_name(self, line: str) -> tuple[str, str, str]: - c_name = None - # Handle custom parameter names. - name, have_as_token, trailing = line.partition(' as ') - if have_as_token: - name = name.strip() - if ' ' not in name: - fields = trailing.strip().split(' ') - if not fields: - fail("Invalid 'as' clause!") - c_name = fields[0] - if c_name.endswith(':'): - name += ':' - c_name = c_name[:-1] - fields[0] = name - line = ' '.join(fields) - return c_name, name, line - def parse_parameter(self, line: str) -> None: assert self.function is not None @@ -4961,7 +4943,21 @@ def parse_parameter(self, line: str) -> None: case st: fail(f"Function {self.function.name} has an unsupported group configuration. (Unexpected state {st}.a)") - c_name, name, line = self.parse_parameter_name(line) + # handle "as" for parameters too + c_name = None + name, have_as_token, trailing = line.partition(' as ') + if have_as_token: + name = name.strip() + if ' ' not in name: + fields = trailing.strip().split(' ') + if not fields: + fail("Invalid 'as' clause!") + c_name = fields[0] + if c_name.endswith(':'): + name += ':' + c_name = c_name[:-1] + fields[0] = name + line = ' '.join(fields) default: str | None base, equals, default = line.rpartition('=') From 8f3750bf023cd6b8f557f497595f4de5297d8267 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Wed, 2 Aug 2023 11:45:26 +0200 Subject: [PATCH 12/24] Print debug stuff to stderr --- Tools/clinic/clinic.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 9c0672cc6b53bc..8680339fc5afa9 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -4722,8 +4722,8 @@ def state_modulename_name(self, line: str) -> None: if existing_function.name == function_name: break else: - print(f"{cls=}, {module=}, {existing=}") - print(f"{(cls or module).functions=}") + print(f"{cls=}, {module=}, {existing=}", file=sys.stderr) + print(f"{(cls or module).functions=}", file=sys.stderr) fail(f"Couldn't find existing function {existing!r}!") fields = [x.strip() for x in full_name.split('.')] From e8b8d7e482887810d564269d027f57145ad9b612 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 2 Aug 2023 12:02:49 +0200 Subject: [PATCH 13/24] Adapt test --- Lib/test/test_clinic.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index 0ae69c81867265..dc5ddde8616674 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -716,14 +716,14 @@ def test_cloning_nonexistent_function_correctly_fails(self): This is trying to clone a nonexistent function!! """ err = "Couldn't find existing function 'fooooooooooooooooo'!" - with support.captured_stdout() as out: + with support.captured_stderr() as stderr: self.expect_failure(block, err, lineno=0) expected_debug_print = dedent("""\ cls=None, module=, existing='fooooooooooooooooo' (cls or module).functions=[] """) - out = out.getvalue() - self.assertIn(expected_debug_print, out) + stderr = stderr.getvalue() + self.assertIn(expected_debug_print, stderr) def test_return_converter(self): function = self.parse_function(""" From 79b9859bfbffc5bbc29c6d5f4361c1a19dc12eca Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 2 Aug 2023 12:06:36 +0200 Subject: [PATCH 14/24] Print to stdout instead of using warnings.warn --- Tools/clinic/clinic.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 8680339fc5afa9..de621b1505257c 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -29,7 +29,6 @@ import sys import textwrap import traceback -import warnings from collections.abc import ( Callable, @@ -153,10 +152,6 @@ def __init__( self.filename = filename -class ClinicWarning(UserWarning): - pass - - @overload def warn_or_fail( *args: object, @@ -194,7 +189,7 @@ def warn_or_fail( if line_number is not None: msg += f" on line {line_number}" msg += f": {joined}" - warnings.warn(msg, ClinicWarning) + print(msg) def warn( From 57eff9d64fc82fb82856816b66074633f96c0243 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 2 Aug 2023 15:47:19 +0200 Subject: [PATCH 15/24] Final touches --- Lib/test/clinic.test.c | 260 ++++++++++++++++++++++++++++++++++++++++ Lib/test/test_clinic.py | 93 +++++++++++++- Tools/clinic/clinic.py | 28 ++--- 3 files changed, 365 insertions(+), 16 deletions(-) diff --git a/Lib/test/clinic.test.c b/Lib/test/clinic.test.c index a660ccf8876e2d..a49c2e77810648 100644 --- a/Lib/test/clinic.test.c +++ b/Lib/test/clinic.test.c @@ -5004,3 +5004,263 @@ PyDoc_STRVAR(new_dest__doc__, "\n" "Only this docstring should be outputted to test1."); /*[clinic end generated code: output=9cac703f51d90e84 input=090db8df4945576d]*/ + + +/*[clinic input] +mangled_c_keyword_identifier + i as int: int +The 'int' param should be mangled as 'int_value' +[clinic start generated code]*/ + +PyDoc_STRVAR(mangled_c_keyword_identifier__doc__, +"mangled_c_keyword_identifier($module, /, i)\n" +"--\n" +"\n" +"The \'int\' param should be mangled as \'int_value\'"); + +#define MANGLED_C_KEYWORD_IDENTIFIER_METHODDEF \ + {"mangled_c_keyword_identifier", _PyCFunction_CAST(mangled_c_keyword_identifier), METH_FASTCALL|METH_KEYWORDS, mangled_c_keyword_identifier__doc__}, + +static PyObject * +mangled_c_keyword_identifier_impl(PyObject *module, int int_value); + +static PyObject * +mangled_c_keyword_identifier(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames) +{ + PyObject *return_value = NULL; + #if defined(Py_BUILD_CORE) && !defined(Py_BUILD_CORE_MODULE) + + #define NUM_KEYWORDS 1 + static struct { + PyGC_Head _this_is_not_used; + PyObject_VAR_HEAD + PyObject *ob_item[NUM_KEYWORDS]; + } _kwtuple = { + .ob_base = PyVarObject_HEAD_INIT(&PyTuple_Type, NUM_KEYWORDS) + .ob_item = { &_Py_ID(i), }, + }; + #undef NUM_KEYWORDS + #define KWTUPLE (&_kwtuple.ob_base.ob_base) + + #else // !Py_BUILD_CORE + # define KWTUPLE NULL + #endif // !Py_BUILD_CORE + + static const char * const _keywords[] = {"i", NULL}; + static _PyArg_Parser _parser = { + .keywords = _keywords, + .fname = "mangled_c_keyword_identifier", + .kwtuple = KWTUPLE, + }; + #undef KWTUPLE + PyObject *argsbuf[1]; + int int_value; + + args = _PyArg_UnpackKeywords(args, nargs, NULL, kwnames, &_parser, 1, 1, 0, argsbuf); + if (!args) { + goto exit; + } + int_value = _PyLong_AsInt(args[0]); + if (int_value == -1 && PyErr_Occurred()) { + goto exit; + } + return_value = mangled_c_keyword_identifier_impl(module, int_value); + +exit: + return return_value; +} + +static PyObject * +mangled_c_keyword_identifier_impl(PyObject *module, int int_value) +/*[clinic end generated code: output=c049d7d79be26cda input=060876448ab567a2]*/ + + +/*[clinic input] +bool_return -> bool +[clinic start generated code]*/ + +PyDoc_STRVAR(bool_return__doc__, +"bool_return($module, /)\n" +"--\n" +"\n"); + +#define BOOL_RETURN_METHODDEF \ + {"bool_return", (PyCFunction)bool_return, METH_NOARGS, bool_return__doc__}, + +static int +bool_return_impl(PyObject *module); + +static PyObject * +bool_return(PyObject *module, PyObject *Py_UNUSED(ignored)) +{ + PyObject *return_value = NULL; + int _return_value; + + _return_value = bool_return_impl(module); + if ((_return_value == -1) && PyErr_Occurred()) { + goto exit; + } + return_value = PyBool_FromLong((long)_return_value); + +exit: + return return_value; +} + +static int +bool_return_impl(PyObject *module) +/*[clinic end generated code: output=3a65f07830e48e98 input=93ba95d39ee98f39]*/ + + +/*[clinic input] +double_return -> double +[clinic start generated code]*/ + +PyDoc_STRVAR(double_return__doc__, +"double_return($module, /)\n" +"--\n" +"\n"); + +#define DOUBLE_RETURN_METHODDEF \ + {"double_return", (PyCFunction)double_return, METH_NOARGS, double_return__doc__}, + +static double +double_return_impl(PyObject *module); + +static PyObject * +double_return(PyObject *module, PyObject *Py_UNUSED(ignored)) +{ + PyObject *return_value = NULL; + double _return_value; + + _return_value = double_return_impl(module); + if ((_return_value == -1.0) && PyErr_Occurred()) { + goto exit; + } + return_value = PyFloat_FromDouble(_return_value); + +exit: + return return_value; +} + +static double +double_return_impl(PyObject *module) +/*[clinic end generated code: output=076dc72595d3f66d input=da11b6255e4cbfd7]*/ + + +/*[clinic input] +Test.__init__ + a: object + [ + b: object + ] + / +Should generate two PyArg_ParseTuple calls. +[clinic start generated code]*/ + +PyDoc_STRVAR(Test___init____doc__, +"Test(a, [b])\n" +"Should generate two PyArg_ParseTuple calls."); + +static int +Test___init___impl(TestObj *self, PyObject *a, int group_right_1, + PyObject *b); + +static int +Test___init__(PyObject *self, PyObject *args, PyObject *kwargs) +{ + int return_value = -1; + PyTypeObject *base_tp = TestType; + PyObject *a; + int group_right_1 = 0; + PyObject *b = NULL; + + if ((Py_IS_TYPE(self, base_tp) || + Py_TYPE(self)->tp_new == base_tp->tp_new) && + !_PyArg_NoKeywords("Test", kwargs)) { + goto exit; + } + switch (PyTuple_GET_SIZE(args)) { + case 1: + if (!PyArg_ParseTuple(args, "O:__init__", &a)) { + goto exit; + } + break; + case 2: + if (!PyArg_ParseTuple(args, "OO:__init__", &a, &b)) { + goto exit; + } + group_right_1 = 1; + break; + default: + PyErr_SetString(PyExc_TypeError, "Test.__init__ requires 1 to 2 arguments"); + goto exit; + } + return_value = Test___init___impl((TestObj *)self, a, group_right_1, b); + +exit: + return return_value; +} + +static int +Test___init___impl(TestObj *self, PyObject *a, int group_right_1, + PyObject *b) +/*[clinic end generated code: output=2bbb8ea60e8f57a6 input=10f5d0f1e8e466ef]*/ + + +/*[clinic input] +Test._pyarg_parsestackandkeywords + cls: defining_class + key: str(accept={str, robuffer}, zeroes=True) + / +Check that _PyArg_ParseStackAndKeywords() is generated. +[clinic start generated code]*/ + +PyDoc_STRVAR(Test__pyarg_parsestackandkeywords__doc__, +"_pyarg_parsestackandkeywords($self, key, /)\n" +"--\n" +"\n" +"Check that _PyArg_ParseStackAndKeywords() is generated."); + +#define TEST__PYARG_PARSESTACKANDKEYWORDS_METHODDEF \ + {"_pyarg_parsestackandkeywords", _PyCFunction_CAST(Test__pyarg_parsestackandkeywords), METH_METHOD|METH_FASTCALL|METH_KEYWORDS, Test__pyarg_parsestackandkeywords__doc__}, + +static PyObject * +Test__pyarg_parsestackandkeywords_impl(TestObj *self, PyTypeObject *cls, + const char *key, + Py_ssize_t key_length); + +static PyObject * +Test__pyarg_parsestackandkeywords(TestObj *self, PyTypeObject *cls, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames) +{ + PyObject *return_value = NULL; + #if defined(Py_BUILD_CORE) && !defined(Py_BUILD_CORE_MODULE) + # define KWTUPLE (PyObject *)&_Py_SINGLETON(tuple_empty) + #else + # define KWTUPLE NULL + #endif + + static const char * const _keywords[] = {"", NULL}; + static _PyArg_Parser _parser = { + .keywords = _keywords, + .format = "s#:_pyarg_parsestackandkeywords", + .kwtuple = KWTUPLE, + }; + #undef KWTUPLE + const char *key; + Py_ssize_t key_length; + + if (!_PyArg_ParseStackAndKeywords(args, nargs, kwnames, &_parser, + &key, &key_length)) { + goto exit; + } + return_value = Test__pyarg_parsestackandkeywords_impl(self, cls, key, key_length); + +exit: + return return_value; +} + +static PyObject * +Test__pyarg_parsestackandkeywords_impl(TestObj *self, PyTypeObject *cls, + const char *key, + Py_ssize_t key_length) +/*[clinic end generated code: output=4fda8a7f2547137c input=fc72ef4b4cfafabc]*/ diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index dc5ddde8616674..27efdfbe81553b 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -8,6 +8,7 @@ from textwrap import dedent from unittest import TestCase import collections +import contextlib import inspect import os.path import re @@ -311,6 +312,70 @@ def converter_init(self): ) self.expect_failure(raw, err) + @staticmethod + @contextlib.contextmanager + def _clinic_version(new_version): + """Helper for test_version_*() tests""" + _saved = clinic.version + clinic.version = new_version + try: + yield + finally: + clinic.version = _saved + + def test_version_directive(self): + dataset = ( + # (clinic version, required version) + ('3', '2'), # required version < clinic version + ('3.1', '3.0'), # required version < clinic version + ('1.2b0', '1.2a7'), # required version < clinic version + ('5', '5'), # required version == clinic version + ('6.1', '6.1'), # required version == clinic version + ('1.2b3', '1.2b3'), # required version == clinic version + ) + for clinic_version, required_version in dataset: + with self.subTest(clinic_version=clinic_version, + required_version=required_version): + with self._clinic_version(clinic_version): + block = dedent(f""" + /*[clinic input] + version {required_version} + [clinic start generated code]*/ + """) + self.clinic.parse(block) + + def test_version_directive_insufficient_version(self): + with self._clinic_version('4'): + err = ( + "Insufficient Clinic version!\n" + " Version: 4\n" + " Required: 5" + ) + block = """ + /*[clinic input] + version 5 + [clinic start generated code]*/ + """ + self.expect_failure(block, err) + + def test_version_directive_illegal_char(self): + err = "Illegal character 'v' in version string 'v5'" + block = """ + /*[clinic input] + version v5 + [clinic start generated code]*/ + """ + self.expect_failure(block, err) + + def test_version_directive_unsupported_string(self): + err = "Unsupported version string: '.-'" + block = """ + /*[clinic input] + version .- + [clinic start generated code]*/ + """ + self.expect_failure(block, err) + class ClinicGroupPermuterTest(TestCase): def _test(self, l, m, r, output): @@ -1369,6 +1434,32 @@ def test_scaffolding(self): self.assertEqual(exc.filename, 'clown.txt') self.assertEqual(exc.lineno, 69) + def test_non_ascii_character_in_docstring(self): + block = """ + module test + test.fn + a: int + á param docstring + docstring fü bár baß + """ + with support.captured_stdout() as stdout: + self.parse(block) + # The line numbers are off; this is a known limitation. + expected = dedent("""\ + Warning on line 0: Non-ascii characters are not allowed in docstrings: 'á' + Warning on line 0: Non-ascii characters are not allowed in docstrings: 'ü', 'á', 'ß' + """) + self.assertEqual(stdout.getvalue(), expected) + + def test_illegal_c_identifier(self): + err = "Illegal C identifier: 17a" + block = """ + module test + test.fn + a as 17a: int + """ + self.expect_failure(block, err) + class ClinicExternalTest(TestCase): maxDiff = None @@ -1447,7 +1538,7 @@ def test_cli_force(self): Computed: 2ed19 Suggested fix: remove all generated code including the end marker, or use the '-f' option. - """).strip() + """) 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: diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index de621b1505257c..d915e2fc4aff69 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -358,7 +358,7 @@ def version_splitter(s: str) -> tuple[int, ...]: accumulator: list[str] = [] def flush() -> None: if not accumulator: - raise ValueError('Unsupported version string: ' + repr(s)) + fail(f'Unsupported version string: {s!r}') version.append(int(''.join(accumulator))) accumulator.clear() @@ -371,7 +371,7 @@ def flush() -> None: flush() version.append('abc'.index(c) - 3) else: - raise ValueError('Illegal character ' + repr(c) + ' in version string ' + repr(s)) + fail(f'Illegal character {c!r} in version string {s!r}') flush() return tuple(version) @@ -796,9 +796,6 @@ def docstring_for_c_string( self, f: Function ) -> str: - if re.search(r'[^\x00-\x7F]', f.docstring): - warn("Non-ascii character appear in docstring.") - text, add, output = _text_accumulator() # turn docstring into a properly quoted C string for line in f.docstring.split('\n'): @@ -2249,11 +2246,7 @@ def parse(self, input: str) -> str: assert dsl_name in parsers, f"No parser to handle {dsl_name!r} block." self.parsers[dsl_name] = parsers[dsl_name](self) parser = self.parsers[dsl_name] - try: - parser.parse(block) - except Exception: - fail('Exception raised during parsing:\n' + - traceback.format_exc().rstrip()) + parser.parse(block) printer.print_block(block) # these are destinations not buffers @@ -5277,6 +5270,11 @@ def state_parameter_docstring_start(self, line: str) -> None: def docstring_append(self, obj: Function | Parameter, line: str) -> None: """Add a rstripped line to the current docstring.""" + matches = re.finditer(r'[^\x00-\x7F]', line) + if offending := ", ".join([repr(m[0]) for m in matches]): + warn("Non-ascii characters are not allowed in docstrings:", + offending) + docstring = obj.docstring if docstring: docstring += "\n" @@ -5733,11 +5731,11 @@ def main(argv: list[str] | None = None) -> NoReturn: try: run_clinic(parser, args) except ClinicError as exc: - msg = textwrap.dedent(f"""\ - Error in file {exc.filename!r} on line {exc.lineno}: - {exc} - """) - sys.stderr.write(str(exc)) + msg = ( + f"Error in file {exc.filename!r} on line {exc.lineno}:\n" + f"{exc}\n" + ) + sys.stderr.write(msg) sys.exit(1) else: sys.exit(0) From 6f5cfae5086931cf9cc9495b9c2235570943ab2c Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 2 Aug 2023 15:50:28 +0200 Subject: [PATCH 16/24] Fix merge --- Lib/test/test_clinic.py | 92 ----------------------------------------- 1 file changed, 92 deletions(-) diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index 3981755d99d21a..27efdfbe81553b 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -376,70 +376,6 @@ def test_version_directive_unsupported_string(self): """ self.expect_failure(block, err) - @staticmethod - @contextlib.contextmanager - def _clinic_version(new_version): - """Helper for test_version_*() tests""" - _saved = clinic.version - clinic.version = new_version - try: - yield - finally: - clinic.version = _saved - - def test_version_directive(self): - dataset = ( - # (clinic version, required version) - ('3', '2'), # required version < clinic version - ('3.1', '3.0'), # required version < clinic version - ('1.2b0', '1.2a7'), # required version < clinic version - ('5', '5'), # required version == clinic version - ('6.1', '6.1'), # required version == clinic version - ('1.2b3', '1.2b3'), # required version == clinic version - ) - for clinic_version, required_version in dataset: - with self.subTest(clinic_version=clinic_version, - required_version=required_version): - with self._clinic_version(clinic_version): - block = dedent(f""" - /*[clinic input] - version {required_version} - [clinic start generated code]*/ - """) - self.clinic.parse(block) - - def test_version_directive_insufficient_version(self): - with self._clinic_version('4'): - err = ( - "Insufficient Clinic version!\n" - " Version: 4\n" - " Required: 5" - ) - out = self.expect_failure(""" - /*[clinic input] - version 5 - [clinic start generated code]*/ - """) - self.assertIn(err, out) - - def test_version_directive_illegal_char(self): - err = "Illegal character 'v' in version string 'v5'" - out = self.expect_failure(""" - /*[clinic input] - version v5 - [clinic start generated code]*/ - """) - self.assertIn(err, out) - - def test_version_directive_unsupported_string(self): - err = "Unsupported version string: '.-'" - out = self.expect_failure(""" - /*[clinic input] - version .- - [clinic start generated code]*/ - """) - self.assertIn(err, out) - class ClinicGroupPermuterTest(TestCase): def _test(self, l, m, r, output): @@ -1524,34 +1460,6 @@ def test_illegal_c_identifier(self): """ self.expect_failure(block, err) - def test_non_ascii_character_in_docstring(self): - block = """ - module test - test.fn - a: int - á param docstring - docstring fü bár baß - """ - with support.captured_stdout() as stdout: - self.parse(block) - # The line numbers are off; this is a known limitation. - expected = dedent("""\ - Warning on line 0: - Non-ascii characters are not allowed in docstrings: 'á' - Warning on line 0: - Non-ascii characters are not allowed in docstrings: 'ü', 'á', 'ß' - """) - self.assertEqual(stdout.getvalue(), expected) - - def test_illegal_c_identifier(self): - err = "Illegal C identifier: 17a" - out = self.parse_function_should_fail(""" - module test - test.fn - a as 17a: int - """) - self.assertIn(err, out) - class ClinicExternalTest(TestCase): maxDiff = None From 3081729114607c2dec9dc5b0b1e823dc732db5e0 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 2 Aug 2023 16:03:57 +0200 Subject: [PATCH 17/24] Try to produce correct line numbers in error messages --- Lib/test/test_clinic.py | 26 +++++++++++++------------- Tools/clinic/clinic.py | 5 ++++- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index 27efdfbe81553b..cca849190f150c 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -652,7 +652,7 @@ def test_param_default_expression(self): os.access follow_symlinks: int = sys.maxsize """ - self.expect_failure(block, err, lineno=0) + self.expect_failure(block, err, lineno=3) def test_param_no_docstring(self): function = self.parse_function(""" @@ -677,7 +677,7 @@ def test_param_default_parameters_out_of_order(self): follow_symlinks: bool = True something_else: str """ - self.expect_failure(block, err, lineno=0) + self.expect_failure(block, err, lineno=4) def disabled_test_converter_arguments(self): function = self.parse_function(""" @@ -782,7 +782,7 @@ def test_cloning_nonexistent_function_correctly_fails(self): """ err = "Couldn't find existing function 'fooooooooooooooooo'!" with support.captured_stderr() as stderr: - self.expect_failure(block, err, lineno=0) + self.expect_failure(block, err, lineno=1) expected_debug_print = dedent("""\ cls=None, module=, existing='fooooooooooooooooo' (cls or module).functions=[] @@ -980,7 +980,7 @@ def test_disallowed_grouping__two_top_groups_on_left(self): ] param: int """ - self.expect_failure(block, err, lineno=0) + self.expect_failure(block, err, lineno=6) def test_disallowed_grouping__two_top_groups_on_right(self): block = """ @@ -1277,7 +1277,7 @@ def test_legacy_converters_non_string_constant_annotation(self): ) for block in dataset: with self.subTest(block=block): - self.expect_failure(block, err, lineno=0) + self.expect_failure(block, err, lineno=3) def test_other_bizarre_things_in_annotations_fail(self): err = "Annotations must be either a name, a function call, or a string" @@ -1288,7 +1288,7 @@ def test_other_bizarre_things_in_annotations_fail(self): ) for block in dataset: with self.subTest(block=block): - self.expect_failure(block, err, lineno=0) + self.expect_failure(block, err, lineno=3) def test_kwarg_splats_disallowed_in_function_call_annotations(self): err = "Cannot use a kwarg splat in a function-call annotation" @@ -1300,7 +1300,7 @@ def test_kwarg_splats_disallowed_in_function_call_annotations(self): ) for block in dataset: with self.subTest(block=block): - self.expect_failure(block, err, lineno=0) + self.expect_failure(block, err) def test_self_param_placement(self): expected_error_msg = ( @@ -1313,7 +1313,7 @@ def test_self_param_placement(self): a: int self: self(type="PyObject *") """ - self.expect_failure(block, expected_error_msg, lineno=0) + self.expect_failure(block, expected_error_msg, lineno=4) def test_self_param_cannot_be_optional(self): err = "A 'self' parameter cannot be marked optional." @@ -1322,7 +1322,7 @@ def test_self_param_cannot_be_optional(self): foo.func self: self(type="PyObject *") = None """ - self.expect_failure(block, err, lineno=0) + self.expect_failure(block, err, lineno=3) def test_defining_class_param_placement(self): expected_error_msg = ( @@ -1336,7 +1336,7 @@ def test_defining_class_param_placement(self): a: int cls: defining_class """ - self.expect_failure(block, expected_error_msg, lineno=0) + self.expect_failure(block, expected_error_msg, lineno=5) def test_defining_class_param_cannot_be_optional(self): err = "A 'defining_class' parameter cannot be marked optional." @@ -1345,7 +1345,7 @@ def test_defining_class_param_cannot_be_optional(self): foo.func cls: defining_class(type="PyObject *") = None """ - self.expect_failure(block, err, lineno=0) + self.expect_failure(block, err, lineno=3) def test_slot_methods_cannot_access_defining_class(self): block = """ @@ -1366,7 +1366,7 @@ def test_new_must_be_a_class_method(self): class Foo "" "" Foo.__new__ """ - self.expect_failure(block, err, lineno=0) + self.expect_failure(block, err, lineno=3) def test_init_must_be_a_normal_method(self): err = "__init__ must be a normal method, not a class or static method!" @@ -1376,7 +1376,7 @@ class Foo "" "" @classmethod Foo.__init__ """ - self.expect_failure(block, err, lineno=0) + self.expect_failure(block, err, lineno=4) def test_unused_param(self): block = self.parse(""" diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index d915e2fc4aff69..fefc87aa15602e 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -4609,7 +4609,10 @@ def parse(self, block: Block) -> None: 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) - self.state(line) + try: + self.state(line) + except ClinicError as exc: + raise ClinicError(str(exc), lineno=line_number+1) self.do_post_block_processing_cleanup() block.output.extend(self.clinic.language.render(self.clinic, block.signatures)) From 716bda819b4f5552dd734c6b25f9c294910737df Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 2 Aug 2023 16:09:24 +0200 Subject: [PATCH 18/24] Fixup CLI error formatting --- Tools/clinic/clinic.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index fefc87aa15602e..f6a35bd258742d 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -5734,11 +5734,13 @@ def main(argv: list[str] | None = None) -> NoReturn: try: run_clinic(parser, args) except ClinicError as exc: - msg = ( - f"Error in file {exc.filename!r} on line {exc.lineno}:\n" - f"{exc}\n" - ) - sys.stderr.write(msg) + sys.stderr.write("Error") + if exc.filename is not None: + sys.stderr.write(f" in file {exc.filename!r}") + if exc.lineno is not None: + sys.stderr.write(f" on line {exc.lineno}") + sys.stderr.write(":\n") + sys.stderr.write(f"{exc}\n") sys.exit(1) else: sys.exit(0) From af794817cf3b5066596d481753a327e64f539210 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 2 Aug 2023 16:56:09 +0200 Subject: [PATCH 19/24] Fix reraise of ClinicError in parse() --- Tools/clinic/clinic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index f6a35bd258742d..c71866fb5f8d65 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -4612,7 +4612,7 @@ def parse(self, block: Block) -> None: try: self.state(line) except ClinicError as exc: - raise ClinicError(str(exc), lineno=line_number+1) + raise ClinicError(str(exc), filename=exc.filename, lineno=line_number) self.do_post_block_processing_cleanup() block.output.extend(self.clinic.language.render(self.clinic, block.signatures)) From d109b63ddf77fe12af5afa5b997d35924db18b75 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 2 Aug 2023 16:56:26 +0200 Subject: [PATCH 20/24] Adjust tests again --- Lib/test/test_clinic.py | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index cca849190f150c..4ed83c2c5bf011 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -652,7 +652,7 @@ def test_param_default_expression(self): os.access follow_symlinks: int = sys.maxsize """ - self.expect_failure(block, err, lineno=3) + self.expect_failure(block, err, lineno=2) def test_param_no_docstring(self): function = self.parse_function(""" @@ -677,7 +677,7 @@ def test_param_default_parameters_out_of_order(self): follow_symlinks: bool = True something_else: str """ - self.expect_failure(block, err, lineno=4) + self.expect_failure(block, err, lineno=3) def disabled_test_converter_arguments(self): function = self.parse_function(""" @@ -782,7 +782,7 @@ def test_cloning_nonexistent_function_correctly_fails(self): """ err = "Couldn't find existing function 'fooooooooooooooooo'!" with support.captured_stderr() as stderr: - self.expect_failure(block, err, lineno=1) + self.expect_failure(block, err, lineno=0) expected_debug_print = dedent("""\ cls=None, module=, existing='fooooooooooooooooo' (cls or module).functions=[] @@ -980,7 +980,7 @@ def test_disallowed_grouping__two_top_groups_on_left(self): ] param: int """ - self.expect_failure(block, err, lineno=6) + self.expect_failure(block, err, lineno=5) def test_disallowed_grouping__two_top_groups_on_right(self): block = """ @@ -1277,7 +1277,7 @@ def test_legacy_converters_non_string_constant_annotation(self): ) for block in dataset: with self.subTest(block=block): - self.expect_failure(block, err, lineno=3) + self.expect_failure(block, err, lineno=2) def test_other_bizarre_things_in_annotations_fail(self): err = "Annotations must be either a name, a function call, or a string" @@ -1288,7 +1288,7 @@ def test_other_bizarre_things_in_annotations_fail(self): ) for block in dataset: with self.subTest(block=block): - self.expect_failure(block, err, lineno=3) + self.expect_failure(block, err, lineno=2) def test_kwarg_splats_disallowed_in_function_call_annotations(self): err = "Cannot use a kwarg splat in a function-call annotation" @@ -1303,7 +1303,7 @@ def test_kwarg_splats_disallowed_in_function_call_annotations(self): self.expect_failure(block, err) def test_self_param_placement(self): - expected_error_msg = ( + err = ( "A 'self' parameter, if specified, must be the very first thing " "in the parameter block." ) @@ -1313,7 +1313,7 @@ def test_self_param_placement(self): a: int self: self(type="PyObject *") """ - self.expect_failure(block, expected_error_msg, lineno=4) + self.expect_failure(block, err, lineno=3) def test_self_param_cannot_be_optional(self): err = "A 'self' parameter cannot be marked optional." @@ -1322,10 +1322,10 @@ def test_self_param_cannot_be_optional(self): foo.func self: self(type="PyObject *") = None """ - self.expect_failure(block, err, lineno=3) + self.expect_failure(block, err, lineno=2) def test_defining_class_param_placement(self): - expected_error_msg = ( + err = ( "A 'defining_class' parameter, if specified, must either be the " "first thing in the parameter block, or come just after 'self'." ) @@ -1336,7 +1336,7 @@ def test_defining_class_param_placement(self): a: int cls: defining_class """ - self.expect_failure(block, expected_error_msg, lineno=5) + self.expect_failure(block, err, lineno=4) def test_defining_class_param_cannot_be_optional(self): err = "A 'defining_class' parameter cannot be marked optional." @@ -1345,7 +1345,7 @@ def test_defining_class_param_cannot_be_optional(self): foo.func cls: defining_class(type="PyObject *") = None """ - self.expect_failure(block, err, lineno=3) + self.expect_failure(block, err, lineno=2) def test_slot_methods_cannot_access_defining_class(self): block = """ @@ -1366,7 +1366,7 @@ def test_new_must_be_a_class_method(self): class Foo "" "" Foo.__new__ """ - self.expect_failure(block, err, lineno=3) + self.expect_failure(block, err, lineno=2) def test_init_must_be_a_normal_method(self): err = "__init__ must be a normal method, not a class or static method!" @@ -1376,7 +1376,7 @@ class Foo "" "" @classmethod Foo.__init__ """ - self.expect_failure(block, err, lineno=4) + self.expect_failure(block, err, lineno=3) def test_unused_param(self): block = self.parse(""" From ecbfe2c20958ef6e37a35c3c216a9d2f11c378d3 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 2 Aug 2023 17:06:38 +0200 Subject: [PATCH 21/24] Remove unused import --- Tools/clinic/clinic.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index c71866fb5f8d65..75e73999ceab3d 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -28,7 +28,6 @@ import string import sys import textwrap -import traceback from collections.abc import ( Callable, From 90e1bc9151059f302db906bcc43159c7e9cabeb0 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 2 Aug 2023 17:36:59 +0200 Subject: [PATCH 22/24] Mutate the exception Co-authored-by: Alex Waygood --- Tools/clinic/clinic.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 75e73999ceab3d..7119a43daee3a2 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -4611,7 +4611,8 @@ def parse(self, block: Block) -> None: try: self.state(line) except ClinicError as exc: - raise ClinicError(str(exc), filename=exc.filename, lineno=line_number) + exc.lineno = line_number + raise self.do_post_block_processing_cleanup() block.output.extend(self.clinic.language.render(self.clinic, block.signatures)) From 7766c3dfa583e1be1f388802634ce239afc706ed Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Wed, 2 Aug 2023 17:06:15 +0100 Subject: [PATCH 23/24] Share more code between the `warn()` and `fail()` branches (#49) --- Lib/test/test_clinic.py | 8 +++++-- Tools/clinic/clinic.py | 48 +++++++++++++++++++---------------------- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index 4ed83c2c5bf011..3c03891fd3617d 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -1446,8 +1446,12 @@ def test_non_ascii_character_in_docstring(self): self.parse(block) # The line numbers are off; this is a known limitation. expected = dedent("""\ - Warning on line 0: Non-ascii characters are not allowed in docstrings: 'á' - Warning on line 0: Non-ascii characters are not allowed in docstrings: 'ü', 'á', 'ß' + Warning on line 0: + Non-ascii characters are not allowed in docstrings: 'á' + + Warning on line 0: + Non-ascii characters are not allowed in docstrings: 'ü', 'á', 'ß' + """) self.assertEqual(stdout.getvalue(), expected) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 7119a43daee3a2..3712b310d9098c 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -137,18 +137,25 @@ def text_accumulator() -> TextAccumulator: return TextAccumulator(append, output) +@dc.dataclass class ClinicError(Exception): - def __init__( - self, - message: str, - /, - *, - lineno: int | None = None, - filename: str | None = None - ) -> None: - super().__init__(message) - self.lineno = lineno - self.filename = filename + message: str + _: dc.KW_ONLY + lineno: int | None = None + filename: str | None = None + + def __post_init__(self) -> None: + super().__init__(self.message) + + def report(self, *, warn_only: bool = False) -> str: + msg = "Warning" if warn_only else "Error" + if self.filename is not None: + msg += f" in file {self.filename!r}" + if self.lineno is not None: + msg += f" on line {self.lineno}" + msg += ":\n" + msg += f"{self.message}\n" + return msg @overload @@ -179,16 +186,11 @@ def warn_or_fail( filename = clinic.filename if getattr(clinic, 'block_parser', None) and (line_number is None): line_number = clinic.block_parser.line_number + error = ClinicError(joined, filename=filename, lineno=line_number) if fail: - raise ClinicError(joined, lineno=line_number, filename=filename) + raise error else: - msg = "Warning" - if filename is not None: - msg += f" in file {filename!r}" - if line_number is not None: - msg += f" on line {line_number}" - msg += f": {joined}" - print(msg) + print(error.report(warn_only=True)) def warn( @@ -5734,13 +5736,7 @@ def main(argv: list[str] | None = None) -> NoReturn: try: run_clinic(parser, args) except ClinicError as exc: - sys.stderr.write("Error") - if exc.filename is not None: - sys.stderr.write(f" in file {exc.filename!r}") - if exc.lineno is not None: - sys.stderr.write(f" on line {exc.lineno}") - sys.stderr.write(":\n") - sys.stderr.write(f"{exc}\n") + sys.stderr.write(exc.report()) sys.exit(1) else: sys.exit(0) From fcd472104ed00e32ff733d45df9ececfacc60632 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Thu, 3 Aug 2023 01:28:01 +0200 Subject: [PATCH 24/24] Add docstring to _expect_failure --- Lib/test/test_clinic.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index 3c03891fd3617d..127008d443e4c6 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -22,6 +22,15 @@ def _expect_failure(tc, parser, code, errmsg, *, filename=None, lineno=None): + """Helper for the parser tests. + + tc: unittest.TestCase; passed self in the wrapper + parser: the clinic parser used for this test case + code: a str with input text (clinic code) + errmsg: the expected error message + filename: str, optional filename + lineno: int, optional line number + """ code = dedent(code).strip() errmsg = re.escape(errmsg) with tc.assertRaisesRegex(clinic.ClinicError, errmsg) as cm: