Suggest using lastIndex instead of length - 1 for strings (#1221)

* Change property length with operation - 1 to property lastIndex

### What's done:
- Added new rule to replace ".length - 1" to ".lastIndex"
- Added warn tests
- Added fix tests
- Updated Readme (rule 6.2.4)

(#1140)

Co-authored-by: Alexey Votintsev <alexeyvotintsev.yandex.ru>

diktat-rules/src/main/kotlin/generated/WarningNames.kt

@@ -266,4 +266,6 @@ public object WarningNames {
     public const val EXTENSION_FUNCTION_WITH_CLASS: String = "EXTENSION_FUNCTION_WITH_CLASS"
 
     public const val RUN_IN_SCRIPT: String = "RUN_IN_SCRIPT"
+
+    public const val USE_LAST_INDEX: String = "USE_LAST_INDEX"
 }

diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt

@@ -178,6 +178,7 @@ enum class Warnings(
     INLINE_CLASS_CAN_BE_USED(true, "6.1.12", "inline class can be used"),
     EXTENSION_FUNCTION_WITH_CLASS(false, "6.2.3", "do not use extension functions for the class defined in the same file"),
     RUN_IN_SCRIPT(true, "6.5.1", "wrap blocks of code in top-level scope functions like `run`"),
+    USE_LAST_INDEX(true, "6.2.4", "Instead of \"length - 1\" need to use built-in \"lastIndex\" operation"),
     ;
 
     /**

diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt

@@ -68,6 +68,7 @@ import org.cqfn.diktat.ruleset.rules.chapter6.ImplicitBackingPropertyRule
 import org.cqfn.diktat.ruleset.rules.chapter6.PropertyAccessorFields
 import org.cqfn.diktat.ruleset.rules.chapter6.RunInScript
 import org.cqfn.diktat.ruleset.rules.chapter6.TrivialPropertyAccessors
+import org.cqfn.diktat.ruleset.rules.chapter6.UseLastIndex
 import org.cqfn.diktat.ruleset.rules.chapter6.UselessSupertype
 import org.cqfn.diktat.ruleset.rules.chapter6.classes.AbstractClassesRule
 import org.cqfn.diktat.ruleset.rules.chapter6.classes.CompactInitialization
@@ -166,6 +167,7 @@ class DiktatRuleSetProvider(private var diktatConfigFile: String = DIKTAT_ANALYS
             ::CustomGetterSetterRule,
             ::CompactInitialization,
             // other rules
+            ::UseLastIndex,
             ::InlineClassesRule,
             ::ExtensionFunctionsInFileRule,
             ::CheckInverseMethodRule,

diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter6/UseLastIndex.kt

@@ -0,0 +1,52 @@
+package org.cqfn.diktat.ruleset.rules.chapter6
+
+import org.cqfn.diktat.common.config.rules.RulesConfig
+import org.cqfn.diktat.ruleset.constants.Warnings
+import org.cqfn.diktat.ruleset.rules.DiktatRule
+import org.cqfn.diktat.ruleset.utils.*
+
+import com.pinterest.ktlint.core.ast.ElementType.BINARY_EXPRESSION
+import com.pinterest.ktlint.core.ast.ElementType.DOT_QUALIFIED_EXPRESSION
+import com.pinterest.ktlint.core.ast.ElementType.INTEGER_CONSTANT
+import com.pinterest.ktlint.core.ast.ElementType.OPERATION_REFERENCE
+import com.pinterest.ktlint.core.ast.ElementType.REFERENCE_EXPRESSION
+import com.pinterest.ktlint.core.ast.children
+import org.jetbrains.kotlin.com.intellij.lang.ASTNode
+
+/**
+ * This rule checks if there use property length with operation - 1 and fix this on lastIndex
+ */
+class UseLastIndex(configRules: List<RulesConfig>) : DiktatRule(
+    "last-index",
+    configRules,
+    listOf(Warnings.USE_LAST_INDEX)
+) {
+    override fun logic(node: ASTNode) {
+        if (node.elementType == BINARY_EXPRESSION) {
+            changeRight(node)
+        }
+    }
+
+    private fun changeRight(node: ASTNode) {
+        val listWithRightLength = node.children().filter {
+            val operation = node.getFirstChildWithType(OPERATION_REFERENCE)
+            val number = node.getFirstChildWithType(INTEGER_CONSTANT)
+            it.elementType == DOT_QUALIFIED_EXPRESSION && it.lastChildNode.text == "length" && it.lastChildNode.elementType == REFERENCE_EXPRESSION &&
+                    operation?.text == "-" && number?.text == "1"
+        }
+        if (listWithRightLength.toList().isNotEmpty()) {
+            Warnings.USE_LAST_INDEX.warnAndFix(configRules, emitWarn, isFixMode, node.text, node.startOffset, node) {
+                fix(node)
+            }
+        }
+    }
+
+    private fun fix(node: ASTNode) {
+        // A.B.length - 1 -> A.B
+        val text = node.firstChildNode.text.replace("length", "lastIndex")
+        val parent = node.treeParent
+        val textParent = parent.text.replace(node.text, text)
+        val newParent = KotlinParser().createNode(textParent)
+        parent.treeParent.replaceChild(parent, newParent)
+    }
+}

diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/UseLastIndexFixTest.kt

@@ -0,0 +1,18 @@
+package org.cqfn.diktat.ruleset.chapter6
+
+import org.cqfn.diktat.ruleset.rules.chapter6.UseLastIndex
+import org.cqfn.diktat.util.FixTestBase
+
+import org.junit.jupiter.api.Test
+
+class UseLastIndexFixTest : FixTestBase("test/chapter6/lastIndex_change", ::UseLastIndex) {
+    @Test
+    fun `fix example with white spaces`() {
+        fixAndCompare("UseAnyWhiteSpacesExpected.kt", "UseAnyWhiteSpacesTest.kt")
+    }
+
+    @Test
+    fun `fix example with incorrect use length`() {
+        fixAndCompare("IncorrectUseLengthMinusOneExpected.kt", "IncorrectUseLengthMinusOneTest.kt")
+    }
+}

diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/UseLastIndexWarnTest.kt

@@ -0,0 +1,101 @@
+package org.cqfn.diktat.ruleset.chapter6
+
+import org.cqfn.diktat.ruleset.constants.Warnings
+import org.cqfn.diktat.ruleset.rules.DIKTAT_RULE_SET_ID
+import org.cqfn.diktat.ruleset.rules.chapter6.UseLastIndex
+import org.cqfn.diktat.util.LintTestBase
+
+import com.pinterest.ktlint.core.LintError
+import generated.WarningNames
+import org.junit.jupiter.api.Tag
+import org.junit.jupiter.api.Test
+
+class UseLastIndexWarnTest : LintTestBase(::UseLastIndex) {
+    private val ruleId = "$DIKTAT_RULE_SET_ID:last-index"
+
+    @Test
+    @Tag(WarningNames.USE_LAST_INDEX)
+    fun `find method Length - 1 with many dot expressions`() {
+        lintMethod(
+            """
+                    |val A = "AAAAAAAA"
+                    |val D = A.B.C.length - 1
+                    |
+                """.trimMargin(),
+            LintError(2, 9, ruleId, "${Warnings.USE_LAST_INDEX.warnText()} A.B.C.length - 1", true)
+        )
+    }
+
+    @Test
+    @Tag(WarningNames.USE_LAST_INDEX)
+    fun `find method Length - 1 for mane line`() {
+        lintMethod(
+            """
+                    |fun foo() {
+                    |   val A : String = "AAAA"
+                    |   var B = A.length
+                    |   -
+                    |   1
+                    |}
+                """.trimMargin()
+        )
+    }
+
+    @Test
+    @Tag(WarningNames.USE_LAST_INDEX)
+    fun `find method Length - 1 with many spaces and tabulation`() {
+        lintMethod(
+            """
+                    |val A : String = "AAAA"
+                    |var B =    A.length   -       1  + 214
+                    |var C = A.length - 19
+                    |
+                """.trimMargin(),
+            LintError(2, 12, ruleId, "${Warnings.USE_LAST_INDEX.warnText() } A.length   -       1", true)
+        )
+    }
+
+    @Test
+    @Tag(WarningNames.USE_LAST_INDEX)
+    fun `find method Length - 1 without spaces`() {
+        lintMethod(
+            """
+                    |val A : String = "AAAA"
+                    |var B = A.length-1
+                    |
+                """.trimMargin(),
+            LintError(2, 9, ruleId, "${Warnings.USE_LAST_INDEX.warnText()} A.length-1", true)
+        )
+    }
+
+    @Test
+    @Tag(WarningNames.USE_LAST_INDEX)
+    fun `find method Length - 1 without length`() {
+        lintMethod(
+            """
+                    |val A = "AAAA"
+                    |val B = -1
+                    |val C = 6 + 121
+                    |var D = B + C
+                    |var E = A.length + 1
+                """.trimMargin()
+        )
+    }
+
+    @Test
+    @Tag(WarningNames.USE_LAST_INDEX)
+    fun `find method Length - 1 without -1`() {
+        lintMethod(
+            """
+                    |val A = "AAAA"
+                    |val B = -1
+                    |val C = 6 + 4
+                    |val D = "AAAA".length - 1
+                    |
+                    |val M = "ASDFG".length
+                    |
+                """.trimMargin(),
+            LintError(4, 9, ruleId, "${Warnings.USE_LAST_INDEX.warnText()} \"AAAA\".length - 1", true)
+        )
+    }
+}

diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/utils/RulesConfigYamlTest.kt

@@ -19,8 +19,9 @@ import kotlinx.serialization.encodeToString
  * Special test that checks that developer has not forgotten to add his warning to a diktat-analysis.yml
  * This file is needed to be in tact with latest changes in Warnings.kt
  */
+@JvmInline
 @Suppress("UNUSED")
-inline class RulesConfigYamlTest(private val pathMap: Map<String, String> =
+value class RulesConfigYamlTest(private val pathMap: Map<String, String> =
         mapOf("diktat-analysis.yml" to "diKTat/diktat-rules/src/main/resources/diktat-analysis.yml",
             "diktat-analysis-huawei.yml" to "diKTat/diktat-rules/src/main/resources/diktat-analysis-huawei.yml",
             "parent/diktat-analysis.yml" to "diKTat/diktat-analysis.yml")) {

diktat-rules/src/test/resources/test/chapter6/lastIndex_change/IncorrectUseLengthMinusOneExpected.kt

@@ -0,0 +1,9 @@
+package test.chapter6.lastIndex_change
+
+fun main(args: Array<String>) {
+
+    val str = "ASDFG"
+    val A = str.lastIndex
+    val B = str.lastIndex
+
+}

diktat-rules/src/test/resources/test/chapter6/lastIndex_change/IncorrectUseLengthMinusOneTest.kt

@@ -0,0 +1,9 @@
+package test.chapter6.lastIndex_change
+
+fun main(args: Array<String>) {
+
+    val str = "ASDFG"
+    val A = str.length - 1
+    val B = str.length - 1
+
+}

diktat-rules/src/test/resources/test/chapter6/lastIndex_change/UseAnyWhiteSpacesExpected.kt

@@ -0,0 +1,12 @@
+package test.chapter6.lastIndex_change
+
+fun main(args: Array<String>) {
+    val str = "ASDFG"
+    val A = str.lastIndex
+    val B = str.lastIndex
+    val C = str.lastIndex
+    val D = str.lastIndex
+    val F = str.lastIndex
+    val E = str[str.lastIndex]
+    val G = str[str.lastIndex ]
+}

diktat-rules/src/test/resources/test/chapter6/lastIndex_change/UseAnyWhiteSpacesTest.kt

@@ -0,0 +1,12 @@
+package test.chapter6.lastIndex_change
+
+fun main(args: Array<String>) {
+    val str = "ASDFG"
+    val A = str.length    -         1
+    val B = str.length-1
+    val C = str.length    -1
+    val D = str.length-     1
+    val F = str.length - 1
+    val E = str[str.length - 1]
+    val G = str[str.length  -  1 ]
+}