Rule 6.1.7 implicit backing property(#443)

diktat-analysis.yml

@@ -339,5 +339,11 @@
   configuration: {}
 # Checks that there are abstract functions in abstract class
 - name: CLASS_SHOULD_NOT_BE_ABSTRACT
+  enabled: true
+  configuration: {}
+# 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: {}

diktat-rules/src/main/kotlin/generated/WarningNames.kt

@@ -200,4 +200,6 @@ public object WarningNames {
     public const val MULTIPLE_INIT_BLOCKS: String = "MULTIPLE_INIT_BLOCKS"
 
     public const val CLASS_SHOULD_NOT_BE_ABSTRACT: String = "CLASS_SHOULD_NOT_BE_ABSTRACT"
+
+    public const val NO_CORRESPONDING_PROPERTY: String = "NO_CORRESPONDING_PROPERTY"
 }

diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt

@@ -121,6 +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, "backing property should have the same name, but there is no corresponding property")
     ;
 
     /**

diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt

@@ -62,6 +62,7 @@ class DiktatRuleSetProvider(private val diktatConfigFile: String = "diktat-analy
                 ::PackageNaming,
                 ::IdentifierNaming,
                 // code structure
+                ::LocalVariablesRule,
                 ::ClassLikeStructuresOrderRule,
                 ::WhenMustHaveElseRule,
                 ::BracesInConditionalsAndLoopsRule,
@@ -71,9 +72,9 @@ class DiktatRuleSetProvider(private val diktatConfigFile: String = "diktat-analy
                 ::SingleLineStatementsRule,
                 ::MultipleModifiersSequence,
                 // other rules
+                ::ImplicitBackingPropertyRule,
                 ::StringTemplateFormatRule,
                 ::DataClassesRule,
-                ::LocalVariablesRule,
                 ::SmartCastRule,
                 ::PropertyAccessorFields,
                 ::AbstractClassesRule,

diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImplicitBackingPropertyRule.kt

@@ -0,0 +1,98 @@
+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.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
+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.cqfn.diktat.ruleset.utils.prettyPrint
+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<RulesConfig>) : 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 == CLASS_BODY) {
+            findAllProperties(node)
+        }
+    }
+
+    @Suppress("UnsafeCallOnNullableType")
+    private fun findAllProperties(node: ASTNode) {
+        val properties = node.getChildren(null).filter { it.elementType == PROPERTY }
+
+        val propsWithBackSymbol = properties
+                .filter { it.getFirstChildWithType(IDENTIFIER)!!.text.startsWith("_") }
+                .map {
+                    it.getFirstChildWithType(IDENTIFIER)!!.text
+                }
+
+        properties.filter { it.hasAnyChildOfTypes(PROPERTY_ACCESSOR) }.forEach {
+            validateAccessors(it, propsWithBackSymbol)
+        }
+    }
+
+    private fun validateAccessors(node: ASTNode, propsWithBackSymbol: List<String>) {
+        val accessors = node.findAllNodesWithSpecificType(PROPERTY_ACCESSOR).filter { it.hasChildOfType(BLOCK) } // exclude inline get
+
+        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<String>) {
+        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<String>) {
+        val refExprs = accessor.findAllNodesWithSpecificType(REFERENCE_EXPRESSION)
+
+        if (refExprs.isNotEmpty()) {
+            handleReferenceExpressions(node, refExprs, propsWithBackSymbol)
+        }
+    }
+
+    @Suppress("UnsafeCallOnNullableType")
+    private fun handleReferenceExpressions(node:ASTNode,
+                                           expressions: List<ASTNode>,
+                                           backingPropertiesNames: List<String>) {
+        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)
+    }
+
+}

diktat-rules/src/main/resources/diktat-analysis-huawei.yml

@@ -338,5 +338,11 @@
   configuration: {}
 # Checks that there are abstract functions in abstract class
 - name: CLASS_SHOULD_NOT_BE_ABSTRACT
+  enabled: true
+  configuration: {}
+# 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: {}

diktat-rules/src/main/resources/diktat-analysis.yml

@@ -340,5 +340,11 @@
   configuration: { }
 # Checks that there are abstract functions in abstract class
 - name: CLASS_SHOULD_NOT_BE_ABSTRACT
+  enabled: true
+  configuration: {}
+# 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: {}

diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/ImplicitBackingPropertyWarnTest.kt

@@ -0,0 +1,115 @@
+package org.cqfn.diktat.ruleset.chapter6
+
+import com.pinterest.ktlint.core.LintError
+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
+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(NO_CORRESPONDING_PROPERTY)
+    fun `not trigger on backing property`() {
+        lintMethod(
+                """
+                    |class Some(val a: Int = 5) {
+                    |   private var _table: Map<String, Int>? = null
+                    |   val table:Map<String, Int>
+                    |       get() {
+                    |           if (_table == null) {
+                    |               _table = HashMap()
+                    |           }
+                    |           return _table ?: throw AssertionError("Set to null by another thread")
+                    |       }
+                    |       set(value) {field = value}
+                    |}
+                """.trimMargin()
+        )
+    }
+
+    @Test
+    @Tag(NO_CORRESPONDING_PROPERTY)
+    fun `trigger on backing property`() {
+        lintMethod(
+                """
+                    |class Some(val a: Int = 5) {
+                    |   private var a: Map<String, Int>? = null
+                    |   val table:Map<String, Int>
+                    |       get() {
+                    |           if (a == null) {
+                    |               a = HashMap()
+                    |           }
+                    |           return a ?: throw AssertionError("Set to null by another thread")
+                    |       }
+                    |       set(value) { field = value }
+                    |}
+                """.trimMargin(),
+                LintError(3,4,ruleId, "${Warnings.NO_CORRESPONDING_PROPERTY.warnText()} table has no corresponding property with name _table")
+        )
+    }
+
+    @Test
+    @Tag(NO_CORRESPONDING_PROPERTY)
+    fun `don't trigger on regular backing property`() {
+        lintMethod(
+                """
+                    |class Some(val a: Int = 5) {
+                    |   private var _a: Map<String, Int>? = null
+                    |   private val _some:Int? = null
+                    |}
+                """.trimMargin()
+        )
+    }
+
+    @Test
+    @Tag(NO_CORRESPONDING_PROPERTY)
+    fun `don't trigger on regular property`() {
+        lintMethod(
+                """
+                    |class Some(val a: Int = 5) {
+                    |   private var a: Map<String, Int>? = null
+                    |   private val some:Int? = null
+                    |   private val _prop: String? = null
+                    |}
+                """.trimMargin()
+        )
+    }
+
+    @Test
+    @Tag(NO_CORRESPONDING_PROPERTY)
+    fun `should not trigger if property has field in accessor`() {
+        lintMethod(
+                """
+                    |class Some(val a: Int = 5) {
+                    |   val table:Map<String, Int>
+                    |       set(value) { field = value }
+                    |   val _table: Map<String,Int>? = null
+                    |
+                    |   val some: Int
+                    |       get() = 3
+                    |}
+                """.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()
+        )
+    }
+}

info/available-rules.md

@@ -94,4 +94,5 @@
 |    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.6 | CLASS_SHOULD_NOT_BE_ABSTRACT           | Checks: if abstract class has any abstract method. If not, warns that class should not be abstract<br>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 | - |