Raise an error about invalid fields order taking inheritance into account (PY-28506, PY-31762)

python/src/com/jetbrains/python/inspections/PyDataclassInspection.kt

@@ -105,6 +105,8 @@ class PyDataclassInspection : PyInspection() {
 
           PyNamedTupleInspection.inspectFieldsOrder(
             node,
+            { parseDataclassParameters(it, myTypeEvalContext) != null },
+            myTypeEvalContext,
             this::registerProblem,
             {
               val stub = it.stub

python/src/com/jetbrains/python/inspections/PyNamedTupleInspection.kt

@@ -12,34 +12,51 @@ import com.jetbrains.python.codeInsight.stdlib.PyNamedTupleTypeProvider
 import com.jetbrains.python.psi.LanguageLevel
 import com.jetbrains.python.psi.PyClass
 import com.jetbrains.python.psi.PyTargetExpression
+import com.jetbrains.python.psi.types.TypeEvalContext
 import java.util.*
 
 class PyNamedTupleInspection : PyInspection() {
 
   companion object {
     fun inspectFieldsOrder(cls: PyClass,
+                           ancestorsFilter: (PyClass) -> Boolean,
+                           context: TypeEvalContext,
                            callback: (PsiElement, String, ProblemHighlightType) -> Unit,
-                           filter: (PyTargetExpression) -> Boolean = { true },
+                           fieldsFilter: (PyTargetExpression) -> Boolean = { true },
                            hasAssignedValue: (PyTargetExpression) -> Boolean = PyTargetExpression::hasAssignedValue) {
-      val fieldsProcessor = FieldsProcessor(filter, hasAssignedValue)
+      val fieldsProcessor = processFields(cls, fieldsFilter, hasAssignedValue)
 
-      cls.processClassLevelDeclarations(fieldsProcessor)
+      val lastFieldWithoutDefaultValue = fieldsProcessor.lastFieldWithoutDefaultValue
+      if (lastFieldWithoutDefaultValue != null) {
+        val ancestorsHavingFieldsWithDefaultValue = cls.getAncestorClasses(context).filter {
+          ancestorsFilter(it) && processFields(it, fieldsFilter, hasAssignedValue).fieldsWithDefaultValue.isNotEmpty()
+        }
 
-      registerErrorOnTargetsAboveBound(fieldsProcessor.lastFieldWithoutDefaultValue,
-                                       fieldsProcessor.fieldsWithDefaultValue,
-                                       "Fields with a default value must come after any fields without a default.",
-                                       callback)
+        if (ancestorsHavingFieldsWithDefaultValue.isNotEmpty()) {
+          cls.nameIdentifier?.let { name ->
+            val ancestorsNames = ancestorsHavingFieldsWithDefaultValue.joinToString { "'${it.name}'" }
+
+            callback(name,
+                     "Non-default argument(s) follows default argument(s) defined in $ancestorsNames",
+                     ProblemHighlightType.GENERIC_ERROR)
+          }
+        }
+
+        fieldsProcessor.fieldsWithDefaultValue.headSet(lastFieldWithoutDefaultValue).forEach {
+          callback(it,
+                   "Fields with a default value must come after any fields without a default.",
+                   ProblemHighlightType.GENERIC_ERROR)
+        }
+      }
     }
 
-    private fun registerErrorOnTargetsAboveBound(bound: PyTargetExpression?,
-                                                 targets: TreeSet<PyTargetExpression>,
-                                                 message: String,
-                                                 callback: (PsiElement, String, ProblemHighlightType) -> Unit) {
-      if (bound != null) {
-        targets
-          .headSet(bound)
-          .forEach { callback(it, message, ProblemHighlightType.GENERIC_ERROR) }
-      }
+    private fun processFields(cls: PyClass,
+                              filter: (PyTargetExpression) -> Boolean,
+                              hasAssignedValue: (PyTargetExpression) -> Boolean): LocalFieldsProcessor {
+      val fieldsProcessor = LocalFieldsProcessor(filter, hasAssignedValue)
+      cls.processClassLevelDeclarations(fieldsProcessor)
+
+      return fieldsProcessor
     }
   }
 
@@ -55,13 +72,13 @@ class PyNamedTupleInspection : PyInspection() {
       if (node != null &&
           LanguageLevel.forElement(node).isAtLeast(LanguageLevel.PYTHON36) &&
           PyNamedTupleTypeProvider.isTypingNamedTupleDirectInheritor(node, myTypeEvalContext)) {
-        inspectFieldsOrder(node, this::registerProblem)
+        inspectFieldsOrder(node, { false }, myTypeEvalContext, this::registerProblem)
       }
     }
   }
 
-  private class FieldsProcessor(private val filter: (PyTargetExpression) -> Boolean,
-                                private val hasAssignedValue: (PyTargetExpression) -> Boolean) : PsiScopeProcessor {
+  private class LocalFieldsProcessor(private val filter: (PyTargetExpression) -> Boolean,
+                                     private val hasAssignedValue: (PyTargetExpression) -> Boolean) : PsiScopeProcessor {
 
     val lastFieldWithoutDefaultValue: PyTargetExpression?
       get() = lastFieldWithoutDefaultValueBox.result

python/testData/inspections/PyDataclassInspection/fieldsOrderInInheritance.py

@@ -0,0 +1,30 @@
+import attr
+import dataclasses
+
+@dataclasses.dataclass
+class A1:
+    x1: int
+    x2: int = 1
+
+@dataclasses.dataclass
+class <error descr="Non-default argument(s) follows default argument(s) defined in 'A1'">B1</error>(A1):
+    y1: str
+    y2: str = "1"
+
+@attr.dataclass
+class A2:
+    x1: int
+    x2: int = 1
+
+@attr.dataclass
+class <error descr="Non-default argument(s) follows default argument(s) defined in 'A2'">B2</error>(A2):
+    y1: str
+    y2: str = "1"
+
+@dataclasses.dataclass
+class A3:
+    x1: int
+
+@dataclasses.dataclass
+class B3(A3):
+    y1: str

python/testData/inspections/PyDataclassInspection/fieldsOrderInMixedInheritance.py

@@ -0,0 +1,38 @@
+import attr
+import dataclasses
+
+class A1:
+    x1: int
+    x2: int = 1
+
+@dataclasses.dataclass
+class B1(A1):
+    y1: str
+    y2: str = "1"
+
+class A2:
+    x1: int
+    x2: int = 1
+
+@attr.dataclass
+class B2(A2):
+    y1: str
+    y2: str = "1"
+
+@dataclasses.dataclass
+class A3:
+    x1: int
+    x2: int = 1
+
+class B3(A3):
+    y1: str
+    y2: str = "1"
+
+@attr.dataclass
+class A4:
+    x1: int
+    x2: int = 1
+
+class B4(A4):
+    y1: str
+    y2: str = "1"

python/testSrc/com/jetbrains/python/inspections/PyDataclassInspectionTest.java

@@ -56,6 +56,16 @@ public class PyDataclassInspectionTest extends PyInspectionTestCase {
     doTest();
   }
 
+  // PY-28506, PY-31762
+  public void testFieldsOrderInInheritance() {
+    doTest();
+  }
+
+  // PY-28506, PY-31762
+  public void testFieldsOrderInMixedInheritance() {
+    doTest();
+  }
+
   // PY-27398
   public void testComparisonForOrdered() {
     doTest();