diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/literal_membership.py b/crates/ruff_linter/resources/test/fixtures/pylint/literal_membership.py new file mode 100644 index 0000000000000..84e0df55c1384 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pylint/literal_membership.py @@ -0,0 +1,10 @@ +# Errors +1 in [1, 2, 3] +1 in (1, 2, 3) +1 in ( + 1, 2, 3 +) + +# OK +fruits = ["cherry", "grapes"] +"cherry" in fruits diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index 8d11a323368a8..a686c2c1b6678 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -1197,6 +1197,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { if checker.enabled(Rule::ComparisonWithItself) { pylint::rules::comparison_with_itself(checker, left, ops, comparators); } + if checker.enabled(Rule::LiteralMembership) { + pylint::rules::literal_membership(checker, compare); + } if checker.enabled(Rule::ComparisonOfConstant) { pylint::rules::comparison_of_constant(checker, left, ops, comparators); } diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index d1fc068d86e38..c67d4e9a6b3e4 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -254,6 +254,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Pylint, "R1722") => (RuleGroup::Stable, rules::pylint::rules::SysExitAlias), (Pylint, "R2004") => (RuleGroup::Stable, rules::pylint::rules::MagicValueComparison), (Pylint, "R5501") => (RuleGroup::Stable, rules::pylint::rules::CollapsibleElseIf), + (Pylint, "R6201") => (RuleGroup::Preview, rules::pylint::rules::LiteralMembership), #[allow(deprecated)] (Pylint, "R6301") => (RuleGroup::Nursery, rules::pylint::rules::NoSelfUse), (Pylint, "W0120") => (RuleGroup::Stable, rules::pylint::rules::UselessElseOnLoop), diff --git a/crates/ruff_linter/src/rules/pylint/mod.rs b/crates/ruff_linter/src/rules/pylint/mod.rs index bd79d334d9276..cf7d903f7a86e 100644 --- a/crates/ruff_linter/src/rules/pylint/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/mod.rs @@ -137,6 +137,7 @@ mod tests { #[test_case(Rule::BadDunderMethodName, Path::new("bad_dunder_method_name.py"))] #[test_case(Rule::NoSelfUse, Path::new("no_self_use.py"))] #[test_case(Rule::MisplacedBareRaise, Path::new("misplaced_bare_raise.py"))] + #[test_case(Rule::LiteralMembership, Path::new("literal_membership.py"))] fn rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); let diagnostics = test_path( diff --git a/crates/ruff_linter/src/rules/pylint/rules/literal_membership.rs b/crates/ruff_linter/src/rules/pylint/rules/literal_membership.rs new file mode 100644 index 0000000000000..e69a09c916067 --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/rules/literal_membership.rs @@ -0,0 +1,65 @@ +use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; +use ruff_macros::{derive_message_formats, violation}; +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 `list` and `tuple` literals. +/// +/// ## Why is this bad? +/// When testing for membership in a static sequence, prefer a `set` literal +/// over a `list` or `tuple`, as Python optimizes `set` membership tests. +/// +/// ## Example +/// ```python +/// 1 in [1, 2, 3] +/// ``` +/// +/// Use instead: +/// ```python +/// 1 in {1, 2, 3} +/// ``` +/// ## References +/// - [What’s New In Python 3.2](https://docs.python.org/3/whatsnew/3.2.html#optimizations) +#[violation] +pub struct LiteralMembership; + +impl AlwaysFixableViolation for LiteralMembership { + #[derive_message_formats] + fn message(&self) -> String { + format!("Use a `set` literal when testing for membership") + } + + fn fix_title(&self) -> String { + format!("Convert to `set`") + } +} + +/// PLR6201 +pub(crate) fn literal_membership(checker: &mut Checker, compare: &ast::ExprCompare) { + let [op] = compare.ops.as_slice() else { + return; + }; + + if !matches!(op, CmpOp::In | CmpOp::NotIn) { + return; + } + + let [right] = compare.comparators.as_slice() else { + return; + }; + + if !matches!(right, Expr::List(_) | Expr::Tuple(_)) { + return; + } + + let mut diagnostic = Diagnostic::new(LiteralMembership, right.range()); + + let literal = checker.locator().slice(right); + let set = format!("{{{}}}", &literal[1..literal.len() - 1]); + diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(set, right.range()))); + + checker.diagnostics.push(diagnostic); +} diff --git a/crates/ruff_linter/src/rules/pylint/rules/mod.rs b/crates/ruff_linter/src/rules/pylint/rules/mod.rs index 4a213ec29f19c..2d7495ac07909 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/mod.rs @@ -24,6 +24,7 @@ pub(crate) use invalid_envvar_value::*; pub(crate) use invalid_str_return::*; pub(crate) use invalid_string_characters::*; pub(crate) use iteration_over_set::*; +pub(crate) use literal_membership::*; pub(crate) use load_before_global_declaration::*; pub(crate) use logging::*; pub(crate) use magic_value_comparison::*; @@ -86,6 +87,7 @@ mod invalid_envvar_value; mod invalid_str_return; mod invalid_string_characters; mod iteration_over_set; +mod literal_membership; mod load_before_global_declaration; mod logging; mod magic_value_comparison; diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR6201_literal_membership.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR6201_literal_membership.py.snap new file mode 100644 index 0000000000000..bb3c795519038 --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR6201_literal_membership.py.snap @@ -0,0 +1,69 @@ +--- +source: crates/ruff_linter/src/rules/pylint/mod.rs +--- +literal_membership.py:2:6: PLR6201 [*] Use a `set` literal when testing for membership + | +1 | # Errors +2 | 1 in [1, 2, 3] + | ^^^^^^^^^ PLR6201 +3 | 1 in (1, 2, 3) +4 | 1 in ( + | + = help: Convert to `set` + +ℹ Fix +1 1 | # Errors +2 |-1 in [1, 2, 3] + 2 |+1 in {1, 2, 3} +3 3 | 1 in (1, 2, 3) +4 4 | 1 in ( +5 5 | 1, 2, 3 + +literal_membership.py:3:6: PLR6201 [*] Use a `set` literal when testing for membership + | +1 | # Errors +2 | 1 in [1, 2, 3] +3 | 1 in (1, 2, 3) + | ^^^^^^^^^ PLR6201 +4 | 1 in ( +5 | 1, 2, 3 + | + = help: Convert to `set` + +ℹ Fix +1 1 | # Errors +2 2 | 1 in [1, 2, 3] +3 |-1 in (1, 2, 3) + 3 |+1 in {1, 2, 3} +4 4 | 1 in ( +5 5 | 1, 2, 3 +6 6 | ) + +literal_membership.py:4:6: PLR6201 [*] Use a `set` literal when testing for membership + | +2 | 1 in [1, 2, 3] +3 | 1 in (1, 2, 3) +4 | 1 in ( + | ______^ +5 | | 1, 2, 3 +6 | | ) + | |_^ PLR6201 +7 | +8 | # OK + | + = help: Convert to `set` + +ℹ Fix +1 1 | # Errors +2 2 | 1 in [1, 2, 3] +3 3 | 1 in (1, 2, 3) +4 |-1 in ( + 4 |+1 in { +5 5 | 1, 2, 3 +6 |-) + 6 |+} +7 7 | +8 8 | # OK +9 9 | fruits = ["cherry", "grapes"] + + diff --git a/ruff.schema.json b/ruff.schema.json index b37f4d78685e8..9c5e70507bc4d 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2968,6 +2968,9 @@ "PLR550", "PLR5501", "PLR6", + "PLR62", + "PLR620", + "PLR6201", "PLR63", "PLR630", "PLR6301",