mirror of https://github.com/astral-sh/ruff
[`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 <brentrwestbrook@gmail.com>
This commit is contained in:
parent
bc44dc2afb
commit
f052bd644c
|
|
@ -216,3 +216,15 @@ def get_items_list():
|
|||
|
||||
def get_items_set():
|
||||
return tuple({item for item in items}) or None # OK
|
||||
|
||||
|
||||
# https://github.com/astral-sh/ruff/issues/21473
|
||||
tuple("") or True # SIM222
|
||||
tuple(t"") or True # OK
|
||||
tuple(0) or True # OK
|
||||
tuple(1) or True # OK
|
||||
tuple(False) or True # OK
|
||||
tuple(None) or True # OK
|
||||
tuple(...) or True # OK
|
||||
tuple(lambda x: x) or True # OK
|
||||
tuple(x for x in range(0)) or True # OK
|
||||
|
|
|
|||
|
|
@ -157,3 +157,15 @@ print(f"{1}{''}" and "bar")
|
|||
|
||||
# https://github.com/astral-sh/ruff/issues/7127
|
||||
def f(a: "'' and 'b'"): ...
|
||||
|
||||
|
||||
# https://github.com/astral-sh/ruff/issues/21473
|
||||
tuple("") and False # SIM223
|
||||
tuple(t"") and False # OK
|
||||
tuple(0) and False # OK
|
||||
tuple(1) and False # OK
|
||||
tuple(False) and False # OK
|
||||
tuple(None) and False # OK
|
||||
tuple(...) and False # OK
|
||||
tuple(lambda x: x) and False # OK
|
||||
tuple(x for x in range(0)) and False # OK
|
||||
|
|
|
|||
|
|
@ -1144,3 +1144,23 @@ help: Replace with `(i for i in range(1))`
|
|||
208 | # https://github.com/astral-sh/ruff/issues/21136
|
||||
209 | def get_items():
|
||||
note: This is an unsafe fix and may change runtime behavior
|
||||
|
||||
SIM222 [*] Use `True` instead of `... or True`
|
||||
--> SIM222.py:222:1
|
||||
|
|
||||
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
|
||||
|
|
||||
help: Replace with `True`
|
||||
219 |
|
||||
220 |
|
||||
221 | # https://github.com/astral-sh/ruff/issues/21473
|
||||
- tuple("") or True # SIM222
|
||||
222 + True # SIM222
|
||||
223 | tuple(t"") or True # OK
|
||||
224 | tuple(0) or True # OK
|
||||
225 | tuple(1) or True # OK
|
||||
note: This is an unsafe fix and may change runtime behavior
|
||||
|
|
|
|||
|
|
@ -1025,3 +1025,23 @@ help: Replace with `f"{''}{''}"`
|
|||
156 |
|
||||
157 |
|
||||
note: This is an unsafe fix and may change runtime behavior
|
||||
|
||||
SIM223 [*] Use `tuple("")` instead of `tuple("") and ...`
|
||||
--> SIM223.py:163:1
|
||||
|
|
||||
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
|
||||
|
|
||||
help: Replace with `tuple("")`
|
||||
160 |
|
||||
161 |
|
||||
162 | # https://github.com/astral-sh/ruff/issues/21473
|
||||
- tuple("") and False # SIM223
|
||||
163 + tuple("") # SIM223
|
||||
164 | tuple(t"") and False # OK
|
||||
165 | tuple(0) and False # OK
|
||||
166 | tuple(1) and False # OK
|
||||
note: This is an unsafe fix and may change runtime behavior
|
||||
|
|
|
|||
|
|
@ -1322,14 +1322,22 @@ impl Truthiness {
|
|||
&& arguments.keywords.is_empty()
|
||||
{
|
||||
// Ex) `list([1, 2, 3])`
|
||||
// For tuple(generator), we can't determine statically if the result will
|
||||
// be empty or not, so return Unknown. The generator itself is truthy, but
|
||||
// tuple(empty_generator) is falsy. ListComp and SetComp are handled by
|
||||
// recursing into Self::from_expr below, which returns Unknown for them.
|
||||
if argument.is_generator_expr() {
|
||||
Self::Unknown
|
||||
} else {
|
||||
Self::from_expr(argument, is_builtin)
|
||||
match argument {
|
||||
// Return Unknown for types with definite truthiness that might
|
||||
// result in empty iterables (t-strings and generators) or will
|
||||
// raise a type error (non-iterable types like numbers, booleans,
|
||||
// None, etc.).
|
||||
Expr::NumberLiteral(_)
|
||||
| Expr::BooleanLiteral(_)
|
||||
| Expr::NoneLiteral(_)
|
||||
| Expr::EllipsisLiteral(_)
|
||||
| Expr::TString(_)
|
||||
| Expr::Lambda(_)
|
||||
| Expr::Generator(_) => Self::Unknown,
|
||||
// Recurse for all other types - collections, comprehensions, variables, etc.
|
||||
// StringLiteral, FString, and BytesLiteral recurse because Self::from_expr
|
||||
// correctly handles their truthiness (checking if empty or not).
|
||||
_ => Self::from_expr(argument, is_builtin),
|
||||
}
|
||||
} else {
|
||||
Self::Unknown
|
||||
|
|
|
|||
Loading…
Reference in New Issue