Skip to content

Commit

Permalink
Update F541 to use new f-string tokens (#7327)
Browse files Browse the repository at this point in the history
## Summary

This PR updates the `F541` rule to use the new f-string tokens.

## Test Plan

Add new test case and uncomment a broken test case.

fixes: #7292
  • Loading branch information
dhruvmanila committed Sep 19, 2023
1 parent dd8c9f7 commit c496d9c
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 32 deletions.
6 changes: 2 additions & 4 deletions crates/ruff/resources/test/fixtures/pyflakes/F541.py
Expand Up @@ -40,7 +40,5 @@
""f""
''f""
(""f""r"")

# To be fixed
# Error: f-string: single '}' is not allowed at line 41 column 8
# f"\{{x}}"
f"{v:{f"0.2f"}}"
f"\{{x}}"
4 changes: 2 additions & 2 deletions crates/ruff/src/checkers/ast/analyze/expression.rs
Expand Up @@ -952,9 +952,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
pylint::rules::await_outside_async(checker, expr);
}
}
Expr::FString(ast::ExprFString { values, .. }) => {
Expr::FString(fstring @ ast::ExprFString { values, .. }) => {
if checker.enabled(Rule::FStringMissingPlaceholders) {
pyflakes::rules::f_string_missing_placeholders(expr, values, checker);
pyflakes::rules::f_string_missing_placeholders(fstring, checker);
}
if checker.enabled(Rule::HardcodedSQLExpression) {
flake8_bandit::rules::hardcoded_sql_expression(checker, expr);
Expand Down
@@ -1,7 +1,7 @@
use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{Expr, PySourceType};
use ruff_python_parser::{lexer, AsMode, StringKind, Tok};
use ruff_python_ast::{self as ast, Expr, PySourceType};
use ruff_python_parser::{lexer, AsMode, Tok};
use ruff_source_file::Locator;
use ruff_text_size::{Ranged, TextRange, TextSize};

Expand Down Expand Up @@ -47,45 +47,69 @@ impl AlwaysAutofixableViolation for FStringMissingPlaceholders {
}
}

/// Find f-strings that don't contain any formatted values in an [`FString`].
fn find_useless_f_strings<'a>(
expr: &'a Expr,
/// Return an iterator containing a two-element tuple for each f-string part
/// in the given [`ExprFString`] expression.
///
/// The first element of the tuple is the f-string prefix range, and the second
/// element is the entire f-string range. It returns an iterator because of the
/// possibility of multiple f-strings implicitly concatenated together.
///
/// For example,
///
/// ```python
/// f"first" rf"second"
/// # ^ ^ (prefix range)
/// # ^^^^^^^^ ^^^^^^^^^^ (token range)
/// ```
///
/// would return `[(0..1, 0..8), (10..11, 9..19)]`.
///
/// This function assumes that the given f-string expression is without any
/// placeholder expressions.
///
/// [`ExprFString`]: `ruff_python_ast::ExprFString`
fn fstring_prefix_and_tok_range<'a>(
fstring: &'a ast::ExprFString,
locator: &'a Locator,
source_type: PySourceType,
) -> impl Iterator<Item = (TextRange, TextRange)> + 'a {
let contents = locator.slice(expr);
lexer::lex_starts_at(contents, source_type.as_mode(), expr.start())
let contents = locator.slice(fstring);
let mut current_f_string_start = fstring.start();
lexer::lex_starts_at(contents, source_type.as_mode(), fstring.start())
.flatten()
.filter_map(|(tok, range)| match tok {
Tok::String {
kind: StringKind::FString | StringKind::RawFString,
..
} => {
let first_char = locator.slice(TextRange::at(range.start(), TextSize::from(1)));
.filter_map(move |(tok, range)| match tok {
Tok::FStringStart => {
current_f_string_start = range.start();
None
}
Tok::FStringEnd => {
let first_char =
locator.slice(TextRange::at(current_f_string_start, TextSize::from(1)));
// f"..." => f_position = 0
// fr"..." => f_position = 0
// rf"..." => f_position = 1
let f_position = u32::from(!(first_char == "f" || first_char == "F"));
Some((
TextRange::at(
range.start() + TextSize::from(f_position),
current_f_string_start + TextSize::from(f_position),
TextSize::from(1),
),
range,
TextRange::new(current_f_string_start, range.end()),
))
}
_ => None,
})
}

/// F541
pub(crate) fn f_string_missing_placeholders(expr: &Expr, values: &[Expr], checker: &mut Checker) {
if !values
pub(crate) fn f_string_missing_placeholders(fstring: &ast::ExprFString, checker: &mut Checker) {
if !fstring
.values
.iter()
.any(|value| matches!(value, Expr::FormattedValue(_)))
{
for (prefix_range, tok_range) in
find_useless_f_strings(expr, checker.locator(), checker.source_type)
fstring_prefix_and_tok_range(fstring, checker.locator(), checker.source_type)
{
let mut diagnostic = Diagnostic::new(FStringMissingPlaceholders, tok_range);
if checker.patch(diagnostic.kind.rule()) {
Expand Down
Expand Up @@ -332,7 +332,7 @@ F541.py:40:3: F541 [*] f-string without any placeholders
40 |+"" ""
41 41 | ''f""
42 42 | (""f""r"")
43 43 |
43 43 | f"{v:{f"0.2f"}}"

F541.py:41:3: F541 [*] f-string without any placeholders
|
Expand All @@ -341,6 +341,7 @@ F541.py:41:3: F541 [*] f-string without any placeholders
41 | ''f""
| ^^^ F541
42 | (""f""r"")
43 | f"{v:{f"0.2f"}}"
|
= help: Remove extraneous `f` prefix

Expand All @@ -351,17 +352,17 @@ F541.py:41:3: F541 [*] f-string without any placeholders
41 |-''f""
41 |+''""
42 42 | (""f""r"")
43 43 |
44 44 | # To be fixed
43 43 | f"{v:{f"0.2f"}}"
44 44 | f"\{{x}}"

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

Expand All @@ -371,8 +372,41 @@ F541.py:42:4: F541 [*] f-string without any placeholders
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
43 43 | f"{v:{f"0.2f"}}"
44 44 | f"\{{x}}"

F541.py:43:7: F541 [*] f-string without any placeholders
|
41 | ''f""
42 | (""f""r"")
43 | f"{v:{f"0.2f"}}"
| ^^^^^^^ F541
44 | f"\{{x}}"
|
= help: Remove extraneous `f` prefix

Fix
40 40 | ""f""
41 41 | ''f""
42 42 | (""f""r"")
43 |-f"{v:{f"0.2f"}}"
43 |+f"{v:{"0.2f"}}"
44 44 | f"\{{x}}"

F541.py:44:1: F541 [*] f-string without any placeholders
|
42 | (""f""r"")
43 | f"{v:{f"0.2f"}}"
44 | f"\{{x}}"
| ^^^^^^^^^ F541
|
= help: Remove extraneous `f` prefix

Fix
41 41 | ''f""
42 42 | (""f""r"")
43 43 | f"{v:{f"0.2f"}}"
44 |-f"\{{x}}"
44 |+"\{x}"


0 comments on commit c496d9c

Please sign in to comment.