Avoid enforcing native-literals rule within nested f-strings (#4488)

crates/ruff/resources/test/fixtures/pyupgrade/UP018.py

@@ -15,6 +15,7 @@ bytes("foo", **a)
 bytes(b"foo"
       b"bar")
 bytes("foo")
+f"{f'{str()}'}"
 
 # These become string or byte literals
 str()

crates/ruff/src/checkers/ast/mod.rs

@@ -116,13 +116,12 @@ impl<'a> Checker<'a> {
 }
 
 impl<'a> Checker<'a> {
-    /// Return `true` if a patch should be generated under the given autofix
+    /// Return `true` if a patch should be generated for a given [`Rule`].
-    /// `Mode`.
     pub(crate) fn patch(&self, code: Rule) -> bool {
         self.settings.rules.should_fix(code)
     }
 
-    /// Return `true` if a `Rule` is disabled by a `noqa` directive.
+    /// Return `true` if a [`Rule`] is disabled by a `noqa` directive.
     pub(crate) fn rule_is_ignored(&self, code: Rule, offset: TextSize) -> bool {
         // TODO(charlie): `noqa` directives are mostly enforced in `check_lines.rs`.
         // However, in rare cases, we need to check them here. For example, when
@@ -3977,7 +3976,11 @@ where
                 }
             }
             Expr::JoinedStr(_) => {
-                self.semantic_model.flags |= SemanticModelFlags::F_STRING;
+                self.semantic_model.flags |= if self.semantic_model.in_f_string() {
+                    SemanticModelFlags::NESTED_F_STRING
+                } else {
+                    SemanticModelFlags::F_STRING
+                };
                 visitor::walk_expr(self, expr);
             }
             _ => visitor::walk_expr(self, expr),

crates/ruff/src/rules/pyupgrade/rules/native_literals.rs

@@ -38,7 +38,10 @@ impl AlwaysAutofixableViolation for NativeLiterals {
 
     fn autofix_title(&self) -> String {
         let NativeLiterals { literal_type } = self;
-        format!("Replace with `{literal_type}`")
+        match literal_type {
+            LiteralType::Str => "Replace with empty string".to_string(),
+            LiteralType::Bytes => "Replace with empty bytes".to_string(),
+        }
     }
 }
 
@@ -50,12 +53,19 @@ pub(crate) fn native_literals(
     args: &[Expr],
     keywords: &[Keyword],
 ) {
-    let Expr::Name(ast::ExprName { id, .. }) = func else { return; };
+    let Expr::Name(ast::ExprName { id, .. }) = func else {
+        return;
+    };
 
     if !keywords.is_empty() || args.len() > 1 {
         return;
     }
 
+    // There's no way to rewrite, e.g., `f"{f'{str()}'}"` within a nested f-string.
+    if checker.semantic_model().in_nested_f_string() {
+        return;
+    }
+
     if (id == "str" || id == "bytes") && checker.semantic_model().is_builtin(id) {
         let Some(arg) = args.get(0) else {
             let mut diagnostic = Diagnostic::new(NativeLiterals{literal_type:if id == "str" {

crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP018.py.snap

@@ -1,148 +1,148 @@
 ---
 source: crates/ruff/src/rules/pyupgrade/mod.rs
 ---
-UP018.py:20:1: UP018 [*] Unnecessary call to `str`
-   |
-20 | # These become string or byte literals
-21 | str()
-   | ^^^^^ UP018
-22 | str("foo")
-23 | str("""
-   |
-   = help: Replace with `str`
-
-ℹ Suggested fix
-17 17 | bytes("foo")
-18 18 | 
-19 19 | # These become string or byte literals
-20    |-str()
-   20 |+""
-21 21 | str("foo")
-22 22 | str("""
-23 23 | foo""")
-
 UP018.py:21:1: UP018 [*] Unnecessary call to `str`
    |
 21 | # These become string or byte literals
 22 | str()
+   | ^^^^^ UP018
 23 | str("foo")
-   | ^^^^^^^^^^ UP018
 24 | str("""
-25 | foo""")
    |
-   = help: Replace with `str`
+   = help: Replace with empty string
 
 ℹ Suggested fix
-18 18 | 
+18 18 | f"{f'{str()}'}"
-19 19 | # These become string or byte literals
+19 19 | 
-20 20 | str()
+20 20 | # These become string or byte literals
-21    |-str("foo")
+21    |-str()
-   21 |+"foo"
+   21 |+""
-22 22 | str("""
+22 22 | str("foo")
-23 23 | foo""")
+23 23 | str("""
-24 24 | bytes()
+24 24 | foo""")
 
 UP018.py:22:1: UP018 [*] Unnecessary call to `str`
    |
-22 |   str()
+22 | # These become string or byte literals
-23 |   str("foo")
+23 | str()
-24 | / str("""
+24 | str("foo")
-25 | | foo""")
+   | ^^^^^^^^^^ UP018
+25 | str("""
+26 | foo""")
+   |
+   = help: Replace with empty string
+
+ℹ Suggested fix
+19 19 | 
+20 20 | # These become string or byte literals
+21 21 | str()
+22    |-str("foo")
+   22 |+"foo"
+23 23 | str("""
+24 24 | foo""")
+25 25 | bytes()
+
+UP018.py:23:1: UP018 [*] Unnecessary call to `str`
+   |
+23 |   str()
+24 |   str("foo")
+25 | / str("""
+26 | | foo""")
    | |_______^ UP018
-26 |   bytes()
+27 |   bytes()
-27 |   bytes(b"foo")
+28 |   bytes(b"foo")
    |
-   = help: Replace with `str`
+   = help: Replace with empty string
 
 ℹ Suggested fix
-19 19 | # These become string or byte literals
+20 20 | # These become string or byte literals
-20 20 | str()
+21 21 | str()
-21 21 | str("foo")
+22 22 | str("foo")
-22    |-str("""
+23    |-str("""
-23    |-foo""")
+24    |-foo""")
-   22 |+"""
+   23 |+"""
-   23 |+foo"""
+   24 |+foo"""
-24 24 | bytes()
+25 25 | bytes()
-25 25 | bytes(b"foo")
+26 26 | bytes(b"foo")
-26 26 | bytes(b"""
+27 27 | bytes(b"""
-
-UP018.py:24:1: UP018 [*] Unnecessary call to `bytes`
-   |
-24 | str("""
-25 | foo""")
-26 | bytes()
-   | ^^^^^^^ UP018
-27 | bytes(b"foo")
-28 | bytes(b"""
-   |
-   = help: Replace with `bytes`
-
-ℹ Suggested fix
-21 21 | str("foo")
-22 22 | str("""
-23 23 | foo""")
-24    |-bytes()
-   24 |+b""
-25 25 | bytes(b"foo")
-26 26 | bytes(b"""
-27 27 | foo""")
 
 UP018.py:25:1: UP018 [*] Unnecessary call to `bytes`
    |
-25 | foo""")
+25 | str("""
-26 | bytes()
+26 | foo""")
-27 | bytes(b"foo")
+27 | bytes()
-   | ^^^^^^^^^^^^^ UP018
+   | ^^^^^^^ UP018
-28 | bytes(b"""
+28 | bytes(b"foo")
-29 | foo""")
+29 | bytes(b"""
    |
-   = help: Replace with `bytes`
+   = help: Replace with empty bytes
 
 ℹ Suggested fix
-22 22 | str("""
+22 22 | str("foo")
-23 23 | foo""")
+23 23 | str("""
-24 24 | bytes()
+24 24 | foo""")
-25    |-bytes(b"foo")
+25    |-bytes()
-   25 |+b"foo"
+   25 |+b""
-26 26 | bytes(b"""
+26 26 | bytes(b"foo")
-27 27 | foo""")
+27 27 | bytes(b"""
-28 28 | f"{str()}"
+28 28 | foo""")
 
 UP018.py:26:1: UP018 [*] Unnecessary call to `bytes`
    |
-26 |   bytes()
+26 | foo""")
-27 |   bytes(b"foo")
+27 | bytes()
-28 | / bytes(b"""
+28 | bytes(b"foo")
-29 | | foo""")
+   | ^^^^^^^^^^^^^ UP018
-   | |_______^ UP018
+29 | bytes(b"""
-30 |   f"{str()}"
+30 | foo""")
    |
-   = help: Replace with `bytes`
+   = help: Replace with empty bytes
 
 ℹ Suggested fix
-23 23 | foo""")
+23 23 | str("""
-24 24 | bytes()
+24 24 | foo""")
-25 25 | bytes(b"foo")
+25 25 | bytes()
-26    |-bytes(b"""
+26    |-bytes(b"foo")
-27    |-foo""")
+   26 |+b"foo"
-   26 |+b"""
+27 27 | bytes(b"""
-   27 |+foo"""
+28 28 | foo""")
-28 28 | f"{str()}"
+29 29 | f"{str()}"
 
-UP018.py:28:4: UP018 [*] Unnecessary call to `str`
+UP018.py:27:1: UP018 [*] Unnecessary call to `bytes`
    |
-28 | bytes(b"""
+27 |   bytes()
-29 | foo""")
+28 |   bytes(b"foo")
-30 | f"{str()}"
+29 | / bytes(b"""
+30 | | foo""")
+   | |_______^ UP018
+31 |   f"{str()}"
+   |
+   = help: Replace with empty bytes
+
+ℹ Suggested fix
+24 24 | foo""")
+25 25 | bytes()
+26 26 | bytes(b"foo")
+27    |-bytes(b"""
+28    |-foo""")
+   27 |+b"""
+   28 |+foo"""
+29 29 | f"{str()}"
+
+UP018.py:29:4: UP018 [*] Unnecessary call to `str`
+   |
+29 | bytes(b"""
+30 | foo""")
+31 | f"{str()}"
    |    ^^^^^ UP018
    |
-   = help: Replace with `str`
+   = help: Replace with empty string
 
 ℹ Suggested fix
-25 25 | bytes(b"foo")
+26 26 | bytes(b"foo")
-26 26 | bytes(b"""
+27 27 | bytes(b"""
-27 27 | foo""")
+28 28 | foo""")
-28    |-f"{str()}"
+29    |-f"{str()}"
-   28 |+f"{''}"
+   29 |+f"{''}"
 
 

crates/ruff_python_semantic/src/model.rs

@@ -734,6 +734,11 @@ impl<'a> SemanticModel<'a> {
         self.flags.contains(SemanticModelFlags::F_STRING)
     }
 
+    /// Return `true` if the context is in a nested f-string.
+    pub const fn in_nested_f_string(&self) -> bool {
+        self.flags.contains(SemanticModelFlags::NESTED_F_STRING)
+    }
+
     /// Return `true` if the context is in boolean test.
     pub const fn in_boolean_test(&self) -> bool {
         self.flags.contains(SemanticModelFlags::BOOLEAN_TEST)
@@ -850,6 +855,14 @@ bitflags! {
         /// ```
         const F_STRING = 1 << 6;
 
+        /// The context is in a nested f-string.
+        ///
+        /// For example, the context could be visiting `x` in:
+        /// ```python
+        /// f'{f"{x}"}'
+        /// ```
+        const NESTED_F_STRING = 1 << 7;
+
         /// The context is in a boolean test.
         ///
         /// For example, the context could be visiting `x` in:
@@ -860,7 +873,7 @@ bitflags! {
         ///
         /// The implication is that the actual value returned by the current expression is
         /// not used, only its truthiness.
-        const BOOLEAN_TEST = 1 << 7;
+        const BOOLEAN_TEST = 1 << 8;
 
         /// The context is in a `typing::Literal` annotation.
         ///
@@ -869,7 +882,7 @@ bitflags! {
         /// def f(x: Literal["A", "B", "C"]):
         ///     ...
         /// ```
-        const LITERAL = 1 << 8;
+        const LITERAL = 1 << 9;
 
         /// The context is in a subscript expression.
         ///
@@ -877,7 +890,7 @@ bitflags! {
         /// ```python
         /// x["a"]["b"]
         /// ```
-        const SUBSCRIPT = 1 << 9;
+        const SUBSCRIPT = 1 << 10;
 
         /// The context is in a type-checking block.
         ///
@@ -889,7 +902,7 @@ bitflags! {
         /// if TYPE_CHECKING:
         ///    x: int = 1
         /// ```
-        const TYPE_CHECKING_BLOCK = 1 << 10;
+        const TYPE_CHECKING_BLOCK = 1 << 11;
 
 
         /// The context has traversed past the "top-of-file" import boundary.
@@ -903,7 +916,7 @@ bitflags! {
         ///
         /// x: int = 1
         /// ```
-        const IMPORT_BOUNDARY = 1 << 11;
+        const IMPORT_BOUNDARY = 1 << 12;
 
         /// The context has traversed past the `__future__` import boundary.
         ///
@@ -918,7 +931,7 @@ bitflags! {
         ///
         /// Python considers it a syntax error to import from `__future__` after
         /// any other non-`__future__`-importing statements.
-        const FUTURES_BOUNDARY = 1 << 12;
+        const FUTURES_BOUNDARY = 1 << 13;
 
         /// `__future__`-style type annotations are enabled in this context.
         ///
@@ -930,7 +943,7 @@ bitflags! {
         /// def f(x: int) -> int:
         ///   ...
         /// ```
-        const FUTURE_ANNOTATIONS = 1 << 13;
+        const FUTURE_ANNOTATIONS = 1 << 14;
     }
 }