Repository: cython/cython
Pull request: Allow `dataclasses.dataclass` to be used directly on extension types
Also implements the same for `functools.total_ordering`.

Closes https://github.com/cython/cython/issues/5292 - as discussed in that issue, if the directives are taken from the `cython` module, `cclass` is automatically implied.

Problem statement:
[ENH] use `@dataclasses.dataclass` to cdef dataclasses (instead of `@cython.dataclasses.dataclass`)
### Is your feature request related to a problem? Please describe.

One confusing feature of Cython dataclasses is that you have to cimport `cython.dataclasses.dataclass` and `cython.dataclasses.field` to use them, but all other dataclass-related imports (e.g. `dataclasses.ClassVar` and any runtime dataclass functions you want to access) come from the standard library `dataclasses` module.

There were good reasons for this at the time, but I think it's just a source of confusion to people. Therefore we should probably just take a `@dataclasses.dataclass` decorator on a `cdef` class to mean "Cython dataclass".

(We should also do the same for `total_ordering` while we're at it)

### Describe the solution you'd like.

_No response_

### Describe alternatives you've considered.

I'm slightly uncomfortable with co-opting someone else's module for our own behaviour. This'd especially be a concern if we ever allowed arbitrary decorators on `cdef classes` (in which case people might legitimately want to use the standard library module directly). However that's probably unlikely.

### Additional context

This was discussed before in a comment somewhere which I now can't find...

Hints from issue discussion:
I agree, both for `@dataclasses.dataclass` and `@functools.total_ordering`, when used on a `cdef ` class (and `@cython.cclass`, obviously).

I additionally suggest that putting `@cython.dataclasses.dataclass` on a  Python class should mean both `@cython.cclass` and `@dataclasses.dataclass`, without the need for an additional `@cython.cclass` decorator.

Commit message: Update documentation

Patch to review:
diff --git a/Cython/Compiler/Builtin.py b/Cython/Compiler/Builtin.py
index 78b2638d5c7..4c7220125fd 100644
--- a/Cython/Compiler/Builtin.py
+++ b/Cython/Compiler/Builtin.py
@@ -591,7 +591,15 @@ def get_known_standard_library_module_scope(module_name):
         var_entry.is_pyglobal = True
         var_entry.scope = mod
         entry.as_variable = var_entry
+        for name in ["dataclass", "field"]:
+            mod.declare_var(EncodedString(name), PyrexTypes.py_object_type, pos=None)
         _known_module_scopes[module_name] = mod
+    elif module_name == "functools":
+        mod = ModuleScope(module_name, None, None)
+        for name in ["total_ordering"]:
+            mod.declare_var(EncodedString(name), PyrexTypes.py_object_type, pos=None)
+        _known_module_scopes[module_name] = mod
+
     return mod
 
 
@@ -609,3 +617,24 @@ def get_known_standard_library_entry(qualified_name):
     if mod and rest:
         return mod.lookup_here(rest[0])
     return None
+
+
+def exprnode_to_known_standard_library_name(node, env):
+    qualified_name_parts = []
+    known_name = None
+    while node.is_attribute:
+        qualified_name_parts.append(node.attribute)
+        node = node.obj
+    if node.is_name:
+        entry = env.lookup(node.name)
+        if entry and entry.known_standard_library_import:
+            if get_known_standard_library_entry(
+                    entry.known_standard_library_import):
+                known_name = entry.known_standard_library_import
+            else:
+                standard_env = get_known_standard_library_module_scope(
+                    entry.known_standard_library_import)
+                if standard_env:
+                    qualified_name_parts.append(standard_env.name)
+                    known_name = ".".join(reversed(qualified_name_parts))
+    return known_name
diff --git a/Cython/Compiler/Dataclass.py b/Cython/Compiler/Dataclass.py
index e775e918280..4a57a983f72 100644
--- a/Cython/Compiler/Dataclass.py
+++ b/Cython/Compiler/Dataclass.py
@@ -252,8 +252,10 @@ def process_class_get_fields(node):
         is_classvar = entry.declared_with_pytyping_modifier("typing.ClassVar")
         if name in default_value_assignments:
             assignment = default_value_assignments[name]
-            if (isinstance(assignment, ExprNodes.CallNode)
-                    and assignment.function.as_cython_attribute() == "dataclasses.field"):
+            if (isinstance(assignment, ExprNodes.CallNode) and (
+                    assignment.function.as_cython_attribute() == "dataclasses.field" or
+                    Builtin.exprnode_to_known_standard_library_name(
+                        assignment.function, node.scope) == "dataclasses.field")):
                 # I believe most of this is well-enforced when it's treated as a directive
                 # but it doesn't hurt to make sure
                 valid_general_call = (isinstance(assignment, ExprNodes.GeneralCallNode)
@@ -271,11 +273,6 @@ def process_class_get_fields(node):
                     continue
                 field = Field(node.pos, **keyword_args)
             else:
-                if isinstance(assignment, ExprNodes.CallNode):
-                    func = assignment.function
-                    if ((func.is_name and func.name == "field")
-                            or (func.is_attribute and func.attribute == "field")):
-                        warning(assignment.pos, "Do you mean cython.dataclasses.field instead?", 1)
                 if assignment.type in [Builtin.list_type, Builtin.dict_type, Builtin.set_type]:
                     # The standard library module generates a TypeError at runtime
                     # in this situation.
diff --git a/Cython/Compiler/Nodes.py b/Cython/Compiler/Nodes.py
index 2302262980b..0cef7cdd3fc 100644
--- a/Cython/Compiler/Nodes.py
+++ b/Cython/Compiler/Nodes.py
@@ -2985,7 +2985,7 @@ def generate_function_definitions(self, env, code):
 class DecoratorNode(Node):
     # A decorator
     #
-    # decorator    NameNode or CallNode or AttributeNode
+    # decorator    ExprNode
     child_attrs = ['decorator']
 
 
@@ -5264,6 +5264,49 @@ def declare(self, env):
         if self.bases and len(self.bases.args) > 1:
             self.entry.type.multiple_bases = True
 
+    def _handle_cclass_decorators(self, env):
+        extra_directives = {}
+        if not self.decorators:
+            return extra_directives
+
+        from . import ExprNodes
+
+        remaining_decorators = []
+
+        for original_decorator in self.decorators:
+            decorator = original_decorator.decorator
+            # entries aren't set at this point, so unfortunately we can't just do
+            #  decorator.get_known_standard_library_import().
+            # Instead we have to manually look it up
+            decorator_call = None
+            if isinstance(decorator, ExprNodes.CallNode):
+                decorator_call = decorator
+                decorator = decorator.function
+            known_name = Builtin.exprnode_to_known_standard_library_name(decorator, env)
+            if known_name == 'functools.total_ordering':
+                if decorator_call:
+                    error(decorator_call.pos, "total_ordering cannot be called.")
+                extra_directives["total_ordering"] = True
+                continue
+            elif known_name == "dataclasses.dataclass":
+                args = None
+                kwds = {}
+                if decorator_call:
+                    if isinstance(decorator_call, ExprNodes.SimpleCallNode):
+                        args = decorator_call.args
+                    else:
+                        args = decorator_call.positional_args.args
+                        kwds_ = decorator_call.keyword_args
+                        if kwds_:
+                            kwds = kwds_.as_python_dict()
+                extra_directives[known_name] = (args, kwds)
+                continue
+            remaining_decorators.append(original_decorator)
+        if remaining_decorators:
+            error(remaining_decorators[0].pos, "Cdef functions/classes cannot take arbitrary decorators.")
+        self.decorators = remaining_decorators
+        return extra_directives
+
     def analyse_declarations(self, env):
         #print "CClassDefNode.analyse_declarations:", self.class_name
         #print "...visibility =", self.visibility
@@ -5271,8 +5314,7 @@ def analyse_declarations(self, env):
 
         if env.in_cinclude and not self.objstruct_name:
             error(self.pos, "Object struct name specification required for C class defined in 'extern from' block")
-        if self.decorators:
-            error(self.pos, "Decorators not allowed on cdef classes (used on type '%s')" % self.class_name)
+        extra_directives = self._handle_cclass_decorators(env)
         self.base_type = None
         # Now that module imports are cached, we need to
         # import the modules for extern classes.
@@ -5361,11 +5403,15 @@ def analyse_declarations(self, env):
             env.add_imported_entry(self.class_name, self.entry, self.pos)
         self.scope = scope = self.entry.type.scope
         if scope is not None:
-            scope.directives = env.directives
-            if "dataclasses.dataclass" in env.directives:
+            if extra_directives:
+                scope.directives = env.directives.copy()
+                scope.directives.update(extra_directives)
+            else:
+                scope.directives = env.directives
+            if "dataclasses.dataclass" in scope.directives:
                 is_frozen = False
                 # Retrieve the @dataclass config (args, kwargs), as passed into the decorator.
-                dataclass_config = env.directives["dataclasses.dataclass"]
+                dataclass_config = scope.directives["dataclasses.dataclass"]
                 if dataclass_config:
                     decorator_kwargs = dataclass_config[1]
                     frozen_flag = decorator_kwargs.get('frozen')
diff --git a/Cython/Compiler/Options.py b/Cython/Compiler/Options.py
index 62930f3e28c..d64205a67bd 100644
--- a/Cython/Compiler/Options.py
+++ b/Cython/Compiler/Options.py
@@ -391,7 +391,7 @@ class DEFER_ANALYSIS_OF_ARGUMENTS:
     'fast_gil': ('module',),
     'iterable_coroutine': ('module', 'function'),
     'trashcan' : ('cclass',),
-    'total_ordering': ('cclass', ),
+    'total_ordering': ('cclass', 'class'),
     'dataclasses.dataclass' : ('class', 'cclass',),
     'cpp_locals': ('module', 'function', 'cclass'),  # I don't think they make sense in a with_statement
     'ufunc': ('function',),
diff --git a/Cython/Compiler/ParseTreeTransforms.py b/Cython/Compiler/ParseTreeTransforms.py
index 8008cba9aee..11e7fe61bf3 100644
--- a/Cython/Compiler/ParseTreeTransforms.py
+++ b/Cython/Compiler/ParseTreeTransforms.py
@@ -1339,17 +1339,6 @@ def _extract_directives(self, node, scope_name):
                         scope_name = 'cclass'
             else:
                 realdecs.append(dec)
-        if realdecs and (scope_name == 'cclass' or
-                         isinstance(node, (Nodes.CClassDefNode, Nodes.CVarDefNode))):
-            for realdec in realdecs:
-                dec_pos = realdec.pos
-                realdec = realdec.decorator
-                if ((realdec.is_name and realdec.name == "dataclass") or
-                        (realdec.is_attribute and realdec.attribute == "dataclass")):
-                    error(dec_pos,
-                          "Use '@cython.dataclasses.dataclass' on cdef classes to create a dataclass")
-            # Note - arbitrary C function decorators are caught later in DecoratorTransform
-            raise PostParseError(realdecs[0].pos, "Cdef functions/classes cannot take arbitrary decorators.")
         node.decorators = realdecs[::-1] + both[::-1]
         # merge or override repeated directives
         optdict = {}
@@ -2872,6 +2861,8 @@ class AdjustDefByDirectives(CythonTransform, SkipDeclarations):
     @cython.inline
     @cython.nogil
     """
+    # list of directives that cause conversion to cclass
+    converts_to_cclass = ('cclass', 'total_ordering', 'dataclasses.dataclass')
 
     def visit_ModuleNode(self, node):
         self.directives = node.directives
@@ -2929,7 +2920,7 @@ def visit_LambdaNode(self, node):
         return node
 
     def visit_PyClassDefNode(self, node):
-        if 'cclass' in self.directives:
+        if any(directive in self.directives for directive in self.converts_to_cclass):
             node = node.as_cclass()
             return self.visit(node)
         else:
diff --git a/docs/examples/userguide/extension_types/dataclass.py b/docs/examples/userguide/extension_types/dataclass.py
index d8ed686666a..de61c4aff6f 100644
--- a/docs/examples/userguide/extension_types/dataclass.py
+++ b/docs/examples/userguide/extension_types/dataclass.py
@@ -5,12 +5,12 @@
 except ImportError:
     pass  # The modules don't actually have to exists for Cython to use them as annotations
 
-@cython.dataclasses.dataclass
+@dataclasses.dataclass
 @cython.cclass
 class MyDataclass:
     # fields can be declared using annotations
     a: cython.int = 0
-    b: double = cython.dataclasses.field(default_factory = lambda: 10, repr=False)
+    b: double = dataclasses.field(default_factory = lambda: 10, repr=False)
 
 
     c: str = 'hello'
diff --git a/docs/examples/userguide/extension_types/dataclass.pyx b/docs/examples/userguide/extension_types/dataclass.pyx
index b03d5f7b136..d6eb1cb7ee2 100644
--- a/docs/examples/userguide/extension_types/dataclass.pyx
+++ b/docs/examples/userguide/extension_types/dataclass.pyx
@@ -6,11 +6,11 @@ except ImportError:
     pass  # The modules don't actually have to exists for Cython to use them as annotations
 
 
-@cython.dataclasses.dataclass
+@dataclasses.dataclass
 cdef class MyDataclass:
     # fields can be declared using annotations
     a: cython.int = 0
-    b: double = cython.dataclasses.field(default_factory = lambda: 10, repr=False)
+    b: double = dataclasses.field(default_factory = lambda: 10, repr=False)
 
     # fields can also be declared using `cdef`:
     cdef str c
diff --git a/docs/examples/userguide/special_methods/total_ordering.py b/docs/examples/userguide/special_methods/total_ordering.py
index 7d164d6dfbe..cef30da624d 100644
--- a/docs/examples/userguide/special_methods/total_ordering.py
+++ b/docs/examples/userguide/special_methods/total_ordering.py
@@ -1,5 +1,7 @@
+import functools
 import cython
-@cython.total_ordering
+
+@functools.total_ordering
 @cython.cclass
 class ExtGe:
     x: cython.int
diff --git a/docs/examples/userguide/special_methods/total_ordering.pyx b/docs/examples/userguide/special_methods/total_ordering.pyx
index 06d2ccef7af..7cc3f71f60d 100644
--- a/docs/examples/userguide/special_methods/total_ordering.pyx
+++ b/docs/examples/userguide/special_methods/total_ordering.pyx
@@ -1,6 +1,8 @@
-import cython
+import functools
 
-@cython.total_ordering
+
+
+@functools.total_ordering
 cdef class ExtGe:
     cdef int x
 
diff --git a/docs/src/userguide/extension_types.rst b/docs/src/userguide/extension_types.rst
index 42d77c37863..9bd51656d28 100644
--- a/docs/src/userguide/extension_types.rst
+++ b/docs/src/userguide/extension_types.rst
@@ -1297,11 +1297,13 @@ here only briefly outlines the differences - if you plan on using them
 then please read `the documentation for the standard library module
 <https://docs.python.org/3/library/dataclasses.html>`_.
 
-Dataclasses can be declared using the ``@cython.dataclasses.dataclass`` 
-decorator on a Cython extension type. ``@cython.dataclasses.dataclass``
-can only be applied to extension types (types marked ``cdef`` or created with the 
-``cython.cclass`` decorator) and not to regular classes. If
-you need to define special properties on a field then use ``cython.dataclasses.field``
+Dataclasses can be declared using the ``@dataclasses.dataclass`` 
+decorator on a Cython extension type (types marked ``cdef`` or created with the 
+``cython.cclass`` decorator). Alternatively the ``@cython.dataclasses.dataclass``
+decorator can be applied to any class to both turn it into an extension type and
+a dataclass. If
+you need to define special properties on a field then use ``dataclasses.field``
+(or ``cython.dataclasses.field`` will work too)
 
 .. tabs::
 
diff --git a/docs/src/userguide/special_methods.rst b/docs/src/userguide/special_methods.rst
index d085aa284a6..38b80e7cd4e 100644
--- a/docs/src/userguide/special_methods.rst
+++ b/docs/src/userguide/special_methods.rst
@@ -223,11 +223,12 @@ Depending on the application, one way or the other may be better:
 
   These constants can be cimported from the ``cpython.object`` module.
 
-* Use the ``@cython.total_ordering`` decorator, which is a low-level
-  re-implementation of the `functools.total_ordering
-  <https://docs.python.org/3/library/functools.html#functools.total_ordering>`_
-  decorator specifically for ``cdef`` classes.  (Normal Python classes can use
-  the original ``functools`` decorator.)
+* If you use the `functools.total_ordering<https://docs.python.org/3/library/functools.html#functools.total_ordering>`_
+  decorator on an extension type/``cdef`` class, Cython replaces it with a low-level reimplementation
+  designed specifically for extension types.  (On a normal Python classes, the ``functools`` 
+  decorator continues to work as before.)  As a shortcut you can also use ``cython.total_ordering``, which
+  applies the same re-implementation but also transforms the class to an extension type if it
+  isn't already.
 
 .. tabs::
 
diff --git a/tests/errors/dataclass_e2.pyx b/tests/errors/dataclass_e2.pyx
deleted file mode 100644
index e259659381c..00000000000
--- a/tests/errors/dataclass_e2.pyx
+++ /dev/null
@@ -1,13 +0,0 @@
-# mode: error
-# tag: dataclass
-
-import dataclasses
-
-@dataclasses.dataclass
-cdef class C:
-    pass
-
-_ERRORS = """
-6:0: Cdef functions/classes cannot take arbitrary decorators.
-6:0: Use '@cython.dataclasses.dataclass' on cdef classes to create a dataclass
-"""
diff --git a/tests/errors/dataclass_e3.pyx b/tests/errors/dataclass_e3.pyx
deleted file mode 100644
index 85a9001726e..00000000000
--- a/tests/errors/dataclass_e3.pyx
+++ /dev/null
@@ -1,13 +0,0 @@
-# mode: compile
-# tag: dataclass, warnings
-
-cimport cython
-from dataclass import field
-
-@cython.dataclasses.dataclass
-cdef class E:
-    a: int = field()
-
-_WARNINGS="""
-9:18: Do you mean cython.dataclasses.field instead?
-"""
diff --git a/tests/run/exttype_total_ordering.pyx b/tests/run/exttype_total_ordering.pyx
index c5f71b8b7e6..7b8aba0b6a7 100644
--- a/tests/run/exttype_total_ordering.pyx
+++ b/tests/run/exttype_total_ordering.pyx
@@ -17,6 +17,7 @@ from __future__ import print_function
 
 cimport cython
 import functools
+from functools import total_ordering as total_ordering_by_another_name
 import operator
 
 COMPARISONS = [
@@ -117,7 +118,7 @@ cdef class ExtTypeTotalOrderingNeGe:
     def __ge__(self, other):
         return self.value >= other.value
 
-@cython.total_ordering
+@functools.total_ordering
 cdef class ExtTypeTotalOrderingNeLe:
     """
     >>> test_all_comp(ExtTypeTotalOrderingNeLe)
@@ -133,7 +134,7 @@ cdef class ExtTypeTotalOrderingNeLe:
     def __le__(self, other):
         return self.value <= other.value
 
-@cython.total_ordering
+@functools.total_ordering
 cdef class ExtTypeTotalOrderingNeLeGe:
     """
     >>> test_all_comp(ExtTypeTotalOrderingNeLeGe)
@@ -392,7 +393,7 @@ cdef class ExtTypeTotalOrderingNeLtGtLeGe:
     def __ge__(self, other):
         return self.value >= other.value
 
-@cython.total_ordering
+@total_ordering_by_another_name
 cdef class ExtTypeTotalOrderingEqGe:
     """
     >>> test_all_comp(ExtTypeTotalOrderingEqGe)
@@ -443,7 +444,7 @@ cdef class ExtTypeTotalOrderingEqLeGe:
     def __ge__(self, other):
         return self.value >= other.value
 
-@cython.total_ordering
+@total_ordering_by_another_name
 cdef class ExtTypeTotalOrderingEqGt:
     """
     >>> test_all_comp(ExtTypeTotalOrderingEqGt)
@@ -519,21 +520,27 @@ cdef class ExtTypeTotalOrderingEqGtLeGe:
     def __ge__(self, other):
         return self.value >= other.value
 
+# cython.total_ordering implicitly means cclass too
 @cython.total_ordering
-cdef class ExtTypeTotalOrderingEqLt:
+class ExtTypeTotalOrderingEqLt:
     """
     >>> test_all_comp(ExtTypeTotalOrderingEqLt)
     True
     """
-    cdef public int value
+    _value: int
+
+    @property
+    def value(self):
+        return self._value  # it's hard to make value public directly in pure syntax
+
     def __init__(self, val):
-        self.value = val
+        self._value = val
 
     def __eq__(self, other):
-        return self.value == other.value
+        return self._value == other.value
 
     def __lt__(self, other):
-        return self.value < other.value
+        return self._value < other.value
 
 @cython.total_ordering
 cdef class ExtTypeTotalOrderingEqLtGe:
diff --git a/tests/run/pure_cdef_class_dataclass.py b/tests/run/pure_cdef_class_dataclass.py
index e5c4bcd3297..f8136958e1c 100644
--- a/tests/run/pure_cdef_class_dataclass.py
+++ b/tests/run/pure_cdef_class_dataclass.py
@@ -5,8 +5,8 @@
 
 import cython
 
+# cython.dataclasses implicitly implies cclass
 @cython.dataclasses.dataclass(order=True, unsafe_hash=True)
-@cython.cclass
 class MyDataclass:
     """
     >>> sorted(list(MyDataclass.__dataclass_fields__.keys()))
diff --git a/tests/run/test_dataclasses.pxi b/tests/run/test_dataclasses.pxi
index 998d837f255..9eff9b3097b 100644
--- a/tests/run/test_dataclasses.pxi
+++ b/tests/run/test_dataclasses.pxi
@@ -1,6 +1,6 @@
-from cython.dataclasses cimport dataclass, field
 from cython cimport cclass
 from dataclasses import (
+    dataclass, field,
     fields, FrozenInstanceError, InitVar, is_dataclass, asdict, astuple, replace
 )
 import unittest


Code review comments:
diff_hunk: @@ -391,7 +391,7 @@ class DEFER_ANALYSIS_OF_ARGUMENTS:
     'fast_gil': ('module',),
     'iterable_coroutine': ('module', 'function'),
     'trashcan' : ('cclass',),
-    'total_ordering': ('cclass', ),
+    'total_ordering': ('cclass', 'class'),
     'dataclasses.dataclass' : ('class', 'cclass',),
path: Cython/Compiler/Options.py
text: @user1:
Let's keep the order consistent here, given that both are even right next to each other.
```suggestion
    'total_ordering': ('class', 'cclass'),
    'dataclasses.dataclass' : ('class', 'cclass'),
```