Skip to content

Commit 4144062

Browse files
taminomaradzherb
andauthored
gh-133956 fix bug where dataclass wouldn't detect ClassVar fields if ClassVar was re-exported from a module other than typing (#140541)
Co-authored-by: Dmitrii Zherbin <zherbin.dima@yandex.ru>
1 parent 68fe899 commit 4144062

8 files changed

Lines changed: 201 additions & 44 deletions

File tree

Lib/dataclasses.py

Lines changed: 30 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -757,22 +757,16 @@ def _is_kw_only(a_type, dataclasses):
757757
return a_type is dataclasses.KW_ONLY
758758

759759

760-
def _is_type(annotation, cls, a_module, a_type, is_type_predicate):
761-
# Given a type annotation string, does it refer to a_type in
762-
# a_module? For example, when checking that annotation denotes a
763-
# ClassVar, then a_module is typing, and a_type is
764-
# typing.ClassVar.
760+
def _get_type_from_annotation(annotation, cls):
761+
# Loosely parse a string annotation and return its type.
765762

766-
# It's possible to look up a_module given a_type, but it involves
767-
# looking in sys.modules (again!), and seems like a waste since
768-
# the caller already knows a_module.
763+
# We can't perform a full type hint evaluation at the point where @dataclass
764+
# was invoked because class's module is not fully initialized yet. So we resort
765+
# to parsing string annotation using regexp, and extracting a type before
766+
# the first square bracket.
769767

770768
# - annotation is a string type annotation
771769
# - cls is the class that this annotation was found in
772-
# - a_module is the module we want to match
773-
# - a_type is the type in that module we want to match
774-
# - is_type_predicate is a function called with (obj, a_module)
775-
# that determines if obj is of the desired type.
776770

777771
# Since this test does not do a local namespace lookup (and
778772
# instead only a module (global) lookup), there are some things it
@@ -803,24 +797,21 @@ def _is_type(annotation, cls, a_module, a_type, is_type_predicate):
803797
# https://github.com/python/cpython/issues/77634 for details.
804798
global _MODULE_IDENTIFIER_RE
805799
if _MODULE_IDENTIFIER_RE is None:
806-
_MODULE_IDENTIFIER_RE = re.compile(r'(?:\s*(\w+)\s*\.)?\s*(\w+)')
800+
_MODULE_IDENTIFIER_RE = re.compile(r'^\s*(\w+(?:\s*\.\s*\w+)*)')
807801

808802
match = _MODULE_IDENTIFIER_RE.prefixmatch(annotation)
809-
if match:
810-
ns = None
811-
module_name = match[1]
812-
if not module_name:
813-
# No module name, assume the class's module did
814-
# "from dataclasses import InitVar".
815-
ns = sys.modules.get(cls.__module__).__dict__
816-
else:
817-
# Look up module_name in the class's module.
818-
module = sys.modules.get(cls.__module__)
819-
if module and module.__dict__.get(module_name) is a_module:
820-
ns = sys.modules.get(a_type.__module__).__dict__
821-
if ns and is_type_predicate(ns.get(match[2]), a_module):
822-
return True
823-
return False
803+
if not match:
804+
return None
805+
806+
# Note: _MODULE_IDENTIFIER_RE guarantees that path is non-empty
807+
path = match[1].split(".")
808+
root = sys.modules.get(cls.__module__)
809+
for path_item in path:
810+
root = getattr(root, path_item.strip(), None)
811+
if root is None:
812+
return None
813+
814+
return root
824815

825816

826817
def _get_field(cls, a_name, a_type, default_kw_only):
@@ -858,17 +849,18 @@ def _get_field(cls, a_name, a_type, default_kw_only):
858849
# is actually of the correct type.
859850

860851
# For the complete discussion, see https://bugs.python.org/issue33453
852+
if isinstance(a_type, str):
853+
a_type_annotation = _get_type_from_annotation(a_type, cls)
854+
else:
855+
a_type_annotation = a_type
861856

862857
# If typing has not been imported, then it's impossible for any
863858
# annotation to be a ClassVar. So, only look for ClassVar if
864859
# typing has been imported by any module (not necessarily cls's
865860
# module).
866861
typing = sys.modules.get('typing')
867862
if typing:
868-
if (_is_classvar(a_type, typing)
869-
or (isinstance(f.type, str)
870-
and _is_type(f.type, cls, typing, typing.ClassVar,
871-
_is_classvar))):
863+
if _is_classvar(a_type_annotation, typing):
872864
f._field_type = _FIELD_CLASSVAR
873865

