Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 14 commits into from
Mar 11, 2022
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions diktat-rules/src/main/kotlin/generated/WarningNames.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -166,6 +167,7 @@ class DiktatRuleSetProvider(private var diktatConfigFile: String = DIKTAT_ANALYS
::CustomGetterSetterRule,
::CompactInitialization,
// other rules
::UseLastIndex,
::InlineClassesRule,
::ExtensionFunctionsInFileRule,
::CheckInverseMethodRule,
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
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")
}

}
Original file line number Diff line number Diff line change
@@ -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)
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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")) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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

}
Original file line number Diff line number Diff line change
@@ -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

}
Original file line number Diff line number Diff line change
@@ -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 ]
}
Original file line number Diff line number Diff line change
@@ -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 ]
}
Loading