diff --git a/crates/common/src/format.rs b/crates/common/src/format.rs index 4357a0c66..28afac735 100644 --- a/crates/common/src/format.rs +++ b/crates/common/src/format.rs @@ -1618,6 +1618,12 @@ impl FormatString { let right = &text[pos..]; let format_part = Self::parse_part_in_brackets(&left)?; Ok((format_part, &right[1..])) + } else if text.len() == 1 { + // Nothing follows the brace, so it is a stray one rather than a field + // that was left open: CPython separates "{" and "a{" from "{0" and + // "a{b" the same way. `{` is one byte, so a length of one is the brace + // on its own. + Err(FormatParseError::UnescapedStartBracketInLiteral) } else { Err(FormatParseError::UnmatchedBracket) } @@ -2338,6 +2344,26 @@ mod tests { ); } + #[test] + fn format_parse_lone_start_bracket() { + // A brace with nothing after it is a stray brace; one holding a field + // that was never closed is not. CPython words the two differently. + for lone in ["{", "a{"] { + assert_eq!( + FormatString::from_str(lone.as_ref()), + Err(FormatParseError::UnescapedStartBracketInLiteral), + "{lone:?}" + ); + } + for unclosed in ["{s", "a{b", "{0"] { + assert_eq!( + FormatString::from_str(unclosed.as_ref()), + Err(FormatParseError::UnmatchedBracket), + "{unclosed:?}" + ); + } + } + #[test] fn square_brackets_inside_format() { assert_eq!( diff --git a/crates/vm/src/format.rs b/crates/vm/src/format.rs index 9a54df3b4..d95def768 100644 --- a/crates/vm/src/format.rs +++ b/crates/vm/src/format.rs @@ -103,12 +103,36 @@ impl IntoPyException for FormatSpecError { impl ToPyException for FormatParseError { fn to_pyexception(&self, vm: &VirtualMachine) -> PyBaseExceptionRef { + // Matched exhaustively on purpose: a catch-all sent most of these to + // "Unexpected error parsing format string", which tells the reader nothing + // about the format string they mistyped. match self { - Self::UnmatchedBracket => vm.new_value_error("expected '}' before end of string"), + Self::UnmatchedBracket | Self::MissingRightBracket => { + vm.new_value_error("expected '}' before end of string") + } + Self::MissingStartBracket => { + vm.new_value_error("Single '}' encountered in format string") + } + // A brace in literal text that was not doubled, i.e. a stray `{` with + // nothing after it. `parse_spec` reports this rather than + // UnmatchedBracket so that "{" and "a{" are separated from "{0" and + // "a{b", which are fields left open. + Self::UnescapedStartBracketInLiteral => { + vm.new_value_error("Single '{' encountered in format string") + } + Self::InvalidFormatSpecifier => vm.new_value_error("unmatched '{' in format spec"), + // Reached both for a conversion that is not one character (`{0!xy}`, + // where CPython says "expected ':' after conversion specifier") and for + // an empty one (`{0!}`, "unmatched '{' in format spec"). Telling those + // apart needs the parser to say which, so the generic message stays. + Self::UnknownConversion => vm.new_value_error("Unexpected error parsing format string"), + Self::EmptyAttribute => vm.new_value_error("Empty attribute in format string"), + Self::InvalidCharacterAfterRightBracket => { + vm.new_value_error("Only '.' or '[' may follow ']' in format field specifier") + } Self::TooManyDecimalDigits => { vm.new_value_error("Too many decimal digits in format string") } - _ => vm.new_value_error("Unexpected error parsing format string"), } } } diff --git a/extra_tests/snippets/builtin_format.py b/extra_tests/snippets/builtin_format.py index 844d9e451..920256c0d 100644 --- a/extra_tests/snippets/builtin_format.py +++ b/extra_tests/snippets/builtin_format.py @@ -336,3 +336,45 @@ assert_raises(ValueError, format, True, ".2") assert_raises(ValueError, format, True, "5.2") assert_raises(ValueError, format, True, "z") assert_raises(ValueError, format, True, "s") + + +def test_format_parse_error_messages(): + # Most FormatParseError variants used to fall into a catch-all that said + # "Unexpected error parsing format string", which does not say what is wrong + # with the format string. Messages are CPython 3.14.0's. + def message(fmt, *args): + try: + fmt.format(*args) + except ValueError as err: + return str(err) + raise AssertionError("ValueError was not raised for " + repr(fmt)) + + assert message("a}b") == "Single '}' encountered in format string" + assert message("{{a}") == "Single '}' encountered in format string" + + assert message("{0.}", 1) == "Empty attribute in format string" + assert message("{0[]}", [1]) == "Empty attribute in format string" + + assert message("{0[1}", [1, 2]) == "expected '}' before end of string" + assert message("{0", 1) == "expected '}' before end of string" + + assert ( + message("{0[0]x}", [1]) + == "Only '.' or '[' may follow ']' in format field specifier" + ) + assert ( + message("{0[0]]}", [1]) + == "Only '.' or '[' may follow ']' in format field specifier" + ) + + assert message("{:{{}}", 1) == "unmatched '{' in format spec" + + # A brace with nothing after it is a stray brace; one holding an unclosed + # field is not. + assert message("{") == "Single '{' encountered in format string" + assert message("a{") == "Single '{' encountered in format string" + assert message("{s") == "expected '}' before end of string" + assert message("a{b") == "expected '}' before end of string" + + +test_format_parse_error_messages()