[refurb] Implement single-item-membership-test (FURB171) (#7815)

## Summary

Implement
[`no-single-item-in`](https://github.com/dosisod/refurb/blob/master/refurb/checks/iterable/no_single_item_in.py)
as `single-item-membership-test` (`FURB171`).

Uses the helper function `generate_comparison` from the `pycodestyle`
implementations; this function should probably be moved, but I am not
sure where at the moment.

Update: moved it to `ruff_python_ast::helpers`.

Related to #1348.

## Test Plan

`cargo test`

crates/ruff_linter/resources/test/fixtures/refurb/FURB171.py

@@ -0,0 +1,45 @@
+# Errors.
+
+if 1 in (1,):
+    print("Single-element tuple")
+
+if 1 in [1]:
+    print("Single-element list")
+
+if 1 in {1}:
+    print("Single-element set")
+
+if "a" in "a":
+    print("Single-element string")
+
+if 1 not in (1,):
+    print("Check `not in` membership test")
+
+if not 1 in (1,):
+    print("Check the negated membership test")
+
+# Non-errors.
+
+if 1 in (1, 2):
+    pass
+
+if 1 in [1, 2]:
+    pass
+
+if 1 in {1, 2}:
+    pass
+
+if "a" in "ab":
+    pass
+
+if 1 == (1,):
+    pass
+
+if 1 > [1]:
+    pass
+
+if 1 is {1}:
+    pass
+
+if "a" == "a":
+    pass

crates/ruff_linter/src/checkers/ast/analyze/expression.rs

@@ -1218,6 +1218,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
                     comparators,
                 );
             }
