Skip to content

bpo-45166: fixes get_type_hints failure on Final #28279

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

Merged
merged 8 commits into from
Sep 25, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
10 changes: 10 additions & 0 deletions Lib/test/ann_module5.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# Used by test_typing to verify that Final wrapped in ForwardRef works.

from __future__ import annotations

from typing import Final

name: Final[str] = "final"

class MyClass:
value: Final = 3000
7 changes: 7 additions & 0 deletions Lib/test/ann_module6.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# Tests that top-level ClassVar is not allowed

from __future__ import annotations
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interestingly, this test fails when you remove this import. So it still allows top-level ClassVar if they aren't strings.

If you'd like to forbid this, you probably need to create another issue/and or PR. This behavior is what typing currently does (since 3.7-3.10). Even if the behavior is wrong, we don't usually backport bugfixes that add extra restrictions (unless it's a security fix) because that usually breaks backwards compatibility.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, good catch! I've opened a new issue: https://bugs.python.org/issue45283
I will work on it after this one will be finished.

Thanks! 👍


from typing import ClassVar

wrong: ClassVar[int] = 1
18 changes: 17 additions & 1 deletion Lib/test/test_typing.py
Original file line number Diff line number Diff line change
Expand Up @@ -2975,7 +2975,7 @@ async def __aexit__(self, etype, eval, tb):

# Definitions needed for features introduced in Python 3.6

from test import ann_module, ann_module2, ann_module3
from test import ann_module, ann_module2, ann_module3, ann_module5, ann_module6
from typing import AsyncContextManager

class A:
Expand Down Expand Up @@ -3339,6 +3339,22 @@ class C(Generic[T]): pass
(Concatenate[int, P], int))
self.assertEqual(get_args(list | str), (list, str))

def test_forward_ref_and_final(self):
# https://bugs.python.org/issue45166
hints = get_type_hints(ann_module5)
self.assertEqual(hints, {'name': Final[str]})

hints = get_type_hints(ann_module5.MyClass)
self.assertEqual(hints, {'value': Final})

def test_top_level_class_var(self):
# https://bugs.python.org/issue45166
with self.assertRaisesRegex(
TypeError,
r'typing.ClassVar\[int\] is not valid as type argument',
):
get_type_hints(ann_module6)


class CollectionsAbcTests(BaseTestCase):

Expand Down
29 changes: 20 additions & 9 deletions Lib/typing.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ def _type_convert(arg, module=None):
return arg


def _type_check(arg, msg, is_argument=True, module=None):
def _type_check(arg, msg, is_argument=True, module=None, *, is_class=False):
"""Check that the argument is a type, and return it (internal helper).

As a special case, accept None and return type(None) instead. Also wrap strings
Expand All @@ -164,14 +164,16 @@ def _type_check(arg, msg, is_argument=True, module=None):
We append the repr() of the actual value (truncated to 100 chars).
"""
invalid_generic_forms = (Generic, Protocol)
if is_argument:
invalid_generic_forms = invalid_generic_forms + (ClassVar, Final)
if not is_class:
invalid_generic_forms += (ClassVar,)
if is_argument:
invalid_generic_forms += (Final,)
Comment on lines +167 to +170
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to self: there's a subtle change in the logic compared to the old code, but it's ok because is_argument defaulted to True in both ForwardRef and _type_check. Which means by default we should always add ClassVar too.


arg = _type_convert(arg, module=module)
if (isinstance(arg, _GenericAlias) and
arg.__origin__ in invalid_generic_forms):
raise TypeError(f"{arg} is not valid as type argument")
if arg in (Any, NoReturn):
if arg in (Any, NoReturn, Final):
return arg
if isinstance(arg, _SpecialForm) or arg in (Generic, Protocol):
raise TypeError(f"Plain {arg} is not valid as type argument")
Expand Down Expand Up @@ -662,9 +664,10 @@ class ForwardRef(_Final, _root=True):

__slots__ = ('__forward_arg__', '__forward_code__',
'__forward_evaluated__', '__forward_value__',
'__forward_is_argument__', '__forward_module__')
'__forward_is_argument__', '__forward_is_class__',
'__forward_module__')

def __init__(self, arg, is_argument=True, module=None):
def __init__(self, arg, is_argument=True, module=None, *, is_class=False):
if not isinstance(arg, str):
raise TypeError(f"Forward reference must be a string -- got {arg!r}")
try:
Expand All @@ -676,6 +679,7 @@ def __init__(self, arg, is_argument=True, module=None):
self.__forward_evaluated__ = False
self.__forward_value__ = None
self.__forward_is_argument__ = is_argument
self.__forward_is_class__ = is_class
self.__forward_module__ = module

def _evaluate(self, globalns, localns, recursive_guard):
Expand All @@ -692,10 +696,11 @@ def _evaluate(self, globalns, localns, recursive_guard):
globalns = getattr(
sys.modules.get(self.__forward_module__, None), '__dict__', globalns
)
type_ =_type_check(
type_ = _type_check(
eval(self.__forward_code__, globalns, localns),
"Forward references must evaluate to types.",
is_argument=self.__forward_is_argument__,
is_class=self.__forward_is_class__,
)
self.__forward_value__ = _eval_type(
type_, globalns, localns, recursive_guard | {self.__forward_arg__}
Expand Down Expand Up @@ -1799,7 +1804,7 @@ def get_type_hints(obj, globalns=None, localns=None, include_extras=False):
if value is None:
value = type(None)
if isinstance(value, str):
value = ForwardRef(value, is_argument=False)
value = ForwardRef(value, is_argument=False, is_class=True)
value = _eval_type(value, base_globals, base_locals)
hints[name] = value
return hints if include_extras else {k: _strip_annotations(t) for k, t in hints.items()}
Expand Down Expand Up @@ -1831,7 +1836,13 @@ def get_type_hints(obj, globalns=None, localns=None, include_extras=False):
if value is None:
value = type(None)
if isinstance(value, str):
value = ForwardRef(value)
# class-level forward refs were handled above, this must be either
# a module-level annotation or a function argument annotation
value = ForwardRef(
value,
is_argument=not isinstance(obj, types.ModuleType),
is_class=False,
)
value = _eval_type(value, globalns, localns)
if name in defaults and defaults[name] is None:
value = Optional[value]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
:func:`typing.get_type_hints` now works with :data:`~typing.Final` wrapped in
:class:`~typing.ForwardRef`.