Skip to content

Commit

Permalink
Avoid syntax errors when removing f-string prefixes (#5319)
Browse files Browse the repository at this point in the history
Closes #5281.

Closes #4827.
  • Loading branch information
charliermarsh authored Jun 22, 2023
1 parent 4a81cfc commit 7819b95
Show file tree
Hide file tree
Showing 4 changed files with 118 additions and 31 deletions.
5 changes: 4 additions & 1 deletion crates/ruff/resources/test/fixtures/pyflakes/F541.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,10 @@
f'{{ 40 }}'
f"{{a {{x}}"
f"{{{{x}}}}"
""f""
''f""
(""f""r"")

# To be fixed
# Error: f-string: single '}' is not allowed at line 41 column 8
# f"\{{x}}"
# f"\{{x}}"
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use rustpython_parser::ast::{self, Expr, Stmt};
use rustpython_parser::ast::{self, Stmt};

use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
Expand Down Expand Up @@ -50,9 +50,9 @@ pub(crate) fn f_string_docstring(checker: &mut Checker, body: &[Stmt]) {
let Stmt::Expr(ast::StmtExpr { value, range: _ }) = stmt else {
return;
};
let Expr::JoinedStr ( _) = value.as_ref() else {
if !value.is_joined_str_expr() {
return;
};
}
checker
.diagnostics
.push(Diagnostic::new(FStringDocstring, stmt.identifier()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,23 +79,6 @@ fn find_useless_f_strings<'a>(
})
}

fn unescape_f_string(content: &str) -> String {
content.replace("{{", "{").replace("}}", "}")
}

fn fix_f_string_missing_placeholders(
prefix_range: TextRange,
tok_range: TextRange,
checker: &mut Checker,
) -> Fix {
let content = &checker.locator.contents()[TextRange::new(prefix_range.end(), tok_range.end())];
Fix::automatic(Edit::replacement(
unescape_f_string(content),
prefix_range.start(),
tok_range.end(),
))
}

/// F541
pub(crate) fn f_string_missing_placeholders(expr: &Expr, values: &[Expr], checker: &mut Checker) {
if !values
Expand All @@ -105,13 +88,51 @@ pub(crate) fn f_string_missing_placeholders(expr: &Expr, values: &[Expr], checke
for (prefix_range, tok_range) in find_useless_f_strings(expr, checker.locator) {
let mut diagnostic = Diagnostic::new(FStringMissingPlaceholders, tok_range);
if checker.patch(diagnostic.kind.rule()) {
diagnostic.set_fix(fix_f_string_missing_placeholders(
diagnostic.set_fix(convert_f_string_to_regular_string(
prefix_range,
tok_range,
checker,
checker.locator,
));
}
checker.diagnostics.push(diagnostic);
}
}
}

/// Unescape an f-string body by replacing `{{` with `{` and `}}` with `}`.
///
/// In Python, curly-brace literals within f-strings must be escaped by doubling the braces.
/// When rewriting an f-string to a regular string, we need to unescape any curly-brace literals.
/// For example, given `{{Hello, world!}}`, return `{Hello, world!}`.
fn unescape_f_string(content: &str) -> String {
content.replace("{{", "{").replace("}}", "}")
}

/// Generate a [`Fix`] to rewrite an f-string as a regular string.
fn convert_f_string_to_regular_string(
prefix_range: TextRange,
tok_range: TextRange,
locator: &Locator,
) -> Fix {
// Extract the f-string body.
let mut content =
unescape_f_string(locator.slice(TextRange::new(prefix_range.end(), tok_range.end())));

// If the preceding character is equivalent to the quote character, insert a space to avoid a
// syntax error. For example, when removing the `f` prefix in `""f""`, rewrite to `"" ""`
// instead of `""""`.
if locator
.slice(TextRange::up_to(prefix_range.start()))
.chars()
.last()
.map_or(false, |char| content.starts_with(char))
{
content.insert(0, ' ');
}

Fix::automatic(Edit::replacement(
content,
prefix_range.start(),
tok_range.end(),
))
}
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ F541.py:37:1: F541 [*] f-string without any placeholders
37 |+'{ 40 }'
38 38 | f"{{a {{x}}"
39 39 | f"{{{{x}}}}"
40 40 |
40 40 | ""f""

F541.py:38:1: F541 [*] f-string without any placeholders
|
Expand All @@ -278,6 +278,7 @@ F541.py:38:1: F541 [*] f-string without any placeholders
38 | f"{{a {{x}}"
| ^^^^^^^^^^^^ F541
39 | f"{{{{x}}}}"
40 | ""f""
|
= help: Remove extraneous `f` prefix

Expand All @@ -288,17 +289,17 @@ F541.py:38:1: F541 [*] f-string without any placeholders
38 |-f"{{a {{x}}"
38 |+"{a {x}"
39 39 | f"{{{{x}}}}"
40 40 |
41 41 | # To be fixed
40 40 | ""f""
41 41 | ''f""

F541.py:39:1: F541 [*] f-string without any placeholders
|
37 | f'{{ 40 }}'
38 | f"{{a {{x}}"
39 | f"{{{{x}}}}"
| ^^^^^^^^^^^^ F541
40 |
41 | # To be fixed
40 | ""f""
41 | ''f""
|
= help: Remove extraneous `f` prefix

Expand All @@ -308,8 +309,70 @@ F541.py:39:1: F541 [*] f-string without any placeholders
38 38 | f"{{a {{x}}"
39 |-f"{{{{x}}}}"
39 |+"{{x}}"
40 40 |
41 41 | # To be fixed
42 42 | # Error: f-string: single '}' is not allowed at line 41 column 8
40 40 | ""f""
41 41 | ''f""
42 42 | (""f""r"")

F541.py:40:3: F541 [*] f-string without any placeholders
|
38 | f"{{a {{x}}"
39 | f"{{{{x}}}}"
40 | ""f""
| ^^^ F541
41 | ''f""
42 | (""f""r"")
|
= help: Remove extraneous `f` prefix

Fix
37 37 | f'{{ 40 }}'
38 38 | f"{{a {{x}}"
39 39 | f"{{{{x}}}}"
40 |-""f""
40 |+"" ""
41 41 | ''f""
42 42 | (""f""r"")
43 43 |

F541.py:41:3: F541 [*] f-string without any placeholders
|
39 | f"{{{{x}}}}"
40 | ""f""
41 | ''f""
| ^^^ F541
42 | (""f""r"")
|
= help: Remove extraneous `f` prefix

Fix
38 38 | f"{{a {{x}}"
39 39 | f"{{{{x}}}}"
40 40 | ""f""
41 |-''f""
41 |+''""
42 42 | (""f""r"")
43 43 |
44 44 | # To be fixed

F541.py:42:4: F541 [*] f-string without any placeholders
|
40 | ""f""
41 | ''f""
42 | (""f""r"")
| ^^^ F541
43 |
44 | # To be fixed
|
= help: Remove extraneous `f` prefix

Fix
39 39 | f"{{{{x}}}}"
40 40 | ""f""
41 41 | ''f""
42 |-(""f""r"")
42 |+("" ""r"")
43 43 |
44 44 | # To be fixed
45 45 | # Error: f-string: single '}' is not allowed at line 41 column 8


0 comments on commit 7819b95

Please sign in to comment.