Clean up issues with dataclasses and inheritance (#5046)

Ban overriding of inherited fields - it's very hard to
make this work so better just go give an error.

Note limitation that dataclass decorator can't see into
cimported base class. Doesn't fix but helps people
avoid #4799

Part of https://github.com/cython/cython/issues/4956

8 files changed, 88 insertions, 13 deletions

diff --git a/Cython/Compiler/Dataclass.py b/Cython/Compiler/Dataclass.py
index e202ed6eb..7cbbab954 100644
--- a/Cython/Compiler/Dataclass.py
+++ b/Cython/Compiler/Dataclass.py
@@ -200,10 +200,16 @@ def process_class_get_fields(node):
     transform(node)
     default_value_assignments = transform.removed_assignments
 
-    if node.base_type and node.base_type.dataclass_fields:
-        fields = node.base_type.dataclass_fields.copy()
-    else:
-        fields = OrderedDict()
+    base_type = node.base_type
+    fields = OrderedDict()
+    while base_type:
+        if base_type.is_external or not base_type.scope.implemented:
+            warning(node.pos, "Cannot reliably handle Cython dataclasses with base types "
+                "in external modules since it is not possible to tell what fields they have", 2)
+        if base_type.dataclass_fields:
+            fields = base_type.dataclass_fields.copy()
+            break
+        base_type = base_type.base_type
 
     for entry in var_entries:
         name = entry.name
diff --git a/Cython/Compiler/ExprNodes.py b/Cython/Compiler/ExprNodes.py
index 1ce459541..4f8207ee8 100644
--- a/Cython/Compiler/ExprNodes.py
+++ b/Cython/Compiler/ExprNodes.py
@@ -2171,6 +2171,9 @@ class NameNode(AtomicExprNode):
             # In a dataclass, an assignment should not prevent a name from becoming an instance attribute.
             # Hence, "as_target = not is_dataclass".
             self.declare_from_annotation(env, as_target=not is_dataclass)
+        elif (self.entry and self.entry.is_inherited and 
+                self.annotation and env.is_c_dataclass_scope):
+            error(self.pos, "Cannot redeclare inherited fields in Cython dataclasses")
         if not self.entry:
             if env.directives['warn.undeclared']:
                 warning(self.pos, "implicit declaration of '%s'" % self.name, 1)
diff --git a/Cython/Compiler/Symtab.py b/Cython/Compiler/Symtab.py
index 0950e1cc2..984e10f05 100644
--- a/Cython/Compiler/Symtab.py
+++ b/Cython/Compiler/Symtab.py
@@ -2618,6 +2618,7 @@ class CClassScope(ClassScope):
                 base_entry.name, adapt(base_entry.cname),
                 base_entry.type, None, 'private')
             entry.is_variable = 1
+            entry.is_inherited = True
             entry.annotation = base_entry.annotation
             self.inherited_var_entries.append(entry)
 
