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>
This commit is contained in:
Jelle Zijlstra 2024-09-23 12:06:19 -07:00 committed by GitHub
parent e7d465a607
commit 2e0d445364
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 35 additions and 15 deletions

View File

@ -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:

View File

@ -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")