+            if checker.enabled(Rule::SingleItemMembershipTest) {
+                refurb::rules::single_item_membership_test(checker, expr, left, ops, comparators);
+            }
         }
         Expr::Constant(ast::ExprConstant {
             value: Constant::Int(_) | Constant::Float(_) | Constant::Complex { .. },

crates/ruff_linter/src/codes.rs

@@ -923,6 +923,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Refurb, "140") => (RuleGroup::Preview, rules::refurb::rules::ReimplementedStarmap),
         (Refurb, "145") => (RuleGroup::Preview, rules::refurb::rules::SliceCopy),
         (Refurb, "148") => (RuleGroup::Preview, rules::refurb::rules::UnnecessaryEnumerate),
+        (Refurb, "171") => (RuleGroup::Preview, rules::refurb::rules::SingleItemMembershipTest),
         (Refurb, "177") => (RuleGroup::Preview, rules::refurb::rules::ImplicitCwd),
 
         // flake8-logging

crates/ruff_linter/src/rules/pycodestyle/helpers.rs

@@ -1,62 +1,3 @@
-use ruff_python_ast::node::AnyNodeRef;
-use ruff_python_ast::parenthesize::parenthesized_range;
-use ruff_python_ast::{CmpOp, Expr};
-use ruff_python_trivia::CommentRanges;
-use ruff_source_file::Locator;
-use ruff_text_size::Ranged;
-
 pub(super) fn is_ambiguous_name(name: &str) -> bool {
     name == "l" || name == "I" || name == "O"
 }
-
-pub(super) fn generate_comparison(
-    left: &Expr,
-    ops: &[CmpOp],
-    comparators: &[Expr],
-    parent: AnyNodeRef,
-    comment_ranges: &CommentRanges,
-    locator: &Locator,
-) -> String {
-    let start = left.start();
-    let end = comparators.last().map_or_else(|| left.end(), Ranged::end);
-    let mut contents = String::with_capacity(usize::from(end - start));
-
-    // Add the left side of the comparison.
-    contents.push_str(
-        locator.slice(
-            parenthesized_range(left.into(), parent, comment_ranges, locator.contents())
-                .unwrap_or(left.range()),
-        ),
-    );
-
-    for (op, comparator) in ops.iter().zip(comparators) {
-        // Add the operator.
-        contents.push_str(match op {
-            CmpOp::Eq => " == ",
-            CmpOp::NotEq => " != ",
-            CmpOp::Lt => " < ",
-            CmpOp::LtE => " <= ",
-            CmpOp::Gt => " > ",
-            CmpOp::GtE => " >= ",
-            CmpOp::In => " in ",
-            CmpOp::NotIn => " not in ",
-            CmpOp::Is => " is ",
-            CmpOp::IsNot => " is not ",
-        });
-
-        // Add the right side of the comparison.
-        contents.push_str(
-            locator.slice(
-                parenthesized_range(
-                    comparator.into(),
-                    parent,
-                    comment_ranges,
-                    locator.contents(),
-                )
-                .unwrap_or(comparator.range()),
-            ),
-        );
-    }
-
-    contents
-}

crates/ruff_linter/src/rules/pycodestyle/rules/literal_comparisons.rs

@@ -3,14 +3,13 @@ use rustc_hash::FxHashMap;
 use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
 use ruff_macros::{derive_message_formats, violation};
 use ruff_python_ast::helpers;
-use ruff_python_ast::helpers::is_const_none;
+use ruff_python_ast::helpers::{generate_comparison, is_const_none};
 use ruff_python_ast::{self as ast, CmpOp, Constant, Expr};
 use ruff_text_size::Ranged;
 
 use crate::checkers::ast::Checker;
 use crate::codes::Rule;
 use crate::registry::AsRule;
-use crate::rules::pycodestyle::helpers::generate_comparison;
 
 #[derive(Debug, PartialEq, Eq, Copy, Clone)]
 enum EqCmpOp {

crates/ruff_linter/src/rules/pycodestyle/rules/not_tests.rs

@@ -1,12 +1,12 @@
 use crate::fix::edits::pad;
 use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
 use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast::helpers::generate_comparison;
 use ruff_python_ast::{self as ast, CmpOp, Expr};
 use ruff_text_size::Ranged;
 
 use crate::checkers::ast::Checker;
 use crate::registry::{AsRule, Rule};
-use crate::rules::pycodestyle::helpers::generate_comparison;
 
 /// ## What it does
 /// Checks for negative comparison using `not {foo} in {bar}`.

crates/ruff_linter/src/rules/refurb/mod.rs

@@ -22,6 +22,7 @@ mod tests {
     #[test_case(Rule::UnnecessaryEnumerate, Path::new("FURB148.py"))]
     #[test_case(Rule::PrintEmptyString, Path::new("FURB105.py"))]
     #[test_case(Rule::ImplicitCwd, Path::new("FURB177.py"))]
+    #[test_case(Rule::SingleItemMembershipTest, Path::new("FURB171.py"))]
     fn rules(rule_code: Rule, path: &Path) -> Result<()> {
         let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
         let diagnostics = test_path(

crates/ruff_linter/src/rules/refurb/rules/mod.rs

@@ -4,6 +4,7 @@ pub(crate) use implicit_cwd::*;
 pub(crate) use print_empty_string::*;
 pub(crate) use reimplemented_starmap::*;
 pub(crate) use repeated_append::*;
+pub(crate) use single_item_membership_test::*;
 pub(crate) use slice_copy::*;
 pub(crate) use unnecessary_enumerate::*;
 
@@ -13,5 +14,6 @@ mod implicit_cwd;
 mod print_empty_string;
 mod reimplemented_starmap;
 mod repeated_append;
+mod single_item_membership_test;
 mod slice_copy;
 mod unnecessary_enumerate;

crates/ruff_linter/src/rules/refurb/rules/single_item_membership_test.rs

@@ -0,0 +1,132 @@
+use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
+use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast::helpers::generate_comparison;
+use ruff_python_ast::ExprConstant;
+use ruff_python_ast::{CmpOp, Constant, Expr};
+use ruff_text_size::Ranged;
+
+use crate::checkers::ast::Checker;
+use crate::fix::edits::pad;
+use crate::registry::AsRule;
+
+/// ## What it does
+/// Checks for membership tests against single-item containers.
+///
+/// ## Why is this bad?
+/// Performing a membership test against a container (like a `list` or `set`)
+/// with a single item is less readable and less efficient than comparing
+/// against the item directly.
+///
+/// ## Example
+/// ```python
+/// 1 in [1]
+/// ```
+///
+/// Use instead:
+/// ```python
+/// 1 == 1
+/// ```
+///
+/// ## References
+/// - [Python documentation: Comparisons](https://docs.python.org/3/reference/expressions.html#comparisons)
+/// - [Python documentation: Membership test operations](https://docs.python.org/3/reference/expressions.html#membership-test-operations)
+#[violation]
+pub struct SingleItemMembershipTest {
+    membership_test: MembershipTest,
+}
+
+impl Violation for SingleItemMembershipTest {
+    const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
+
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        format!("Membership test against single-item container")
+    }
+
+    fn fix_title(&self) -> Option<String> {
+        let SingleItemMembershipTest { membership_test } = self;
+        match membership_test {
+            MembershipTest::In => Some("Convert to equality test".to_string()),
+            MembershipTest::NotIn => Some("Convert to inequality test".to_string()),
+        }
+    }
+}
+
+/// FURB171
+pub(crate) fn single_item_membership_test(
+    checker: &mut Checker,
+    expr: &Expr,
+    left: &Expr,
+    ops: &[CmpOp],
+    comparators: &[Expr],
+) {
+    let ([op], [right]) = (ops, comparators) else {
+        return;
+    };
+
+    // Ensure that the comparison is a membership test.
+    let membership_test = match op {
+        CmpOp::In => MembershipTest::In,
+        CmpOp::NotIn => MembershipTest::NotIn,
+        _ => return,
+    };
+
+    // Check if the right-hand side is a single-item object.
+    let Some(item) = single_item(right) else {
+        return;
+    };
+
+    let mut diagnostic =
+        Diagnostic::new(SingleItemMembershipTest { membership_test }, expr.range());
+    if checker.patch(diagnostic.kind.rule()) {
+        diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
+            pad(
+                generate_comparison(
+                    left,
+                    &[membership_test.replacement_op()],
+                    &[item.clone()],
+                    expr.into(),
+                    checker.indexer().comment_ranges(),
+                    checker.locator(),
+                ),
+                expr.range(),
+                checker.locator(),
+            ),
+            expr.range(),
+        )));
+    }
+    checker.diagnostics.push(diagnostic);
+}
+
+/// Return the single item wrapped in Some if the expression contains a single
+/// item, otherwise return None.
+fn single_item(expr: &Expr) -> Option<&Expr> {
+    match expr {
+        Expr::List(list) if list.elts.len() == 1 => Some(&list.elts[0]),
+        Expr::Tuple(tuple) if tuple.elts.len() == 1 => Some(&tuple.elts[0]),
+        Expr::Set(set) if set.elts.len() == 1 => Some(&set.elts[0]),
+        string_expr @ Expr::Constant(ExprConstant {
+            value: Constant::Str(string),
+            ..
+        }) if string.chars().count() == 1 => Some(string_expr),
+        _ => None,
+    }
+}
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+enum MembershipTest {
+    /// Ex) `1 in [1]`
+    In,
+    /// Ex) `1 not in [1]`
+    NotIn,
+}
+
+impl MembershipTest {
+    /// Returns the replacement comparison operator for this membership test.
+    fn replacement_op(self) -> CmpOp {
+        match self {
+            MembershipTest::In => CmpOp::Eq,
+            MembershipTest::NotIn => CmpOp::NotEq,
+        }
+    }
+}

crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB171_FURB171.py.snap

@@ -0,0 +1,123 @@
+---
+source: crates/ruff_linter/src/rules/refurb/mod.rs
+---
+FURB171.py:3:4: FURB171 [*] Membership test against single-item container
+  |
+1 | # Errors.
+2 | 
+3 | if 1 in (1,):
+  |    ^^^^^^^^^ FURB171
+4 |     print("Single-element tuple")
+  |
+  = help: Convert to equality test
+
+ℹ Fix
+1 1 | # Errors.
+2 2 | 
+3   |-if 1 in (1,):
+  3 |+if 1 == 1:
+4 4 |     print("Single-element tuple")
+5 5 | 
+6 6 | if 1 in [1]:
+
+FURB171.py:6:4: FURB171 [*] Membership test against single-item container
+  |
+4 |     print("Single-element tuple")
+5 | 
+6 | if 1 in [1]:
+  |    ^^^^^^^^ FURB171
+7 |     print("Single-element list")
+  |
+  = help: Convert to equality test
+
+ℹ Fix
+3 3 | if 1 in (1,):
+4 4 |     print("Single-element tuple")
+5 5 | 
+6   |-if 1 in [1]:
+  6 |+if 1 == 1:
+7 7 |     print("Single-element list")
+8 8 | 
+9 9 | if 1 in {1}:
+
+FURB171.py:9:4: FURB171 [*] Membership test against single-item container
+   |
+ 7 |     print("Single-element list")
+ 8 | 
+ 9 | if 1 in {1}:
+   |    ^^^^^^^^ FURB171
+10 |     print("Single-element set")
+   |
+   = help: Convert to equality test
+
+ℹ Fix
+6  6  | if 1 in [1]:
+7  7  |     print("Single-element list")
+8  8  | 
+9     |-if 1 in {1}:
+   9  |+if 1 == 1:
+10 10 |     print("Single-element set")
+11 11 | 
+12 12 | if "a" in "a":
+
+FURB171.py:12:4: FURB171 [*] Membership test against single-item container
+   |
+10 |     print("Single-element set")
+11 | 
+12 | if "a" in "a":
+   |    ^^^^^^^^^^ FURB171
+13 |     print("Single-element string")
+   |
+   = help: Convert to equality test
+
+ℹ Fix
+9  9  | if 1 in {1}:
+10 10 |     print("Single-element set")
+11 11 | 
+12    |-if "a" in "a":
+   12 |+if "a" == "a":
+13 13 |     print("Single-element string")
+14 14 | 
+15 15 | if 1 not in (1,):
+
+FURB171.py:15:4: FURB171 [*] Membership test against single-item container
+   |
+13 |     print("Single-element string")
+14 | 
+15 | if 1 not in (1,):
+   |    ^^^^^^^^^^^^^ FURB171
+16 |     print("Check `not in` membership test")
+   |
+   = help: Convert to inequality test
+
+ℹ Fix
+12 12 | if "a" in "a":
+13 13 |     print("Single-element string")
+14 14 | 
+15    |-if 1 not in (1,):
+   15 |+if 1 != 1:
+16 16 |     print("Check `not in` membership test")
+17 17 | 
+18 18 | if not 1 in (1,):
+
+FURB171.py:18:8: FURB171 [*] Membership test against single-item container
+   |
+16 |     print("Check `not in` membership test")
+17 | 
+18 | if not 1 in (1,):
+   |        ^^^^^^^^^ FURB171
+19 |     print("Check the negated membership test")
+   |
+   = help: Convert to equality test
+
+ℹ Fix
+15 15 | if 1 not in (1,):
+16 16 |     print("Check `not in` membership test")
+17 17 | 
+18    |-if not 1 in (1,):
+   18 |+if not 1 == 1:
+19 19 |     print("Check the negated membership test")
+20 20 | 
+21 21 | # Non-errors.
+
+

crates/ruff_python_ast/src/helpers.rs

@@ -1,14 +1,19 @@
 use std::borrow::Cow;
 use std::path::Path;
 
+use ruff_python_trivia::CommentRanges;
+use ruff_source_file::Locator;
 use smallvec::SmallVec;
 
 use ruff_text_size::{Ranged, TextRange};
 
 use crate::call_path::CallPath;
+use crate::node::AnyNodeRef;
+use crate::parenthesize::parenthesized_range;
 use crate::statement_visitor::{walk_body, walk_stmt, StatementVisitor};
 use crate::{
-    self as ast, Arguments, Constant, ExceptHandler, Expr, MatchCase, Pattern, Stmt, TypeParam,
+    self as ast, Arguments, CmpOp, Constant, ExceptHandler, Expr, MatchCase, Pattern, Stmt,
+    TypeParam,
 };
 
 /// Return `true` if the `Stmt` is a compound statement (as opposed to a simple statement).
@@ -1129,6 +1134,58 @@ impl Truthiness {
     }
 }
 
