Rule 6.1.4 several init blocks are redundant and generally should not…

… be used in your class (#459) ### What's done: * Implementation * Fixed typos in code style * paragraph6 -> chapter6
saveourtool · Oct 29, 2020 · fa9a4fc · fa9a4fc · 0pdd · Oct 29, 2020
1 parent 6852628
commit fa9a4fc
Show file tree

Hide file tree

Showing 23 changed files with 303 additions and 14 deletions.
diff --git a/diktat-analysis.yml b/diktat-analysis.yml
@@ -333,6 +333,10 @@
 - name: WRONG_NAME_OF_VARIABLE_INSIDE_ACCESSOR
   enabled: true
   configuration: {}
+# Checks that classes have only one init block
+- name: MULTIPLE_INIT_BLOCKS
+  enabled: true
+  configuration: {}
 # Checks that there are abstract functions in abstract class
 - name: CLASS_SHOULD_NOT_BE_ABSTRACT
   enabled: true

diff --git a/diktat-rules/src/main/kotlin/generated/WarningNames.kt b/diktat-rules/src/main/kotlin/generated/WarningNames.kt
@@ -197,5 +197,7 @@ public object WarningNames {
     public const val WRONG_NAME_OF_VARIABLE_INSIDE_ACCESSOR: String =
             "WRONG_NAME_OF_VARIABLE_INSIDE_ACCESSOR"
 
+    public const val MULTIPLE_INIT_BLOCKS: String = "MULTIPLE_INIT_BLOCKS"
+
     public const val CLASS_SHOULD_NOT_BE_ABSTRACT: String = "CLASS_SHOULD_NOT_BE_ABSTRACT"
 }
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
@@ -119,6 +119,7 @@ enum class Warnings(private val canBeAutoCorrected: Boolean, private val warn: S
     // ======== chapter 6 ========
     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"),
+    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"),
     ;
 

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
@@ -9,13 +9,15 @@ import org.cqfn.diktat.ruleset.constants.Warnings
 import org.cqfn.diktat.ruleset.rules.calculations.AccurateCalculationsRule
 import org.cqfn.diktat.ruleset.rules.classes.AbstractClassesRule
 import org.cqfn.diktat.ruleset.rules.classes.DataClassesRule
+import org.cqfn.diktat.ruleset.rules.classes.SingleInitRule
 import org.cqfn.diktat.ruleset.rules.comments.CommentsRule
 import org.cqfn.diktat.ruleset.rules.comments.HeaderCommentRule
 import org.cqfn.diktat.ruleset.rules.files.BlankLinesRule
 import org.cqfn.diktat.ruleset.rules.files.FileSize
 import org.cqfn.diktat.ruleset.rules.files.FileStructureRule
 import org.cqfn.diktat.ruleset.rules.files.IndentationRule
 import org.cqfn.diktat.ruleset.rules.files.NewlinesRule
+import org.cqfn.diktat.ruleset.rules.files.WhiteSpaceRule
 import org.cqfn.diktat.ruleset.rules.identifiers.LocalVariablesRule
 import org.cqfn.diktat.ruleset.rules.kdoc.CommentsFormatting
 import org.cqfn.diktat.ruleset.rules.kdoc.KdocComments
@@ -75,6 +77,7 @@ class DiktatRuleSetProvider(private val diktatConfigFile: String = "diktat-analy
                 ::SmartCastRule,
                 ::PropertyAccessorFields,
                 ::AbstractClassesRule,
+                ::SingleInitRule,
                 ::VariableGenericTypeDeclarationRule,
                 ::LongNumericalValuesSeparatedRule,
                 ::NestedFunctionBlock,
@@ -95,7 +98,7 @@ class DiktatRuleSetProvider(private val diktatConfigFile: String = "diktat-analy
                 ::AvoidNestedFunctionsRule,
                 // formatting: moving blocks, adding line breaks, indentations etc.
                 ::ConsecutiveSpacesRule,
-                ::WhiteSpaceRule,
+                ::WhiteSpaceRule,  // this rule should be after other rules that can cause wrong spacing
                 ::HeaderCommentRule,
                 ::FileStructureRule,  // this rule should be right before indentation because it should operate on already valid code
                 ::NewlinesRule,  // newlines need to be inserted right before fixing indentation

diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/classes/SingleInitRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/classes/SingleInitRule.kt
@@ -0,0 +1,132 @@
+package org.cqfn.diktat.ruleset.rules.classes
+
+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.CLASS_INITIALIZER
+import com.pinterest.ktlint.core.ast.ElementType.EQ
+import com.pinterest.ktlint.core.ast.ElementType.PROPERTY
+import com.pinterest.ktlint.core.ast.ElementType.WHITE_SPACE
+import com.pinterest.ktlint.core.ast.parent
+import org.cqfn.diktat.common.config.rules.RulesConfig
+import org.cqfn.diktat.ruleset.constants.Warnings
+import org.cqfn.diktat.ruleset.utils.getAllChildrenWithType
+import org.cqfn.diktat.ruleset.utils.getIdentifierName
+import org.jetbrains.kotlin.backend.common.onlyIf
+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.impl.source.tree.PsiWhiteSpaceImpl
+import org.jetbrains.kotlin.psi.KtBlockExpression
+import org.jetbrains.kotlin.psi.KtNameReferenceExpression
+import org.jetbrains.kotlin.psi.KtProperty
+import org.jetbrains.kotlin.psi.psiUtil.asAssignment
+import org.jetbrains.kotlin.psi.psiUtil.children
+
+class SingleInitRule(private val configRule: List<RulesConfig>) : Rule("multiple-init-block") {
+    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
+
+        when (node.elementType) {
+            CLASS_BODY -> handleInitBlocks(node)
+        }
+    }
+
+    private fun handleInitBlocks(node: ASTNode) {
+        // merge init blocks if there are multiple
+        node.children()
+            .filter { it.elementType == CLASS_INITIALIZER }
+            .toList()
+            .onlyIf({ size > 1 }) { initBlocks ->
+                val className = node.treeParent.getIdentifierName()?.text
+                Warnings.MULTIPLE_INIT_BLOCKS.warnAndFix(configRule, emitWarn, isFixMode,
+                    "in class <$className> found ${initBlocks.size} `init` blocks", node.startOffset, node) {
+                    mergeInitBlocks(initBlocks)
+                }
+            }
+
+        // move property assignments from init block to property declarations
+        node.findChildByType(CLASS_INITIALIZER)?.let { initBlock ->
+            val properties = node
+                .children()
+                .filter { it.elementType == PROPERTY }
+                .toList()
+            moveAssignmentsToProperties(properties, initBlock)
+        }
+    }
+
+    private fun mergeInitBlocks(initBlocks: List<ASTNode>) {
+        val firstInitBlock = initBlocks.first()
+        initBlocks.drop(1).forEach { initBlock ->
+            firstInitBlock.findChildByType(BLOCK)?.run {
+                val beforeNode = lastChildNode.treePrev.takeIf { it.elementType == WHITE_SPACE } ?: lastChildNode
+                (initBlock.findChildByType(BLOCK)?.psi as? KtBlockExpression)?.statements?.forEach {
+                    addChild(PsiWhiteSpaceImpl("\n"), beforeNode)
+                    addChild(it.node.clone() as ASTNode, beforeNode)
+                }
+            }
+            if (initBlock.treePrev.elementType == WHITE_SPACE && initBlock.treeNext.elementType == WHITE_SPACE) {
+                initBlock.treeParent.removeChild(initBlock.treeNext)
+            }
+            initBlock.treeParent.removeChild(initBlock)
+        }
+        firstInitBlock.parent(CLASS_BODY)?.let(::removeEmptyBlocks)
+    }
+
+    @Suppress("UnsafeCallOnNullableType")
+    private fun moveAssignmentsToProperties(properties: List<ASTNode>, initBlock: ASTNode) {
+        initBlock
+            .findChildByType(BLOCK)
+            ?.run {
+                (psi as KtBlockExpression).statements
+                    .mapNotNull { it.asAssignment() }
+                    .filter { it.left is KtNameReferenceExpression }
+                    .groupBy { assignment ->
+                        val assignedRef = assignment.left as KtNameReferenceExpression
+                        properties.find { (it.psi as KtProperty).name == assignedRef.getReferencedName() }
+                    }
+                    .filterKeys { it != null }
+                    .mapKeys { (k, _) -> k as ASTNode }
+                    .filter { (property, assignments) ->
+                        !(property.psi as KtProperty).hasBody() && assignments.size == 1
+                    }
+                    .onlyIf({ isNotEmpty() }) {
+                        Warnings.MULTIPLE_INIT_BLOCKS.warnAndFix(configRule, emitWarn, isFixMode,
+                            "`init` block has assignments that can be moved to declarations", initBlock.startOffset, initBlock
+                        ) {
+                            it.forEach { (property, assignments) ->
+                                val assignment = assignments.single()
+                                property.addChild(PsiWhiteSpaceImpl(" "), null)
+                                property.addChild(LeafPsiElement(EQ, "="), null)
+                                property.addChild(PsiWhiteSpaceImpl(" "), null)
+                                property.addChild(assignment.right!!.node.clone() as ASTNode, null)
+                                assignment.node.run {
+                                    if (treePrev.elementType == WHITE_SPACE) {
+                                        treeParent.removeChild(treePrev)
+                                    }
+                                    treeParent.removeChild(this)
+                                }
+                            }
+                        }
+                    }
+            }
+        initBlock.parent(CLASS_BODY)?.let(::removeEmptyBlocks)
+    }
+
+    private fun removeEmptyBlocks(node: ASTNode) {
+        node.getAllChildrenWithType(CLASS_INITIALIZER)
+            .filter {
+                (it.findChildByType(BLOCK)?.psi as KtBlockExpression?)?.statements?.isEmpty() ?: false
+            }
+            .forEach {
+                it.treeParent.removeChild(it)
+            }
+    }
+}
diff --git a/...fn/diktat/ruleset/rules/WhiteSpaceRule.kt → ...tat/ruleset/rules/files/WhiteSpaceRule.kt b/...fn/diktat/ruleset/rules/WhiteSpaceRule.kt → ...tat/ruleset/rules/files/WhiteSpaceRule.kt
@@ -1,4 +1,4 @@
-package org.cqfn.diktat.ruleset.rules
+package org.cqfn.diktat.ruleset.rules.files
 
 import com.pinterest.ktlint.core.Rule
 import com.pinterest.ktlint.core.ast.ElementType.ANNOTATION_ENTRY

diff --git a/diktat-rules/src/main/resources/diktat-analysis-huawei.yml b/diktat-rules/src/main/resources/diktat-analysis-huawei.yml
@@ -332,6 +332,10 @@
 - name: WRONG_NAME_OF_VARIABLE_INSIDE_ACCESSOR
   enabled: true
   configuration: {}
+# Checks that classes have only one init block
+- name: MULTIPLE_INIT_BLOCKS
+  enabled: true
+  configuration: {}
 # Checks that there are abstract functions in abstract class
 - name: CLASS_SHOULD_NOT_BE_ABSTRACT
   enabled: true

diff --git a/diktat-rules/src/main/resources/diktat-analysis.yml b/diktat-rules/src/main/resources/diktat-analysis.yml
@@ -334,6 +334,10 @@
 - name: WRONG_NAME_OF_VARIABLE_INSIDE_ACCESSOR
   enabled: true
   configuration: {}
+# Checks that classes have only one init block
+- name: MULTIPLE_INIT_BLOCKS
+  enabled: true
+  configuration: { }
 # Checks that there are abstract functions in abstract class
 - name: CLASS_SHOULD_NOT_BE_ABSTRACT
   enabled: true

diff --git a/...at-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/spaces/WhiteSpaceRuleFixTest.kt b/...at-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/spaces/WhiteSpaceRuleFixTest.kt
@@ -1,7 +1,7 @@
 package org.cqfn.diktat.ruleset.chapter3.spaces
 
 import generated.WarningNames
-import org.cqfn.diktat.ruleset.rules.WhiteSpaceRule
+import org.cqfn.diktat.ruleset.rules.files.WhiteSpaceRule
 import org.cqfn.diktat.util.FixTestBase
 import org.junit.jupiter.api.Tag
 import org.junit.jupiter.api.Test

diff --git a/...t-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/spaces/WhiteSpaceRuleWarnTest.kt b/...t-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/spaces/WhiteSpaceRuleWarnTest.kt
@@ -4,7 +4,7 @@ import com.pinterest.ktlint.core.LintError
 import generated.WarningNames
 import org.cqfn.diktat.ruleset.constants.Warnings.WRONG_WHITESPACE
 import org.cqfn.diktat.ruleset.rules.DIKTAT_RULE_SET_ID
-import org.cqfn.diktat.ruleset.rules.WhiteSpaceRule
+import org.cqfn.diktat.ruleset.rules.files.WhiteSpaceRule
 import org.cqfn.diktat.util.LintTestBase
 import org.junit.jupiter.api.Tag
 import org.junit.jupiter.api.Test

diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/AbstractClassesFixTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/AbstractClassesFixTest.kt
@@ -1,13 +1,12 @@
 package org.cqfn.diktat.ruleset.chapter6
 
-import generated.WarningNames
 import generated.WarningNames.CLASS_SHOULD_NOT_BE_ABSTRACT
 import org.cqfn.diktat.util.FixTestBase
 import org.cqfn.diktat.ruleset.rules.classes.AbstractClassesRule
 import org.junit.jupiter.api.Tag
 import org.junit.jupiter.api.Test
 
-class AbstractClassesFixTest : FixTestBase("test/paragraph6/abstract_classes", ::AbstractClassesRule) {
+class AbstractClassesFixTest : FixTestBase("test/chapter6/abstract_classes", ::AbstractClassesRule) {
     @Test
     @Tag(CLASS_SHOULD_NOT_BE_ABSTRACT)
     fun `fix abstract class`() {

diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/SingleInitRuleFixTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/SingleInitRuleFixTest.kt
@@ -0,0 +1,27 @@
+package org.cqfn.diktat.ruleset.chapter6
+
+import generated.WarningNames
+import org.cqfn.diktat.util.FixTestBase
+import org.cqfn.diktat.ruleset.rules.classes.SingleInitRule
+import org.junit.jupiter.api.Tag
+import org.junit.jupiter.api.Test
+
+class SingleInitRuleFixTest: FixTestBase("test/chapter6/init_blocks", ::SingleInitRule) {
+    @Test
+    @Tag(WarningNames.MULTIPLE_INIT_BLOCKS)
+    fun `should merge init blocks`() {
+        fixAndCompare("InitBlocksExpected.kt", "InitBlocksTest.kt")
+    }
+
+    @Test
+    @Tag(WarningNames.MULTIPLE_INIT_BLOCKS)
+    fun `should move property assignments from init blocks to declarations`() {
+        fixAndCompare("InitBlockWithAssignmentsExpected.kt", "InitBlockWithAssignmentsTest.kt")
+    }
+
+    @Test
+    @Tag(WarningNames.MULTIPLE_INIT_BLOCKS)
+    fun `should merge init blocks and move property assignments from init blocks to declarations`() {
+        fixAndCompare("InitBlocksWithAssignmentsExpected.kt", "InitBlocksWithAssignmentsTest.kt")
+    }
+}
diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/SingleInitRuleWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/SingleInitRuleWarnTest.kt
@@ -0,0 +1,59 @@
+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.classes.SingleInitRule
+import org.cqfn.diktat.util.LintTestBase
+import org.junit.jupiter.api.Tag
+import org.junit.jupiter.api.Test
+
+class SingleInitRuleWarnTest: LintTestBase(::SingleInitRule) {
+    private val ruleId = "$DIKTAT_RULE_SET_ID:multiple-init-block"
+
+    @Test
+    @Tag(WarningNames.MULTIPLE_INIT_BLOCKS)
+    fun `should allow single init block`() {
+        lintMethod(
+            """
+                |class Example {
+                |    init { println("Lorem ipsum") }
+                |}
+            """.trimMargin()
+        )
+    }
+
+    @Test
+    @Tag(WarningNames.MULTIPLE_INIT_BLOCKS)
+    fun `should forbid multiple init blocks`() {
+        lintMethod(
+            """
+                |class Example {
+                |    init { println("Lorem ipsum") }
+                |    
+                |    val foo = 0
+                |    
+                |    init { println("Dolor sit amet") }
+                |}
+            """.trimMargin(),
+            LintError(1, 15, ruleId, "${Warnings.MULTIPLE_INIT_BLOCKS.warnText()} in class <Example> found 2 `init` blocks", true)
+        )
+    }
+
+    @Test
+    @Tag(WarningNames.MULTIPLE_INIT_BLOCKS)
+    fun `should warn if properties are assigned in init block`() {
+        lintMethod(
+            """
+                |class A(baseUrl: String) {
+                |    private val customUrl: String
+                |    init {
+                |        customUrl = "${'$'}baseUrl/myUrl"
+                |    }
+                |}
+            """.trimMargin(),
+            LintError(3, 5, ruleId, "${Warnings.MULTIPLE_INIT_BLOCKS.warnText()} `init` block has assignments that can be moved to declarations", true)
+        )
+    }
+}
diff --git a/...es/ShouldRemoveAbstractKeywordExpected.kt → ...es/ShouldRemoveAbstractKeywordExpected.kt b/...es/ShouldRemoveAbstractKeywordExpected.kt → ...es/ShouldRemoveAbstractKeywordExpected.kt
diff --git a/...lasses/ShouldRemoveAbstractKeywordTest.kt → ...lasses/ShouldRemoveAbstractKeywordTest.kt b/...lasses/ShouldRemoveAbstractKeywordTest.kt → ...lasses/ShouldRemoveAbstractKeywordTest.kt
diff --git a/...at-rules/src/test/resources/test/chapter6/init_blocks/InitBlockWithAssignmentsExpected.kt b/...at-rules/src/test/resources/test/chapter6/init_blocks/InitBlockWithAssignmentsExpected.kt
@@ -0,0 +1,6 @@
+package test.chapter6.init_blocks
+
+class A(baseUrl: String) {
+    private val customUrl: String = "$baseUrl/myUrl"
+
+}
diff --git a/diktat-rules/src/test/resources/test/chapter6/init_blocks/InitBlockWithAssignmentsTest.kt b/diktat-rules/src/test/resources/test/chapter6/init_blocks/InitBlockWithAssignmentsTest.kt
@@ -0,0 +1,8 @@
+package test.chapter6.init_blocks
+
+class A(baseUrl: String) {
+    private val customUrl: String
+init {
+        customUrl = "$baseUrl/myUrl"
+    }
+}
diff --git a/diktat-rules/src/test/resources/test/chapter6/init_blocks/InitBlocksExpected.kt b/diktat-rules/src/test/resources/test/chapter6/init_blocks/InitBlocksExpected.kt
@@ -0,0 +1,9 @@
+package test.chapter6.init_blocks
+
+class Example {
+    init { println("Lorem ipsum")
+println("Dolor sit amet") }
+
+    val foo = 0
+
+    }
diff --git a/diktat-rules/src/test/resources/test/chapter6/init_blocks/InitBlocksTest.kt b/diktat-rules/src/test/resources/test/chapter6/init_blocks/InitBlocksTest.kt
@@ -0,0 +1,9 @@
+package test.chapter6.init_blocks
+
+class Example {
+    init { println("Lorem ipsum") }
+
+    val foo = 0
+
+    init { println("Dolor sit amet") }
+}
diff --git a/...t-rules/src/test/resources/test/chapter6/init_blocks/InitBlocksWithAssignmentsExpected.kt b/...t-rules/src/test/resources/test/chapter6/init_blocks/InitBlocksWithAssignmentsExpected.kt
@@ -0,0 +1,10 @@
+package test.chapter6.init_blocks
+
+class A(baseUrl: String) {
+    private val customUrl: String = "$baseUrl/myUrl"
+    init {
+        println("Lorem ipsum")
+println("Dolor sit amet")
+    }
+
+    }