From 463c7d3d149283814d879a9bb8411af64e656c8e Mon Sep 17 00:00:00 2001 From: kj <28750310+Fidget-Spinner@users.noreply.github.com> Date: Mon, 14 Dec 2020 02:38:24 +0800 Subject: [PATCH] bpo-42195: Ensure consistency of Callable's __args__ in collections.abc and typing (GH-23060) --- Lib/_collections_abc.py | 69 ++++++++++++++++++- Lib/collections/abc.py | 1 + Lib/test/test_genericalias.py | 58 +++++++++++++++- Lib/test/test_types.py | 12 ++-- Lib/test/test_typing.py | 26 ++----- Lib/typing.py | 32 +++++---- .../2020-11-20-00-57-47.bpo-42195.HeqcpS.rst | 11 +++ Objects/genericaliasobject.c | 60 ++++++++++------ Objects/unionobject.c | 6 +- 9 files changed, 212 insertions(+), 63 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2020-11-20-00-57-47.bpo-42195.HeqcpS.rst diff --git a/Lib/_collections_abc.py b/Lib/_collections_abc.py index 28690f8c0bd..7c3faa64ea7 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 891600d16be..86ca8b8a841 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 c113e538248..5de13fe6d2f 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 3058a02d6ee..83196ad3c17 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 f3e38b6f47d..8e86e769a0d 100644 --- a/Lib/test/test_typing.py +++ b/Lib/test/test_typing.py @@ -446,14 +446,6 @@ class CallableTests(BaseTestCase): type(c)() 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] @@ -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 148a505dad1..7f07321cda8 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 00000000000..ac52a008e35 --- /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 51a12377b7e..756a7ce474a 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 2308bfc9f2a..32aa5078afc 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); }