From 060ca69b95a968062c0176f87bf7c189018ddbc0 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Wed, 29 Jan 2025 21:30:19 -0800 Subject: [PATCH 01/10] gh-129463: gh-128593: Simplify ForwardRef --- Lib/annotationlib.py | 28 ----- Lib/test/test_annotationlib.py | 54 ++++----- Lib/test/test_typing.py | 110 ++++-------------- ...-01-29-21-27-45.gh-issue-128593.r3j4l-.rst | 3 + ...-01-29-21-29-46.gh-issue-129463.qePexX.rst | 4 + 5 files changed, 52 insertions(+), 147 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2025-01-29-21-27-45.gh-issue-128593.r3j4l-.rst create mode 100644 Misc/NEWS.d/next/Library/2025-01-29-21-29-46.gh-issue-129463.qePexX.rst diff --git a/Lib/annotationlib.py b/Lib/annotationlib.py index 42f1f3877514d9..319bdb0c616386 100644 --- a/Lib/annotationlib.py +++ b/Lib/annotationlib.py @@ -34,8 +34,6 @@ class Format(enum.IntEnum): # preserved for compatibility with the old typing.ForwardRef class. The remaining # names are private. _SLOTS = ( - "__forward_evaluated__", - "__forward_value__", "__forward_is_argument__", "__forward_is_class__", "__forward_module__", @@ -78,8 +76,6 @@ def __init__( raise TypeError(f"Forward reference must be a string -- got {arg!r}") self.__arg__ = arg - self.__forward_evaluated__ = False - self.__forward_value__ = None self.__forward_is_argument__ = is_argument self.__forward_is_class__ = is_class self.__forward_module__ = module @@ -97,16 +93,12 @@ def evaluate(self, *, globals=None, locals=None, type_params=None, owner=None): If the forward reference cannot be evaluated, raise an exception. """ - if self.__forward_evaluated__: - return self.__forward_value__ if self.__cell__ is not None: try: value = self.__cell__.cell_contents except ValueError: pass else: - self.__forward_evaluated__ = True - self.__forward_value__ = value return value if owner is None: owner = self.__owner__ @@ -173,8 +165,6 @@ def evaluate(self, *, globals=None, locals=None, type_params=None, owner=None): else: code = self.__forward_code__ value = eval(code, globals=globals, locals=locals) - self.__forward_evaluated__ = True - self.__forward_value__ = value return value def _evaluate(self, globalns, localns, type_params=_sentinel, *, recursive_guard): @@ -229,22 +219,6 @@ def __forward_code__(self): raise SyntaxError(f"Forward reference must be an expression -- got {arg!r}") return self.__code__ - def __eq__(self, other): - if not isinstance(other, ForwardRef): - return NotImplemented - if self.__forward_evaluated__ and other.__forward_evaluated__: - return ( - self.__forward_arg__ == other.__forward_arg__ - and self.__forward_value__ == other.__forward_value__ - ) - return ( - self.__forward_arg__ == other.__forward_arg__ - and self.__forward_module__ == other.__forward_module__ - ) - - def __hash__(self): - return hash((self.__forward_arg__, self.__forward_module__)) - def __or__(self, other): global _Union if _Union is None: @@ -284,8 +258,6 @@ def __init__( # represent a single name). assert isinstance(node, (ast.AST, str)) self.__arg__ = None - self.__forward_evaluated__ = False - self.__forward_value__ = None self.__forward_is_argument__ = False self.__forward_is_class__ = is_class self.__forward_module__ = None diff --git a/Lib/test/test_annotationlib.py b/Lib/test/test_annotationlib.py index 20f74b4ed0aadb..6851b36c7c13ff 100644 --- a/Lib/test/test_annotationlib.py +++ b/Lib/test/test_annotationlib.py @@ -32,6 +32,11 @@ def wrapper(a, b): return wrapper +def assert_is_fwdref(case, obj, value): + case.assertIsInstance(obj, annotationlib.ForwardRef) + case.assertEqual(obj.__forward_arg__, value) + + class MyClass: def __repr__(self): return "my repr" @@ -59,8 +64,7 @@ def inner(arg: x): anno = annotationlib.get_annotations(inner, format=Format.FORWARDREF) fwdref = anno["arg"] - self.assertIsInstance(fwdref, annotationlib.ForwardRef) - self.assertEqual(fwdref.__forward_arg__, "x") + assert_is_fwdref(self, fwdref, "x") with self.assertRaises(NameError): fwdref.evaluate() @@ -77,8 +81,7 @@ def f(x: int, y: doesntexist): anno = annotationlib.get_annotations(f, format=Format.FORWARDREF) self.assertIs(anno["x"], int) fwdref = anno["y"] - self.assertIsInstance(fwdref, annotationlib.ForwardRef) - self.assertEqual(fwdref.__forward_arg__, "doesntexist") + assert_is_fwdref(self, fwdref, "doesntexist") with self.assertRaises(NameError): fwdref.evaluate() self.assertEqual(fwdref.evaluate(globals={"doesntexist": 1}), 1) @@ -96,28 +99,22 @@ def f( anno = annotationlib.get_annotations(f, format=Format.FORWARDREF) x_anno = anno["x"] - self.assertIsInstance(x_anno, ForwardRef) - self.assertEqual(x_anno, ForwardRef("some.module")) + assert_is_fwdref(self, x_anno, "some.module") y_anno = anno["y"] - self.assertIsInstance(y_anno, ForwardRef) - self.assertEqual(y_anno, ForwardRef("some[module]")) + assert_is_fwdref(self, y_anno, "some[module]") z_anno = anno["z"] - self.assertIsInstance(z_anno, ForwardRef) - self.assertEqual(z_anno, ForwardRef("some(module)")) + assert_is_fwdref(self, z_anno, "some(module)") alpha_anno = anno["alpha"] - self.assertIsInstance(alpha_anno, ForwardRef) - self.assertEqual(alpha_anno, ForwardRef("some | obj")) + assert_is_fwdref(self, alpha_anno, "some | obj") beta_anno = anno["beta"] - self.assertIsInstance(beta_anno, ForwardRef) - self.assertEqual(beta_anno, ForwardRef("+some")) + assert_is_fwdref(self, beta_anno, "+some") gamma_anno = anno["gamma"] - self.assertIsInstance(gamma_anno, ForwardRef) - self.assertEqual(gamma_anno, ForwardRef("some < obj")) + assert_is_fwdref(self, gamma_anno, "some < obj") class TestSourceFormat(unittest.TestCase): @@ -362,13 +359,6 @@ def test_fwdref_to_builtin(self): obj = object() self.assertIs(ForwardRef("int").evaluate(globals={"int": obj}), obj) - def test_fwdref_value_is_cached(self): - fr = ForwardRef("hello") - with self.assertRaises(NameError): - fr.evaluate() - self.assertIs(fr.evaluate(globals={"hello": str}), str) - self.assertIs(fr.evaluate(), str) - def test_fwdref_with_owner(self): self.assertEqual( ForwardRef("Counter[int]", owner=collections).evaluate(), @@ -457,12 +447,10 @@ def f2(a: undefined): ) self.assertEqual(annotationlib.get_annotations(f1, format=1), {"a": int}) - fwd = annotationlib.ForwardRef("undefined") - self.assertEqual( - annotationlib.get_annotations(f2, format=Format.FORWARDREF), - {"a": fwd}, - ) - self.assertEqual(annotationlib.get_annotations(f2, format=3), {"a": fwd}) + for fmt in (Format.FORWARDREF, 3): + annos = annotationlib.get_annotations(f2, format=fmt) + self.assertEqual(list(annos), ["a"]) + assert_is_fwdref(self, annos["a"], "undefined") self.assertEqual( annotationlib.get_annotations(f1, format=Format.STRING), @@ -1012,10 +1000,10 @@ def evaluate(format, exc=NotImplementedError): with self.assertRaises(NameError): annotationlib.call_evaluate_function(evaluate, Format.VALUE) - self.assertEqual( - annotationlib.call_evaluate_function(evaluate, Format.FORWARDREF), - annotationlib.ForwardRef("undefined"), - ) + + fwdref = annotationlib.call_evaluate_function(evaluate, Format.FORWARDREF) + assert_is_fwdref(self, fwdref, "undefined") + self.assertEqual( annotationlib.call_evaluate_function(evaluate, Format.STRING), "undefined", diff --git a/Lib/test/test_typing.py b/Lib/test/test_typing.py index f002d28df60e9c..20587add028b01 100644 --- a/Lib/test/test_typing.py +++ b/Lib/test/test_typing.py @@ -76,6 +76,17 @@ def wrapper(self): return wrapper +class EqualToForwardRef: + def __init__(self, arg, module=None): + self.arg = arg + self.module = module + + def __eq__(self, other): + if not isinstance(other, ForwardRef): + return NotImplemented + return self.arg == other.__forward_arg__ and self.module == other.__forward_module__ + + class Employee: pass @@ -467,8 +478,8 @@ def test_or(self): self.assertEqual(X | "x", Union[X, "x"]) self.assertEqual("x" | X, Union["x", X]) # make sure the order is correct - self.assertEqual(get_args(X | "x"), (X, ForwardRef("x"))) - self.assertEqual(get_args("x" | X), (ForwardRef("x"), X)) + self.assertEqual(get_args(X | "x"), (X, EqualToForwardRef("x"))) + self.assertEqual(get_args("x" | X), (EqualToForwardRef("x"), X)) def test_union_constrained(self): A = TypeVar('A', str, bytes) @@ -4965,7 +4976,7 @@ class C3: def f(x: X): ... self.assertEqual( get_type_hints(f, globals(), locals()), - {'x': list[list[ForwardRef('X')]]} + {'x': list[list[EqualToForwardRef('X')]]} ) def test_pep695_generic_class_with_future_annotations(self): @@ -5183,12 +5194,15 @@ class Node(Generic[T]): ... Callable[..., T], Callable[[int], int], Tuple[Any, Any], Node[T], Node[int], Node[Any], typing.Iterable[T], typing.Iterable[Any], typing.Iterable[int], typing.Dict[int, str], - typing.Dict[T, Any], ClassVar[int], ClassVar[List[T]], Tuple['T', 'T'], - Union['T', int], List['T'], typing.Mapping['T', int]] + typing.Dict[T, Any], ClassVar[int], ClassVar[List[T]]] for t in things + [Any]: self.assertEqual(t, copy(t)) self.assertEqual(t, deepcopy(t)) + shallow_things = [Tuple['T', 'T'], Union['T', int], List['T'], typing.Mapping['T', int]] + for t in things + [Any]: + self.assertEqual(t, copy(t)) + def test_immutability_by_copy_and_pickle(self): # Special forms like Union, Any, etc., generic aliases to containers like List, # Mapping, etc., and type variabcles are considered immutable by copy and pickle. @@ -6087,82 +6101,6 @@ def test_forwardref_only_str_arg(self): with self.assertRaises(TypeError): typing.ForwardRef(1) # only `str` type is allowed - def test_forward_equality(self): - fr = typing.ForwardRef('int') - self.assertEqual(fr, typing.ForwardRef('int')) - self.assertNotEqual(List['int'], List[int]) - self.assertNotEqual(fr, typing.ForwardRef('int', module=__name__)) - frm = typing.ForwardRef('int', module=__name__) - self.assertEqual(frm, typing.ForwardRef('int', module=__name__)) - self.assertNotEqual(frm, typing.ForwardRef('int', module='__other_name__')) - - def test_forward_equality_gth(self): - c1 = typing.ForwardRef('C') - c1_gth = typing.ForwardRef('C') - c2 = typing.ForwardRef('C') - c2_gth = typing.ForwardRef('C') - - class C: - pass - def foo(a: c1_gth, b: c2_gth): - pass - - self.assertEqual(get_type_hints(foo, globals(), locals()), {'a': C, 'b': C}) - self.assertEqual(c1, c2) - self.assertEqual(c1, c1_gth) - self.assertEqual(c1_gth, c2_gth) - self.assertEqual(List[c1], List[c1_gth]) - self.assertNotEqual(List[c1], List[C]) - self.assertNotEqual(List[c1_gth], List[C]) - self.assertEqual(Union[c1, c1_gth], Union[c1]) - self.assertEqual(Union[c1, c1_gth, int], Union[c1, int]) - - def test_forward_equality_hash(self): - c1 = typing.ForwardRef('int') - c1_gth = typing.ForwardRef('int') - c2 = typing.ForwardRef('int') - c2_gth = typing.ForwardRef('int') - - def foo(a: c1_gth, b: c2_gth): - pass - get_type_hints(foo, globals(), locals()) - - self.assertEqual(hash(c1), hash(c2)) - self.assertEqual(hash(c1_gth), hash(c2_gth)) - self.assertEqual(hash(c1), hash(c1_gth)) - - c3 = typing.ForwardRef('int', module=__name__) - c4 = typing.ForwardRef('int', module='__other_name__') - - self.assertNotEqual(hash(c3), hash(c1)) - self.assertNotEqual(hash(c3), hash(c1_gth)) - self.assertNotEqual(hash(c3), hash(c4)) - self.assertEqual(hash(c3), hash(typing.ForwardRef('int', module=__name__))) - - def test_forward_equality_namespace(self): - class A: - pass - def namespace1(): - a = typing.ForwardRef('A') - def fun(x: a): - pass - get_type_hints(fun, globals(), locals()) - return a - - def namespace2(): - a = typing.ForwardRef('A') - - class A: - pass - def fun(x: a): - pass - - get_type_hints(fun, globals(), locals()) - return a - - self.assertEqual(namespace1(), namespace1()) - self.assertNotEqual(namespace1(), namespace2()) - def test_forward_repr(self): self.assertEqual(repr(List['int']), "typing.List[ForwardRef('int')]") self.assertEqual(repr(List[ForwardRef('int', module='mod')]), @@ -6226,7 +6164,7 @@ def cmp(o1, o2): r1 = namespace1() r2 = namespace2() self.assertIsNot(r1, r2) - self.assertRaises(RecursionError, cmp, r1, r2) + self.assertNotEqual(r1, r2) def test_union_forward_recursion(self): ValueList = List['Value'] @@ -7146,7 +7084,7 @@ def func(x: undefined) -> undefined: ... # FORWARDREF self.assertEqual( get_type_hints(func, format=annotationlib.Format.FORWARDREF), - {'x': ForwardRef('undefined'), 'return': ForwardRef('undefined')}, + {'x': EqualToForwardRef('undefined'), 'return': EqualToForwardRef('undefined')}, ) # STRING @@ -8030,7 +7968,7 @@ class Y(NamedTuple): class Z(NamedTuple): a: None b: "str" - annos = {'a': type(None), 'b': ForwardRef("str")} + annos = {'a': type(None), 'b': EqualToForwardRef("str")} self.assertEqual(Z.__annotations__, annos) self.assertEqual(Z.__annotate__(annotationlib.Format.VALUE), annos) self.assertEqual(Z.__annotate__(annotationlib.Format.FORWARDREF), annos) @@ -8046,7 +7984,7 @@ class X(NamedTuple): """ ns = run_code(textwrap.dedent(code)) X = ns['X'] - self.assertEqual(X.__annotations__, {'a': ForwardRef("int"), 'b': ForwardRef("None")}) + self.assertEqual(X.__annotations__, {'a': EqualToForwardRef("int"), 'b': EqualToForwardRef("None")}) def test_deferred_annotations(self): class X(NamedTuple): @@ -9032,7 +8970,7 @@ class X(TypedDict): class Y(TypedDict): a: None b: "int" - fwdref = ForwardRef('int', module=__name__) + fwdref = EqualToForwardRef('int', module=__name__) self.assertEqual(Y.__annotations__, {'a': type(None), 'b': fwdref}) self.assertEqual(Y.__annotate__(annotationlib.Format.FORWARDREF), {'a': type(None), 'b': fwdref}) diff --git a/Misc/NEWS.d/next/Library/2025-01-29-21-27-45.gh-issue-128593.r3j4l-.rst b/Misc/NEWS.d/next/Library/2025-01-29-21-27-45.gh-issue-128593.r3j4l-.rst new file mode 100644 index 00000000000000..03fb5a5413a4a8 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-01-29-21-27-45.gh-issue-128593.r3j4l-.rst @@ -0,0 +1,3 @@ +:class:`annotationlib.ForwardRef` objects no longer cache their value when +they are successfully evaluated. Successive calls to +:meth:`annotationlib.ForwardRef.evaluate` may return different values. diff --git a/Misc/NEWS.d/next/Library/2025-01-29-21-29-46.gh-issue-129463.qePexX.rst b/Misc/NEWS.d/next/Library/2025-01-29-21-29-46.gh-issue-129463.qePexX.rst new file mode 100644 index 00000000000000..497b5e997038df --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-01-29-21-29-46.gh-issue-129463.qePexX.rst @@ -0,0 +1,4 @@ +:class:`annotationlib.ForwardRef` objects no longer compare or hash equal +when they refer to the same string. The implementation of equality was +error-prone because it did not take all attributes of the +:class:`!ForwardRef` object into account. From 9c1c8c8dcd79f14eb88ccee9a6ec61d276ba2089 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Wed, 29 Jan 2025 22:07:06 -0800 Subject: [PATCH 02/10] fix test_inspect --- Lib/test/test_inspect/test_inspect.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_inspect/test_inspect.py b/Lib/test/test_inspect/test_inspect.py index 8e47df21cfef2e..2b706163874ab1 100644 --- a/Lib/test/test_inspect/test_inspect.py +++ b/Lib/test/test_inspect/test_inspect.py @@ -48,6 +48,7 @@ from test.test_inspect import inspect_fodder2 as mod2 from test.test_inspect import inspect_stringized_annotations from test.test_inspect import inspect_deferred_annotations +from test.test_typing import EqualToForwardRef # Functions tested in this suite: @@ -4912,7 +4913,7 @@ def test_signature_annotation_format(self): ) self.assertEqual( signature_func(ida.f, annotation_format=Format.FORWARDREF), - sig([par("x", PORK, annotation=ForwardRef("undefined"))]) + sig([par("x", PORK, annotation=EqualToForwardRef("undefined"))]) ) with self.assertRaisesRegex(NameError, "undefined"): signature_func(ida.f, annotation_format=Format.VALUE) From f59d4fb724f09a7a081df80b5894d0f7775b64c8 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Fri, 21 Feb 2025 22:14:58 -0800 Subject: [PATCH 03/10] Instead implement __eq__/__hash__ correctly --- Lib/annotationlib.py | 41 ++++++- Lib/test/support/__init__.py | 42 +++++++ Lib/test/test_annotationlib.py | 55 +++++---- Lib/test/test_inspect/test_inspect.py | 10 +- Lib/test/test_typing.py | 114 ++++++++++++++---- ...-01-29-21-29-46.gh-issue-129463.qePexX.rst | 7 +- 6 files changed, 209 insertions(+), 60 deletions(-) diff --git a/Lib/annotationlib.py b/Lib/annotationlib.py index 319bdb0c616386..7bc8609334e0ad 100644 --- a/Lib/annotationlib.py +++ b/Lib/annotationlib.py @@ -219,6 +219,34 @@ def __forward_code__(self): raise SyntaxError(f"Forward reference must be an expression -- got {arg!r}") return self.__code__ + def __eq__(self, other): + if not isinstance(other, ForwardRef): + return NotImplemented + return ( + self.__forward_arg__ == other.__forward_arg__ + and self.__forward_module__ == other.__forward_module__ + and self.__forward_is_class__ == other.__forward_is_class__ + and self.__code__ == other.__code__ + and self.__ast_node__ == other.__ast_node__ + # Use "is" here because we use id() for this in __hash__ + # because dictionaries are not hashable. + and self.__globals__ is other.__globals__ + and self.__cell__ == other.__cell__ + and self.__owner__ == other.__owner__ + ) + + def __hash__(self): + return hash(( + self.__forward_arg__, + self.__forward_module__, + self.__forward_is_class__, + self.__code__, + self.__ast_node__, + id(self.__globals__), # dictionaries are not hashable, so hash by identity + self.__cell__, + self.__owner__, + )) + def __or__(self, other): global _Union if _Union is None: @@ -232,11 +260,14 @@ def __ror__(self, other): return _Union[other, self] def __repr__(self): - if self.__forward_module__ is None: - module_repr = "" - else: - module_repr = f", module={self.__forward_module__!r}" - return f"ForwardRef({self.__forward_arg__!r}{module_repr})" + extra = [] + if self.__forward_module__ is not None: + extra.append(f", module={self.__forward_module__!r}") + if self.__forward_is_class__: + extra.append(", is_class=True") + if self.__owner__ is not None: + extra.append(f", owner={self.__owner__!r}") + return f"ForwardRef({self.__forward_arg__!r}{''.join(extra)})" class _Stringifier: diff --git a/Lib/test/support/__init__.py b/Lib/test/support/__init__.py index 88ac5a56ee928e..419de381662498 100644 --- a/Lib/test/support/__init__.py +++ b/Lib/test/support/__init__.py @@ -3,6 +3,7 @@ if __name__ != 'test.support': raise ImportError('support must be imported from the test package') +import annotationlib import contextlib import functools import inspect @@ -3012,3 +3013,44 @@ def is_libssl_fips_mode(): except ImportError: return False # more of a maybe, unless we add this to the _ssl module. return get_fips_mode() != 0 + + +class EqualToForwardRef: + """Helper to ease use of annotationlib.ForwardRef in tests. + + This checks only attributes that can be set using the constructor. + + """ + + def __init__( + self, + arg, + *, + module=None, + owner=None, + is_class=False, + ): + self.__forward_arg__ = arg + self.__forward_is_class__ = is_class + self.__forward_module__ = module + self.__owner__ = owner + + def __eq__(self, other): + if not isinstance(other, (EqualToForwardRef, annotationlib.ForwardRef)): + return NotImplemented + return ( + self.__forward_arg__ == other.__forward_arg__ + and self.__forward_module__ == other.__forward_module__ + and self.__forward_is_class__ == other.__forward_is_class__ + and self.__owner__ == other.__owner__ + ) + + def __repr__(self): + extra = [] + if self.__forward_module__ is not None: + extra.append(f", module={self.__forward_module__!r}") + if self.__forward_is_class__: + extra.append(", is_class=True") + if self.__owner__ is not None: + extra.append(f", owner={self.__owner__!r}") + return f"EqualToForwardRef({self.__forward_arg__!r}{''.join(extra)})" diff --git a/Lib/test/test_annotationlib.py b/Lib/test/test_annotationlib.py index 6851b36c7c13ff..9f2601a5751f85 100644 --- a/Lib/test/test_annotationlib.py +++ b/Lib/test/test_annotationlib.py @@ -32,11 +32,6 @@ def wrapper(a, b): return wrapper -def assert_is_fwdref(case, obj, value): - case.assertIsInstance(obj, annotationlib.ForwardRef) - case.assertEqual(obj.__forward_arg__, value) - - class MyClass: def __repr__(self): return "my repr" @@ -64,7 +59,8 @@ def inner(arg: x): anno = annotationlib.get_annotations(inner, format=Format.FORWARDREF) fwdref = anno["arg"] - assert_is_fwdref(self, fwdref, "x") + self.assertIsInstance(fwdref, annotationlib.ForwardRef) + self.assertEqual(fwdref.__forward_arg__, "x") with self.assertRaises(NameError): fwdref.evaluate() @@ -81,7 +77,8 @@ def f(x: int, y: doesntexist): anno = annotationlib.get_annotations(f, format=Format.FORWARDREF) self.assertIs(anno["x"], int) fwdref = anno["y"] - assert_is_fwdref(self, fwdref, "doesntexist") + self.assertIsInstance(fwdref, annotationlib.ForwardRef) + self.assertEqual(fwdref.__forward_arg__, "doesntexist") with self.assertRaises(NameError): fwdref.evaluate() self.assertEqual(fwdref.evaluate(globals={"doesntexist": 1}), 1) @@ -99,22 +96,28 @@ def f( anno = annotationlib.get_annotations(f, format=Format.FORWARDREF) x_anno = anno["x"] - assert_is_fwdref(self, x_anno, "some.module") + self.assertIsInstance(x_anno, ForwardRef) + self.assertEqual(x_anno, support.EqualToForwardRef("some.module", owner=f)) y_anno = anno["y"] - assert_is_fwdref(self, y_anno, "some[module]") + self.assertIsInstance(y_anno, ForwardRef) + self.assertEqual(y_anno, support.EqualToForwardRef("some[module]", owner=f)) z_anno = anno["z"] - assert_is_fwdref(self, z_anno, "some(module)") + self.assertIsInstance(z_anno, ForwardRef) + self.assertEqual(z_anno, support.EqualToForwardRef("some(module)", owner=f)) alpha_anno = anno["alpha"] - assert_is_fwdref(self, alpha_anno, "some | obj") + self.assertIsInstance(alpha_anno, ForwardRef) + self.assertEqual(alpha_anno, support.EqualToForwardRef("some | obj", owner=f)) beta_anno = anno["beta"] - assert_is_fwdref(self, beta_anno, "+some") + self.assertIsInstance(beta_anno, ForwardRef) + self.assertEqual(beta_anno, support.EqualToForwardRef("+some", owner=f)) gamma_anno = anno["gamma"] - assert_is_fwdref(self, gamma_anno, "some < obj") + self.assertIsInstance(gamma_anno, ForwardRef) + self.assertEqual(gamma_anno, support.EqualToForwardRef("some < obj", owner=f)) class TestSourceFormat(unittest.TestCase): @@ -359,6 +362,14 @@ def test_fwdref_to_builtin(self): obj = object() self.assertIs(ForwardRef("int").evaluate(globals={"int": obj}), obj) + def test_fwdref_value_is_not_cached(self): + fr = ForwardRef("hello") + with self.assertRaises(NameError): + fr.evaluate() + self.assertIs(fr.evaluate(globals={"hello": str}), str) + with self.assertRaises(NameError): + fr.evaluate() + def test_fwdref_with_owner(self): self.assertEqual( ForwardRef("Counter[int]", owner=collections).evaluate(), @@ -447,10 +458,12 @@ def f2(a: undefined): ) self.assertEqual(annotationlib.get_annotations(f1, format=1), {"a": int}) - for fmt in (Format.FORWARDREF, 3): - annos = annotationlib.get_annotations(f2, format=fmt) - self.assertEqual(list(annos), ["a"]) - assert_is_fwdref(self, annos["a"], "undefined") + fwd = support.EqualToForwardRef("undefined", owner=f2) + self.assertEqual( + annotationlib.get_annotations(f2, format=Format.FORWARDREF), + {"a": fwd}, + ) + self.assertEqual(annotationlib.get_annotations(f2, format=3), {"a": fwd}) self.assertEqual( annotationlib.get_annotations(f1, format=Format.STRING), @@ -1000,10 +1013,10 @@ def evaluate(format, exc=NotImplementedError): with self.assertRaises(NameError): annotationlib.call_evaluate_function(evaluate, Format.VALUE) - - fwdref = annotationlib.call_evaluate_function(evaluate, Format.FORWARDREF) - assert_is_fwdref(self, fwdref, "undefined") - + self.assertEqual( + annotationlib.call_evaluate_function(evaluate, Format.FORWARDREF), + support.EqualToForwardRef("undefined"), + ) self.assertEqual( annotationlib.call_evaluate_function(evaluate, Format.STRING), "undefined", diff --git a/Lib/test/test_inspect/test_inspect.py b/Lib/test/test_inspect/test_inspect.py index 8ca765b17e408c..6c2d9925e71fce 100644 --- a/Lib/test/test_inspect/test_inspect.py +++ b/Lib/test/test_inspect/test_inspect.py @@ -1,4 +1,4 @@ -from annotationlib import Format, ForwardRef +from annotationlib import Format import asyncio import builtins import collections @@ -37,7 +37,7 @@ from test.support import cpython_only, import_helper from test.support import MISSING_C_DOCSTRINGS, ALWAYS_EQ -from test.support import run_no_yield_async_fn +from test.support import run_no_yield_async_fn, EqualToForwardRef from test.support.import_helper import DirsOnSysPath, ready_to_import from test.support.os_helper import TESTFN, temp_cwd from test.support.script_helper import assert_python_ok, assert_python_failure, kill_python @@ -48,7 +48,6 @@ from test.test_inspect import inspect_fodder2 as mod2 from test.test_inspect import inspect_stringized_annotations from test.test_inspect import inspect_deferred_annotations -from test.test_typing import EqualToForwardRef # Functions tested in this suite: @@ -4932,9 +4931,12 @@ def test_signature_annotation_format(self): signature_func(ida.f, annotation_format=Format.STRING), sig([par("x", PORK, annotation="undefined")]) ) + s1 = signature_func(ida.f, annotation_format=Format.FORWARDREF) + s2 = sig([par("x", PORK, annotation=EqualToForwardRef("undefined", owner=ida.f))]) + #breakpoint() self.assertEqual( signature_func(ida.f, annotation_format=Format.FORWARDREF), - sig([par("x", PORK, annotation=EqualToForwardRef("undefined"))]) + sig([par("x", PORK, annotation=EqualToForwardRef("undefined", owner=ida.f))]) ) with self.assertRaisesRegex(NameError, "undefined"): signature_func(ida.f, annotation_format=Format.VALUE) diff --git a/Lib/test/test_typing.py b/Lib/test/test_typing.py index 20587add028b01..bffdd7277f97d6 100644 --- a/Lib/test/test_typing.py +++ b/Lib/test/test_typing.py @@ -48,7 +48,10 @@ import warnings import types -from test.support import captured_stderr, cpython_only, infinite_recursion, requires_docstrings, import_helper, run_code +from test.support import ( + captured_stderr, cpython_only, infinite_recursion, requires_docstrings, import_helper, run_code, + EqualToForwardRef, +) from test.typinganndata import ann_module695, mod_generics_cache, _typed_dict_helper @@ -76,17 +79,6 @@ def wrapper(self): return wrapper -class EqualToForwardRef: - def __init__(self, arg, module=None): - self.arg = arg - self.module = module - - def __eq__(self, other): - if not isinstance(other, ForwardRef): - return NotImplemented - return self.arg == other.__forward_arg__ and self.module == other.__forward_module__ - - class Employee: pass @@ -5194,15 +5186,12 @@ class Node(Generic[T]): ... Callable[..., T], Callable[[int], int], Tuple[Any, Any], Node[T], Node[int], Node[Any], typing.Iterable[T], typing.Iterable[Any], typing.Iterable[int], typing.Dict[int, str], - typing.Dict[T, Any], ClassVar[int], ClassVar[List[T]]] + typing.Dict[T, Any], ClassVar[int], ClassVar[List[T]], Tuple['T', 'T'], + Union['T', int], List['T'], typing.Mapping['T', int]] for t in things + [Any]: self.assertEqual(t, copy(t)) self.assertEqual(t, deepcopy(t)) - shallow_things = [Tuple['T', 'T'], Union['T', int], List['T'], typing.Mapping['T', int]] - for t in things + [Any]: - self.assertEqual(t, copy(t)) - def test_immutability_by_copy_and_pickle(self): # Special forms like Union, Any, etc., generic aliases to containers like List, # Mapping, etc., and type variabcles are considered immutable by copy and pickle. @@ -6101,6 +6090,82 @@ def test_forwardref_only_str_arg(self): with self.assertRaises(TypeError): typing.ForwardRef(1) # only `str` type is allowed + def test_forward_equality(self): + fr = typing.ForwardRef('int') + self.assertEqual(fr, typing.ForwardRef('int')) + self.assertNotEqual(List['int'], List[int]) + self.assertNotEqual(fr, typing.ForwardRef('int', module=__name__)) + frm = typing.ForwardRef('int', module=__name__) + self.assertEqual(frm, typing.ForwardRef('int', module=__name__)) + self.assertNotEqual(frm, typing.ForwardRef('int', module='__other_name__')) + + def test_forward_equality_gth(self): + c1 = typing.ForwardRef('C') + c1_gth = typing.ForwardRef('C') + c2 = typing.ForwardRef('C') + c2_gth = typing.ForwardRef('C') + + class C: + pass + def foo(a: c1_gth, b: c2_gth): + pass + + self.assertEqual(get_type_hints(foo, globals(), locals()), {'a': C, 'b': C}) + self.assertEqual(c1, c2) + self.assertEqual(c1, c1_gth) + self.assertEqual(c1_gth, c2_gth) + self.assertEqual(List[c1], List[c1_gth]) + self.assertNotEqual(List[c1], List[C]) + self.assertNotEqual(List[c1_gth], List[C]) + self.assertEqual(Union[c1, c1_gth], Union[c1]) + self.assertEqual(Union[c1, c1_gth, int], Union[c1, int]) + + def test_forward_equality_hash(self): + c1 = typing.ForwardRef('int') + c1_gth = typing.ForwardRef('int') + c2 = typing.ForwardRef('int') + c2_gth = typing.ForwardRef('int') + + def foo(a: c1_gth, b: c2_gth): + pass + get_type_hints(foo, globals(), locals()) + + self.assertEqual(hash(c1), hash(c2)) + self.assertEqual(hash(c1_gth), hash(c2_gth)) + self.assertEqual(hash(c1), hash(c1_gth)) + + c3 = typing.ForwardRef('int', module=__name__) + c4 = typing.ForwardRef('int', module='__other_name__') + + self.assertNotEqual(hash(c3), hash(c1)) + self.assertNotEqual(hash(c3), hash(c1_gth)) + self.assertNotEqual(hash(c3), hash(c4)) + self.assertEqual(hash(c3), hash(typing.ForwardRef('int', module=__name__))) + + def test_forward_equality_namespace(self): + class A: + pass + def namespace1(): + a = typing.ForwardRef('A') + def fun(x: a): + pass + get_type_hints(fun, globals(), locals()) + return a + + def namespace2(): + a = typing.ForwardRef('A') + + class A: + pass + def fun(x: a): + pass + + get_type_hints(fun, globals(), locals()) + return a + + self.assertEqual(namespace1(), namespace1()) + self.assertEqual(namespace1(), namespace2()) + def test_forward_repr(self): self.assertEqual(repr(List['int']), "typing.List[ForwardRef('int')]") self.assertEqual(repr(List[ForwardRef('int', module='mod')]), @@ -6157,14 +6222,10 @@ def fun(x: a): pass ret = get_type_hints(fun, globals(), locals()) return a - def cmp(o1, o2): - return o1 == o2 - - with infinite_recursion(25): - r1 = namespace1() - r2 = namespace2() - self.assertIsNot(r1, r2) - self.assertNotEqual(r1, r2) + r1 = namespace1() + r2 = namespace2() + self.assertIsNot(r1, r2) + self.assertEqual(r1, r2) def test_union_forward_recursion(self): ValueList = List['Value'] @@ -7084,7 +7145,8 @@ def func(x: undefined) -> undefined: ... # FORWARDREF self.assertEqual( get_type_hints(func, format=annotationlib.Format.FORWARDREF), - {'x': EqualToForwardRef('undefined'), 'return': EqualToForwardRef('undefined')}, + {'x': EqualToForwardRef('undefined', owner=func), + 'return': EqualToForwardRef('undefined', owner=func)}, ) # STRING diff --git a/Misc/NEWS.d/next/Library/2025-01-29-21-29-46.gh-issue-129463.qePexX.rst b/Misc/NEWS.d/next/Library/2025-01-29-21-29-46.gh-issue-129463.qePexX.rst index 497b5e997038df..2dea03d2384578 100644 --- a/Misc/NEWS.d/next/Library/2025-01-29-21-29-46.gh-issue-129463.qePexX.rst +++ b/Misc/NEWS.d/next/Library/2025-01-29-21-29-46.gh-issue-129463.qePexX.rst @@ -1,4 +1,3 @@ -:class:`annotationlib.ForwardRef` objects no longer compare or hash equal -when they refer to the same string. The implementation of equality was -error-prone because it did not take all attributes of the -:class:`!ForwardRef` object into account. +The implementations of equality and hashing for :class:`annotationlib.ForwardRef` +now use all attributes on the object. Two :class:`!ForwardRef` objects +are equal only if all attributes are equal. From 2b26af0ccc117bc268fba7bb11858204235ccea1 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Mon, 3 Mar 2025 14:04:27 -0800 Subject: [PATCH 04/10] fix doctest --- Doc/whatsnew/3.14.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Doc/whatsnew/3.14.rst b/Doc/whatsnew/3.14.rst index eee9bc865357ee..64b607988bfe49 100644 --- a/Doc/whatsnew/3.14.rst +++ b/Doc/whatsnew/3.14.rst @@ -108,7 +108,7 @@ This example shows how these formats behave: ... NameError: name 'Undefined' is not defined >>> get_annotations(func, format=Format.FORWARDREF) - {'arg': ForwardRef('Undefined')} + {'arg': ForwardRef('Undefined', owner=)} >>> get_annotations(func, format=Format.STRING) {'arg': 'Undefined'} From fbb281f41dc1d3c1907335d11048bcd72705a26d Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Mon, 3 Mar 2025 14:44:48 -0800 Subject: [PATCH 05/10] update docs --- Doc/library/annotationlib.rst | 6 ------ 1 file changed, 6 deletions(-) diff --git a/Doc/library/annotationlib.rst b/Doc/library/annotationlib.rst index dcaff3d7fdbec5..6dcb0ccf97cc03 100644 --- a/Doc/library/annotationlib.rst +++ b/Doc/library/annotationlib.rst @@ -196,12 +196,6 @@ Classes annotation from which the forward reference derives, usually a function, class, or module. - .. important:: - - Once a :class:`~ForwardRef` instance has been evaluated, it caches - the evaluated value, and future calls to :meth:`evaluate` will return - the cached value, regardless of the parameters passed in. - .. versionadded:: 3.14 From d273c0583346eb8accb19dd7f20311a3b735e420 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Fri, 4 Apr 2025 10:24:45 -0700 Subject: [PATCH 06/10] fix test_types --- Lib/test/test_types.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_types.py b/Lib/test/test_types.py index f014f7e9ee08c9..350567ec7e990d 100644 --- a/Lib/test/test_types.py +++ b/Lib/test/test_types.py @@ -2,7 +2,7 @@ from test.support import ( run_with_locale, cpython_only, no_rerun, - MISSING_C_DOCSTRINGS, + MISSING_C_DOCSTRINGS, EqualToForwardRef, ) import collections.abc from collections import namedtuple, UserDict @@ -1089,7 +1089,13 @@ def test_instantiation(self): self.assertIs(int, types.UnionType[int]) self.assertIs(int, types.UnionType[int, int]) self.assertEqual(int | str, types.UnionType[int, str]) - self.assertEqual(int | typing.ForwardRef("str"), types.UnionType[int, "str"]) + + for obj in ( + int | typing.ForwardRef("str"), + typing.Union[int, "str"], + ): + self.assertIsInstance(obj, types.UnionType) + self.assertEqual(obj.__args__, (int, EqualToForwardRef("str"))) class MappingProxyTests(unittest.TestCase): From 34c8ad561cc2aea5ea35f8b5fac09fd45b8bb6f5 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Fri, 4 Apr 2025 10:25:32 -0700 Subject: [PATCH 07/10] fix test_inspect --- Lib/test/test_inspect/test_inspect.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_inspect/test_inspect.py b/Lib/test/test_inspect/test_inspect.py index 118fbe3d708e4a..daae990458d708 100644 --- a/Lib/test/test_inspect/test_inspect.py +++ b/Lib/test/test_inspect/test_inspect.py @@ -1,4 +1,4 @@ -from annotationlib import Format +from annotationlib import Format, ForwardRef import asyncio import builtins import collections From 46593f82b62f4d53a2cb25b69b84cc395bd050e2 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Fri, 4 Apr 2025 10:28:12 -0700 Subject: [PATCH 08/10] Nikita comment --- Lib/annotationlib.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Lib/annotationlib.py b/Lib/annotationlib.py index d9aa09d91dbce0..d62a98551e21d5 100644 --- a/Lib/annotationlib.py +++ b/Lib/annotationlib.py @@ -93,11 +93,9 @@ def evaluate(self, *, globals=None, locals=None, type_params=None, owner=None): """ if self.__cell__ is not None: try: - value = self.__cell__.cell_contents + return self.__cell__.cell_contents except ValueError: pass - else: - return value if owner is None: owner = self.__owner__ From e71eecf572f4e33cba6358290875509ed5a2fc4d Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Fri, 4 Apr 2025 11:17:59 -0700 Subject: [PATCH 09/10] add note --- Doc/library/typing.rst | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Doc/library/typing.rst b/Doc/library/typing.rst index d5870498fa35b9..84f77e8f206438 100644 --- a/Doc/library/typing.rst +++ b/Doc/library/typing.rst @@ -3449,7 +3449,9 @@ Introspection helpers .. versionadded:: 3.7.4 .. versionchanged:: 3.14 - This is now an alias for :class:`annotationlib.ForwardRef`. + This is now an alias for :class:`annotationlib.ForwardRef`. Several undocumented + behaviors of this class have been changed; for example, after a ``ForwardRef`` has + been evaluated, the evaluated value is no longer cached. .. function:: evaluate_forward_ref(forward_ref, *, owner=None, globals=None, locals=None, type_params=None, format=annotationlib.Format.VALUE) From 0bb5bc77d0813c51f707acb23cc9d5452c3521c0 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Fri, 4 Apr 2025 21:12:34 -0700 Subject: [PATCH 10/10] reorder --- Lib/annotationlib.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Lib/annotationlib.py b/Lib/annotationlib.py index d62a98551e21d5..d6243c8863610e 100644 --- a/Lib/annotationlib.py +++ b/Lib/annotationlib.py @@ -37,11 +37,11 @@ class Format(enum.IntEnum): "__forward_module__", "__weakref__", "__arg__", - "__ast_node__", - "__code__", "__globals__", - "__owner__", + "__code__", + "__ast_node__", "__cell__", + "__owner__", "__stringifier_dict__", ) @@ -77,9 +77,9 @@ def __init__( self.__forward_is_argument__ = is_argument self.__forward_is_class__ = is_class self.__forward_module__ = module + self.__globals__ = None self.__code__ = None self.__ast_node__ = None - self.__globals__ = None self.__cell__ = None self.__owner__ = owner @@ -221,12 +221,12 @@ def __eq__(self, other): return ( self.__forward_arg__ == other.__forward_arg__ and self.__forward_module__ == other.__forward_module__ - and self.__forward_is_class__ == other.__forward_is_class__ - and self.__code__ == other.__code__ - and self.__ast_node__ == other.__ast_node__ # Use "is" here because we use id() for this in __hash__ # because dictionaries are not hashable. and self.__globals__ is other.__globals__ + and self.__forward_is_class__ == other.__forward_is_class__ + and self.__code__ == other.__code__ + and self.__ast_node__ == other.__ast_node__ and self.__cell__ == other.__cell__ and self.__owner__ == other.__owner__ ) @@ -235,10 +235,10 @@ def __hash__(self): return hash(( self.__forward_arg__, self.__forward_module__, + id(self.__globals__), # dictionaries are not hashable, so hash by identity self.__forward_is_class__, self.__code__, self.__ast_node__, - id(self.__globals__), # dictionaries are not hashable, so hash by identity self.__cell__, self.__owner__, ))