Fixed LineLength

diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter3/LineLength.kt

@@ -8,14 +8,21 @@
 import com.saveourtool.diktat.ruleset.utils.KotlinParser
 import com.saveourtool.diktat.ruleset.utils.appendNewlineMergingWhiteSpace
 import com.saveourtool.diktat.ruleset.utils.calculateLineColByOffset
+import com.saveourtool.diktat.ruleset.utils.countCodeLines
 import com.saveourtool.diktat.ruleset.utils.findAllNodesWithConditionOnLine
+import com.saveourtool.diktat.ruleset.utils.findChildAfter
+import com.saveourtool.diktat.ruleset.utils.findChildBefore
+import com.saveourtool.diktat.ruleset.utils.findChildrenMatching
 import com.saveourtool.diktat.ruleset.utils.findParentNodeWithSpecificType
 import com.saveourtool.diktat.ruleset.utils.getAllChildrenWithType
 import com.saveourtool.diktat.ruleset.utils.getFirstChildWithType
 import com.saveourtool.diktat.ruleset.utils.getLineNumber
 import com.saveourtool.diktat.ruleset.utils.hasChildOfType
+import com.saveourtool.diktat.ruleset.utils.isChildAfterAnother
 import com.saveourtool.diktat.ruleset.utils.isWhiteSpace
 import com.saveourtool.diktat.ruleset.utils.isWhiteSpaceWithNewline
+import com.saveourtool.diktat.ruleset.utils.nextSibling
+import com.saveourtool.diktat.ruleset.utils.prevSibling
 
 import org.jetbrains.kotlin.KtNodeTypes.BINARY_EXPRESSION
 import org.jetbrains.kotlin.KtNodeTypes.BLOCK
@@ -38,7 +45,6 @@
 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.IElementType
-import org.jetbrains.kotlin.kdoc.lexer.KDocTokens
 import org.jetbrains.kotlin.kdoc.lexer.KDocTokens.MARKDOWN_INLINE_LINK
 import org.jetbrains.kotlin.kdoc.lexer.KDocTokens.TEXT
 import org.jetbrains.kotlin.lexer.KtTokens.ANDAND
