From 1f3009f663994e8521f4e7fc7522d34531bad0d8 Mon Sep 17 00:00:00 2001 From: "Eric V. Smith" Date: Thu, 22 Mar 2018 07:57:20 -0400 Subject: [PATCH 1/3] First crack at detecting Fields without annotations. --- Lib/dataclasses.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index d61643249148899..aef73892b2b3eff 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -684,6 +684,11 @@ def _process_class(cls, init, repr, eq, order, unsafe_hash, frozen): else: setattr(cls, f.name, f.default) + # Do we have any Field members that are not annotations? + for name, value in cls.__dict__.items(): + if isinstance(value, Field) and not name in cls.__dict__.get('__annotations__', {}): + print(f'{name} is a field with no annotation') + # Check rules that apply if we are derived from any dataclasses. if has_dataclass_bases: # Raise an exception if any of our bases are frozen, but we're not. From 4cc1fd1ce4a43a3e6add720e3212fe3696f4baff Mon Sep 17 00:00:00 2001 From: "Eric V. Smith" Date: Thu, 22 Mar 2018 15:59:45 -0400 Subject: [PATCH 2/3] Raise TypeError if a member of type Field doesn't have a type annotation. Reuse the class's annotations. Add tests. --- Lib/dataclasses.py | 39 ++++++++++++++++---------------- Lib/test/test_dataclasses.py | 43 ++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 20 deletions(-) diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index aef73892b2b3eff..d5d18e317e2b0fc 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -572,22 +572,6 @@ def _get_field(cls, a_name, a_type): return f -def _find_fields(cls): - # Return a list of Field objects, in order, for this class (and no - # base classes). Fields are found from the class dict's - # __annotations__ (which is guaranteed to be ordered). Default - # values are from class attributes, if a field has a default. If - # the default value is a Field(), then it contains additional - # info beyond (and possibly including) the actual default value. - # Pseudo-fields ClassVars and InitVars are included, despite the - # fact that they're not real fields. That's dealt with later. - - # If __annotations__ isn't present, then this class adds no new - # annotations. - annotations = cls.__dict__.get('__annotations__', {}) - return [_get_field(cls, name, type) for name, type in annotations.items()] - - def _set_new_attribute(cls, name, value): # Never overwrites an existing attribute. Returns True if the # attribute already exists. @@ -662,10 +646,25 @@ def _process_class(cls, init, repr, eq, order, unsafe_hash, frozen): if getattr(b, _PARAMS).frozen: any_frozen_base = True + # Annotations that are defined in this class (not in base + # classes). If __annotations__ isn't present, then this class + # adds no new annotations. We use this to compute fields that + # are added by this class. + # Fields are found from cls_annotations, which is guaranteed to be + # ordered. Default values are from class attributes, if a field + # has a default. If the default value is a Field(), then it + # contains additional info beyond (and possibly including) the + # actual default value. Pseudo-fields ClassVars and InitVars are + # included, despite the fact that they're not real fields. + # That's dealt with later. + cls_annotations = cls.__dict__.get('__annotations__', {}) + # Now find fields in our class. While doing so, validate some # things, and set the default values (as class attributes) # where we can. - for f in _find_fields(cls): + cls_fields = [_get_field(cls, name, type) + for name, type in cls_annotations.items()] + for f in cls_fields: fields[f.name] = f # If the class attribute (which is the default value for @@ -684,10 +683,10 @@ def _process_class(cls, init, repr, eq, order, unsafe_hash, frozen): else: setattr(cls, f.name, f.default) - # Do we have any Field members that are not annotations? + # Do we have any Field members that don't also have annotations? for name, value in cls.__dict__.items(): - if isinstance(value, Field) and not name in cls.__dict__.get('__annotations__', {}): - print(f'{name} is a field with no annotation') + if isinstance(value, Field) and not name in cls_annotations: + raise TypeError(f'{name!r} is a field but has no type annotation') # Check rules that apply if we are derived from any dataclasses. if has_dataclass_bases: diff --git a/Lib/test/test_dataclasses.py b/Lib/test/test_dataclasses.py index 9b5aad25745f7ff..38b6855b0487124 100755 --- a/Lib/test/test_dataclasses.py +++ b/Lib/test/test_dataclasses.py @@ -23,6 +23,14 @@ class C: o = C() self.assertEqual(len(fields(C)), 0) + def test_no_fields_but_member_variable(self): + @dataclass + class C: + i = 0 + + o = C() + self.assertEqual(len(fields(C)), 0) + def test_one_field_no_default(self): @dataclass class C: @@ -1905,6 +1913,41 @@ def test_helper_make_dataclass_no_types(self): 'z': 'typing.Any'}) +class TestFieldNoAnnotation(unittest.TestCase): + def test_field_without_annotation(self): + with self.assertRaisesRegex(TypeError, + "'f' is a field but has no type annotation"): + @dataclass + class C: + f = field() + + def test_field_without_annotation_but_annotation_in_base(self): + @dataclass + class B: + f: int + + with self.assertRaisesRegex(TypeError, + "'f' is a field but has no type annotation"): + # This is still an error: make sure we don't pick up the + # type annotation in the base class. + @dataclass + class C(B): + f = field() + + def test_field_without_annotation_but_annotation_in_base_not_dataclass(self): + # Same test, but with the base class not a dataclass. + class B: + f: int + + with self.assertRaisesRegex(TypeError, + "'f' is a field but has no type annotation"): + # This is still an error: make sure we don't pick up the + # type annotation in the base class. + @dataclass + class C(B): + f = field() + + class TestDocString(unittest.TestCase): def assertDocStrEqual(self, a, b): # Because 3.6 and 3.7 differ in how inspect.signature work From 35825f037fa6273d298117bed2ca01d16140d351 Mon Sep 17 00:00:00 2001 From: "Eric V. Smith" Date: Thu, 22 Mar 2018 16:06:02 -0400 Subject: [PATCH 3/3] Add blurb. --- .../next/Library/2018-03-22-16-05-56.bpo-32505.YK1N8v.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2018-03-22-16-05-56.bpo-32505.YK1N8v.rst diff --git a/Misc/NEWS.d/next/Library/2018-03-22-16-05-56.bpo-32505.YK1N8v.rst b/Misc/NEWS.d/next/Library/2018-03-22-16-05-56.bpo-32505.YK1N8v.rst new file mode 100644 index 000000000000000..91e97bf53f658bb --- /dev/null +++ b/Misc/NEWS.d/next/Library/2018-03-22-16-05-56.bpo-32505.YK1N8v.rst @@ -0,0 +1,2 @@ +Raise TypeError if a member variable of a dataclass is of type Field, but +doesn't have a type annotation.