+pub fn generate_comparison(
+    left: &Expr,
+    ops: &[CmpOp],
+    comparators: &[Expr],
+    parent: AnyNodeRef,
+    comment_ranges: &CommentRanges,
+    locator: &Locator,
+) -> String {
+    let start = left.start();
+    let end = comparators.last().map_or_else(|| left.end(), Ranged::end);
+    let mut contents = String::with_capacity(usize::from(end - start));
+
+    // Add the left side of the comparison.
+    contents.push_str(
+        locator.slice(
+            parenthesized_range(left.into(), parent, comment_ranges, locator.contents())
+                .unwrap_or(left.range()),
+        ),
+    );
+
+    for (op, comparator) in ops.iter().zip(comparators) {
+        // Add the operator.
+        contents.push_str(match op {
+            CmpOp::Eq => " == ",
+            CmpOp::NotEq => " != ",
+            CmpOp::Lt => " < ",
+            CmpOp::LtE => " <= ",
+            CmpOp::Gt => " > ",
+            CmpOp::GtE => " >= ",
+            CmpOp::In => " in ",
+            CmpOp::NotIn => " not in ",
+            CmpOp::Is => " is ",
+            CmpOp::IsNot => " is not ",
+        });
+
+        // Add the right side of the comparison.
+        contents.push_str(
+            locator.slice(
+                parenthesized_range(
+                    comparator.into(),
+                    parent,
+                    comment_ranges,
+                    locator.contents(),
+                )
+                .unwrap_or(comparator.range()),
+            ),
+        );
+    }
+
+    contents
+}
+
 #[cfg(test)]
 mod tests {
     use std::borrow::Cow;

ruff.schema.json

@@ -2684,6 +2684,7 @@
         "FURB145",
         "FURB148",
         "FURB17",
+        "FURB171",
         "FURB177",
         "G",
         "G0",