From 5e7e6c90149109f2bdde30c3a7c9f4820771c419 Mon Sep 17 00:00:00 2001 From: kentr0w Date: Wed, 16 Dec 2020 14:58:09 +0300 Subject: [PATCH 1/7] Fix for incorrect variable name handle ### What's done: Added fix for incorrect variable handle and tests --- .../cqfn/diktat/ruleset/rules/IdentifierNaming.kt | 9 +++++++++ .../naming/identifiers/VariableNamingExpected.kt | 15 +++++++++++++++ .../naming/identifiers/VariableNamingTest.kt | 15 +++++++++++++++ 3 files changed, 39 insertions(+) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/IdentifierNaming.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/IdentifierNaming.kt index efaa07aa12..af9d89ca89 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/IdentifierNaming.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/IdentifierNaming.kt @@ -47,16 +47,20 @@ import com.pinterest.ktlint.core.ast.ElementType.CATCH import com.pinterest.ktlint.core.ast.ElementType.CATCH_KEYWORD import com.pinterest.ktlint.core.ast.ElementType.DESTRUCTURING_DECLARATION import com.pinterest.ktlint.core.ast.ElementType.DESTRUCTURING_DECLARATION_ENTRY +import com.pinterest.ktlint.core.ast.ElementType.FILE import com.pinterest.ktlint.core.ast.ElementType.FUNCTION_TYPE import com.pinterest.ktlint.core.ast.ElementType.REFERENCE_EXPRESSION import com.pinterest.ktlint.core.ast.ElementType.TYPE_PARAMETER import com.pinterest.ktlint.core.ast.ElementType.TYPE_REFERENCE import com.pinterest.ktlint.core.ast.ElementType.VALUE_PARAMETER_LIST +import com.pinterest.ktlint.core.ast.parent import com.pinterest.ktlint.core.ast.prevCodeSibling +import org.cqfn.diktat.ruleset.utils.search.findAllVariablesWithUsages import org.jetbrains.kotlin.builtins.PrimitiveType 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.tree.TokenSet +import org.jetbrains.kotlin.psi.psiUtil.isPrivate import org.jetbrains.kotlin.psi.psiUtil.parents /** @@ -162,6 +166,11 @@ class IdentifierNaming(private val configRules: List) : Rule("ident // variable name should be in camel case. The only exception is a list of industry standard variables like i, j, k. VARIABLE_NAME_INCORRECT_FORMAT.warnAndFix(configRules, emitWarn, isFixMode, variableName.text, variableName.startOffset, node) { // FixMe: cover fixes with tests + variableName + .parent({it.elementType == FILE}) + ?.findAllVariablesWithUsages { it.name == variableName.text && (it.isLocal || it.isPrivate()) } + ?.flatMap { it.value.toList() } + ?.forEach { (it.node.firstChildNode as LeafPsiElement).replaceWithText(variableName.text.toLowerCamelCase())} (variableName as LeafPsiElement).replaceWithText(variableName.text.toLowerCamelCase()) } } diff --git a/diktat-rules/src/test/resources/test/paragraph1/naming/identifiers/VariableNamingExpected.kt b/diktat-rules/src/test/resources/test/paragraph1/naming/identifiers/VariableNamingExpected.kt index 336cb28db3..e1fc3b40f7 100644 --- a/diktat-rules/src/test/resources/test/paragraph1/naming/identifiers/VariableNamingExpected.kt +++ b/diktat-rules/src/test/resources/test/paragraph1/naming/identifiers/VariableNamingExpected.kt @@ -10,3 +10,18 @@ private val loWerValue = "" private val lower = "" private val valNX256 = "" private val voiceIpPort = "" + +class A { + private val voiceIpPort = "" + public val valNX2567 = "" + + fun foo() { + val voiceIpPorts = "" + val upperSnakers = voiceIpPort + } + + fun goo() { + val qwe = lowerSnake + val pre = valNX256 + } +} \ No newline at end of file diff --git a/diktat-rules/src/test/resources/test/paragraph1/naming/identifiers/VariableNamingTest.kt b/diktat-rules/src/test/resources/test/paragraph1/naming/identifiers/VariableNamingTest.kt index 8d04545cf5..16d72c2307 100644 --- a/diktat-rules/src/test/resources/test/paragraph1/naming/identifiers/VariableNamingTest.kt +++ b/diktat-rules/src/test/resources/test/paragraph1/naming/identifiers/VariableNamingTest.kt @@ -10,3 +10,18 @@ private val loWer_VAlue = "" private val lower = "" private val ValN_x256 = "" private val VoiceIP_port = "" + +class A { + private val VoiceIP_port = "" + public val ValN_x2567 = "" + + fun foo() { + val VoiceIP_ports = "" + val UPPER_SNAKERS = VoiceIP_port + } + + fun goo() { + val qwe = lower_snake + val pre = ValN_x256 + } +} From 57668bdd5a77d3f59135ef8399aad230e4892a22 Mon Sep 17 00:00:00 2001 From: kentr0w Date: Wed, 16 Dec 2020 15:11:46 +0300 Subject: [PATCH 2/7] Fix for incorrect variable name handle ### What's done: Fixed according our code-style --- .../org/cqfn/diktat/ruleset/rules/IdentifierNaming.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/IdentifierNaming.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/IdentifierNaming.kt index af9d89ca89..ff968e00ff 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/IdentifierNaming.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/IdentifierNaming.kt @@ -37,6 +37,7 @@ import org.cqfn.diktat.ruleset.utils.isPascalCase import org.cqfn.diktat.ruleset.utils.isTextLengthInRange import org.cqfn.diktat.ruleset.utils.isUpperSnakeCase import org.cqfn.diktat.ruleset.utils.removePrefix +import org.cqfn.diktat.ruleset.utils.search.findAllVariablesWithUsages import org.cqfn.diktat.ruleset.utils.toLowerCamelCase import org.cqfn.diktat.ruleset.utils.toPascalCase import org.cqfn.diktat.ruleset.utils.toUpperSnakeCase @@ -55,7 +56,6 @@ import com.pinterest.ktlint.core.ast.ElementType.TYPE_REFERENCE import com.pinterest.ktlint.core.ast.ElementType.VALUE_PARAMETER_LIST import com.pinterest.ktlint.core.ast.parent import com.pinterest.ktlint.core.ast.prevCodeSibling -import org.cqfn.diktat.ruleset.utils.search.findAllVariablesWithUsages import org.jetbrains.kotlin.builtins.PrimitiveType import org.jetbrains.kotlin.com.intellij.lang.ASTNode import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.LeafPsiElement @@ -138,7 +138,7 @@ class IdentifierNaming(private val configRules: List) : Rule("ident /** * all checks for case and naming for vals/vars/constants */ - @Suppress("SAY_NO_TO_VAR") + @Suppress("SAY_NO_TO_VAR", "TOO_LONG_FUNCTION", "ComplexMethod") private fun checkVariableName(node: ASTNode): List { // special case for Destructuring declarations that can be treated as parameters in lambda: var namesOfVariables = extractVariableIdentifiers(node) @@ -170,7 +170,7 @@ class IdentifierNaming(private val configRules: List) : Rule("ident .parent({it.elementType == FILE}) ?.findAllVariablesWithUsages { it.name == variableName.text && (it.isLocal || it.isPrivate()) } ?.flatMap { it.value.toList() } - ?.forEach { (it.node.firstChildNode as LeafPsiElement).replaceWithText(variableName.text.toLowerCamelCase())} + ?.forEach { (it.node.firstChildNode as LeafPsiElement).replaceWithText(variableName.text.toLowerCamelCase())} (variableName as LeafPsiElement).replaceWithText(variableName.text.toLowerCamelCase()) } } From 2de3caa5d5c0d0ab1954b47965480fc6f791bf71 Mon Sep 17 00:00:00 2001 From: kentr0w Date: Thu, 17 Dec 2020 17:33:16 +0300 Subject: [PATCH 3/7] Fix for incorrect variable name handle ### What's done: Fixed after review --- .../diktat/ruleset/rules/IdentifierNaming.kt | 7 ++++-- .../identifiers/VariableNamingExpected.kt | 22 ++++++++++++++++++- .../naming/identifiers/VariableNamingTest.kt | 22 ++++++++++++++++++- 3 files changed, 47 insertions(+), 4 deletions(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/IdentifierNaming.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/IdentifierNaming.kt index ff968e00ff..1087ef2774 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/IdentifierNaming.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/IdentifierNaming.kt @@ -50,6 +50,7 @@ import com.pinterest.ktlint.core.ast.ElementType.DESTRUCTURING_DECLARATION import com.pinterest.ktlint.core.ast.ElementType.DESTRUCTURING_DECLARATION_ENTRY import com.pinterest.ktlint.core.ast.ElementType.FILE import com.pinterest.ktlint.core.ast.ElementType.FUNCTION_TYPE +import com.pinterest.ktlint.core.ast.ElementType.PROPERTY import com.pinterest.ktlint.core.ast.ElementType.REFERENCE_EXPRESSION import com.pinterest.ktlint.core.ast.ElementType.TYPE_PARAMETER import com.pinterest.ktlint.core.ast.ElementType.TYPE_REFERENCE @@ -60,6 +61,7 @@ import org.jetbrains.kotlin.builtins.PrimitiveType 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.tree.TokenSet +import org.jetbrains.kotlin.psi.KtProperty import org.jetbrains.kotlin.psi.psiUtil.isPrivate import org.jetbrains.kotlin.psi.psiUtil.parents @@ -142,6 +144,7 @@ class IdentifierNaming(private val configRules: List) : Rule("ident private fun checkVariableName(node: ASTNode): List { // special case for Destructuring declarations that can be treated as parameters in lambda: var namesOfVariables = extractVariableIdentifiers(node) + val isFix = isFixMode && if (node.elementType == PROPERTY) (node.psi as KtProperty).run { isLocal || isPrivate() } else true namesOfVariables .forEach { variableName -> // variable should not contain only one letter in it's name. This is a bad example: b512 @@ -158,13 +161,13 @@ class IdentifierNaming(private val configRules: List) : Rule("ident // it should be in UPPER_CASE, no need to raise this warning if it is one-letter variable if (node.isConstant()) { if (!variableName.text.isUpperSnakeCase() && variableName.text.length > 1) { - CONSTANT_UPPERCASE.warnAndFix(configRules, emitWarn, isFixMode, variableName.text, variableName.startOffset, node) { + CONSTANT_UPPERCASE.warnAndFix(configRules, emitWarn, isFix, variableName.text, variableName.startOffset, node) { (variableName as LeafPsiElement).replaceWithText(variableName.text.toUpperSnakeCase()) } } } else if (variableName.text != "_" && !variableName.text.isLowerCamelCase()) { // variable name should be in camel case. The only exception is a list of industry standard variables like i, j, k. - VARIABLE_NAME_INCORRECT_FORMAT.warnAndFix(configRules, emitWarn, isFixMode, variableName.text, variableName.startOffset, node) { + VARIABLE_NAME_INCORRECT_FORMAT.warnAndFix(configRules, emitWarn, isFix, variableName.text, variableName.startOffset, node) { // FixMe: cover fixes with tests variableName .parent({it.elementType == FILE}) diff --git a/diktat-rules/src/test/resources/test/paragraph1/naming/identifiers/VariableNamingExpected.kt b/diktat-rules/src/test/resources/test/paragraph1/naming/identifiers/VariableNamingExpected.kt index e1fc3b40f7..c855c2ff02 100644 --- a/diktat-rules/src/test/resources/test/paragraph1/naming/identifiers/VariableNamingExpected.kt +++ b/diktat-rules/src/test/resources/test/paragraph1/naming/identifiers/VariableNamingExpected.kt @@ -13,7 +13,7 @@ private val voiceIpPort = "" class A { private val voiceIpPort = "" - public val valNX2567 = "" + public val valN_x2567 = "" fun foo() { val voiceIpPorts = "" @@ -24,4 +24,24 @@ class A { val qwe = lowerSnake val pre = valNX256 } +} + +class B { + companion object { + val QQ = 1 + private val qwe = 11 + } + + fun foo() { + var qq = 20 + while (qq < 20) { + qq = 10 + } + } + + class Ba { + fun goo() { + val qwe = QQ + } + } } \ No newline at end of file diff --git a/diktat-rules/src/test/resources/test/paragraph1/naming/identifiers/VariableNamingTest.kt b/diktat-rules/src/test/resources/test/paragraph1/naming/identifiers/VariableNamingTest.kt index 16d72c2307..b8ad06f6b9 100644 --- a/diktat-rules/src/test/resources/test/paragraph1/naming/identifiers/VariableNamingTest.kt +++ b/diktat-rules/src/test/resources/test/paragraph1/naming/identifiers/VariableNamingTest.kt @@ -13,7 +13,7 @@ private val VoiceIP_port = "" class A { private val VoiceIP_port = "" - public val ValN_x2567 = "" + public val valN_x2567 = "" fun foo() { val VoiceIP_ports = "" @@ -25,3 +25,23 @@ class A { val pre = ValN_x256 } } + +class B { + companion object { + val QQ = 1 + private val QWE = 11 + } + + fun foo() { + var QQ = 20 + while (QQ < 20) { + QQ = 10 + } + } + + class BA { + fun goo() { + val qwe = QQ + } + } +} From f42ce90ddd654be7371b47051b3202336af160f4 Mon Sep 17 00:00:00 2001 From: kentr0w Date: Mon, 21 Dec 2020 16:58:34 +0300 Subject: [PATCH 4/7] Fix for incorrect variable name handle ### What's done: Fixed after review --- .../kotlin/org/cqfn/diktat/ruleset/rules/IdentifierNaming.kt | 3 ++- .../paragraph1/naming/identifiers/VariableNamingExpected.kt | 1 + .../test/paragraph1/naming/identifiers/VariableNamingTest.kt | 1 + 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/IdentifierNaming.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/IdentifierNaming.kt index 1087ef2774..2e12d1845a 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/IdentifierNaming.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/IdentifierNaming.kt @@ -144,6 +144,7 @@ class IdentifierNaming(private val configRules: List) : Rule("ident private fun checkVariableName(node: ASTNode): List { // special case for Destructuring declarations that can be treated as parameters in lambda: var namesOfVariables = extractVariableIdentifiers(node) + // only local private properties will be autofix in order not to break code if there are usages in other files val isFix = isFixMode && if (node.elementType == PROPERTY) (node.psi as KtProperty).run { isLocal || isPrivate() } else true namesOfVariables .forEach { variableName -> @@ -171,7 +172,7 @@ class IdentifierNaming(private val configRules: List) : Rule("ident // FixMe: cover fixes with tests variableName .parent({it.elementType == FILE}) - ?.findAllVariablesWithUsages { it.name == variableName.text && (it.isLocal || it.isPrivate()) } + ?.findAllVariablesWithUsages { it.name == variableName.text } ?.flatMap { it.value.toList() } ?.forEach { (it.node.firstChildNode as LeafPsiElement).replaceWithText(variableName.text.toLowerCamelCase())} (variableName as LeafPsiElement).replaceWithText(variableName.text.toLowerCamelCase()) diff --git a/diktat-rules/src/test/resources/test/paragraph1/naming/identifiers/VariableNamingExpected.kt b/diktat-rules/src/test/resources/test/paragraph1/naming/identifiers/VariableNamingExpected.kt index c855c2ff02..a3f5eae19d 100644 --- a/diktat-rules/src/test/resources/test/paragraph1/naming/identifiers/VariableNamingExpected.kt +++ b/diktat-rules/src/test/resources/test/paragraph1/naming/identifiers/VariableNamingExpected.kt @@ -18,6 +18,7 @@ class A { fun foo() { val voiceIpPorts = "" val upperSnakers = voiceIpPort + qww(voiceIpPorts) } fun goo() { diff --git a/diktat-rules/src/test/resources/test/paragraph1/naming/identifiers/VariableNamingTest.kt b/diktat-rules/src/test/resources/test/paragraph1/naming/identifiers/VariableNamingTest.kt index b8ad06f6b9..10bfbdc829 100644 --- a/diktat-rules/src/test/resources/test/paragraph1/naming/identifiers/VariableNamingTest.kt +++ b/diktat-rules/src/test/resources/test/paragraph1/naming/identifiers/VariableNamingTest.kt @@ -18,6 +18,7 @@ class A { fun foo() { val VoiceIP_ports = "" val UPPER_SNAKERS = VoiceIP_port + qww(VoiceIP_ports) } fun goo() { From 23de811d4ac4a68fa4fc9aae196d2a9b1202bbcb Mon Sep 17 00:00:00 2001 From: kentr0w Date: Mon, 21 Dec 2020 19:36:54 +0300 Subject: [PATCH 5/7] Fix for incorrect variable name handle ### What's done: Fixed after review --- .../org/cqfn/diktat/ruleset/rules/IdentifierNaming.kt | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/IdentifierNaming.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/IdentifierNaming.kt index 2e12d1845a..fdd6911614 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/IdentifierNaming.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/IdentifierNaming.kt @@ -145,7 +145,8 @@ class IdentifierNaming(private val configRules: List) : Rule("ident // special case for Destructuring declarations that can be treated as parameters in lambda: var namesOfVariables = extractVariableIdentifiers(node) // only local private properties will be autofix in order not to break code if there are usages in other files - val isFix = isFixMode && if (node.elementType == PROPERTY) (node.psi as KtProperty).run { isLocal || isPrivate() } else true + val isFix = isFixMode && if (node.elementType == PROPERTY) (node.psi as KtProperty).run { isLocal || isPrivate() } else true + // Destructuring declarations are only allowed for local variables/values namesOfVariables .forEach { variableName -> // variable should not contain only one letter in it's name. This is a bad example: b512 @@ -170,12 +171,13 @@ class IdentifierNaming(private val configRules: List) : Rule("ident // variable name should be in camel case. The only exception is a list of industry standard variables like i, j, k. VARIABLE_NAME_INCORRECT_FORMAT.warnAndFix(configRules, emitWarn, isFix, variableName.text, variableName.startOffset, node) { // FixMe: cover fixes with tests + val correctVariableName = variableName.text.toLowerCamelCase() variableName .parent({it.elementType == FILE}) ?.findAllVariablesWithUsages { it.name == variableName.text } ?.flatMap { it.value.toList() } - ?.forEach { (it.node.firstChildNode as LeafPsiElement).replaceWithText(variableName.text.toLowerCamelCase())} - (variableName as LeafPsiElement).replaceWithText(variableName.text.toLowerCamelCase()) + ?.forEach { (it.node.firstChildNode as LeafPsiElement).replaceWithText(correctVariableName)} + (variableName as LeafPsiElement).replaceWithText(correctVariableName) } } } From 0491cabfac9eab265ef02802b6d0a244810e134e Mon Sep 17 00:00:00 2001 From: kentr0w Date: Tue, 22 Dec 2020 10:18:37 +0300 Subject: [PATCH 6/7] Fix for incorrect variable name handle ### What's done: Fixed after review --- .../kotlin/org/cqfn/diktat/ruleset/rules/IdentifierNaming.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/IdentifierNaming.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/IdentifierNaming.kt index fdd6911614..a43a8778f0 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/IdentifierNaming.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/IdentifierNaming.kt @@ -145,7 +145,7 @@ class IdentifierNaming(private val configRules: List) : Rule("ident // special case for Destructuring declarations that can be treated as parameters in lambda: var namesOfVariables = extractVariableIdentifiers(node) // only local private properties will be autofix in order not to break code if there are usages in other files - val isFix = isFixMode && if (node.elementType == PROPERTY) (node.psi as KtProperty).run { isLocal || isPrivate() } else true + val isFix = isFixMode && if (node.elementType == PROPERTY) (node.psi as KtProperty).run { isLocal || isPrivate() } else true // Destructuring declarations are only allowed for local variables/values namesOfVariables .forEach { variableName -> From b8735e1c0032a55a538559f7a989a2e0c34ae5fa Mon Sep 17 00:00:00 2001 From: kentr0w Date: Tue, 22 Dec 2020 12:00:36 +0300 Subject: [PATCH 7/7] Fix for incorrect variable name handle ### What's done: Final fix --- .../kotlin/org/cqfn/diktat/ruleset/rules/IdentifierNaming.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/IdentifierNaming.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/IdentifierNaming.kt index a43a8778f0..aa1c669d34 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/IdentifierNaming.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/IdentifierNaming.kt @@ -144,9 +144,9 @@ class IdentifierNaming(private val configRules: List) : Rule("ident private fun checkVariableName(node: ASTNode): List { // special case for Destructuring declarations that can be treated as parameters in lambda: var namesOfVariables = extractVariableIdentifiers(node) - // only local private properties will be autofix in order not to break code if there are usages in other files + // Only local private properties will be autofix in order not to break code if there are usages in other files. + // Destructuring declarations are only allowed for local variables/values, so we don't need to calculate `isFix` for every node in `namesOfVariables` val isFix = isFixMode && if (node.elementType == PROPERTY) (node.psi as KtProperty).run { isLocal || isPrivate() } else true - // Destructuring declarations are only allowed for local variables/values namesOfVariables .forEach { variableName -> // variable should not contain only one letter in it's name. This is a bad example: b512