bpo-37648: Fixed minor inconsistency in some __contains__. (GH-14904)

The collection's item is now always at the left and
the needle is on the right of ==.

12 files changed, 86 insertions, 22 deletions

diff --git a/Doc/library/test.rst b/Doc/library/test.rst
index da6a85d..6eef5c6 100644
--- a/Doc/library/test.rst
+++ b/Doc/library/test.rst
@@ -367,6 +367,12 @@ The :mod:`test.support` module defines the following constants:
    Object that is equal to anything.  Used to test mixed type comparison.
 
 
+.. data:: NEVER_EQ
+
+   Object that is not equal to anything (even to :data:`ALWAYS_EQ`).
+   Used to test mixed type comparison.
+
+
 .. data:: LARGEST
 
    Object that is greater than anything (except itself).
diff --git a/Lib/test/list_tests.py b/Lib/test/list_tests.py
index 40316de..44bc2ae 100644
--- a/Lib/test/list_tests.py
+++ b/Lib/test/list_tests.py
@@ -7,6 +7,7 @@ import os
 from functools import cmp_to_key
 
 from test import support, seq_tests
+from test.support import ALWAYS_EQ, NEVER_EQ
 
 
 class CommonTest(seq_tests.CommonTest):
@@ -329,6 +330,20 @@ class CommonTest(seq_tests.CommonTest):
 
         self.assertRaises(TypeError, a.remove)
 
+        a = self.type2test([1, 2])
+        self.assertRaises(ValueError, a.remove, NEVER_EQ)
+        self.assertEqual(a, [1, 2])
+        a.remove(ALWAYS_EQ)
+        self.assertEqual(a, [2])
+        a = self.type2test([ALWAYS_EQ])
+        a.remove(1)
+        self.assertEqual(a, [])
+        a = self.type2test([ALWAYS_EQ])
+        a.remove(NEVER_EQ)
+        self.assertEqual(a, [])
+        a = self.type2test([NEVER_EQ])
+        self.assertRaises(ValueError, a.remove, ALWAYS_EQ)
+
         class BadExc(Exception):
             pass
 
diff --git a/Lib/test/seq_tests.py b/Lib/test/seq_tests.py
index 65b110e..1d9ad58 100644
--- a/Lib/test/seq_tests.py
+++ b/Lib/test/seq_tests.py
@@ -6,6 +6,7 @@ import unittest
 import sys
 import pickle
 from test import support
+from test.support import ALWAYS_EQ, NEVER_EQ
 
 # Various iterables
 # This is used for checking the constructor (here and in test_deque.py)
@@ -221,15 +222,15 @@ class CommonTest(unittest.TestCase):
         self.assertRaises(TypeError, u.__contains__)
 
     def test_contains_fake(self):
-        class AllEq:
-            # Sequences must use rich comparison against each item
-            # (unless "is" is true, or an earlier item answered)
-            # So instances of AllEq must be found in all non-empty sequences.
-            def __eq__(self, other):
-                return True
-            __hash__ = None # Can't meet hash invariant requirements
-        self.assertNotIn(AllEq(), self.type2test([]))
-        self.assertIn(AllEq(), self.type2test([1]))
+        # Sequences must use rich comparison against each item
+        # (unless "is" is true, or an earlier item answered)
+        # So ALWAYS_EQ must be found in all non-empty sequences.
+        self.assertNotIn(ALWAYS_EQ, self.type2test([]))
+        self.assertIn(ALWAYS_EQ, self.type2test([1]))
+        self.assertIn(1, self.type2test([ALWAYS_EQ]))
+        self.assertNotIn(NEVER_EQ, self.type2test([]))
+        self.assertNotIn(ALWAYS_EQ, self.type2test([NEVER_EQ]))
+        self.assertIn(NEVER_EQ, self.type2test([ALWAYS_EQ]))
 
     def test_contains_order(self):
         # Sequences must test in-order.  If a rich comparison has side
@@ -350,6 +351,11 @@ class CommonTest(unittest.TestCase):
         self.assertEqual(a.count(1), 3)
         self.assertEqual(a.count(3), 0)
 
+        self.assertEqual(a.count(ALWAYS_EQ), 9)
+        self.assertEqual(self.type2test([ALWAYS_EQ, ALWAYS_EQ]).count(1), 2)
+        self.assertEqual(self.type2test([ALWAYS_EQ, ALWAYS_EQ]).count(NEVER_EQ), 2)
+        self.assertEqual(self.type2test([NEVER_EQ, NEVER_EQ]).count(ALWAYS_EQ), 0)
+
         self.assertRaises(TypeError, a.count)
 
         class BadExc(Exception):
@@ -378,6 +384,11 @@ class CommonTest(unittest.TestCase):
         self.assertEqual(u.index(0, 3, 4), 3)
         self.assertRaises(ValueError, u.index, 2, 0, -10)
 
