Supertypes should be on separate lines (#1902)

* ### What's done: - Fixed bug where more than two supertypes should be on separate lines - Added warning and smoke tests Closes #1890 --------- Co-authored-by: Andrey Kuleshov <[email protected]>
saveourtool · Jan 18, 2024 · 8f4b730 · 8f4b730
1 parent 66e03a1
commit 8f4b730
Show file tree

Hide file tree

Showing 12 changed files with 262 additions and 38 deletions.
diff --git a/diktat-cli/src/test/kotlin/com/saveourtool/diktat/smoke/DiktatSmokeTestBase.kt b/diktat-cli/src/test/kotlin/com/saveourtool/diktat/smoke/DiktatSmokeTestBase.kt
@@ -20,6 +20,7 @@ import com.saveourtool.diktat.ruleset.constants.Warnings.MISSING_KDOC_CLASS_ELEM
 import com.saveourtool.diktat.ruleset.constants.Warnings.MISSING_KDOC_ON_FUNCTION
 import com.saveourtool.diktat.ruleset.constants.Warnings.MISSING_KDOC_TOP_LEVEL
 import com.saveourtool.diktat.ruleset.constants.Warnings.WRONG_INDENTATION
+import com.saveourtool.diktat.ruleset.constants.Warnings.WRONG_NEWLINES
 import com.saveourtool.diktat.ruleset.rules.chapter1.FileNaming
 import com.saveourtool.diktat.ruleset.rules.chapter2.comments.CommentsRule
 import com.saveourtool.diktat.ruleset.rules.chapter2.comments.HeaderCommentRule
@@ -378,6 +379,13 @@ abstract class DiktatSmokeTestBase {
         fixAndCompare(prepareOverriddenRulesConfig(), "SemicolonsExpected.kt", "SemicolonsTest.kt")
     }
 
+    @Test
+    @Tag("DiktatRuleSetProvider")
+    @Timeout(TEST_TIMEOUT_SECONDS, unit = SECONDS)
+    fun `should add newlines between interfaces`() {
+        fixAndCompare(prepareOverriddenRulesConfig(), "NewlinesAfterInterfacesExpected.kt", "NewlinesAfterInterfacesTest.kt")
+    }
+
     abstract fun fixAndCompare(
         config: Path,
         expected: String,

diff --git a/diktat-cli/src/test/resources/test/smoke/src/main/kotlin/NewlinesAfterInterfacesExpected.kt b/diktat-cli/src/test/resources/test/smoke/src/main/kotlin/NewlinesAfterInterfacesExpected.kt
@@ -0,0 +1,6 @@
+package com.saveourtool.diktat
+
+class A<K : Any, P : Any, G : Any> :
+    B<K>(),
+    C<P>,
+    D<G> {}
diff --git a/diktat-cli/src/test/resources/test/smoke/src/main/kotlin/NewlinesAfterInterfacesTest.kt b/diktat-cli/src/test/resources/test/smoke/src/main/kotlin/NewlinesAfterInterfacesTest.kt
@@ -0,0 +1,3 @@
+package com.saveourtool.diktat
+
+class A<K : Any, P : Any, G : Any> : B<K>(), C<P>, D<G> {}
diff --git a/diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter3/EnumsSeparated.kt b/diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter3/EnumsSeparated.kt
@@ -5,6 +5,7 @@ import com.saveourtool.diktat.ruleset.constants.Warnings.ENUMS_SEPARATED
 import com.saveourtool.diktat.ruleset.rules.DiktatRule
 import com.saveourtool.diktat.ruleset.utils.AstNodePredicate
 import com.saveourtool.diktat.ruleset.utils.allSiblings
+import com.saveourtool.diktat.ruleset.utils.appendNewline
 import com.saveourtool.diktat.ruleset.utils.appendNewlineMergingWhiteSpace
 import com.saveourtool.diktat.ruleset.utils.getAllChildrenWithType
 import com.saveourtool.diktat.ruleset.utils.hasChildOfType
@@ -58,15 +59,6 @@ class EnumsSeparated(configRules: List<RulesConfig>) : DiktatRule(
         checkLastEnum(enumEntries.last())
     }
 
-    private fun ASTNode.appendNewline() {
-        val nextNode = this.treeNext
-        if (nextNode.elementType == WHITE_SPACE) {
-            (nextNode as LeafPsiElement).rawReplaceWithText("\n${nextNode.text}")
-        } else {
-            this.treeParent.addChild(PsiWhiteSpaceImpl("\n"), nextNode)
-        }
-    }
-
     private fun isEnumOneLine(nodes: List<ASTNode>) =
         nodes.dropLast(1).none { it.treeNext.isWhiteSpaceWithNewline() }
 

diff --git a/...rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter3/files/NewlinesRule.kt b/...rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter3/files/NewlinesRule.kt
@@ -7,10 +7,12 @@ import com.saveourtool.diktat.ruleset.constants.Warnings.COMPLEX_EXPRESSION
 import com.saveourtool.diktat.ruleset.constants.Warnings.WRONG_NEWLINES
 import com.saveourtool.diktat.ruleset.rules.DiktatRule
 import com.saveourtool.diktat.ruleset.utils.appendNewlineMergingWhiteSpace
+import com.saveourtool.diktat.ruleset.utils.changeWhiteSpaceOnNewline
 import com.saveourtool.diktat.ruleset.utils.emptyBlockList
 import com.saveourtool.diktat.ruleset.utils.findAllDescendantsWithSpecificType
 import com.saveourtool.diktat.ruleset.utils.findAllNodesWithCondition
 import com.saveourtool.diktat.ruleset.utils.findParentNodeWithSpecificType
+import com.saveourtool.diktat.ruleset.utils.getAllChildrenWithType
 import com.saveourtool.diktat.ruleset.utils.getFilePath
 import com.saveourtool.diktat.ruleset.utils.getFirstChildWithType
 import com.saveourtool.diktat.ruleset.utils.getIdentifierName
@@ -49,6 +51,8 @@ import org.jetbrains.kotlin.KtNodeTypes.REFERENCE_EXPRESSION
 import org.jetbrains.kotlin.KtNodeTypes.RETURN
 import org.jetbrains.kotlin.KtNodeTypes.SAFE_ACCESS_EXPRESSION
 import org.jetbrains.kotlin.KtNodeTypes.SECONDARY_CONSTRUCTOR
+import org.jetbrains.kotlin.KtNodeTypes.SUPER_TYPE_CALL_ENTRY
+import org.jetbrains.kotlin.KtNodeTypes.SUPER_TYPE_ENTRY
 import org.jetbrains.kotlin.KtNodeTypes.SUPER_TYPE_LIST
 import org.jetbrains.kotlin.KtNodeTypes.VALUE_ARGUMENT
 import org.jetbrains.kotlin.KtNodeTypes.VALUE_ARGUMENT_LIST
@@ -491,35 +495,116 @@ class NewlinesRule(configRules: List<RulesConfig>) : DiktatRule(
             }
         }
 
-    private fun handleValueParameterList(node: ASTNode, entryType: String) = node
-        .children()
-        .filter {
-            val isNewLineNext = it.treeNext?.isNewLineNode() ?: false
-            val isNewLinePrev = it.treePrev?.isNewLineNode() ?: false
-            (it.elementType == COMMA && !isNewLineNext) ||
-                    // Move RPAR to the new line
-                    (it.elementType == RPAR && it.treePrev?.elementType != COMMA && !isNewLinePrev)
+    /**
+     * Check that super classes are on separate lines (if there are three or more)
+     *
+     * @return true if there are less than three super classes or if all of them are on separate lines
+     */
+    private fun isCorrectSuperTypeList(valueParameterList: List<ASTNode>): Boolean {
+        val superTypeList = valueParameterList.filter { it.elementType in listOf(SUPER_TYPE_CALL_ENTRY, SUPER_TYPE_ENTRY) }
+        if (superTypeList.size <= 2) {
+            return true
         }
-        .toList()
-        .takeIf { it.isNotEmpty() }
-        ?.let { invalidCommas ->
-            val warnText = node.getParentIdentifier()?.let {
-                "$entryType should be placed on different lines in declaration of <${node.getParentIdentifier()}>"
-            } ?: "$entryType should be placed on different lines"
-            WRONG_NEWLINES.warnAndFix(configRules, emitWarn, isFixMode,
-                warnText, node.startOffset, node) {
-                invalidCommas.forEach { commaOrRpar ->
-                    val nextWhiteSpace = commaOrRpar.treeNext?.takeIf { it.elementType == WHITE_SPACE }
-                    if (commaOrRpar.elementType == COMMA) {
-                        nextWhiteSpace?.treeNext?.let {
-                            commaOrRpar.appendNewlineMergingWhiteSpace(nextWhiteSpace, nextWhiteSpace.treeNext)
-                        } ?: commaOrRpar.treeNext?.treeParent?.appendNewlineMergingWhiteSpace(nextWhiteSpace, commaOrRpar.treeNext)
-                    } else {
-                        commaOrRpar.treeParent?.appendNewlineMergingWhiteSpace(nextWhiteSpace, commaOrRpar)
+
+        val classDefinitionNode = valueParameterList[0].treeParent?.treeParent
+        val colonNodes = classDefinitionNode?.getAllChildrenWithType(COLON)
+        val newlineBeforeSuperTypeList = colonNodes?.find {colonNode ->
+            val newlineNode = colonNode.treeNext
+            val superClassType = newlineNode?.treeNext
+            newlineNode?.text?.count { it == '\n' } == 1 && superClassType?.elementType == SUPER_TYPE_LIST
+        }
+
+        // list elements are correct if they are sequence of (COMMA, '\n', supertype)
+        var areElementsCorrect = true
+        var valueParameter = valueParameterList[0].treeNext
+        while (valueParameter != null) {
+            val newlineNode = valueParameter.treeNext
+            val superClassType = valueParameter.treeNext?.treeNext
+            if (valueParameter.elementType != COMMA || newlineNode?.text?.count { it == '\n' } != 1 ||
+                    superClassType?.elementType != SUPER_TYPE_ENTRY) {
+                areElementsCorrect = false
+                break
+            }
+            valueParameter = superClassType?.treeNext
+        }
+        return newlineBeforeSuperTypeList != null && areElementsCorrect
+    }
+
+    private fun setSuperClassesOnSeparateLines(
+        node: ASTNode,
+        valueParameterList: List<ASTNode>,
+        warnText: String,
+        colonNodes: List<ASTNode>?
+    ) {
+        WRONG_NEWLINES.warnAndFix(
+            configRules, emitWarn, isFixMode,
+            warnText, node.startOffset, node
+        ) {
+            valueParameterList.forEach { superClassNode ->
+                val commaNode = superClassNode.treeNext
+                val whiteSpaceNode = commaNode?.treeNext
+                // put super classes on separate lines
+                if (superClassNode.elementType in listOf(SUPER_TYPE_CALL_ENTRY, SUPER_TYPE_ENTRY) &&
+                        commaNode?.text == "," && whiteSpaceNode?.elementType == WHITE_SPACE
+                ) {
+                    commaNode.changeWhiteSpaceOnNewline(whiteSpaceNode, commaNode)
+                }
+
+                // add newline before the first element of super class list
+                val colonNodeBeforeList = colonNodes?.find { colonNode ->
+                    colonNode.treeNext.text.count { it == '\n' } == 0 &&
+                            colonNode.treeNext.treeNext.elementType == SUPER_TYPE_LIST
+                }
+                colonNodeBeforeList?.changeWhiteSpaceOnNewline(colonNodeBeforeList.treeNext, colonNodeBeforeList)
+            }
+        }
+    }
+
+    private fun fixInvalidCommas(node: ASTNode, warnText: String) {
+        node.children()
+            .filter {
+                val isNewLineNext = it.treeNext?.isNewLineNode() ?: false
+                val isNewLinePrev = it.treePrev?.isNewLineNode() ?: false
+
+                (it.elementType == COMMA && !isNewLineNext) ||
+                        // Move RPAR to the new line
+                        (it.elementType == RPAR && it.treePrev?.elementType != COMMA && !isNewLinePrev)
+            }
+            .toList()
+            .takeIf { it.isNotEmpty() }
+            ?.let { invalidCommas ->
+                WRONG_NEWLINES.warnAndFix(
+                    configRules, emitWarn, isFixMode,
+                    warnText, node.startOffset, node
+                ) {
+                    invalidCommas.forEach { commaOrRpar ->
+                        val nextWhiteSpace = commaOrRpar.treeNext?.takeIf { it.elementType == WHITE_SPACE }
+                        if (commaOrRpar.elementType == COMMA) {
+                            nextWhiteSpace?.treeNext?.let {
+                                commaOrRpar.appendNewlineMergingWhiteSpace(nextWhiteSpace, nextWhiteSpace.treeNext)
+                            } ?: commaOrRpar.treeNext?.treeParent?.appendNewlineMergingWhiteSpace(nextWhiteSpace, commaOrRpar.treeNext)
+                        } else {
+                            commaOrRpar.treeParent?.appendNewlineMergingWhiteSpace(nextWhiteSpace, commaOrRpar)
+                        }
                     }
                 }
             }
+    }
+
+    private fun handleValueParameterList(node: ASTNode, entryType: String) {
+        val valueParameterList = node.children().toList()
+        val warnText = node.getParentIdentifier()?.let {
+            "$entryType should be placed on different lines in declaration of <${node.getParentIdentifier()}>"
+        } ?: "$entryType should be placed on different lines"
+        val classDefinitionNode = valueParameterList[0].treeParent?.treeParent
+        val colonNodes = classDefinitionNode?.getAllChildrenWithType(COLON)
+
+        if (!isCorrectSuperTypeList(valueParameterList)) {
+            setSuperClassesOnSeparateLines(node, valueParameterList, warnText, colonNodes)
+        } else {
+            fixInvalidCommas(node, warnText)
         }
+    }
 
     private fun ASTNode.isNewLineNode(): Boolean = this.run { elementType == WHITE_SPACE && textContains('\n') }
 

diff --git a/diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/utils/AstNodeUtils.kt b/diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/utils/AstNodeUtils.kt
@@ -621,6 +621,34 @@ fun ASTNode.appendNewlineMergingWhiteSpace(whiteSpaceNode: ASTNode?, beforeNode:
     }
 }
 
+/**
+ * Appends newline after this node
+ */
+fun ASTNode.appendNewline() {
+    val nextNode = this.treeNext
+    if (nextNode.elementType == WHITE_SPACE) {
+        (nextNode as LeafPsiElement).rawReplaceWithText("\n${nextNode.text}")
+    } else {
+        this.treeParent.addChild(PsiWhiteSpaceImpl("\n"), nextNode)
+    }
+}
+
+/**
+ * Changes any whitespace node on newline node
+ *
+ * @param whiteSpaceNode
+ * @param beforeNode
+ */
+fun ASTNode.changeWhiteSpaceOnNewline(whiteSpaceNode: ASTNode?, beforeNode: ASTNode?) {
+    if (whiteSpaceNode != null && whiteSpaceNode.elementType == WHITE_SPACE) {
+        if (whiteSpaceNode.text.lines().size == 1) {
+            (whiteSpaceNode as LeafPsiElement).rawReplaceWithText("\n")
+        }
+    } else {
+        addChild(PsiWhiteSpaceImpl("\n"), beforeNode)
+    }
+}
+
 /**
  * Transforms last line of this WHITE_SPACE to exactly [indent] spaces
  */

diff --git a/...t-rules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter3/SuperClassListWarnTest.kt b/...t-rules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter3/SuperClassListWarnTest.kt
@@ -0,0 +1,86 @@
+package com.saveourtool.diktat.ruleset.chapter3
+
+import com.saveourtool.diktat.common.config.rules.DIKTAT_RULE_SET_ID
+import com.saveourtool.diktat.ruleset.constants.Warnings.WRONG_NEWLINES
+import com.saveourtool.diktat.ruleset.rules.chapter3.files.NewlinesRule
+import com.saveourtool.diktat.util.LintTestBase
+
+import com.saveourtool.diktat.api.DiktatError
+import com.saveourtool.diktat.ruleset.constants.Warnings
+import generated.WarningNames
+import org.junit.jupiter.api.Tag
+import org.junit.jupiter.api.Test
+
+class SuperClassListWarnTest : LintTestBase(::NewlinesRule) {
+    private val ruleId = "$DIKTAT_RULE_SET_ID:${NewlinesRule.NAME_ID}"
+
+    @Test
+    @Tag(WarningNames.WRONG_NEWLINES)
+    fun `superclass list on the same line`() {
+        lintMethod(
+            """
+                    |package com.saveourtool.diktat
+                    |
+                    |class A<K : Any, P : Any, G : Any> : B<K>(), C<P>, D<G> {}
+            """.trimMargin(),
+            DiktatError(3, 38, ruleId, "${Warnings.WRONG_NEWLINES.warnText()} supertype list entries should be placed on different lines in declaration of <A>", true),
+        )
+    }
+
+    @Test
+    @Tag(WarningNames.WRONG_NEWLINES)
+    fun `first superclass also on a new line`() {
+        lintMethod(
+            """
+                    |package com.saveourtool.diktat
+                    |
+                    |class A<K : Any, P : Any, G : Any> : B<K>(),
+                    |C<P>,
+                    |D<G> {}
+            """.trimMargin(),
+            DiktatError(3, 38, ruleId, "${Warnings.WRONG_NEWLINES.warnText()} supertype list entries should be placed on different lines in declaration of <A>", true),
+        )
+    }
+
+    @Test
+    @Tag(WarningNames.WRONG_NEWLINES)
+    fun `superclass list of 2 elements on the same line`() {
+        lintMethod(
+            """
+                    |package com.saveourtool.diktat
+                    |
+                    |class A<K : Any, P : Any, G : Any> : B<K>(), C<P> {}
+            """.trimMargin(),
+        )
+    }
+
+    @Test
+    @Tag(WarningNames.WRONG_NEWLINES)
+    fun `superclass list on separate lines`() {
+        lintMethod(
+            """
+                    |package com.saveourtool.diktat
+                    |
+                    |class A<K : Any, P : Any, G : Any> :
+                    |B<K>(),
+                    |C<P>,
+                    |D<G> {}
+            """.trimMargin(),
+        )
+    }
+
+    @Test
+    @Tag(WarningNames.WRONG_NEWLINES)
+    fun `superclass list different whitespaces`() {
+        lintMethod(
+            """
+                    |package com.saveourtool.diktat
+                    |
+                    |class A<K : Any, P : Any, G : Any> :
+                    |B<K>(),
+                    |    C<P>, D<G> {}
+            """.trimMargin(),
+            DiktatError(4, 1, ruleId, "${Warnings.WRONG_NEWLINES.warnText()} supertype list entries should be placed on different lines in declaration of <A>", true),
+            )
+    }
+}
diff --git a/...ules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter3/files/NewlinesRuleFixTest.kt b/...ules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter3/files/NewlinesRuleFixTest.kt
@@ -69,6 +69,12 @@ class NewlinesRuleFixTest : FixTestBase("test/paragraph3/newlines", ::NewlinesRu
         fixAndCompare("ParameterListExpected.kt", "ParameterListTest.kt")
     }
 
+    @Test
+    @Tag(WarningNames.WRONG_NEWLINES)
+    fun `should insert newlines in supertype list`() {
+        fixAndCompare("SuperClassListOnTheSameLineExpected.kt", "SuperClassListOnTheSameLineTest.kt")
+    }
+
     @Test
     @Tag(WarningNames.WRONG_NEWLINES)
     fun `should fix one line function`() {

diff --git a/diktat-rules/src/test/resources/test/paragraph3/newlines/ParameterListExpected.kt b/diktat-rules/src/test/resources/test/paragraph3/newlines/ParameterListExpected.kt
@@ -6,9 +6,10 @@ arg1: Int,
  arg3: Int
 ) { }
 
-class Foo : FooBase<Bar>(),
- BazInterface,
- BazSuperclass { }
+class Foo :
+FooBase<Bar>(),
+BazInterface,
+BazSuperclass { }
 
 class Foo(val arg1: Int, arg2: Int) { }
 
@@ -31,4 +32,4 @@ arg1,
 class Foo(val arg1: Int,
           var arg2: Int,
           arg3: Int
-) { }
+) { }
diff --git a/diktat-rules/src/test/resources/test/paragraph3/newlines/ParameterListTest.kt b/diktat-rules/src/test/resources/test/paragraph3/newlines/ParameterListTest.kt
@@ -12,4 +12,4 @@ class Foo(val arg1: Int, arg2: Int, arg3: Int) {
 
 class Foo(val arg1: Int,
           var arg2: Int,
-          arg3: Int) { }
+          arg3: Int) { }
diff --git a/...-rules/src/test/resources/test/paragraph3/newlines/SuperClassListOnTheSameLineExpected.kt b/...-rules/src/test/resources/test/paragraph3/newlines/SuperClassListOnTheSameLineExpected.kt
@@ -0,0 +1,6 @@
+package test.paragraph3.newlines
+
+class A<K : Any, P : Any, G : Any> :
+B<K>(),
+C<P>,
+D<G> {}
diff --git a/diktat-rules/src/test/resources/test/paragraph3/newlines/SuperClassListOnTheSameLineTest.kt b/diktat-rules/src/test/resources/test/paragraph3/newlines/SuperClassListOnTheSameLineTest.kt
@@ -0,0 +1,3 @@
+package test.paragraph3.newlines
+
+class A<K : Any, P : Any, G : Any> : B<K>(),     C<P>, D<G> {}