Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix. FP of LOCAL_VARIABLE_EARLY_DECLARATION with other variables and a loop #635

Merged
merged 6 commits into from
Dec 14, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)
aktsay6 marked this conversation as resolved.
Show resolved Hide resolved
}
}
}

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()
aktsay6 marked this conversation as resolved.
Show resolved Hide resolved
)
}

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`() {
aktsay6 marked this conversation as resolved.
Show resolved Hide resolved
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()
)
}
}