[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,
+        }
+    }
+}