Adding rule 6.1.8 regarding getters and setters

diktat-analysis.yml

@@ -353,4 +353,12 @@
 # Checks if there are any trivial getters or setters
 - name: TRIVIAL_ACCESSORS_ARE_NOT_RECOMMENDED
   enabled: true
-  configuration: {}
+  configuration: {}
+# Checks that no custom getters and setters are used for properties. It is a more wide rule than TRIVIAL_ACCESSORS_ARE_NOT_RECOMMENDED
+# Kotlin compiler automatically generates `get` and `set` methods for properties and also lets the possibility to override it.
+# But in all cases it is very confusing when `get` and `set` are overridden for a developer who uses this particular class.
+# Developer expects to get the value of the property, but receives some unknown value and some extra side effect hidden by the custom getter/setter.
+# Use extra functions for it instead.
+- name: CUSTOM_GETTERS_SETTERS
+  enabled: true
+  configuration: { }

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

@@ -209,4 +209,6 @@ public object WarningNames {
 
     public const val TRIVIAL_ACCESSORS_ARE_NOT_RECOMMENDED: String =
             "TRIVIAL_ACCESSORS_ARE_NOT_RECOMMENDED"
+
+    public const val CUSTOM_GETTERS_SETTERS: String = "CUSTOM_GETTERS_SETTERS"
 }

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

