Fix false negatives in Truthiness::from_expr for lambdas, generators, and f-strings (#20704)

crates/ruff_linter/resources/test/fixtures/flake8_bandit/S602.py

@@ -33,3 +33,10 @@ class ShellConfig:
 
     def run(self, username):
         Popen("true", shell={**self.shell_defaults, **self.fetch_shell_config(username)})
+
+# Additional truthiness cases for generator, lambda, and f-strings
+Popen("true", shell=(i for i in ()))
+Popen("true", shell=lambda: 0)
+Popen("true", shell=f"{b''}")
+x = 1
+Popen("true", shell=f"{x=}")

crates/ruff_linter/resources/test/fixtures/flake8_bandit/S604.py

@@ -6,3 +6,19 @@ foo(shell=True)
 
 foo(shell={**{}})
 foo(shell={**{**{}}})
+
+# Truthy non-bool values for `shell`
+foo(shell=(i for i in ()))
+foo(shell=lambda: 0)
+
+# f-strings guaranteed non-empty
+foo(shell=f"{b''}")
+x = 1
+foo(shell=f"{x=}")
+
+# Additional truthiness cases for generator, lambda, and f-strings
+foo(shell=(i for i in ()))
+foo(shell=lambda: 0)
+foo(shell=f"{b''}")
+x = 1
+foo(shell=f"{x=}")

crates/ruff_linter/resources/test/fixtures/flake8_bandit/S609.py

@@ -9,3 +9,10 @@ os.system("tar cf foo.tar bar/*")
 
 subprocess.Popen(["chmod", "+w", "*.py"], shell={**{}})
 subprocess.Popen(["chmod", "+w", "*.py"], shell={**{**{}}})
+
+# Additional truthiness cases for generator, lambda, and f-strings
+subprocess.Popen("chmod +w foo*", shell=(i for i in ()))
+subprocess.Popen("chmod +w foo*", shell=lambda: 0)
+subprocess.Popen("chmod +w foo*", shell=f"{b''}")
+x = 1
+subprocess.Popen("chmod +w foo*", shell=f"{x=}")

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

@@ -197,3 +197,10 @@ for x in {**a, **b} or [None]:
 
 # https://github.com/astral-sh/ruff/issues/7127
 def f(a: "'b' or 'c'"): ...
+
+# https://github.com/astral-sh/ruff/issues/20703
+print(f"{b''}" or "bar")  # SIM222
+x = 1
+print(f"{x=}" or "bar")  # SIM222
+(lambda: 1) or True  # SIM222
+(i for i in range(1)) or "bar"  # SIM222

crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S602_S602.py.snap

@@ -127,3 +127,44 @@ S602 `subprocess` call with `shell=True` identified, security issue
 21 |
 22 | # Check dict display with only double-starred expressions can be falsey.
    |
+
+S602 `subprocess` call with truthy `shell` seems safe, but may be changed in the future; consider rewriting without `shell`
+  --> S602.py:38:1
+   |
+37 | # Additional truthiness cases for generator, lambda, and f-strings
+38 | Popen("true", shell=(i for i in ()))
+   | ^^^^^
+39 | Popen("true", shell=lambda: 0)
+40 | Popen("true", shell=f"{b''}")
+   |
+
+S602 `subprocess` call with truthy `shell` seems safe, but may be changed in the future; consider rewriting without `shell`
+  --> S602.py:39:1
+   |
+37 | # Additional truthiness cases for generator, lambda, and f-strings
+38 | Popen("true", shell=(i for i in ()))
+39 | Popen("true", shell=lambda: 0)
+   | ^^^^^
+40 | Popen("true", shell=f"{b''}")
+41 | x = 1
+   |
+
+S602 `subprocess` call with truthy `shell` seems safe, but may be changed in the future; consider rewriting without `shell`
+  --> S602.py:40:1
+   |
+38 | Popen("true", shell=(i for i in ()))
+39 | Popen("true", shell=lambda: 0)
+40 | Popen("true", shell=f"{b''}")
+   | ^^^^^
+41 | x = 1
+42 | Popen("true", shell=f"{x=}")
+   |
+
+S602 `subprocess` call with truthy `shell` seems safe, but may be changed in the future; consider rewriting without `shell`
+  --> S602.py:42:1
+   |
+40 | Popen("true", shell=f"{b''}")
+41 | x = 1
+42 | Popen("true", shell=f"{x=}")
+   | ^^^^^
+   |

crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S604_S604.py.snap

@@ -9,3 +9,85 @@ S604 Function call with `shell=True` parameter identified, security issue
 6 |
 7 | foo(shell={**{}})
   |
+
+S604 Function call with truthy `shell` parameter identified, security issue
+  --> S604.py:11:1
+   |
+10 | # Truthy non-bool values for `shell`
+11 | foo(shell=(i for i in ()))
+   | ^^^
+12 | foo(shell=lambda: 0)
+   |
+
+S604 Function call with truthy `shell` parameter identified, security issue
+  --> S604.py:12:1
+   |
+10 | # Truthy non-bool values for `shell`
+11 | foo(shell=(i for i in ()))
+12 | foo(shell=lambda: 0)
+   | ^^^
+13 |
+14 | # f-strings guaranteed non-empty
+   |
+
+S604 Function call with truthy `shell` parameter identified, security issue
+  --> S604.py:15:1
+   |
+14 | # f-strings guaranteed non-empty
+15 | foo(shell=f"{b''}")
+   | ^^^
+16 | x = 1
+17 | foo(shell=f"{x=}")
+   |
+
+S604 Function call with truthy `shell` parameter identified, security issue
+  --> S604.py:17:1
+   |
+15 | foo(shell=f"{b''}")
+16 | x = 1
+17 | foo(shell=f"{x=}")
+   | ^^^
+18 |
+19 | # Additional truthiness cases for generator, lambda, and f-strings
+   |
+
+S604 Function call with truthy `shell` parameter identified, security issue
+  --> S604.py:20:1
+   |
+19 | # Additional truthiness cases for generator, lambda, and f-strings
+20 | foo(shell=(i for i in ()))
+   | ^^^
+21 | foo(shell=lambda: 0)
+22 | foo(shell=f"{b''}")
+   |
+
+S604 Function call with truthy `shell` parameter identified, security issue
+  --> S604.py:21:1
+   |
+19 | # Additional truthiness cases for generator, lambda, and f-strings
+20 | foo(shell=(i for i in ()))
+21 | foo(shell=lambda: 0)
+   | ^^^
+22 | foo(shell=f"{b''}")
+23 | x = 1
+   |
+
+S604 Function call with truthy `shell` parameter identified, security issue
+  --> S604.py:22:1
+   |
+20 | foo(shell=(i for i in ()))
+21 | foo(shell=lambda: 0)
+22 | foo(shell=f"{b''}")
+   | ^^^
+23 | x = 1
+24 | foo(shell=f"{x=}")
+   |
+
+S604 Function call with truthy `shell` parameter identified, security issue
+  --> S604.py:24:1
+   |
+22 | foo(shell=f"{b''}")
+23 | x = 1
+24 | foo(shell=f"{x=}")
+   | ^^^
+   |

crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S609_S609.py.snap

@@ -43,3 +43,44 @@ S609 Possible wildcard injection in call due to `*` usage
  9 |
 10 | subprocess.Popen(["chmod", "+w", "*.py"], shell={**{}})
    |
+
+S609 Possible wildcard injection in call due to `*` usage
+  --> S609.py:14:18
+   |
+13 | # Additional truthiness cases for generator, lambda, and f-strings
+14 | subprocess.Popen("chmod +w foo*", shell=(i for i in ()))
+   |                  ^^^^^^^^^^^^^^^
+15 | subprocess.Popen("chmod +w foo*", shell=lambda: 0)
+16 | subprocess.Popen("chmod +w foo*", shell=f"{b''}")
+   |
+
+S609 Possible wildcard injection in call due to `*` usage
+  --> S609.py:15:18
+   |
+13 | # Additional truthiness cases for generator, lambda, and f-strings
+14 | subprocess.Popen("chmod +w foo*", shell=(i for i in ()))
+15 | subprocess.Popen("chmod +w foo*", shell=lambda: 0)
+   |                  ^^^^^^^^^^^^^^^
+16 | subprocess.Popen("chmod +w foo*", shell=f"{b''}")
+17 | x = 1
+   |
+
+S609 Possible wildcard injection in call due to `*` usage
+  --> S609.py:16:18
+   |
+14 | subprocess.Popen("chmod +w foo*", shell=(i for i in ()))
+15 | subprocess.Popen("chmod +w foo*", shell=lambda: 0)
+16 | subprocess.Popen("chmod +w foo*", shell=f"{b''}")
+   |                  ^^^^^^^^^^^^^^^
+17 | x = 1
+18 | subprocess.Popen("chmod +w foo*", shell=f"{x=}")
+   |
+
+S609 Possible wildcard injection in call due to `*` usage
+  --> S609.py:18:18
+   |
+16 | subprocess.Popen("chmod +w foo*", shell=f"{b''}")
+17 | x = 1
+18 | subprocess.Popen("chmod +w foo*", shell=f"{x=}")
+   |                  ^^^^^^^^^^^^^^^
+   |

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

@@ -1062,3 +1062,77 @@ help: Replace with `"bar"`
 170 | 
 171 | 
 note: This is an unsafe fix and may change runtime behavior
+
+SIM222 [*] Use `f"{b''}"` instead of `f"{b''}" or ...`
+   --> SIM222.py:202:7
+    |
+201 | # https://github.com/astral-sh/ruff/issues/20703
+202 | print(f"{b''}" or "bar")  # SIM222
+    |       ^^^^^^^^^^^^^^^^^
+203 | x = 1
+204 | print(f"{x=}" or "bar")  # SIM222
+    |
+help: Replace with `f"{b''}"`
+199 | def f(a: "'b' or 'c'"): ...
+200 | 
+201 | # https://github.com/astral-sh/ruff/issues/20703
+    - print(f"{b''}" or "bar")  # SIM222
+202 + print(f"{b''}")  # SIM222
+203 | x = 1
+204 | print(f"{x=}" or "bar")  # SIM222
+205 | (lambda: 1) or True  # SIM222
+note: This is an unsafe fix and may change runtime behavior
+
+SIM222 [*] Use `f"{x=}"` instead of `f"{x=}" or ...`
+   --> SIM222.py:204:7
+    |
+202 | print(f"{b''}" or "bar")  # SIM222
+203 | x = 1
+204 | print(f"{x=}" or "bar")  # SIM222
+    |       ^^^^^^^^^^^^^^^^
+205 | (lambda: 1) or True  # SIM222
+206 | (i for i in range(1)) or "bar"  # SIM222
+    |
+help: Replace with `f"{x=}"`
+201 | # https://github.com/astral-sh/ruff/issues/20703
+202 | print(f"{b''}" or "bar")  # SIM222
+203 | x = 1
+    - print(f"{x=}" or "bar")  # SIM222
+204 + print(f"{x=}")  # SIM222
+205 | (lambda: 1) or True  # SIM222
+206 | (i for i in range(1)) or "bar"  # SIM222
+note: This is an unsafe fix and may change runtime behavior
+
+SIM222 [*] Use `lambda: 1` instead of `lambda: 1 or ...`
+   --> SIM222.py:205:1
+    |
+203 | x = 1
+204 | print(f"{x=}" or "bar")  # SIM222
+205 | (lambda: 1) or True  # SIM222
+    | ^^^^^^^^^^^^^^^^^^^
+206 | (i for i in range(1)) or "bar"  # SIM222
+    |
+help: Replace with `lambda: 1`
+202 | print(f"{b''}" or "bar")  # SIM222
+203 | x = 1
+204 | print(f"{x=}" or "bar")  # SIM222
+    - (lambda: 1) or True  # SIM222
+205 + lambda: 1  # SIM222
+206 | (i for i in range(1)) or "bar"  # SIM222
+note: This is an unsafe fix and may change runtime behavior
+
+SIM222 [*] Use `(i for i in range(1))` instead of `(i for i in range(1)) or ...`
+   --> SIM222.py:206:1
+    |
+204 | print(f"{x=}" or "bar")  # SIM222
+205 | (lambda: 1) or True  # SIM222
+206 | (i for i in range(1)) or "bar"  # SIM222
+    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+    |
+help: Replace with `(i for i in range(1))`
+203 | x = 1
+204 | print(f"{x=}" or "bar")  # SIM222
+205 | (lambda: 1) or True  # SIM222
+    - (i for i in range(1)) or "bar"  # SIM222
+206 + (i for i in range(1))  # SIM222
+note: This is an unsafe fix and may change runtime behavior

crates/ruff_python_ast/src/helpers.rs

@@ -12,7 +12,7 @@ use crate::parenthesize::parenthesized_range;
 use crate::statement_visitor::StatementVisitor;
 use crate::visitor::Visitor;
 use crate::{
-    self as ast, Arguments, AtomicNodeIndex, CmpOp, DictItem, ExceptHandler, Expr,
+    self as ast, Arguments, AtomicNodeIndex, CmpOp, DictItem, ExceptHandler, Expr, ExprNoneLiteral,
     InterpolatedStringElement, MatchCase, Operator, Pattern, Stmt, TypeParam,
 };
 use crate::{AnyNodeRef, ExprContext};
@@ -1219,6 +1219,8 @@ impl Truthiness {
         F: Fn(&str) -> bool,
     {
         match expr {
+            Expr::Lambda(_) => Self::Truthy,
+            Expr::Generator(_) => Self::Truthy,
             Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) => {
                 if value.is_empty() {
                     Self::Falsey
@@ -1388,7 +1390,9 @@ fn is_non_empty_f_string(expr: &ast::ExprFString) -> bool {
             Expr::FString(f_string) => is_non_empty_f_string(f_string),
             // These literals may or may not be empty.
             Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) => !value.is_empty(),
-            Expr::BytesLiteral(ast::ExprBytesLiteral { value, .. }) => !value.is_empty(),
+            // Confusingly, f"{b""}" renders as the string 'b""', which is non-empty.
+            // Therefore, any bytes interpolation is guaranteed non-empty when stringified.
+            Expr::BytesLiteral(_) => true,
         }
     }
 
@@ -1397,7 +1401,9 @@ fn is_non_empty_f_string(expr: &ast::ExprFString) -> bool {
         ast::FStringPart::FString(f_string) => {
             f_string.elements.iter().all(|element| match element {
                 InterpolatedStringElement::Literal(string_literal) => !string_literal.is_empty(),
-                InterpolatedStringElement::Interpolation(f_string) => inner(&f_string.expression),
+                InterpolatedStringElement::Interpolation(f_string) => {
+                    f_string.debug_text.is_some() || inner(&f_string.expression)
+                }
             })
         }
     })
@@ -1493,7 +1499,7 @@ pub fn pep_604_optional(expr: &Expr) -> Expr {
     ast::ExprBinOp {
         left: Box::new(expr.clone()),
         op: Operator::BitOr,
-        right: Box::new(Expr::NoneLiteral(ast::ExprNoneLiteral::default())),
+        right: Box::new(Expr::NoneLiteral(ExprNoneLiteral::default())),
         range: TextRange::default(),
         node_index: AtomicNodeIndex::NONE,
     }