diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF010.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF010.py index 031e08412f..af7e596dbf 100644 --- a/crates/ruff_linter/resources/test/fixtures/ruff/RUF010.py +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF010.py @@ -36,5 +36,19 @@ f"{ascii(bla)}" # OK ) -# OK +# https://github.com/astral-sh/ruff/issues/16325 f"{str({})}" + +f"{str({} | {})}" + +import builtins + +f"{builtins.repr(1)}" + +f"{repr(1)=}" + +f"{repr(lambda: 1)}" + +f"{repr(x := 2)}" + +f"{str(object=3)}" diff --git a/crates/ruff_linter/src/cst/matchers.rs b/crates/ruff_linter/src/cst/matchers.rs index 3f4e32cca1..9e6ed0f613 100644 --- a/crates/ruff_linter/src/cst/matchers.rs +++ b/crates/ruff_linter/src/cst/matchers.rs @@ -3,8 +3,8 @@ use anyhow::{Result, bail}; use libcst_native::{ Arg, Attribute, Call, Comparison, CompoundStatement, Dict, Expression, FormattedString, FormattedStringContent, FormattedStringExpression, FunctionDef, GeneratorExp, If, Import, - ImportAlias, ImportFrom, ImportNames, IndentedBlock, Lambda, ListComp, Module, Name, - SmallStatement, Statement, Suite, Tuple, With, + ImportAlias, ImportFrom, ImportNames, IndentedBlock, Lambda, ListComp, Module, SmallStatement, + Statement, Suite, Tuple, With, }; use ruff_python_codegen::Stylist; @@ -104,14 +104,6 @@ pub(crate) fn match_attribute<'a, 'b>( } } -pub(crate) fn match_name<'a, 'b>(expression: &'a Expression<'b>) -> Result<&'a Name<'b>> { - if let Expression::Name(name) = expression { - Ok(name) - } else { - bail!("Expected Expression::Name") - } -} - pub(crate) fn match_arg<'a, 'b>(call: &'a Call<'b>) -> Result<&'a Arg<'b>> { if let Some(arg) = call.args.first() { Ok(arg) diff --git a/crates/ruff_linter/src/rules/ruff/rules/explicit_f_string_type_conversion.rs b/crates/ruff_linter/src/rules/ruff/rules/explicit_f_string_type_conversion.rs index 0d396bdf50..e3af51eee2 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/explicit_f_string_type_conversion.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/explicit_f_string_type_conversion.rs @@ -1,17 +1,19 @@ -use anyhow::{Result, bail}; +use std::fmt::Display; +use anyhow::Result; + +use libcst_native::{LeftParen, ParenthesizedNode, RightParen}; use ruff_macros::{ViolationMetadata, derive_message_formats}; -use ruff_python_ast::{self as ast, Arguments, Expr}; -use ruff_python_codegen::Stylist; +use ruff_python_ast::{self as ast, Expr, OperatorPrecedence}; +use ruff_python_parser::TokenKind; use ruff_text_size::Ranged; -use crate::Locator; use crate::checkers::ast::Checker; +use crate::cst::helpers::space; use crate::cst::matchers::{ - match_call_mut, match_formatted_string, match_formatted_string_expression, match_name, - transform_expression, + match_call_mut, match_formatted_string, match_formatted_string_expression, transform_expression, }; -use crate::{AlwaysFixableViolation, Edit, Fix}; +use crate::{Edit, Fix, FixAvailability, Violation}; /// ## What it does /// Checks for uses of `str()`, `repr()`, and `ascii()` as explicit type @@ -40,14 +42,16 @@ use crate::{AlwaysFixableViolation, Edit, Fix}; #[derive(ViolationMetadata)] pub(crate) struct ExplicitFStringTypeConversion; -impl AlwaysFixableViolation for ExplicitFStringTypeConversion { +impl Violation for ExplicitFStringTypeConversion { + const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes; + #[derive_message_formats] fn message(&self) -> String { "Use explicit conversion flag".to_string() } - fn fix_title(&self) -> String { - "Replace with conversion flag".to_string() + fn fix_title(&self) -> Option { + Some("Replace with conversion flag".to_string()) } } @@ -68,84 +72,142 @@ pub(crate) fn explicit_f_string_type_conversion(checker: &Checker, f_string: &as continue; } - let Expr::Call(ast::ExprCall { - func, - arguments: - Arguments { - args, - keywords, - range: _, - node_index: _, - }, - .. - }) = expression.as_ref() + let Expr::Call(call) = expression.as_ref() else { + continue; + }; + + let Some(conversion) = checker + .semantic() + .resolve_builtin_symbol(&call.func) + .and_then(Conversion::from_str) else { continue; }; + let arg = match conversion { + // Handles the cases: `f"{str(object=arg)}"` and `f"{str(arg)}"` + Conversion::Str if call.arguments.len() == 1 => { + let Some(arg) = call.arguments.find_argument_value("object", 0) else { + continue; + }; + arg + } + Conversion::Str | Conversion::Repr | Conversion::Ascii => { + // Can't be a conversion otherwise. + if !call.arguments.keywords.is_empty() { + continue; + } - // Can't be a conversion otherwise. - if !keywords.is_empty() { - continue; - } - - // Can't be a conversion otherwise. - let [arg] = &**args else { - continue; + // Can't be a conversion otherwise. + let [arg] = call.arguments.args.as_ref() else { + continue; + }; + arg + } }; - // Avoid attempting to rewrite, e.g., `f"{str({})}"`; the curly braces are problematic. - if matches!( - arg, - Expr::Dict(_) | Expr::Set(_) | Expr::DictComp(_) | Expr::SetComp(_) - ) { - continue; - } - - if !checker - .semantic() - .resolve_builtin_symbol(func) - .is_some_and(|builtin| matches!(builtin, "str" | "repr" | "ascii")) - { - continue; + // Suppress lint for starred expressions. + if arg.is_starred_expr() { + return; } let mut diagnostic = checker.report_diagnostic(ExplicitFStringTypeConversion, expression.range()); + + // Don't support fixing f-string with debug text. + if element + .as_interpolation() + .is_some_and(|interpolation| interpolation.debug_text.is_some()) + { + return; + } + diagnostic.try_set_fix(|| { - convert_call_to_conversion_flag(f_string, index, checker.locator(), checker.stylist()) + convert_call_to_conversion_flag(checker, conversion, f_string, index, arg) }); } } /// Generate a [`Fix`] to replace an explicit type conversion with a conversion flag. fn convert_call_to_conversion_flag( + checker: &Checker, + conversion: Conversion, f_string: &ast::FString, index: usize, - locator: &Locator, - stylist: &Stylist, + arg: &Expr, ) -> Result { - let source_code = locator.slice(f_string); - transform_expression(source_code, stylist, |mut expression| { + let source_code = checker.locator().slice(f_string); + transform_expression(source_code, checker.stylist(), |mut expression| { let formatted_string = match_formatted_string(&mut expression)?; // Replace the formatted call expression at `index` with a conversion flag. let formatted_string_expression = match_formatted_string_expression(&mut formatted_string.parts[index])?; let call = match_call_mut(&mut formatted_string_expression.expression)?; - let name = match_name(&call.func)?; - match name.value { - "str" => { - formatted_string_expression.conversion = Some("s"); - } - "repr" => { - formatted_string_expression.conversion = Some("r"); - } - "ascii" => { - formatted_string_expression.conversion = Some("a"); - } - _ => bail!("Unexpected function call: `{:?}`", name.value), + + formatted_string_expression.conversion = Some(conversion.as_str()); + + if starts_with_brace(checker, arg) { + formatted_string_expression.whitespace_before_expression = space(); } - formatted_string_expression.expression = call.args[0].value.clone(); + + formatted_string_expression.expression = if needs_paren(OperatorPrecedence::from_expr(arg)) + { + call.args[0] + .value + .clone() + .with_parens(LeftParen::default(), RightParen::default()) + } else { + call.args[0].value.clone() + }; + Ok(expression) }) .map(|output| Fix::safe_edit(Edit::range_replacement(output, f_string.range()))) } + +fn starts_with_brace(checker: &Checker, arg: &Expr) -> bool { + checker + .tokens() + .in_range(arg.range()) + .iter() + // Skip the trivia tokens + .find(|token| !token.kind().is_trivia()) + .is_some_and(|token| matches!(token.kind(), TokenKind::Lbrace)) +} + +fn needs_paren(precedence: OperatorPrecedence) -> bool { + precedence <= OperatorPrecedence::Lambda +} + +/// Represents the three built-in Python conversion functions that can be replaced +/// with f-string conversion flags. +#[derive(Copy, Clone)] +enum Conversion { + Ascii, + Str, + Repr, +} + +impl Conversion { + fn from_str(value: &str) -> Option { + Some(match value { + "ascii" => Self::Ascii, + "str" => Self::Str, + "repr" => Self::Repr, + _ => return None, + }) + } + + fn as_str(self) -> &'static str { + match self { + Conversion::Ascii => "a", + Conversion::Str => "s", + Conversion::Repr => "r", + } + } +} + +impl Display for Conversion { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.as_str()) + } +} diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF010_RUF010.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF010_RUF010.py.snap index 1910bdf6f9..7c3cdf80a6 100644 --- a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF010_RUF010.py.snap +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF010_RUF010.py.snap @@ -244,4 +244,134 @@ RUF010.py:35:20: RUF010 [*] Use explicit conversion flag 35 |+ f" that flows {obj!r} of type {type(obj)}.{additional_message}" # RUF010 36 36 | ) 37 37 | -38 38 | +38 38 | + +RUF010.py:40:4: RUF010 [*] Use explicit conversion flag + | +39 | # https://github.com/astral-sh/ruff/issues/16325 +40 | f"{str({})}" + | ^^^^^^^ RUF010 +41 | +42 | f"{str({} | {})}" + | + = help: Replace with conversion flag + +ℹ Safe fix +37 37 | +38 38 | +39 39 | # https://github.com/astral-sh/ruff/issues/16325 +40 |-f"{str({})}" + 40 |+f"{ {}!s}" +41 41 | +42 42 | f"{str({} | {})}" +43 43 | + +RUF010.py:42:4: RUF010 [*] Use explicit conversion flag + | +40 | f"{str({})}" +41 | +42 | f"{str({} | {})}" + | ^^^^^^^^^^^^ RUF010 +43 | +44 | import builtins + | + = help: Replace with conversion flag + +ℹ Safe fix +39 39 | # https://github.com/astral-sh/ruff/issues/16325 +40 40 | f"{str({})}" +41 41 | +42 |-f"{str({} | {})}" + 42 |+f"{ {} | {}!s}" +43 43 | +44 44 | import builtins +45 45 | + +RUF010.py:46:4: RUF010 [*] Use explicit conversion flag + | +44 | import builtins +45 | +46 | f"{builtins.repr(1)}" + | ^^^^^^^^^^^^^^^^ RUF010 +47 | +48 | f"{repr(1)=}" + | + = help: Replace with conversion flag + +ℹ Safe fix +43 43 | +44 44 | import builtins +45 45 | +46 |-f"{builtins.repr(1)}" + 46 |+f"{1!r}" +47 47 | +48 48 | f"{repr(1)=}" +49 49 | + +RUF010.py:48:4: RUF010 Use explicit conversion flag + | +46 | f"{builtins.repr(1)}" +47 | +48 | f"{repr(1)=}" + | ^^^^^^^ RUF010 +49 | +50 | f"{repr(lambda: 1)}" + | + = help: Replace with conversion flag + +RUF010.py:50:4: RUF010 [*] Use explicit conversion flag + | +48 | f"{repr(1)=}" +49 | +50 | f"{repr(lambda: 1)}" + | ^^^^^^^^^^^^^^^ RUF010 +51 | +52 | f"{repr(x := 2)}" + | + = help: Replace with conversion flag + +ℹ Safe fix +47 47 | +48 48 | f"{repr(1)=}" +49 49 | +50 |-f"{repr(lambda: 1)}" + 50 |+f"{(lambda: 1)!r}" +51 51 | +52 52 | f"{repr(x := 2)}" +53 53 | + +RUF010.py:52:4: RUF010 [*] Use explicit conversion flag + | +50 | f"{repr(lambda: 1)}" +51 | +52 | f"{repr(x := 2)}" + | ^^^^^^^^^^^^ RUF010 +53 | +54 | f"{str(object=3)}" + | + = help: Replace with conversion flag + +ℹ Safe fix +49 49 | +50 50 | f"{repr(lambda: 1)}" +51 51 | +52 |-f"{repr(x := 2)}" + 52 |+f"{(x := 2)!r}" +53 53 | +54 54 | f"{str(object=3)}" + +RUF010.py:54:4: RUF010 [*] Use explicit conversion flag + | +52 | f"{repr(x := 2)}" +53 | +54 | f"{str(object=3)}" + | ^^^^^^^^^^^^^ RUF010 + | + = help: Replace with conversion flag + +ℹ Safe fix +51 51 | +52 52 | f"{repr(x := 2)}" +53 53 | +54 |-f"{str(object=3)}" + 54 |+f"{3!s}"