874866
# If the type is InitVar, or if it's a matching string annotation,
@@ -877,10 +869,7 @@ def _get_field(cls, a_name, a_type, default_kw_only):
877869
# The module we're checking against is the module we're
878870
# currently in (dataclasses.py).
879871
dataclasses = sys.modules[__name__]
880-
if (_is_initvar(a_type, dataclasses)
881-
or (isinstance(f.type, str)
882-
and _is_type(f.type, cls, dataclasses, dataclasses.InitVar,
883-
_is_initvar))):
872+
if _is_initvar(a_type_annotation, dataclasses):
884873
f._field_type = _FIELD_INITVAR
885874

886875
# Validations for individual fields. This is delayed until now,
@@ -1073,10 +1062,11 @@ def _process_class(cls, init, repr, eq, order, unsafe_hash, frozen,
10731062
dataclasses = sys.modules[__name__]
10741063
for name, type in cls_annotations.items():
10751064
# See if this is a marker to change the value of kw_only.
1076-
if (_is_kw_only(type, dataclasses)
1077-
or (isinstance(type, str)
1078-
and _is_type(type, cls, dataclasses, dataclasses.KW_ONLY,
1079-
_is_kw_only))):
1065+
if isinstance(type, str):
1066+
a_type_annotation = _get_type_from_annotation(type, cls)
1067+
else:
1068+
a_type_annotation = type
1069+
if _is_kw_only(a_type_annotation, dataclasses):
10801070
# Switch the default to kw_only=True, and ignore this
10811071
# annotation: it's not a real field.
10821072
if KW_ONLY_seen:

Lib/test/test_dataclasses/__init__.py

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4353,10 +4353,17 @@ def test_classvar_module_level_import(self):
43534353
from test.test_dataclasses import dataclass_module_1_str
43544354
from test.test_dataclasses import dataclass_module_2
43554355
from test.test_dataclasses import dataclass_module_2_str
4356-
4357-
for m in (dataclass_module_1, dataclass_module_1_str,
4358-
dataclass_module_2, dataclass_module_2_str,
4359-
):
4356+
from test.test_dataclasses import dataclass_module_3
4357+
from test.test_dataclasses import dataclass_module_3_str
4358+
from test.test_dataclasses import dataclass_module_4
4359+
from test.test_dataclasses import dataclass_module_4_str
4360+
4361+
for m in (
4362+
dataclass_module_1, dataclass_module_1_str,
4363+
dataclass_module_2, dataclass_module_2_str,
4364+
dataclass_module_3, dataclass_module_3_str,
4365+
dataclass_module_4, dataclass_module_4_str,
4366+
):
43604367
with self.subTest(m=m):
43614368
# There's a difference in how the ClassVars are
43624369
# interpreted when using string annotations or
@@ -4660,6 +4667,14 @@ def custom_dataclass(cls, *args, **kwargs):
46604667
self.assertEqual(c.x, 10)
46614668
self.assertEqual(c.__custom__, True)
46624669

4670+
def test_empty_annotation_string(self):
4671+
@dataclass
4672+
class DataclassWithEmptyTypeAnnotation:
4673+
x: ""
4674+
4675+
c = DataclassWithEmptyTypeAnnotation(10)
4676+
self.assertEqual(c.x, 10)
4677+
46634678

46644679
class TestReplace(unittest.TestCase):
46654680
def test(self):
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
# We need this to test a case when a type
2+
# is imported via some other package,
3+
# like ClassVar from typing_extensions instead of typing.
4+
# https://github.com/python/cpython/issues/133956
5+
from typing import ClassVar
6+
from dataclasses import InitVar
7+
8+
__all__ = ["ClassVar", "InitVar"]
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
#from __future__ import annotations
2+
USING_STRINGS = False
3+
4+
# dataclass_module_3.py and dataclass_module_3_str.py are identical
5+
# except only the latter uses string annotations.
6+
7+
from dataclasses import dataclass
8+
import test.test_dataclasses._types_proxy as tp
9+
10+
T_CV2 = tp.ClassVar[int]
11+
T_CV3 = tp.ClassVar
12+
13+
T_IV2 = tp.InitVar[int]
14+
T_IV3 = tp.InitVar
15+
16+
@dataclass
17+
class CV:
18+
T_CV4 = tp.ClassVar
19+
cv0: tp.ClassVar[int] = 20
20+
cv1: tp.ClassVar = 30
21+
cv2: T_CV2
22+
cv3: T_CV3
23+
not_cv4: T_CV4 # When using string annotations, this field is not recognized as a ClassVar.
24+
25+
@dataclass
26+
class IV:
27+
T_IV4 = tp.InitVar
28+
iv0: tp.InitVar[int]
29+
iv1: tp.InitVar
30+
iv2: T_IV2
31+
iv3: T_IV3
32+
not_iv4: T_IV4 # When using string annotations, this field is not recognized as an InitVar.
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
from __future__ import annotations
2+
USING_STRINGS = True
3+
4+
# dataclass_module_3.py and dataclass_module_3_str.py are identical
5+
# except only the latter uses string annotations.
6+
7+
from dataclasses import dataclass
8+
import test.test_dataclasses._types_proxy as tp
9+
10+
T_CV2 = tp.ClassVar[int]
11+
T_CV3 = tp.ClassVar
12+
13+
T_IV2 = tp.InitVar[int]
14+
T_IV3 = tp.InitVar
15+
16+
@dataclass
17+
class CV:
18+
T_CV4 = tp.ClassVar
19+
cv0: tp.ClassVar[int] = 20
20+
cv1: tp.ClassVar = 30
21+
cv2: T_CV2
22+
cv3: T_CV3
23+
not_cv4: T_CV4 # When using string annotations, this field is not recognized as a ClassVar.
24+
25+
@dataclass
26+
class IV:
27+
T_IV4 = tp.InitVar
28+
iv0: tp.InitVar[int]
29+
iv1: tp.InitVar
30+
iv2: T_IV2
31+
iv3: T_IV3
32+
not_iv4: T_IV4 # When using string annotations, this field is not recognized as an InitVar.
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
#from __future__ import annotations
2+
USING_STRINGS = False
3+
4+
# dataclass_module_4.py and dataclass_module_4_str.py are identical
5+
# except only the latter uses string annotations.
6+
7+
from dataclasses import dataclass
8+
import dataclasses
9+
import typing
10+
11+
class TypingProxy:
12+
class Nested:
13+
ClassVar = typing.ClassVar
14+
InitVar = dataclasses.InitVar
15+
16+
T_CV2 = TypingProxy.Nested.ClassVar[int]
17+
T_CV3 = TypingProxy.Nested.ClassVar
18+
19+
T_IV2 = TypingProxy.Nested.InitVar[int]
20+
T_IV3 = TypingProxy.Nested.InitVar
21+
22+
@dataclass
23+
class CV:
24+
T_CV4 = TypingProxy.Nested.ClassVar
25+
cv0: TypingProxy.Nested.ClassVar[int] = 20
26+
cv1: TypingProxy.Nested.ClassVar = 30
27+
cv2: T_CV2
28+
cv3: T_CV3
29+
not_cv4: T_CV4 # When using string annotations, this field is not recognized as a ClassVar.
30+
31+
@dataclass
32+
class IV:
33+
T_IV4 = TypingProxy.Nested.InitVar
34+
iv0: TypingProxy.Nested.InitVar[int]
35+
iv1: TypingProxy.Nested.InitVar
36+
iv2: T_IV2
37+
iv3: T_IV3
38+
not_iv4: T_IV4 # When using string annotations, this field is not recognized as an InitVar.
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
from __future__ import annotations
2+
USING_STRINGS = True
3+
4+
# dataclass_module_4.py and dataclass_module_4_str.py are identical
5+
# except only the latter uses string annotations.
6+
7+
from dataclasses import dataclass
8+
import dataclasses
9+
import typing
10+
11+
class TypingProxy:
12+
class Nested:
13+
ClassVar = typing.ClassVar
14+
InitVar = dataclasses.InitVar
15+
16+
T_CV2 = TypingProxy.Nested.ClassVar[int]
17+
T_CV3 = TypingProxy.Nested.ClassVar
18+
19+
T_IV2 = TypingProxy.Nested.InitVar[int]
20+
T_IV3 = TypingProxy.Nested.InitVar
21+
22+
@dataclass
23+
class CV:
24+
T_CV4 = TypingProxy.Nested.ClassVar
25+
cv0: TypingProxy.Nested.ClassVar[int] = 20
26+
cv1: TypingProxy.Nested.ClassVar = 30
27+
cv2: T_CV2
28+
cv3: T_CV3
29+
not_cv4: T_CV4 # When using string annotations, this field is not recognized as a ClassVar.
30+
31+
@dataclass
32+
class IV:
33+
T_IV4 = TypingProxy.Nested.InitVar
34+
iv0: TypingProxy.Nested.InitVar[int]
35+
iv1: TypingProxy.Nested.InitVar
36+
iv2: T_IV2
37+
iv3: T_IV3
38+
not_iv4: T_IV4 # When using string annotations, this field is not recognized as an InitVar.
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Fix bug where :func:`@dataclass <dataclasses.dataclass>`
2+
wouldn't detect ``ClassVar`` fields
3+
if ``ClassVar`` was re-exported from a module
4+
other than :mod:`typing`.

0 commit comments

Comments
 (0)