Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Sep 23, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 19 additions & 13 deletions Lib/annotationlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,17 @@ class Format(enum.IntEnum):


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.
JelleZijlstra marked this conversation as resolved.
Show resolved Hide resolved

"""

__slots__ = _SLOTS

Expand All @@ -57,8 +67,6 @@ def __init__(
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}")
Expand All @@ -71,8 +79,8 @@ def __init__(
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):
Expand Down Expand Up @@ -115,6 +123,10 @@ def evaluate(self, *, globals=None, locals=None, type_params=None, owner=None):
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):
Expand All @@ -134,14 +146,8 @@ def evaluate(self, *, globals=None, locals=None, type_params=None, owner=None):
# 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:
Expand Down
18 changes: 16 additions & 2 deletions Lib/test/test_annotationlib.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""Tests for the annotations module."""

import annotationlib
import collections
import functools
import itertools
import pickle
Expand Down Expand Up @@ -278,11 +279,24 @@ class Gen[T]:
)

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")
Expand Down
Loading