diff --git a/crates/ruff/resources/test/fixtures/ruff/RUF010.py b/crates/ruff/resources/test/fixtures/ruff/RUF010.py index 77e459c21496a..6416b79c9e3d3 100644 --- a/crates/ruff/resources/test/fixtures/ruff/RUF010.py +++ b/crates/ruff/resources/test/fixtures/ruff/RUF010.py @@ -34,3 +34,12 @@ def ascii(arg): " intermediary content " f" that flows {repr(obj)} of type {type(obj)}.{additional_message}" # RUF010 ) + + +f"{str(bla)}" # RUF010 + +f"{str(bla):20}" # RUF010 + +f"{bla!s}" # RUF010 + +f"{bla!s:20}" # OK diff --git a/crates/ruff/src/rules/ruff/rules/explicit_f_string_type_conversion.rs b/crates/ruff/src/rules/ruff/rules/explicit_f_string_type_conversion.rs index b41e8ae77b7ae..e564f35eab467 100644 --- a/crates/ruff/src/rules/ruff/rules/explicit_f_string_type_conversion.rs +++ b/crates/ruff/src/rules/ruff/rules/explicit_f_string_type_conversion.rs @@ -6,6 +6,7 @@ use rustpython_parser::ast::{self, Expr, Ranged}; use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::prelude::ConversionFlag; use ruff_python_ast::source_code::{Locator, Stylist}; use crate::autofix::codemods::CodegenStylist; @@ -21,31 +22,62 @@ use crate::registry::AsRule; /// f-strings support dedicated conversion flags for these types, which are /// more succinct and idiomatic. /// +/// In the case of `str()`, it's also redundant, since `str()` is the default +/// conversion. +/// /// ## Example /// ```python /// a = "some string" +/// f"{str(a)}" /// f"{repr(a)}" /// ``` /// /// Use instead: /// ```python /// a = "some string" +/// f"{a}" /// f"{a!r}" /// ``` #[violation] -pub struct ExplicitFStringTypeConversion; +pub struct ExplicitFStringTypeConversion { + operation: Operation, +} impl AlwaysAutofixableViolation for ExplicitFStringTypeConversion { #[derive_message_formats] fn message(&self) -> String { - format!("Use conversion in f-string") + let ExplicitFStringTypeConversion { operation } = self; + match operation { + Operation::ConvertCallToConversionFlag => { + format!("Use explicit conversion flag") + } + Operation::RemoveCall => format!("Remove unnecessary `str` conversion"), + Operation::RemoveConversionFlag => format!("Remove unnecessary conversion flag"), + } } fn autofix_title(&self) -> String { - "Replace f-string function call with conversion".to_string() + let ExplicitFStringTypeConversion { operation } = self; + match operation { + Operation::ConvertCallToConversionFlag => { + format!("Replace with conversion flag") + } + Operation::RemoveCall => format!("Remove `str` call"), + Operation::RemoveConversionFlag => format!("Remove conversion flag"), + } } } +#[derive(Debug, PartialEq, Eq)] +enum Operation { + /// Ex) Convert `f"{repr(bla)}"` to `f"{bla!r}"` + ConvertCallToConversionFlag, + /// Ex) Convert `f"{bla!s}"` to `f"{bla}"` + RemoveConversionFlag, + /// Ex) Convert `f"{str(bla)}"` to `f"{bla}"` + RemoveCall, +} + /// RUF010 pub(crate) fn explicit_f_string_type_conversion( checker: &mut Checker, @@ -64,52 +96,158 @@ pub(crate) fn explicit_f_string_type_conversion( .enumerate() { let ast::ExprFormattedValue { - value, conversion, .. + value, + conversion, + format_spec, + range: _, } = formatted_value; - // Skip if there's already a conversion flag. - if !conversion.is_none() { - continue; - } + match conversion { + ConversionFlag::Ascii | ConversionFlag::Repr => { + // Nothing to do. + continue; + } + ConversionFlag::Str => { + // Skip if there's a format spec. + if format_spec.is_some() { + continue; + } - let Expr::Call(ast::ExprCall { - func, - args, - keywords, - .. - }) = value.as_ref() else { - continue; - }; - - // Can't be a conversion otherwise. - if args.len() != 1 || !keywords.is_empty() { - continue; - } + // Remove the conversion flag entirely. + // Ex) `f"{bla!s}"` + let mut diagnostic = Diagnostic::new( + ExplicitFStringTypeConversion { + operation: Operation::RemoveConversionFlag, + }, + value.range(), + ); + if checker.patch(diagnostic.kind.rule()) { + diagnostic.try_set_fix(|| { + remove_conversion_flag(expr, index, checker.locator, checker.stylist) + }); + } + checker.diagnostics.push(diagnostic); + } + ConversionFlag::None => { + // Replace with the appropriate conversion flag. + let Expr::Call(ast::ExprCall { + func, + args, + keywords, + .. + }) = value.as_ref() else { + continue; + }; + + // Can't be a conversion otherwise. + if args.len() != 1 || !keywords.is_empty() { + continue; + } - let Expr::Name(ast::ExprName { id, .. }) = func.as_ref() else { - continue; - }; + let Expr::Name(ast::ExprName { id, .. }) = func.as_ref() else { + continue; + }; - if !matches!(id.as_str(), "str" | "repr" | "ascii") { - continue; - }; + if !matches!(id.as_str(), "str" | "repr" | "ascii") { + continue; + }; - if !checker.semantic_model().is_builtin(id) { - continue; - } + if !checker.semantic_model().is_builtin(id) { + continue; + } - let mut diagnostic = Diagnostic::new(ExplicitFStringTypeConversion, value.range()); - if checker.patch(diagnostic.kind.rule()) { - diagnostic.try_set_fix(|| { - fix_explicit_f_string_type_conversion(expr, index, checker.locator, checker.stylist) - }); + if id == "str" && format_spec.is_none() { + // Ex) `f"{str(bla)}"` + let mut diagnostic = Diagnostic::new( + ExplicitFStringTypeConversion { + operation: Operation::RemoveCall, + }, + value.range(), + ); + if checker.patch(diagnostic.kind.rule()) { + diagnostic.try_set_fix(|| { + remove_conversion_call(expr, index, checker.locator, checker.stylist) + }); + } + checker.diagnostics.push(diagnostic); + } else { + // Ex) `f"{repr(bla)}"` + let mut diagnostic = Diagnostic::new( + ExplicitFStringTypeConversion { + operation: Operation::ConvertCallToConversionFlag, + }, + value.range(), + ); + if checker.patch(diagnostic.kind.rule()) { + diagnostic.try_set_fix(|| { + convert_call_to_conversion_flag( + expr, + index, + checker.locator, + checker.stylist, + ) + }); + } + checker.diagnostics.push(diagnostic); + } + } } - checker.diagnostics.push(diagnostic); } } +/// Generate a [`Fix`] to remove a conversion flag from a formatted expression. +fn remove_conversion_flag( + expr: &Expr, + index: usize, + locator: &Locator, + stylist: &Stylist, +) -> Result { + // Parenthesize the expression, to support implicit concatenation. + let range = expr.range(); + let content = locator.slice(range); + let parenthesized_content = format!("({content})"); + let mut expression = match_expression(&parenthesized_content)?; + + // Replace the formatted call expression at `index` with a conversion flag. + let mut formatted_string_expression = match_part(index, &mut expression)?; + formatted_string_expression.conversion = None; + + // Remove the parentheses (first and last characters). + let mut content = expression.codegen_stylist(stylist); + content.remove(0); + content.pop(); + + Ok(Fix::automatic(Edit::range_replacement(content, range))) +} + +/// Generate a [`Fix`] to remove a call from a formatted expression. +fn remove_conversion_call( + expr: &Expr, + index: usize, + locator: &Locator, + stylist: &Stylist, +) -> Result { + // Parenthesize the expression, to support implicit concatenation. + let range = expr.range(); + let content = locator.slice(range); + let parenthesized_content = format!("({content})"); + let mut expression = match_expression(&parenthesized_content)?; + + // Replace the formatted call expression at `index` with a conversion flag. + let mut formatted_string_expression = match_part(index, &mut expression)?; + let call = match_call_mut(&mut formatted_string_expression.expression)?; + formatted_string_expression.expression = call.args[0].value.clone(); + + // Remove the parentheses (first and last characters). + let mut content = expression.codegen_stylist(stylist); + content.remove(0); + content.pop(); + + Ok(Fix::automatic(Edit::range_replacement(content, range))) +} + /// Generate a [`Fix`] to replace an explicit type conversion with a conversion flag. -fn fix_explicit_f_string_type_conversion( +fn convert_call_to_conversion_flag( expr: &Expr, index: usize, locator: &Locator, diff --git a/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF010_RUF010.py.snap b/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF010_RUF010.py.snap index 4a82c0546383a..72861be06ff87 100644 --- a/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF010_RUF010.py.snap +++ b/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF010_RUF010.py.snap @@ -1,33 +1,33 @@ --- source: crates/ruff/src/rules/ruff/mod.rs --- -RUF010.py:9:4: RUF010 [*] Use conversion in f-string +RUF010.py:9:4: RUF010 [*] Remove unnecessary `str` conversion | 9 | f"{str(bla)}, {repr(bla)}, {ascii(bla)}" # RUF010 | ^^^^^^^^ RUF010 10 | 11 | f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}" # RUF010 | - = help: Replace f-string function call with conversion + = help: Remove `str` call ℹ Fix 6 6 | pass 7 7 | 8 8 | 9 |-f"{str(bla)}, {repr(bla)}, {ascii(bla)}" # RUF010 - 9 |+f"{bla!s}, {repr(bla)}, {ascii(bla)}" # RUF010 + 9 |+f"{bla}, {repr(bla)}, {ascii(bla)}" # RUF010 10 10 | 11 11 | f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}" # RUF010 12 12 | -RUF010.py:9:16: RUF010 [*] Use conversion in f-string +RUF010.py:9:16: RUF010 [*] Use explicit conversion flag | 9 | f"{str(bla)}, {repr(bla)}, {ascii(bla)}" # RUF010 | ^^^^^^^^^ RUF010 10 | 11 | f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}" # RUF010 | - = help: Replace f-string function call with conversion + = help: Replace with conversion flag ℹ Fix 6 6 | pass @@ -39,14 +39,14 @@ RUF010.py:9:16: RUF010 [*] Use conversion in f-string 11 11 | f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}" # RUF010 12 12 | -RUF010.py:9:29: RUF010 [*] Use conversion in f-string +RUF010.py:9:29: RUF010 [*] Use explicit conversion flag | 9 | f"{str(bla)}, {repr(bla)}, {ascii(bla)}" # RUF010 | ^^^^^^^^^^ RUF010 10 | 11 | f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}" # RUF010 | - = help: Replace f-string function call with conversion + = help: Replace with conversion flag ℹ Fix 6 6 | pass @@ -58,7 +58,7 @@ RUF010.py:9:29: RUF010 [*] Use conversion in f-string 11 11 | f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}" # RUF010 12 12 | -RUF010.py:11:4: RUF010 [*] Use conversion in f-string +RUF010.py:11:4: RUF010 [*] Remove unnecessary `str` conversion | 11 | f"{str(bla)}, {repr(bla)}, {ascii(bla)}" # RUF010 12 | @@ -67,19 +67,19 @@ RUF010.py:11:4: RUF010 [*] Use conversion in f-string 14 | 15 | f"{(str(bla))}, {(repr(bla))}, {(ascii(bla))}" # RUF010 | - = help: Replace f-string function call with conversion + = help: Remove `str` call ℹ Fix 8 8 | 9 9 | f"{str(bla)}, {repr(bla)}, {ascii(bla)}" # RUF010 10 10 | 11 |-f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}" # RUF010 - 11 |+f"{d['a']!s}, {repr(d['b'])}, {ascii(d['c'])}" # RUF010 + 11 |+f"{d['a']}, {repr(d['b'])}, {ascii(d['c'])}" # RUF010 12 12 | 13 13 | f"{(str(bla))}, {(repr(bla))}, {(ascii(bla))}" # RUF010 14 14 | -RUF010.py:11:19: RUF010 [*] Use conversion in f-string +RUF010.py:11:19: RUF010 [*] Use explicit conversion flag | 11 | f"{str(bla)}, {repr(bla)}, {ascii(bla)}" # RUF010 12 | @@ -88,7 +88,7 @@ RUF010.py:11:19: RUF010 [*] Use conversion in f-string 14 | 15 | f"{(str(bla))}, {(repr(bla))}, {(ascii(bla))}" # RUF010 | - = help: Replace f-string function call with conversion + = help: Replace with conversion flag ℹ Fix 8 8 | @@ -100,7 +100,7 @@ RUF010.py:11:19: RUF010 [*] Use conversion in f-string 13 13 | f"{(str(bla))}, {(repr(bla))}, {(ascii(bla))}" # RUF010 14 14 | -RUF010.py:11:35: RUF010 [*] Use conversion in f-string +RUF010.py:11:35: RUF010 [*] Use explicit conversion flag | 11 | f"{str(bla)}, {repr(bla)}, {ascii(bla)}" # RUF010 12 | @@ -109,7 +109,7 @@ RUF010.py:11:35: RUF010 [*] Use conversion in f-string 14 | 15 | f"{(str(bla))}, {(repr(bla))}, {(ascii(bla))}" # RUF010 | - = help: Replace f-string function call with conversion + = help: Replace with conversion flag ℹ Fix 8 8 | @@ -121,7 +121,7 @@ RUF010.py:11:35: RUF010 [*] Use conversion in f-string 13 13 | f"{(str(bla))}, {(repr(bla))}, {(ascii(bla))}" # RUF010 14 14 | -RUF010.py:13:5: RUF010 [*] Use conversion in f-string +RUF010.py:13:5: RUF010 [*] Remove unnecessary `str` conversion | 13 | f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}" # RUF010 14 | @@ -130,19 +130,19 @@ RUF010.py:13:5: RUF010 [*] Use conversion in f-string 16 | 17 | f"{bla!s}, {(repr(bla))}, {(ascii(bla))}" # RUF010 | - = help: Replace f-string function call with conversion + = help: Remove `str` call ℹ Fix 10 10 | 11 11 | f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}" # RUF010 12 12 | 13 |-f"{(str(bla))}, {(repr(bla))}, {(ascii(bla))}" # RUF010 - 13 |+f"{bla!s}, {(repr(bla))}, {(ascii(bla))}" # RUF010 + 13 |+f"{bla}, {(repr(bla))}, {(ascii(bla))}" # RUF010 14 14 | 15 15 | f"{bla!s}, {(repr(bla))}, {(ascii(bla))}" # RUF010 16 16 | -RUF010.py:13:19: RUF010 [*] Use conversion in f-string +RUF010.py:13:19: RUF010 [*] Use explicit conversion flag | 13 | f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}" # RUF010 14 | @@ -151,7 +151,7 @@ RUF010.py:13:19: RUF010 [*] Use conversion in f-string 16 | 17 | f"{bla!s}, {(repr(bla))}, {(ascii(bla))}" # RUF010 | - = help: Replace f-string function call with conversion + = help: Replace with conversion flag ℹ Fix 10 10 | @@ -163,7 +163,7 @@ RUF010.py:13:19: RUF010 [*] Use conversion in f-string 15 15 | f"{bla!s}, {(repr(bla))}, {(ascii(bla))}" # RUF010 16 16 | -RUF010.py:13:34: RUF010 [*] Use conversion in f-string +RUF010.py:13:34: RUF010 [*] Use explicit conversion flag | 13 | f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}" # RUF010 14 | @@ -172,7 +172,7 @@ RUF010.py:13:34: RUF010 [*] Use conversion in f-string 16 | 17 | f"{bla!s}, {(repr(bla))}, {(ascii(bla))}" # RUF010 | - = help: Replace f-string function call with conversion + = help: Replace with conversion flag ℹ Fix 10 10 | @@ -184,7 +184,28 @@ RUF010.py:13:34: RUF010 [*] Use conversion in f-string 15 15 | f"{bla!s}, {(repr(bla))}, {(ascii(bla))}" # RUF010 16 16 | -RUF010.py:15:14: RUF010 [*] Use conversion in f-string +RUF010.py:15:4: RUF010 [*] Remove unnecessary conversion flag + | +15 | f"{(str(bla))}, {(repr(bla))}, {(ascii(bla))}" # RUF010 +16 | +17 | f"{bla!s}, {(repr(bla))}, {(ascii(bla))}" # RUF010 + | ^^^ RUF010 +18 | +19 | f"{foo(bla)}" # OK + | + = help: Remove conversion flag + +ℹ Fix +12 12 | +13 13 | f"{(str(bla))}, {(repr(bla))}, {(ascii(bla))}" # RUF010 +14 14 | +15 |-f"{bla!s}, {(repr(bla))}, {(ascii(bla))}" # RUF010 + 15 |+f"{bla}, {(repr(bla))}, {(ascii(bla))}" # RUF010 +16 16 | +17 17 | f"{foo(bla)}" # OK +18 18 | + +RUF010.py:15:14: RUF010 [*] Use explicit conversion flag | 15 | f"{(str(bla))}, {(repr(bla))}, {(ascii(bla))}" # RUF010 16 | @@ -193,7 +214,7 @@ RUF010.py:15:14: RUF010 [*] Use conversion in f-string 18 | 19 | f"{foo(bla)}" # OK | - = help: Replace f-string function call with conversion + = help: Replace with conversion flag ℹ Fix 12 12 | @@ -205,7 +226,7 @@ RUF010.py:15:14: RUF010 [*] Use conversion in f-string 17 17 | f"{foo(bla)}" # OK 18 18 | -RUF010.py:15:29: RUF010 [*] Use conversion in f-string +RUF010.py:15:29: RUF010 [*] Use explicit conversion flag | 15 | f"{(str(bla))}, {(repr(bla))}, {(ascii(bla))}" # RUF010 16 | @@ -214,7 +235,7 @@ RUF010.py:15:29: RUF010 [*] Use conversion in f-string 18 | 19 | f"{foo(bla)}" # OK | - = help: Replace f-string function call with conversion + = help: Replace with conversion flag ℹ Fix 12 12 | @@ -226,7 +247,28 @@ RUF010.py:15:29: RUF010 [*] Use conversion in f-string 17 17 | f"{foo(bla)}" # OK 18 18 | -RUF010.py:35:20: RUF010 [*] Use conversion in f-string +RUF010.py:21:4: RUF010 [*] Remove unnecessary conversion flag + | +21 | f"{str(bla, 'ascii')}, {str(bla, encoding='cp1255')}" # OK +22 | +23 | f"{bla!s} {[]!r} {'bar'!a}" # OK + | ^^^ RUF010 +24 | +25 | "Not an f-string {str(bla)}, {repr(bla)}, {ascii(bla)}" # OK + | + = help: Remove conversion flag + +ℹ Fix +18 18 | +19 19 | f"{str(bla, 'ascii')}, {str(bla, encoding='cp1255')}" # OK +20 20 | +21 |-f"{bla!s} {[]!r} {'bar'!a}" # OK + 21 |+f"{bla} {[]!r} {'bar'!a}" # OK +22 22 | +23 23 | "Not an f-string {str(bla)}, {repr(bla)}, {ascii(bla)}" # OK +24 24 | + +RUF010.py:35:20: RUF010 [*] Use explicit conversion flag | 35 | f"Member of tuple mismatches type at index {i}. Expected {of_shape_i}. Got " 36 | " intermediary content " @@ -234,7 +276,7 @@ RUF010.py:35:20: RUF010 [*] Use conversion in f-string | ^^^^^^^^^ RUF010 38 | ) | - = help: Replace f-string function call with conversion + = help: Replace with conversion flag ℹ Fix 32 32 | ( @@ -243,5 +285,67 @@ RUF010.py:35:20: RUF010 [*] Use conversion in f-string 35 |- f" that flows {repr(obj)} of type {type(obj)}.{additional_message}" # RUF010 35 |+ f" that flows {obj!r} of type {type(obj)}.{additional_message}" # RUF010 36 36 | ) +37 37 | +38 38 | + +RUF010.py:39:4: RUF010 [*] Remove unnecessary `str` conversion + | +39 | f"{str(bla)}" # RUF010 + | ^^^^^^^^ RUF010 +40 | +41 | f"{str(bla):20}" # RUF010 + | + = help: Remove `str` call + +ℹ Fix +36 36 | ) +37 37 | +38 38 | +39 |-f"{str(bla)}" # RUF010 + 39 |+f"{bla}" # RUF010 +40 40 | +41 41 | f"{str(bla):20}" # RUF010 +42 42 | + +RUF010.py:41:4: RUF010 [*] Use explicit conversion flag + | +41 | f"{str(bla)}" # RUF010 +42 | +43 | f"{str(bla):20}" # RUF010 + | ^^^^^^^^ RUF010 +44 | +45 | f"{bla!s}" # RUF010 + | + = help: Replace with conversion flag + +ℹ Fix +38 38 | +39 39 | f"{str(bla)}" # RUF010 +40 40 | +41 |-f"{str(bla):20}" # RUF010 + 41 |+f"{bla!s:20}" # RUF010 +42 42 | +43 43 | f"{bla!s}" # RUF010 +44 44 | + +RUF010.py:43:4: RUF010 [*] Remove unnecessary conversion flag + | +43 | f"{str(bla):20}" # RUF010 +44 | +45 | f"{bla!s}" # RUF010 + | ^^^ RUF010 +46 | +47 | f"{bla!s:20}" # OK + | + = help: Remove conversion flag + +ℹ Fix +40 40 | +41 41 | f"{str(bla):20}" # RUF010 +42 42 | +43 |-f"{bla!s}" # RUF010 + 43 |+f"{bla}" # RUF010 +44 44 | +45 45 | f"{bla!s:20}" # OK