@@ -89,42 +95,91 @@
     private lateinit var positionByOffset: (Int) -> Pair<Int, Int>
 
     override fun logic(node: ASTNode) {
-        if (node.elementType == KtFileElementType.INSTANCE) {
-            node.getChildren(null).forEach {
-                if (it.elementType != PACKAGE_DIRECTIVE && it.elementType != IMPORT_LIST) {
-                    checkLength(it, configuration)
+        var currentFixNumber = 0
+        var isFixedSmthInPreviousStep: Boolean
+
+        // loop that trying to fix LineLength rule warnings until warnings run out
+        do {
+            isFixedSmthInPreviousStep = false
+            currentFixNumber++
+
+            if (node.elementType == KtFileElementType.INSTANCE) {
+                node.getChildren(null).forEach {
+                    if (it.elementType != PACKAGE_DIRECTIVE && it.elementType != IMPORT_LIST) {
+                        val isFixedSmthInChildNode = checkLength(it, configuration)
+
+                        if (!isFixedSmthInPreviousStep && isFixedSmthInChildNode) {
+                            isFixedSmthInPreviousStep = true
+                        }
+                    }
                 }
             }
-        }
+        } while (isFixedSmthInPreviousStep && currentFixNumber < MAX_FIX_NUMBER)
     }
 
-    @Suppress("UnsafeCallOnNullableType", "TOO_LONG_FUNCTION")
-    private fun checkLength(node: ASTNode, configuration: LineLengthConfiguration) {
+    @Suppress(
+        "UnsafeCallOnNullableType",
+        "TOO_LONG_FUNCTION",
+        "FUNCTION_BOOLEAN_PREFIX"
+    )
+    private fun checkLength(node: ASTNode, configuration: LineLengthConfiguration): Boolean {
+        var isFixedSmthInChildNode = false
+
         var offset = 0
         node.text.lines().forEach { line ->
             if (line.length > configuration.lineLength) {
                 val newNode = node.psi.findElementAt(offset + configuration.lineLength.toInt() - 1)!!.node
+
                 if ((newNode.elementType != TEXT && newNode.elementType != MARKDOWN_INLINE_LINK) || !isKdocValid(newNode)) {
                     positionByOffset = node.treeParent.calculateLineColByOffset()
+
                     val fixableType = isFixable(newNode, configuration)
+
                     LONG_LINE.warnOnlyOrWarnAndFix(
                         configRules, emitWarn,
                         "max line length ${configuration.lineLength}, but was ${line.length}",
                         offset + node.startOffset, node,
                         shouldBeAutoCorrected = fixableType !is None,
                         isFixMode,
                     ) {
-                        // we should keep in mind, that in the course of fixing we change the offset
+                        val textBeforeFix = node.text
                         val textLenBeforeFix = node.textLength
+                        val blankLinesBeforeFix = node.text.lines().size - countCodeLines(node)
+
                         fixableType.fix()
+
+                        val textAfterFix = node.text
                         val textLenAfterFix = node.textLength
-                        // offset for all next nodes changed to this delta
-                        offset += (textLenAfterFix - textLenBeforeFix)
+                        val blankLinesAfterFix = node.text.lines().size - countCodeLines(node)
+
+                        // checking that any fix may have been made
+                        isFixedSmthInChildNode = fixableType !is None
+
+                        // for cases when text doesn't change, and then we need to stop fixes
+                        if (textBeforeFix == textAfterFix) {
+                            isFixedSmthInChildNode = false
+                        }
+
+                        // in some kernel cases of long lines, when in fix step we adding `\n` to certain place of the line
+                        // and part of the line is transferred to the new line, this part may still be too long,
+                        // and then in next fix step we can start generating unnecessary blank lines,
+                        // to detect this we count blank lines and make unfix, if necessary
+                        if (blankLinesAfterFix > blankLinesBeforeFix) {
+                            isFixedSmthInChildNode = false
+                            fixableType.unFix()
+                        } else {
+                            // we should keep in mind, that in the course of fixing we change the offset
+                            // offset for all next nodes changed to this delta
+                            offset += (textLenAfterFix - textLenBeforeFix)
+                        }
                     }
                 }
             }
+
             offset += line.length + 1
         }
+
+        return isFixedSmthInChildNode
     }
 
     @Suppress(
@@ -342,7 +397,7 @@
 
     // fixme json method
     private fun isKdocValid(node: ASTNode) = try {
-        if (node.elementType == KDocTokens.TEXT) {
+        if (node.elementType == TEXT) {
             URL(node.text.split("\\s".toRegex()).last { it.isNotEmpty() })
         } else {
             URL(node.text.substring(node.text.indexOfFirst { it == ']' } + 2, node.textLength - 1))
@@ -458,14 +513,22 @@
          * Abstract fix - fix anything nodes
          */
         abstract fun fix()
+
+        /**
+         * Abstract unFix - unfix fix-changes in anything nodes
+         */
+        abstract fun unFix()
     }
 
     /**
-     * Class None show error long line have unidentified type or something else that we can't analyze
+     * Class None show error that long line have unidentified type or something else that we can't analyze
      */
     private class None : LongLineFixableCases(KotlinParser().createNode("ERROR")) {
         @Suppress("EmptyFunctionBlock")
         override fun fix() {}
+
+        @Suppress("EmptyFunctionBlock")
+        override fun unFix() {}
     }
 
     /**
@@ -493,9 +556,19 @@
                 if (node.treePrev.isWhiteSpace()) {
                     node.treeParent.removeChild(node.treePrev)
                 }
-                val newLineNodeOnPreviousLine = node.findAllNodesWithConditionOnLine(node.getLineNumber() - 1) {
-                    it.elementType == WHITE_SPACE && it.textContains('\n')
-                }?.lastOrNull()
+
+                // for cases when property has multiline initialization, and then we need to move comment before first line
+                val newLineNodeOnPreviousLine = if (node.treeParent.elementType == PROPERTY) {
+                    node.treeParent.treeParent.findChildrenMatching {
+                        it.elementType == WHITE_SPACE && node.treeParent.treeParent.isChildAfterAnother(node.treeParent, it) && it.textContains('\n')
+                    }
+                        .lastOrNull()
+                } else {
+                    node.findAllNodesWithConditionOnLine(node.getLineNumber() - 1) {
+                        it.elementType == WHITE_SPACE && it.textContains('\n')
+                    }?.lastOrNull()
+                }
+
                 newLineNodeOnPreviousLine?.let {
                     val parent = node.treeParent
                     parent.removeChild(node)
@@ -504,6 +577,9 @@
                 }
             }
         }
+
+        @Suppress("EmptyFunctionBlock")
+        override fun unFix() {}
     }
 
     /**
@@ -529,6 +605,9 @@
             val correctNode = KotlinParser().createNode("$firstPart$textBetweenParts$secondPart")
             node.treeParent.replaceChild(node, correctNode)
         }
+
+        @Suppress("EmptyFunctionBlock")
+        override fun unFix() {}
     }
 
     /**
@@ -553,6 +632,9 @@
             }
             binNode?.appendNewlineMergingWhiteSpace(nextNode, nextNode)
         }
+
+        @Suppress("EmptyFunctionBlock")
+        override fun unFix() {}
     }
 
     /**
@@ -596,6 +678,9 @@
             }
         }
 
+        @Suppress("EmptyFunctionBlock")
+        override fun unFix() {}
+
         /**
          * This method stored all the nodes that have [BINARY_EXPRESSION] or [PREFIX_EXPRESSION] element type.
          * - First elem in List - Logic Binary Expression (`&&`, `||`)
@@ -667,6 +752,18 @@
         override fun fix() {
             node.appendNewlineMergingWhiteSpace(null, node.findChildByType(EQ)?.treeNext)
         }
+
+        override fun unFix() {
+            node.findChildAfter(EQ, WHITE_SPACE)?.let { correctWhiteSpace ->
+                if (correctWhiteSpace.textContains('\n')) {
+                    correctWhiteSpace.nextSibling()?.let { wrongWhiteSpace ->
+                        if (wrongWhiteSpace.textContains('\n')) {
+                            node.removeChild(wrongWhiteSpace)
+                        }
+                    }
+                }
+            }
+        }
     }
 
     /**
@@ -680,6 +777,9 @@
             node.appendNewlineMergingWhiteSpace(node.findChildByType(LBRACE)?.treeNext, node.findChildByType(LBRACE)?.treeNext)
             node.appendNewlineMergingWhiteSpace(node.findChildByType(RBRACE)?.treePrev, node.findChildByType(RBRACE)?.treePrev)
         }
+
+        @Suppress("EmptyFunctionBlock")
+        override fun unFix() {}
     }
 
     /**
@@ -697,6 +797,9 @@
             val nodeBeforeDot = splitNode?.treePrev
             node.appendNewlineMergingWhiteSpace(nodeBeforeDot, splitNode)
         }
+
+        @Suppress("EmptyFunctionBlock")
+        override fun unFix() {}
     }
 
     /**
@@ -736,6 +839,18 @@
             }
         }
 
+        override fun unFix() {
+            node.findChildBefore(RPAR, WHITE_SPACE)?.let { correctWhiteSpace ->
+                if (correctWhiteSpace.textContains('\n')) {
+                    correctWhiteSpace.prevSibling()?.let { wrongWhiteSpace ->
+                        if (wrongWhiteSpace.textContains('\n')) {
+                            node.removeChild(wrongWhiteSpace)
+                        }
+                    }
+                }
+            }
+        }
+
         private fun fixFirst(): Int {
             val lineLength = maximumLineLength.lineLength
             var startOffset = 0
@@ -765,9 +880,13 @@
                 node.appendNewlineMergingWhiteSpace(it.treeNext, it.treeNext)
             }
         }
+
+        @Suppress("EmptyFunctionBlock")
+        override fun unFix() {}
     }
 
     companion object {
+        private const val MAX_FIX_NUMBER = 10
         private const val MAX_LENGTH = 120L
         const val NAME_ID = "line-length"
     }

diktat-rules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter3/LineLengthFixTest.kt

@@ -4,7 +4,6 @@ import com.saveourtool.diktat.common.config.rules.RulesConfig
 import com.saveourtool.diktat.ruleset.constants.Warnings.LONG_LINE
 import com.saveourtool.diktat.ruleset.rules.chapter3.LineLength
 import com.saveourtool.diktat.util.FixTestBase
-import org.junit.jupiter.api.Disabled
 import org.junit.jupiter.api.Test
 
 class LineLengthFixTest : FixTestBase("test/paragraph3/long_line", ::LineLength) {
@@ -29,7 +28,6 @@ class LineLengthFixTest : FixTestBase("test/paragraph3/long_line", ::LineLength)
             mapOf("lineLength" to "151"))
     )
 
-    @Disabled("https://github.com/saveourtool/diktat/issues/1737")
     @Test
     fun `should fix long comment`() {
         fixAndCompare("LongLineCommentExpected.kt", "LongLineCommentTest.kt", rulesConfigListLineLength)
@@ -40,55 +38,46 @@ class LineLengthFixTest : FixTestBase("test/paragraph3/long_line", ::LineLength)
         fixAndCompare("LongInlineCommentsExpected.kt", "LongInlineCommentsTest.kt", rulesConfigListLineLength)
     }
 
-    @Disabled("https://github.com/saveourtool/diktat/issues/1737")
     @Test
-    fun `should not fix long comment which located on the line length limit`() {
+    fun `should fix long comment 2`() {
         fixAndCompare("LongLineCommentExpected2.kt", "LongLineCommentTest2.kt", rulesConfigListDefaultLineLength)
     }
 
-    @Disabled("https://github.com/saveourtool/diktat/issues/1737")
     @Test
     fun `should fix long string template while some fix is already done`() {
         fixAndCompare("LongStringTemplateExpected.kt", "LongStringTemplateTest.kt", rulesConfigListDefaultLineLength)
     }
 
-    @Disabled("https://github.com/saveourtool/diktat/issues/1737")
     @Test
     fun `should fix long binary expression`() {
         fixAndCompare("LongLineExpressionExpected.kt", "LongLineExpressionTest.kt", rulesConfigListLineLength)
     }
 
-    @Disabled("https://github.com/saveourtool/diktat/issues/1737")
     @Test
     fun `should fix complex long binary expressions`() {
         fixAndCompare("LongBinaryExpressionExpected.kt", "LongBinaryExpressionTest.kt", rulesConfigListLineLength)
     }
 
-    @Disabled("https://github.com/saveourtool/diktat/issues/1737")
     @Test
     fun `should fix long function`() {
         fixAndCompare("LongLineFunExpected.kt", "LongLineFunTest.kt", rulesConfigListLineLength)
     }
 
-    @Disabled("https://github.com/saveourtool/diktat/issues/1737")
     @Test
     fun `should fix long right value`() {
         fixAndCompare("LongLineRValueExpected.kt", "LongLineRValueTest.kt", rulesConfigListLineLength)
     }
 
-    @Disabled("https://github.com/saveourtool/diktat/issues/1737")
     @Test
     fun `should fix short long right value`() {
         fixAndCompare("LongShortRValueExpected.kt", "LongShortRValueTest.kt", rulesConfigListShortLineLength)
     }
 
-    @Disabled("https://github.com/saveourtool/diktat/issues/1737")
     @Test
     fun `shouldn't fix`() {
         fixAndCompare("LongExpressionNoFixExpected.kt", "LongExpressionNoFixTest.kt", rulesConfigListShortLineLength)
     }
 
-    @Disabled("https://github.com/saveourtool/diktat/issues/1737")
     @Test
     fun `should fix annotation`() {
         fixAndCompare("LongLineAnnotationExpected.kt", "LongLineAnnotationTest.kt", rulesConfigListLineLength)
@@ -104,7 +93,6 @@ class LineLengthFixTest : FixTestBase("test/paragraph3/long_line", ::LineLength)
         fixAndCompare("LongExpressionInConditionExpected.kt", "LongExpressionInConditionTest.kt", rulesConfigListLineLength)
     }
 
-    @Disabled("https://github.com/saveourtool/diktat/issues/1737")
     @Test
     fun `fix long Dot Qualified Expression`() {
         fixAndCompare("LongDotQualifiedExpressionExpected.kt", "LongDotQualifiedExpressionTest.kt", rulesConfigListLineLength)

diktat-rules/src/test/resources/test/paragraph3/long_line/LongBinaryExpressionExpected.kt

@@ -37,12 +37,14 @@ fun foo() {
 ?: 123 + Methood().linelength
 
     val variable =
- Methoooooooooooooooooooooooooood() ?: "some loooooong string"
+ Methoooooooooooooooooooooooooood()
+ ?: "some loooooong string"
 
     val variable = Methooooood()
  ?: "some looong string"
 
     var headerKdoc = firstCodeNode.prevSibling {
  it.elementType == KDOC
- } ?: if (firstCodeNode == packageDirectiveNode) importsList?.prevSibling { it.elementType == KDOC } else null
+ }
+ ?: if (firstCodeNode == packageDirectiveNode) importsList?.prevSibling { it.elementType == KDOC } else null
 }