Reverse order of arguments for `operator.contains` (#9192)

Closes https://github.com/astral-sh/ruff/issues/9191.

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

@@ -26,24 +26,29 @@ op_gt = lambda x, y: x > y
 op_gte = lambda x, y: x >= y
 op_is = lambda x, y: x is y
 op_isnot = lambda x, y: x is not y
-op_in = lambda x, y: x in y
+op_in = lambda x, y: y in x
+
 
 def op_not2(x):
     return not x
 
+
 def op_add2(x, y):
     return x + y
 
+
 class Adder:
     def add(x, y):
         return x + y
 
-# Ok.
+# OK.
 op_add3 = lambda x, y = 1: x + y
 op_neg2 = lambda x, y: y - x
 op_notin = lambda x, y: y not in x
 op_and = lambda x, y: y and x
 op_or = lambda x, y: y or x
+op_in = lambda x, y: x in y
+
 
 def op_neg3(x, y):
     return y - x

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

@@ -223,23 +223,88 @@ fn cmp_op(expr: &ast::ExprCompare, params: &ast::Parameters) -> Option<&'static
     let [right] = expr.comparators.as_slice() else {
         return None;
     };
-    if !is_same_expression(arg1, &expr.left) || !is_same_expression(arg2, right) {
+
-        return None;
-    }
     match op {
-        ast::CmpOp::Eq => Some("eq"),
+        ast::CmpOp::Eq => {
-        ast::CmpOp::NotEq => Some("ne"),
+            if match_arguments(arg1, arg2, &expr.left, right) {
-        ast::CmpOp::Lt => Some("lt"),
+                Some("eq")
-        ast::CmpOp::LtE => Some("le"),
+            } else {
-        ast::CmpOp::Gt => Some("gt"),
+                None
-        ast::CmpOp::GtE => Some("ge"),
+            }
-        ast::CmpOp::Is => Some("is_"),
+        }
-        ast::CmpOp::IsNot => Some("is_not"),
+        ast::CmpOp::NotEq => {
-        ast::CmpOp::In => Some("contains"),
+            if match_arguments(arg1, arg2, &expr.left, right) {
+                Some("ne")
+            } else {
+                None
+            }
+        }
+        ast::CmpOp::Lt => {
+            if match_arguments(arg1, arg2, &expr.left, right) {
+                Some("lt")
+            } else {
+                None
+            }
+        }
+        ast::CmpOp::LtE => {
+            if match_arguments(arg1, arg2, &expr.left, right) {
+                Some("le")
+            } else {
+                None
+            }
+        }
+        ast::CmpOp::Gt => {
+            if match_arguments(arg1, arg2, &expr.left, right) {
+                Some("gt")
+            } else {
+                None
+            }
+        }
+        ast::CmpOp::GtE => {
+            if match_arguments(arg1, arg2, &expr.left, right) {
+                Some("ge")
+            } else {
+                None
+            }
+        }
+        ast::CmpOp::Is => {
+            if match_arguments(arg1, arg2, &expr.left, right) {
+                Some("is_")
+            } else {
+                None
+            }
+        }
+        ast::CmpOp::IsNot => {
+            if match_arguments(arg1, arg2, &expr.left, right) {
+                Some("is_not")
+            } else {
+                None
+            }
+        }
+        ast::CmpOp::In => {
+            // Note: `operator.contains` reverses the order of arguments. That is:
+            // `operator.contains` is equivalent to `lambda x, y: y in x`, rather than
+            // `lambda x, y: x in y`.
+            if match_arguments(arg1, arg2, right, &expr.left) {
+                Some("contains")
+            } else {
+                None
+            }
+        }
         ast::CmpOp::NotIn => None,
     }
 }
 
+/// Returns `true` if the given arguments match the expected operands.
+fn match_arguments(
+    arg1: &ast::ParameterWithDefault,
+    arg2: &ast::ParameterWithDefault,
+    operand1: &Expr,
+    operand2: &Expr,
+) -> bool {
+    is_same_expression(arg1, operand1) && is_same_expression(arg2, operand2)
+}
+
 /// Returns `true` if the given argument is the "same" as the given expression. For example, if
 /// the argument has a default, it is not considered the same as any expression; if both match the
 /// same name, they are considered the same.

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

@@ -590,7 +590,7 @@ FURB118.py:26:10: FURB118 [*] Use `operator.ge` instead of defining a lambda
    27 |+op_gte = operator.ge
 27 28 | op_is = lambda x, y: x is y
 28 29 | op_isnot = lambda x, y: x is not y
