Account for possibly-empty f-string values in truthiness logic

crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM222.py

@@ -160,3 +160,10 @@ def secondToTime(s0: int) -> (int, int, int) or str:
 
 def secondToTime(s0: int) -> ((int, int, int) or str):
     m, s = divmod(s0, 60)
+
+
+# Regression test for: https://github.com/astral-sh/ruff/issues/9479
+print(f"{a}{b}" or "bar")
+print(f"{a}{''}" or "bar")
+print(f"{''}{''}" or "bar")
+print(f"{1}{''}" or "bar")

crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM223.py

@@ -147,3 +147,9 @@
 
 if f(a and [] and False and []):  # SIM223
     pass
+
+# Regression test for: https://github.com/astral-sh/ruff/issues/9479
+print(f"{a}{b}" and "bar")
+print(f"{a}{''}" and "bar")
+print(f"{''}{''}" and "bar")
+print(f"{1}{''}" and "bar")

crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__SIM222_SIM222.py.snap

@@ -1040,5 +1040,25 @@ SIM222.py:161:31: SIM222 [*] Use `(int, int, int)` instead of `(int, int, int) o
 161     |-def secondToTime(s0: int) -> ((int, int, int) or str):
     161 |+def secondToTime(s0: int) -> ((int, int, int)):
 162 162 |     m, s = divmod(s0, 60)
+163 163 | 
+164 164 | 
+
+SIM222.py:168:7: SIM222 [*] Use `"bar"` instead of `... or "bar"`
+    |
+166 | print(f"{a}{b}" or "bar")
+167 | print(f"{a}{''}" or "bar")
+168 | print(f"{''}{''}" or "bar")
+    |       ^^^^^^^^^^^^^^^^^^^^ SIM222
+169 | print(f"{1}{''}" or "bar")
+    |
+    = help: Replace with `"bar"`
+
+ℹ Unsafe fix
+165 165 | # Regression test for: https://github.com/astral-sh/ruff/issues/9479
+166 166 | print(f"{a}{b}" or "bar")
+167 167 | print(f"{a}{''}" or "bar")
+168     |-print(f"{''}{''}" or "bar")
+    168 |+print("bar")
+169 169 | print(f"{1}{''}" or "bar")
 
 

crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__SIM223_SIM223.py.snap

@@ -1003,5 +1003,25 @@ SIM223.py:148:12: SIM223 [*] Use `[]` instead of `[] and ...`
 148     |-if f(a and [] and False and []):  # SIM223
     148 |+if f(a and []):  # SIM223
 149 149 |     pass
+150 150 | 
+151 151 | # Regression test for: https://github.com/astral-sh/ruff/issues/9479
+
+SIM223.py:154:7: SIM223 [*] Use `f"{''}{''}"` instead of `f"{''}{''}" and ...`
+    |
+152 | print(f"{a}{b}" and "bar")
+153 | print(f"{a}{''}" and "bar")
+154 | print(f"{''}{''}" and "bar")
+    |       ^^^^^^^^^^^^^^^^^^^^^ SIM223
+155 | print(f"{1}{''}" and "bar")
+    |
+    = help: Replace with `f"{''}{''}"`
+
+ℹ Unsafe fix
+151 151 | # Regression test for: https://github.com/astral-sh/ruff/issues/9479
+152 152 | print(f"{a}{b}" and "bar")
+153 153 | print(f"{a}{''}" and "bar")
+154     |-print(f"{''}{''}" and "bar")
+    154 |+print(f"{''}{''}")
+155 155 | print(f"{1}{''}" and "bar")
 
 

crates/ruff_python_ast/src/helpers.rs

@@ -308,10 +308,13 @@ pub fn any_over_pattern(pattern: &Pattern, func: &dyn Fn(&Expr) -> bool) -> bool
     }
 }
 
-pub fn any_over_f_string_element(element: &FStringElement, func: &dyn Fn(&Expr) -> bool) -> bool {
+pub fn any_over_f_string_element(
+    element: &ast::FStringElement,
+    func: &dyn Fn(&Expr) -> bool,
+) -> bool {
     match element {
-        FStringElement::Literal(_) => false,
-        FStringElement::Expression(ast::FStringExpressionElement {
+        ast::FStringElement::Literal(_) => false,
+        ast::FStringElement::Expression(ast::FStringExpressionElement {
             expression,
             format_spec,
             ..
@@ -1171,21 +1174,10 @@ impl Truthiness {
             }
             Expr::NoneLiteral(_) => Self::Falsey,
             Expr::EllipsisLiteral(_) => Self::Truthy,
-            Expr::FString(ast::ExprFString { value, .. }) => {
-                if value.iter().all(|part| match part {
-                    ast::FStringPart::Literal(string_literal) => string_literal.is_empty(),
-                    ast::FStringPart::FString(f_string) => f_string.elements.is_empty(),
-                }) {
+            Expr::FString(f_string) => {
+                if is_empty_f_string(f_string) {
                     Self::Falsey
-                } else if value
-                    .elements()
-                    .any(|f_string_element| match f_string_element {
-                        ast::FStringElement::Literal(ast::FStringLiteralElement {
-                            value, ..
-                        }) => !value.is_empty(),
-                        ast::FStringElement::Expression(_) => true,
-                    })
-                {
+                } else if is_non_empty_f_string(f_string) {
                     Self::Truthy
                 } else {
                     Self::Unknown
@@ -1243,6 +1235,99 @@ impl Truthiness {
     }
 }
 
+/// Returns `true` if the expression definitely resolves to a non-empty string, when used as an
+/// f-string expression, or `false` if the expression may resolve to an empty string.
+fn is_non_empty_f_string(expr: &ast::ExprFString) -> bool {
+    fn inner(expr: &Expr) -> bool {
+        match expr {
+            // When stringified, these expressions are always non-empty.
+            Expr::Lambda(_) => true,
+            Expr::Dict(_) => true,
+            Expr::Set(_) => true,
+            Expr::ListComp(_) => true,
+            Expr::SetComp(_) => true,
+            Expr::DictComp(_) => true,
+            Expr::Compare(_) => true,
+            Expr::NumberLiteral(_) => true,
+            Expr::BooleanLiteral(_) => true,
+            Expr::NoneLiteral(_) => true,
+            Expr::EllipsisLiteral(_) => true,
+            Expr::List(_) => true,
+            Expr::Tuple(_) => true,
+
+            // These expressions must resolve to the inner expression.
+            Expr::IfExp(ast::ExprIfExp { body, orelse, .. }) => inner(body) && inner(orelse),
+            Expr::NamedExpr(ast::ExprNamedExpr { value, .. }) => inner(value),
+
+            // These expressions are complex. We can't determine whether they're empty or not.
+            Expr::BoolOp(ast::ExprBoolOp { .. }) => false,
+            Expr::BinOp(ast::ExprBinOp { .. }) => false,
+            Expr::UnaryOp(ast::ExprUnaryOp { .. }) => false,
+            Expr::GeneratorExp(_) => false,
+            Expr::Await(_) => false,
+            Expr::Yield(_) => false,
+            Expr::YieldFrom(_) => false,
+            Expr::Call(_) => false,
+            Expr::Attribute(_) => false,
+            Expr::Subscript(_) => false,
+            Expr::Starred(_) => false,
+            Expr::Name(_) => false,
+            Expr::Slice(_) => false,
+            Expr::IpyEscapeCommand(_) => false,
+
+            // These literals may or may not be empty.
+            Expr::FString(f_string) => is_non_empty_f_string(f_string),
+            Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) => !value.is_empty(),
+            Expr::BytesLiteral(ast::ExprBytesLiteral { value, .. }) => !value.is_empty(),
+        }
+    }
+
+    expr.value.iter().any(|part| match part {
+        ast::FStringPart::Literal(string_literal) => !string_literal.is_empty(),
+        ast::FStringPart::FString(f_string) => {
+            f_string.elements.iter().all(|element| match element {
+                FStringElement::Literal(string_literal) => !string_literal.is_empty(),
+                FStringElement::Expression(f_string) => inner(&f_string.expression),
+            })
+        }
+    })
+}
+
+/// Returns `true` if the expression definitely resolves to the empty string, when used as an f-string
+/// expression.
+fn is_empty_f_string(expr: &ast::ExprFString) -> bool {
+    fn inner(expr: &Expr) -> bool {
+        match expr {
+            Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) => value.is_empty(),
+            Expr::BytesLiteral(ast::ExprBytesLiteral { value, .. }) => value.is_empty(),
+            Expr::FString(ast::ExprFString { value, .. }) => {
+                value
+                    .elements()
+                    .all(|f_string_element| match f_string_element {
+                        FStringElement::Literal(ast::FStringLiteralElement { value, .. }) => {
+                            value.is_empty()
+                        }
+                        FStringElement::Expression(ast::FStringExpressionElement {
+                            expression,
+                            ..
+                        }) => inner(expression),
+                    })
+            }
+            _ => false,
+        }
+    }
+
+    expr.value.iter().all(|part| match part {
+        ast::FStringPart::Literal(string_literal) => string_literal.is_empty(),
+        ast::FStringPart::FString(f_string) => {
+            f_string.elements.iter().all(|element| match element {
+                FStringElement::Literal(string_literal) => string_literal.is_empty(),
+                FStringElement::Expression(f_string) => inner(&f_string.expression),
+            })
+        }
+    })
+}
+
 pub fn generate_comparison(
     left: &Expr,
     ops: &[CmpOp],