@@ -125,7 +125,8 @@ 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"),
-    TRIVIAL_ACCESSORS_ARE_NOT_RECOMMENDED(true, "trivial property accessors are not recommended")
+    TRIVIAL_ACCESSORS_ARE_NOT_RECOMMENDED(true, "trivial property accessors are not recommended"),
+    CUSTOM_GETTERS_SETTERS(false, "Custom getters and setters are not recommended, use class methods instead"),
     ;
 
     /**

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

@@ -0,0 +1,42 @@
+package org.cqfn.diktat.ruleset.rules
+
+import com.pinterest.ktlint.core.Rule
+import com.pinterest.ktlint.core.ast.ElementType.GET_KEYWORD
+import com.pinterest.ktlint.core.ast.ElementType.PROPERTY_ACCESSOR
+import com.pinterest.ktlint.core.ast.ElementType.SET_KEYWORD
+import org.cqfn.diktat.common.config.rules.RulesConfig
+import org.cqfn.diktat.ruleset.constants.Warnings
+import org.cqfn.diktat.ruleset.utils.*
+import org.jetbrains.kotlin.com.intellij.lang.ASTNode
+
+/**
+ * Inspection that checks that no custom getters and setters are used for properties.
+ */
+class CustomGetterSetterRule(private val configRules: List<RulesConfig>) : Rule("custom-getter-setter") {
+    private var isFixMode: Boolean = false
+    private lateinit var emitWarn: ((offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit)
+
+    override fun visit(node: ASTNode,
+                       autoCorrect: Boolean,
+                       emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit) {
+        emitWarn = emit
+        isFixMode = autoCorrect
+
+        if(node.elementType == PROPERTY_ACCESSOR) {
+            checkForCustomGetersSetters(node)
+        }
+    }
+
+    private fun checkForCustomGetersSetters(node: ASTNode) {
+        val setter = node.getFirstChildWithType(SET_KEYWORD)
+        val getter = node.getFirstChildWithType(GET_KEYWORD)
+
+        setter?.let {
+            Warnings.CUSTOM_GETTERS_SETTERS.warn(configRules, emitWarn, isFixMode, setter.text, setter.startOffset, node)
+        }
+
+        getter?.let {
+            Warnings.CUSTOM_GETTERS_SETTERS.warn(configRules, emitWarn, isFixMode, getter.text, getter.startOffset, node)
+        }
+    }
+}

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

@@ -73,6 +73,7 @@ class DiktatRuleSetProvider(private val diktatConfigFile: String = "diktat-analy
                 ::SingleLineStatementsRule,
                 ::MultipleModifiersSequence,
                 ::TrivialPropertyAccessors,
+                ::CustomGetterSetterRule,
                 // other rules
                 ::StringTemplateFormatRule,
                 ::DataClassesRule,

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

@@ -352,4 +352,12 @@
 # Checks if there are any trivial getters or setters
 - name: TRIVIAL_ACCESSORS_ARE_NOT_RECOMMENDED
   enabled: true
-  configuration: {}
+  configuration: {}
+# Checks that no custom getters and setters are used for properties. It is a more wide rule than TRIVIAL_ACCESSORS_ARE_NOT_RECOMMENDED
+# Kotlin compiler automatically generates `get` and `set` methods for properties and also lets the possibility to override it.
+# But in all cases it is very confusing when `get` and `set` are overridden for a developer who uses this particular class.
+# Developer expects to get the value of the property, but receives some unknown value and some extra side effect hidden by the custom getter/setter.
+# Use extra functions for it instead.
+- name: CUSTOM_GETTERS_SETTERS
+  enabled: true
+  configuration: { }

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

@@ -354,4 +354,12 @@
 # Checks if there are any trivial getters or setters
 - name: TRIVIAL_ACCESSORS_ARE_NOT_RECOMMENDED
   enabled: true
-  configuration: {}
+  configuration: {}
+# Checks that no custom getters and setters are used for properties. It is a more wide rule than TRIVIAL_ACCESSORS_ARE_NOT_RECOMMENDED
+# Kotlin compiler automatically generates `get` and `set` methods for properties and also lets the possibility to override it.
+# But in all cases it is very confusing when `get` and `set` are overridden for a developer who uses this particular class.
+# Developer expects to get the value of the property, but receives some unknown value and some extra side effect hidden by the custom getter/setter.
+# Use extra functions for it instead.
+- name: CUSTOM_GETTERS_SETTERS
+  enabled: true
+  configuration: { }

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

@@ -0,0 +1,51 @@
+package org.cqfn.diktat.ruleset.chapter6
+
+
+import com.pinterest.ktlint.core.LintError
+import generated.WarningNames.CUSTOM_GETTERS_SETTERS
+import org.cqfn.diktat.ruleset.constants.Warnings
+import org.cqfn.diktat.ruleset.rules.CustomGetterSetterRule
+import org.cqfn.diktat.ruleset.rules.DIKTAT_RULE_SET_ID
+import org.cqfn.diktat.util.LintTestBase
+import org.junit.jupiter.api.Tag
+import org.junit.jupiter.api.Test
+
+class CustomGetterSetterWarnTest : LintTestBase(::CustomGetterSetterRule) {
+    private val ruleId = "$DIKTAT_RULE_SET_ID:custom-getter-setter"
+
+    @Test
+    @Tag(CUSTOM_GETTERS_SETTERS)
+    fun `no custom getters and setters allowed`() {
+        lintMethod(
+                """
+                    |class A {
+                    |    var size: Int = 0
+                    |        set(value) {
+                    |            println("Side effect")
+                    |            field = value
+                    |        }
+                    |        get() = this.hashCode() * 2
+                    |}
+                """.trimMargin(),
+                LintError(3, 9, ruleId, "${Warnings.CUSTOM_GETTERS_SETTERS.warnText()} set"),
+                LintError(7, 9, ruleId, "${Warnings.CUSTOM_GETTERS_SETTERS.warnText()} get"),
+        )
+    }
+
+    @Test
+    @Tag(CUSTOM_GETTERS_SETTERS)
+    fun `no custom getters allowed`() {
+        lintMethod(
+                """
+                    |class A {
+                    |  
+                    |        fun set(value) {
+                    |            println("Side effect")
+                    |        }
+                    |        
+                    |        fun get() = 47
+                    |}
+                """.trimMargin()
+        )
+    }
+}

info/available-rules.md

@@ -89,12 +89,13 @@
 |    5 |     5.1.1 | TOO_LONG_FUNCTION                      | Check: if length of function is too long                                                                                                                                     |  no | maxFunctionLength isIncludeHeader|
 |    5 |     5.1.2 | NESTED_BLOCK                           | Check if function has more nested blocks than expected                                                                                                                       |  no | maxNestedBlockQuantity
 |    5 |     5.1.3 | AVOID_NESTED_FUNCTIONS                 | Check: if there are any nested functions<br>Fix: declare function in the outer scope                                                                                         | yes | - | + |
-|    5 |     5.2.1 | LAMBDA_IS_NOT_LAST_PARAMETER         | Checks that lambda inside function parameters isn't in the end                                                                                                               | no  | - |
+|    5 |     5.2.1 | LAMBDA_IS_NOT_LAST_PARAMETER           | Checks that lambda inside function parameters isn't in the end                                                                                                               | no  | - |
 |    5 |     5.2.2 | TOO_MANY_PARAMETERS                    | Check: if function contains more parameters than allowed                                                                                                                     |  no | maxParameterListSize |
 |    5 |     5.2.3 | WRONG_OVERLOADING_FUNCTION_ARGUMENTS   | Check: function has overloading instead use default arguments                                                                                                                |  no |  -|
 |    6 |     6.1.1 | SINGLE_CONSTRUCTOR_SHOULD_BE_PRIMARY   | Check: warns if there is only one secondary constructor in a class<br>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.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.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 <br> Fix: Delete trivial getter or setter                                                                                 | yes | - | - |
+|    6 |    6.1.10 | TRIVIAL_ACCESSORS_ARE_NOT_RECOMMENDED  | Check: if there are any trivial getters or setters <br> 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 | - | - |

info/guide/guide-chapter-6.md

@@ -246,7 +246,7 @@ class A {
 ```
 
 From the callee code these methods look like an access to this property: `A().isEmpty = true` for setter and `A().isEmpty` for getter.
-But in all cases it is very confusing when `get` and `set` are overriden for a developer who uses this particular class. 
+But in all cases it is very confusing when `get` and `set` are overridden for a developer who uses this particular class. 
 Developer expects to get the value of the property, but receives some unknown value and some extra side effect hidden by the custom getter/setter. 
 Use extra functions for it instead.