-29 30 | op_in = lambda x, y: x in y
+29 30 | op_in = lambda x, y: y in x
 
 FURB118.py:27:9: FURB118 [*] Use `operator.is_` instead of defining a lambda
    |
@@ -599,7 +599,7 @@ FURB118.py:27:9: FURB118 [*] Use `operator.is_` instead of defining a lambda
 27 | op_is = lambda x, y: x is y
    |         ^^^^^^^^^^^^^^^^^^^ FURB118
 28 | op_isnot = lambda x, y: x is not y
-29 | op_in = lambda x, y: x in y
+29 | op_in = lambda x, y: y in x
    |
    = help: Replace with `operator.is_`
 
@@ -616,7 +616,7 @@ FURB118.py:27:9: FURB118 [*] Use `operator.is_` instead of defining a lambda
 27    |-op_is = lambda x, y: x is y
    28 |+op_is = operator.is_
 28 29 | op_isnot = lambda x, y: x is not y
-29 30 | op_in = lambda x, y: x in y
+29 30 | op_in = lambda x, y: y in x
 30 31 | 
 
 FURB118.py:28:12: FURB118 [*] Use `operator.is_not` instead of defining a lambda
@@ -625,7 +625,7 @@ FURB118.py:28:12: FURB118 [*] Use `operator.is_not` instead of defining a lambda
 27 | op_is = lambda x, y: x is y
 28 | op_isnot = lambda x, y: x is not y
    |            ^^^^^^^^^^^^^^^^^^^^^^^ FURB118
-29 | op_in = lambda x, y: x in y
+29 | op_in = lambda x, y: y in x
    |
    = help: Replace with `operator.is_not`
 
@@ -641,18 +641,16 @@ FURB118.py:28:12: FURB118 [*] Use `operator.is_not` instead of defining a lambda
 27 28 | op_is = lambda x, y: x is y
 28    |-op_isnot = lambda x, y: x is not y
    29 |+op_isnot = operator.is_not
-29 30 | op_in = lambda x, y: x in y
+29 30 | op_in = lambda x, y: y in x
 30 31 | 
-31 32 | def op_not2(x):
+31 32 | 
 
 FURB118.py:29:9: FURB118 [*] Use `operator.contains` instead of defining a lambda
    |
 27 | op_is = lambda x, y: x is y
 28 | op_isnot = lambda x, y: x is not y
-29 | op_in = lambda x, y: x in y
+29 | op_in = lambda x, y: y in x
    |         ^^^^^^^^^^^^^^^^^^^ FURB118
-30 | 
-31 | def op_not2(x):
    |
    = help: Replace with `operator.contains`
 
@@ -666,45 +664,37 @@ FURB118.py:29:9: FURB118 [*] Use `operator.contains` instead of defining a lambd
 26 27 | op_gte = lambda x, y: x >= y
 27 28 | op_is = lambda x, y: x is y
 28 29 | op_isnot = lambda x, y: x is not y
-29    |-op_in = lambda x, y: x in y
+29    |-op_in = lambda x, y: y in x
    30 |+op_in = operator.contains
 30 31 | 
-31 32 | def op_not2(x):
+31 32 | 
-32 33 |     return not x
+32 33 | def op_not2(x):
 
-FURB118.py:31:1: FURB118 Use `operator.not_` instead of defining a function
+FURB118.py:32:1: FURB118 Use `operator.not_` instead of defining a function
    |
-29 |   op_in = lambda x, y: x in y
+32 | / def op_not2(x):
-30 |   
+33 | |     return not x
-31 | / def op_not2(x):
-32 | |     return not x
    | |________________^ FURB118
-33 |   
-34 |   def op_add2(x, y):
    |
    = help: Replace with `operator.not_`
 
-FURB118.py:34:1: FURB118 Use `operator.add` instead of defining a function
+FURB118.py:36:1: FURB118 Use `operator.add` instead of defining a function
    |
-32 |       return not x
+36 | / def op_add2(x, y):
-33 |   
+37 | |     return x + y
-34 | / def op_add2(x, y):
-35 | |     return x + y
    | |________________^ FURB118
-36 |   
-37 |   class Adder:
    |
    = help: Replace with `operator.add`
 
-FURB118.py:38:5: FURB118 Use `operator.add` instead of defining a function
+FURB118.py:41:5: FURB118 Use `operator.add` instead of defining a function
    |
-37 |   class Adder:
+40 |   class Adder:
-38 |       def add(x, y):
+41 |       def add(x, y):
    |  _____^
-39 | |         return x + y
+42 | |         return x + y
    | |____________________^ FURB118
-40 |   
+43 |   
-41 |   # Ok.
+44 |   # OK.
    |
    = help: Replace with `operator.add`