bpo-42195: Ensure consistency of Callable's __args__ in collections.abc and typing (GH-23060)

9 files changed, 212 insertions, 63 deletions

diff --git a/Lib/_collections_abc.py b/Lib/_collections_abc.py
index 28690f8..7c3faa6 100644
--- a/Lib/_collections_abc.py
+++ b/Lib/_collections_abc.py
@@ -10,6 +10,10 @@ from abc import ABCMeta, abstractmethod
 import sys
 
 GenericAlias = type(list[int])
+EllipsisType = type(...)
+def _f(): pass
+FunctionType = type(_f)
+del _f
 
 __all__ = ["Awaitable", "Coroutine",
            "AsyncIterable", "AsyncIterator", "AsyncGenerator",
@@ -409,6 +413,69 @@ class Collection(Sized, Iterable, Container):
         return NotImplemented
 
 
+class _CallableGenericAlias(GenericAlias):
+    """ Represent `Callable[argtypes, resulttype]`.
+
+    This sets ``__args__`` to a tuple containing the flattened``argtypes``
+    followed by ``resulttype``.
+
+    Example: ``Callable[[int, str], float]`` sets ``__args__`` to
+    ``(int, str, float)``.
+    """
+
+    __slots__ = ()
+
+    def __new__(cls, origin, args):
+        return cls.__create_ga(origin, args)
+
+    @classmethod
+    def __create_ga(cls, origin, args):
+        if not isinstance(args, tuple) or len(args) != 2:
+            raise TypeError(
+                "Callable must be used as Callable[[arg, ...], result].")
+        t_args, t_result = args
+        if isinstance(t_args, list):
+            ga_args = tuple(t_args) + (t_result,)
+        # This relaxes what t_args can be on purpose to allow things like
+        # PEP 612 ParamSpec.  Responsibility for whether a user is using
+        # Callable[...] properly is deferred to static type checkers.
+        else:
+            ga_args = args
+        return super().__new__(cls, origin, ga_args)
+
+    def __repr__(self):
+        if len(self.__args__) == 2 and self.__args__[0] is Ellipsis:
+            return super().__repr__()
+        return (f'collections.abc.Callable'
+                f'[[{", ".join([_type_repr(a) for a in self.__args__[:-1]])}], '
+                f'{_type_repr(self.__args__[-1])}]')
+
+    def __reduce__(self):
+        args = self.__args__
+        if not (len(args) == 2 and args[0] is Ellipsis):
+            args = list(args[:-1]), args[-1]
+        return _CallableGenericAlias, (Callable, args)
+
+
+def _type_repr(obj):
+    """Return the repr() of an object, special-casing types (internal helper).
+
+    Copied from :mod:`typing` since collections.abc
+    shouldn't depend on that module.
+    """
+    if isinstance(obj, GenericAlias):
+        return repr(obj)
+    if isinstance(obj, type):
+        if obj.__module__ == 'builtins':
+            return obj.__qualname__
+        return f'{obj.__module__}.{obj.__qualname__}'
+    if obj is Ellipsis:
+        return '...'
+    if isinstance(obj, FunctionType):
+        return obj.__name__
+    return repr(obj)
+
+
 class Callable(metaclass=ABCMeta):
 
     __slots__ = ()
@@ -423,7 +490,7 @@ class Callable(metaclass=ABCMeta):
             return _check_methods(C, "__call__")
         return NotImplemented
 
-    __class_getitem__ = classmethod(GenericAlias)
+    __class_getitem__ = classmethod(_CallableGenericAlias)
 
 
 ### SETS ###
diff --git a/Lib/collections/abc.py b/Lib/collections/abc.py
index 891600d..86ca8b8 100644
--- a/Lib/collections/abc.py
+++ b/Lib/collections/abc.py
@@ -1,2 +1,3 @@
 from _collections_abc import *
 from _collections_abc import __all__
+from _collections_abc import _CallableGenericAlias
diff --git a/Lib/test/test_genericalias.py b/Lib/test/test_genericalias.py
index c113e53..5de13fe 100644
--- a/Lib/test/test_genericalias.py
+++ b/Lib/test/test_genericalias.py
@@ -62,7 +62,6 @@ class BaseTest(unittest.TestCase):
                      Iterable, Iterator,
                      Reversible,
                      Container, Collection,
-                     Callable,
                      Mailbox, _PartialFile,
                      ContextVar, Token,
                      Field,
@@ -307,6 +306,63 @@ class BaseTest(unittest.TestCase):
         with self.assertRaises(TypeError):
             GenericAlias(bad=float)
 
+    def test_subclassing_types_genericalias(self):
+        class SubClass(GenericAlias): ...
+        alias = SubClass(list, int)
+        class Bad(GenericAlias):
+            def __new__(cls, *args, **kwargs):
+                super().__new__(cls, *args, **kwargs)
+
+        self.assertEqual(alias, list[int])
+        with self.assertRaises(TypeError):
+            Bad(list, int, bad=int)
+
+    def test_abc_callable(self):
+        # A separate test is needed for Callable since it uses a subclass of
+        # GenericAlias.
+        alias = Callable[[int, str], float]
+        with self.subTest("Testing subscription"):
+            self.assertIs(alias.__origin__, Callable)
+            self.assertEqual(alias.__args__, (int, str, float))
+            self.assertEqual(alias.__parameters__, ())
+
+        with self.subTest("Testing instance checks"):
+            self.assertIsInstance(alias, GenericAlias)
+
+        with self.subTest("Testing weakref"):
+            self.assertEqual(ref(alias)(), alias)
+
+        with self.subTest("Testing pickling"):
+            s = pickle.dumps(alias)
+            loaded = pickle.loads(s)
+            self.assertEqual(alias.__origin__, loaded.__origin__)
+            self.assertEqual(alias.__args__, loaded.__args__)
+            self.assertEqual(alias.__parameters__, loaded.__parameters__)
+
+        with self.subTest("Testing TypeVar substitution"):
+            C1 = Callable[[int, T], T]
+            C2 = Callable[[K, T], V]
+            C3 = Callable[..., T]
+            self.assertEqual(C1[str], Callable[[int, str], str])
+            self.assertEqual(C2[int, float, str], Callable[[int, float], str])
+            self.assertEqual(C3[int], Callable[..., int])
+
+        with self.subTest("Testing type erasure"):
+            class C1(Callable):
+                def __call__(self):
+                    return None
+            a = C1[[int], T]
+            self.assertIs(a().__class__, C1)
+            self.assertEqual(a().__orig_class__, C1[[int], T])
+
+        # bpo-42195
+        with self.subTest("Testing collections.abc.Callable's consistency "
+                          "with typing.Callable"):
+            c1 = typing.Callable[[int, str], dict]
+            c2 = Callable[[int, str], dict]
+            self.assertEqual(c1.__args__, c2.__args__)
+            self.assertEqual(hash(c1.__args__), hash(c2.__args__))
+
 
 if __name__ == "__main__":
     unittest.main()
diff --git a/Lib/test/test_types.py b/Lib/test/test_types.py
index 3058a02..83196ad 100644
--- a/Lib/test/test_types.py
+++ b/Lib/test/test_types.py
@@ -717,14 +717,16 @@ class TypesTests(unittest.TestCase):
         a = list[int]
         b = list[str]
         c = dict[float, str]
+        class SubClass(types.GenericAlias): ...
+        d = SubClass(list, float)
         # equivalence with typing.Union
-        self.assertEqual(a | b | c, typing.Union[a, b, c])
+        self.assertEqual(a | b | c | d, typing.Union[a, b, c, d])
         # de-duplicate
-        self.assertEqual(a | c | b | b | a | c, a | b | c)
+        self.assertEqual(a | c | b | b | a | c | d | d, a | b | c | d)
         # order shouldn't matter
-        self.assertEqual(a | b, b | a)
-        self.assertEqual(repr(a | b | c),
-                         "list[int] | list[str] | dict[float, str]")
+        self.assertEqual(a | b | d, b | a | d)
+        self.assertEqual(repr(a | b | c | d),
+                         "list[int] | list[str] | dict[float, str] | list[float]")
 
         class BadType(type):
             def __eq__(self, other):
diff --git a/Lib/test/test_typing.py b/Lib/test/test_typing.py
index f3e38b6..8e86e76 100644
--- a/Lib/test/test_typing.py
+++ b/Lib/test/test_typing.py
@@ -447,14 +447,6 @@ class CallableTests(BaseTestCase):
 
     def test_callable_wrong_forms(self):
         with self.assertRaises(TypeError):
-            Callable[[...], int]
-        with self.assertRaises(TypeError):
-            Callable[(), int]
-        with self.assertRaises(TypeError):
-            Callable[[()], int]
-        with self.assertRaises(TypeError):
-            Callable[[int, 1], 2]
-        with self.assertRaises(TypeError):
             Callable[int]
 
     def test_callable_instance_works(self):
@@ -1807,10 +1799,9 @@ class GenericTests(BaseTestCase):
     def test_extended_generic_rules_subclassing(self):
         class T1(Tuple[T, KT]): ...
         class T2(Tuple[T, ...]): ...
-        class C1(Callable[[T], T]): ...
-        class C2(Callable[..., int]):
-            def __call__(self):
-                return None
+        class C1(typing.Container[T]):
+            def __contains__(self, item):
+                return False
 
         self.assertEqual(T1.__parameters__, (T, KT))
         self.assertEqual(T1[int, str].__args__, (int, str))
@@ -1824,10 +1815,9 @@ class GenericTests(BaseTestCase):
         ##     T2[int, str]
 
         self.assertEqual(repr(C1[int]).split('.')[-1], 'C1[int]')
-        self.assertEqual(C2.__parameters__, ())
-        self.assertIsInstance(C2(), collections.abc.Callable)
-        self.assertIsSubclass(C2, collections.abc.Callable)
-        self.assertIsSubclass(C1, collections.abc.Callable)
+        self.assertEqual(C1.__parameters__, (T,))
+        self.assertIsInstance(C1(), collections.abc.Container)
+        self.assertIsSubclass(C1, collections.abc.Container)
         self.assertIsInstance(T1(), tuple)
         self.assertIsSubclass(T2, tuple)
         with self.assertRaises(TypeError):
@@ -1861,10 +1851,6 @@ class GenericTests(BaseTestCase):
         class MyTup(Tuple[T, T]): ...
         self.assertIs(MyTup[int]().__class__, MyTup)
         self.assertEqual(MyTup[int]().__orig_class__, MyTup[int])
-        class MyCall(Callable[..., T]):
-            def __call__(self): return None
-        self.assertIs(MyCall[T]().__class__, MyCall)
-        self.assertEqual(MyCall[T]().__orig_class__, MyCall[T])
         class MyDict(typing.Dict[T, T]): ...
         self.assertIs(MyDict[int]().__class__, MyDict)
         self.assertEqual(MyDict[int]().__orig_class__, MyDict[int])
diff --git a/Lib/typing.py b/Lib/typing.py
index 148a505..7f07321 100644
--- a/Lib/typing.py
+++ b/Lib/typing.py
@@ -120,6 +120,16 @@ __all__ = [
 # namespace, but excluded from __all__ because they might stomp on
 # legitimate imports of those modules.
 
+
+def _type_convert(arg):
+    """For converting None to type(None), and strings to ForwardRef."""
+    if arg is None:
+        return type(None)
+    if isinstance(arg, str):
+        return ForwardRef(arg)
+    return arg
+
+
 def _type_check(arg, msg, is_argument=True):
     """Check that the argument is a type, and return it (internal helper).
 
@@ -136,10 +146,7 @@ def _type_check(arg, msg, is_argument=True):
     if is_argument:
         invalid_generic_forms = invalid_generic_forms + (ClassVar, Final)
 
-    if arg is None:
-        return type(None)
-    if isinstance(arg, str):
-        return ForwardRef(arg)
+    arg = _type_convert(arg)
     if (isinstance(arg, _GenericAlias) and
             arg.__origin__ in invalid_generic_forms):
         raise TypeError(f"{arg} is not valid as type argument")
@@ -900,13 +907,13 @@ class _CallableType(_SpecialGenericAlias, _root=True):
             raise TypeError("Callable must be used as "
                             "Callable[[arg, ...], result].")
         args, result = params
-        if args is Ellipsis:
-            params = (Ellipsis, result)
-        else:
-            if not isinstance(args, list):
-                raise TypeError(f"Callable[args, result]: args must be a list."
-                                f" Got {args}")
+        # This relaxes what args can be on purpose to allow things like
+        # PEP 612 ParamSpec.  Responsibility for whether a user is using
+        # Callable[...] properly is deferred to static type checkers.
+        if isinstance(args, list):
             params = (tuple(args), result)
+        else:
+            params = (args, result)
         return self.__getitem_inner__(params)
 
     @_tp_cache
@@ -916,8 +923,9 @@ class _CallableType(_SpecialGenericAlias, _root=True):
         result = _type_check(result, msg)
         if args is Ellipsis:
             return self.copy_with((_TypingEllipsis, result))
-        msg = "Callable[[arg, ...], result]: each arg must be a type."
-        args = tuple(_type_check(arg, msg) for arg in args)
+        if not isinstance(args, tuple):
+            args = (args,)
+        args = tuple(_type_convert(arg) for arg in args)
         params = args + (result,)
         return self.copy_with(params)
 
diff --git a/Misc/NEWS.d/next/Core and Builtins/2020-11-20-00-57-47.bpo-42195.HeqcpS.rst b/Misc/NEWS.d/next/Core and Builtins/2020-11-20-00-57-47.bpo-42195.HeqcpS.rst
new file mode 100644
index 0000000..ac52a00
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2020-11-20-00-57-47.bpo-42195.HeqcpS.rst
@@ -0,0 +1,11 @@
+The ``__args__`` of the parameterized generics for :data:`typing.Callable`
+and :class:`collections.abc.Callable` are now consistent.  The ``__args__`` 
+for :class:`collections.abc.Callable` are now flattened while 
+:data:`typing.Callable`'s have not changed.  To allow this change, 
+:class:`types.GenericAlias` can now be subclassed and 
+``collections.abc.Callable``'s ``__class_getitem__`` will now return a subclass
+of ``types.GenericAlias``.  Tests for typing were also updated to not subclass 
+things like ``Callable[..., T]`` as that is not a valid base class.  Finally,
+both ``Callable``s no longer validate their ``argtypes``, in 
+``Callable[[argtypes], resulttype]`` to prepare for :pep:`612`.  Patch by Ken Jin.  
+
diff --git a/Objects/genericaliasobject.c b/Objects/genericaliasobject.c
index 51a1237..756a7ce 100644
--- a/Objects/genericaliasobject.c
+++ b/Objects/genericaliasobject.c
@@ -429,8 +429,8 @@ ga_getattro(PyObject *self, PyObject *name)
 static PyObject *
 ga_richcompare(PyObject *a, PyObject *b, int op)
 {
-    if (!Py_IS_TYPE(a, &Py_GenericAliasType) ||
-        !Py_IS_TYPE(b, &Py_GenericAliasType) ||
+    if (!PyObject_TypeCheck(a, &Py_GenericAliasType) ||
+        !PyObject_TypeCheck(b, &Py_GenericAliasType) ||
         (op != Py_EQ && op != Py_NE))
     {
         Py_RETURN_NOTIMPLEMENTED;
@@ -564,6 +564,29 @@ static PyGetSetDef ga_properties[] = {
     {0}
 };
 
+/* A helper function to create GenericAlias' args tuple and set its attributes.
+ * Returns 1 on success, 0 on failure. 
+ */
+static inline int
+setup_ga(gaobject *alias, PyObject *origin, PyObject *args) {
+    if (!PyTuple_Check(args)) {
+        args = PyTuple_Pack(1, args);
+        if (args == NULL) {
+            return 0;
+        }
+    }
+    else {
+        Py_INCREF(args);
+    }
+
+    Py_INCREF(origin);
+    alias->origin = origin;
+    alias->args = args;
+    alias->parameters = NULL;
+    alias->weakreflist = NULL;
+    return 1;
+}
+
 static PyObject *
 ga_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
 {
@@ -575,7 +598,15 @@ ga_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
     }
     PyObject *origin = PyTuple_GET_ITEM(args, 0);
     PyObject *arguments = PyTuple_GET_ITEM(args, 1);
-    return Py_GenericAlias(origin, arguments);
+    gaobject *self = (gaobject *)type->tp_alloc(type, 0);
+    if (self == NULL) {
+        return NULL;
+    }
+    if (!setup_ga(self, origin, arguments)) {
+        type->tp_free((PyObject *)self);
+        return NULL;
+    }
+    return (PyObject *)self;
 }
 
 static PyNumberMethods ga_as_number = {
@@ -600,7 +631,7 @@ PyTypeObject Py_GenericAliasType = {
     .tp_hash = ga_hash,
     .tp_call = ga_call,
     .tp_getattro = ga_getattro,
-    .tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC,
+    .tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_BASETYPE,
     .tp_traverse = ga_traverse,
     .tp_richcompare = ga_richcompare,
     .tp_weaklistoffset = offsetof(gaobject, weakreflist),
@@ -615,27 +646,14 @@ PyTypeObject Py_GenericAliasType = {
 PyObject *
 Py_GenericAlias(PyObject *origin, PyObject *args)
 {
-    if (!PyTuple_Check(args)) {
-        args = PyTuple_Pack(1, args);
-        if (args == NULL) {
-            return NULL;
-        }
-    }
-    else {
-        Py_INCREF(args);
-    }
-
     gaobject *alias = PyObject_GC_New(gaobject, &Py_GenericAliasType);
     if (alias == NULL) {
-        Py_DECREF(args);
         return NULL;
     }
-
-    Py_INCREF(origin);
-    alias->origin = origin;
-    alias->args = args;
-    alias->parameters = NULL;
-    alias->weakreflist = NULL;
+    if (!setup_ga(alias, origin, args)) {
+        PyObject_GC_Del((PyObject *)alias);
+        return NULL;
+    }
     _PyObject_GC_TRACK(alias);
     return (PyObject *)alias;
 }
diff --git a/Objects/unionobject.c b/Objects/unionobject.c
index 2308bfc..32aa507 100644
--- a/Objects/unionobject.c
+++ b/Objects/unionobject.c
@@ -237,8 +237,8 @@ dedup_and_flatten_args(PyObject* args)
         PyObject* i_element = PyTuple_GET_ITEM(args, i);
         for (Py_ssize_t j = i + 1; j < arg_length; j++) {
             PyObject* j_element = PyTuple_GET_ITEM(args, j);
-            int is_ga = Py_TYPE(i_element) == &Py_GenericAliasType &&
-                        Py_TYPE(j_element) == &Py_GenericAliasType;
+            int is_ga = PyObject_TypeCheck(i_element, &Py_GenericAliasType) &&
+                        PyObject_TypeCheck(j_element, &Py_GenericAliasType);
             // RichCompare to also deduplicate GenericAlias types (slower)
             is_duplicate = is_ga ? PyObject_RichCompareBool(i_element, j_element, Py_EQ)
                 : i_element == j_element;
@@ -296,7 +296,7 @@ is_unionable(PyObject *obj)
         is_new_type(obj) ||
         is_special_form(obj) ||
         PyType_Check(obj) ||
-        type == &Py_GenericAliasType ||
+        PyObject_TypeCheck(obj, &Py_GenericAliasType) ||
         type == &_Py_UnionType);
 }