Skip to content

Commit

Permalink
Range to until (#985)
Browse files Browse the repository at this point in the history
### What's done:
- Implemented "Range to until" inspection
  • Loading branch information
kentr0w authored Jan 20, 2022
1 parent 662ccff commit 2011aff
Show file tree
Hide file tree
Showing 14 changed files with 292 additions and 3 deletions.
7 changes: 6 additions & 1 deletion diktat-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -507,4 +507,9 @@
enabled: true
# Check if boolean expression can be simplified
- name: COMPLEX_BOOLEAN_EXPRESSION
enabled: true
enabled: true
# Check if range can replace with until or `rangeTo` function with range
- name: CONVENTIONAL_RANGE
enabled: true
configuration:
isRangeToIgnore: false
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ enum class Warnings(
STRING_TEMPLATE_QUOTES(true, "3.15.2", "string template has redundant quotes"),
FILE_NAME_MATCH_CLASS(true, "3.1.2", "file name is incorrect - it should match with the class described in it if there is the only one class declared"),
COLLAPSE_IF_STATEMENTS(true, "3.16.1", "avoid using redundant nested if-statements, which could be collapsed into a single one"),
CONVENTIONAL_RANGE(true, "3.17.1", "use conventional rule for range case"),

// ======== chapter 4 ========
NULLABLE_PROPERTY_TYPE(true, "4.3.1", "try to avoid use of nullable types"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import org.cqfn.diktat.ruleset.rules.chapter3.LongNumericalValuesSeparatedRule
import org.cqfn.diktat.ruleset.rules.chapter3.MagicNumberRule
import org.cqfn.diktat.ruleset.rules.chapter3.MultipleModifiersSequence
import org.cqfn.diktat.ruleset.rules.chapter3.NullableTypeRule
import org.cqfn.diktat.ruleset.rules.chapter3.RangeConventionalRule
import org.cqfn.diktat.ruleset.rules.chapter3.SingleLineStatementsRule
import org.cqfn.diktat.ruleset.rules.chapter3.SortRule
import org.cqfn.diktat.ruleset.rules.chapter3.StringConcatenationRule
Expand Down Expand Up @@ -179,6 +180,7 @@ class DiktatRuleSetProvider(private var diktatConfigFile: String = DIKTAT_ANALYS
::AbstractClassesRule,
::TrailingCommaRule,
::SingleInitRule,
::RangeConventionalRule,
::CustomLabel,
::VariableGenericTypeDeclarationRule,
::LongNumericalValuesSeparatedRule,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
package org.cqfn.diktat.ruleset.rules.chapter3

import org.cqfn.diktat.common.config.rules.RuleConfiguration
import org.cqfn.diktat.common.config.rules.RulesConfig
import org.cqfn.diktat.common.config.rules.getRuleConfig
import org.cqfn.diktat.ruleset.constants.Warnings.CONVENTIONAL_RANGE
import org.cqfn.diktat.ruleset.rules.DiktatRule
import org.cqfn.diktat.ruleset.utils.KotlinParser
import org.cqfn.diktat.ruleset.utils.getIdentifierName
import org.cqfn.diktat.ruleset.utils.hasChildOfType
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.MINUS
import com.pinterest.ktlint.core.ast.ElementType.RANGE
import com.pinterest.ktlint.core.ast.ElementType.REFERENCE_EXPRESSION
import com.pinterest.ktlint.core.ast.ElementType.VALUE_ARGUMENT
import com.pinterest.ktlint.core.ast.ElementType.VALUE_ARGUMENT_LIST
import com.pinterest.ktlint.core.ast.isWhiteSpace
import com.pinterest.ktlint.core.ast.parent
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
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.KtDotQualifiedExpression

/**
* This rule warn and fix cases when it possible to replace range with until or replace rangeTo function with range
*/
@Suppress("UnsafeCallOnNullableType")
class RangeConventionalRule(configRules: List<RulesConfig>) : DiktatRule(
"range",
configRules,
listOf(CONVENTIONAL_RANGE)
) {
private val configuration by lazy {
RangeConventionalConfiguration(
this.configRules.getRuleConfig(CONVENTIONAL_RANGE)?.configuration ?: emptyMap(),
)
}
override fun logic(node: ASTNode) {
if (node.elementType == DOT_QUALIFIED_EXPRESSION && !configuration.isRangeToIgnore) {
handleQualifiedExpression(node)
}
if (node.elementType == RANGE) {
handleRange(node)
}
}

@Suppress("TOO_MANY_LINES_IN_LAMBDA")
private fun handleQualifiedExpression(node: ASTNode) {
(node.psi as KtDotQualifiedExpression).selectorExpression?.node?.let {
if (it.findChildByType(REFERENCE_EXPRESSION)?.getIdentifierName()?.text == "rangeTo") {
val arguments = it.findChildByType(VALUE_ARGUMENT_LIST)?.getChildren(TokenSet.create(VALUE_ARGUMENT))
if (arguments?.size == 1) {
CONVENTIONAL_RANGE.warnAndFix(configRules, emitWarn, isFixMode, "replace `rangeTo` with `..`: ${node.text}", node.startOffset, node) {
val receiverExpression = (node.psi as KtDotQualifiedExpression).receiverExpression.text
val correctNode = KotlinParser().createNode("$receiverExpression..${arguments[0].text}")
node.treeParent.addChild(correctNode, node)
node.treeParent.removeChild(node)
}
}
}
}
}

@Suppress("TOO_MANY_LINES_IN_LAMBDA")
private fun handleRange(node: ASTNode) {
val binaryInExpression = (node.parent({ it.elementType == BINARY_EXPRESSION })?.psi as KtBinaryExpression?)
(binaryInExpression
?.right
?.node
?.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)
}
}
}
}

private fun replaceUntil(node: ASTNode) {
val untilNode = LeafPsiElement(IDENTIFIER, "until")
val parent = node.treeParent
if (parent.treePrev?.isWhiteSpace() != true) {
parent.treeParent.addChild(PsiWhiteSpaceImpl(" "), parent)
}
if (parent.treeNext?.isWhiteSpace() != true) {
parent.treeParent.addChild(PsiWhiteSpaceImpl(" "), parent.treeNext)
}
parent.addChild(untilNode, node)
parent.removeChild(node)
}

/**
*
* [RuleConfiguration] for rangeTo function
*/
class RangeConventionalConfiguration(config: Map<String, String>) : RuleConfiguration(config) {
/**
* Does ignore rangeTo function
*/
val isRangeToIgnore = config["isRangeToIgnore"]?.toBoolean() ?: false
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import com.pinterest.ktlint.core.ast.ElementType.LBRACE
import com.pinterest.ktlint.core.ast.ElementType.MODIFIER_LIST
import com.pinterest.ktlint.core.ast.ElementType.OPERATION_REFERENCE
import com.pinterest.ktlint.core.ast.ElementType.OVERRIDE_KEYWORD
import com.pinterest.ktlint.core.ast.ElementType.PARENTHESIZED
import com.pinterest.ktlint.core.ast.ElementType.PRIVATE_KEYWORD
import com.pinterest.ktlint.core.ast.ElementType.PROTECTED_KEYWORD
import com.pinterest.ktlint.core.ast.ElementType.PUBLIC_KEYWORD
Expand Down Expand Up @@ -802,6 +803,24 @@ fun ASTNode.hasEqBinaryExpression(): Boolean =
fun ASTNode.getLineNumber(): Int =
calculateLineNumber()

/**
* Get node by taking children by types and ignore `PARENTHESIZED`
*
* @return child of type
*/
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)
}
}
return node
}

