Skip to content

Commit

Permalink
Bugfix. FP of LOCAL_VARIABLE_EARLY_DECLARATION with other variables a…
Browse files Browse the repository at this point in the history
…nd a loop (#635)

* bugfix/local-variable-loops(#581)

### What's done:
 * Fixed bugs
 * Added tests
  • Loading branch information
aktsay6 authored Dec 14, 2020
1 parent 4edbd78 commit c1c8d2d
Show file tree
Hide file tree
Showing 7 changed files with 58 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,7 @@ class ExtensionFunctionsSameNameRule(private val configRules: List<RulesConfig>)
return pairs
}

/**
* FixMe: warning suppressed until https://github.com/cqfn/diKTat/issues/581
*/
@Suppress("UnsafeCallOnNullableType", "LOCAL_VARIABLE_EARLY_DECLARATION", "TYPE_ALIAS")
@Suppress("UnsafeCallOnNullableType", "TYPE_ALIAS")
private fun collectAllExtensionFunctions(node: ASTNode): SimilarSignatures {
val extensionFunctionList = node.findAllNodesWithSpecificType(FUN).filter { it.hasChildOfType(TYPE_REFERENCE) && it.hasChildOfType(DOT) }
val distinctFunctionSignatures: MutableMap<FunctionSignature, ASTNode> = mutableMapOf() // maps function signatures on node it is used by
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ class LineLength(private val configRules: List<RulesConfig>) : Rule("line-length
}
}

@Suppress("UnsafeCallOnNullableType", "LOCAL_VARIABLE_EARLY_DECLARATION")
@Suppress("UnsafeCallOnNullableType")
private fun checkLength(node: ASTNode, configuration: LineLengthConfiguration) {
var offset = 0
node.text.lines().forEach {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class SmartCastRule(private val configRules: List<RulesConfig>) : Rule("smart-ca
}

// Divide in is and as expr
@Suppress("TYPE_ALIAS", "LOCAL_VARIABLE_EARLY_DECLARATION")
@Suppress("TYPE_ALIAS")
private fun handleProp(propMap: Map<KtProperty, List<KtNameReferenceExpression>>) {
propMap.forEach { (property, references) ->
val isExpr: MutableList<KtNameReferenceExpression> = mutableListOf()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,10 @@ class SortRule(private val configRules: List<RulesConfig>) : Rule("sort-rule") {
}
}

@Suppress("LOCAL_VARIABLE_EARLY_DECLARATION", "TYPE_ALIAS")
@Suppress("TYPE_ALIAS")
private fun createOrderListOfList(propertyList: List<ASTNode>): List<List<ASTNode>> {
val orderListOfList: MutableList<MutableList<ASTNode>> = mutableListOf()
var oneOrderList = mutableListOf(propertyList.first())
val orderListOfList: MutableList<MutableList<ASTNode>> = mutableListOf()
propertyList.zipWithNext().forEach { (currentProperty, nextProperty) ->
if (currentProperty.nextSibling { it.elementType == PROPERTY } == nextProperty) {
oneOrderList.add(nextProperty)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@ import org.cqfn.diktat.ruleset.utils.containsOnlyConstants
import org.cqfn.diktat.ruleset.utils.getDeclarationScope
import org.cqfn.diktat.ruleset.utils.getLineNumber
import org.cqfn.diktat.ruleset.utils.lastLineNumber
import org.cqfn.diktat.ruleset.utils.numNewLines
import org.cqfn.diktat.ruleset.utils.search.findAllVariablesWithUsages

import com.pinterest.ktlint.core.Rule
import com.pinterest.ktlint.core.ast.ElementType.FILE
import com.pinterest.ktlint.core.ast.ElementType.WHITE_SPACE
import com.pinterest.ktlint.core.ast.isPartOfComment
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import org.jetbrains.kotlin.com.intellij.psi.PsiElement
Expand Down Expand Up @@ -94,12 +96,31 @@ class LocalVariablesRule(private val configRules: List<RulesConfig>) : Rule("loc
}

private fun handleConsecutiveDeclarations(statement: PsiElement, properties: List<KtProperty>) {
val numLinesAfterLastProp =
properties
.last()
.node
.treeNext
.takeIf { it.elementType == WHITE_SPACE }
?.let {
// minus one is needed to except \n after property
it.numNewLines() - 1
}
?: 0

// need to check that properties are declared consecutively with only maybe empty lines
properties
.sortedBy { it.node.getLineNumber() }
.zip(properties.size - 1 downTo 0)
.forEach { (property, offset) ->
checkLineNumbers(property, statement.node.getLineNumber(), offset)
.zip(
(properties.size - 1 downTo 0).map { it + numLinesAfterLastProp }
)
.forEachIndexed { index, (property, offset) ->
if (index != properties.lastIndex) {
checkLineNumbers(property, statement.node.getLineNumber(), offset)
} else {
// since offset after last property is calculated in this method, we pass offset = 0
checkLineNumbers(property, statement.node.getLineNumber(), 0)
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -636,7 +636,7 @@ fun List<ASTNode>.handleIncorrectOrder(
* This method returns text of this [ASTNode] plus text from it's siblings after last and until next newline, if present in siblings.
* I.e., if this node occupies no more than a single line, this whole line or it's part will be returned.
*/
@Suppress("LOCAL_VARIABLE_EARLY_DECLARATION", "WRONG_NEWLINES")
@Suppress("WRONG_NEWLINES")
fun ASTNode.extractLineOfText(): String {
var text: MutableList<String> = mutableListOf()
siblings(false)
Expand Down Expand Up @@ -743,7 +743,6 @@ fun ASTNode.getLineNumber(): Int =
* This function calculates line number instead of using cached values.
* It should be used when AST could be previously mutated by auto fixers.
*/
@Suppress("LOCAL_VARIABLE_EARLY_DECLARATION")
private fun ASTNode.calculateLineNumber(): Int {
var count = 0
// todo use runningFold or something similar when we migrate to apiVersion 1.4
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -471,7 +471,7 @@ class LocalVariablesWarnTest : LintTestBase(::LocalVariablesRule) {

@Test
@Tag(WarningNames.LOCAL_VARIABLE_EARLY_DECLARATION)
fun `should emit only one warning when same variables are used more than once`() {
fun `should not trigger on properties`() {
lintMethod(
"""
|private fun checkDoc(node: ASTNode, warning: Warnings) {
Expand All @@ -483,9 +483,7 @@ class LocalVariablesWarnTest : LintTestBase(::LocalVariablesRule) {
| foo(a, b, c)
| }
|}
""".trimMargin(),
LintError(2, 5, ruleId, "${LOCAL_VARIABLE_EARLY_DECLARATION.warnText()} ${warnMessage("a", 2, 6)}", false),
LintError(3, 5, ruleId, "${LOCAL_VARIABLE_EARLY_DECLARATION.warnText()} ${warnMessage("b", 3, 6)}", false)
""".trimMargin()
)
}

Expand Down Expand Up @@ -573,4 +571,30 @@ class LocalVariablesWarnTest : LintTestBase(::LocalVariablesRule) {
""".trimMargin()
)
}

@Test
@Tag(WarningNames.LOCAL_VARIABLE_EARLY_DECLARATION)
fun `should not trigger on space after last val`() {
lintMethod(
"""
| private fun collectAllExtensionFunctions(node: ASTNode): SimilarSignatures {
| val extensionFunctionList = node.findAllNodesWithSpecificType(FUN).filter { it.hasChildOfType(TYPE_REFERENCE) && it.hasChildOfType(DOT) }
| val distinctFunctionSignatures = mutableMapOf<FunctionSignature, ASTNode>() // maps function signatures on node it is used by
| val extensionFunctionsPairs = mutableListOf<Pair<ExtensionFunction, ExtensionFunction>>() // pairs extension functions with same signature
|
| extensionFunctionList.forEach { func ->
| if (distinctFunctionSignatures.contains(signature)) {
| val secondFuncClassName = distinctFunctionSignatures[signature]!!.findChildBefore(DOT, TYPE_REFERENCE)!!.text
| extensionFunctionsPairs.add(Pair(
| ExtensionFunction(secondFuncClassName, signature, distinctFunctionSignatures[signature]!!),
| ExtensionFunction(className, signature, func)))
| } else {
| distinctFunctionSignatures[signature] = func
| }
| }
| return extensionFunctionsPairs
| }
""".trimMargin()
)
}
}

0 comments on commit c1c8d2d

Please sign in to comment.