From 395029b0bd343648b4da8044c7509672ea768775 Mon Sep 17 00:00:00 2001 From: Nikita Sobolev Date: Sat, 19 Feb 2022 04:53:29 +0300 Subject: [PATCH] bpo-46571: improve `typing.no_type_check` to skip foreign objects (GH-31042) There are several changes: 1. We now don't explicitly check for any base / sub types, because new name check covers it 2. I've also checked that `no_type_check` do not modify foreign functions. It was the same as with `type`s 3. I've also covered `except TypeError` in `no_type_check` with a simple test case, it was not covered at all 4. I also felt like adding `lambda` test is a good idea: because `lambda` is a bit of both in class bodies: a function and an assignment https://bugs.python.org/issue46571 --- Doc/library/typing.rst | 4 +- Lib/test/ann_module8.py | 10 ++ Lib/test/test_typing.py | 101 ++++++++++++++++++ Lib/typing.py | 20 +++- .../2022-02-01-11-21-34.bpo-46571.L40xUJ.rst | 4 + 5 files changed, 132 insertions(+), 7 deletions(-) create mode 100644 Lib/test/ann_module8.py create mode 100644 Misc/NEWS.d/next/Library/2022-02-01-11-21-34.bpo-46571.L40xUJ.rst diff --git a/Doc/library/typing.rst b/Doc/library/typing.rst index 8240c912c64..a02ad244d9f 100644 --- a/Doc/library/typing.rst +++ b/Doc/library/typing.rst @@ -2145,8 +2145,8 @@ Functions and decorators Decorator to indicate that annotations are not type hints. This works as class or function :term:`decorator`. With a class, it - applies recursively to all methods defined in that class (but not - to methods defined in its superclasses or subclasses). + applies recursively to all methods and classes defined in that class + (but not to methods defined in its superclasses or subclasses). This mutates the function(s) in place. diff --git a/Lib/test/ann_module8.py b/Lib/test/ann_module8.py new file mode 100644 index 00000000000..bd031481378 --- /dev/null +++ b/Lib/test/ann_module8.py @@ -0,0 +1,10 @@ +# Test `@no_type_check`, +# see https://bugs.python.org/issue46571 + +class NoTypeCheck_Outer: + class Inner: + x: int + + +def NoTypeCheck_function(arg: int) -> int: + ... diff --git a/Lib/test/test_typing.py b/Lib/test/test_typing.py index 1b3eb066766..d24a3571343 100644 --- a/Lib/test/test_typing.py +++ b/Lib/test/test_typing.py @@ -2744,6 +2744,18 @@ class CastTests(BaseTestCase): cast('hello', 42) +# We need this to make sure that `@no_type_check` respects `__module__` attr: +from test import ann_module8 + +@no_type_check +class NoTypeCheck_Outer: + Inner = ann_module8.NoTypeCheck_Outer.Inner + +@no_type_check +class NoTypeCheck_WithFunction: + NoTypeCheck_function = ann_module8.NoTypeCheck_function + + class ForwardRefTests(BaseTestCase): def test_basics(self): @@ -3058,9 +3070,98 @@ class ForwardRefTests(BaseTestCase): @no_type_check class D(C): c = C + # verify that @no_type_check never affects bases self.assertEqual(get_type_hints(C.meth), {'x': int}) + # and never child classes: + class Child(D): + def foo(self, x: int): ... + + self.assertEqual(get_type_hints(Child.foo), {'x': int}) + + def test_no_type_check_nested_types(self): + # See https://bugs.python.org/issue46571 + class Other: + o: int + class B: # Has the same `__name__`` as `A.B` and different `__qualname__` + o: int + @no_type_check + class A: + a: int + class B: + b: int + class C: + c: int + class D: + d: int + + Other = Other + + for klass in [A, A.B, A.B.C, A.D]: + with self.subTest(klass=klass): + self.assertTrue(klass.__no_type_check__) + self.assertEqual(get_type_hints(klass), {}) + + for not_modified in [Other, B]: + with self.subTest(not_modified=not_modified): + with self.assertRaises(AttributeError): + not_modified.__no_type_check__ + self.assertNotEqual(get_type_hints(not_modified), {}) + + def test_no_type_check_class_and_static_methods(self): + @no_type_check + class Some: + @staticmethod + def st(x: int) -> int: ... + @classmethod + def cl(cls, y: int) -> int: ... + + self.assertTrue(Some.st.__no_type_check__) + self.assertEqual(get_type_hints(Some.st), {}) + self.assertTrue(Some.cl.__no_type_check__) + self.assertEqual(get_type_hints(Some.cl), {}) + + def test_no_type_check_other_module(self): + self.assertTrue(NoTypeCheck_Outer.__no_type_check__) + with self.assertRaises(AttributeError): + ann_module8.NoTypeCheck_Outer.__no_type_check__ + with self.assertRaises(AttributeError): + ann_module8.NoTypeCheck_Outer.Inner.__no_type_check__ + + self.assertTrue(NoTypeCheck_WithFunction.__no_type_check__) + with self.assertRaises(AttributeError): + ann_module8.NoTypeCheck_function.__no_type_check__ + + def test_no_type_check_foreign_functions(self): + # We should not modify this function: + def some(*args: int) -> int: + ... + + @no_type_check + class A: + some_alias = some + some_class = classmethod(some) + some_static = staticmethod(some) + + with self.assertRaises(AttributeError): + some.__no_type_check__ + self.assertEqual(get_type_hints(some), {'args': int, 'return': int}) + + def test_no_type_check_lambda(self): + @no_type_check + class A: + # Corner case: `lambda` is both an assignment and a function: + bar: Callable[[int], int] = lambda arg: arg + + self.assertTrue(A.bar.__no_type_check__) + self.assertEqual(get_type_hints(A.bar), {}) + + def test_no_type_check_TypeError(self): + # This simply should not fail with + # `TypeError: can't set attributes of built-in/extension type 'dict'` + no_type_check(dict) + def test_no_type_check_forward_ref_as_string(self): class C: foo: typing.ClassVar[int] = 7 diff --git a/Lib/typing.py b/Lib/typing.py index 8f923fa20f5..ad1435ed23d 100644 --- a/Lib/typing.py +++ b/Lib/typing.py @@ -2131,13 +2131,23 @@ def no_type_check(arg): This mutates the function(s) or class(es) in place. """ if isinstance(arg, type): - arg_attrs = arg.__dict__.copy() - for attr, val in arg.__dict__.items(): - if val in arg.__bases__ + (arg,): - arg_attrs.pop(attr) - for obj in arg_attrs.values(): + for key in dir(arg): + obj = getattr(arg, key) + if ( + not hasattr(obj, '__qualname__') + or obj.__qualname__ != f'{arg.__qualname__}.{obj.__name__}' + or getattr(obj, '__module__', None) != arg.__module__ + ): + # We only modify objects that are defined in this type directly. + # If classes / methods are nested in multiple layers, + # we will modify them when processing their direct holders. + continue + # Instance, class, and static methods: if isinstance(obj, types.FunctionType): obj.__no_type_check__ = True + if isinstance(obj, types.MethodType): + obj.__func__.__no_type_check__ = True + # Nested types: if isinstance(obj, type): no_type_check(obj) try: diff --git a/Misc/NEWS.d/next/Library/2022-02-01-11-21-34.bpo-46571.L40xUJ.rst b/Misc/NEWS.d/next/Library/2022-02-01-11-21-34.bpo-46571.L40xUJ.rst new file mode 100644 index 00000000000..f56c9e4fd76 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2022-02-01-11-21-34.bpo-46571.L40xUJ.rst @@ -0,0 +1,4 @@ +Improve :func:`typing.no_type_check`. + +Now it does not modify external classes and functions. +We also now correctly mark classmethods as not to be type checked.