private fun ASTNode.findOffsetByLine(line: Int, positionByOffset: (Int) -> Pair<Int, Int>): Int {
val currentLine = this.getLineNumber()
val currentOffset = this.startOffset
Expand Down
7 changes: 6 additions & 1 deletion diktat-rules/src/main/resources/diktat-analysis-huawei.yml
Original file line number Diff line number Diff line change
Expand Up @@ -507,4 +507,9 @@
enabled: true
# Check if boolean expression can be simplified
- name: COMPLEX_BOOLEAN_EXPRESSION
enabled: true
enabled: true
# Check if range can replace with until or `rangeTo` function with range
- name: CONVENTIONAL_RANGE
enabled: true
configuration:
isRangeToIgnore: false
7 changes: 6 additions & 1 deletion diktat-rules/src/main/resources/diktat-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -503,4 +503,9 @@
enabled: true
# Check if boolean expression can be simplified
- name: COMPLEX_BOOLEAN_EXPRESSION
enabled: true
enabled: true
# Check if range can replace with until or `rangeTo` function with range
- name: CONVENTIONAL_RANGE
enabled: true
configuration:
isRangeToIgnore: false
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package org.cqfn.diktat.ruleset.chapter3

import org.cqfn.diktat.ruleset.rules.chapter3.RangeConventionalRule
import org.cqfn.diktat.util.FixTestBase
import org.junit.jupiter.api.Test

class RangeConventionalRuleFixTest : FixTestBase("test/paragraph3/range", ::RangeConventionalRule) {
@Test
fun `should fix with until`() {
fixAndCompare("RangeToUntilExpected.kt", "RangeToUntilTest.kt")
}

@Test
fun `should fix with rangeTo`() {
fixAndCompare("RangeToExpected.kt", "RangeToTest.kt")
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
package org.cqfn.diktat.ruleset.chapter3

import org.cqfn.diktat.common.config.rules.RulesConfig
import org.cqfn.diktat.ruleset.constants.Warnings
import org.cqfn.diktat.ruleset.rules.DIKTAT_RULE_SET_ID
import org.cqfn.diktat.ruleset.rules.chapter3.RangeConventionalRule
import org.cqfn.diktat.util.LintTestBase

import com.pinterest.ktlint.core.LintError
import org.junit.jupiter.api.Test

class RangeConventionalRuleWarnTest : LintTestBase(::RangeConventionalRule) {
private val ruleId = "$DIKTAT_RULE_SET_ID:range"
private val rulesConfigRangeRule: List<RulesConfig> = listOf(
RulesConfig(
Warnings.CONVENTIONAL_RANGE.name, true,
mapOf("isRangeToIgnore" to "true"))
)

@Test
fun `check simple examples with until`() {
lintMethod(
"""
|fun foo() {
| 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 4 downTo 1) print(i)
| for (i in 1..4) print(i)
| for (i in 1..4 step 2) print(i)
| for (i in 4 downTo 1 step 3) print(i)
| if (6 in (1..10) && true) {}
| for (i in 1..(4 - 1) step 3) print(i)
|}
""".trimMargin(),
LintError(2, 15, ruleId, "${Warnings.CONVENTIONAL_RANGE.warnText()} replace `..` with `until`: 1..(4 - 1)", true),
LintError(3, 15, ruleId, "${Warnings.CONVENTIONAL_RANGE.warnText()} replace `..` with `until`: 1..(b - 1)", true),
LintError(4, 17, ruleId, "${Warnings.CONVENTIONAL_RANGE.warnText()} replace `..` with `until`: 1 .. ((4 - 1))", true),
LintError(10, 15, ruleId, "${Warnings.CONVENTIONAL_RANGE.warnText()} replace `..` with `until`: 1..(4 - 1)", true)
)
}

@Test
fun `check simple examples with rangeTo`() {
lintMethod(
"""
|fun foo() {
| val num = 1
| val w = num.rangeTo(num, num)
| val q = 1..5
| val w = num.rangeTo(num)
|}
""".trimMargin(),
LintError(5, 13, ruleId, "${Warnings.CONVENTIONAL_RANGE.warnText()} replace `rangeTo` with `..`: num.rangeTo(num)", true)
)
}

@Test
fun `check simple examples with rangeTo with config`() {
lintMethod(
"""
|fun foo() {
| val w = num.rangeTo(num, num)
| val w = num.rangeTo(num)
|}
""".trimMargin(),
rulesConfigList = rulesConfigRangeRule
)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package test.paragraph3.range

fun foo() {
val num = 1
val w = num.rangeTo(num, num)
val q = 1..5
val e = num..num
if (1 in num..10) { }
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package test.paragraph3.range

fun foo() {
val num = 1
val w = num.rangeTo(num, num)
val q = 1..5
val e = num.rangeTo(num)
if (1 in num.rangeTo(10)) { }
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package test.paragraph3.range

class A {
fun foo() {
for (i in 1..4) print(i)
for (i in 4 downTo 1) print(i)
for (i in 1 until 4) print(i)
for (i in 1..4 step 2) print(i)
for (i in 4 downTo 1 step 3) print(i)
if (6 in (1..10) && true) {}
for (i in 1 until (4)) print(i)
for (i in 1 until (b)) print(i)
for (i in ((1 until ((4))))) print(i)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package test.paragraph3.range

class A {
fun foo() {
for (i in 1..4) print(i)
for (i in 4 downTo 1) print(i)
for (i in 1 until 4) print(i)
for (i in 1..4 step 2) print(i)
for (i in 4 downTo 1 step 3) print(i)
if (6 in (1..10) && true) {}
for (i in 1..(4 - 1)) print(i)
for (i in 1..(b - 1)) print(i)
for (i in ((1 .. ((4 - 1))))) print(i)
}
}
1 change: 1 addition & 0 deletions info/available-rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@
| 3 | 3.15.2 | STRING_TEMPLATE_QUOTES | Check: warns if there are redundant quotes in string template<br> Fix: deletes quotes and $ symbol | yes | no | + |
| 3 | 3.16.1 | COLLAPSE_IF_STATEMENTS | Check: warns if there are redundant nested if-statements, which could be collapsed into a single one by concatenating their conditions | yes | no | - |
| 3 | 3.16.2 | COMPLEX_BOOLEAN_EXPRESSION | Check: warns if boolean expression is complex and can be simplified.<br>Fix: replaces boolean expression with the simpler one | yes | no | + |
| 3 | 3.17 | CONVENTIONAL_RANGE | Check: warns if possible to replace range with until or replace `rangeTo` function with range.<br>Fix: replace range with until or replace `rangeTo` function with range | yes | no | - |
| 4 | 4.2.1 | SMART_CAST_NEEDED | Check: warns if casting can be omitted<br>Fix: Deletes casting | yes | no | - | |
| 4 | 4.3.1 | NULLABLE_PROPERTY_TYPE | Check: warns if immutable property is initialized with null or if immutable property can have non-nullable type instead of nullable<br>Fix: suggests initial value instead of null or changes immutable property type | yes | no | - |
| 4 | 4.2.2 | TYPE_ALIAS | Check: if type reference of property is longer than expected | yes | typeReferenceLength | -|
Expand Down

0 comments on commit 2011aff

Please sign in to comment.