diff --git a/Tools/make_dataclass_tests.py b/Tools/make_dataclass_tests.py
index c39a4b2db..dc38eee70 100644
--- a/Tools/make_dataclass_tests.py
+++ b/Tools/make_dataclass_tests.py
@@ -130,19 +130,16 @@ skip_tests = frozenset(
         ('TestOrdering', 'test_no_order'),
         # not possible to add attributes on extension types
         ("TestCase", "test_post_init_classmethod"),
+        # Cannot redefine the same field in a base dataclass (tested in dataclass_e6)
+        ("TestCase", "test_field_order"),
+        (
+            "TestCase",
+            "test_overwrite_fields_in_derived_class",
+        ),
         # Bugs
         #======
         # not specifically a dataclass issue - a C int crashes classvar
         ("TestCase", "test_class_var"),
-        ("TestCase", "test_field_order"),  # invalid C code (__pyx_base?)
-        (
-            "TestCase",
-            "test_overwrite_fields_in_derived_class",
-        ),  # invalid C code (__pyx_base?)
-        (
-            "TestCase",
-            "test_intermediate_non_dataclass",
-        ),  # issue with propagating through intermediate class
         (
             "TestFrozen",
         ),  # raises AttributeError, not FrozenInstanceError (may be hard to fix)
diff --git a/tests/errors/dataclass_e6.pyx b/tests/errors/dataclass_e6.pyx
new file mode 100644
index 000000000..64dc1ae05
--- /dev/null
+++ b/tests/errors/dataclass_e6.pyx
@@ -0,0 +1,23 @@
+# mode: error
+
+from cython.dataclasses cimport dataclass
+
+@dataclass
+cdef class BaseDataclass:
+    a: str = "value"
+
+@dataclass
+cdef class MainDataclass(BaseDataclass):
+    a: str = "new value"
+
+cdef class Intermediate(BaseDataclass):
+    pass
+
+@dataclass
+cdef class AnotherDataclass(Intermediate):
+    a: str = "ooops"
+
+_ERRORS = """
+11:4: Cannot redeclare inherited fields in Cython dataclasses
+18:4: Cannot redeclare inherited fields in Cython dataclasses
+"""
diff --git a/tests/errors/dataclass_w1.pyx b/tests/errors/dataclass_w1.pyx
new file mode 100644
index 000000000..c0d9790e2
--- /dev/null
+++ b/tests/errors/dataclass_w1.pyx
@@ -0,0 +1,13 @@
+# mode: compile
+# tag: warnings
+
+from dataclass_w1_othermod cimport SomeBase
+from cython.dataclasses cimport dataclass
+
+@dataclass
+cdef class DC(SomeBase):
+    a: str = ""
+
+_WARNINGS = """
+8:5: Cannot reliably handle Cython dataclasses with base types in external modules since it is not possible to tell what fields they have
+"""
diff --git a/tests/errors/dataclass_w1_othermod.pxd b/tests/errors/dataclass_w1_othermod.pxd
new file mode 100644
index 000000000..02dddf492
--- /dev/null
+++ b/tests/errors/dataclass_w1_othermod.pxd
@@ -0,0 +1,3 @@
+# Extern class for test "dataclass_w1"
+cdef class SomeBase:
+    pass
diff --git a/tests/run/test_dataclasses.pyx b/tests/run/test_dataclasses.pyx
index 59e1f0a49..4daf62cf8 100644
--- a/tests/run/test_dataclasses.pyx
+++ b/tests/run/test_dataclasses.pyx
@@ -153,6 +153,23 @@ class Bar_TestCase_test_default_factory_derived(Foo_TestCase_test_default_factor
 class Baz_TestCase_test_default_factory_derived(Foo_TestCase_test_default_factory_derived):
     pass
 
+@dataclass
+@cclass
+class A_TestCase_test_intermediate_non_dataclass:
+    x: int
+
+@cclass
+class B_TestCase_test_intermediate_non_dataclass(A_TestCase_test_intermediate_non_dataclass):
+    y: int
+
+@dataclass
+@cclass
+class C_TestCase_test_intermediate_non_dataclass(B_TestCase_test_intermediate_non_dataclass):
+    z: int
+
+class D_TestCase_test_intermediate_non_dataclass(C_TestCase_test_intermediate_non_dataclass):
+    t: int
+
 class NotDataClass_TestCase_test_is_dataclass:
     pass
 
@@ -729,6 +746,18 @@ class TestCase(unittest.TestCase):
         Baz = Baz_TestCase_test_default_factory_derived
         self.assertEqual(Baz().x, {})
 
+    def test_intermediate_non_dataclass(self):
+        A = A_TestCase_test_intermediate_non_dataclass
+        B = B_TestCase_test_intermediate_non_dataclass
+        C = C_TestCase_test_intermediate_non_dataclass
+        c = C(1, 3)
+        self.assertEqual((c.x, c.z), (1, 3))
+        with self.assertRaises(AttributeError):
+            c.y
+        D = D_TestCase_test_intermediate_non_dataclass
+        d = D(4, 5)
+        self.assertEqual((d.x, d.z), (4, 5))
+
     def test_is_dataclass(self):
         NotDataClass = NotDataClass_TestCase_test_is_dataclass
         self.assertFalse(is_dataclass(0))