Repository: analysis-dev/diktat (kotlin)

Issue:
SINGLE_CONSTRUCTOR_SHOULD_BE_PRIMARY: text is not valid (KotlinParseException)
```
package io.reflekt.plugin.scripting

import io.reflekt.plugin.analysis.models.Import
import org.reflections.ReflectionUtils
import org.reflections.Reflections
import org.reflections.scanners.SubTypesScanner
import org.reflections.util.ConfigurationBuilder
import java.io.File
import java.lang.reflect.Field
import java.lang.reflect.Method
import java.lang.reflect.Modifier
import java.net.URLClassLoader

/* Checks if specified imports can be found in classpath. */
@Suppress("ConvertSecondaryConstructorToPrimary")
class ImportChecker {
    /* Fully qualified names of public packages, classes, functions and properties in classpath */
    private val allNames = HashSet<String>()

    constructor(classpath: List<File>) {
        if (classpath.isEmpty()) return

        val urls = classpath.map { it.toURI().toURL() }
        val classLoader = URLClassLoader(urls.toTypedArray())
        // Scan each class in classpath
        val reflections = Reflections(
            ConfigurationBuilder()
                .addClassLoader(classLoader)
                .setUrls(urls)
                .setScanners(SubTypesScanner(false))
        )

        // Get all classes (each class is subtype of java.lang.Object)
        reflections.getSubTypesOf(Object::class.java)
            // Only public classes can be imported
            .filter { Modifier.isPublic(it.modifiers) }
            .filter {
                try {
                    // Skip if no canonical name
                    it.canonicalName != null
                 } catch (e: Throwable) {
                    // Skip if canonical name cannot be resolved
                    false
                }
            }
            .forEach { clazz ->
                // All public methods of class
                val methods = try {
                    ReflectionUtils.getAllMethods(clazz, ReflectionUtils.withModifier(Modifier.PUBLIC))
                } catch (e: Throwable) {
                    emptySet<Method>()
                }
                // All public fields of class
                val fields = try {
                    ReflectionUtils.getAllFields(clazz, ReflectionUtils.withModifier(Modifier.PUBLIC))
                } catch (e: Throwable) {
                    emptySet<Field>()
                }

                // Save method and field names with specified prefix
                val addMembers: (prefix: String) -> Unit = { prefix ->
                    allNames.addAll(methods.map { method -> "$prefix.${method.name}" })
                    allNames.addAll(fields.map { field -> "$prefix.${field.name}" })
                }

                // Full package may be imported
                allNames.add(clazz.packageName)
                // Class may be imported
                allNames.add(clazz.canonicalName)
                // Class methods/fields may be imported
                addMembers(clazz.canonicalName)

                // Top-level functions and properties
                if (clazz.enclosingClass == null && clazz.simpleName.endsWith("Kt")) {
                    addMembers(clazz.packageName)
                }

                // Functions and properties in companion object
                if (clazz.enclosingClass != null && clazz.simpleName == "Companion") {
                    addMembers(clazz.enclosingClass.canonicalName)
                }
            }
    }

    // Checks if specified import may be found in the classpath.
    private fun checkImport(import: Import) = import.fqName in allNames

    // Returns imports that may be found in the classpath.
    fun filterImports(imports: List<Import>) = imports.filter { checkImport(it) }
}



```
[ERROR] 2021-11-07 13:44:19 Internal error has occurred in rule [single-constructor]. Please make an issue on this bug at https://github.com/cqfn/diKTat/.
                       As a workaround you can disable these inspections in yml config: <[SINGLE_CONSTRUCTOR_SHOULD_BE_PRIMARY]>.
                       Root cause of the problem is in [D:\reflekt\reflekt-plugin\src\main\kotlin\io\reflekt\plugin\scripting\ImportChecker.kt] file.
