Repository: analysis-dev/diktat (kotlin)

Issue:
IndexOutOfBoundsException when calculating line number in fixing mode
For example, when `diktat:fix@diktat` is run on `TestComparatorUnit` as of commit https://github.com/cqfn/diKTat/commit/6dbf1b0639c718bee8fb2e150d3e8ca3046a91af, the following error occurs (error arises from tree mutations and `getLineNumber` operating on old offsets and doesn't depend on runner):
```
[ERROR] Unhandled exception during rule execution: 
com.pinterest.ktlint.core.RuleExecutionException: java.lang.IndexOutOfBoundsException: Wrong offset: 2040. Should be in range: [0, 2019]
    at com.pinterest.ktlint.core.KtLint$format$2.invoke (KtLint.kt:384)
    at com.pinterest.ktlint.core.KtLint$format$2.invoke (KtLint.kt:43)
    at com.pinterest.ktlint.core.KtLint$visitor$2$1.invoke (KtLint.kt:246)
    at com.pinterest.ktlint.core.KtLint$visitor$2$1.invoke (KtLint.kt:43)
    at com.pinterest.ktlint.core.KtLint.visit (KtLint.kt:572)
    at com.pinterest.ktlint.core.KtLint.visit (KtLint.kt:573)
    at com.pinterest.ktlint.core.KtLint.visit (KtLint.kt:573)
    at com.pinterest.ktlint.core.KtLint.visit (KtLint.kt:573)
    at com.pinterest.ktlint.core.KtLint.access$visit (KtLint.kt:43)
    at com.pinterest.ktlint.core.KtLint$visitor$2.invoke (KtLint.kt:244)
    at com.pinterest.ktlint.core.KtLint$visitor$2.invoke (KtLint.kt:43)
    at com.pinterest.ktlint.core.KtLint.format (KtLint.kt:369)
    at org.cqfn.diktat.plugin.maven.DiktatFixMojo.runAction (DiktatMojo.kt:38)
    at org.cqfn.diktat.plugin.maven.DiktatBaseMojo.checkFile (DiktatBaseMojo.kt:134)
    at org.cqfn.diktat.plugin.maven.DiktatBaseMojo.checkDirectory (DiktatBaseMojo.kt:108)
    at org.cqfn.diktat.plugin.maven.DiktatBaseMojo.execute (DiktatBaseMojo.kt:74)
    at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo (DefaultBuildPluginManager.java:137)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:210)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:156)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:148)
    at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:117)
    at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:81)
    at org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build (SingleThreadedBuilder.java:56)
    at org.apache.maven.lifecycle.internal.LifecycleStarter.execute (LifecycleStarter.java:128)
    at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:305)
    at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:192)
    at org.apache.maven.DefaultMaven.execute (DefaultMaven.java:105)
    at org.apache.maven.cli.MavenCli.execute (MavenCli.java:957)
    at org.apache.maven.cli.MavenCli.doMain (MavenCli.java:289)
    at org.apache.maven.cli.MavenCli.main (MavenCli.java:193)
    at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0 (Native Method)
    at jdk.internal.reflect.NativeMethodAccessorImpl.invoke (NativeMethodAccessorImpl.java:62)
    at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke (DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke (Method.java:564)
    at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced (Launcher.java:282)
    at org.codehaus.plexus.classworlds.launcher.Launcher.launch (Launcher.java:225)
    at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode (Launcher.java:406)
    at org.codehaus.plexus.classworlds.launcher.Launcher.main (Launcher.java:347)
    at org.codehaus.classworlds.Launcher.main (Launcher.java:47)
Caused by: java.lang.IndexOutOfBoundsException: Wrong offset: 2040. Should be in range: [0, 2019]
    at org.jetbrains.kotlin.com.intellij.openapi.editor.impl.LineSet.findLineIndex (LineSet.java:178)
    at org.jetbrains.kotlin.com.intellij.openapi.editor.impl.DocumentImpl.getLineNumber (DocumentImpl.java:991)
    at com.pinterest.ktlint.core.ast.PackageKt.lineNumber (package.kt:235)
    at org.cqfn.diktat.ruleset.utils.AstNodeUtilsKt.isGoingAfter (AstNodeUtils.kt:521)
    at org.cqfn.diktat.ruleset.utils.VariableSearchASTUtilsKt.getAllUsagesOfProperty (VariableSearchASTUtils.kt:63)
    at org.cqfn.diktat.ruleset.utils.VariableSearchASTUtilsKt.getAllUsages (VariableSearchASTUtils.kt:49)
    at org.cqfn.diktat.ruleset.rules.ImmutableValNoVarRule.visit (ImmutableValNoVarRule.kt:39)
    at com.pinterest.ktlint.core.KtLint$format$2.invoke (KtLint.kt:374)
    at com.pinterest.ktlint.core.KtLint$format$2.invoke (KtLint.kt:43)
    at com.pinterest.ktlint.core.KtLint$visitor$2$1.invoke (KtLint.kt:246)
    at com.pinterest.ktlint.core.KtLint$visitor$2$1.invoke (KtLint.kt:43)
    at com.pinterest.ktlint.core.KtLint.visit (KtLint.kt:572)
    at com.pinterest.ktlint.core.KtLint.visit (KtLint.kt:573)
    at com.pinterest.ktlint.core.KtLint.visit (KtLint.kt:573)
    at com.pinterest.ktlint.core.KtLint.visit (KtLint.kt:573)
    at com.pinterest.ktlint.core.KtLint.access$visit (KtLint.kt:43)
    at com.pinterest.ktlint.core.KtLint$visitor$2.invoke (KtLint.kt:244)
    at com.pinterest.ktlint.core.KtLint$visitor$2.invoke (KtLint.kt:43)
    at com.pinterest.ktlint.core.KtLint.format (KtLint.kt:369)
    at org.cqfn.diktat.plugin.maven.DiktatFixMojo.runAction (DiktatMojo.kt:38)
    at org.cqfn.diktat.plugin.maven.DiktatBaseMojo.checkFile (DiktatBaseMojo.kt:134)
    at org.cqfn.diktat.plugin.maven.DiktatBaseMojo.checkDirectory (DiktatBaseMojo.kt:108)
    at org.cqfn.diktat.plugin.maven.DiktatBaseMojo.execute (DiktatBaseMojo.kt:74)
    at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo (DefaultBuildPluginManager.java:137)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:210)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:156)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:148)
    at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:117)
    at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:81)
    at org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build (SingleThreadedBuilder.java:56)
    at org.apache.maven.lifecycle.internal.LifecycleStarter.execute (LifecycleStarter.java:128)
    at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:305)
    at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:192)
    at org.apache.maven.DefaultMaven.execute (DefaultMaven.java:105)
    at org.apache.maven.cli.MavenCli.execute (MavenCli.java:957)
    at org.apache.maven.cli.MavenCli.doMain (MavenCli.java:289)
    at org.apache.maven.cli.MavenCli.main (MavenCli.java:193)
    at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0 (Native Method)
    at jdk.internal.reflect.NativeMethodAccessorImpl.invoke (NativeMethodAccessorImpl.java:62)
    at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke (DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke (Method.java:564)
    at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced (Launcher.java:282)
    at org.codehaus.plexus.classworlds.launcher.Launcher.launch (Launcher.java:225)
    at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode (Launcher.java:406)
    at org.codehaus.plexus.classworlds.launcher.Launcher.main (Launcher.java:347)
    at org.codehaus.classworlds.Launcher.main (Launcher.java:47)
```

