Fix: Don't remove newline when moving comments into class KDoc (#1119)

### What's done:
* Refactoring in `KdocComments`
* Fix in logic: don't remove `WHITE_SPACE` nodes when moving comment node
* Added tests
* Fixed bug with incorrect `isFixMode` flag in `KdocComments`

This pull request closes #1115

diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter2/kdoc/KdocComments.kt

@@ -32,13 +32,15 @@ import com.pinterest.ktlint.core.ast.ElementType.VAL_KEYWORD
 import com.pinterest.ktlint.core.ast.ElementType.VAR_KEYWORD
 import com.pinterest.ktlint.core.ast.ElementType.WHITE_SPACE
 import com.pinterest.ktlint.core.ast.parent
+import com.pinterest.ktlint.core.ast.prevSibling
 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.com.intellij.psi.tree.TokenSet
 import org.jetbrains.kotlin.kdoc.parser.KDocKnownTag
 import org.jetbrains.kotlin.psi.KtParameterList
 import org.jetbrains.kotlin.psi.psiUtil.parents
+import org.jetbrains.kotlin.psi.psiUtil.siblings
 
 /**
  * This rule checks the following features in KDocs:
@@ -51,11 +53,12 @@ class KdocComments(configRules: List<RulesConfig>) : DiktatRule(
     configRules,
     listOf(KDOC_EXTRA_PROPERTY, KDOC_NO_CONSTRUCTOR_PROPERTY,
         KDOC_NO_CONSTRUCTOR_PROPERTY_WITH_COMMENT, MISSING_KDOC_CLASS_ELEMENTS, MISSING_KDOC_TOP_LEVEL)) {
+    private val config by lazy { configRules.getCommonConfiguration() }
+
     /**
      * @param node
      */
     override fun logic(node: ASTNode) {
-        val config = configRules.getCommonConfiguration()
         val filePath = node.getFilePath()
         if (!node.hasTestAnnotation() && !isLocatedInTest(filePath.splitPathToDirs(), config.testAnchors)) {
             when (node.elementType) {
@@ -86,34 +89,36 @@ class KdocComments(configRules: List<RulesConfig>) : DiktatRule(
     }
 
     @Suppress("UnsafeCallOnNullableType", "ComplexMethod")
-    private fun checkValueParameter(node: ASTNode) {
-        if (node.parents().none { it.elementType == PRIMARY_CONSTRUCTOR } ||
-                !node.hasChildOfType(VAL_KEYWORD) && !node.hasChildOfType(VAR_KEYWORD)) {
+    private fun checkValueParameter(valueParameterNode: ASTNode) {
+        if (valueParameterNode.parents().none { it.elementType == PRIMARY_CONSTRUCTOR } ||
+                !valueParameterNode.hasChildOfType(VAL_KEYWORD) &&
+                        !valueParameterNode.hasChildOfType(VAR_KEYWORD)
+        ) {
             return
         }
-        val prevComment = if (node.treePrev.elementType == WHITE_SPACE &&
-                (node.treePrev.treePrev.elementType == EOL_COMMENT ||
-                        node.treePrev.treePrev.elementType == BLOCK_COMMENT)) {
-            node.treePrev.treePrev
-        } else if (node.hasChildOfType(KDOC)) {
-            node.findChildByType(KDOC)!!
-        } else if (node.treePrev.elementType == BLOCK_COMMENT) {
-            node.treePrev
+        val prevComment = if (valueParameterNode.siblings(forward = false)
+            .takeWhile { it.elementType != EOL_COMMENT && it.elementType != BLOCK_COMMENT }
+            .all { it.elementType == WHITE_SPACE }
+        ) {
+            // take previous comment, if it's immediately before `valueParameterNode` or separated only with white space
+            valueParameterNode.prevSibling { it.elementType == EOL_COMMENT || it.elementType == BLOCK_COMMENT }
+        } else if (valueParameterNode.hasChildOfType(KDOC)) {
+            valueParameterNode.findChildByType(KDOC)!!
         } else {
             null
         }
-        val kdocBeforeClass = node.parent({ it.elementType == CLASS })!!.findChildByType(KDOC)
+        val kdocBeforeClass = valueParameterNode.parent({ it.elementType == CLASS })!!.findChildByType(KDOC)
 
         prevComment?.let {
             kdocBeforeClass?.let {
-                checkKdocBeforeClass(node, kdocBeforeClass, prevComment)
+                checkKdocBeforeClass(valueParameterNode, kdocBeforeClass, prevComment)
             }
-                ?: createKdocWithProperty(node, prevComment)
+                ?: createKdocWithProperty(valueParameterNode, prevComment)
         }
             ?: kdocBeforeClass?.let {
-                checkBasicKdocBeforeClass(node, kdocBeforeClass)
+                checkBasicKdocBeforeClass(valueParameterNode, kdocBeforeClass)
             }
-            ?: createKdocBasicKdoc(node)
+            ?: createKdocBasicKdoc(valueParameterNode)
     }
 
     @Suppress("UnsafeCallOnNullableType")
@@ -170,27 +175,20 @@ class KdocComments(configRules: List<RulesConfig>) : DiktatRule(
     }
 
     @Suppress("UnsafeCallOnNullableType")
-    private fun createKdocWithProperty(node: ASTNode, prevComment: ASTNode) {
-        KDOC_NO_CONSTRUCTOR_PROPERTY_WITH_COMMENT.warnAndFix(configRules, emitWarn, isFixMode, prevComment.text, prevComment.startOffset, node) {
-            val classNode = node.parent({ it.elementType == CLASS })!!
+    private fun createKdocWithProperty(valueParameterNode: ASTNode, prevComment: ASTNode) {
+        KDOC_NO_CONSTRUCTOR_PROPERTY_WITH_COMMENT.warnAndFix(configRules, emitWarn, isFixMode, prevComment.text, prevComment.startOffset, valueParameterNode) {
+            val classNode = valueParameterNode.parent({ it.elementType == CLASS })!!
             val newKdocText = if (prevComment.elementType == KDOC) {
                 prevComment.text
             } else if (prevComment.elementType == EOL_COMMENT) {
-                "/**\n * @property ${node.findChildByType(IDENTIFIER)!!.text} ${prevComment.text.removePrefix("//")}\n */"
+                "/**\n * @property ${valueParameterNode.findChildByType(IDENTIFIER)!!.text} ${prevComment.text.removePrefix("//")}\n */"
             } else {
-                "/**\n * @property ${node.findChildByType(IDENTIFIER)!!.text}${prevComment.text.removePrefix("/*").removeSuffix("*/")} */"
+                "/**\n * @property ${valueParameterNode.findChildByType(IDENTIFIER)!!.text}${prevComment.text.removePrefix("/*").removeSuffix("*/")} */"
             }
             val newKdoc = KotlinParser().createNode(newKdocText).findChildByType(KDOC)!!
             classNode.addChild(PsiWhiteSpaceImpl("\n"), classNode.firstChildNode)
             classNode.addChild(newKdoc, classNode.firstChildNode)
-            if (prevComment.elementType == EOL_COMMENT) {
-                node.treeParent.removeRange(prevComment, node)
-            } else {
-                if (prevComment.treeNext.elementType == WHITE_SPACE) {
-                    node.removeChild(prevComment.treeNext)
-                }
-                node.removeChild(prevComment)
-            }
+            valueParameterNode.removeWithWhiteSpace(prevComment)
         }
     }
 
@@ -209,17 +207,14 @@ class KdocComments(configRules: List<RulesConfig>) : DiktatRule(
         // if property is documented with KDoc, which has a property tag inside, then it can contain some additional more complicated
         // structure, that will be hard to move automatically
         val isFixable = propertyInLocalKdoc == null
-        KDOC_NO_CONSTRUCTOR_PROPERTY.warnAndFix(configRules, emitWarn, isFixable, prevComment.text, prevComment.startOffset, node, isFixable) {
+        KDOC_NO_CONSTRUCTOR_PROPERTY.warnAndFix(configRules, emitWarn, isFixMode, prevComment.text, prevComment.startOffset, node, isFixable) {
             propertyInClassKdoc?.let {
                 // local docs should be appended to docs in class
                 appendKdocTagContent(propertyInClassKdoc, "\n$kdocText")
             }
                 ?: insertTextInKdoc(kdocBeforeClass, " * @property ${node.findChildByType(IDENTIFIER)!!.text} ${kdocText.removePrefix("*")}\n")
 
-            if (prevComment.treeNext.elementType == WHITE_SPACE) {
-                node.removeChild(prevComment.treeNext)
-            }
-            node.removeChild(prevComment)
+            node.removeWithWhiteSpace(prevComment)
         }
     }
 
@@ -240,7 +235,7 @@ class KdocComments(configRules: List<RulesConfig>) : DiktatRule(
             ?: run {
                 insertTextInKdoc(kdocBeforeClass, "* @property ${node.findChildByType(IDENTIFIER)!!.text} ${prevComment.text.removeRange(0, 2)}\n")
             }
-        node.treeParent.removeRange(prevComment, node)
+        node.treeParent.removeChildMergingSurroundingWhitespaces(prevComment)
     }
 
     @Suppress("UnsafeCallOnNullableType")
@@ -330,3 +325,35 @@ class KdocComments(configRules: List<RulesConfig>) : DiktatRule(
         private val statementsToDocument = TokenSet.create(CLASS, FUN, PROPERTY)
     }
 }
+
+private fun ASTNode.removeWithWhiteSpace(prevComment: ASTNode) {
+    if (prevComment.elementType == KDOC) {
+        if (prevComment.treeNext.elementType == WHITE_SPACE) {
+            removeChild(prevComment.treeNext)
+        }
+        removeChild(prevComment)
+    } else {
+        removeChildMergingSurroundingWhitespaces(prevComment)
+    }
+}
+
+/**
+ * If [child] node is surrounded by nodes with type `WHITE_SPACE`, remove [child] and merge surrounding nodes,
+ * preserving only a single newline if present (i.e. leaving no empty lines after merging). In any case, [child] is removed
+ * from the tree.
+ */
+private fun ASTNode.removeChildMergingSurroundingWhitespaces(child: ASTNode) {
+    with(child) {
+        if (treeNext?.elementType == WHITE_SPACE && treePrev?.elementType == WHITE_SPACE) {
+            val mergedText = (treePrev.text + treeNext.text)
+            val mergedSpace = if (mergedText.contains('\n')) {
+                '\n' + mergedText.substringAfterLast('\n')
+            } else {
+                mergedText
+            }
+            treeParent.replaceWhiteSpaceText(treePrev, mergedSpace)
+        }
+        treeParent.removeChild(treeNext)
+    }
+    removeChild(child)
+}

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

@@ -131,7 +131,7 @@ fun ASTNode.reversedChildren(): Sequence<ASTNode> = sequence {
 }
 
 /**
- * Replaces the [beforeNode] of type [WHITE_SPACE] with the node with specified [text]
+ * Replaces `this` node's child [beforeNode] of type [WHITE_SPACE] with the node with specified [text]
  *
  * @param beforeNode a node to replace
  * @param text a text (white space characters only) for the new node

diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter2/KdocCommentsFixTest.kt

@@ -19,4 +19,10 @@ class KdocCommentsFixTest : FixTestBase("test/paragraph2/kdoc/", ::KdocComments)
     fun `check fix without class kdoc`() {
         fixAndCompare("ConstructorCommentNoKDocExpected.kt", "ConstructorCommentNoKDocTest.kt")
     }
+
+    @Test
+    @Tag(WarningNames.KDOC_NO_CONSTRUCTOR_PROPERTY)
+    fun `should preserve newlines when moving comments from value parameters`() {
+        fixAndCompare("ConstructorCommentNewlineExpected.kt", "ConstructorCommentNewlineTest.kt")
+    }
 }

diktat-rules/src/test/resources/test/paragraph2/kdoc/ConstructorCommentNewlineExpected.kt

@@ -0,0 +1,19 @@
+package test.paragraph2.kdoc
+
+/**
+ * @property param1
+ * @property param2  first comment
+*/
+class Example(
+    val param1: String,
+    val param2: String, // second comment
+)
+
+/**
+ * @property param1
+  * @property param2 first comment
+*/
+class Example(
+    val param1: String,
+    val param2: String, /* second comment */
+)

diktat-rules/src/test/resources/test/paragraph2/kdoc/ConstructorCommentNewlineTest.kt

@@ -0,0 +1,11 @@
+package test.paragraph2.kdoc
+
+class Example(
+    val param1: String,  // first comment
+    val param2: String, // second comment
+)
+
+class Example(
+    val param1: String,  /* first comment */
+    val param2: String, /* second comment */
+)