Skip to content

Commit

Permalink
pythongh-107614: Normalise Argument Clinic error messages (python#107615
Browse files Browse the repository at this point in the history
)

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

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

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
  • Loading branch information
erlend-aasland and AlexWaygood authored Aug 4, 2023
1 parent 2ba7c7f commit ac7605e
Show file tree
Hide file tree
Showing 2 changed files with 179 additions and 122 deletions.
117 changes: 71 additions & 46 deletions Lib/test/test_clinic.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,11 +161,8 @@ def test_checksum_mismatch(self):
[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):
Expand Down Expand Up @@ -403,7 +400,7 @@ def test_clone_mismatch(self):
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):
Expand Down Expand Up @@ -509,6 +506,15 @@ def test_dest_buffer_not_empty_at_eof(self):
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 = """
Expand Down Expand Up @@ -598,7 +604,7 @@ def test_directive_printout(self):
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
Expand Down Expand Up @@ -637,13 +643,24 @@ def test_directive_preserve_output(self):
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
Expand Down Expand Up @@ -919,7 +936,7 @@ def test_param_default_expr_named_constant(self):
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 = """
Expand All @@ -931,7 +948,7 @@ def test_param_default_expr_named_constant(self):

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 = """
Expand All @@ -954,7 +971,7 @@ def test_param_no_docstring(self):

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 = """
Expand Down Expand Up @@ -1088,7 +1105,7 @@ def test_return_converter_invalid_syntax(self):
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):
Expand Down Expand Up @@ -1252,8 +1269,8 @@ def test_nested_groups(self):

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
Expand Down Expand Up @@ -1281,7 +1298,7 @@ def test_disallowed_grouping__two_top_groups_on_right(self):
]
"""
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)
Expand Down Expand Up @@ -1317,7 +1334,7 @@ def test_disallowed_grouping__group_after_parameter_on_left(self):
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)
Expand All @@ -1334,7 +1351,7 @@ def test_disallowed_grouping__empty_group_on_left(self):
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)
Expand All @@ -1351,7 +1368,7 @@ def test_disallowed_grouping__empty_group_on_right(self):
]
"""
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)
Expand All @@ -1365,7 +1382,7 @@ def test_disallowed_grouping__no_matching_bracket(self):
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):
Expand Down Expand Up @@ -1400,7 +1417,7 @@ def test_illegal_module_line(self):
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):
Expand All @@ -1409,7 +1426,7 @@ def test_illegal_c_basename(self):
foo.bar as 935
/
"""
err = "Illegal C basename: 935"
err = "Illegal C basename: '935'"
self.expect_failure(block, err)

def test_single_star(self):
Expand All @@ -1419,7 +1436,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):
Expand All @@ -1429,7 +1446,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)
Expand All @@ -1442,7 +1459,7 @@ def test_single_slash(self):
/
"""
err = (
"Function bar has an unsupported group configuration. "
"Function 'bar' has an unsupported group configuration. "
"(Unexpected state 0.d)"
)
self.expect_failure(block, err)
Expand All @@ -1456,7 +1473,7 @@ def test_double_slash(self):
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):
Expand All @@ -1470,7 +1487,7 @@ def test_mix_star_and_slash(self):
/
"""
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)
Expand All @@ -1483,7 +1500,7 @@ def test_parameters_not_permitted_after_slash_for_now(self):
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)
Expand Down Expand Up @@ -1582,7 +1599,8 @@ def test_indent_stack_no_tabs(self):
*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):
Expand Down Expand Up @@ -1841,16 +1859,25 @@ def test_vararg_cannot_take_default_value(self):
"""
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'
"""
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
Expand All @@ -1877,7 +1904,7 @@ def test_param_requires_custom_c_name(self):
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
Expand All @@ -1887,7 +1914,7 @@ def test_state_func_docstring_assert_no_group(self):
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
Expand Down Expand Up @@ -1983,13 +2010,11 @@ def test_cli_force(self):
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:
Expand Down Expand Up @@ -2214,7 +2239,7 @@ def test_file_dest(self):
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.
Expand Down
Loading

0 comments on commit ac7605e

Please sign in to comment.