mirror of
https://github.com/RustPython/RustPython.git
synced 2026-10-28 10:44:36 +08:00
Report which format string mistake was made (#8659)
* fix(vm): report which format string mistake was made
`ToPyException for FormatParseError` mapped two variants and sent the rest to
"Unexpected error parsing format string", so most format-string mistakes gave
no hint about the mistake. Against CPython 3.14.0:
| expression | before | CPython |
| --- | --- | --- |
| `"a}b".format()` | Unexpected error… | Single `}` encountered in format string |
| `"{0.}".format(1)` | Unexpected error… | Empty attribute in format string |
| `"{0[]}".format([1])` | Unexpected error… | Empty attribute in format string |
| `"{0[1}".format([1,2])` | Unexpected error… | expected `}` before end of string |
| `"{0[0]x}".format([1])` | Unexpected error… | Only `.` or `[` may follow `]` in format field specifier |
| `"{:{{}}".format(1)` | Unexpected error… | unmatched `{` in format spec |
The match is now exhaustive, so a new variant cannot silently inherit the
generic message.
Two cases are deliberately left generic, with the reason in a comment beside
each. `UnknownConversion` is raised both for a multi-character conversion
(`{0!xy}`, where CPython says "expected ':' after conversion specifier") and
for an empty one (`{0!}`, "unmatched '{' in format spec"), so one message
cannot serve both without the parser saying which it hit. And a trailing lone
`{` reaches `UnmatchedBracket` rather than
`UnescapedStartBracketInLiteral`, so `"{".format()` still differs from CPython;
that needs a parser change, not a message.
Expectations in the new snippet are CPython 3.14.0's output and pass under both
interpreters. Reverting only format.rs fails the snippet's first assertion.
Assisted-by: Claude Code:claude-opus-5
* fix(common): separate a stray `{` from an unclosed field
`parse_literal` already reports `UnescapedStartBracketInLiteral` for a brace
that was not doubled, but `from_str` discards it with `or_else` and retries as a
field, so `"{".format()` surfaced `UnmatchedBracket` and read as
"expected '}' before end of string". CPython separates the two by whether
anything follows the brace:
"{" "a{" Single '{' encountered in format string
"{s" "a{b" expected '}' before end of string
"{0" expected '}' before end of string
`parse_spec` now returns the literal error when the text is the brace alone,
which is also what makes `UnescapedStartBracketInLiteral` reachable — the
previous commit could map it but not trigger it.
`format_parse_lone_start_bracket` pins both directions at the parser level, and
the snippet covers the four messages under CPython 3.14.0 as well. The existing
`format_parse_fail` assertion for `"{s"` is unchanged.
Assisted-by: Claude Code:claude-opus-5
This commit is contained in:
1 parent
bfd52eda61
commit
b701e5cbe9
3 files changed
+94
-2
No files matched your search
@@ -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!(
|
||||
|
||||
+26
-2
@@ -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"),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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()
|
||||
Reference in new issue
Block a user