Rule 6.1.3 Do not use the primary constructor if it is empty and has …

…no sense (#528) * Rule 6.1.3 ### What's done: Made rule, added tests and documentations
saveourtool · Nov 23, 2020 · d8651d2 · d8651d2
1 parent 0dd9f90
commit d8651d2
Show file tree

Hide file tree

Showing 12 changed files with 163 additions and 0 deletions.
diff --git a/diktat-analysis.yml b/diktat-analysis.yml
@@ -297,4 +297,7 @@
   enabled: true
 # Checks if extension function with the same signature don't have related classes
 - name: EXTENSION_FUNCTION_SAME_SIGNATURE
+  enabled: true
+# Checks if there is empty primary constructor
+- name: EMPTY_PRIMARY_CONSTRUCTOR
   enabled: true
diff --git a/diktat-rules/src/main/kotlin/generated/WarningNames.kt b/diktat-rules/src/main/kotlin/generated/WarningNames.kt
@@ -219,4 +219,6 @@ public object WarningNames {
             "TRIVIAL_ACCESSORS_ARE_NOT_RECOMMENDED"
 
     public const val EXTENSION_FUNCTION_SAME_SIGNATURE: String = "EXTENSION_FUNCTION_SAME_SIGNATURE"
+
+    public const val EMPTY_PRIMARY_CONSTRUCTOR: String = "EMPTY_PRIMARY_CONSTRUCTOR"
 }
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
@@ -130,6 +130,7 @@ enum class Warnings(val canBeAutoCorrected: Boolean, val ruleId: String, private
     USELESS_SUPERTYPE(true, "6.1.5", "unnecessary supertype specification"),
     TRIVIAL_ACCESSORS_ARE_NOT_RECOMMENDED(true, "6.1.10", "trivial property accessors are not recommended"),
     EXTENSION_FUNCTION_SAME_SIGNATURE(false, "6.2.2", "extension functions should not have same signature if their receiver classes are related"),
+    EMPTY_PRIMARY_CONSTRUCTOR(true,"6.1.3", "avoid empty primary constructor"),
     ;
 
     /**

diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/AvoidEmptyPrimaryConstructor.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/AvoidEmptyPrimaryConstructor.kt
@@ -0,0 +1,35 @@
+package org.cqfn.diktat.ruleset.rules
+
+import com.pinterest.ktlint.core.Rule
+import com.pinterest.ktlint.core.ast.ElementType.CLASS
+import org.cqfn.diktat.common.config.rules.RulesConfig
+import org.cqfn.diktat.ruleset.constants.Warnings.EMPTY_PRIMARY_CONSTRUCTOR
+import org.jetbrains.kotlin.com.intellij.lang.ASTNode
+import org.jetbrains.kotlin.psi.KtClass
+
+class AvoidEmptyPrimaryConstructor(private val configRules: List<RulesConfig>) : Rule("avoid-empty-primary-constructor") {
+
+
+    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 == CLASS)
+            checkCLass(node.psi as KtClass)
+    }
+
+    @Suppress("UnsafeCallOnNullableType")
+    private fun checkCLass(ktClass: KtClass) {
+        if(ktClass.primaryConstructor?.valueParameters?.isNotEmpty() != false || ktClass.primaryConstructorModifierList != null)
+            return
+        EMPTY_PRIMARY_CONSTRUCTOR.warnAndFix(configRules, emitWarn, isFixMode, ktClass.nameIdentifier!!.text,
+                ktClass.node.startOffset, ktClass.node) {
+            ktClass.node.removeChild(ktClass.primaryConstructor!!.node)
+        }
+    }
+}
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
@@ -71,6 +71,7 @@ class DiktatRuleSetProvider(private val diktatConfigFile: String = "diktat-analy
                 ::BracesInConditionalsAndLoopsRule,
                 ::BlockStructureBraces,
                 ::EmptyBlock,
+                ::AvoidEmptyPrimaryConstructor,
                 ::EnumsSeparated,
                 ::SingleLineStatementsRule,
                 ::MultipleModifiersSequence,

diff --git a/diktat-rules/src/main/resources/diktat-analysis-huawei.yml b/diktat-rules/src/main/resources/diktat-analysis-huawei.yml
@@ -296,3 +296,6 @@
 # Checks if extension function with the same signature don't have related classes
 - name: EXTENSION_FUNCTION_SAME_SIGNATURE
   enabled: true
+# Checks if there is empty primary constructor
+- name: EMPTY_PRIMARY_CONSTRUCTOR
+  enabled: true
diff --git a/diktat-rules/src/main/resources/diktat-analysis.yml b/diktat-rules/src/main/resources/diktat-analysis.yml
@@ -298,3 +298,6 @@
 # Checks if extension function with the same signature don't have related classes
 - name: EXTENSION_FUNCTION_SAME_SIGNATURE
   enabled: true
+# Checks if there is empty primary constructor
+- name: EMPTY_PRIMARY_CONSTRUCTOR
+  enabled: true
diff --git a/...-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/EmptyPrimaryConstructorFixTest.kt b/...-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/EmptyPrimaryConstructorFixTest.kt
@@ -0,0 +1,16 @@
+package org.cqfn.diktat.ruleset.chapter6
+
+import generated.WarningNames
+import org.cqfn.diktat.ruleset.rules.AvoidEmptyPrimaryConstructor
+import org.cqfn.diktat.util.FixTestBase
+import org.junit.jupiter.api.Tag
+import org.junit.jupiter.api.Test
+
+class EmptyPrimaryConstructorFixTest: FixTestBase("test/chapter6/primary_constructor", ::AvoidEmptyPrimaryConstructor) {
+
+    @Test
+    @Tag(WarningNames.EMPTY_PRIMARY_CONSTRUCTOR)
+    fun `should remove empty primary constructor`() {
+        fixAndCompare("EmptyPCExpected.kt", "EmptyPCTest.kt")
+    }
+}
diff --git a/...rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/EmptyPrimaryConstructorWarnTest.kt b/...rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/EmptyPrimaryConstructorWarnTest.kt
@@ -0,0 +1,62 @@
+package org.cqfn.diktat.ruleset.chapter6
+
+import com.pinterest.ktlint.core.LintError
+import generated.WarningNames
+import org.cqfn.diktat.ruleset.constants.Warnings.EMPTY_PRIMARY_CONSTRUCTOR
+import org.cqfn.diktat.ruleset.rules.AvoidEmptyPrimaryConstructor
+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 EmptyPrimaryConstructorWarnTest: LintTestBase(::AvoidEmptyPrimaryConstructor) {
+    private val ruleId = "$DIKTAT_RULE_SET_ID:avoid-empty-primary-constructor"
+
+    @Test
+    @Tag(WarningNames.EMPTY_PRIMARY_CONSTRUCTOR)
+    fun `simple classes with empty primary constructor`() {
+        lintMethod(
+                """
+                    |class Some() {
+                    |   val a = 10
+                    |   constructor(a: String): this() {
+                    |       this.a = a
+                    |   }
+                    |}
+                    |
+                    |class Some1() {
+                    |   val a = 10
+                    |   companion object {}
+                    |}
+                    |
+                    |class Some2 {
+                    |   val a = 10
+                    |   constructor(a: String): this() {
+                    |       this.a = a
+                    |   }
+                    |}
+                    |
+                    |class Some3 private constructor () {
+                    |
+                    |}
+                """.trimMargin(),
+                LintError(1,1,ruleId, "${EMPTY_PRIMARY_CONSTRUCTOR.warnText()} Some", true),
+                LintError(8,1,ruleId, "${EMPTY_PRIMARY_CONSTRUCTOR.warnText()} Some1", true)
+        )
+    }
+
+    @Test
+    @Tag(WarningNames.EMPTY_PRIMARY_CONSTRUCTOR)
+    fun `correct example with empty primary constructor and modifiers`() {
+        lintMethod(
+                """
+                    |class Some1 private constructor () {
+                    |
+                    |}
+                    |
+                    |class Some2 @Inject constructor() {
+                    |}
+                """.trimMargin()
+        )
+    }
+}
diff --git a/diktat-rules/src/test/resources/test/chapter6/primary_constructor/EmptyPCExpected.kt b/diktat-rules/src/test/resources/test/chapter6/primary_constructor/EmptyPCExpected.kt
@@ -0,0 +1,18 @@
+package test.chapter6.primary_constructor
+
+class Test {
+    var a: Int = 0
+    var b: Int = 0
+}
+
+class Test {
+    var a  = "Property"
+
+    init {
+        println("some init")
+    }
+
+    constructor(a: String): this() {
+        this.a = a
+    }
+}
diff --git a/diktat-rules/src/test/resources/test/chapter6/primary_constructor/EmptyPCTest.kt b/diktat-rules/src/test/resources/test/chapter6/primary_constructor/EmptyPCTest.kt
@@ -0,0 +1,18 @@
+package test.chapter6.primary_constructor
+
+class Test() {
+    var a: Int = 0
+    var b: Int = 0
+}
+
+class Test() {
+    var a  = "Property"
+
+    init {
+        println("some init")
+    }
+
+    constructor(a: String): this() {
+        this.a = a
+    }
+}
diff --git a/info/available-rules.md b/info/available-rules.md
@@ -95,6 +95,7 @@
 |    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.3 | EMPTY_PRIMARY_CONSTRUCTOR              | Check: if there is empty primary constructor                                                                                                                                 | yes| - | 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<br>Fix: deletes abstract modifier                                         | yes | - | - |