From 9e004118cbf780d9632e6df4a11bdf72f90ca6e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcus=20N=C3=A4slund?= Date: Wed, 7 May 2025 15:49:59 +0200 Subject: [PATCH 1/2] Recursive check for RUF060 --- .../resources/test/fixtures/ruff/RUF060.py | 4 +++ .../rules/ruff/rules/in_empty_collection.rs | 33 ++++++++++++++----- ..._rules__ruff__tests__RUF060_RUF060.py.snap | 24 ++++++++++++-- 3 files changed, 51 insertions(+), 10 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF060.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF060.py index 52ea18de39a913..f69a34997ace80 100644 --- a/crates/ruff_linter/resources/test/fixtures/ruff/RUF060.py +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF060.py @@ -19,6 +19,8 @@ b"a" in bytearray() b"a" in bytes() 1 in frozenset() +1 in set(set()) +2 in frozenset([]) # OK 1 in [2] @@ -35,3 +37,5 @@ b"a" in bytearray([2]) b"a" in bytes("a", "utf-8") 1 in frozenset("c") +1 in set(set((1,2))) +1 in set(set([1])) diff --git a/crates/ruff_linter/src/rules/ruff/rules/in_empty_collection.rs b/crates/ruff_linter/src/rules/ruff/rules/in_empty_collection.rs index 142b968b1b8eea..76519b537af4b9 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/in_empty_collection.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/in_empty_collection.rs @@ -1,6 +1,7 @@ use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, ViolationMetadata}; use ruff_python_ast::{self as ast, CmpOp, Expr}; +use ruff_python_semantic::SemanticModel; use ruff_text_size::Ranged; use crate::checkers::ast::Checker; @@ -48,6 +49,14 @@ pub(crate) fn in_empty_collection(checker: &Checker, compare: &ast::ExprCompare) }; let semantic = checker.semantic(); + + if is_empty(right, semantic) { + checker.report_diagnostic(Diagnostic::new(InEmptyCollection, compare.range())); + } +} + +fn is_empty(expr: &Expr, semantic: &SemanticModel) -> bool { + let set_methods = ["set", "frozenset"]; let collection_methods = [ "list", "tuple", @@ -59,10 +68,14 @@ pub(crate) fn in_empty_collection(checker: &Checker, compare: &ast::ExprCompare) "str", ]; - let is_empty_collection = match right { + match expr { Expr::List(ast::ExprList { elts, .. }) => elts.is_empty(), Expr::Tuple(ast::ExprTuple { elts, .. }) => elts.is_empty(), - Expr::Set(ast::ExprSet { elts, .. }) => elts.is_empty(), + Expr::Set(ast::ExprSet { elts, .. }) => match elts.as_slice() { + [] => true, + [element] => is_empty(element, semantic), + _ => false, + }, Expr::Dict(ast::ExprDict { items, .. }) => items.is_empty(), Expr::BytesLiteral(ast::ExprBytesLiteral { value, .. }) => value.is_empty(), Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) => value.is_empty(), @@ -75,15 +88,19 @@ pub(crate) fn in_empty_collection(checker: &Checker, compare: &ast::ExprCompare) arguments, range: _, }) => { - arguments.is_empty() - && collection_methods + if arguments.is_empty() { + collection_methods .iter() .any(|s| semantic.match_builtin_expr(func, s)) + } else if let Some(arg) = arguments.find_positional(0) { + set_methods + .iter() + .any(|s| semantic.match_builtin_expr(func, s)) + && is_empty(arg, semantic) + } else { + false + } } _ => false, - }; - - if is_empty_collection { - checker.report_diagnostic(Diagnostic::new(InEmptyCollection, compare.range())); } } diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF060_RUF060.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF060_RUF060.py.snap index a269459cd7c0c1..ccca3b19b2b0a9 100644 --- a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF060_RUF060.py.snap +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF060_RUF060.py.snap @@ -187,6 +187,7 @@ RUF060.py:20:1: RUF060 Unnecessary membership test on empty collection 20 | b"a" in bytes() | ^^^^^^^^^^^^^^^ RUF060 21 | 1 in frozenset() +22 | 1 in set(set()) | RUF060.py:21:1: RUF060 Unnecessary membership test on empty collection @@ -195,6 +196,25 @@ RUF060.py:21:1: RUF060 Unnecessary membership test on empty collection 20 | b"a" in bytes() 21 | 1 in frozenset() | ^^^^^^^^^^^^^^^^ RUF060 -22 | -23 | # OK +22 | 1 in set(set()) +23 | 2 in frozenset([]) + | + +RUF060.py:22:1: RUF060 Unnecessary membership test on empty collection + | +20 | b"a" in bytes() +21 | 1 in frozenset() +22 | 1 in set(set()) + | ^^^^^^^^^^^^^^^ RUF060 +23 | 2 in frozenset([]) + | + +RUF060.py:23:1: RUF060 Unnecessary membership test on empty collection + | +21 | 1 in frozenset() +22 | 1 in set(set()) +23 | 2 in frozenset([]) + | ^^^^^^^^^^^^^^^^^^ RUF060 +24 | +25 | # OK | From 356ab4bc806145e08d0eacb28024ae09ce55c5f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcus=20N=C3=A4slund?= Date: Fri, 9 May 2025 22:10:46 +0200 Subject: [PATCH 2/2] Limited recursive check in RUF060 + more test cases --- .../resources/test/fixtures/ruff/RUF060.py | 3 +++ .../src/rules/ruff/rules/in_empty_collection.rs | 6 +----- ...nter__rules__ruff__tests__RUF060_RUF060.py.snap | 14 ++++++++++++-- 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF060.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF060.py index f69a34997ace80..c3b96c6b7be52f 100644 --- a/crates/ruff_linter/resources/test/fixtures/ruff/RUF060.py +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF060.py @@ -21,6 +21,7 @@ 1 in frozenset() 1 in set(set()) 2 in frozenset([]) +'' in set("") # OK 1 in [2] @@ -39,3 +40,5 @@ 1 in frozenset("c") 1 in set(set((1,2))) 1 in set(set([1])) +'' in {""} +frozenset() in {frozenset()} diff --git a/crates/ruff_linter/src/rules/ruff/rules/in_empty_collection.rs b/crates/ruff_linter/src/rules/ruff/rules/in_empty_collection.rs index 76519b537af4b9..9132508e8cf687 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/in_empty_collection.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/in_empty_collection.rs @@ -71,11 +71,7 @@ fn is_empty(expr: &Expr, semantic: &SemanticModel) -> bool { match expr { Expr::List(ast::ExprList { elts, .. }) => elts.is_empty(), Expr::Tuple(ast::ExprTuple { elts, .. }) => elts.is_empty(), - Expr::Set(ast::ExprSet { elts, .. }) => match elts.as_slice() { - [] => true, - [element] => is_empty(element, semantic), - _ => false, - }, + Expr::Set(ast::ExprSet { elts, .. }) => elts.is_empty(), Expr::Dict(ast::ExprDict { items, .. }) => items.is_empty(), Expr::BytesLiteral(ast::ExprBytesLiteral { value, .. }) => value.is_empty(), Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) => value.is_empty(), diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF060_RUF060.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF060_RUF060.py.snap index ccca3b19b2b0a9..ac92e967a97926 100644 --- a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF060_RUF060.py.snap +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF060_RUF060.py.snap @@ -207,6 +207,7 @@ RUF060.py:22:1: RUF060 Unnecessary membership test on empty collection 22 | 1 in set(set()) | ^^^^^^^^^^^^^^^ RUF060 23 | 2 in frozenset([]) +24 | '' in set("") | RUF060.py:23:1: RUF060 Unnecessary membership test on empty collection @@ -215,6 +216,15 @@ RUF060.py:23:1: RUF060 Unnecessary membership test on empty collection 22 | 1 in set(set()) 23 | 2 in frozenset([]) | ^^^^^^^^^^^^^^^^^^ RUF060 -24 | -25 | # OK +24 | '' in set("") + | + +RUF060.py:24:1: RUF060 Unnecessary membership test on empty collection + | +22 | 1 in set(set()) +23 | 2 in frozenset([]) +24 | '' in set("") + | ^^^^^^^^^^^^^ RUF060 +25 | +26 | # OK |