Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for nested replacements inside format specifications #6616

Merged
merged 13 commits into from
Aug 17, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@

"{:s} {:y}".format("hello", "world") # [bad-format-character]

"{:*^30s}".format("centered")
"{:*^30s}".format("centered") # OK
"{:{s}}".format("hello", s="s") # OK (nested replacement value not checked)

"{:{s:y}}".format("hello", s="s") # [bad-format-character] (nested replacement format spec checked)

## f-strings

Expand Down
4 changes: 3 additions & 1 deletion crates/ruff/src/rules/pyflakes/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ pub(crate) fn error_to_string(err: &FormatParseError) -> String {
FormatParseError::InvalidCharacterAfterRightBracket => {
"Only '.' or '[' may follow ']' in format field specifier"
}
FormatParseError::InvalidFormatSpecifier => "Max string recursion exceeded",
FormatParseError::PlaceholderRecursionExceeded => {
"Max format placeholder recursion exceeded"
}
FormatParseError::MissingStartBracket => "Single '}' encountered in format string",
FormatParseError::MissingRightBracket => "Expected '}' before end of string",
FormatParseError::UnmatchedBracket => "Single '{' encountered in format string",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ F521.py:3:1: F521 `.format` call has invalid format string: Expected '}' before
5 | "{:{:{}}}".format(1, 2, 3)
|

F521.py:5:1: F521 `.format` call has invalid format string: Max string recursion exceeded
F521.py:5:1: F521 `.format` call has invalid format string: Max format placeholder recursion exceeded
|
3 | "{foo[}".format(foo=1)
4 | # too much string recursion (placeholder-in-placeholder)
Expand Down
43 changes: 33 additions & 10 deletions crates/ruff/src/rules/pylint/rules/bad_string_format_character.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,16 +49,39 @@ pub(crate) fn call(checker: &mut Checker, string: &str, range: TextRange) {
continue;
};

if let Err(FormatSpecError::InvalidFormatType) = FormatSpec::parse(format_spec) {
checker.diagnostics.push(Diagnostic::new(
BadStringFormatCharacter {
// The format type character is always the last one.
// More info in the official spec:
// https://docs.python.org/3/library/string.html#format-specification-mini-language
format_char: format_spec.chars().last().unwrap(),
},
range,
));
match FormatSpec::parse(format_spec) {
Err(FormatSpecError::InvalidFormatType) => {
checker.diagnostics.push(Diagnostic::new(
BadStringFormatCharacter {
// The format type character is always the last one.
// More info in the official spec:
// https://docs.python.org/3/library/string.html#format-specification-mini-language
format_char: format_spec.chars().last().unwrap(),
},
range,
));
}
Err(_) => {}
Ok(format_spec) => {
for replacement in format_spec.replacements() {
let FormatPart::Field { format_spec, .. } = replacement else {
continue;
};
if let Err(FormatSpecError::InvalidFormatType) =
FormatSpec::parse(format_spec)
{
checker.diagnostics.push(Diagnostic::new(
BadStringFormatCharacter {
// The format type character is always the last one.
// More info in the official spec:
// https://docs.python.org/3/library/string.html#format-specification-mini-language
format_char: format_spec.chars().last().unwrap(),
},
range,
));
}
}
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,17 @@ bad_string_format_character.py:15:1: PLE1300 Unsupported format character 'y'
15 | "{:s} {:y}".format("hello", "world") # [bad-format-character]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLE1300
16 |
17 | "{:*^30s}".format("centered")
17 | "{:*^30s}".format("centered") # OK
|

bad_string_format_character.py:20:1: PLE1300 Unsupported format character 'y'
|
18 | "{:{s}}".format("hello", s="s") # OK (nested replacement value not checked)
19 |
20 | "{:{s:y}}".format("hello", s="s") # [bad-format-character] (nested replacement format spec checked)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLE1300
Comment on lines +58 to +59
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd really like better range information here. Would that be hard for me to add to FormatSpec?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be challenging because the thing we get on the left-hand side is the parsed representation which doesn't always match the literal representation. For example, the string on the left could be an implicit concatenation, or it could contain escaped characters, etc...

21 |
22 | ## f-strings
|


Loading