New modifier checking strategy: only one error but any number of warnings, a warning can never shadow an error

compiler/frontend/src/org/jetbrains/kotlin/resolve/ModifiersChecker.kt

@@ -162,19 +162,13 @@ public object ModifierCheckerCore {
             Compatibility.REPEATED -> if (incorrectNodes.add(secondNode)) {
                 trace.report(Errors.REPEATED_MODIFIER.on (secondNode.psi, first))
             }
-            Compatibility.REDUNDANT -> if (incorrectNodes.add(secondNode)) {
+            Compatibility.REDUNDANT ->
                 trace.report(Errors.REDUNDANT_MODIFIER.on(secondNode.psi, first, second))
-            }
+            Compatibility.REVERSE_REDUNDANT ->
-            Compatibility.REVERSE_REDUNDANT -> if (incorrectNodes.add(firstNode)) {
                 trace.report(Errors.REDUNDANT_MODIFIER.on(firstNode.psi,  second, first))
-            }
             Compatibility.DEPRECATED -> {
-                if (incorrectNodes.add(firstNode)) {
+                trace.report(Errors.DEPRECATED_MODIFIER_PAIR.on(firstNode.psi, first, second))
-                    trace.report(Errors.DEPRECATED_MODIFIER_PAIR.on(firstNode.psi, first, second))
+                trace.report(Errors.DEPRECATED_MODIFIER_PAIR.on(secondNode.psi, second, first))
-                }
-                if (incorrectNodes.add(secondNode)) {
-                    trace.report(Errors.DEPRECATED_MODIFIER_PAIR.on(secondNode.psi, second, first))
-                }
             }
             Compatibility.INCOMPATIBLE -> {
                 if (incorrectNodes.add(firstNode)) {
@@ -187,6 +181,7 @@ public object ModifierCheckerCore {
         }
     }
 
+    // Should return false if error is reported, true otherwise
     private fun checkTarget(trace: BindingTrace, node: ASTNode, actualTargets: List<KotlinTarget>): Boolean {
         val modifier = node.elementType as JetModifierKeywordToken
         val possibleTargets = possibleTargetMap[modifier] ?: emptySet()
@@ -201,6 +196,7 @@ public object ModifierCheckerCore {
         return true
     }
 
+    // Should return false if error is reported, true otherwise
     private fun checkParent(trace: BindingTrace, node: ASTNode, parentDescriptor: DeclarationDescriptor?): Boolean {
         val modifier = node.elementType as JetModifierKeywordToken
         val actualParents: List<KotlinTarget> = when (parentDescriptor) {
@@ -211,7 +207,7 @@ public object ModifierCheckerCore {
         val deprecatedParents = deprecatedParentTargetMap[modifier]
         if (deprecatedParents != null && actualParents.any { it in deprecatedParents }) {
             trace.report(Errors.DEPRECATED_MODIFIER_CONTAINING_DECLARATION.on(node.psi, modifier, actualParents.firstOrNull()?.description ?: "this scope"))
-            return false
+            return true
         }
         val possibleParents = possibleParentTargetMap[modifier] ?: return true
         if (possibleParents == KotlinTarget.ALL_TARGET_SET) return true
@@ -223,6 +219,8 @@ public object ModifierCheckerCore {
     private val MODIFIER_KEYWORD_SET = TokenSet.orSet(JetTokens.SOFT_KEYWORDS, TokenSet.create(JetTokens.IN_KEYWORD))
 
     private fun checkModifierList(list: JetModifierList, trace: BindingTrace, parentDescriptor: DeclarationDescriptor?, actualTargets: List<KotlinTarget>) {
+        // It's a list of all nodes with error already reported
+        // General strategy: report no more than one error but any number of warnings
         val incorrectNodes = hashSetOf<ASTNode>()
         val children = list.node.getChildren(MODIFIER_KEYWORD_SET)
         for (second in children) {

compiler/testData/cli/jvm/multipleTextRangesInDiagnosticsOrder.out

@@ -1,6 +1,9 @@
 compiler/testData/cli/jvm/multipleTextRangesInDiagnosticsOrder.kt:6:14: error: cannot weaken access privilege 'public' for 'c' in 'A'
     override protected private val c: Int
              ^
+compiler/testData/cli/jvm/multipleTextRangesInDiagnosticsOrder.kt:6:14: error: modifier 'protected' is incompatible with 'private'
+    override protected private val c: Int
+             ^
 compiler/testData/cli/jvm/multipleTextRangesInDiagnosticsOrder.kt:6:24: error: modifier 'private' is incompatible with 'protected'
     override protected private val c: Int
                        ^

compiler/testData/diagnostics/tests/dataClasses/innerOuterDataClass.kt

@@ -0,0 +1 @@
+<!WRONG_MODIFIER_TARGET, DEPRECATED_MODIFIER_PAIR!>inner<!> <!DEPRECATED_MODIFIER_PAIR!>data<!> class Outer(val x: Int)

compiler/testData/diagnostics/tests/dataClasses/innerOuterDataClass.txt

@@ -0,0 +1,11 @@
+package
+
+@kotlin.data() public final class Outer {
+    public constructor Outer(/*0*/ x: kotlin.Int)
+    public final val x: kotlin.Int
+    public final /*synthesized*/ fun component1(): kotlin.Int
+    public final /*synthesized*/ fun copy(/*0*/ x: kotlin.Int = ...): Outer
+    public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+    public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+    public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+}

compiler/testData/diagnostics/tests/modifiers/IllegalModifiers.kt

@@ -119,7 +119,7 @@ class IllegalModifiers7() {
 class IllegalModifiers8 {
     <!WRONG_MODIFIER_TARGET!>abstract<!>
     <!WRONG_MODIFIER_TARGET!>enum<!>
-    <!REDUNDANT_MODIFIER!>open<!>
+    <!REDUNDANT_MODIFIER, WRONG_MODIFIER_TARGET, REDUNDANT_MODIFIER!>open<!>
     <!WRONG_MODIFIER_TARGET!>inner<!>
     <!WRONG_MODIFIER_TARGET!>annotation<!>
     <!WRONG_MODIFIER_TARGET!>override<!>
@@ -144,7 +144,7 @@ class IllegalModifiers9 {
 class IllegalModifiers10
 <!WRONG_MODIFIER_TARGET!>abstract<!>
 <!WRONG_MODIFIER_TARGET!>enum<!>
-<!REDUNDANT_MODIFIER!>open<!>
+<!REDUNDANT_MODIFIER, WRONG_MODIFIER_TARGET, REDUNDANT_MODIFIER!>open<!>
 <!WRONG_MODIFIER_TARGET!>inner<!>
 <!WRONG_MODIFIER_TARGET!>annotation<!>
 <!WRONG_MODIFIER_TARGET!>override<!>

compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java

@@ -3360,6 +3360,12 @@ public class JetDiagnosticsTestGenerated extends AbstractJetDiagnosticsTest {
                 doTest(fileName);
             }
 
+            @TestMetadata("innerOuterDataClass.kt")
+            public void testInnerOuterDataClass() throws Exception {
+                String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/dataClasses/innerOuterDataClass.kt");
+                doTest(fileName);
+            }
+
             @TestMetadata("multiDeclaration.kt")
             public void testMultiDeclaration() throws Exception {
                 String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/dataClasses/multiDeclaration.kt");