From 879abeba0b170dfa760f0e0115c98413055039bd Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Sat, 8 Jun 2024 15:47:17 +0100 Subject: [PATCH 1/7] gh-114053: Fix another edge case involving `get_type_hints`, PEP 695 and PEP 563 --- Lib/test/test_typing.py | 6 +----- Lib/typing.py | 6 +++--- .../Library/2024-06-08-15-46-35.gh-issue-114053.Ub2XgJ.rst | 4 ++++ 3 files changed, 8 insertions(+), 8 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2024-06-08-15-46-35.gh-issue-114053.Ub2XgJ.rst diff --git a/Lib/test/test_typing.py b/Lib/test/test_typing.py index dac55ceb9e99e0..1dd382361da0ae 100644 --- a/Lib/test/test_typing.py +++ b/Lib/test/test_typing.py @@ -4866,11 +4866,7 @@ def test_pep695_generic_with_future_annotations(self): self.assertIs(hints_for_A["z"].__args__[0], A_type_params[2]) hints_for_B = get_type_hints(ann_module695.B) - self.assertEqual(hints_for_B.keys(), {"x", "y", "z"}) - self.assertEqual( - set(hints_for_B.values()) ^ set(ann_module695.B.__type_params__), - set() - ) + self.assertEqual(hints_for_B, {"x": int, "y": str, "z": bytes}) hints_for_generic_function = get_type_hints(ann_module695.generic_function) func_t_params = ann_module695.generic_function.__type_params__ diff --git a/Lib/typing.py b/Lib/typing.py index be49aa63464f05..02bdfabb442f12 100644 --- a/Lib/typing.py +++ b/Lib/typing.py @@ -1064,11 +1064,11 @@ def _evaluate(self, globalns, localns, type_params=_sentinel, *, recursive_guard # "Inject" type parameters into the local namespace # (unless they are shadowed by assignments *in* the local namespace), # as a way of emulating annotation scopes when calling `eval()` - locals_to_pass = {param.__name__: param for param in type_params} | localns + globals_to_pass = {param.__name__: param for param in type_params} | globalns else: - locals_to_pass = localns + globals_to_pass = globalns type_ = _type_check( - eval(self.__forward_code__, globalns, locals_to_pass), + eval(self.__forward_code__, globals_to_pass, localns), "Forward references must evaluate to types.", is_argument=self.__forward_is_argument__, allow_special_forms=self.__forward_is_class__, diff --git a/Misc/NEWS.d/next/Library/2024-06-08-15-46-35.gh-issue-114053.Ub2XgJ.rst b/Misc/NEWS.d/next/Library/2024-06-08-15-46-35.gh-issue-114053.Ub2XgJ.rst new file mode 100644 index 00000000000000..8aea591da5274c --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-06-08-15-46-35.gh-issue-114053.Ub2XgJ.rst @@ -0,0 +1,4 @@ +Fix edge-case bug where :func:`typing.get_type_hints` would produce +incorrect results if type parameters in a class scope were overridden by +assignments in a class scope and ``from __future__ import annotations`` +semantics were enabled. Patch by Alex Waygood. From 373b23cb14e84270293cf2065291b5529807d015 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Sun, 9 Jun 2024 11:58:52 +0100 Subject: [PATCH 2/7] Add failing test --- Lib/test/test_typing.py | 5 +++++ Lib/test/typinganndata/ann_module695.py | 9 +++++++++ 2 files changed, 14 insertions(+) diff --git a/Lib/test/test_typing.py b/Lib/test/test_typing.py index 1dd382361da0ae..c3a76c659b4f5f 100644 --- a/Lib/test/test_typing.py +++ b/Lib/test/test_typing.py @@ -4868,6 +4868,11 @@ def test_pep695_generic_with_future_annotations(self): hints_for_B = get_type_hints(ann_module695.B) self.assertEqual(hints_for_B, {"x": int, "y": str, "z": bytes}) + hints_for_C = get_type_hints(ann_module695.C) + self.assertNotIn(int, hints_for_C.values()) + self.assertNotIn(str, hints_for_C.values()) + self.assertEqual(set(hints_for_C.values()), set(C.__type_params__)) + hints_for_generic_function = get_type_hints(ann_module695.generic_function) func_t_params = ann_module695.generic_function.__type_params__ self.assertEqual( diff --git a/Lib/test/typinganndata/ann_module695.py b/Lib/test/typinganndata/ann_module695.py index 2ede9fe382564f..c98bfd28532bd9 100644 --- a/Lib/test/typinganndata/ann_module695.py +++ b/Lib/test/typinganndata/ann_module695.py @@ -17,6 +17,15 @@ class B[T, *Ts, **P]: z: P +Eggs = int +Spam = str + + +class C[Eggs, **Spam]: + x: Eggs + y: Spam + + def generic_function[T, *Ts, **P]( x: T, *y: *Ts, z: P.args, zz: P.kwargs ) -> None: ... From 4f95b1c394a0ca1e3c59e43655b945572548a551 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Sun, 9 Jun 2024 12:36:05 +0100 Subject: [PATCH 3/7] Fix failing test --- Lib/test/test_typing.py | 17 ++++++++++++++++- Lib/test/typinganndata/ann_module695.py | 9 +++++++++ Lib/typing.py | 24 +++++++++++++++++------- 3 files changed, 42 insertions(+), 8 deletions(-) diff --git a/Lib/test/test_typing.py b/Lib/test/test_typing.py index c3a76c659b4f5f..1d8dd16d40fcdb 100644 --- a/Lib/test/test_typing.py +++ b/Lib/test/test_typing.py @@ -4871,7 +4871,10 @@ def test_pep695_generic_with_future_annotations(self): hints_for_C = get_type_hints(ann_module695.C) self.assertNotIn(int, hints_for_C.values()) self.assertNotIn(str, hints_for_C.values()) - self.assertEqual(set(hints_for_C.values()), set(C.__type_params__)) + self.assertEqual( + set(hints_for_C.values()), + set(ann_module695.C.__type_params__) + ) hints_for_generic_function = get_type_hints(ann_module695.generic_function) func_t_params = ann_module695.generic_function.__type_params__ @@ -4883,6 +4886,18 @@ def test_pep695_generic_with_future_annotations(self): self.assertIs(hints_for_generic_function["z"].__origin__, func_t_params[2]) self.assertIs(hints_for_generic_function["zz"].__origin__, func_t_params[2]) + hints_for_generic_method = get_type_hints(ann_module695.D.generic_method) + self.assertNotIn(int, hints_for_generic_method.values()) + self.assertNotIn(str, hints_for_generic_method.values()) + params = { + param.__name__: param + for param in ann_module695.D.generic_method.__type_params__ + } + self.assertEqual( + hints_for_generic_method, + {"x": params["Foo"], "y": params["Bar"], "return": types.NoneType} + ) + def test_extended_generic_rules_subclassing(self): class T1(Tuple[T, KT]): ... class T2(Tuple[T, ...]): ... diff --git a/Lib/test/typinganndata/ann_module695.py b/Lib/test/typinganndata/ann_module695.py index c98bfd28532bd9..b7f4c64156b210 100644 --- a/Lib/test/typinganndata/ann_module695.py +++ b/Lib/test/typinganndata/ann_module695.py @@ -29,3 +29,12 @@ class C[Eggs, **Spam]: def generic_function[T, *Ts, **P]( x: T, *y: *Ts, z: P.args, zz: P.kwargs ) -> None: ... + + +class D: + Foo = int + Bar = str + + def generic_method[Foo, **Bar]( + self, x: Foo, y: Bar + ) -> None: ... diff --git a/Lib/typing.py b/Lib/typing.py index 02bdfabb442f12..ee1bd276ef1bda 100644 --- a/Lib/typing.py +++ b/Lib/typing.py @@ -1060,15 +1060,25 @@ def _evaluate(self, globalns, localns, type_params=_sentinel, *, recursive_guard globalns = getattr( sys.modules.get(self.__forward_module__, None), '__dict__', globalns ) - if type_params: - # "Inject" type parameters into the local namespace - # (unless they are shadowed by assignments *in* the local namespace), - # as a way of emulating annotation scopes when calling `eval()` - globals_to_pass = {param.__name__: param for param in type_params} | globalns + + # type parameters require some special handling, + # as they exist in their own scope + # but `eval()` does not have a dedicated parameter for that scope. + # For classes, names in type parameter scopes should override + # names in the global scope (which here are called `localns`!), + # but should in turn be overridden by names in the class scope + # (which here are called `globalns`!) + type_params = {param.__name__: param for param in type_params} + if self.__forward_is_class__: + for name, param in type_params.items(): + if name not in globalns: + globalns[name] = param + localns.pop(name, None) else: - globals_to_pass = globalns + localns = type_params | localns + type_ = _type_check( - eval(self.__forward_code__, globals_to_pass, localns), + eval(self.__forward_code__, globalns, localns), "Forward references must evaluate to types.", is_argument=self.__forward_is_argument__, allow_special_forms=self.__forward_is_class__, From ffdba91b2879a4100e4e98dd8ed92276c0ccc97d Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Sun, 9 Jun 2024 12:38:25 +0100 Subject: [PATCH 4/7] remove redundant assertions in test --- Lib/test/test_typing.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/Lib/test/test_typing.py b/Lib/test/test_typing.py index 1d8dd16d40fcdb..2ae9f894fa3d9f 100644 --- a/Lib/test/test_typing.py +++ b/Lib/test/test_typing.py @@ -4869,8 +4869,6 @@ def test_pep695_generic_with_future_annotations(self): self.assertEqual(hints_for_B, {"x": int, "y": str, "z": bytes}) hints_for_C = get_type_hints(ann_module695.C) - self.assertNotIn(int, hints_for_C.values()) - self.assertNotIn(str, hints_for_C.values()) self.assertEqual( set(hints_for_C.values()), set(ann_module695.C.__type_params__) @@ -4887,8 +4885,6 @@ def test_pep695_generic_with_future_annotations(self): self.assertIs(hints_for_generic_function["zz"].__origin__, func_t_params[2]) hints_for_generic_method = get_type_hints(ann_module695.D.generic_method) - self.assertNotIn(int, hints_for_generic_method.values()) - self.assertNotIn(str, hints_for_generic_method.values()) params = { param.__name__: param for param in ann_module695.D.generic_method.__type_params__ From 5505d971a667cdcc10c31ace527f411e22112213 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Sun, 9 Jun 2024 13:27:45 +0100 Subject: [PATCH 5/7] Don't mutate globals! --- Lib/test/test_typing.py | 5 +++++ Lib/typing.py | 18 ++++++++++-------- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/Lib/test/test_typing.py b/Lib/test/test_typing.py index 1d8dd16d40fcdb..531d251b7e2f0d 100644 --- a/Lib/test/test_typing.py +++ b/Lib/test/test_typing.py @@ -4859,6 +4859,8 @@ def f(x: X): ... ) def test_pep695_generic_with_future_annotations(self): + original_globals = dict(ann_module695.__dict__) + hints_for_A = get_type_hints(ann_module695.A) A_type_params = ann_module695.A.__type_params__ self.assertIs(hints_for_A["x"], A_type_params[0]) @@ -4898,6 +4900,9 @@ def test_pep695_generic_with_future_annotations(self): {"x": params["Foo"], "y": params["Bar"], "return": types.NoneType} ) + # should not have changed as a result of the get_type_hints() calls! + self.assertEqual(ann_module695.__dict__, original_globals) + def test_extended_generic_rules_subclassing(self): class T1(Tuple[T, KT]): ... class T2(Tuple[T, ...]): ... diff --git a/Lib/typing.py b/Lib/typing.py index ee1bd276ef1bda..76e49d6855c3c1 100644 --- a/Lib/typing.py +++ b/Lib/typing.py @@ -1068,14 +1068,16 @@ def _evaluate(self, globalns, localns, type_params=_sentinel, *, recursive_guard # names in the global scope (which here are called `localns`!), # but should in turn be overridden by names in the class scope # (which here are called `globalns`!) - type_params = {param.__name__: param for param in type_params} - if self.__forward_is_class__: - for name, param in type_params.items(): - if name not in globalns: - globalns[name] = param - localns.pop(name, None) - else: - localns = type_params | localns + if type_params: + name_to_param_map = {param.__name__: param for param in type_params} + if self.__forward_is_class__: + globalns, localns = dict(globalns), dict(localns) + for name, param in name_to_param_map.items(): + if name not in globalns: + globalns[name] = param + localns.pop(name, None) + else: + localns = name_to_param_map | localns type_ = _type_check( eval(self.__forward_code__, globalns, localns), From bccaea2f237699fd3be6db9ab4646ac61e28f28a Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Sun, 9 Jun 2024 13:35:01 +0100 Subject: [PATCH 6/7] simplify --- Lib/typing.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Lib/typing.py b/Lib/typing.py index 76e49d6855c3c1..dc1024dabe5e87 100644 --- a/Lib/typing.py +++ b/Lib/typing.py @@ -1069,15 +1069,15 @@ def _evaluate(self, globalns, localns, type_params=_sentinel, *, recursive_guard # but should in turn be overridden by names in the class scope # (which here are called `globalns`!) if type_params: - name_to_param_map = {param.__name__: param for param in type_params} if self.__forward_is_class__: globalns, localns = dict(globalns), dict(localns) - for name, param in name_to_param_map.items(): - if name not in globalns: - globalns[name] = param - localns.pop(name, None) + for param in type_params: + param_name = param.__name__ + if param_name not in globalns: + globalns[param_name] = param + localns.pop(param_name, None) else: - localns = name_to_param_map | localns + localns = {param.__name__: param for param in type_params} | localns type_ = _type_check( eval(self.__forward_code__, globalns, localns), From 2fbcd7b10c5c614ae23eccfb2eadba67269e8ca2 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Sun, 9 Jun 2024 23:31:18 +0100 Subject: [PATCH 7/7] Fix yet another bug and add many more tests --- Lib/test/test_typing.py | 47 +++++++++++++++++++++++-- Lib/test/typinganndata/ann_module695.py | 32 +++++++++++++++++ Lib/typing.py | 15 ++++---- 3 files changed, 82 insertions(+), 12 deletions(-) diff --git a/Lib/test/test_typing.py b/Lib/test/test_typing.py index 39dab58d267ede..b4df549a03a1c6 100644 --- a/Lib/test/test_typing.py +++ b/Lib/test/test_typing.py @@ -4858,7 +4858,7 @@ def f(x: X): ... {'x': list[list[ForwardRef('X')]]} ) - def test_pep695_generic_with_future_annotations(self): + def test_pep695_generic_class_with_future_annotations(self): original_globals = dict(ann_module695.__dict__) hints_for_A = get_type_hints(ann_module695.A) @@ -4867,15 +4867,21 @@ def test_pep695_generic_with_future_annotations(self): self.assertEqual(hints_for_A["y"].__args__[0], Unpack[A_type_params[1]]) self.assertIs(hints_for_A["z"].__args__[0], A_type_params[2]) + # should not have changed as a result of the get_type_hints() calls! + self.assertEqual(ann_module695.__dict__, original_globals) + + def test_pep695_generic_class_with_future_annotations_and_local_shadowing(self): hints_for_B = get_type_hints(ann_module695.B) self.assertEqual(hints_for_B, {"x": int, "y": str, "z": bytes}) + def test_pep695_generic_class_with_future_annotations_name_clash_with_global_vars(self): hints_for_C = get_type_hints(ann_module695.C) self.assertEqual( set(hints_for_C.values()), set(ann_module695.C.__type_params__) ) + def test_pep_695_generic_function_with_future_annotations(self): hints_for_generic_function = get_type_hints(ann_module695.generic_function) func_t_params = ann_module695.generic_function.__type_params__ self.assertEqual( @@ -4886,6 +4892,13 @@ def test_pep695_generic_with_future_annotations(self): self.assertIs(hints_for_generic_function["z"].__origin__, func_t_params[2]) self.assertIs(hints_for_generic_function["zz"].__origin__, func_t_params[2]) + def test_pep_695_generic_function_with_future_annotations_name_clash_with_global_vars(self): + self.assertEqual( + set(get_type_hints(ann_module695.generic_function_2).values()), + set(ann_module695.generic_function_2.__type_params__) + ) + + def test_pep_695_generic_method_with_future_annotations(self): hints_for_generic_method = get_type_hints(ann_module695.D.generic_method) params = { param.__name__: param @@ -4896,8 +4909,36 @@ def test_pep695_generic_with_future_annotations(self): {"x": params["Foo"], "y": params["Bar"], "return": types.NoneType} ) - # should not have changed as a result of the get_type_hints() calls! - self.assertEqual(ann_module695.__dict__, original_globals) + def test_pep_695_generic_method_with_future_annotations_name_clash_with_global_vars(self): + self.assertEqual( + set(get_type_hints(ann_module695.D.generic_method_2).values()), + set(ann_module695.D.generic_method_2.__type_params__) + ) + + def test_pep_695_generics_with_future_annotations_nested_in_function(self): + results = ann_module695.nested() + + self.assertEqual( + set(results.hints_for_E.values()), + set(results.E.__type_params__) + ) + self.assertEqual( + set(results.hints_for_E_meth.values()), + set(results.E.generic_method.__type_params__) + ) + self.assertNotEqual( + set(results.hints_for_E_meth.values()), + set(results.E.__type_params__) + ) + self.assertEqual( + set(results.hints_for_E_meth.values()).intersection(results.E.__type_params__), + set() + ) + + self.assertEqual( + set(results.hints_for_generic_func.values()), + set(results.generic_func.__type_params__) + ) def test_extended_generic_rules_subclassing(self): class T1(Tuple[T, KT]): ... diff --git a/Lib/test/typinganndata/ann_module695.py b/Lib/test/typinganndata/ann_module695.py index b7f4c64156b210..b6f3b06bd5065f 100644 --- a/Lib/test/typinganndata/ann_module695.py +++ b/Lib/test/typinganndata/ann_module695.py @@ -31,6 +31,9 @@ def generic_function[T, *Ts, **P]( ) -> None: ... +def generic_function_2[Eggs, **Spam](x: Eggs, y: Spam): pass + + class D: Foo = int Bar = str @@ -38,3 +41,32 @@ class D: def generic_method[Foo, **Bar]( self, x: Foo, y: Bar ) -> None: ... + + def generic_method_2[Eggs, **Spam](self, x: Eggs, y: Spam): pass + + +def nested(): + from types import SimpleNamespace + from typing import get_type_hints + + Eggs = bytes + Spam = memoryview + + + class E[Eggs, **Spam]: + x: Eggs + y: Spam + + def generic_method[Eggs, **Spam](self, x: Eggs, y: Spam): pass + + + def generic_function[Eggs, **Spam](x: Eggs, y: Spam): pass + + + return SimpleNamespace( + E=E, + hints_for_E=get_type_hints(E), + hints_for_E_meth=get_type_hints(E.generic_method), + generic_func=generic_function, + hints_for_generic_func=get_type_hints(generic_function) + ) diff --git a/Lib/typing.py b/Lib/typing.py index dc1024dabe5e87..cd12084587ed52 100644 --- a/Lib/typing.py +++ b/Lib/typing.py @@ -1069,15 +1069,12 @@ def _evaluate(self, globalns, localns, type_params=_sentinel, *, recursive_guard # but should in turn be overridden by names in the class scope # (which here are called `globalns`!) if type_params: - if self.__forward_is_class__: - globalns, localns = dict(globalns), dict(localns) - for param in type_params: - param_name = param.__name__ - if param_name not in globalns: - globalns[param_name] = param - localns.pop(param_name, None) - else: - localns = {param.__name__: param for param in type_params} | localns + globalns, localns = dict(globalns), dict(localns) + for param in type_params: + param_name = param.__name__ + if not self.__forward_is_class__ or param_name not in globalns: + globalns[param_name] = param + localns.pop(param_name, None) type_ = _type_check( eval(self.__forward_code__, globalns, localns),