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

Avoid syntax errors when removing f-string prefixes #5319

Merged
merged 1 commit into from
Jun 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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