From f052bd644c6ca86a8b7f3c8d7523ac677f2bd665 Mon Sep 17 00:00:00 2001 From: Dan Parizher <105245560+danparizher@users.noreply.github.com> Date: Mon, 1 Dec 2025 16:57:51 -0500 Subject: [PATCH] [`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 --- .../test/fixtures/flake8_simplify/SIM222.py | 12 ++++++++++ .../test/fixtures/flake8_simplify/SIM223.py | 12 ++++++++++ ...ke8_simplify__tests__SIM222_SIM222.py.snap | 20 ++++++++++++++++ ...ke8_simplify__tests__SIM223_SIM223.py.snap | 20 ++++++++++++++++ crates/ruff_python_ast/src/helpers.rs | 24 ++++++++++++------- 5 files changed, 80 insertions(+), 8 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM222.py b/crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM222.py index 71fc606386..62814c796f 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM222.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM222.py @@ -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 diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM223.py b/crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM223.py index 12edbff6bd..abcf2536bb 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM223.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM223.py @@ -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 diff --git a/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__SIM222_SIM222.py.snap b/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__SIM222_SIM222.py.snap index 0e65033b21..a1ca861205 100644 --- a/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__SIM222_SIM222.py.snap +++ b/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__SIM222_SIM222.py.snap @@ -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 diff --git a/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__SIM223_SIM223.py.snap b/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__SIM223_SIM223.py.snap index 4fe01c8146..08f3f48ba2 100644 --- a/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__SIM223_SIM223.py.snap +++ b/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__SIM223_SIM223.py.snap @@ -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 diff --git a/crates/ruff_python_ast/src/helpers.rs b/crates/ruff_python_ast/src/helpers.rs index 66ad66d9b1..4879e04780 100644 --- a/crates/ruff_python_ast/src/helpers.rs +++ b/crates/ruff_python_ast/src/helpers.rs @@ -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