From 76b6d53d8bc0960afc41108eac633836eaf77202 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcus=20N=C3=A4slund?= Date: Tue, 6 May 2025 15:52:07 +0200 Subject: [PATCH] Add new rule InEmptyCollection (#16480) ## Summary Introducing a new rule based on discussions in #15732 and #15729 that checks for unnecessary in with empty collections. I called it in_empty_collection and gave the rule number RUF060. Rule is in preview group. --- .../resources/test/fixtures/ruff/RUF060.py | 37 ++++ .../src/checkers/ast/analyze/expression.rs | 3 + crates/ruff_linter/src/codes.rs | 1 + crates/ruff_linter/src/rules/ruff/mod.rs | 1 + .../rules/ruff/rules/in_empty_collection.rs | 89 ++++++++ .../ruff_linter/src/rules/ruff/rules/mod.rs | 2 + ..._rules__ruff__tests__RUF060_RUF060.py.snap | 200 ++++++++++++++++++ ruff.schema.json | 2 + 8 files changed, 335 insertions(+) create mode 100644 crates/ruff_linter/resources/test/fixtures/ruff/RUF060.py create mode 100644 crates/ruff_linter/src/rules/ruff/rules/in_empty_collection.rs create mode 100644 crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF060_RUF060.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF060.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF060.py new file mode 100644 index 0000000000..52ea18de39 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF060.py @@ -0,0 +1,37 @@ +# Errors +1 in [] +1 not in [] +2 in list() +2 not in list() +_ in () +_ not in () +'x' in tuple() +'y' not in tuple() +'a' in set() +'a' not in set() +'b' in {} +'b' not in {} +1 in dict() +2 not in dict() +"a" in "" +b'c' in b"" +"b" in f"" +b"a" in bytearray() +b"a" in bytes() +1 in frozenset() + +# OK +1 in [2] +1 in [1, 2, 3] +_ in ('a') +_ not in ('a') +'a' in set('a', 'b') +'a' not in set('b', 'c') +'b' in {1: 2} +'b' not in {3: 4} +"a" in "x" +b'c' in b"x" +"b" in f"x" +b"a" in bytearray([2]) +b"a" in bytes("a", "utf-8") +1 in frozenset("c") diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index 896b6f9dfe..4e332e3d9f 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -1504,6 +1504,9 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) { if checker.enabled(Rule::NanComparison) { pylint::rules::nan_comparison(checker, left, comparators); } + if checker.enabled(Rule::InEmptyCollection) { + ruff::rules::in_empty_collection(checker, compare); + } if checker.enabled(Rule::InDictKeys) { flake8_simplify::rules::key_in_dict_compare(checker, compare); } diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 4f58c6a116..f2bde04729 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -1014,6 +1014,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Ruff, "057") => (RuleGroup::Preview, rules::ruff::rules::UnnecessaryRound), (Ruff, "058") => (RuleGroup::Preview, rules::ruff::rules::StarmapZip), (Ruff, "059") => (RuleGroup::Preview, rules::ruff::rules::UnusedUnpackedVariable), + (Ruff, "060") => (RuleGroup::Preview, rules::ruff::rules::InEmptyCollection), (Ruff, "100") => (RuleGroup::Stable, rules::ruff::rules::UnusedNOQA), (Ruff, "101") => (RuleGroup::Stable, rules::ruff::rules::RedirectedNOQA), (Ruff, "102") => (RuleGroup::Preview, rules::ruff::rules::InvalidRuleCode), diff --git a/crates/ruff_linter/src/rules/ruff/mod.rs b/crates/ruff_linter/src/rules/ruff/mod.rs index f1dc5714f1..ab7944e989 100644 --- a/crates/ruff_linter/src/rules/ruff/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/mod.rs @@ -99,6 +99,7 @@ mod tests { #[test_case(Rule::UnusedUnpackedVariable, Path::new("RUF059_1.py"))] #[test_case(Rule::UnusedUnpackedVariable, Path::new("RUF059_2.py"))] #[test_case(Rule::UnusedUnpackedVariable, Path::new("RUF059_3.py"))] + #[test_case(Rule::InEmptyCollection, Path::new("RUF060.py"))] #[test_case(Rule::RedirectedNOQA, Path::new("RUF101_0.py"))] #[test_case(Rule::RedirectedNOQA, Path::new("RUF101_1.py"))] #[test_case(Rule::InvalidRuleCode, Path::new("RUF102.py"))] 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 new file mode 100644 index 0000000000..142b968b1b --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/rules/in_empty_collection.rs @@ -0,0 +1,89 @@ +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, ViolationMetadata}; +use ruff_python_ast::{self as ast, CmpOp, Expr}; +use ruff_text_size::Ranged; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Checks for membership tests on empty collections (such as `list`, `tuple`, `set` or `dict`). +/// +/// ## Why is this bad? +/// If the collection is always empty, the check is unnecessary, and can be removed. +/// +/// ## Example +/// +/// ```python +/// if 1 not in set(): +/// print("got it!") +/// ``` +/// +/// Use instead: +/// +/// ```python +/// print("got it!") +/// ``` +#[derive(ViolationMetadata)] +pub(crate) struct InEmptyCollection; + +impl Violation for InEmptyCollection { + #[derive_message_formats] + fn message(&self) -> String { + "Unnecessary membership test on empty collection".to_string() + } +} + +/// RUF060 +pub(crate) fn in_empty_collection(checker: &Checker, compare: &ast::ExprCompare) { + let [op] = &*compare.ops else { + return; + }; + + if !matches!(op, CmpOp::In | CmpOp::NotIn) { + return; + } + + let [right] = &*compare.comparators else { + return; + }; + + let semantic = checker.semantic(); + let collection_methods = [ + "list", + "tuple", + "set", + "frozenset", + "dict", + "bytes", + "bytearray", + "str", + ]; + + let is_empty_collection = match right { + 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::Dict(ast::ExprDict { items, .. }) => items.is_empty(), + Expr::BytesLiteral(ast::ExprBytesLiteral { value, .. }) => value.is_empty(), + Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) => value.is_empty(), + Expr::FString(s) => s + .value + .elements() + .all(|elt| elt.as_literal().is_some_and(|elt| elt.is_empty())), + Expr::Call(ast::ExprCall { + func, + arguments, + range: _, + }) => { + arguments.is_empty() + && collection_methods + .iter() + .any(|s| semantic.match_builtin_expr(func, s)) + } + _ => false, + }; + + if is_empty_collection { + checker.report_diagnostic(Diagnostic::new(InEmptyCollection, compare.range())); + } +} diff --git a/crates/ruff_linter/src/rules/ruff/rules/mod.rs b/crates/ruff_linter/src/rules/ruff/rules/mod.rs index b0aff0016e..4706c713c6 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/mod.rs @@ -13,6 +13,7 @@ pub(crate) use function_call_in_dataclass_default::*; pub(crate) use if_key_in_dict_del::*; pub(crate) use implicit_classvar_in_dataclass::*; pub(crate) use implicit_optional::*; +pub(crate) use in_empty_collection::*; pub(crate) use incorrectly_parenthesized_tuple_in_subscript::*; pub(crate) use indented_form_feed::*; pub(crate) use invalid_assert_message_literal_argument::*; @@ -73,6 +74,7 @@ mod helpers; mod if_key_in_dict_del; mod implicit_classvar_in_dataclass; mod implicit_optional; +mod in_empty_collection; mod incorrectly_parenthesized_tuple_in_subscript; mod indented_form_feed; mod invalid_assert_message_literal_argument; 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 new file mode 100644 index 0000000000..a269459cd7 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF060_RUF060.py.snap @@ -0,0 +1,200 @@ +--- +source: crates/ruff_linter/src/rules/ruff/mod.rs +--- +RUF060.py:2:1: RUF060 Unnecessary membership test on empty collection + | +1 | # Errors +2 | 1 in [] + | ^^^^^^^ RUF060 +3 | 1 not in [] +4 | 2 in list() + | + +RUF060.py:3:1: RUF060 Unnecessary membership test on empty collection + | +1 | # Errors +2 | 1 in [] +3 | 1 not in [] + | ^^^^^^^^^^^ RUF060 +4 | 2 in list() +5 | 2 not in list() + | + +RUF060.py:4:1: RUF060 Unnecessary membership test on empty collection + | +2 | 1 in [] +3 | 1 not in [] +4 | 2 in list() + | ^^^^^^^^^^^ RUF060 +5 | 2 not in list() +6 | _ in () + | + +RUF060.py:5:1: RUF060 Unnecessary membership test on empty collection + | +3 | 1 not in [] +4 | 2 in list() +5 | 2 not in list() + | ^^^^^^^^^^^^^^^ RUF060 +6 | _ in () +7 | _ not in () + | + +RUF060.py:6:1: RUF060 Unnecessary membership test on empty collection + | +4 | 2 in list() +5 | 2 not in list() +6 | _ in () + | ^^^^^^^ RUF060 +7 | _ not in () +8 | 'x' in tuple() + | + +RUF060.py:7:1: RUF060 Unnecessary membership test on empty collection + | +5 | 2 not in list() +6 | _ in () +7 | _ not in () + | ^^^^^^^^^^^ RUF060 +8 | 'x' in tuple() +9 | 'y' not in tuple() + | + +RUF060.py:8:1: RUF060 Unnecessary membership test on empty collection + | + 6 | _ in () + 7 | _ not in () + 8 | 'x' in tuple() + | ^^^^^^^^^^^^^^ RUF060 + 9 | 'y' not in tuple() +10 | 'a' in set() + | + +RUF060.py:9:1: RUF060 Unnecessary membership test on empty collection + | + 7 | _ not in () + 8 | 'x' in tuple() + 9 | 'y' not in tuple() + | ^^^^^^^^^^^^^^^^^^ RUF060 +10 | 'a' in set() +11 | 'a' not in set() + | + +RUF060.py:10:1: RUF060 Unnecessary membership test on empty collection + | + 8 | 'x' in tuple() + 9 | 'y' not in tuple() +10 | 'a' in set() + | ^^^^^^^^^^^^ RUF060 +11 | 'a' not in set() +12 | 'b' in {} + | + +RUF060.py:11:1: RUF060 Unnecessary membership test on empty collection + | + 9 | 'y' not in tuple() +10 | 'a' in set() +11 | 'a' not in set() + | ^^^^^^^^^^^^^^^^ RUF060 +12 | 'b' in {} +13 | 'b' not in {} + | + +RUF060.py:12:1: RUF060 Unnecessary membership test on empty collection + | +10 | 'a' in set() +11 | 'a' not in set() +12 | 'b' in {} + | ^^^^^^^^^ RUF060 +13 | 'b' not in {} +14 | 1 in dict() + | + +RUF060.py:13:1: RUF060 Unnecessary membership test on empty collection + | +11 | 'a' not in set() +12 | 'b' in {} +13 | 'b' not in {} + | ^^^^^^^^^^^^^ RUF060 +14 | 1 in dict() +15 | 2 not in dict() + | + +RUF060.py:14:1: RUF060 Unnecessary membership test on empty collection + | +12 | 'b' in {} +13 | 'b' not in {} +14 | 1 in dict() + | ^^^^^^^^^^^ RUF060 +15 | 2 not in dict() +16 | "a" in "" + | + +RUF060.py:15:1: RUF060 Unnecessary membership test on empty collection + | +13 | 'b' not in {} +14 | 1 in dict() +15 | 2 not in dict() + | ^^^^^^^^^^^^^^^ RUF060 +16 | "a" in "" +17 | b'c' in b"" + | + +RUF060.py:16:1: RUF060 Unnecessary membership test on empty collection + | +14 | 1 in dict() +15 | 2 not in dict() +16 | "a" in "" + | ^^^^^^^^^ RUF060 +17 | b'c' in b"" +18 | "b" in f"" + | + +RUF060.py:17:1: RUF060 Unnecessary membership test on empty collection + | +15 | 2 not in dict() +16 | "a" in "" +17 | b'c' in b"" + | ^^^^^^^^^^^ RUF060 +18 | "b" in f"" +19 | b"a" in bytearray() + | + +RUF060.py:18:1: RUF060 Unnecessary membership test on empty collection + | +16 | "a" in "" +17 | b'c' in b"" +18 | "b" in f"" + | ^^^^^^^^^^ RUF060 +19 | b"a" in bytearray() +20 | b"a" in bytes() + | + +RUF060.py:19:1: RUF060 Unnecessary membership test on empty collection + | +17 | b'c' in b"" +18 | "b" in f"" +19 | b"a" in bytearray() + | ^^^^^^^^^^^^^^^^^^^ RUF060 +20 | b"a" in bytes() +21 | 1 in frozenset() + | + +RUF060.py:20:1: RUF060 Unnecessary membership test on empty collection + | +18 | "b" in f"" +19 | b"a" in bytearray() +20 | b"a" in bytes() + | ^^^^^^^^^^^^^^^ RUF060 +21 | 1 in frozenset() + | + +RUF060.py:21:1: RUF060 Unnecessary membership test on empty collection + | +19 | b"a" in bytearray() +20 | b"a" in bytes() +21 | 1 in frozenset() + | ^^^^^^^^^^^^^^^^ RUF060 +22 | +23 | # OK + | diff --git a/ruff.schema.json b/ruff.schema.json index 446ce35e58..0059b7bb2f 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -4035,6 +4035,8 @@ "RUF057", "RUF058", "RUF059", + "RUF06", + "RUF060", "RUF1", "RUF10", "RUF100",