Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed LineLength #1759

Merged
merged 10 commits into from
Oct 16, 2023
Merged

Fixed LineLength #1759

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
b4de362
Fixed LineLength
DrAlexD Oct 5, 2023
e24ce4b
- fix
DrAlexD Oct 5, 2023
8b7bc55
- added comments
DrAlexD Oct 6, 2023
fa36952
Merge branch 'master' into feature/fix-line-length
DrAlexD Oct 6, 2023
577ba03
- added `MAX_FIX_NUMBER` for limit on fixing to avoid infinite loop
DrAlexD Oct 6, 2023
ee3038a
Merge branch 'master' into feature/fix-line-length
DrAlexD Oct 9, 2023
eef4827
Merge branch 'master' into feature/fix-line-length
DrAlexD Oct 9, 2023
19aae80
- fix
DrAlexD Oct 16, 2023
95c9bbd
Merge branch 'master' into feature/fix-line-length
DrAlexD Oct 16, 2023
3908f06
- fix
DrAlexD Oct 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
140 changes: 124 additions & 16 deletions diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter3/LineLength.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,21 @@ import com.saveourtool.diktat.ruleset.rules.DiktatRule
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
Expand All @@ -38,7 +45,6 @@ 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.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
Expand Down Expand Up @@ -89,42 +95,82 @@ class LineLength(configRules: List<RulesConfig>) : DiktatRule(
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 isFixedSmthInPreviousStep: Boolean

do {
isFixedSmthInPreviousStep = false

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)
}

@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)

isFixedSmthInChildNode = fixableType !is None

if (textBeforeFix == textAfterFix) {
isFixedSmthInChildNode = false
}

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(
Expand Down Expand Up @@ -342,7 +388,7 @@ class LineLength(configRules: List<RulesConfig>) : DiktatRule(

// 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))
Expand Down Expand Up @@ -458,14 +504,22 @@ class LineLength(configRules: List<RulesConfig>) : DiktatRule(
* 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() {}
}

/**
Expand Down Expand Up @@ -493,9 +547,18 @@ class LineLength(configRules: List<RulesConfig>) : DiktatRule(
if (node.treePrev.isWhiteSpace()) {
node.treeParent.removeChild(node.treePrev)
}
val newLineNodeOnPreviousLine = node.findAllNodesWithConditionOnLine(node.getLineNumber() - 1) {
it.elementType == WHITE_SPACE && it.textContains('\n')
}?.lastOrNull()

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)
Expand All @@ -504,6 +567,9 @@ class LineLength(configRules: List<RulesConfig>) : DiktatRule(
}
}
}

@Suppress("EmptyFunctionBlock")
override fun unFix() {}
}

/**
Expand All @@ -529,6 +595,9 @@ class LineLength(configRules: List<RulesConfig>) : DiktatRule(
val correctNode = KotlinParser().createNode("$firstPart$textBetweenParts$secondPart")
node.treeParent.replaceChild(node, correctNode)
}

@Suppress("EmptyFunctionBlock")
override fun unFix() {}
}

/**
Expand All @@ -553,6 +622,9 @@ class LineLength(configRules: List<RulesConfig>) : DiktatRule(
}
binNode?.appendNewlineMergingWhiteSpace(nextNode, nextNode)
}

@Suppress("EmptyFunctionBlock")
override fun unFix() {}
}

/**
Expand Down Expand Up @@ -596,6 +668,9 @@ class LineLength(configRules: List<RulesConfig>) : DiktatRule(
}
}

@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 (`&&`, `||`)
Expand Down Expand Up @@ -667,6 +742,18 @@ class LineLength(configRules: List<RulesConfig>) : DiktatRule(
override fun fix() {
node.appendNewlineMergingWhiteSpace(null, node.findChildByType(EQ)?.treeNext)
}

override fun unFix() {
node.findChildAfter(EQ, WHITE_SPACE)?.let {
if (it.textContains('\n')) {
it.nextSibling()?.let { it2 ->
if (it2.textContains('\n')) {
node.removeChild(it2)
}
}
}
}
}
}

/**
Expand All @@ -680,6 +767,9 @@ class LineLength(configRules: List<RulesConfig>) : DiktatRule(
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() {}
}

/**
Expand All @@ -697,6 +787,9 @@ class LineLength(configRules: List<RulesConfig>) : DiktatRule(
val nodeBeforeDot = splitNode?.treePrev
node.appendNewlineMergingWhiteSpace(nodeBeforeDot, splitNode)
}

@Suppress("EmptyFunctionBlock")
override fun unFix() {}
}

/**
Expand Down Expand Up @@ -736,6 +829,18 @@ class LineLength(configRules: List<RulesConfig>) : DiktatRule(
}
}

override fun unFix() {
node.findChildBefore(RPAR, WHITE_SPACE)?.let {
if (it.textContains('\n')) {
it.prevSibling()?.let { it2 ->
if (it2.textContains('\n')) {
node.removeChild(it2)
}
}
}
}
}

private fun fixFirst(): Int {
val lineLength = maximumLineLength.lineLength
var startOffset = 0
Expand Down Expand Up @@ -765,6 +870,9 @@ class LineLength(configRules: List<RulesConfig>) : DiktatRule(
node.appendNewlineMergingWhiteSpace(it.treeNext, it.treeNext)
}
}

@Suppress("EmptyFunctionBlock")
override fun unFix() {}
}

companion object {
Expand Down
14 changes: 1 addition & 13 deletions diktat-rules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter3/LineLengthFixTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down
6 changes: 4 additions & 2 deletions diktat-rules/src/test/resources/test/paragraph3/long_line/LongBinaryExpressionExpected.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
4 changes: 3 additions & 1 deletion diktat-rules/src/test/resources/test/paragraph3/long_line/LongLineAnnotationExpected.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,6 @@ value = "select * from test inner join test_execution on test.id = test_executio
nativeQuery = true
)
fun some(limit: Int, offset: Int, executionId: Long) =
println("testtesttesttesttesttesttesttesttesttesttesttest")
println(
"testtesttesttesttesttesttesttesttesttesttesttest"
)
Loading