gh-119180: Fix annotationlib.ForwardRef.evaluate with no globals (#124326)

We were sometimes passing None as the globals argument to eval(), which makes it
inherit the globals from the calling scope. Instead, ensure that globals is always
non-None. The test was passing accidentally because I passed "annotationlib" as a
module object; fix that. Also document the parameters to ForwardRef() and remove
two unused private ones.

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

diff --git a/Lib/annotationlib.py b/Lib/annotationlib.py
index 09a844ddb56f062bca96054fda21d0c94eeeae9f..0a67742a2b3081fb6d718fed365ee3c79c3f9556 100644 (file)
--- a/Lib/annotationlib.py
+++ b/Lib/annotationlib.py
@@ -45,7 +45,17 @@ _SLOTS = (
 
 
 class ForwardRef:
-    """Wrapper that holds a forward reference."""
+    """Wrapper that holds a forward reference.
+
+    Constructor arguments:
+    * arg: a string representing the code to be evaluated.
+    * module: the module where the forward reference was created.
+      Must be a string, not a module object.
+    * owner: The owning object (module, class, or function).
+    * is_argument: Does nothing, retained for compatibility.
+    * is_class: True if the forward reference was created in class scope.
+
+    """
 
     __slots__ = _SLOTS
 
@@ -57,8 +67,6 @@ class ForwardRef:
         owner=None,
         is_argument=True,
         is_class=False,
-        _globals=None,
-        _cell=None,
     ):
         if not isinstance(arg, str):
             raise TypeError(f"Forward reference must be a string -- got {arg!r}")
@@ -71,8 +79,8 @@ class ForwardRef:
         self.__forward_module__ = module
         self.__code__ = None
         self.__ast_node__ = None
-        self.__globals__ = _globals
-        self.__cell__ = _cell
+        self.__globals__ = None
+        self.__cell__ = None
         self.__owner__ = owner
 
     def __init_subclass__(cls, /, *args, **kwds):
@@ -115,6 +123,10 @@ class ForwardRef:
             elif callable(owner):
                 globals = getattr(owner, "__globals__", None)
 
+        # If we pass None to eval() below, the globals of this module are used.
+        if globals is None:
+            globals = {}
+
         if locals is None:
             locals = {}
             if isinstance(owner, type):
@@ -134,14 +146,8 @@ class ForwardRef:
         # but should in turn be overridden by names in the class scope
         # (which here are called `globalns`!)
         if type_params is not None:
-            if globals is None:
-                globals = {}
-            else:
-                globals = dict(globals)
-            if locals is None:
-                locals = {}
-            else:
-                locals = dict(locals)
+            globals = dict(globals)
+            locals = dict(locals)
             for param in type_params:
                 param_name = param.__name__
                 if not self.__forward_is_class__ or param_name not in globals:

diff --git a/Lib/test/test_annotationlib.py b/Lib/test/test_annotationlib.py
index 309f6d2120109a9972b8e54454b7d0471627a661..dd8ceb55a411fbf87936512a45f9339e59188d1f 100644 (file)
--- a/Lib/test/test_annotationlib.py
+++ b/Lib/test/test_annotationlib.py
@@ -1,6 +1,7 @@
 """Tests for the annotations module."""
 
 import annotationlib
+import collections
 import functools
 import itertools
 import pickle
@@ -278,11 +279,24 @@ class TestForwardRefClass(unittest.TestCase):
         )
 
     def test_fwdref_with_module(self):
-        self.assertIs(ForwardRef("Format", module=annotationlib).evaluate(), Format)
+        self.assertIs(ForwardRef("Format", module="annotationlib").evaluate(), Format)
+        self.assertIs(ForwardRef("Counter", module="collections").evaluate(), collections.Counter)
 
         with self.assertRaises(NameError):
             # If globals are passed explicitly, we don't look at the module dict
-            ForwardRef("Format", module=annotationlib).evaluate(globals={})
+            ForwardRef("Format", module="annotationlib").evaluate(globals={})
+
+    def test_fwdref_to_builtin(self):
+        self.assertIs(ForwardRef("int").evaluate(), int)
+        self.assertIs(ForwardRef("int", module="collections").evaluate(), int)
+        self.assertIs(ForwardRef("int", owner=str).evaluate(), int)
+
+        # builtins are still searched with explicit globals
+        self.assertIs(ForwardRef("int").evaluate(globals={}), int)
+
+        # explicit values in globals have precedence
+        obj = object()
+        self.assertIs(ForwardRef("int").evaluate(globals={"int": obj}), obj)
 
     def test_fwdref_value_is_cached(self):
         fr = ForwardRef("hello")