+        self.assertEqual(u.index(ALWAYS_EQ), 0)
+        self.assertEqual(self.type2test([ALWAYS_EQ, ALWAYS_EQ]).index(1), 0)
+        self.assertEqual(self.type2test([ALWAYS_EQ, ALWAYS_EQ]).index(NEVER_EQ), 0)
+        self.assertRaises(ValueError, self.type2test([NEVER_EQ, NEVER_EQ]).index, ALWAYS_EQ)
+
         self.assertRaises(TypeError, u.index)
 
         class BadExc(Exception):
diff --git a/Lib/test/support/__init__.py b/Lib/test/support/__init__.py
index c82037e..d34f2ef 100644
--- a/Lib/test/support/__init__.py
+++ b/Lib/test/support/__init__.py
@@ -113,7 +113,7 @@ __all__ = [
     "run_with_locale", "swap_item",
     "swap_attr", "Matcher", "set_memlimit", "SuppressCrashReport", "sortdict",
     "run_with_tz", "PGO", "missing_compiler_executable", "fd_count",
-    "ALWAYS_EQ", "LARGEST", "SMALLEST"
+    "ALWAYS_EQ", "NEVER_EQ", "LARGEST", "SMALLEST"
     ]
 
 class Error(Exception):
@@ -3115,6 +3115,17 @@ class _ALWAYS_EQ:
 
 ALWAYS_EQ = _ALWAYS_EQ()
 
