diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF010.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF010.py index 765bb412a4826..ef2b08af91ae5 100644 --- a/crates/ruff_linter/resources/test/fixtures/ruff/RUF010.py +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF010.py @@ -23,11 +23,11 @@ def foo(one_arg): "Not an f-string {str(bla)}, {repr(bla)}, {ascii(bla)}" # OK -def ascii(arg): - pass - +def ascii_shadowing(): + def ascii(arg): + pass -f"{ascii(bla)}" # OK + f"{ascii(bla)}" # OK ( f"Member of tuple mismatches type at index {i}. Expected {of_shape_i}. Got " @@ -64,3 +64,59 @@ def ascii(arg): f"{str('hello')=}" f"{ascii('hello')=}" f"{repr('hello')=}" + +# Fix should be unsafe when it deletes a comment (https://github.com/astral-sh/ruff/issues/19745) +f"{ascii( + # comment + 1 +)}" + +f"{repr( + # comment + 1 +)}" + +f"{str( + # comment + 1 +)}" + +# Fix should be unsafe when it deletes comments after the argument +f"{ascii(1 # comment +)}" + +f"{repr(( + 1 +) # comment +)}" + +f"{str(( + 1 +) + # comment +)}" + +# Fix should be safe when the comment is preserved inside extra parentheses +f"{ascii(( + # comment + 1 +))}" + +f"{repr(( + 1 # comment +))}" + +f"{repr(( + 1 + # comment +))}" + +f"{repr(( + # comment + 1 +))}" + +f"{str(( + # comment + 1 +))}" 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 2ba444d6b9245..b5dea310a2401 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 @@ -4,16 +4,16 @@ use anyhow::Result; use libcst_native::{LeftParen, ParenthesizedNode, RightParen}; use ruff_macros::{ViolationMetadata, derive_message_formats}; -use ruff_python_ast::token::TokenKind; +use ruff_python_ast::token::{TokenKind, parenthesized_range}; use ruff_python_ast::{self as ast, Expr, OperatorPrecedence}; -use ruff_text_size::Ranged; +use ruff_text_size::{Ranged, TextRange}; use crate::checkers::ast::Checker; use crate::cst::helpers::space; use crate::cst::matchers::{ match_call_mut, match_formatted_string, match_formatted_string_expression, transform_expression, }; -use crate::{Edit, Fix, FixAvailability, Violation}; +use crate::{Applicability, Edit, Fix, FixAvailability, Violation}; /// ## What it does /// Checks for uses of `str()`, `repr()`, and `ascii()` as explicit type @@ -39,6 +39,11 @@ use crate::{Edit, Fix, FixAvailability, Violation}; /// a = "some string" /// f"{a!r}" /// ``` +/// +/// ## Fix safety +/// +/// This rule's fix is marked as unsafe if the call expression contains +/// comments that would be deleted by applying the fix. #[derive(ViolationMetadata)] #[violation_metadata(stable_since = "v0.0.267")] pub(crate) struct ExplicitFStringTypeConversion; @@ -123,7 +128,7 @@ pub(crate) fn explicit_f_string_type_conversion(checker: &Checker, f_string: &as checker.report_diagnostic(ExplicitFStringTypeConversion, expression.range()); diagnostic.try_set_fix(|| { - convert_call_to_conversion_flag(checker, conversion, f_string, index, arg) + convert_call_to_conversion_flag(checker, conversion, f_string, index, call, arg) }); } } @@ -134,15 +139,16 @@ fn convert_call_to_conversion_flag( conversion: Conversion, f_string: &ast::FString, index: usize, + call: &ast::ExprCall, arg: &Expr, ) -> Result { let source_code = checker.locator().slice(f_string); - transform_expression(source_code, checker.stylist(), |mut expression| { + let output = 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 call_cst = match_call_mut(&mut formatted_string_expression.expression)?; formatted_string_expression.conversion = Some(conversion.as_str()); @@ -151,17 +157,45 @@ fn convert_call_to_conversion_flag( } formatted_string_expression.expression = if needs_paren_expr(arg) { - call.args[0] + call_cst.args[0] .value .clone() .with_parens(LeftParen::default(), RightParen::default()) } else { - call.args[0].value.clone() + call_cst.args[0].value.clone() }; Ok(expression) - }) - .map(|output| Fix::safe_edit(Edit::range_replacement(output, f_string.range()))) + })?; + + // Determine applicability: mark the fix as unsafe if there are comments in the + // call expression that fall outside the effective argument range (i.e., comments + // that would be deleted by replacing the call with a conversion flag). + // + // Extra parentheses wrapping the argument are preserved by the libcst transformation + // (e.g., `ascii((arg))` → `(arg)!a`), so comments inside them are not deleted. + let comment_ranges = checker.comment_ranges(); + let call_range = call.range(); + // Use the parenthesized range of the arg (within call.arguments) to account for + // any extra parens that wrap the argument and whose content will be preserved. + let effective_arg_range = + parenthesized_range(arg.into(), (&call.arguments).into(), checker.tokens()) + .unwrap_or_else(|| arg.range()); + let has_deletable_comments = comment_ranges.intersects(TextRange::new( + call_range.start(), + effective_arg_range.start(), + )) || comment_ranges + .intersects(TextRange::new(effective_arg_range.end(), call_range.end())); + let applicability = if has_deletable_comments { + Applicability::Unsafe + } else { + Applicability::Safe + }; + + Ok(Fix::applicable_edit( + Edit::range_replacement(output, f_string.range()), + applicability, + )) } fn starts_with_brace(checker: &Checker, arg: &Expr) -> bool { 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 8ef03b90fc7b3..3cf3700d89d5d 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 @@ -392,3 +392,301 @@ help: Replace with conversion flag 59 | 60 | # Debug text cases - should not trigger RUF010 61 | f"{str(1)=}" + +RUF010 [*] Use explicit conversion flag + --> RUF010.py:69:4 + | +68 | # Fix should be unsafe when it deletes a comment (https://github.com/astral-sh/ruff/issues/19745) +69 | f"{ascii( + | ____^ +70 | | # comment +71 | | 1 +72 | | )}" + | |_^ +73 | +74 | f"{repr( + | +help: Replace with conversion flag +66 | f"{repr('hello')=}" +67 | +68 | # Fix should be unsafe when it deletes a comment (https://github.com/astral-sh/ruff/issues/19745) + - f"{ascii( + - # comment + - 1 + - )}" +69 + f"{1!a}" +70 | +71 | f"{repr( +72 | # comment +note: This is an unsafe fix and may change runtime behavior + +RUF010 [*] Use explicit conversion flag + --> RUF010.py:74:4 + | +72 | )}" +73 | +74 | f"{repr( + | ____^ +75 | | # comment +76 | | 1 +77 | | )}" + | |_^ +78 | +79 | f"{str( + | +help: Replace with conversion flag +71 | 1 +72 | )}" +73 | + - f"{repr( + - # comment + - 1 + - )}" +74 + f"{1!r}" +75 | +76 | f"{str( +77 | # comment +note: This is an unsafe fix and may change runtime behavior + +RUF010 [*] Use explicit conversion flag + --> RUF010.py:79:4 + | +77 | )}" +78 | +79 | f"{str( + | ____^ +80 | | # comment +81 | | 1 +82 | | )}" + | |_^ +83 | +84 | # Fix should be unsafe when it deletes comments after the argument + | +help: Replace with conversion flag +76 | 1 +77 | )}" +78 | + - f"{str( + - # comment + - 1 + - )}" +79 + f"{1!s}" +80 | +81 | # Fix should be unsafe when it deletes comments after the argument +82 | f"{ascii(1 # comment +note: This is an unsafe fix and may change runtime behavior + +RUF010 [*] Use explicit conversion flag + --> RUF010.py:85:4 + | +84 | # Fix should be unsafe when it deletes comments after the argument +85 | f"{ascii(1 # comment + | ____^ +86 | | )}" + | |_^ +87 | +88 | f"{repr(( + | +help: Replace with conversion flag +82 | )}" +83 | +84 | # Fix should be unsafe when it deletes comments after the argument + - f"{ascii(1 # comment + - )}" +85 + f"{1!a}" +86 | +87 | f"{repr(( +88 | 1 +note: This is an unsafe fix and may change runtime behavior + +RUF010 [*] Use explicit conversion flag + --> RUF010.py:88:4 + | +86 | )}" +87 | +88 | f"{repr(( + | ____^ +89 | | 1 +90 | | ) # comment +91 | | )}" + | |_^ +92 | +93 | f"{str(( + | +help: Replace with conversion flag +85 | f"{ascii(1 # comment +86 | )}" +87 | + - f"{repr(( +88 + f"{( +89 | 1 + - ) # comment + - )}" +90 + )!r}" +91 | +92 | f"{str(( +93 | 1 +note: This is an unsafe fix and may change runtime behavior + +RUF010 [*] Use explicit conversion flag + --> RUF010.py:93:4 + | +91 | )}" +92 | +93 | f"{str(( + | ____^ +94 | | 1 +95 | | ) +96 | | # comment +97 | | )}" + | |_^ +98 | +99 | # Fix should be safe when the comment is preserved inside extra parentheses + | +help: Replace with conversion flag +90 | ) # comment +91 | )}" +92 | + - f"{str(( +93 + f"{( +94 | 1 + - ) + - # comment + - )}" +95 + )!s}" +96 | +97 | # Fix should be safe when the comment is preserved inside extra parentheses +98 | f"{ascii(( +note: This is an unsafe fix and may change runtime behavior + +RUF010 [*] Use explicit conversion flag + --> RUF010.py:100:4 + | + 99 | # Fix should be safe when the comment is preserved inside extra parentheses +100 | f"{ascii(( + | ____^ +101 | | # comment +102 | | 1 +103 | | ))}" + | |__^ +104 | +105 | f"{repr(( + | +help: Replace with conversion flag +97 | )}" +98 | +99 | # Fix should be safe when the comment is preserved inside extra parentheses + - f"{ascii(( +100 + f"{( +101 | # comment +102 | 1 + - ))}" +103 + )!a}" +104 | +105 | f"{repr(( +106 | 1 # comment + +RUF010 [*] Use explicit conversion flag + --> RUF010.py:105:4 + | +103 | ))}" +104 | +105 | f"{repr(( + | ____^ +106 | | 1 # comment +107 | | ))}" + | |__^ +108 | +109 | f"{repr(( + | +help: Replace with conversion flag +102 | 1 +103 | ))}" +104 | + - f"{repr(( +105 + f"{( +106 | 1 # comment + - ))}" +107 + )!r}" +108 | +109 | f"{repr(( +110 | 1 + +RUF010 [*] Use explicit conversion flag + --> RUF010.py:109:4 + | +107 | ))}" +108 | +109 | f"{repr(( + | ____^ +110 | | 1 +111 | | # comment +112 | | ))}" + | |__^ +113 | +114 | f"{repr(( + | +help: Replace with conversion flag +106 | 1 # comment +107 | ))}" +108 | + - f"{repr(( +109 + f"{( +110 | 1 +111 | # comment + - ))}" +112 + )!r}" +113 | +114 | f"{repr(( +115 | # comment + +RUF010 [*] Use explicit conversion flag + --> RUF010.py:114:4 + | +112 | ))}" +113 | +114 | f"{repr(( + | ____^ +115 | | # comment +116 | | 1 +117 | | ))}" + | |__^ +118 | +119 | f"{str(( + | +help: Replace with conversion flag +111 | # comment +112 | ))}" +113 | + - f"{repr(( +114 + f"{( +115 | # comment +116 | 1 + - ))}" +117 + )!r}" +118 | +119 | f"{str(( +120 | # comment + +RUF010 [*] Use explicit conversion flag + --> RUF010.py:119:4 + | +117 | ))}" +118 | +119 | f"{str(( + | ____^ +120 | | # comment +121 | | 1 +122 | | ))}" + | |__^ + | +help: Replace with conversion flag +116 | 1 +117 | ))}" +118 | + - f"{str(( +119 + f"{( +120 | # comment +121 | 1 + - ))}" +122 + )!s}"