Skip to content

Commit f052bd6

Browse files
danparizherntBre
andauthored
[flake8-simplify] Fix truthiness assumption for non-iterable arguments in tuple/list/set calls (SIM222, SIM223) (#21479)
## Summary Fixes false positives in SIM222 and SIM223 where truthiness was incorrectly assumed for `tuple(x)`, `list(x)`, `set(x)` when `x` is not iterable. Fixes #21473. ## Problem `Truthiness::from_expr` recursively called itself on arguments to iterable initializers (`tuple`, `list`, `set`) without checking if the argument is iterable, causing false positives for cases like `tuple(0) or True` and `tuple("") or True`. ## Approach Added `is_definitely_not_iterable` helper and updated `Truthiness::from_expr` to return `Unknown` for non-iterable arguments (numbers, booleans, None) and string literals when called with iterable initializers, preventing incorrect truthiness assumptions. ## Test Plan Added test cases to `SIM222.py` and `SIM223.py` for `tuple("")`, `tuple(0)`, `tuple(1)`, `tuple(False)`, and `tuple(None)` with `or True` and `and False` patterns. --------- Co-authored-by: Brent Westbrook <[email protected]>
1 parent bc44dc2 commit f052bd6

File tree

5 files changed

+80
-8
lines changed

5 files changed

+80
-8
lines changed

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,3 +216,15 @@ def get_items_list():
216216

217217
def get_items_set():
218218
return tuple({item for item in items}) or None # OK
219+
220+
221+
# https://github.com/astral-sh/ruff/issues/21473
222+
tuple("") or True # SIM222
223+
tuple(t"") or True # OK
224+
tuple(0) or True # OK
225+
tuple(1) or True # OK
226+
tuple(False) or True # OK
227+
tuple(None) or True # OK
228+
tuple(...) or True # OK
229+
tuple(lambda x: x) or True # OK
230+
tuple(x for x in range(0)) or True # OK

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,3 +157,15 @@
157157

158158
# https://github.com/astral-sh/ruff/issues/7127
159159
def f(a: "'' and 'b'"): ...
160+
161+
162+
# https://github.com/astral-sh/ruff/issues/21473
163+
tuple("") and False # SIM223
164+
tuple(t"") and False # OK
165+
tuple(0) and False # OK
166+
tuple(1) and False # OK
167+
tuple(False) and False # OK
168+
tuple(None) and False # OK
169+
tuple(...) and False # OK
170+
tuple(lambda x: x) and False # OK
171+
tuple(x for x in range(0)) and False # OK

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1144,3 +1144,23 @@ help: Replace with `(i for i in range(1))`
11441144
208 | # https://github.com/astral-sh/ruff/issues/21136
11451145
209 | def get_items():
11461146
note: This is an unsafe fix and may change runtime behavior
1147+
1148+
SIM222 [*] Use `True` instead of `... or True`
1149+
--> SIM222.py:222:1
1150+
|
1151+
221 | # https://github.com/astral-sh/ruff/issues/21473
1152+
222 | tuple("") or True # SIM222
1153+
| ^^^^^^^^^^^^^^^^^
1154+
223 | tuple(t"") or True # OK
1155+
224 | tuple(0) or True # OK
1156+
|
1157+
help: Replace with `True`
1158+
219 |
1159+
220 |
1160+
221 | # https://github.com/astral-sh/ruff/issues/21473
1161+
- tuple("") or True # SIM222
1162+
222 + True # SIM222
1163+
223 | tuple(t"") or True # OK
1164+
224 | tuple(0) or True # OK
1165+
225 | tuple(1) or True # OK
1166+
note: This is an unsafe fix and may change runtime behavior

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1025,3 +1025,23 @@ help: Replace with `f"{''}{''}"`
10251025
156 |
10261026
157 |
10271027
note: This is an unsafe fix and may change runtime behavior
1028+
1029+
SIM223 [*] Use `tuple("")` instead of `tuple("") and ...`
1030+
--> SIM223.py:163:1
1031+
|
1032+
162 | # https://github.com/astral-sh/ruff/issues/21473
1033+
163 | tuple("") and False # SIM223
1034+
| ^^^^^^^^^^^^^^^^^^^
1035+
164 | tuple(t"") and False # OK
1036+
165 | tuple(0) and False # OK
1037+
|
1038+
help: Replace with `tuple("")`
1039+
160 |
1040+
161 |
1041+
162 | # https://github.com/astral-sh/ruff/issues/21473
1042+
- tuple("") and False # SIM223
1043+
163 + tuple("") # SIM223
1044+
164 | tuple(t"") and False # OK
1045+
165 | tuple(0) and False # OK
1046+
166 | tuple(1) and False # OK
1047+
note: This is an unsafe fix and may change runtime behavior

crates/ruff_python_ast/src/helpers.rs

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1322,14 +1322,22 @@ impl Truthiness {
13221322
&& arguments.keywords.is_empty()
13231323
{
13241324
// Ex) `list([1, 2, 3])`
1325-
// For tuple(generator), we can't determine statically if the result will
1326-
// be empty or not, so return Unknown. The generator itself is truthy, but
1327-
// tuple(empty_generator) is falsy. ListComp and SetComp are handled by
1328-
// recursing into Self::from_expr below, which returns Unknown for them.
1329-
if argument.is_generator_expr() {
1330-
Self::Unknown
1331-
} else {
1332-
Self::from_expr(argument, is_builtin)
1325+
match argument {
1326+
// Return Unknown for types with definite truthiness that might
1327+
// result in empty iterables (t-strings and generators) or will
1328+
// raise a type error (non-iterable types like numbers, booleans,
1329+
// None, etc.).
1330+
Expr::NumberLiteral(_)
1331+
| Expr::BooleanLiteral(_)
1332+
| Expr::NoneLiteral(_)
1333+
| Expr::EllipsisLiteral(_)
1334+
| Expr::TString(_)
1335+
| Expr::Lambda(_)
1336+
| Expr::Generator(_) => Self::Unknown,
1337+
// Recurse for all other types - collections, comprehensions, variables, etc.
1338+
// StringLiteral, FString, and BytesLiteral recurse because Self::from_expr
1339+
// correctly handles their truthiness (checking if empty or not).
1340+
_ => Self::from_expr(argument, is_builtin),
13331341
}
13341342
} else {
13351343
Self::Unknown

0 commit comments

Comments
 (0)