Repository: analysis-dev/diktat (kotlin)

Issue:
FP of LOCAL_VARIABLE_EARLY_DECLARATION with other variables and a loop
## Describe the bug
```kotlin
    private fun collectAllExtensionFunctions(node: ASTNode): SimilarSignatures {
        val extensionFunctionList = node.findAllNodesWithSpecificType(FUN).filter { it.hasChildOfType(TYPE_REFERENCE) && it.hasChildOfType(DOT) }
        val distinctFunctionSignatures = mutableMapOf<FunctionSignature, ASTNode>()  // maps function signatures on node it is used by
        val extensionFunctionsPairs = mutableListOf<Pair<ExtensionFunction, ExtensionFunction>>()  // pairs extension functions with same signature

        extensionFunctionList.forEach { func ->
            val functionName = (func.psi as KtNamedFunction).name!!
            // List<String> is used to show param names in warning
            val params = (func.getFirstChildWithType(VALUE_PARAMETER_LIST)!!.psi as KtParameterList).parameters.map { it.name!! }
            val returnType = func.findChildAfter(COLON, TYPE_REFERENCE)?.text
            val className = func.findChildBefore(DOT, TYPE_REFERENCE)!!.text
            val signature = FunctionSignature(functionName, params, returnType)

            if (distinctFunctionSignatures.contains(signature)) {
                val secondFuncClassName = distinctFunctionSignatures[signature]!!.findChildBefore(DOT, TYPE_REFERENCE)!!.text
                extensionFunctionsPairs.add(Pair(
                    ExtensionFunction(secondFuncClassName, signature, distinctFunctionSignatures[signature]!!),
                    ExtensionFunction(className, signature, func)))
            } else {
                distinctFunctionSignatures[signature] = func
            }
        }

        return extensionFunctionsPairs
    }
```
=>
```
ExtensionFunctionsSameNameRule.kt:82:9: [LOCAL_VARIABLE_EARLY_DECLARATION] local variables should be declared close to the line where they are first used: <distinctFunctionSignatures> is declared on line <82> and is used for the first time on line <85>
```

## Environment information
* diktat version: 0.1.5

Pull request:
### What's done:
 * Fixed bugs
 * Added tests
 
This pull request closes #581

Patch:
diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ExtensionFunctionsSameNameRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ExtensionFunctionsSameNameRule.kt
index 2b852b4784..3d6d6432be 100644
--- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ExtensionFunctionsSameNameRule.kt
+++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ExtensionFunctionsSameNameRule.kt
@@ -76,10 +76,7 @@ class ExtensionFunctionsSameNameRule(private val configRules: List<RulesConfig>)
         return pairs
     }
 
-    /**
-     * FixMe: warning suppressed until https://github.com/cqfn/diKTat/issues/581
-     */
-    @Suppress("UnsafeCallOnNullableType", "LOCAL_VARIABLE_EARLY_DECLARATION", "TYPE_ALIAS")
+    @Suppress("UnsafeCallOnNullableType", "TYPE_ALIAS")
     private fun collectAllExtensionFunctions(node: ASTNode): SimilarSignatures {
         val extensionFunctionList = node.findAllNodesWithSpecificType(FUN).filter { it.hasChildOfType(TYPE_REFERENCE) && it.hasChildOfType(DOT) }
         val distinctFunctionSignatures: MutableMap<FunctionSignature, ASTNode> = mutableMapOf()  // maps function signatures on node it is used by
diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/LineLength.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/LineLength.kt
index d3deb5d092..50d39a7dad 100644
--- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/LineLength.kt
+++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/LineLength.kt
@@ -81,7 +81,7 @@ class LineLength(private val configRules: List<RulesConfig>) : Rule("line-length
         }
     }
 
