From 4b00836a937079262eb416cdf751fe2671f79424 Mon Sep 17 00:00:00 2001 From: soWhoAmI Date: Thu, 29 Oct 2020 19:05:18 +0300 Subject: [PATCH 01/13] feature/rule-6.1.7-implicit-backing-property(#443) ### What's done: * First logic * Added tests --- .../cqfn/diktat/ruleset/constants/Warnings.kt | 1 + .../rules/ImplicitBackingPropertyRule.kt | 83 +++++++++++++++++++ .../ImplicitBackingPropertyWarnTest.kt | 54 ++++++++++++ 3 files changed, 138 insertions(+) create mode 100644 diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt create mode 100644 diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/ImplicitBackingPropertyWarnTest.kt diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt index 2fded88830..a8362d0066 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt @@ -120,6 +120,7 @@ enum class Warnings(private val canBeAutoCorrected: Boolean, private val warn: S USE_DATA_CLASS(false, "this class can be converted to a data class"), WRONG_NAME_OF_VARIABLE_INSIDE_ACCESSOR(false, "Use `field` keyword instead of property name inside property accessors"), CLASS_SHOULD_NOT_BE_ABSTRACT(true, "class should not be abstract, because it has no abstract functions"), + NO_CORRESPONDING_PROPERTY(false, "there is no corresponding property") ; /** diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt new file mode 100644 index 0000000000..b38826fb4c --- /dev/null +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt @@ -0,0 +1,83 @@ +package org.cqfn.diktat.ruleset.rules + +import com.pinterest.ktlint.core.Rule +import com.pinterest.ktlint.core.ast.ElementType.FILE +import com.pinterest.ktlint.core.ast.ElementType.IDENTIFIER +import com.pinterest.ktlint.core.ast.ElementType.PROPERTY +import com.pinterest.ktlint.core.ast.ElementType.PROPERTY_ACCESSOR +import com.pinterest.ktlint.core.ast.ElementType.REFERENCE_EXPRESSION +import com.pinterest.ktlint.core.ast.ElementType.VALUE_PARAMETER +import org.cqfn.diktat.common.config.rules.RulesConfig +import org.cqfn.diktat.ruleset.constants.Warnings.NO_CORRESPONDING_PROPERTY +import org.cqfn.diktat.ruleset.utils.findAllNodesWithSpecificType +import org.cqfn.diktat.ruleset.utils.getFirstChildWithType +import org.cqfn.diktat.ruleset.utils.getIdentifierName +import org.cqfn.diktat.ruleset.utils.hasAnyChildOfTypes +import org.jetbrains.kotlin.com.intellij.lang.ASTNode + +class ImplicitBackingPropertyRule(private val configRules: List) : Rule("implicit-backing-property") { + private lateinit var emitWarn: ((offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit) + private var isFixMode: Boolean = false + + override fun visit(node: ASTNode, + autoCorrect: Boolean, + emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit) { + emitWarn = emit + isFixMode = autoCorrect + + if (node.elementType == FILE) { + findAllProperties(node) + } + } + + private fun findAllProperties(node: ASTNode) { + val properties = node.findAllNodesWithSpecificType(PROPERTY) + + val propsWithBackSymbol = mutableListOf() + + properties + .filter { it.getFirstChildWithType(IDENTIFIER)!!.text.startsWith("_") } + .forEach { + propsWithBackSymbol.add(it.getFirstChildWithType(IDENTIFIER)!!.text) + } + + properties.filter { it.hasAnyChildOfTypes(PROPERTY_ACCESSOR) }.forEach { + validateAccessors(it, propsWithBackSymbol) + } + } + + private fun validateAccessors(node: ASTNode, propsWithBackSymbol: List) { + val accessors = node.findAllNodesWithSpecificType(PROPERTY_ACCESSOR) + + accessors.forEach { + val refExprs = it.findAllNodesWithSpecificType(REFERENCE_EXPRESSION) + + if (refExprs.isNotEmpty()) { + handleReferenceExpressions(node, refExprs, propsWithBackSymbol) + } + } + } + + private fun handleReferenceExpressions(node:ASTNode, + expressions: List, + backingPropertiesNames: List) { + if (expressions.none { backingPropertiesNames.contains(it.text) || it.text != "field" }) { + raiseWarning(node, node.getFirstChildWithType(IDENTIFIER)!!.text) + } + } + + private fun raiseWarning(node:ASTNode, propName: String) { + NO_CORRESPONDING_PROPERTY.warn(configRules, emitWarn, isFixMode, + "_$propName has no corresponding property with name $propName", node.startOffset, node) + } + + private fun List.hasElementWithIdentifierName(name: String): Boolean { + forEach { + val identifier = it.getIdentifierName()!!.text + + if (identifier == name) + return true + } + return false + } +} \ No newline at end of file diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/ImplicitBackingPropertyWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/ImplicitBackingPropertyWarnTest.kt new file mode 100644 index 0000000000..b81696d8ac --- /dev/null +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/ImplicitBackingPropertyWarnTest.kt @@ -0,0 +1,54 @@ +package org.cqfn.diktat.ruleset.chapter6 + +import com.pinterest.ktlint.core.LintError +import generated.WarningNames +import org.cqfn.diktat.ruleset.constants.Warnings +import org.cqfn.diktat.ruleset.rules.DIKTAT_RULE_SET_ID +import org.cqfn.diktat.ruleset.rules.ImplicitBackingPropertyRule +import org.cqfn.diktat.util.LintTestBase +import org.junit.jupiter.api.Tag +import org.junit.jupiter.api.Test + +class ImplicitBackingPropertyWarnTest: LintTestBase(::ImplicitBackingPropertyRule) { + private val ruleId = "$DIKTAT_RULE_SET_ID:implicit-backing-property" + + @Test + @Tag("") + fun `not trigger on backing property`() { + lintMethod( + """ + |class Some(val a: Int = 5) { + | private var _table: Map? = null + | val table:Map + | get() { + | if (_table == null) { + | _table = HashMap() + | } + | return _table ?: throw AssertionError("Set to null by another thread") + | } + | set(value) {field = value} + |} + """.trimMargin() + ) + } + + @Test + @Tag("") + fun `trigger on backing property`() { + lintMethod( + """ + |class Some(val a: Int = 5) { + | private var _a: Map? = null + | val table:Map + | get() { + | if (_a == null) { + | _a = HashMap() + | } + | return _a ?: throw AssertionError("Set to null by another thread") + | } + | set(value) {field = value} + |} + """.trimMargin() + ) + } +} \ No newline at end of file From 09efa9ef8d1338761da1ebeb0d679248b1711a42 Mon Sep 17 00:00:00 2001 From: soWhoAmI Date: Fri, 30 Oct 2020 15:16:48 +0300 Subject: [PATCH 02/13] feature/rule-6.1.7-implicit-backing-property(#443) ### What's done: * Logic made * Added warn tests --- diktat-analysis.yml | 4 ++ .../src/main/kotlin/generated/WarningNames.kt | 2 + .../ruleset/rules/DiktatRuleSetProvider.kt | 1 + .../rules/ImplicitBackingPropertyRule.kt | 14 ++----- .../main/resources/diktat-analysis-huawei.yml | 4 ++ .../src/main/resources/diktat-analysis.yml | 4 ++ .../ImplicitBackingPropertyWarnTest.kt | 41 +++++++++++++++++++ info/available-rules.md | 3 +- 8 files changed, 61 insertions(+), 12 deletions(-) diff --git a/diktat-analysis.yml b/diktat-analysis.yml index 5bd9ce5ee7..02a6f16b98 100644 --- a/diktat-analysis.yml +++ b/diktat-analysis.yml @@ -335,5 +335,9 @@ configuration: {} # Checks that there are abstract functions in abstract class - name: CLASS_SHOULD_NOT_BE_ABSTRACT + enabled: true + configuration: {} +# In case of using "implicit backing property" scheme, the name of real and back property should be the same +- name: NO_CORRESPONDING_PROPERTY enabled: true configuration: {} \ No newline at end of file diff --git a/diktat-rules/src/main/kotlin/generated/WarningNames.kt b/diktat-rules/src/main/kotlin/generated/WarningNames.kt index 7b8c4475b5..fa7f0426b4 100644 --- a/diktat-rules/src/main/kotlin/generated/WarningNames.kt +++ b/diktat-rules/src/main/kotlin/generated/WarningNames.kt @@ -198,4 +198,6 @@ public object WarningNames { "WRONG_NAME_OF_VARIABLE_INSIDE_ACCESSOR" public const val CLASS_SHOULD_NOT_BE_ABSTRACT: String = "CLASS_SHOULD_NOT_BE_ABSTRACT" + + public const val NO_CORRESPONDING_PROPERTY: String = "NO_CORRESPONDING_PROPERTY" } diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt index 25cc20cb2e..3cc0d37094 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt @@ -56,6 +56,7 @@ class DiktatRuleSetProvider(private val diktatConfigFile: String = "diktat-analy ::FileSize, ::DataClassesRule, ::IdentifierNaming, + ::ImplicitBackingPropertyRule, ::LocalVariablesRule, ::ClassLikeStructuresOrderRule, ::BracesInConditionalsAndLoopsRule, diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt index b38826fb4c..fc8196aaa6 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt @@ -1,6 +1,7 @@ package org.cqfn.diktat.ruleset.rules import com.pinterest.ktlint.core.Rule +import com.pinterest.ktlint.core.ast.ElementType.CLASS_BODY import com.pinterest.ktlint.core.ast.ElementType.FILE import com.pinterest.ktlint.core.ast.ElementType.IDENTIFIER import com.pinterest.ktlint.core.ast.ElementType.PROPERTY @@ -25,13 +26,13 @@ class ImplicitBackingPropertyRule(private val configRules: List) : emitWarn = emit isFixMode = autoCorrect - if (node.elementType == FILE) { + if (node.elementType == CLASS_BODY) { findAllProperties(node) } } private fun findAllProperties(node: ASTNode) { - val properties = node.findAllNodesWithSpecificType(PROPERTY) + val properties = node.getChildren(null).filter { it.elementType == PROPERTY } val propsWithBackSymbol = mutableListOf() @@ -71,13 +72,4 @@ class ImplicitBackingPropertyRule(private val configRules: List) : "_$propName has no corresponding property with name $propName", node.startOffset, node) } - private fun List.hasElementWithIdentifierName(name: String): Boolean { - forEach { - val identifier = it.getIdentifierName()!!.text - - if (identifier == name) - return true - } - return false - } } \ No newline at end of file diff --git a/diktat-rules/src/main/resources/diktat-analysis-huawei.yml b/diktat-rules/src/main/resources/diktat-analysis-huawei.yml index de907e9834..3a8346c5d6 100644 --- a/diktat-rules/src/main/resources/diktat-analysis-huawei.yml +++ b/diktat-rules/src/main/resources/diktat-analysis-huawei.yml @@ -334,5 +334,9 @@ configuration: {} # Checks that there are abstract functions in abstract class - name: CLASS_SHOULD_NOT_BE_ABSTRACT + enabled: true + configuration: {} +# In case of using "implicit backing property" scheme, the name of real and back property should be the same +- name: NO_CORRESPONDING_PROPERTY enabled: true configuration: {} \ No newline at end of file diff --git a/diktat-rules/src/main/resources/diktat-analysis.yml b/diktat-rules/src/main/resources/diktat-analysis.yml index 639bcdd582..fe46a2463e 100644 --- a/diktat-rules/src/main/resources/diktat-analysis.yml +++ b/diktat-rules/src/main/resources/diktat-analysis.yml @@ -336,5 +336,9 @@ configuration: {} # Checks that there are abstract functions in abstract class - name: CLASS_SHOULD_NOT_BE_ABSTRACT + enabled: true + configuration: {} +# In case of using "implicit backing property" scheme, the name of real and back property should be the same +- name: NO_CORRESPONDING_PROPERTY enabled: true configuration: {} \ No newline at end of file diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/ImplicitBackingPropertyWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/ImplicitBackingPropertyWarnTest.kt index b81696d8ac..dbc2445ec6 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/ImplicitBackingPropertyWarnTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/ImplicitBackingPropertyWarnTest.kt @@ -51,4 +51,45 @@ class ImplicitBackingPropertyWarnTest: LintTestBase(::ImplicitBackingPropertyRul """.trimMargin() ) } + + @Test + @Tag("") + fun `don't trigger on regular backing property`() { + lintMethod( + """ + |class Some(val a: Int = 5) { + | private var _a: Map? = null + | private val _some:Int? = null + |} + """.trimMargin() + ) + } + + @Test + @Tag("") + fun `don't trigger on regular property`() { + lintMethod( + """ + |class Some(val a: Int = 5) { + | private var a: Map? = null + | private val some:Int? = null + | private val _prop: String? = null + |} + """.trimMargin() + ) + } + + @Test + @Tag("") + fun `should not trigger if property has field in accessor`() { + lintMethod( + """ + |class Some(val a: Int = 5) { + | val table:Map + | set(value) { field = value } + | val _table: Map? = null + |} + """.trimMargin() + ) + } } \ No newline at end of file diff --git a/info/available-rules.md b/info/available-rules.md index 64d0f9a380..871b2f2917 100644 --- a/info/available-rules.md +++ b/info/available-rules.md @@ -93,4 +93,5 @@ | 5 | 5.2.3 | WRONG_OVERLOADING_FUNCTION_ARGUMENTS | Check: function has overloading instead use default arguments | no | -| | 6 | 6.1.2 | USE_DATA_CLASS | Check: if class can be made as data class | no | - | yes | | 6 | 6.1.9 | WRONG_NAME_OF_VARIABLE_INSIDE_ACCESSOR | Check: used the name of a variable in the custom getter or setter | no | - | -| 6 | 6.1.6 | CLASS_SHOULD_NOT_BE_ABSTRACT | Checks: if abstract class has any abstract method. If not, warns that class should not be abstract
Fix: deletes abstract modifier | yes | - | - | \ No newline at end of file +| 6 | 6.1.6 | CLASS_SHOULD_NOT_BE_ABSTRACT | Checks: if abstract class has any abstract method. If not, warns that class should not be abstract
Fix: deletes abstract modifier | yes | - | - | +| 6 | 6.1.7 | NO_CORRESPONDING_PROPERTY | Checks: if in case of using "implicit backing property" scheme, the name of real and back property are the same | no | - | - | \ No newline at end of file From d4d447dd779ae292638f7a2a85a041dae0e8a454 Mon Sep 17 00:00:00 2001 From: soWhoAmI Date: Fri, 30 Oct 2020 15:20:40 +0300 Subject: [PATCH 03/13] feature/rule-6.1.7-implicit-backing-property(#443) ### What's done: * Fixed bugs --- .../cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt | 3 ++- .../diktat/ruleset/chapter6/ImplicitBackingPropertyWarnTest.kt | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt index fc8196aaa6..891427878d 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt @@ -59,6 +59,7 @@ class ImplicitBackingPropertyRule(private val configRules: List) : } } + @Suppress("UnsafeCallOnNullableType") private fun handleReferenceExpressions(node:ASTNode, expressions: List, backingPropertiesNames: List) { @@ -72,4 +73,4 @@ class ImplicitBackingPropertyRule(private val configRules: List) : "_$propName has no corresponding property with name $propName", node.startOffset, node) } -} \ No newline at end of file +} diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/ImplicitBackingPropertyWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/ImplicitBackingPropertyWarnTest.kt index dbc2445ec6..9542570f47 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/ImplicitBackingPropertyWarnTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/ImplicitBackingPropertyWarnTest.kt @@ -92,4 +92,4 @@ class ImplicitBackingPropertyWarnTest: LintTestBase(::ImplicitBackingPropertyRul """.trimMargin() ) } -} \ No newline at end of file +} From bd87c965812484e57c5ca8a95cad79aa8be7de1c Mon Sep 17 00:00:00 2001 From: soWhoAmI Date: Fri, 30 Oct 2020 15:36:57 +0300 Subject: [PATCH 04/13] feature/rule-6.1.7-implicit-backing-property(#443) ### What's done: * Fixed bugs --- .../org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt | 1 - .../org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt index d4fdaa86ca..1052c7d2a2 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt @@ -75,7 +75,6 @@ class DiktatRuleSetProvider(private val diktatConfigFile: String = "diktat-analy ::ImplicitBackingPropertyRule, ::StringTemplateFormatRule, ::DataClassesRule, - ::LocalVariablesRule, ::SmartCastRule, ::PropertyAccessorFields, ::AbstractClassesRule, diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt index 891427878d..e2192c1a41 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt @@ -31,6 +31,7 @@ class ImplicitBackingPropertyRule(private val configRules: List) : } } + @Suppress("UnsafeCallOnNullableType") private fun findAllProperties(node: ASTNode) { val properties = node.getChildren(null).filter { it.elementType == PROPERTY } From 92e41e65b229de2c5999e1be902e478a9524b7e1 Mon Sep 17 00:00:00 2001 From: soWhoAmI Date: Mon, 2 Nov 2020 12:03:37 +0300 Subject: [PATCH 05/13] feature/rule-6.1.7-implicit-backing-property(#443) ### What's done: * Fixed bugs --- diktat-analysis.yml | 4 +++- .../cqfn/diktat/ruleset/constants/Warnings.kt | 2 +- .../rules/ImplicitBackingPropertyRule.kt | 15 ++++++------ .../main/resources/diktat-analysis-huawei.yml | 4 +++- .../src/main/resources/diktat-analysis.yml | 4 +++- .../ImplicitBackingPropertyWarnTest.kt | 23 ++++++++++--------- 6 files changed, 30 insertions(+), 22 deletions(-) diff --git a/diktat-analysis.yml b/diktat-analysis.yml index 7b2773e95c..eee21bce69 100644 --- a/diktat-analysis.yml +++ b/diktat-analysis.yml @@ -341,7 +341,9 @@ - name: CLASS_SHOULD_NOT_BE_ABSTRACT enabled: true configuration: {} -# In case of using "implicit backing property" scheme, the name of real and back property should be the same +# In case of not using field keyword in property accessors, +# there should be explicit backing property with the name of real property +# Example: val table get() {if (_table == null) ...} -> table should have _table - name: NO_CORRESPONDING_PROPERTY enabled: true configuration: {} \ No newline at end of file diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt index 5df1639278..bf6eb0d412 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt @@ -121,7 +121,7 @@ enum class Warnings(private val canBeAutoCorrected: Boolean, private val warn: S WRONG_NAME_OF_VARIABLE_INSIDE_ACCESSOR(false, "Use `field` keyword instead of property name inside property accessors"), MULTIPLE_INIT_BLOCKS(true, "Avoid using multiple `init` blocks, this logic can be moved to constructors or properties declarations"), CLASS_SHOULD_NOT_BE_ABSTRACT(true, "class should not be abstract, because it has no abstract functions"), - NO_CORRESPONDING_PROPERTY(false, "there is no corresponding property") + NO_CORRESPONDING_PROPERTY(false, "backing property should have the same name, but there is no corresponding property") ; /** diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt index e2192c1a41..a442284453 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt @@ -16,6 +16,9 @@ import org.cqfn.diktat.ruleset.utils.getIdentifierName import org.cqfn.diktat.ruleset.utils.hasAnyChildOfTypes import org.jetbrains.kotlin.com.intellij.lang.ASTNode +/** + * This rule checks if there is a backing property for field with property accessors, in case they don't use field keyword + */ class ImplicitBackingPropertyRule(private val configRules: List) : Rule("implicit-backing-property") { private lateinit var emitWarn: ((offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit) private var isFixMode: Boolean = false @@ -35,12 +38,10 @@ class ImplicitBackingPropertyRule(private val configRules: List) : private fun findAllProperties(node: ASTNode) { val properties = node.getChildren(null).filter { it.elementType == PROPERTY } - val propsWithBackSymbol = mutableListOf() - - properties + val propsWithBackSymbol = properties .filter { it.getFirstChildWithType(IDENTIFIER)!!.text.startsWith("_") } - .forEach { - propsWithBackSymbol.add(it.getFirstChildWithType(IDENTIFIER)!!.text) + .map { + it.getFirstChildWithType(IDENTIFIER)!!.text } properties.filter { it.hasAnyChildOfTypes(PROPERTY_ACCESSOR) }.forEach { @@ -64,14 +65,14 @@ class ImplicitBackingPropertyRule(private val configRules: List) : private fun handleReferenceExpressions(node:ASTNode, expressions: List, backingPropertiesNames: List) { - if (expressions.none { backingPropertiesNames.contains(it.text) || it.text != "field" }) { + if (expressions.none { backingPropertiesNames.contains(it.text) || it.text == "field" }) { raiseWarning(node, node.getFirstChildWithType(IDENTIFIER)!!.text) } } private fun raiseWarning(node:ASTNode, propName: String) { NO_CORRESPONDING_PROPERTY.warn(configRules, emitWarn, isFixMode, - "_$propName has no corresponding property with name $propName", node.startOffset, node) + "$propName has no corresponding property with name _$propName", node.startOffset, node) } } diff --git a/diktat-rules/src/main/resources/diktat-analysis-huawei.yml b/diktat-rules/src/main/resources/diktat-analysis-huawei.yml index b1fed63228..37762166af 100644 --- a/diktat-rules/src/main/resources/diktat-analysis-huawei.yml +++ b/diktat-rules/src/main/resources/diktat-analysis-huawei.yml @@ -340,7 +340,9 @@ - name: CLASS_SHOULD_NOT_BE_ABSTRACT enabled: true configuration: {} -# In case of using "implicit backing property" scheme, the name of real and back property should be the same +# In case of not using field keyword in property accessors, +# there should be explicit backing property with the name of real property +# Example: val table get() {if (_table == null) ...} -> table should have _table - name: NO_CORRESPONDING_PROPERTY enabled: true configuration: {} \ No newline at end of file diff --git a/diktat-rules/src/main/resources/diktat-analysis.yml b/diktat-rules/src/main/resources/diktat-analysis.yml index eae5f22c6e..626b05c614 100644 --- a/diktat-rules/src/main/resources/diktat-analysis.yml +++ b/diktat-rules/src/main/resources/diktat-analysis.yml @@ -342,7 +342,9 @@ - name: CLASS_SHOULD_NOT_BE_ABSTRACT enabled: true configuration: {} -# In case of using "implicit backing property" scheme, the name of real and back property should be the same +# In case of not using field keyword in property accessors, +# there should be explicit backing property with the name of real property +# Example: val table get() {if (_table == null) ...} -> table should have _table - name: NO_CORRESPONDING_PROPERTY enabled: true configuration: {} \ No newline at end of file diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/ImplicitBackingPropertyWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/ImplicitBackingPropertyWarnTest.kt index 9542570f47..c84140eaf7 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/ImplicitBackingPropertyWarnTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/ImplicitBackingPropertyWarnTest.kt @@ -1,7 +1,7 @@ package org.cqfn.diktat.ruleset.chapter6 import com.pinterest.ktlint.core.LintError -import generated.WarningNames +import generated.WarningNames.NO_CORRESPONDING_PROPERTY import org.cqfn.diktat.ruleset.constants.Warnings import org.cqfn.diktat.ruleset.rules.DIKTAT_RULE_SET_ID import org.cqfn.diktat.ruleset.rules.ImplicitBackingPropertyRule @@ -13,7 +13,7 @@ class ImplicitBackingPropertyWarnTest: LintTestBase(::ImplicitBackingPropertyRul private val ruleId = "$DIKTAT_RULE_SET_ID:implicit-backing-property" @Test - @Tag("") + @Tag(NO_CORRESPONDING_PROPERTY) fun `not trigger on backing property`() { lintMethod( """ @@ -33,27 +33,28 @@ class ImplicitBackingPropertyWarnTest: LintTestBase(::ImplicitBackingPropertyRul } @Test - @Tag("") + @Tag(NO_CORRESPONDING_PROPERTY) fun `trigger on backing property`() { lintMethod( """ |class Some(val a: Int = 5) { - | private var _a: Map? = null + | private var a: Map? = null | val table:Map | get() { - | if (_a == null) { - | _a = HashMap() + | if (a == null) { + | a = HashMap() | } - | return _a ?: throw AssertionError("Set to null by another thread") + | return a ?: throw AssertionError("Set to null by another thread") | } | set(value) {field = value} |} - """.trimMargin() + """.trimMargin(), + LintError(3,4,ruleId, "${Warnings.NO_CORRESPONDING_PROPERTY.warnText()} table has no corresponding property with name _table") ) } @Test - @Tag("") + @Tag(NO_CORRESPONDING_PROPERTY) fun `don't trigger on regular backing property`() { lintMethod( """ @@ -66,7 +67,7 @@ class ImplicitBackingPropertyWarnTest: LintTestBase(::ImplicitBackingPropertyRul } @Test - @Tag("") + @Tag(NO_CORRESPONDING_PROPERTY) fun `don't trigger on regular property`() { lintMethod( """ @@ -80,7 +81,7 @@ class ImplicitBackingPropertyWarnTest: LintTestBase(::ImplicitBackingPropertyRul } @Test - @Tag("") + @Tag(NO_CORRESPONDING_PROPERTY) fun `should not trigger if property has field in accessor`() { lintMethod( """ From 8a684485a089cfd31c054f64a55c4354198f6360 Mon Sep 17 00:00:00 2001 From: soWhoAmI Date: Tue, 3 Nov 2020 13:49:10 +0300 Subject: [PATCH 06/13] feature/rule-6.1.7-implicit-backing-property(#443) ### What's done: * Fixed bugs --- .../cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt | 4 +++- .../ruleset/chapter6/ImplicitBackingPropertyWarnTest.kt | 5 ++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt index a442284453..9cb31c519d 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt @@ -1,6 +1,7 @@ package org.cqfn.diktat.ruleset.rules import com.pinterest.ktlint.core.Rule +import com.pinterest.ktlint.core.ast.ElementType.BLOCK import com.pinterest.ktlint.core.ast.ElementType.CLASS_BODY import com.pinterest.ktlint.core.ast.ElementType.FILE import com.pinterest.ktlint.core.ast.ElementType.IDENTIFIER @@ -14,6 +15,7 @@ import org.cqfn.diktat.ruleset.utils.findAllNodesWithSpecificType import org.cqfn.diktat.ruleset.utils.getFirstChildWithType import org.cqfn.diktat.ruleset.utils.getIdentifierName import org.cqfn.diktat.ruleset.utils.hasAnyChildOfTypes +import org.cqfn.diktat.ruleset.utils.hasChildOfType import org.jetbrains.kotlin.com.intellij.lang.ASTNode /** @@ -50,7 +52,7 @@ class ImplicitBackingPropertyRule(private val configRules: List) : } private fun validateAccessors(node: ASTNode, propsWithBackSymbol: List) { - val accessors = node.findAllNodesWithSpecificType(PROPERTY_ACCESSOR) + val accessors = node.findAllNodesWithSpecificType(PROPERTY_ACCESSOR).filter { it.hasChildOfType(BLOCK) } // exclude inline get accessors.forEach { val refExprs = it.findAllNodesWithSpecificType(REFERENCE_EXPRESSION) diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/ImplicitBackingPropertyWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/ImplicitBackingPropertyWarnTest.kt index c84140eaf7..9d59f35b26 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/ImplicitBackingPropertyWarnTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/ImplicitBackingPropertyWarnTest.kt @@ -46,7 +46,7 @@ class ImplicitBackingPropertyWarnTest: LintTestBase(::ImplicitBackingPropertyRul | } | return a ?: throw AssertionError("Set to null by another thread") | } - | set(value) {field = value} + | set(value) { field = value } |} """.trimMargin(), LintError(3,4,ruleId, "${Warnings.NO_CORRESPONDING_PROPERTY.warnText()} table has no corresponding property with name _table") @@ -89,6 +89,9 @@ class ImplicitBackingPropertyWarnTest: LintTestBase(::ImplicitBackingPropertyRul | val table:Map | set(value) { field = value } | val _table: Map? = null + | + | val some: Int + | get() = 3 |} """.trimMargin() ) From 02f0053fddd2cda74603bccda29228411a2ad30b Mon Sep 17 00:00:00 2001 From: soWhoAmI Date: Tue, 3 Nov 2020 14:37:41 +0300 Subject: [PATCH 07/13] feature/rule-6.1.7-implicit-backing-property(#443) ### What's done: * Fixed bugs --- .../rules/ImplicitBackingPropertyRule.kt | 28 +++++++++++++++---- .../ImplicitBackingPropertyWarnTest.kt | 16 +++++++++++ 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt index 9cb31c519d..28a4611050 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt @@ -4,10 +4,13 @@ import com.pinterest.ktlint.core.Rule import com.pinterest.ktlint.core.ast.ElementType.BLOCK import com.pinterest.ktlint.core.ast.ElementType.CLASS_BODY import com.pinterest.ktlint.core.ast.ElementType.FILE +import com.pinterest.ktlint.core.ast.ElementType.GET_KEYWORD import com.pinterest.ktlint.core.ast.ElementType.IDENTIFIER import com.pinterest.ktlint.core.ast.ElementType.PROPERTY import com.pinterest.ktlint.core.ast.ElementType.PROPERTY_ACCESSOR import com.pinterest.ktlint.core.ast.ElementType.REFERENCE_EXPRESSION +import com.pinterest.ktlint.core.ast.ElementType.RETURN +import com.pinterest.ktlint.core.ast.ElementType.SET_KEYWORD import com.pinterest.ktlint.core.ast.ElementType.VALUE_PARAMETER import org.cqfn.diktat.common.config.rules.RulesConfig import org.cqfn.diktat.ruleset.constants.Warnings.NO_CORRESPONDING_PROPERTY @@ -16,6 +19,7 @@ import org.cqfn.diktat.ruleset.utils.getFirstChildWithType import org.cqfn.diktat.ruleset.utils.getIdentifierName import org.cqfn.diktat.ruleset.utils.hasAnyChildOfTypes import org.cqfn.diktat.ruleset.utils.hasChildOfType +import org.cqfn.diktat.ruleset.utils.prettyPrint import org.jetbrains.kotlin.com.intellij.lang.ASTNode /** @@ -54,12 +58,26 @@ class ImplicitBackingPropertyRule(private val configRules: List) : private fun validateAccessors(node: ASTNode, propsWithBackSymbol: List) { val accessors = node.findAllNodesWithSpecificType(PROPERTY_ACCESSOR).filter { it.hasChildOfType(BLOCK) } // exclude inline get - accessors.forEach { - val refExprs = it.findAllNodesWithSpecificType(REFERENCE_EXPRESSION) + accessors.filter { it.hasChildOfType(GET_KEYWORD) }.forEach { handleGetAccessors(it, node, propsWithBackSymbol) } + accessors.filter { it.hasChildOfType(SET_KEYWORD) }.forEach { handleSetAccessors(it, node, propsWithBackSymbol) } + } + + private fun handleGetAccessors(accessor: ASTNode, node: ASTNode, propsWithBackSymbol: List) { + val refExprs = accessor + .findAllNodesWithSpecificType(RETURN) + .flatMap { _return -> _return.findAllNodesWithSpecificType(REFERENCE_EXPRESSION) } + + // If refExprs is empty then we assume that it returns some constant + if (refExprs.isNotEmpty()) { + handleReferenceExpressions(node, refExprs, propsWithBackSymbol) + } + } + + private fun handleSetAccessors(accessor: ASTNode, node: ASTNode, propsWithBackSymbol: List) { + val refExprs = accessor.findAllNodesWithSpecificType(REFERENCE_EXPRESSION) - if (refExprs.isNotEmpty()) { - handleReferenceExpressions(node, refExprs, propsWithBackSymbol) - } + if (refExprs.isNotEmpty()) { + handleReferenceExpressions(node, refExprs, propsWithBackSymbol) } } diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/ImplicitBackingPropertyWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/ImplicitBackingPropertyWarnTest.kt index 9d59f35b26..ae462d37ba 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/ImplicitBackingPropertyWarnTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/ImplicitBackingPropertyWarnTest.kt @@ -96,4 +96,20 @@ class ImplicitBackingPropertyWarnTest: LintTestBase(::ImplicitBackingPropertyRul """.trimMargin() ) } + + @Test + @Tag(NO_CORRESPONDING_PROPERTY) + fun `should not trigger on property with constant return`() { + lintMethod( + """ + |class Some(val a: Int = 5) { + | val table:Int + | get() { + | return 3 + | } + | set(value) { field = value } + |} + """.trimMargin() + ) + } } From b9cc0f7f5809b73f2297529b97fcc987d0d9860c Mon Sep 17 00:00:00 2001 From: soWhoAmI Date: Fri, 20 Nov 2020 17:34:27 +0300 Subject: [PATCH 08/13] feature/rule-6.1.7-implicit-backing-property(#443) ### What's done: * Fixed bugs --- .../rules/ImplicitBackingPropertyRule.kt | 6 ++++-- .../chapter6/ImplicitBackingPropertyWarnTest.kt | 17 +++++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt index 28a4611050..7d1645ca21 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt @@ -3,6 +3,7 @@ package org.cqfn.diktat.ruleset.rules import com.pinterest.ktlint.core.Rule import com.pinterest.ktlint.core.ast.ElementType.BLOCK import com.pinterest.ktlint.core.ast.ElementType.CLASS_BODY +import com.pinterest.ktlint.core.ast.ElementType.DOT_QUALIFIED_EXPRESSION import com.pinterest.ktlint.core.ast.ElementType.FILE import com.pinterest.ktlint.core.ast.ElementType.GET_KEYWORD import com.pinterest.ktlint.core.ast.ElementType.IDENTIFIER @@ -56,7 +57,7 @@ class ImplicitBackingPropertyRule(private val configRules: List) : } private fun validateAccessors(node: ASTNode, propsWithBackSymbol: List) { - val accessors = node.findAllNodesWithSpecificType(PROPERTY_ACCESSOR).filter { it.hasChildOfType(BLOCK) } // exclude inline get + val accessors = node.findAllNodesWithSpecificType(PROPERTY_ACCESSOR).filter { it.hasChildOfType(BLOCK) } // exclude get with expression body accessors.filter { it.hasChildOfType(GET_KEYWORD) }.forEach { handleGetAccessors(it, node, propsWithBackSymbol) } accessors.filter { it.hasChildOfType(SET_KEYWORD) }.forEach { handleSetAccessors(it, node, propsWithBackSymbol) } @@ -65,7 +66,8 @@ class ImplicitBackingPropertyRule(private val configRules: List) : private fun handleGetAccessors(accessor: ASTNode, node: ASTNode, propsWithBackSymbol: List) { val refExprs = accessor .findAllNodesWithSpecificType(RETURN) - .flatMap { _return -> _return.findAllNodesWithSpecificType(REFERENCE_EXPRESSION) } + .filterNot { it.hasChildOfType(DOT_QUALIFIED_EXPRESSION) } + .flatMap { it.findAllNodesWithSpecificType(REFERENCE_EXPRESSION) } // If refExprs is empty then we assume that it returns some constant if (refExprs.isNotEmpty()) { diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/ImplicitBackingPropertyWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/ImplicitBackingPropertyWarnTest.kt index ae462d37ba..7a46ea173d 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/ImplicitBackingPropertyWarnTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/ImplicitBackingPropertyWarnTest.kt @@ -112,4 +112,21 @@ class ImplicitBackingPropertyWarnTest: LintTestBase(::ImplicitBackingPropertyRul """.trimMargin() ) } + + @Test + @Tag(NO_CORRESPONDING_PROPERTY) + fun `should not trigger on property with chain call return`() { + lintMethod( + """ + |class Some(val a: Int = 5) { + | val table:Int + | get() { + | val some = listOf(1,2,3) + | return some.filter { it -> it == 3}.first() + | } + | set(value) { field = value } + |} + """.trimMargin() + ) + } } From e5dccb484e70010b606118af11d046622afa5e9f Mon Sep 17 00:00:00 2001 From: soWhoAmI Date: Mon, 23 Nov 2020 11:14:11 +0300 Subject: [PATCH 09/13] feature/rule-6.1.7-implicit-backing-property(#443) ### What's done: * Fixed bugs --- .../src/main/kotlin/generated/WarningNames.kt | 16 ++++++++-------- .../ruleset/rules/DiktatRuleSetProvider.kt | 1 - info/available-rules.md | 1 + 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/diktat-rules/src/main/kotlin/generated/WarningNames.kt b/diktat-rules/src/main/kotlin/generated/WarningNames.kt index 46a43f0891..ac6feb66db 100644 --- a/diktat-rules/src/main/kotlin/generated/WarningNames.kt +++ b/diktat-rules/src/main/kotlin/generated/WarningNames.kt @@ -35,14 +35,14 @@ public object WarningNames { public const val GENERIC_NAME: String = "GENERIC_NAME" + public const val BACKTICKS_PROHIBITED: String = "BACKTICKS_PROHIBITED" + public const val FUNCTION_NAME_INCORRECT_CASE: String = "FUNCTION_NAME_INCORRECT_CASE" public const val FUNCTION_BOOLEAN_PREFIX: String = "FUNCTION_BOOLEAN_PREFIX" public const val FILE_NAME_INCORRECT: String = "FILE_NAME_INCORRECT" - public const val FILE_NAME_MATCH_CLASS: String = "FILE_NAME_MATCH_CLASS" - public const val EXCEPTION_SUFFIX: String = "EXCEPTION_SUFFIX" public const val CONFUSING_IDENTIFIER_NAMING: String = "CONFUSING_IDENTIFIER_NAMING" @@ -137,8 +137,6 @@ public object WarningNames { public const val LONG_LINE: String = "LONG_LINE" - public const val BACKTICKS_PROHIBITED: String = "BACKTICKS_PROHIBITED" - public const val REDUNDANT_SEMICOLON: String = "REDUNDANT_SEMICOLON" public const val WRONG_NEWLINES: String = "WRONG_NEWLINES" @@ -165,6 +163,12 @@ public object WarningNames { public const val LOCAL_VARIABLE_EARLY_DECLARATION: String = "LOCAL_VARIABLE_EARLY_DECLARATION" + public const val STRING_TEMPLATE_CURLY_BRACES: String = "STRING_TEMPLATE_CURLY_BRACES" + + public const val STRING_TEMPLATE_QUOTES: String = "STRING_TEMPLATE_QUOTES" + + public const val FILE_NAME_MATCH_CLASS: String = "FILE_NAME_MATCH_CLASS" + public const val NULLABLE_PROPERTY_TYPE: String = "NULLABLE_PROPERTY_TYPE" public const val TYPE_ALIAS: String = "TYPE_ALIAS" @@ -176,10 +180,6 @@ public object WarningNames { public const val GENERIC_VARIABLE_WRONG_DECLARATION: String = "GENERIC_VARIABLE_WRONG_DECLARATION" - public const val STRING_TEMPLATE_CURLY_BRACES: String = "STRING_TEMPLATE_CURLY_BRACES" - - public const val STRING_TEMPLATE_QUOTES: String = "STRING_TEMPLATE_QUOTES" - public const val FLOAT_IN_ACCURATE_CALCULATIONS: String = "FLOAT_IN_ACCURATE_CALCULATIONS" public const val AVOID_NULL_CHECKS: String = "AVOID_NULL_CHECKS" diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt index 9ceb12225e..5657006a6d 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt @@ -66,7 +66,6 @@ class DiktatRuleSetProvider(private val diktatConfigFile: String = "diktat-analy ::IdentifierNaming, // code structure ::UselessSupertype, - ::LocalVariablesRule, ::ClassLikeStructuresOrderRule, ::WhenMustHaveElseRule, ::BracesInConditionalsAndLoopsRule, diff --git a/info/available-rules.md b/info/available-rules.md index 88e042b50f..260e213a58 100644 --- a/info/available-rules.md +++ b/info/available-rules.md @@ -96,6 +96,7 @@ | 6 | 6.1.1 | SINGLE_CONSTRUCTOR_SHOULD_BE_PRIMARY | Check: warns if there is only one secondary constructor in a class
Fix: converts it to a primary constructor | yes | no | Support more complicated logic of constructor conversion | | 6 | 6.1.2 | USE_DATA_CLASS | Check: if class can be made as data class | no | - | yes | | 6 | 6.1.4 | MULTIPLE_INIT_BLOCKS | Checks that classes have only one init block | yes | no | - | +| 6 | 6.1.5 | USELESS_SUPERTYPE | Check: if override function can be removed | yes| - | | | 6 | 6.1.6 | CLASS_SHOULD_NOT_BE_ABSTRACT | Checks: if abstract class has any abstract method. If not, warns that class should not be abstract
Fix: deletes abstract modifier | yes | - | - | | 6 | 6.1.7 | NO_CORRESPONDING_PROPERTY | Checks: if in case of using "implicit backing property" scheme, the name of real and back property are the same | no | - | - | | 6 | 6.1.9 | WRONG_NAME_OF_VARIABLE_INSIDE_ACCESSOR | Check: used the name of a variable in the custom getter or setter | no | - | From 75dc0352488d3c0e71f7a999334879009628f468 Mon Sep 17 00:00:00 2001 From: soWhoAmI Date: Mon, 23 Nov 2020 13:22:03 +0300 Subject: [PATCH 10/13] feature/rule-6.1.7-implicit-backing-property(#443) ### What's done: * Fixed bugs --- diktat-analysis.yml | 1 - .../rules/ImplicitBackingPropertyRule.kt | 22 ++++++++++++++----- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/diktat-analysis.yml b/diktat-analysis.yml index ec29b2effa..46bdbb7e4b 100644 --- a/diktat-analysis.yml +++ b/diktat-analysis.yml @@ -251,7 +251,6 @@ # Checks that function use default values, instead overloading - name: WRONG_OVERLOADING_FUNCTION_ARGUMENTS enabled: true - configuration: {} # Checks that property in constructor doesn't contains comment - name: KDOC_NO_CONSTRUCTOR_PROPERTY enabled: true diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt index 7d1645ca21..819d908015 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt @@ -22,6 +22,7 @@ import org.cqfn.diktat.ruleset.utils.hasAnyChildOfTypes import org.cqfn.diktat.ruleset.utils.hasChildOfType import org.cqfn.diktat.ruleset.utils.prettyPrint import org.jetbrains.kotlin.com.intellij.lang.ASTNode +import org.jetbrains.kotlin.psi.KtProperty /** * This rule checks if there is a backing property for field with property accessors, in case they don't use field keyword @@ -69,30 +70,39 @@ class ImplicitBackingPropertyRule(private val configRules: List) : .filterNot { it.hasChildOfType(DOT_QUALIFIED_EXPRESSION) } .flatMap { it.findAllNodesWithSpecificType(REFERENCE_EXPRESSION) } + val localProps = accessor + .findAllNodesWithSpecificType(PROPERTY) + .map { (it.psi as KtProperty).name!! } // If refExprs is empty then we assume that it returns some constant if (refExprs.isNotEmpty()) { - handleReferenceExpressions(node, refExprs, propsWithBackSymbol) + handleReferenceExpressions(node, refExprs, propsWithBackSymbol, localProps) } } private fun handleSetAccessors(accessor: ASTNode, node: ASTNode, propsWithBackSymbol: List) { val refExprs = accessor.findAllNodesWithSpecificType(REFERENCE_EXPRESSION) + + val localProps = accessor + .findAllNodesWithSpecificType(PROPERTY) + .map { (it.psi as KtProperty).name!! } + if (refExprs.isNotEmpty()) { - handleReferenceExpressions(node, refExprs, propsWithBackSymbol) + handleReferenceExpressions(node, refExprs, propsWithBackSymbol, localProps) } } @Suppress("UnsafeCallOnNullableType") - private fun handleReferenceExpressions(node:ASTNode, + private fun handleReferenceExpressions(node: ASTNode, expressions: List, - backingPropertiesNames: List) { - if (expressions.none { backingPropertiesNames.contains(it.text) || it.text == "field" }) { + backingPropertiesNames: List, + localProperties: List) { + if (expressions.none { backingPropertiesNames.contains(it.text) || it.text == "field" || localProperties.contains(it.text) }) { raiseWarning(node, node.getFirstChildWithType(IDENTIFIER)!!.text) } } - private fun raiseWarning(node:ASTNode, propName: String) { + private fun raiseWarning(node: ASTNode, propName: String) { NO_CORRESPONDING_PROPERTY.warn(configRules, emitWarn, isFixMode, "$propName has no corresponding property with name _$propName", node.startOffset, node) } From a361d2dcd86b4bbd029d5becc627bd2e11b1ceee Mon Sep 17 00:00:00 2001 From: soWhoAmI Date: Mon, 23 Nov 2020 13:54:31 +0300 Subject: [PATCH 11/13] feature/rule-6.1.7-implicit-backing-property(#443) ### What's done: * Fixed bugs --- .../cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt index 819d908015..da59b757ec 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt @@ -64,6 +64,7 @@ class ImplicitBackingPropertyRule(private val configRules: List) : accessors.filter { it.hasChildOfType(SET_KEYWORD) }.forEach { handleSetAccessors(it, node, propsWithBackSymbol) } } + @Suppress("UnsafeCallOnNullableType") private fun handleGetAccessors(accessor: ASTNode, node: ASTNode, propsWithBackSymbol: List) { val refExprs = accessor .findAllNodesWithSpecificType(RETURN) @@ -79,6 +80,7 @@ class ImplicitBackingPropertyRule(private val configRules: List) : } } + @Suppress("UnsafeCallOnNullableType") private fun handleSetAccessors(accessor: ASTNode, node: ASTNode, propsWithBackSymbol: List) { val refExprs = accessor.findAllNodesWithSpecificType(REFERENCE_EXPRESSION) From beaa8f2429c5f824bc52fffbdcbb37fed2741df9 Mon Sep 17 00:00:00 2001 From: soWhoAmI Date: Mon, 23 Nov 2020 16:15:07 +0300 Subject: [PATCH 12/13] feature/rule-6.1.7-implicit-backing-property(#443) ### What's done: * Fixed bugs --- .../ruleset/rules/ImplicitBackingPropertyRule.kt | 15 +++++++-------- .../chapter6/ImplicitBackingPropertyWarnTest.kt | 16 ++++++++++++++++ info/available-rules.md | 2 +- 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt index da59b757ec..7ce899b7ed 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt @@ -84,13 +84,9 @@ class ImplicitBackingPropertyRule(private val configRules: List) : private fun handleSetAccessors(accessor: ASTNode, node: ASTNode, propsWithBackSymbol: List) { val refExprs = accessor.findAllNodesWithSpecificType(REFERENCE_EXPRESSION) - - val localProps = accessor - .findAllNodesWithSpecificType(PROPERTY) - .map { (it.psi as KtProperty).name!! } - + // In set we don't check for local properties. At least one reference expression should contain field or _prop if (refExprs.isNotEmpty()) { - handleReferenceExpressions(node, refExprs, propsWithBackSymbol, localProps) + handleReferenceExpressions(node, refExprs, propsWithBackSymbol, null) } } @@ -98,8 +94,11 @@ class ImplicitBackingPropertyRule(private val configRules: List) : private fun handleReferenceExpressions(node: ASTNode, expressions: List, backingPropertiesNames: List, - localProperties: List) { - if (expressions.none { backingPropertiesNames.contains(it.text) || it.text == "field" || localProperties.contains(it.text) }) { + localProperties: List?) { + if (expressions.none { + (backingPropertiesNames.contains(it.text) || it.text == "field") && + (localProperties == null || localProperties.contains(it.text)) + }) { raiseWarning(node, node.getFirstChildWithType(IDENTIFIER)!!.text) } } diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/ImplicitBackingPropertyWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/ImplicitBackingPropertyWarnTest.kt index 7a46ea173d..ddc2850b27 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/ImplicitBackingPropertyWarnTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/ImplicitBackingPropertyWarnTest.kt @@ -129,4 +129,20 @@ class ImplicitBackingPropertyWarnTest: LintTestBase(::ImplicitBackingPropertyRul """.trimMargin() ) } + + @Test + @Tag(NO_CORRESPONDING_PROPERTY) + fun `should not trigger set accessor`() { + lintMethod( + """ + |class Some(val a: Int = 5) { + | val foo + | set(value) { + | if(isDelegate) log.debug(value) + | field = value + | } + |} + """.trimMargin() + ) + } } diff --git a/info/available-rules.md b/info/available-rules.md index 260e213a58..38e740dd44 100644 --- a/info/available-rules.md +++ b/info/available-rules.md @@ -98,7 +98,7 @@ | 6 | 6.1.4 | MULTIPLE_INIT_BLOCKS | Checks that classes have only one init block | yes | no | - | | 6 | 6.1.5 | USELESS_SUPERTYPE | Check: if override function can be removed | yes| - | | | 6 | 6.1.6 | CLASS_SHOULD_NOT_BE_ABSTRACT | Checks: if abstract class has any abstract method. If not, warns that class should not be abstract
Fix: deletes abstract modifier | yes | - | - | -| 6 | 6.1.7 | NO_CORRESPONDING_PROPERTY | Checks: if in case of using "implicit backing property" scheme, the name of real and back property are the same | no | - | - | +| 6 | 6.1.7 | NO_CORRESPONDING_PROPERTY | Checks: if in case of using "backing property" scheme, the name of real and back property are the same | no | - | - | | 6 | 6.1.9 | WRONG_NAME_OF_VARIABLE_INSIDE_ACCESSOR | Check: used the name of a variable in the custom getter or setter | no | - | | 6 | 6.1.10 | TRIVIAL_ACCESSORS_ARE_NOT_RECOMMENDED | Check: if there are any trivial getters or setters
Fix: Delete trivial getter or setter | yes | - | - | | 6 | 6.1.8 | CUSTOM_GETTERS_SETTERS | Check: Inspection that checks that no custom getters and setters are used for properties | no | - | - | From 3b65bf5d7e2437e1bd9b5e360396780276093123 Mon Sep 17 00:00:00 2001 From: soWhoAmI Date: Mon, 23 Nov 2020 17:22:13 +0300 Subject: [PATCH 13/13] feature/rule-6.1.7-implicit-backing-property ### What's done: * Fixed bugs --- .../rules/ImplicitBackingPropertyRule.kt | 3 +-- .../ImplicitBackingPropertyWarnTest.kt | 19 ++++++++++++++++++- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt index 7ce899b7ed..2677036941 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt @@ -96,8 +96,7 @@ class ImplicitBackingPropertyRule(private val configRules: List) : backingPropertiesNames: List, localProperties: List?) { if (expressions.none { - (backingPropertiesNames.contains(it.text) || it.text == "field") && - (localProperties == null || localProperties.contains(it.text)) + backingPropertiesNames.contains(it.text) || it.text == "field" || localProperties?.contains(it.text) == true }) { raiseWarning(node, node.getFirstChildWithType(IDENTIFIER)!!.text) } diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/ImplicitBackingPropertyWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/ImplicitBackingPropertyWarnTest.kt index ddc2850b27..54cc0cf0b2 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/ImplicitBackingPropertyWarnTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/ImplicitBackingPropertyWarnTest.kt @@ -26,7 +26,7 @@ class ImplicitBackingPropertyWarnTest: LintTestBase(::ImplicitBackingPropertyRul | } | return _table ?: throw AssertionError("Set to null by another thread") | } - | set(value) {field = value} + | set(value) { field = value } |} """.trimMargin() ) @@ -145,4 +145,21 @@ class ImplicitBackingPropertyWarnTest: LintTestBase(::ImplicitBackingPropertyRul """.trimMargin() ) } + + @Test + @Tag(NO_CORRESPONDING_PROPERTY) + fun `should trigger set accessor`() { + lintMethod( + """ + |class Some(val a: Int = 5) { + | val foo + | set(value) { + | if(isDelegate) log.debug(value) + | a = value + | } + |} + """.trimMargin(), + LintError(2,4 ,ruleId, "${Warnings.NO_CORRESPONDING_PROPERTY.warnText()} foo has no corresponding property with name _foo") + ) + } }