From 216645dd4c3653d017182d59516010ecf1f8b6b9 Mon Sep 17 00:00:00 2001 From: Dan Date: Sun, 16 Nov 2025 12:24:34 -0500 Subject: [PATCH 01/12] fix-21473 --- .../test/fixtures/flake8_simplify/SIM222.py | 8 ++++++ .../test/fixtures/flake8_simplify/SIM223.py | 8 ++++++ crates/ruff_python_ast/src/helpers.rs | 26 +++++++++++++++++++ 3 files changed, 42 insertions(+) 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 71fc6063867b0..86603f413cd51 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,11 @@ 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 # OK +tuple(0) or True # OK +tuple(1) or True # OK +tuple(False) or True # OK +tuple(None) 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 12edbff6bd8bd..affbabe6e0811 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,11 @@ # https://github.com/astral-sh/ruff/issues/7127 def f(a: "'' and 'b'"): ... + + +# https://github.com/astral-sh/ruff/issues/21473 +tuple("") and False # OK +tuple(0) and False # OK +tuple(1) and False # OK +tuple(False) and False # OK +tuple(None) and False # OK diff --git a/crates/ruff_python_ast/src/helpers.rs b/crates/ruff_python_ast/src/helpers.rs index 66ad66d9b14f3..ce4607d647129 100644 --- a/crates/ruff_python_ast/src/helpers.rs +++ b/crates/ruff_python_ast/src/helpers.rs @@ -1212,6 +1212,19 @@ pub enum Truthiness { Unknown, } +/// Return `true` if the expression is definitely not iterable (e.g., numbers, booleans, None). +/// This is used to avoid incorrectly assuming truthiness for `tuple(x)`, `list(x)`, etc. +/// when `x` is not iterable. +fn is_definitely_not_iterable(expr: &Expr) -> bool { + matches!( + expr, + Expr::NumberLiteral(_) + | Expr::BooleanLiteral(_) + | Expr::NoneLiteral(_) + | Expr::EllipsisLiteral(_) + ) +} + impl Truthiness { /// Return the truthiness of an expression. pub fn from_expr(expr: &Expr, is_builtin: F) -> Self @@ -1328,6 +1341,19 @@ impl Truthiness { // recursing into Self::from_expr below, which returns Unknown for them. if argument.is_generator_expr() { Self::Unknown + } else if is_definitely_not_iterable(argument) { + // For non-iterable arguments like numbers, booleans, None, etc., + // we can't assume truthiness. For example, tuple(0) raises TypeError. + Self::Unknown + } else if matches!(argument, Expr::StringLiteral(_)) { + // For strings, tuple("") creates an empty tuple which is falsy, + // but tuple("a") creates a non-empty tuple which is truthy. + // However, we can't assume truthiness matches because: + // - tuple("") is falsy (empty tuple), but "" is also falsy + // - The issue is that we conflate truthiness with non-emptiness + // - For SIM222/SIM223, we should be conservative and return Unknown + // to avoid false positives + Self::Unknown } else { Self::from_expr(argument, is_builtin) } From 0ee00edcdfc3262cc319a27f8dbe367e2b7ecbf7 Mon Sep 17 00:00:00 2001 From: Dan Date: Sun, 16 Nov 2025 12:43:09 -0500 Subject: [PATCH 02/12] Update ruff_linter__rules__flake8_pytest_style__tests__PT015.snap --- ...ter__rules__flake8_pytest_style__tests__PT015.snap | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT015.snap b/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT015.snap index 51ddc586d57de..9ae9c29884a4e 100644 --- a/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT015.snap +++ b/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT015.snap @@ -174,14 +174,3 @@ PT015 Assertion always fails, replace with `pytest.fail()` | ^^^^^^^^^^^^^^^^^ 25 | assert tuple("") | - -PT015 Assertion always fails, replace with `pytest.fail()` - --> PT015.py:25:5 - | -23 | assert list([]) -24 | assert set(set()) -25 | assert tuple("") - | ^^^^^^^^^^^^^^^^ -26 | -27 | # https://github.com/astral-sh/ruff/issues/19935 - | From 2cc5cb708af569250bd27a5d5890de92bfef5807 Mon Sep 17 00:00:00 2001 From: Dan Date: Sun, 16 Nov 2025 13:11:35 -0500 Subject: [PATCH 03/12] Update analyze_graph.rs --- crates/ruff/tests/cli/analyze_graph.rs | 64 +++++++++++++------------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/crates/ruff/tests/cli/analyze_graph.rs b/crates/ruff/tests/cli/analyze_graph.rs index f0edb291c79bd..90c6b175fa584 100644 --- a/crates/ruff/tests/cli/analyze_graph.rs +++ b/crates/ruff/tests/cli/analyze_graph.rs @@ -27,43 +27,43 @@ fn type_checking_imports() -> anyhow::Result<()> { ("ruff/c.py", ""), ])?; - assert_cmd_snapshot!(test.analyze_graph_command(), @r###" + assert_cmd_snapshot!(test.analyze_graph_command(), @r#" success: true exit_code: 0 ----- stdout ----- { - "ruff/__init__.py": [], - "ruff/a.py": [ - "ruff/b.py", - "ruff/c.py" + "ruff\/__init__.py": [], + "ruff\\a.py": [ + "ruff\\b.py", + "ruff\\c.py" ], - "ruff/b.py": [ - "ruff/c.py" + "ruff\\b.py": [ + "ruff\\c.py" ], - "ruff/c.py": [] + "ruff\\c.py": [] } ----- stderr ----- - "###); + "#); assert_cmd_snapshot!( test.analyze_graph_command() .arg("--no-type-checking-imports"), - @r###" + @r#" success: true exit_code: 0 ----- stdout ----- { - "ruff/__init__.py": [], - "ruff/a.py": [ - "ruff/b.py" + "ruff\/__init__.py": [], + "ruff\\a.py": [ + "ruff\\b.py" ], - "ruff/b.py": [], - "ruff/c.py": [] + "ruff\\b.py": [], + "ruff\\c.py": [] } ----- stderr ----- - "### + "# ); Ok(()) @@ -101,21 +101,21 @@ fn type_checking_imports_from_config() -> anyhow::Result<()> { ), ])?; - assert_cmd_snapshot!(test.analyze_graph_command(), @r###" + assert_cmd_snapshot!(test.analyze_graph_command(), @r#" success: true exit_code: 0 ----- stdout ----- { - "ruff/__init__.py": [], - "ruff/a.py": [ - "ruff/b.py" + "ruff\/__init__.py": [], + "ruff\\a.py": [ + "ruff\\b.py" ], - "ruff/b.py": [], - "ruff/c.py": [] + "ruff\\b.py": [], + "ruff\\c.py": [] } ----- stderr ----- - "###); + "#); test.write_file( "ruff.toml", @@ -125,24 +125,24 @@ fn type_checking_imports_from_config() -> anyhow::Result<()> { "#, )?; - assert_cmd_snapshot!(test.analyze_graph_command(), @r###" + assert_cmd_snapshot!(test.analyze_graph_command(), @r#" success: true exit_code: 0 ----- stdout ----- { - "ruff/__init__.py": [], - "ruff/a.py": [ - "ruff/b.py", - "ruff/c.py" + "ruff\/__init__.py": [], + "ruff\\a.py": [ + "ruff\\b.py", + "ruff\\c.py" ], - "ruff/b.py": [ - "ruff/c.py" + "ruff\\b.py": [ + "ruff\\c.py" ], - "ruff/c.py": [] + "ruff\\c.py": [] } ----- stderr ----- - "### + "# ); Ok(()) From d1987b287af99285e2bb8ebb3f39ea8ca6f16afe Mon Sep 17 00:00:00 2001 From: Dan Date: Sun, 16 Nov 2025 13:13:50 -0500 Subject: [PATCH 04/12] Revert "Update analyze_graph.rs" This reverts commit 2cc5cb708af569250bd27a5d5890de92bfef5807. --- crates/ruff/tests/cli/analyze_graph.rs | 64 +++++++++++++------------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/crates/ruff/tests/cli/analyze_graph.rs b/crates/ruff/tests/cli/analyze_graph.rs index 90c6b175fa584..f0edb291c79bd 100644 --- a/crates/ruff/tests/cli/analyze_graph.rs +++ b/crates/ruff/tests/cli/analyze_graph.rs @@ -27,43 +27,43 @@ fn type_checking_imports() -> anyhow::Result<()> { ("ruff/c.py", ""), ])?; - assert_cmd_snapshot!(test.analyze_graph_command(), @r#" + assert_cmd_snapshot!(test.analyze_graph_command(), @r###" success: true exit_code: 0 ----- stdout ----- { - "ruff\/__init__.py": [], - "ruff\\a.py": [ - "ruff\\b.py", - "ruff\\c.py" + "ruff/__init__.py": [], + "ruff/a.py": [ + "ruff/b.py", + "ruff/c.py" ], - "ruff\\b.py": [ - "ruff\\c.py" + "ruff/b.py": [ + "ruff/c.py" ], - "ruff\\c.py": [] + "ruff/c.py": [] } ----- stderr ----- - "#); + "###); assert_cmd_snapshot!( test.analyze_graph_command() .arg("--no-type-checking-imports"), - @r#" + @r###" success: true exit_code: 0 ----- stdout ----- { - "ruff\/__init__.py": [], - "ruff\\a.py": [ - "ruff\\b.py" + "ruff/__init__.py": [], + "ruff/a.py": [ + "ruff/b.py" ], - "ruff\\b.py": [], - "ruff\\c.py": [] + "ruff/b.py": [], + "ruff/c.py": [] } ----- stderr ----- - "# + "### ); Ok(()) @@ -101,21 +101,21 @@ fn type_checking_imports_from_config() -> anyhow::Result<()> { ), ])?; - assert_cmd_snapshot!(test.analyze_graph_command(), @r#" + assert_cmd_snapshot!(test.analyze_graph_command(), @r###" success: true exit_code: 0 ----- stdout ----- { - "ruff\/__init__.py": [], - "ruff\\a.py": [ - "ruff\\b.py" + "ruff/__init__.py": [], + "ruff/a.py": [ + "ruff/b.py" ], - "ruff\\b.py": [], - "ruff\\c.py": [] + "ruff/b.py": [], + "ruff/c.py": [] } ----- stderr ----- - "#); + "###); test.write_file( "ruff.toml", @@ -125,24 +125,24 @@ fn type_checking_imports_from_config() -> anyhow::Result<()> { "#, )?; - assert_cmd_snapshot!(test.analyze_graph_command(), @r#" + assert_cmd_snapshot!(test.analyze_graph_command(), @r###" success: true exit_code: 0 ----- stdout ----- { - "ruff\/__init__.py": [], - "ruff\\a.py": [ - "ruff\\b.py", - "ruff\\c.py" + "ruff/__init__.py": [], + "ruff/a.py": [ + "ruff/b.py", + "ruff/c.py" ], - "ruff\\b.py": [ - "ruff\\c.py" + "ruff/b.py": [ + "ruff/c.py" ], - "ruff\\c.py": [] + "ruff/c.py": [] } ----- stderr ----- - "# + "### ); Ok(()) From 392858c1cb10bf76fe632d2ea23a5e906277899c Mon Sep 17 00:00:00 2001 From: Dan Date: Wed, 19 Nov 2025 16:08:19 -0500 Subject: [PATCH 05/12] Update truthiness checks to include t-strings and f-strings --- .../test/fixtures/flake8_simplify/SIM222.py | 1 + .../test/fixtures/flake8_simplify/SIM223.py | 1 + crates/ruff_python_ast/src/helpers.rs | 55 ++++++++++--------- 3 files changed, 30 insertions(+), 27 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 86603f413cd51..cf75b194efcf0 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM222.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM222.py @@ -220,6 +220,7 @@ def get_items_set(): # https://github.com/astral-sh/ruff/issues/21473 tuple("") or True # OK +tuple(t"") or True # OK tuple(0) or True # OK tuple(1) or True # OK tuple(False) 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 affbabe6e0811..15faa19680cff 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM223.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM223.py @@ -161,6 +161,7 @@ def f(a: "'' and 'b'"): ... # https://github.com/astral-sh/ruff/issues/21473 tuple("") and False # OK +tuple(t"") and False # OK tuple(0) and False # OK tuple(1) and False # OK tuple(False) and False # OK diff --git a/crates/ruff_python_ast/src/helpers.rs b/crates/ruff_python_ast/src/helpers.rs index ce4607d647129..a88ddd8f43c2b 100644 --- a/crates/ruff_python_ast/src/helpers.rs +++ b/crates/ruff_python_ast/src/helpers.rs @@ -1212,19 +1212,6 @@ pub enum Truthiness { Unknown, } -/// Return `true` if the expression is definitely not iterable (e.g., numbers, booleans, None). -/// This is used to avoid incorrectly assuming truthiness for `tuple(x)`, `list(x)`, etc. -/// when `x` is not iterable. -fn is_definitely_not_iterable(expr: &Expr) -> bool { - matches!( - expr, - Expr::NumberLiteral(_) - | Expr::BooleanLiteral(_) - | Expr::NoneLiteral(_) - | Expr::EllipsisLiteral(_) - ) -} - impl Truthiness { /// Return the truthiness of an expression. pub fn from_expr(expr: &Expr, is_builtin: F) -> Self @@ -1341,21 +1328,35 @@ impl Truthiness { // recursing into Self::from_expr below, which returns Unknown for them. if argument.is_generator_expr() { Self::Unknown - } else if is_definitely_not_iterable(argument) { - // For non-iterable arguments like numbers, booleans, None, etc., - // we can't assume truthiness. For example, tuple(0) raises TypeError. - Self::Unknown - } else if matches!(argument, Expr::StringLiteral(_)) { - // For strings, tuple("") creates an empty tuple which is falsy, - // but tuple("a") creates a non-empty tuple which is truthy. - // However, we can't assume truthiness matches because: - // - tuple("") is falsy (empty tuple), but "" is also falsy - // - The issue is that we conflate truthiness with non-emptiness - // - For SIM222/SIM223, we should be conservative and return Unknown - // to avoid false positives - Self::Unknown } else { - Self::from_expr(argument, is_builtin) + // Return Unknown for types with definite truthiness that might result + // in empty iterables or will raise a type error. We only recurse for + // types that don't have definite truthiness (e.g., collections, variables). + match argument { + // Types that raise TypeError when passed to tuple/list/set + Expr::NumberLiteral(_) + | Expr::BooleanLiteral(_) + | Expr::NoneLiteral(_) + | Expr::EllipsisLiteral(_) => { + // For non-iterable arguments like numbers, booleans, None, etc., + // we can't assume truthiness. For example, tuple(0) raises TypeError. + Self::Unknown + } + // Types with definite truthiness that might result in empty iterables + Expr::StringLiteral(_) + | Expr::TString(_) + | Expr::FString(_) + | Expr::BytesLiteral(_) => { + // For strings/t-strings/f-strings/bytes, tuple("") creates an empty + // tuple which is falsy, but tuple("a") creates a non-empty tuple + // which is truthy. However, we can't assume truthiness matches + // because we conflate truthiness with non-emptiness. For SIM222/SIM223, + // we should be conservative and return Unknown to avoid false positives. + Self::Unknown + } + // Recurse for types without definite truthiness (collections, variables, etc.) + _ => Self::from_expr(argument, is_builtin), + } } } else { Self::Unknown From a3d00c10a482ffbe693ef8f3a90cc48a36c26219 Mon Sep 17 00:00:00 2001 From: Dan Date: Wed, 19 Nov 2025 16:14:14 -0500 Subject: [PATCH 06/12] Explicitly handle various expression types --- crates/ruff_python_ast/src/helpers.rs | 45 ++++++++++++++++++++++----- 1 file changed, 37 insertions(+), 8 deletions(-) diff --git a/crates/ruff_python_ast/src/helpers.rs b/crates/ruff_python_ast/src/helpers.rs index a88ddd8f43c2b..f51946c5146f8 100644 --- a/crates/ruff_python_ast/src/helpers.rs +++ b/crates/ruff_python_ast/src/helpers.rs @@ -1330,8 +1330,9 @@ impl Truthiness { Self::Unknown } else { // Return Unknown for types with definite truthiness that might result - // in empty iterables or will raise a type error. We only recurse for - // types that don't have definite truthiness (e.g., collections, variables). + // in empty iterables or will raise a type error. Explicitly list types + // we recurse for (types without definite truthiness or where Self::from_expr + // correctly handles the truthiness). match argument { // Types that raise TypeError when passed to tuple/list/set Expr::NumberLiteral(_) @@ -1347,15 +1348,43 @@ impl Truthiness { | Expr::TString(_) | Expr::FString(_) | Expr::BytesLiteral(_) => { - // For strings/t-strings/f-strings/bytes, tuple("") creates an empty - // tuple which is falsy, but tuple("a") creates a non-empty tuple - // which is truthy. However, we can't assume truthiness matches - // because we conflate truthiness with non-emptiness. For SIM222/SIM223, + // For strings/t-strings/f-strings/bytes, tuple("") creates an empty tuple + // which is falsy, but tuple("a") creates a non-empty tuple which + // is truthy. However, we can't assume truthiness matches because + // we conflate truthiness with non-emptiness. For SIM222/SIM223, // we should be conservative and return Unknown to avoid false positives. Self::Unknown } - // Recurse for types without definite truthiness (collections, variables, etc.) - _ => Self::from_expr(argument, is_builtin), + // Recurse for collections - we need to check if they're empty + Expr::List(_) + | Expr::Set(_) + | Expr::Tuple(_) + | Expr::Dict(_) => Self::from_expr(argument, is_builtin), + // Recurse for comprehensions - they return Unknown in from_expr + Expr::ListComp(_) | Expr::SetComp(_) | Expr::DictComp(_) => { + Self::from_expr(argument, is_builtin) + } + // Recurse for variables and other expressions without definite truthiness + Expr::Name(_) + | Expr::Call(_) + | Expr::Attribute(_) + | Expr::Subscript(_) + | Expr::BinOp(_) + | Expr::UnaryOp(_) + | Expr::If(_) + | Expr::Starred(_) + | Expr::Await(_) + | Expr::Yield(_) + | Expr::YieldFrom(_) + | Expr::Compare(_) => Self::from_expr(argument, is_builtin), + // Return Unknown for types with definite truthiness that don't make sense to recurse + Expr::Lambda(_) | Expr::Generator(_) => { + // Lambda and Generator are always truthy, but when passed to tuple/list/set, + // they might result in empty iterables or raise errors. Return Unknown. + Self::Unknown + } + // Default to Unknown for any other types + _ => Self::Unknown, } } } else { From c4041b42b49200e28db668d58dd8ea5026added6 Mon Sep 17 00:00:00 2001 From: Dan Date: Wed, 19 Nov 2025 17:52:43 -0500 Subject: [PATCH 07/12] Consolidate logic --- crates/ruff_python_ast/src/helpers.rs | 85 +++++++-------------------- 1 file changed, 22 insertions(+), 63 deletions(-) diff --git a/crates/ruff_python_ast/src/helpers.rs b/crates/ruff_python_ast/src/helpers.rs index f51946c5146f8..139a584549a86 100644 --- a/crates/ruff_python_ast/src/helpers.rs +++ b/crates/ruff_python_ast/src/helpers.rs @@ -1322,70 +1322,29 @@ 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 { + // Return Unknown for types with definite truthiness that might result + // in empty iterables or will raise a type error. Explicitly list types + // we recurse for (types without definite truthiness or where Self::from_expr + // correctly handles the truthiness). + match argument { // Return Unknown for types with definite truthiness that might result - // in empty iterables or will raise a type error. Explicitly list types - // we recurse for (types without definite truthiness or where Self::from_expr - // correctly handles the truthiness). - match argument { - // Types that raise TypeError when passed to tuple/list/set - Expr::NumberLiteral(_) - | Expr::BooleanLiteral(_) - | Expr::NoneLiteral(_) - | Expr::EllipsisLiteral(_) => { - // For non-iterable arguments like numbers, booleans, None, etc., - // we can't assume truthiness. For example, tuple(0) raises TypeError. - Self::Unknown - } - // Types with definite truthiness that might result in empty iterables - Expr::StringLiteral(_) - | Expr::TString(_) - | Expr::FString(_) - | Expr::BytesLiteral(_) => { - // For strings/t-strings/f-strings/bytes, tuple("") creates an empty tuple - // which is falsy, but tuple("a") creates a non-empty tuple which - // is truthy. However, we can't assume truthiness matches because - // we conflate truthiness with non-emptiness. For SIM222/SIM223, - // we should be conservative and return Unknown to avoid false positives. - Self::Unknown - } - // Recurse for collections - we need to check if they're empty - Expr::List(_) - | Expr::Set(_) - | Expr::Tuple(_) - | Expr::Dict(_) => Self::from_expr(argument, is_builtin), - // Recurse for comprehensions - they return Unknown in from_expr - Expr::ListComp(_) | Expr::SetComp(_) | Expr::DictComp(_) => { - Self::from_expr(argument, is_builtin) - } - // Recurse for variables and other expressions without definite truthiness - Expr::Name(_) - | Expr::Call(_) - | Expr::Attribute(_) - | Expr::Subscript(_) - | Expr::BinOp(_) - | Expr::UnaryOp(_) - | Expr::If(_) - | Expr::Starred(_) - | Expr::Await(_) - | Expr::Yield(_) - | Expr::YieldFrom(_) - | Expr::Compare(_) => Self::from_expr(argument, is_builtin), - // Return Unknown for types with definite truthiness that don't make sense to recurse - Expr::Lambda(_) | Expr::Generator(_) => { - // Lambda and Generator are always truthy, but when passed to tuple/list/set, - // they might result in empty iterables or raise errors. Return Unknown. - Self::Unknown - } - // Default to Unknown for any other types - _ => Self::Unknown, - } + // in empty iterables or will raise a type error: + // - Non-iterable types (numbers, booleans, None, etc.) raise TypeError + // - String types: can't reliably determine truthiness of tuple("") from "" + // (tuple("") creates empty tuple, but tuple("a") creates non-empty tuple) + // - Lambda/Generator: always truthy but might result in empty iterables + Expr::NumberLiteral(_) + | Expr::BooleanLiteral(_) + | Expr::NoneLiteral(_) + | Expr::EllipsisLiteral(_) + | Expr::StringLiteral(_) + | Expr::TString(_) + | Expr::FString(_) + | Expr::BytesLiteral(_) + | Expr::Lambda(_) + | Expr::Generator(_) => Self::Unknown, + // Recurse for all other types - collections, comprehensions, variables, etc. + _ => Self::from_expr(argument, is_builtin), } } else { Self::Unknown From cc53973723eb2be37f86d300dfbc7984cfd84501 Mon Sep 17 00:00:00 2001 From: Dan Date: Fri, 28 Nov 2025 18:31:59 -0500 Subject: [PATCH 08/12] Recurse on string literals --- ...ke8_simplify__tests__SIM222_SIM222.py.snap | 20 +++++++++++++++++++ ...ke8_simplify__tests__SIM223_SIM223.py.snap | 20 +++++++++++++++++++ crates/ruff_python_ast/src/helpers.rs | 8 +++----- 3 files changed, 43 insertions(+), 5 deletions(-) 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 0e65033b21867..48893c8d6db94 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 # OK + | ^^^^^^^^^^^^^^^^^ +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 # OK +222 + True # OK +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 4fe01c8146a55..a5a4ed0499481 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 # OK + | ^^^^^^^^^^^^^^^^^^^ +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 # OK +163 + tuple("") # OK +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 139a584549a86..7f2cbf7549003 100644 --- a/crates/ruff_python_ast/src/helpers.rs +++ b/crates/ruff_python_ast/src/helpers.rs @@ -1330,20 +1330,18 @@ impl Truthiness { // Return Unknown for types with definite truthiness that might result // in empty iterables or will raise a type error: // - Non-iterable types (numbers, booleans, None, etc.) raise TypeError - // - String types: can't reliably determine truthiness of tuple("") from "" - // (tuple("") creates empty tuple, but tuple("a") creates non-empty tuple) + // - TString: always truthy but might result in empty iterables // - Lambda/Generator: always truthy but might result in empty iterables Expr::NumberLiteral(_) | Expr::BooleanLiteral(_) | Expr::NoneLiteral(_) | Expr::EllipsisLiteral(_) - | Expr::StringLiteral(_) | Expr::TString(_) - | Expr::FString(_) - | Expr::BytesLiteral(_) | 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 { From ad364955300810b405335f679bc21c8401e29374 Mon Sep 17 00:00:00 2001 From: Dan Date: Fri, 28 Nov 2025 18:44:22 -0500 Subject: [PATCH 09/12] Update ruff_linter__rules__flake8_pytest_style__tests__PT015.snap --- ...ter__rules__flake8_pytest_style__tests__PT015.snap | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT015.snap b/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT015.snap index 9ae9c29884a4e..51ddc586d57de 100644 --- a/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT015.snap +++ b/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT015.snap @@ -174,3 +174,14 @@ PT015 Assertion always fails, replace with `pytest.fail()` | ^^^^^^^^^^^^^^^^^ 25 | assert tuple("") | + +PT015 Assertion always fails, replace with `pytest.fail()` + --> PT015.py:25:5 + | +23 | assert list([]) +24 | assert set(set()) +25 | assert tuple("") + | ^^^^^^^^^^^^^^^^ +26 | +27 | # https://github.com/astral-sh/ruff/issues/19935 + | From 7349e9d5cc20b2baac93600067df113a224fd90a Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 1 Dec 2025 16:02:24 -0500 Subject: [PATCH 10/12] remove comment that is redundant with the one below --- crates/ruff_python_ast/src/helpers.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/crates/ruff_python_ast/src/helpers.rs b/crates/ruff_python_ast/src/helpers.rs index 7f2cbf7549003..7f8f90fee3d75 100644 --- a/crates/ruff_python_ast/src/helpers.rs +++ b/crates/ruff_python_ast/src/helpers.rs @@ -1322,10 +1322,6 @@ impl Truthiness { && arguments.keywords.is_empty() { // Ex) `list([1, 2, 3])` - // Return Unknown for types with definite truthiness that might result - // in empty iterables or will raise a type error. Explicitly list types - // we recurse for (types without definite truthiness or where Self::from_expr - // correctly handles the truthiness). match argument { // Return Unknown for types with definite truthiness that might result // in empty iterables or will raise a type error: From 8a5c075fb588d5e9b75564f58ce2da3eedc8a438 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 1 Dec 2025 16:02:40 -0500 Subject: [PATCH 11/12] update ok comment, add all covered cases --- .../resources/test/fixtures/flake8_simplify/SIM222.py | 5 ++++- .../resources/test/fixtures/flake8_simplify/SIM223.py | 5 ++++- ...er__rules__flake8_simplify__tests__SIM222_SIM222.py.snap | 6 +++--- ...er__rules__flake8_simplify__tests__SIM223_SIM223.py.snap | 6 +++--- 4 files changed, 14 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 cf75b194efcf0..62814c796f7e7 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM222.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM222.py @@ -219,9 +219,12 @@ def get_items_set(): # https://github.com/astral-sh/ruff/issues/21473 -tuple("") or True # OK +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 15faa19680cff..abcf2536bbb2c 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM223.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM223.py @@ -160,9 +160,12 @@ def f(a: "'' and 'b'"): ... # https://github.com/astral-sh/ruff/issues/21473 -tuple("") and False # OK +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 48893c8d6db94..a1ca861205b2c 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 @@ -1149,7 +1149,7 @@ SIM222 [*] Use `True` instead of `... or True` --> SIM222.py:222:1 | 221 | # https://github.com/astral-sh/ruff/issues/21473 -222 | tuple("") or True # OK +222 | tuple("") or True # SIM222 | ^^^^^^^^^^^^^^^^^ 223 | tuple(t"") or True # OK 224 | tuple(0) or True # OK @@ -1158,8 +1158,8 @@ help: Replace with `True` 219 | 220 | 221 | # https://github.com/astral-sh/ruff/issues/21473 - - tuple("") or True # OK -222 + True # OK + - 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 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 a5a4ed0499481..08f3f48ba200c 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 @@ -1030,7 +1030,7 @@ SIM223 [*] Use `tuple("")` instead of `tuple("") and ...` --> SIM223.py:163:1 | 162 | # https://github.com/astral-sh/ruff/issues/21473 -163 | tuple("") and False # OK +163 | tuple("") and False # SIM223 | ^^^^^^^^^^^^^^^^^^^ 164 | tuple(t"") and False # OK 165 | tuple(0) and False # OK @@ -1039,8 +1039,8 @@ help: Replace with `tuple("")` 160 | 161 | 162 | # https://github.com/astral-sh/ruff/issues/21473 - - tuple("") and False # OK -163 + tuple("") # OK + - 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 From 893bcdfca9c6e8b4a8c15b76db8d629f1c511d14 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 1 Dec 2025 16:08:05 -0500 Subject: [PATCH 12/12] simplify comment slightly --- crates/ruff_python_ast/src/helpers.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/crates/ruff_python_ast/src/helpers.rs b/crates/ruff_python_ast/src/helpers.rs index 7f8f90fee3d75..4879e047804f2 100644 --- a/crates/ruff_python_ast/src/helpers.rs +++ b/crates/ruff_python_ast/src/helpers.rs @@ -1323,11 +1323,10 @@ impl Truthiness { { // Ex) `list([1, 2, 3])` match argument { - // Return Unknown for types with definite truthiness that might result - // in empty iterables or will raise a type error: - // - Non-iterable types (numbers, booleans, None, etc.) raise TypeError - // - TString: always truthy but might result in empty iterables - // - Lambda/Generator: always truthy but might result in empty iterables + // 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(_)