bpo-32960: For dataclasses, disallow inheriting frozen from non-frozen classes and vice-versa, (GH-5919)

This restriction will be relaxed at a future date.

3 files changed, 77 insertions, 29 deletions

diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py
index db92b10..54478fe 100644
--- a/Lib/dataclasses.py
+++ b/Lib/dataclasses.py
@@ -623,14 +623,21 @@ def _process_class(cls, repr, eq, order, unsafe_hash, init, frozen):
             else:
                 setattr(cls, f.name, f.default)
 
+    # We're inheriting from a frozen dataclass, but we're not frozen.
+    if cls.__setattr__ is _frozen_setattr and not frozen:
+        raise TypeError('cannot inherit non-frozen dataclass from a '
+                        'frozen one')
+
+    # We're inheriting from a non-frozen dataclass, but we're frozen.
+    if (hasattr(cls, _MARKER) and cls.__setattr__ is not _frozen_setattr
+        and frozen):
+        raise TypeError('cannot inherit frozen dataclass from a '
+                        'non-frozen one')
+
     # Remember all of the fields on our class (including bases).  This
     #  marks this class as being a dataclass.
     setattr(cls, _MARKER, fields)
 
-    # We also need to check if a parent class is frozen: frozen has to
-    #  be inherited down.
-    is_frozen = frozen or cls.__setattr__ is _frozen_setattr
-
     # Was this class defined with an explicit __hash__?  Note that if
     #  __eq__ is defined in this class, then python will automatically
     #  set __hash__ to None.  This is a heuristic, as it's possible
@@ -654,7 +661,7 @@ def _process_class(cls, repr, eq, order, unsafe_hash, init, frozen):
                 if f._field_type in (_FIELD, _FIELD_INITVAR)]
         _set_new_attribute(cls, '__init__',
                            _init_fn(flds,
-                                    is_frozen,
+                                    frozen,
                                     has_post_init,
                                     # The name to use for the "self" param
                                     #  in __init__.  Use "self" if possible.
@@ -696,7 +703,7 @@ def _process_class(cls, repr, eq, order, unsafe_hash, init, frozen):
                                 f'in class {cls.__name__}. Consider using '
                                 'functools.total_ordering')
 
-    if is_frozen:
+    if frozen:
         for name, fn in [('__setattr__', _frozen_setattr),
                          ('__delattr__', _frozen_delattr)]:
             if _set_new_attribute(cls, name, fn):
diff --git a/Lib/test/test_dataclasses.py b/Lib/test/test_dataclasses.py
index 582cb34..46d485c 100755
--- a/Lib/test/test_dataclasses.py
+++ b/Lib/test/test_dataclasses.py
@@ -637,29 +637,6 @@ class TestCase(unittest.TestCase):
             y: int
         self.assertNotEqual(Point(1, 3), C(1, 3))
 
-    def test_frozen(self):
-        @dataclass(frozen=True)
-        class C:
-            i: int
-
-        c = C(10)
-        self.assertEqual(c.i, 10)
-        with self.assertRaises(FrozenInstanceError):
-            c.i = 5
-        self.assertEqual(c.i, 10)
-
-        # Check that a derived class is still frozen, even if not
-        #  marked so.
-        @dataclass
-        class D(C):
-            pass
-
-        d = D(20)
-        self.assertEqual(d.i, 20)
-        with self.assertRaises(FrozenInstanceError):
-            d.i = 5
-        self.assertEqual(d.i, 20)
-
     def test_not_tuple(self):
         # Test that some of the problems with namedtuple don't happen
         #  here.
@@ -2475,5 +2452,66 @@ class TestHash(unittest.TestCase):
                     assert False, f'unknown value for expected={expected!r}'
 
 
+class TestFrozen(unittest.TestCase):
+    def test_frozen(self):
+        @dataclass(frozen=True)
+        class C:
+            i: int
+
+        c = C(10)
+        self.assertEqual(c.i, 10)
+        with self.assertRaises(FrozenInstanceError):
+            c.i = 5
+        self.assertEqual(c.i, 10)
+
+    def test_inherit(self):
+        @dataclass(frozen=True)
+        class C:
+            i: int
+
+        @dataclass(frozen=True)
+        class D(C):
+            j: int
+
+        d = D(0, 10)
+        with self.assertRaises(FrozenInstanceError):
+            d.i = 5
+        self.assertEqual(d.i, 0)
+
+    def test_inherit_from_nonfrozen_from_frozen(self):
+        @dataclass(frozen=True)
+        class C:
+            i: int
+
+        with self.assertRaisesRegex(TypeError,
+                                    'cannot inherit non-frozen dataclass from a frozen one'):
+            @dataclass
+            class D(C):
+                pass
+
+    def test_inherit_from_frozen_from_nonfrozen(self):
+        @dataclass
+        class C:
+            i: int
+
+        with self.assertRaisesRegex(TypeError,
+                                    'cannot inherit frozen dataclass from a non-frozen one'):
+            @dataclass(frozen=True)
+            class D(C):
+                pass
+
+    def test_inherit_from_normal_class(self):
+        class C:
+            pass
+
+        @dataclass(frozen=True)
+        class D(C):
+            i: int
+
+        d = D(10)
+        with self.assertRaises(FrozenInstanceError):
+            d.i = 5
+
+
 if __name__ == '__main__':
     unittest.main()
diff --git a/Misc/NEWS.d/next/Library/2018-02-26-20-04-40.bpo-32960.48r0Ml.rst b/Misc/NEWS.d/next/Library/2018-02-26-20-04-40.bpo-32960.48r0Ml.rst
new file mode 100644
index 0000000..4ad1fa1
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2018-02-26-20-04-40.bpo-32960.48r0Ml.rst
@@ -0,0 +1,3 @@
+For dataclasses, disallow inheriting frozen from non-frozen classes, and
+also disallow inheriting non-frozen from frozen classes. This restriction
+will be relaxed at a future date.