-    @Suppress("UnsafeCallOnNullableType", "LOCAL_VARIABLE_EARLY_DECLARATION")
+    @Suppress("UnsafeCallOnNullableType")
     private fun checkLength(node: ASTNode, configuration: LineLengthConfiguration) {
         var offset = 0
         node.text.lines().forEach {
diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/SmartCastRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/SmartCastRule.kt
index ec94aa9bb7..ad4a3fd533 100644
--- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/SmartCastRule.kt
+++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/SmartCastRule.kt
@@ -65,7 +65,7 @@ class SmartCastRule(private val configRules: List<RulesConfig>) : Rule("smart-ca
     }
 
     // Divide in is and as expr
-    @Suppress("TYPE_ALIAS", "LOCAL_VARIABLE_EARLY_DECLARATION")
+    @Suppress("TYPE_ALIAS")
     private fun handleProp(propMap: Map<KtProperty, List<KtNameReferenceExpression>>) {
         propMap.forEach { (property, references) ->
             val isExpr: MutableList<KtNameReferenceExpression> = mutableListOf()
diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/SortRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/SortRule.kt
index 9837d2b705..47abc84e10 100644
--- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/SortRule.kt
+++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/SortRule.kt
@@ -90,10 +90,10 @@ class SortRule(private val configRules: List<RulesConfig>) : Rule("sort-rule") {
         }
     }
 
-    @Suppress("LOCAL_VARIABLE_EARLY_DECLARATION", "TYPE_ALIAS")
+    @Suppress("TYPE_ALIAS")
     private fun createOrderListOfList(propertyList: List<ASTNode>): List<List<ASTNode>> {
-        val orderListOfList: MutableList<MutableList<ASTNode>> = mutableListOf()
         var oneOrderList = mutableListOf(propertyList.first())
+        val orderListOfList: MutableList<MutableList<ASTNode>> = mutableListOf()
         propertyList.zipWithNext().forEach { (currentProperty, nextProperty) ->
             if (currentProperty.nextSibling { it.elementType == PROPERTY } == nextProperty) {
                 oneOrderList.add(nextProperty)
diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/identifiers/LocalVariablesRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/identifiers/LocalVariablesRule.kt
index 6f8204bdf2..5847964df3 100644
--- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/identifiers/LocalVariablesRule.kt
+++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/identifiers/LocalVariablesRule.kt
@@ -7,10 +7,12 @@ import org.cqfn.diktat.ruleset.utils.containsOnlyConstants
 import org.cqfn.diktat.ruleset.utils.getDeclarationScope
 import org.cqfn.diktat.ruleset.utils.getLineNumber
 import org.cqfn.diktat.ruleset.utils.lastLineNumber
+import org.cqfn.diktat.ruleset.utils.numNewLines
 import org.cqfn.diktat.ruleset.utils.search.findAllVariablesWithUsages
 
 import com.pinterest.ktlint.core.Rule
 import com.pinterest.ktlint.core.ast.ElementType.FILE
+import com.pinterest.ktlint.core.ast.ElementType.WHITE_SPACE
 import com.pinterest.ktlint.core.ast.isPartOfComment
 import org.jetbrains.kotlin.com.intellij.lang.ASTNode
 import org.jetbrains.kotlin.com.intellij.psi.PsiElement
@@ -94,12 +96,31 @@ class LocalVariablesRule(private val configRules: List<RulesConfig>) : Rule("loc
     }
 
     private fun handleConsecutiveDeclarations(statement: PsiElement, properties: List<KtProperty>) {
+        val numLinesAfterLastProp =
+                properties
+                    .last()
+                    .node
+                    .treeNext
+                    .takeIf { it.elementType == WHITE_SPACE }
+                    ?.let {
+                        // minus one is needed to except \n after property
+                        it.numNewLines() - 1
+                    }
+                    ?: 0
+
         // need to check that properties are declared consecutively with only maybe empty lines
         properties
             .sortedBy { it.node.getLineNumber() }
-            .zip(properties.size - 1 downTo 0)
-            .forEach { (property, offset) ->
-                checkLineNumbers(property, statement.node.getLineNumber(), offset)
+            .zip(
+                (properties.size - 1 downTo 0).map { it + numLinesAfterLastProp }
+            )
+            .forEachIndexed { index, (property, offset) ->
+                if (index != properties.lastIndex) {
+                    checkLineNumbers(property, statement.node.getLineNumber(), offset)
+                } else {
+                    // since offset after last property is calculated in this method, we pass offset = 0
+                    checkLineNumbers(property, statement.node.getLineNumber(), 0)
+                }
             }
     }
 
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 eae3ab814b..e4f1f59590 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
@@ -636,7 +636,7 @@ fun List<ASTNode>.handleIncorrectOrder(
  * This method returns text of this [ASTNode] plus text from it's siblings after last and until next newline, if present in siblings.
  * I.e., if this node occupies no more than a single line, this whole line or it's part will be returned.
  */
-@Suppress("LOCAL_VARIABLE_EARLY_DECLARATION", "WRONG_NEWLINES")
+@Suppress("WRONG_NEWLINES")
 fun ASTNode.extractLineOfText(): String {
     var text: MutableList<String> = mutableListOf()
     siblings(false)
@@ -743,7 +743,6 @@ fun ASTNode.getLineNumber(): Int =
  * This function calculates line number instead of using cached values.
  * It should be used when AST could be previously mutated by auto fixers.
  */
-@Suppress("LOCAL_VARIABLE_EARLY_DECLARATION")
 private fun ASTNode.calculateLineNumber(): Int {
     var count = 0
     // todo use runningFold or something similar when we migrate to apiVersion 1.4


Test patch:
diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/LocalVariablesWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/LocalVariablesWarnTest.kt
index b7d08e82cc..1fcc78bf3a 100644
--- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/LocalVariablesWarnTest.kt
+++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/LocalVariablesWarnTest.kt
@@ -471,7 +471,7 @@ class LocalVariablesWarnTest : LintTestBase(::LocalVariablesRule) {
 
     @Test
     @Tag(WarningNames.LOCAL_VARIABLE_EARLY_DECLARATION)
-    fun `should emit only one warning when same variables are used more than once`() {
+    fun `should not trigger on properties`() {
         lintMethod(
             """
                     |private fun checkDoc(node: ASTNode, warning: Warnings) {
@@ -483,9 +483,7 @@ class LocalVariablesWarnTest : LintTestBase(::LocalVariablesRule) {
                     |        foo(a, b, c)
                     |    }
                     |}
-                """.trimMargin(),
-            LintError(2, 5, ruleId, "${LOCAL_VARIABLE_EARLY_DECLARATION.warnText()} ${warnMessage("a", 2, 6)}", false),
-            LintError(3, 5, ruleId, "${LOCAL_VARIABLE_EARLY_DECLARATION.warnText()} ${warnMessage("b", 3, 6)}", false)
+                """.trimMargin()
         )
     }
 
@@ -573,4 +571,30 @@ class LocalVariablesWarnTest : LintTestBase(::LocalVariablesRule) {
                 """.trimMargin()
         )
     }
+
+    @Test
+    @Tag(WarningNames.LOCAL_VARIABLE_EARLY_DECLARATION)
+    fun `should not trigger on space after last val`() {
+        lintMethod(
+            """
+                    |    private fun collectAllExtensionFunctions(node: ASTNode): SimilarSignatures {
+                    |       val extensionFunctionList = node.findAllNodesWithSpecificType(FUN).filter { it.hasChildOfType(TYPE_REFERENCE) && it.hasChildOfType(DOT) }
+                    |       val distinctFunctionSignatures = mutableMapOf<FunctionSignature, ASTNode>()  // maps function signatures on node it is used by
+                    |       val extensionFunctionsPairs = mutableListOf<Pair<ExtensionFunction, ExtensionFunction>>()  // pairs extension functions with same signature
+                    |       
+                    |       extensionFunctionList.forEach { func ->
+                    |           if (distinctFunctionSignatures.contains(signature)) {
+                    |               val secondFuncClassName = distinctFunctionSignatures[signature]!!.findChildBefore(DOT, TYPE_REFERENCE)!!.text
+                    |               extensionFunctionsPairs.add(Pair(
+                    |               ExtensionFunction(secondFuncClassName, signature, distinctFunctionSignatures[signature]!!),
+                    |               ExtensionFunction(className, signature, func)))
+                    |           } else {
+                    |               distinctFunctionSignatures[signature] = func
+                    |           }
+                    |       }
+                    |       return extensionFunctionsPairs
+                    |   }
+                """.trimMargin()
+        )
+    }
 }