Pull request:
### What's done:
* Changed logic
* Added tests

This pull request closes #371

Patch:
diff --git a/diktat-rules/pom.xml b/diktat-rules/pom.xml
index 59c608d024..6503fb69f3 100644
--- a/diktat-rules/pom.xml
+++ b/diktat-rules/pom.xml
@@ -110,7 +110,8 @@
                         </goals>
                         <configuration>
                             <sourceDirs>
-                                <source>src/test/kotlin</source>
+                                <source>${project.basedir}/src/main/kotlin</source>
+                                <source>${project.basedir}/src/test/kotlin</source>
                             </sourceDirs>
                         </configuration>
                     </execution>
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 bbbece9074..6f8204bdf2 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
@@ -88,19 +88,18 @@ class LocalVariablesRule(private val configRules: List<RulesConfig>) : Rule("loc
     private fun handleLocalProperty(property: KtProperty, usages: List<KtNameReferenceExpression>) {
         val declarationScope = property.getDeclarationScope()
 
-        val firstUsageStatementLine = getFirstUsageStatementOrBlock(usages, declarationScope).node.getLineNumber(isFixMode)!!
-        val firstUsage = usages.minBy { it.node.getLineNumber(isFixMode)!! }!!
-        checkLineNumbers(property, firstUsageStatementLine, firstUsageLine = firstUsage.node.getLineNumber(isFixMode)!!)
+        val firstUsageStatementLine = getFirstUsageStatementOrBlock(usages, declarationScope).node.getLineNumber()
+        val firstUsage = usages.minBy { it.node.getLineNumber() }!!
+        checkLineNumbers(property, firstUsageStatementLine, firstUsageLine = firstUsage.node.getLineNumber())
     }
 
-    @Suppress("UnsafeCallOnNullableType")
     private fun handleConsecutiveDeclarations(statement: PsiElement, properties: List<KtProperty>) {
         // need to check that properties are declared consecutively with only maybe empty lines
         properties
-            .sortedBy { it.node.getLineNumber(isFixMode)!! }
+            .sortedBy { it.node.getLineNumber() }
             .zip(properties.size - 1 downTo 0)
             .forEach { (property, offset) ->
-                checkLineNumbers(property, statement.node.getLineNumber(isFixMode)!!, offset)
+                checkLineNumbers(property, statement.node.getLineNumber(), offset)
             }
     }
 
@@ -117,15 +116,15 @@ class LocalVariablesRule(private val configRules: List<RulesConfig>) : Rule("loc
                 siblings
                     .last()
                     .node
-                    .lastLineNumber(isFixMode)!! - siblings
+                    .lastLineNumber() - siblings
                     .first()
                     .node
-                    .getLineNumber(isFixMode)!! - 1
+                    .getLineNumber() - 1
             }
 
-        if (firstUsageStatementLine - numLinesToSkip != property.node.lastLineNumber(isFixMode)!! + 1 + offset) {
+        if (firstUsageStatementLine - numLinesToSkip != property.node.lastLineNumber() + 1 + offset) {
             LOCAL_VARIABLE_EARLY_DECLARATION.warn(configRules, emitWarn, isFixMode,
-                warnMessage(property.name!!, property.node.getLineNumber(isFixMode)!!, firstUsageLine
+                warnMessage(property.name!!, property.node.getLineNumber(), firstUsageLine
                     ?: firstUsageStatementLine), property.startOffset, property.node)
         }
     }
@@ -137,7 +136,7 @@ class LocalVariablesRule(private val configRules: List<RulesConfig>) : Rule("loc
      */
     @Suppress("UnsafeCallOnNullableType", "GENERIC_VARIABLE_WRONG_DECLARATION")
     private fun getFirstUsageStatementOrBlock(usages: List<KtNameReferenceExpression>, declarationScope: KtBlockExpression?): PsiElement {
-        val firstUsage = usages.minBy { it.node.getLineNumber(isFixMode)!! }!!
+        val firstUsage = usages.minBy { it.node.getLineNumber() }!!
         val firstUsageScope = firstUsage.getParentOfType<KtBlockExpression>(true)
 
         return if (firstUsageScope == declarationScope) {
@@ -147,7 +146,7 @@ class LocalVariablesRule(private val configRules: List<RulesConfig>) : Rule("loc
                 .find { it.parent == declarationScope }!!
         } else {
             // first usage is in deeper block compared to declaration, need to check how close is declaration to the first line of the block
-            usages.minBy { it.node.getLineNumber(isFixMode)!! }!!
+            usages.minBy { it.node.getLineNumber() }!!
                 .parentsWithSelf
                 .find { it.parent == declarationScope }!!
         }
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 3f7fe4a1c8..e7713a5a6c 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
@@ -39,6 +39,7 @@ import org.jetbrains.kotlin.psi.KtAnnotationEntry
 import org.jetbrains.kotlin.psi.KtClass
 import org.jetbrains.kotlin.psi.KtIfExpression
 import org.jetbrains.kotlin.psi.psiUtil.children
+import org.jetbrains.kotlin.psi.psiUtil.endOffset
 import org.jetbrains.kotlin.psi.psiUtil.parents
 import org.jetbrains.kotlin.psi.psiUtil.siblings
 import org.slf4j.Logger
@@ -656,7 +657,7 @@ fun ASTNode.extractLineOfText(): String {
  * Checks node has `@Test` annotation
  */
 fun ASTNode.hasTestAnnotation() = findChildByType(MODIFIER_LIST)
-    ?.getAllChildrenWithType(ElementType.ANNOTATION_ENTRY)
+    ?.getAllChildrenWithType(ANNOTATION_ENTRY)
     ?.flatMap { it.findAllNodesWithSpecificType(ElementType.CONSTRUCTOR_CALLEE) }
     ?.any { it.findLeafWithSpecificType(ElementType.IDENTIFIER)?.text == "Test" }
     ?: false
@@ -683,11 +684,9 @@ fun isLocatedInTest(filePathParts: List<String>, testAnchors: List<String>) = fi
 fun ASTNode.firstLineOfText(suffix: String = "") = text.lines().run { singleOrNull() ?: (first() + suffix) }
 
 /**
- * Return the number of the last line in the file
- *
- * @param isFixMode whether autofix mode is on
+ * Return the number in the file of the last line of this node's text
  */
-fun ASTNode.lastLineNumber(isFixMode: Boolean) = getLineNumber(isFixMode)?.plus(text.count { it == '\n' })
+fun ASTNode.lastLineNumber() = getLineNumber() + text.count { it == '\n' }
 
 /**
  * copy-pasted method from ktlint to determine line and column number by offset
@@ -720,38 +719,35 @@ data class ReplacementResult(val oldNodes: List<ASTNode>, val newNodes: List<AST
  * checks that this one node is placed after the other node in code (by comparing lines of code where nodes start)
  */
 fun ASTNode.isGoingAfter(otherNode: ASTNode): Boolean {
-    val thisLineNumber = this.calculateLineNumber()
-    val otherLineNumber = otherNode.calculateLineNumber()
-
-    requireNotNull(thisLineNumber) { "Node ${this.text} should have a line number" }
-    requireNotNull(otherLineNumber) { "Node ${otherNode.text} should have a line number" }
+    val thisLineNumber = this.getLineNumber()
+    val otherLineNumber = otherNode.getLineNumber()
 
     return (thisLineNumber > otherLineNumber)
 }
 
 /**
- * Get line number, where this node's content starts
+ * Get line number, where this node's content starts. To avoid `ArrayIndexOutOfBoundsException`s we check whether node's maximum offset is less than
+ * Document's maximum offset, and calculate line number manually if needed.
  *
- * @param isFixMode if fix mode is off, then the text can't be altered and we can use cached values for line numbers
  * @return line number or null if it cannot be calculated
  */
-fun ASTNode.getLineNumber(isFixMode: Boolean): Int? =
-        if (!isFixMode) {
-            lineNumber()
-        } else {
-            calculateLineNumber()
-        }
+fun ASTNode.getLineNumber(): Int =
+        psi.containingFile
+            .viewProvider
+            .document
+            ?.takeIf { it.getLineEndOffset(it.lineCount - 1) >= psi.endOffset }
+            ?.let { lineNumber() }
+            ?: calculateLineNumber()
 
 /**
  * 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? {
+private fun ASTNode.calculateLineNumber(): Int {
     var count = 0
     // todo use runningFold or something similar when we migrate to apiVersion 1.4
-    return parents()
-        .last()
+    return getRootNode()
         .text
         .lineSequence()
         // calculate offset for every line end, `+1` for `\n` which is trimmed in `lineSequence`
@@ -759,5 +755,8 @@ private fun ASTNode.calculateLineNumber(): Int? {
             count += it.length + 1
             count > startOffset
         }
-        .let { if (it == -1) null else it + 1 }
+        .let {
+            require(it >= 0) { "Cannot calculate line number correctly, node's offset $startOffset is greater than file length ${getRootNode().textLength}" }
+            it + 1
+        }
 }


Test patch:
diff --git a/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Bug1Expected.kt b/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Bug1Expected.kt
index 7fa743a166..21214634b2 100644
--- a/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Bug1Expected.kt
+++ b/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Bug1Expected.kt
@@ -9,5 +9,13 @@ fun readFile(foo: Foo) {
 
 class D {
     val x = 0
+
+    /**
+     * @return
+     */
+    fun bar(): Bar {
+        val qux = 42
+        return Bar(qux)
+    }
 }
 
diff --git a/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Bug1Test.kt b/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Bug1Test.kt
index 41ddbf1625..5bc52612b6 100644
--- a/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Bug1Test.kt
+++ b/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Bug1Test.kt
@@ -4,4 +4,6 @@ fun readFile(foo: Foo) {
     var bar: Bar
 }
 
-class D {val x = 0}
+class D {val x = 0
+fun bar(): Bar {val qux = 42; return Bar(qux)}
+}