Only apply RANGE_TO_UNTIL rule if right-hand side of .. contains …

…`X-1` (#1524) ### What's done: * Fix logic * Minor refactorings * Add fix test case
saveourtool · Sep 14, 2022 · 914a1a7 · 914a1a7
1 parent 9d7a8d3
commit 914a1a7
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 22 deletions.
diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/RangeConventionalRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/RangeConventionalRule.kt
@@ -13,6 +13,7 @@ import org.cqfn.diktat.ruleset.utils.takeByChainOfTypes
 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.IDENTIFIER
+import com.pinterest.ktlint.core.ast.ElementType.INTEGER_LITERAL
 import com.pinterest.ktlint.core.ast.ElementType.MINUS
 import com.pinterest.ktlint.core.ast.ElementType.RANGE
 import com.pinterest.ktlint.core.ast.ElementType.REFERENCE_EXPRESSION
@@ -25,12 +26,13 @@ import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.LeafPsiElement
 import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.PsiWhiteSpaceImpl
 import org.jetbrains.kotlin.com.intellij.psi.tree.TokenSet
 import org.jetbrains.kotlin.psi.KtBinaryExpression
+import org.jetbrains.kotlin.psi.KtConstantExpression
 import org.jetbrains.kotlin.psi.KtDotQualifiedExpression
 
 /**
- * This rule warn and fix cases when it possible to replace range with until or replace rangeTo function with range
+ * This rule warn and fix cases when it's possible to replace range operator `..` with infix function `until`
+ * or replace `rangeTo` function with range operator `..`
  */
-@Suppress("UnsafeCallOnNullableType")
 class RangeConventionalRule(configRules: List<RulesConfig>) : DiktatRule(
     NAME_ID,
     configRules,
@@ -69,23 +71,31 @@ class RangeConventionalRule(configRules: List<RulesConfig>) : DiktatRule(
 
     @Suppress("TOO_MANY_LINES_IN_LAMBDA")
     private fun handleRange(node: ASTNode) {
-        val binaryInExpression = (node.parent({ it.elementType == BINARY_EXPRESSION })?.psi as KtBinaryExpression?)
-        (binaryInExpression
+        val binaryInExpression = node.parent(BINARY_EXPRESSION)?.psi as KtBinaryExpression?
+        binaryInExpression
             ?.right
             ?.node
+            // Unwrap parentheses and get `BINARY_EXPRESSION` on the RHS of `..`
             ?.takeByChainOfTypes(BINARY_EXPRESSION)
-            ?.psi as KtBinaryExpression?)
-            ?.let {
-                if (it.operationReference.node.hasChildOfType(MINUS)) {
-                    val errorNode = binaryInExpression!!.node
-                    CONVENTIONAL_RANGE.warnAndFix(configRules, emitWarn, isFixMode, "replace `..` with `until`: ${errorNode.text}", errorNode.startOffset, errorNode) {
-                        replaceUntil(node)
-                        // fix right side of binary expression to correct form (remove ` - 1 `) : (b-1) -> (b)
-                        val astNode = it.node
-                        val parent = astNode.treeParent
-                        parent.addChild(astNode.firstChildNode, astNode)
-                        parent.removeChild(astNode)
-                    }
+            ?.run { psi as? KtBinaryExpression }
+            ?.takeIf { it.operationReference.node.hasChildOfType(MINUS) }
+            ?.let { upperBoundExpression ->
+                val isMinusOne = (upperBoundExpression.right as? KtConstantExpression)?.firstChild?.let {
+                    it.node.elementType == INTEGER_LITERAL && it.text == "1"
+                } ?: false
+                if (!isMinusOne) {
+                    return@let
+                }
+                // At this point we are sure that `upperBoundExpression` is `[left] - 1` and should be replaced.
+                val errorNode = binaryInExpression.node
+                CONVENTIONAL_RANGE.warnAndFix(configRules, emitWarn, isFixMode, "replace `..` with `until`: ${errorNode.text}", errorNode.startOffset, errorNode) {
+                    // Replace `..` with `until`
+                    replaceUntil(node)
+                    // fix right side of binary expression to correct form (remove ` - 1 `) : (b-1) -> (b)
+                    val astNode = upperBoundExpression.node
+                    val parent = astNode.treeParent
+                    parent.addChild(astNode.firstChildNode, astNode)
+                    parent.removeChild(astNode)
                 }
             }
     }

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
@@ -816,12 +816,10 @@ fun ASTNode.getLineNumber(): Int =
 fun ASTNode.takeByChainOfTypes(vararg types: IElementType): ASTNode? {
     var node: ASTNode? = this
     types.forEach {
-        node = node?.findChildByType(it) ?: run {
-            while (node?.hasChildOfType(PARENTHESIZED) == true) {
-                node = node?.findChildByType(PARENTHESIZED)
-            }
-            node?.findChildByType(it)
+        while (node?.hasChildOfType(PARENTHESIZED) == true) {
+            node = node?.findChildByType(PARENTHESIZED)
         }
+        node = node?.findChildByType(it)
     }
     return node
 }

diff --git a/diktat-rules/src/test/resources/test/paragraph3/range/RangeToUntilExpected.kt b/diktat-rules/src/test/resources/test/paragraph3/range/RangeToUntilExpected.kt
@@ -11,5 +11,7 @@ class A {
         for (i in 1 until (4)) print(i)
         for (i in 1 until (b)) print(i)
         for (i in ((1 until ((4))))) print(i)
+        for (i in 1..(4 - 2)) print(i)
+        for (i in 1..(b - 10)) print(i)
     }
-}
+}
diff --git a/diktat-rules/src/test/resources/test/paragraph3/range/RangeToUntilTest.kt b/diktat-rules/src/test/resources/test/paragraph3/range/RangeToUntilTest.kt
@@ -11,5 +11,7 @@ class A {
         for (i in 1..(4 - 1)) print(i)
         for (i in 1..(b - 1)) print(i)
         for (i in ((1 .. ((4 - 1))))) print(i)
+        for (i in 1..(4 - 2)) print(i)
+        for (i in 1..(b - 10)) print(i)
     }
 }