Repository: analysis-dev/diktat (kotlin)

Issue:
Conventional Range rule appears to be broken
## Describe the bug
We're being required to replace a range created with `..` with a non-equal range.

## Expected behavior
Range replacements should be equal.

## Observed behavior
They are not.

## Steps to Reproduce
Write code like `0..(string.length - 2))` and see that it's corrected to `0 until (string.length))`. Here's a [demonstration of the difference](https://play.kotlinlang.org/#eyJ2ZXJzaW9uIjoiMS42LjIxIiwicGxhdGZvcm0iOiJqYXZhIiwiYXJncyI6IiIsIm5vbmVNYXJrZXJzIjp0cnVlLCJ0aGVtZSI6ImlkZWEiLCJjb2RlIjoiZnVuIG1haW4oYXJnczogQXJyYXk8U3RyaW5nPikge1xuICAgIHZhbCBzdHJpbmcgPSBcImFiYzEyM1wiXG4gICAgcHJpbnRsbihzdHJpbmcuc3Vic3RyaW5nKDAuLihzdHJpbmcubGVuZ3RoIC0gMikpKSAgLy8gcHJpbnRzIGFiYzEyXG4gICAgcHJpbnRsbihzdHJpbmcuc3Vic3RyaW5nKDAgdW50aWwgKHN0cmluZy5sZW5ndGgpKSkgLy8gcHJpbnRzIGFiYzEyM1xufSJ9): 

```kotlin
fun main(args: Array<String>) {
    val string = "abc123"
    println(string.substring(0..(string.length - 2)))  // prints abc12
    println(string.substring(0 until (string.length))) // prints abc123
}
```

## Environment information
* diktat version: 1.0.3 (and cannot upgrade easily due to conflicts between Spotless/Ktlint/DiKTat)
* build tool (maven/gradle): gradle
* how is diktat run (CLI, plugin, etc.): spotless
* kotlin version: 1.6.21
* operating system: Mac
* link to a project (if your project is public):

Pull request:
### What's done:
* Fix logic
* Minor refactorings
* Add fix test case

This pull request closes #1502

Patch:
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
index f198da071f..9e5d5f5170 100644
--- 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
index 71c996f9bf..bf2ea5a8b1 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
@@ -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
 }


Test patch:
diff --git a/diktat-rules/src/test/resources/test/paragraph3/range/RangeToUntilExpected.kt b/diktat-rules/src/test/resources/test/paragraph3/range/RangeToUntilExpected.kt
index 4aac33e657..2eed359a01 100644
--- 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)
     }
-}
\ No newline at end of file
+}
diff --git a/diktat-rules/src/test/resources/test/paragraph3/range/RangeToUntilTest.kt b/diktat-rules/src/test/resources/test/paragraph3/range/RangeToUntilTest.kt
index 34681ea975..166cadd1ae 100644
--- 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)
     }
 }