Repository: analysis-dev/diktat (kotlin)

Issue:
Describe and implement rules for newlines/indentation when function argument is split
For example, for diktat 0.1.3 this is valid code:
```kotlin
fun foo(x
    : Int,
        y: Int) { }
```
Also, newline before `:` looks like bug in `NewlinesRule`.

Pull request:
### What's done:
* Changed warning
* Added new test

This pull request closes #530

## Actions checklist
* [ ] Added tests on checks
* [ ] Added tests on fixers

Patch:
diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt
index 54ef1927b8..cc115cd7ce 100644
--- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt
+++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt
@@ -6,7 +6,9 @@ import org.cqfn.diktat.common.config.rules.isRuleEnabled
 import org.cqfn.diktat.ruleset.utils.hasSuppress
 import org.jetbrains.kotlin.com.intellij.lang.ASTNode
 
-typealias EmitType = ((offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit)
+typealias EmitType = ((offset: Int,
+                       errorMessage: String,
+                       canBeAutoCorrected: Boolean) -> Unit)
 
 typealias ListOfList = MutableList<MutableList<ASTNode>>
 
diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/files/NewlinesRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/files/NewlinesRule.kt
index bf98038dfe..01829f787e 100644
--- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/files/NewlinesRule.kt
+++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/files/NewlinesRule.kt
@@ -8,7 +8,18 @@ import org.cqfn.diktat.ruleset.constants.Warnings.COMPLEX_EXPRESSION
 import org.cqfn.diktat.ruleset.constants.Warnings.REDUNDANT_SEMICOLON
 import org.cqfn.diktat.ruleset.constants.Warnings.WRONG_NEWLINES
 import org.cqfn.diktat.ruleset.rules.DiktatRule
-import org.cqfn.diktat.ruleset.utils.*
+import org.cqfn.diktat.ruleset.utils.appendNewlineMergingWhiteSpace
+import org.cqfn.diktat.ruleset.utils.emptyBlockList
+import org.cqfn.diktat.ruleset.utils.extractLineOfText
+import org.cqfn.diktat.ruleset.utils.findAllNodesWithSpecificType
+import org.cqfn.diktat.ruleset.utils.getIdentifierName
+import org.cqfn.diktat.ruleset.utils.hasParent
+import org.cqfn.diktat.ruleset.utils.isBeginByNewline
+import org.cqfn.diktat.ruleset.utils.isEol
+import org.cqfn.diktat.ruleset.utils.isFollowedByNewline
+import org.cqfn.diktat.ruleset.utils.isSingleLineIfElse
+import org.cqfn.diktat.ruleset.utils.leaveOnlyOneNewLine
+import org.cqfn.diktat.ruleset.utils.log
 
 import com.pinterest.ktlint.core.ast.ElementType.ANDAND
 import com.pinterest.ktlint.core.ast.ElementType.ARROW
@@ -111,6 +122,7 @@ class NewlinesRule(configRules: List<RulesConfig>) : DiktatRule(
             in lineBreakBeforeOperators -> handleOperatorWithLineBreakBefore(node)
             LPAR -> handleOpeningParentheses(node)
             COMMA -> handleComma(node)
+            COLON -> handleColon(node)
             BLOCK -> handleLambdaBody(node)
             RETURN -> handleReturnStatement(node)
             SUPER_TYPE_LIST, VALUE_PARAMETER_LIST, VALUE_ARGUMENT_LIST -> handleList(node)
@@ -231,6 +243,40 @@ class NewlinesRule(configRules: List<RulesConfig>) : DiktatRule(
         }
     }
 
+    /**
+     * Check that newline is not placed before or after a colon
+     */
+    private fun handleColon(node: ASTNode) {
+        val prevNewLine = node
+            .parent({ it.treePrev != null }, strict = false)
+            ?.treePrev
+            ?.takeIf {
+                it.elementType == WHITE_SPACE && it.text.contains("\n")
+            }
+        prevNewLine?.let { whiteSpace ->
+            WRONG_NEWLINES.warnAndFix(configRules, emitWarn, isFixMode, "newline shouldn't be placed before colon", node.startOffset, node) {
+                whiteSpace.treeParent.removeChild(whiteSpace)
+
+                if (node.hasParent(VALUE_PARAMETER_LIST)) {
+                    // If inside the list of arguments the rule for a new line before the colon has worked,
+                    // then we delete the new line on both sides of the colon
+                    whiteSpace.treeParent?.let {
+                        val nextNewLine = node
+                            .parent({ it.treeNext != null }, strict = false)
+                            ?.treeNext
+                            ?.takeIf {
+                                it.elementType == WHITE_SPACE && it.text.contains("\n")
+                            }
+                        nextNewLine?.let {
+                            it.treeParent.addChild(PsiWhiteSpaceImpl(" "), it)
+                            it.treeParent.removeChild(it)
+                        }
+                    }
+                }
+            }
+        }
+    }
+
     private fun handleLambdaBody(node: ASTNode) {
         if (node.treeParent.elementType == FUNCTION_LITERAL) {
             val isSingleLineLambda = node.treeParent.text.lines().size == 1
diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter4/SmartCastRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter4/SmartCastRule.kt
index e34ceb96cb..ba59348f89 100644
--- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter4/SmartCastRule.kt
+++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter4/SmartCastRule.kt
@@ -117,8 +117,7 @@ class SmartCastRule(configRules: List<RulesConfig>) : DiktatRule(
     @Suppress("TYPE_ALIAS")
     private fun groupIsAndAsExpr(isExpr: List<KtNameReferenceExpression>,
                                  asExpr: List<KtNameReferenceExpression>,
-                                 prop: KtProperty)
-    : Map<KtNameReferenceExpression, List<KtNameReferenceExpression>> {
+                                 prop: KtProperty): Map<KtNameReferenceExpression, List<KtNameReferenceExpression>> {
         if (isExpr.isEmpty() && asExpr.isNotEmpty()) {
             handleZeroIsCase(asExpr, prop)
             return emptyMap()
@@ -267,8 +266,7 @@ class SmartCastRule(configRules: List<RulesConfig>) : DiktatRule(
      * @return Map of property and list of expressions
      */
     @Suppress("TYPE_ALIAS")
-    private fun collectReferenceList(propertiesToUsages: Map<KtProperty, List<KtNameReferenceExpression>>)
-    : Map<KtProperty, List<KtNameReferenceExpression>> =
+    private fun collectReferenceList(propertiesToUsages: Map<KtProperty, List<KtNameReferenceExpression>>): Map<KtProperty, List<KtNameReferenceExpression>> =
             propertiesToUsages.mapValues { (_, value) ->
                 value.filter { entry ->
                     entry.parent.node.elementType == IS_EXPRESSION || entry.parent.node.elementType == BINARY_WITH_TYPE
diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt
index a7a52b8a82..b872f1bb30 100644
--- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt
+++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt
@@ -422,7 +422,8 @@ fun ASTNode.findAllNodesWithSpecificType(elementType: IElementType, withSelf: Bo
  * This method performs tree traversal and returns all nodes which satisfy the condition
  */
 @Suppress("LAMBDA_IS_NOT_LAST_PARAMETER")
-fun ASTNode.findAllNodesWithCondition(condition: (ASTNode) -> Boolean, withSelf: Boolean = true): List<ASTNode> {
+fun ASTNode.findAllNodesWithCondition(condition: (ASTNode) -> Boolean,
+                                      withSelf: Boolean = true): List<ASTNode> {
     val result = if (condition(this) && withSelf) mutableListOf(this) else mutableListOf()
     return result + this.getChildren(null).flatMap {
         it.findAllNodesWithCondition(condition)
@@ -443,14 +444,16 @@ fun ASTNode.findParentNodeWithSpecificType(elementType: IElementType) =
 /**
  * Finds all children of optional type which match the predicate
  */
-fun ASTNode.findChildrenMatching(elementType: IElementType? = null, predicate: (ASTNode) -> Boolean): List<ASTNode> =
+fun ASTNode.findChildrenMatching(elementType: IElementType? = null,
+                                 predicate: (ASTNode) -> Boolean): List<ASTNode> =
         getChildren(elementType?.let { TokenSet.create(it) })
             .filter(predicate)
 
 /**
  * Check if this node has any children of optional type matching the predicate
  */
-fun ASTNode.hasChildMatching(elementType: IElementType? = null, predicate: (ASTNode) -> Boolean): Boolean =
+fun ASTNode.hasChildMatching(elementType: IElementType? = null,
+                             predicate: (ASTNode) -> Boolean): Boolean =
         findChildrenMatching(elementType, predicate).isNotEmpty()
 
 /**
diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesSearch.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesSearch.kt
index 86985471e4..be144ce1ca 100644
--- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesSearch.kt
+++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesSearch.kt
@@ -31,7 +31,8 @@ import org.jetbrains.kotlin.psi.psiUtil.referenceExpression
  *  it should be ONLY node of File elementType
  *  @param filterForVariables condition to filter
  */
-abstract class VariablesSearch(val node: ASTNode, private val filterForVariables: (KtProperty) -> Boolean) {
+abstract class VariablesSearch(val node: ASTNode,
+                               private val filterForVariables: (KtProperty) -> Boolean) {
     /**
      * to complete implementation of a search mechanism you need to specify what and how you will search in current scope
      * [this] - scope where to search the usages/assignments/e.t.c of the variable (can be of types KtBlockExpression/KtFile/KtClassBody)


Test patch:
diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/files/NewlinesRuleFixTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/files/NewlinesRuleFixTest.kt
index 0cda5145d4..12372c7ee8 100644
--- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/files/NewlinesRuleFixTest.kt
+++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/files/NewlinesRuleFixTest.kt
@@ -45,6 +45,18 @@ class NewlinesRuleFixTest : FixTestBase("test/paragraph3/newlines", ::NewlinesRu
         fixAndCompare("CommaExpected.kt", "CommaTest.kt")
     }
 
+    @Test
+    @Tag(WarningNames.WRONG_NEWLINES)
+    fun `should fix wrong newlines before colon`() {
+        fixAndCompare("ColonExpected.kt", "ColonTest.kt")
+    }
+
+    @Test
+    @Tag(WarningNames.WRONG_NEWLINES)
+    fun `One line parameters list sheet must contain no more than 2 parameters`() {
+        fixAndCompare("sizeParameterListExpected.kt", "sizeParameterListTest.kt")
+    }
+
     @Test
     @Tag(WarningNames.WRONG_NEWLINES)
     fun `should fix wrong newlines in lambdas`() {
diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/files/NewlinesRuleWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/files/NewlinesRuleWarnTest.kt
index 06973b02e1..233e468308 100644
--- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/files/NewlinesRuleWarnTest.kt
+++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/files/NewlinesRuleWarnTest.kt
@@ -31,9 +31,11 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) {
     private val functionalStyleWarn = "${WRONG_NEWLINES.warnText()} should follow functional style at"
     private val lparWarn = "${WRONG_NEWLINES.warnText()} opening parentheses should not be separated from constructor or function name"
     private val commaWarn = "${WRONG_NEWLINES.warnText()} newline should be placed only after comma"
+    private val prevColonWarn = "${WRONG_NEWLINES.warnText()} newline shouldn't be placed before colon"
     private val lambdaWithArrowWarn = "${WRONG_NEWLINES.warnText()} in lambda with several lines in body newline should be placed after an arrow"
     private val lambdaWithoutArrowWarn = "${WRONG_NEWLINES.warnText()} in lambda with several lines in body newline should be placed after an opening brace"
     private val singleReturnWarn = "${WRONG_NEWLINES.warnText()} functions with single return statement should be simplified to expression body"
+    private val listParameter = "${WRONG_NEWLINES.warnText()} list parameter should be placed on different lines in declaration of <foo>"
 
     @Test
     @Tag(WarningNames.REDUNDANT_SEMICOLON)
@@ -313,6 +315,68 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) {
         )
     }
 
+    @Test
+    @Tag(WarningNames.WRONG_NEWLINES)
+    fun `newline shouldn't be placed before colon`() {
+        lintMethod(
+            """
+                    |fun foo(a
+                    |        : Int,
+                    |        b
+                    |        : Int) {
+                    |    bar(a, b)
+                    |}
+            """.trimMargin(),
+            LintError(2, 9, ruleId, prevColonWarn, true),
+            LintError(4, 9, ruleId, prevColonWarn, true)
+        )
+    }
+
+    @Test
+    @Tag(WarningNames.WRONG_NEWLINES)
+    fun `One line parameters list sheet must contain no more than 2 parameters`() {
+        lintMethod(
+            """
+                    |fun foo(a: Int, b: Int, c: Int) {
+                    |      bar(a, b)
+                    |}
+            """.trimMargin(),
+            LintError(1, 8, ruleId, "${WRONG_NEWLINES.warnText()} first parameter should be placed on a separate line or all other parameters " +
+                    "should be aligned with it in declaration of <foo>", true),
+            LintError(1, 8, ruleId, "${WRONG_NEWLINES.warnText()} value parameters should be placed on different lines in declaration of <foo>", true)
+        )
+    }
+
+    @Test
+    @Tag(WarningNames.WRONG_NEWLINES)
+    fun `newline after colon`() {
+        lintMethod(
+            """
+                    |fun foo(a:
+                    |      Int,
+                    |      b:
+                    |      Int) {
+                    |      bar(a, b)
+                    |}
+            """.trimMargin()
+        )
+    }
+
+    @Test
+    @Tag(WarningNames.WRONG_NEWLINES)
+    fun `list parameter should be placed on different lines`() {
+        lintMethod(
+            """
+                    |fun foo(
+                    |      a: Int, 
+                    |      b: Int,
+                    |      c: Int
+                    |      ) {
+                    |}
+            """.trimMargin(),
+        )
+    }
+
     @Test
     @Tag(WarningNames.WRONG_NEWLINES)
     fun `function name should not be separated from ( - positive example`() {
diff --git a/diktat-rules/src/test/resources/test/paragraph3/newlines/ColonExpected.kt b/diktat-rules/src/test/resources/test/paragraph3/newlines/ColonExpected.kt
new file mode 100644
index 0000000000..81edec3196
--- /dev/null
+++ b/diktat-rules/src/test/resources/test/paragraph3/newlines/ColonExpected.kt
@@ -0,0 +1,4 @@
+package test.paragraph3.newlines
+
+fun foo(a: Int,
+        b: Int) { }
\ No newline at end of file
diff --git a/diktat-rules/src/test/resources/test/paragraph3/newlines/ColonTest.kt b/diktat-rules/src/test/resources/test/paragraph3/newlines/ColonTest.kt
new file mode 100644
index 0000000000..842f0c3dcc
--- /dev/null
+++ b/diktat-rules/src/test/resources/test/paragraph3/newlines/ColonTest.kt
@@ -0,0 +1,6 @@
+package test.paragraph3.newlines
+
+fun foo(a
+        : Int,
+        b
+        : Int) { }
diff --git a/diktat-rules/src/test/resources/test/paragraph3/newlines/sizeParameterListExpected.kt b/diktat-rules/src/test/resources/test/paragraph3/newlines/sizeParameterListExpected.kt
new file mode 100644
index 0000000000..a6238bcb0a
--- /dev/null
+++ b/diktat-rules/src/test/resources/test/paragraph3/newlines/sizeParameterListExpected.kt
@@ -0,0 +1,8 @@
+package test.paragraph3.newlines
+
+fun foo(
+a: Int,
+ b: Int,
+ c: Int) {
+    bar(a, b, c)
+}
\ No newline at end of file
diff --git a/diktat-rules/src/test/resources/test/paragraph3/newlines/sizeParameterListTest.kt b/diktat-rules/src/test/resources/test/paragraph3/newlines/sizeParameterListTest.kt
new file mode 100644
index 0000000000..f37d687c44
--- /dev/null
+++ b/diktat-rules/src/test/resources/test/paragraph3/newlines/sizeParameterListTest.kt
@@ -0,0 +1,5 @@
+package test.paragraph3.newlines
+
+fun foo(a: Int, b: Int, c: Int) {
+    bar(a, b, c)
+}
\ No newline at end of file