+class _NEVER_EQ:
+    """
+    Object that is not equal to anything.
+    """
+    def __eq__(self, other):
+        return False
+    def __ne__(self, other):
+        return True
+
+NEVER_EQ = _NEVER_EQ()
+
 @functools.total_ordering
 class _LARGEST:
     """
diff --git a/Lib/test/test_iter.py b/Lib/test/test_iter.py
index 542b284..6aceda2 100644
--- a/Lib/test/test_iter.py
+++ b/Lib/test/test_iter.py
@@ -3,7 +3,7 @@
 import sys
 import unittest
 from test.support import run_unittest, TESTFN, unlink, cpython_only
-from test.support import check_free_after_iterating
+from test.support import check_free_after_iterating, ALWAYS_EQ, NEVER_EQ
 import pickle
 import collections.abc
 
@@ -41,6 +41,14 @@ class IteratingSequenceClass:
     def __iter__(self):
         return BasicIterClass(self.n)
 
+class IteratorProxyClass:
+    def __init__(self, i):
+        self.i = i
+    def __next__(self):
+        return next(self.i)
+    def __iter__(self):
+        return self
+
 class SequenceClass:
     def __init__(self, n):
         self.n = n
@@ -50,6 +58,12 @@ class SequenceClass:
         else:
             raise IndexError
 
+class SequenceProxyClass:
+    def __init__(self, s):
+        self.s = s
+    def __getitem__(self, i):
+        return self.s[i]
+
 class UnlimitedSequenceClass:
     def __getitem__(self, i):
         return i
@@ -635,6 +649,13 @@ class TestCase(unittest.TestCase):
             for i in "abc", -1, 5, 42.42, (3, 4), [], {1: 1}, 3-12j, sc5:
                 self.assertNotIn(i, sc5)
 
+        self.assertIn(ALWAYS_EQ, IteratorProxyClass(iter([1])))
+        self.assertIn(ALWAYS_EQ, SequenceProxyClass([1]))
+        self.assertNotIn(ALWAYS_EQ, IteratorProxyClass(iter([NEVER_EQ])))
+        self.assertNotIn(ALWAYS_EQ, SequenceProxyClass([NEVER_EQ]))
+        self.assertIn(NEVER_EQ, IteratorProxyClass(iter([ALWAYS_EQ])))
+        self.assertIn(NEVER_EQ, SequenceProxyClass([ALWAYS_EQ]))
+
         self.assertRaises(TypeError, lambda: 3 in 12)
         self.assertRaises(TypeError, lambda: 3 not in map)
 
diff --git a/Misc/NEWS.d/next/Core and Builtins/2019-07-22-11-05-05.bpo-37648.6TY2L-.rst b/Misc/NEWS.d/next/Core and Builtins/2019-07-22-11-05-05.bpo-37648.6TY2L-.rst
new file mode 100644
index 0000000..3c11d3d
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2019-07-22-11-05-05.bpo-37648.6TY2L-.rst
@@ -0,0 +1,3 @@
+Fixed minor inconsistency in :meth:`list.__contains__`,
+:meth:`tuple.__contains__` and a few other places. The collection's item is
+now always at the left and the needle is on the right of ``==``.
diff --git a/Modules/_asynciomodule.c b/Modules/_asynciomodule.c
index e9e6c56..4d503a4 100644
--- a/Modules/_asynciomodule.c
+++ b/Modules/_asynciomodule.c
@@ -937,7 +937,7 @@ _asyncio_Future_remove_done_callback(FutureObj *self, PyObject *fn)
     ENSURE_FUTURE_ALIVE(self)
 
     if (self->fut_callback0 != NULL) {
-        int cmp = PyObject_RichCompareBool(fn, self->fut_callback0, Py_EQ);
+        int cmp = PyObject_RichCompareBool(self->fut_callback0, fn, Py_EQ);
         if (cmp == -1) {
             return NULL;
         }
@@ -962,7 +962,7 @@ _asyncio_Future_remove_done_callback(FutureObj *self, PyObject *fn)
     if (len == 1) {
         PyObject *cb_tup = PyList_GET_ITEM(self->fut_callbacks, 0);
         int cmp = PyObject_RichCompareBool(
-            fn, PyTuple_GET_ITEM(cb_tup, 0), Py_EQ);
+            PyTuple_GET_ITEM(cb_tup, 0), fn, Py_EQ);
         if (cmp == -1) {
             return NULL;
         }
@@ -984,7 +984,7 @@ _asyncio_Future_remove_done_callback(FutureObj *self, PyObject *fn)
         int ret;
         PyObject *item = PyList_GET_ITEM(self->fut_callbacks, i);
         Py_INCREF(item);
-        ret = PyObject_RichCompareBool(fn, PyTuple_GET_ITEM(item, 0), Py_EQ);
+        ret = PyObject_RichCompareBool(PyTuple_GET_ITEM(item, 0), fn, Py_EQ);
         if (ret == 0) {
             if (j < len) {
                 PyList_SET_ITEM(newlist, j, item);
diff --git a/Modules/_ssl.c b/Modules/_ssl.c
index da30cbb..3d54b84 100644
--- a/Modules/_ssl.c
+++ b/Modules/_ssl.c
@@ -5600,8 +5600,7 @@ list_contains(PyListObject *a, PyObject *el)
     int cmp;
 
     for (i = 0, cmp = 0 ; cmp == 0 && i < Py_SIZE(a); ++i)
-        cmp = PyObject_RichCompareBool(el, PyList_GET_ITEM(a, i),
-                                           Py_EQ);
+        cmp = PyObject_RichCompareBool(PyList_GET_ITEM(a, i), el, Py_EQ);
     return cmp;
 }
 
diff --git a/Objects/abstract.c b/Objects/abstract.c
index db1c306..f93d73f 100644
--- a/Objects/abstract.c
+++ b/Objects/abstract.c
@@ -2016,7 +2016,7 @@ _PySequence_IterSearch(PyObject *seq, PyObject *obj, int operation)
             break;
         }
 
-        cmp = PyObject_RichCompareBool(obj, item, Py_EQ);
+        cmp = PyObject_RichCompareBool(item, obj, Py_EQ);
         Py_DECREF(item);
         if (cmp < 0)
             goto Fail;
diff --git a/Objects/dictobject.c b/Objects/dictobject.c
index b6205d9..f168ad5 100644
--- a/Objects/dictobject.c
+++ b/Objects/dictobject.c
@@ -4392,7 +4392,7 @@ dictitems_contains(_PyDictViewObject *dv, PyObject *obj)
         return 0;
     }
     Py_INCREF(found);
-    result = PyObject_RichCompareBool(value, found, Py_EQ);
+    result = PyObject_RichCompareBool(found, value, Py_EQ);
     Py_DECREF(found);
     return result;
 }
diff --git a/Objects/listobject.c b/Objects/listobject.c
index d012ab9..cea9b24 100644
--- a/Objects/listobject.c
+++ b/Objects/listobject.c
@@ -449,8 +449,7 @@ list_contains(PyListObject *a, PyObject *el)
     int cmp;
 
     for (i = 0, cmp = 0 ; cmp == 0 && i < Py_SIZE(a); ++i)
-        cmp = PyObject_RichCompareBool(el, PyList_GET_ITEM(a, i),
-                                           Py_EQ);
+        cmp = PyObject_RichCompareBool(PyList_GET_ITEM(a, i), el, Py_EQ);
     return cmp;
 }
 
diff --git a/Objects/tupleobject.c b/Objects/tupleobject.c
index fc2d274..aeaf845 100644
--- a/Objects/tupleobject.c
+++ b/Objects/tupleobject.c
@@ -403,8 +403,7 @@ tuplecontains(PyTupleObject *a, PyObject *el)
     int cmp;
 
     for (i = 0, cmp = 0 ; cmp == 0 && i < Py_SIZE(a); ++i)
-        cmp = PyObject_RichCompareBool(el, PyTuple_GET_ITEM(a, i),
-                                           Py_EQ);
+        cmp = PyObject_RichCompareBool(PyTuple_GET_ITEM(a, i), el, Py_EQ);
     return cmp;
 }