org.cqfn.diktat.ruleset.utils.KotlinParseException: Text is not valid: [init {

```

Pull request:
3 SINGLE_CONSTRUCTOR_SHOULD_BE_PRIMARY bugs

This pull request closes #1093 

## Actions checklist
* [ ] Fixed bug with keywords
* [ ] Fixed bug with comments
* [ ] Fixed bug with wrong 'constructor to init' transformation
* [ ] Added tests

Patch:
diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter6/classes/SingleConstructorRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter6/classes/SingleConstructorRule.kt
index 96a1f4ff96..9df0499c23 100644
--- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter6/classes/SingleConstructorRule.kt
+++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter6/classes/SingleConstructorRule.kt
@@ -4,17 +4,22 @@ import org.cqfn.diktat.common.config.rules.RulesConfig
 import org.cqfn.diktat.ruleset.constants.Warnings.SINGLE_CONSTRUCTOR_SHOULD_BE_PRIMARY
 import org.cqfn.diktat.ruleset.rules.DiktatRule
 import org.cqfn.diktat.ruleset.utils.KotlinParser
+import org.cqfn.diktat.ruleset.utils.findChildrenMatching
 import org.cqfn.diktat.ruleset.utils.getAllChildrenWithType
 import org.cqfn.diktat.ruleset.utils.getIdentifierName
 import org.cqfn.diktat.ruleset.utils.hasChildOfType
 import org.cqfn.diktat.ruleset.utils.isGoingAfter
 
+import com.pinterest.ktlint.core.ast.ElementType.BLOCK_COMMENT
 import com.pinterest.ktlint.core.ast.ElementType.CLASS
 import com.pinterest.ktlint.core.ast.ElementType.CLASS_BODY
+import com.pinterest.ktlint.core.ast.ElementType.EOL_COMMENT
+import com.pinterest.ktlint.core.ast.ElementType.KDOC
 import com.pinterest.ktlint.core.ast.ElementType.MODIFIER_LIST
 import com.pinterest.ktlint.core.ast.ElementType.PRIMARY_CONSTRUCTOR
 import com.pinterest.ktlint.core.ast.ElementType.SECONDARY_CONSTRUCTOR
 import com.pinterest.ktlint.core.ast.ElementType.WHITE_SPACE
+import com.pinterest.ktlint.core.ast.nextCodeSibling
 import org.jetbrains.kotlin.com.intellij.lang.ASTNode
 import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.PsiWhiteSpaceImpl
 import org.jetbrains.kotlin.psi.KtBinaryExpression
@@ -83,7 +88,15 @@ class SingleConstructorRule(configRules: List<RulesConfig>) : DiktatRule(
             ?.statements
             ?.partition { it is KtBinaryExpression && it.asAssignment() != null }
             ?.run { first.map { it as KtBinaryExpression } to second }
-            ?: emptyList<KtBinaryExpression>() to emptyList()
+            ?: (emptyList<KtBinaryExpression>() to emptyList())
+
+        val comments = (secondaryCtor.psi as KtSecondaryConstructor)
+            .bodyBlockExpression
+            ?.findChildrenMatching { it.elementType == EOL_COMMENT || it.elementType == BLOCK_COMMENT || it.elementType == KDOC }
+            ?.associate {
+                it.text to it.nextCodeSibling()
+            }
+            ?.filterValues { it != null }
 
         val classProperties = (classNode.psi as KtClass).getProperties()
         val localProperties = secondaryCtor.psi.collectDescendantsOfType<KtProperty> { it.isLocal }
@@ -109,7 +122,7 @@ class SingleConstructorRule(configRules: List<RulesConfig>) : DiktatRule(
             }
             .distinct()
 
-        classNode.convertSecondaryConstructorToPrimary(secondaryCtor, declarationsAssignedInCtor, nonTrivialAssignments, otherStatements)
+        classNode.convertSecondaryConstructorToPrimary(secondaryCtor, declarationsAssignedInCtor, nonTrivialAssignments, otherStatements, comments)
     }
 
     @Suppress("UnsafeCallOnNullableType")
@@ -136,12 +149,17 @@ class SingleConstructorRule(configRules: List<RulesConfig>) : DiktatRule(
                 }
                 .filterValues { left -> left.getReferencedName() in classProperties.mapNotNull { it.name } }
 
-    @Suppress("NestedBlockDepth", "GENERIC_VARIABLE_WRONG_DECLARATION")
+    @Suppress(
+        "NestedBlockDepth",
+        "GENERIC_VARIABLE_WRONG_DECLARATION",
+        "TOO_LONG_FUNCTION"
+    )
     private fun ASTNode.convertSecondaryConstructorToPrimary(
         secondaryCtor: ASTNode,
         declarationsAssignedInCtor: List<KtProperty>,
         nonTrivialAssignments: Map<KtBinaryExpression, KtNameReferenceExpression>,
-        otherStatements: List<KtExpression>
+        otherStatements: List<KtExpression>,
+        comments: Map<String, ASTNode?>?
     ) {
         require(elementType == CLASS)
 
@@ -150,6 +168,21 @@ class SingleConstructorRule(configRules: List<RulesConfig>) : DiktatRule(
         val nonTrivialSecondaryCtorParameters = getNonTrivialParameters(secondaryCtor, nonTrivialAssignments.keys, localProperties)
 
         val primaryCtorNode = createPrimaryCtor(secondaryCtor, declarationsAssignedInCtor, nonTrivialSecondaryCtorParameters)
+
+        val initBody: MutableList<String> = mutableListOf()
+        initBody.addAll(otherStatements.map { it.text })
+        initBody.addAll(nonTrivialAssignments.keys.map { it.text })
+
+        val newArgumentListOfSecondaryCtor: List<KtParameter> = (secondaryCtor.psi as KtSecondaryConstructor)
+            .valueParameters
+            .filter { arg -> arg.name !in nonTrivialSecondaryCtorParameters.map { it.name } }  // get rid of ctor arguments
+            .filter { arg -> arg.name !in declarationsAssignedInCtor.map { it.name } }  // get rid of ctor arguments
+            .filter { arg -> initBody.any { expr -> arg.name.toString() in expr } }  // get rid of parameters that do not appear in text
+
+        if (newArgumentListOfSecondaryCtor.isNotEmpty()) {
+            return
+        }
+
         addChild(primaryCtorNode, findChildByType(CLASS_BODY))
         declarationsAssignedInCtor.forEach { ktProperty ->
             ktProperty.node.run {
@@ -158,14 +191,19 @@ class SingleConstructorRule(configRules: List<RulesConfig>) : DiktatRule(
             }
         }
 
+        comments?.forEach { (comment, nextExpression) ->
+            if (initBody.indexOf(nextExpression?.text) != -1) {
+                initBody.add(initBody.indexOf(nextExpression?.text), comment)
+            }
+        }
+
         if (otherStatements.isNotEmpty() || nonTrivialAssignments.isNotEmpty()) {
             findChildByType(CLASS_BODY)?.run {
-                val classInitializer = kotlinParser.createNode(
+                val classInitializer = kotlinParser.createNodeForInit(
                     """|init {
-                       |    ${(otherStatements + nonTrivialAssignments.keys).joinToString("\n") { it.text }}
+                       |    ${initBody.joinToString("\n")}
                        |}
-                    """.trimMargin()
-                )
+                    """.trimMargin())
                 addChild(classInitializer, secondaryCtor)
                 addChild(PsiWhiteSpaceImpl("\n"), secondaryCtor)
             }
diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/KotlinParser.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/KotlinParser.kt
index 9307619f1c..e71655b9c2 100644
--- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/KotlinParser.kt
+++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/KotlinParser.kt
@@ -74,6 +74,43 @@ class KotlinParser {
      */
     fun createNode(text: String, isPackage: Boolean = false) = makeNode(text, isPackage) ?: throw KotlinParseException("Text is not valid: [$text]")
 
+    /**
+     * @param text kotlin code
+     * @return [ASTNode] which is secondary constructor
+     * @throws KotlinParseException if code is incorrect
+     */
+    @Suppress("SAY_NO_TO_VAR")
+    fun createNodeForSecondaryConstructor(text: String): ASTNode {
+        val node: ASTNode = ktPsiFactory
+            .createSecondaryConstructor(text)
+            .node
+        if (!node.isCorrect()) {
+            throw KotlinParseException("Text is not valid: [$text]")
+        }
+        return node
+    }
+
+    /**
+     * @param text kotlin code
+     * @return [ASTNode] which is init block
+     * @throws KotlinParseException if code is incorrect
+     */
+    @Suppress(
+        "SAY_NO_TO_VAR",
+        "UnsafeCallOnNullableType"
+    )
+    fun createNodeForInit(text: String): ASTNode {
+        val node: ASTNode = ktPsiFactory
+            .createBlockCodeFragment(text, null)
+            .node
+            .findChildByType(BLOCK)!!
+            .firstChildNode
+        if (!node.isCorrect()) {
+            throw KotlinParseException("Text is not valid: [$text]")
+        }
+        return node
+    }
+
     /**
      * @param text kotlin code
      * @return [KtPrimaryConstructor]


Test patch:
diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/SingleConstructorRuleFixTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/SingleConstructorRuleFixTest.kt
index 9c9b099053..f956271e02 100644
--- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/SingleConstructorRuleFixTest.kt
+++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/SingleConstructorRuleFixTest.kt
@@ -37,4 +37,16 @@ class SingleConstructorRuleFixTest : FixTestBase("test/chapter6/classes", ::Sing
     fun `should keep assignments and required local variables in an init block`() {
         fixAndCompare("AssignmentWithLocalPropertyExpected.kt", "AssignmentWithLocalPropertyTest.kt")
     }
+
+    @Test
+    @Tag(WarningNames.SINGLE_CONSTRUCTOR_SHOULD_BE_PRIMARY)
+    fun `should not remove different comments`() {
+        fixAndCompare("ConstructorWithCommentsExpected.kt", "ConstructorWithCommentsTest.kt")
+    }
+
+    @Test
+    @Tag(WarningNames.SINGLE_CONSTRUCTOR_SHOULD_BE_PRIMARY)
+    fun `should not replace constructor with init block`() {
+        fixAndCompare("ConstructorWithComplexAssignmentsExpected.kt", "ConstructorWithComplexAssignmentsTest.kt")
+    }
 }
diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/utils/KotlinParserTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/utils/KotlinParserTest.kt
index 16f2bfc6eb..38d177e5ff 100644
--- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/utils/KotlinParserTest.kt
+++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/utils/KotlinParserTest.kt
@@ -2,6 +2,7 @@ package org.cqfn.diktat.ruleset.utils
 
 import org.cqfn.diktat.util.applyToCode
 
+import com.pinterest.ktlint.core.ast.ElementType.CALL_EXPRESSION
 import com.pinterest.ktlint.core.ast.ElementType.CLASS
 import com.pinterest.ktlint.core.ast.ElementType.CLASS_BODY
 import com.pinterest.ktlint.core.ast.ElementType.CLASS_KEYWORD
@@ -14,6 +15,7 @@ import com.pinterest.ktlint.core.ast.ElementType.KDOC
 import com.pinterest.ktlint.core.ast.ElementType.PACKAGE_DIRECTIVE
 import com.pinterest.ktlint.core.ast.ElementType.PROPERTY
 import com.pinterest.ktlint.core.ast.ElementType.RBRACE
+import com.pinterest.ktlint.core.ast.ElementType.SECONDARY_CONSTRUCTOR
 import com.pinterest.ktlint.core.ast.ElementType.WHITE_SPACE
 import org.jetbrains.kotlin.com.intellij.lang.ASTNode
 import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.PsiWhiteSpaceImpl
@@ -209,4 +211,31 @@ class KotlinParserTest {
         """.trimIndent()
         Assertions.assertEquals(kdocText, node.findChildByType(FUN)!!.firstChildNode.text)
     }
+
+    @Test
+    fun `test createNodeForInit`() {
+        val code = """
+            |init {
+            |   println("A")
+            |   // import is a weak keyword
+            |   println("B")
+            |}
+            """.trimMargin()
+        val node = KotlinParser().createNodeForInit(code)
+        Assertions.assertEquals(CALL_EXPRESSION, node.elementType)
+        Assertions.assertEquals(code, node.text)
+    }
+
+    @Test
+    fun `test createNodeForSecondaryConstructor`() {
+        val code = """
+            |constructor(a: Int) {
+            |   // import is a weak keyword
+            |   b = a.toString()
+            |}
+            """.trimMargin()
+        val node = KotlinParser().createNodeForSecondaryConstructor(code)
+        Assertions.assertEquals(SECONDARY_CONSTRUCTOR, node.elementType)
+        Assertions.assertEquals(code, node.text)
+    }
 }
diff --git a/diktat-rules/src/test/resources/test/chapter6/classes/ConstructorWithCommentsExpected.kt b/diktat-rules/src/test/resources/test/chapter6/classes/ConstructorWithCommentsExpected.kt
new file mode 100644
index 0000000000..421503380f
--- /dev/null
+++ b/diktat-rules/src/test/resources/test/chapter6/classes/ConstructorWithCommentsExpected.kt
@@ -0,0 +1,15 @@
+package test.chapter6.classes
+
+class Test (){
+init {
+    println("A")
+// import is a weak keyword imported
+println("B")
+/* import is a weak keyword */
+println("C")
+/**
+* import is a weak keyword
+*/
+println("D")
+}
+}
diff --git a/diktat-rules/src/test/resources/test/chapter6/classes/ConstructorWithCommentsTest.kt b/diktat-rules/src/test/resources/test/chapter6/classes/ConstructorWithCommentsTest.kt
new file mode 100644
index 0000000000..89dd55498f
--- /dev/null
+++ b/diktat-rules/src/test/resources/test/chapter6/classes/ConstructorWithCommentsTest.kt
@@ -0,0 +1,15 @@
+package test.chapter6.classes
+
+class Test {
+constructor() {
+println("A")
+// import is a weak keyword imported
+println("B")
+/* import is a weak keyword */
+println("C")
+/**
+* import is a weak keyword
+*/
+println("D")
+}
+}
diff --git a/diktat-rules/src/test/resources/test/chapter6/classes/ConstructorWithComplexAssignmentsExpected.kt b/diktat-rules/src/test/resources/test/chapter6/classes/ConstructorWithComplexAssignmentsExpected.kt
new file mode 100644
index 0000000000..69d1d5424f
--- /dev/null
+++ b/diktat-rules/src/test/resources/test/chapter6/classes/ConstructorWithComplexAssignmentsExpected.kt
@@ -0,0 +1,16 @@
+package test.chapter6.classes
+
+class A {
+    var b: String = ""
+    constructor(a: Int) {
+        // help
+        b = a.toString()
+    }
+    fun foo1() {
+
+    }
+    // ssss
+    fun foo() {
+
+    }
+}
\ No newline at end of file
diff --git a/diktat-rules/src/test/resources/test/chapter6/classes/ConstructorWithComplexAssignmentsTest.kt b/diktat-rules/src/test/resources/test/chapter6/classes/ConstructorWithComplexAssignmentsTest.kt
new file mode 100644
index 0000000000..69d1d5424f
--- /dev/null
+++ b/diktat-rules/src/test/resources/test/chapter6/classes/ConstructorWithComplexAssignmentsTest.kt
@@ -0,0 +1,16 @@
+package test.chapter6.classes
+
+class A {
+    var b: String = ""
+    constructor(a: Int) {
+        // help
+        b = a.toString()
+    }
+    fun foo1() {
+
+    }
+    // ssss
+    fun foo() {
+
+    }
+}
\ No newline at end of file