Skip to content

Commit

Permalink
Only apply RANGE_TO_UNTIL rule if right-hand side of .. contains …
Browse files Browse the repository at this point in the history
…`X-1`

### What's done:
* Fix logic
* Minor refactorings
* Add fix test case
  • Loading branch information
petertrr committed Sep 14, 2022
1 parent 1756fc3 commit 8ceb012
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

0 comments on commit 8ceb012

Please sign in to comment.