From fb06399fb404f27a443a857d5f5eadbc3967c1ca Mon Sep 17 00:00:00 2001 From: Andrey Kuleshov Date: Fri, 1 Jul 2022 22:28:46 +0300 Subject: [PATCH 1/4] Making dot qualified expression configurable ### What's done: - now dot qualified expression will be configured with 'maxCallsInOneLine' again --- .../rules/chapter3/files/NewlinesRule.kt | 45 ++++++++++--------- .../chapter3/files/NewlinesRuleWarnTest.kt | 41 +++++++++++------ 2 files changed, 52 insertions(+), 34 deletions(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/files/NewlinesRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/files/NewlinesRule.kt index baa0b95d71..7d3faecd3c 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/files/NewlinesRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/files/NewlinesRule.kt @@ -26,6 +26,7 @@ import org.cqfn.diktat.ruleset.utils.isFollowedByNewline import org.cqfn.diktat.ruleset.utils.isGradleScript import org.cqfn.diktat.ruleset.utils.isSingleLineIfElse import org.cqfn.diktat.ruleset.utils.leaveOnlyOneNewLine +import org.cqfn.diktat.ruleset.utils.prettyPrint import com.pinterest.ktlint.core.ast.ElementType.ANDAND import com.pinterest.ktlint.core.ast.ElementType.ARROW @@ -141,36 +142,40 @@ class NewlinesRule(configRules: List) : DiktatRule( RETURN -> handleReturnStatement(node) SUPER_TYPE_LIST, VALUE_PARAMETER_LIST, VALUE_ARGUMENT_LIST -> handleList(node) // this logic splits long expressions into multiple lines - DOT_QUALIFIED_EXPRESSION, SAFE_ACCESS_EXPRESSION, POSTFIX_EXPRESSION -> handDotQualifiedAndSafeAccessExpression(node) + DOT_QUALIFIED_EXPRESSION, SAFE_ACCESS_EXPRESSION, POSTFIX_EXPRESSION -> handleDotQualifiedExpressions(node) else -> { } } } @Suppress("GENERIC_VARIABLE_WRONG_DECLARATION", "MagicNumber") - private fun handDotQualifiedAndSafeAccessExpression(node: ASTNode) { + private fun handleDotQualifiedExpressions(node: ASTNode) { val listParentTypesNoFix = listOf(PACKAGE_DIRECTIVE, IMPORT_DIRECTIVE, VALUE_PARAMETER_LIST, VALUE_ARGUMENT_LIST, DOT_QUALIFIED_EXPRESSION, SAFE_ACCESS_EXPRESSION, POSTFIX_EXPRESSION) - if (isNotFindParentNodeWithSpecificManyType(node, listParentTypesNoFix)) { - val listDot = node.findAllNodesWithCondition( + if (isNotFoundParentNodeOfTypes(node, listParentTypesNoFix)) { + val listOfDotQualifiedExpressions = node.findAllNodesWithCondition( withSelf = true, excludeChildrenCondition = { !isDotQuaOrSafeAccessOrPostfixExpression(it) } ) { isDotQuaOrSafeAccessOrPostfixExpression(it) && it.elementType != POSTFIX_EXPRESSION }.reversed() - if (listDot.size > 3) { - val without = listDot.filterIndexed { index, it -> + if (listOfDotQualifiedExpressions.size > configuration.maxCallsInOneLine) { + // corner case: fully-qualified expression names + val expressionsWithoutCornerCases = listOfDotQualifiedExpressions.filterIndexed { index, it -> val nodeBeforeDotOrSafeAccess = it.findChildByType(DOT)?.treePrev ?: it.findChildByType(SAFE_ACCESS)?.treePrev - val firstElem = it.firstChildNode - val isTextContainsParenthesized = isTextContainsFunctionCall(firstElem) - val isNotWhiteSpaceBeforeDotOrSafeAccessContainNewLine = nodeBeforeDotOrSafeAccess?.elementType != WHITE_SPACE || - (nodeBeforeDotOrSafeAccess.elementType != WHITE_SPACE && !nodeBeforeDotOrSafeAccess.textContains('\n')) - isTextContainsParenthesized && (index > 0) && isNotWhiteSpaceBeforeDotOrSafeAccessContainNewLine + val isTextContainsParenthesized = isTextContainsFunctionCall(it.firstChildNode) + + isTextContainsParenthesized && (index > 0) && + nodeBeforeDotOrSafeAccess?.elementType != WHITE_SPACE && + nodeBeforeDotOrSafeAccess?.textContains('\n') != true } - if (without.isNotEmpty()) { - WRONG_NEWLINES.warnAndFix(configRules, emitWarn, isFixMode, "wrong split long `dot qualified expression` or `safe access expression`", - node.startOffset, node) { - fixDotQualifiedExpression(without) + if (expressionsWithoutCornerCases.isNotEmpty()) { + WRONG_NEWLINES.warnAndFix( + configRules, emitWarn, isFixMode, + "Dot qualified expression chain (more than ${configuration.maxCallsInOneLine}) should be split with newlines", + node.startOffset, node + ) { + fixDotQualifiedExpression(expressionsWithoutCornerCases) } } } @@ -180,8 +185,8 @@ class NewlinesRule(configRules: List) : DiktatRule( /** * Return false, if you find parent with types in list else return true */ - private fun isNotFindParentNodeWithSpecificManyType(node: ASTNode, list: List): Boolean { - list.forEach { elem -> + private fun isNotFoundParentNodeOfTypes(node: ASTNode, types: List): Boolean { + types.forEach { elem -> node.findParentNodeWithSpecificType(elem)?.let { return false } @@ -228,7 +233,7 @@ class NewlinesRule(configRules: List) : DiktatRule( // at the beginning of the line. if (node.prevCodeSibling()?.isFollowedByNewline() == true) { WRONG_NEWLINES.warnAndFix(configRules, emitWarn, isFixMode, - "should break a line after and not before ${node.text}", node.startOffset, node) { + "need to break a line after and not before ${node.text}", node.startOffset, node) { node.run { treeParent.removeChild(treePrev) if (!isFollowedByNewline()) { @@ -263,9 +268,9 @@ class NewlinesRule(configRules: List) : DiktatRule( } if (isIncorrect || node.isElvisCorrect()) { val freeText = if (node.isInvalidCallsChain() || node.isElvisCorrect()) { - "should follow functional style at ${node.text}" + "need to follow functional style at ${node.text}" } else { - "should break a line before and not after ${node.text}" + "need to break a line before and not after ${node.text}" } WRONG_NEWLINES.warnAndFix(configRules, emitWarn, isFixMode, freeText, node.startOffset, node) { node.selfOrOperationReferenceParent().run { diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/files/NewlinesRuleWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/files/NewlinesRuleWarnTest.kt index 3008ad44ee..54152db4ae 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/files/NewlinesRuleWarnTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/files/NewlinesRuleWarnTest.kt @@ -26,10 +26,11 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { mapOf("maxCallsInOneLine" to "1")) ) private val ruleId = "$DIKTAT_RULE_SET_ID:${NewlinesRule.NAME_ID}" - private val dotQuaOrSafeAccessOrPostfixExpression = "${WRONG_NEWLINES.warnText()} wrong split long `dot qualified expression` or `safe access expression`" - private val shouldBreakAfter = "${WRONG_NEWLINES.warnText()} should break a line after and not before" - private val shouldBreakBefore = "${WRONG_NEWLINES.warnText()} should break a line before and not after" - private val functionalStyleWarn = "${WRONG_NEWLINES.warnText()} should follow functional style at" + private fun dotQualifiedExpr(maxCallsInOneLine: Int = 3) = "${WRONG_NEWLINES.warnText()} " + + "Dot qualified expression chain (more than ${maxCallsInOneLine}) should be split with newlines" + private val shouldBreakAfter = "${WRONG_NEWLINES.warnText()} need to break a line after and not before" + private val shouldBreakBefore = "${WRONG_NEWLINES.warnText()} need to break a line before and not after" + private val functionalStyleWarn = "${WRONG_NEWLINES.warnText()} need to follow functional style at" private val lparWarn = "${WRONG_NEWLINES.warnText()} opening parentheses should not be separated from constructor or function name" private val commaWarn = "${WRONG_NEWLINES.warnText()} newline should be placed only after comma" private val prevColonWarn = "${WRONG_NEWLINES.warnText()} newline shouldn't be placed before colon" @@ -263,7 +264,6 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { | }?.qux() |} """.trimMargin(), - LintError(2, 5, ruleId, dotQuaOrSafeAccessOrPostfixExpression, true), LintError(2, 11, ruleId, "$functionalStyleWarn .", true), LintError(3, 26, ruleId, "$functionalStyleWarn .", true), LintError(5, 10, ruleId, "$functionalStyleWarn ?.", true), @@ -668,7 +668,7 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { | } |} """.trimMargin(), - LintError(19, 20, ruleId, "${WRONG_NEWLINES.warnText()} should follow functional style at .", true), + LintError(19, 20, ruleId, "$functionalStyleWarn .", true), rulesConfigList = rulesConfigListShort ) } @@ -814,10 +814,9 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { | x.map().gre().few().qwe() |} """.trimMargin(), - LintError(2, 4, ruleId, dotQuaOrSafeAccessOrPostfixExpression, true), - LintError(3, 22, ruleId, "${WRONG_NEWLINES.warnText()} should follow functional style at .", true), - LintError(13, 4, ruleId, dotQuaOrSafeAccessOrPostfixExpression, true), - LintError(13, 23, ruleId, "${WRONG_NEWLINES.warnText()} should follow functional style at .", true) + LintError(3, 22, ruleId, "$functionalStyleWarn .", true), + LintError(13, 4, ruleId, dotQualifiedExpr(), true), + LintError(13, 23, ruleId, "$functionalStyleWarn .", true) ) } @@ -844,9 +843,7 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { | .few() |} """.trimMargin(), - LintError(2, 4, ruleId, dotQuaOrSafeAccessOrPostfixExpression, true), LintError(4, 22, ruleId, "$functionalStyleWarn .", true), - LintError(8, 4, ruleId, dotQuaOrSafeAccessOrPostfixExpression, true), LintError(9, 22, ruleId, "$functionalStyleWarn .", true) ) } @@ -972,7 +969,7 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { | x.gf().fge().qwe().fd() |} """.trimMargin(), - LintError(6, 4, ruleId, dotQuaOrSafeAccessOrPostfixExpression, true), + LintError(6, 4, ruleId, dotQualifiedExpr(), true), LintError(6, 22, ruleId, "$functionalStyleWarn .", true), rulesConfigList = rulesConfigList ) } @@ -1000,7 +997,7 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { | .qwe() |} """.trimMargin(), - LintError(9, 4, ruleId, dotQuaOrSafeAccessOrPostfixExpression, true), + LintError(9, 4, ruleId, dotQualifiedExpr(), true), LintError(9, 29, ruleId, "$functionalStyleWarn .", true) ) } @@ -1037,6 +1034,7 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { """.trimMargin(), LintError(2, 14, ruleId, "$functionalStyleWarn ?:", true), LintError(4, 8, ruleId, "$functionalStyleWarn ?:", true), + LintError(4, 11, ruleId, dotQualifiedExpr(1), true), LintError(4, 22, ruleId, "$functionalStyleWarn .", true), rulesConfigList = rulesConfigListShort ) @@ -1052,7 +1050,9 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { | a().b.c()!! |} """.trimMargin(), + LintError(2, 4, ruleId, dotQualifiedExpr(1), true), LintError(2, 11, ruleId, "$functionalStyleWarn .", true), + LintError(3, 4, ruleId, dotQualifiedExpr(1), true), LintError(3, 9, ruleId, "$functionalStyleWarn .", true), rulesConfigList = rulesConfigListShort ) @@ -1078,6 +1078,7 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { | .qwe() |} """.trimMargin(), + LintError(7, 11, ruleId, dotQualifiedExpr(1), true), LintError(7, 22, ruleId, "$functionalStyleWarn .", true), LintError(9, 15, ruleId, "$functionalStyleWarn ?:", true), LintError(10, 16, ruleId, "$functionalStyleWarn ?:", true), @@ -1138,12 +1139,24 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { | if(a().b().c()) {} |} """.trimMargin(), + LintError(2, 7, ruleId, dotQualifiedExpr(1), false), LintError(2, 14, ruleId, "${COMPLEX_EXPRESSION.warnText()} .", false), LintError(2, 14, ruleId, "$functionalStyleWarn .", true), rulesConfigList = rulesConfigListShort ) } + @Test + @Tags(Tag(WarningNames.WRONG_NEWLINES), Tag(WarningNames.COMPLEX_EXPRESSION)) + fun `should not split fully qualified names`() { + lintMethod( + """ + |val a = a.b.c.d.Java + """.trimMargin(), + rulesConfigList = rulesConfigListShort + ) + } + @Test @Tag(WarningNames.WRONG_NEWLINES) fun `not complaining on fun without return type`() { From 62d0bbcf39ac7fc5f965f968c0c60cd18d9ad927 Mon Sep 17 00:00:00 2001 From: Vfrolov Date: Fri, 3 Feb 2023 16:38:58 +0300 Subject: [PATCH 2/4] Merge branch 'master' into bugfix/configurable-dot-qualified --- .../cqfn/diktat/ruleset/rules/chapter3/files/NewlinesRule.kt | 1 - .../diktat/ruleset/chapter3/files/NewlinesRuleWarnTest.kt | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/files/NewlinesRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/files/NewlinesRule.kt index 58fb9207af..3824bf084b 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/files/NewlinesRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/files/NewlinesRule.kt @@ -26,7 +26,6 @@ import org.cqfn.diktat.ruleset.utils.isFollowedByNewline import org.cqfn.diktat.ruleset.utils.isGradleScript import org.cqfn.diktat.ruleset.utils.isSingleLineIfElse import org.cqfn.diktat.ruleset.utils.leaveOnlyOneNewLine -import org.cqfn.diktat.ruleset.utils.prettyPrint import com.pinterest.ktlint.core.ast.ElementType.ANDAND import com.pinterest.ktlint.core.ast.ElementType.ARROW diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/files/NewlinesRuleWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/files/NewlinesRuleWarnTest.kt index 8b64153f29..b3a09f0be0 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/files/NewlinesRuleWarnTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/files/NewlinesRuleWarnTest.kt @@ -26,8 +26,6 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { mapOf("maxCallsInOneLine" to "1")) ) private val ruleId = "$DIKTAT_RULE_SET_ID:${NewlinesRule.NAME_ID}" - private fun dotQualifiedExpr(maxCallsInOneLine: Int = 3) = "${WRONG_NEWLINES.warnText()} " + - "Dot qualified expression chain (more than ${maxCallsInOneLine}) should be split with newlines" private val shouldBreakAfter = "${WRONG_NEWLINES.warnText()} need to break a line after and not before" private val shouldBreakBefore = "${WRONG_NEWLINES.warnText()} need to break a line before and not after" private val functionalStyleWarn = "${WRONG_NEWLINES.warnText()} need to follow functional style at" @@ -37,6 +35,8 @@ class NewlinesRuleWarnTest : LintTestBase(::NewlinesRule) { private val lambdaWithArrowWarn = "${WRONG_NEWLINES.warnText()} in lambda with several lines in body newline should be placed after an arrow" private val lambdaWithoutArrowWarn = "${WRONG_NEWLINES.warnText()} in lambda with several lines in body newline should be placed after an opening brace" private val singleReturnWarn = "${WRONG_NEWLINES.warnText()} functions with single return statement should be simplified to expression body" + private fun dotQualifiedExpr(maxCallsInOneLine: Int = 3) = "${WRONG_NEWLINES.warnText()} " + + "Dot qualified expression chain (more than $maxCallsInOneLine) should be split with newlines" @Test @Tag(WarningNames.REDUNDANT_SEMICOLON) From e877f73f6cf08485253f55047bcdcc873821a216 Mon Sep 17 00:00:00 2001 From: Nariman Abdullin Date: Mon, 20 Mar 2023 14:32:08 +0300 Subject: [PATCH 3/4] ### What's done: updated test to new functionality --- .../LongDotQualifiedExpressionExpected.kt | 21 ++++++------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/diktat-rules/src/test/resources/test/paragraph3/newlines/LongDotQualifiedExpressionExpected.kt b/diktat-rules/src/test/resources/test/paragraph3/newlines/LongDotQualifiedExpressionExpected.kt index 49615088f2..76d115813a 100644 --- a/diktat-rules/src/test/resources/test/paragraph3/newlines/LongDotQualifiedExpressionExpected.kt +++ b/diktat-rules/src/test/resources/test/paragraph3/newlines/LongDotQualifiedExpressionExpected.kt @@ -9,15 +9,12 @@ val elem1 = firstArgumentDot()?.secondArgumentDot val elem2 = firstArgumentDot?.secondArgumentDot() ?.thirdArgumentDot - ?.fourthArgumentDot -?.fifthArgumentDot -?.sixthArgumentDot + ?.fourthArgumentDot?.fifthArgumentDot?.sixthArgumentDot val elem3 = firstArgumentDot?.secondArgumentDot?.thirdArgumentDot() ?.fourthArgumentDot - ?.fifthArgumentDot -?.sixthArgumentDot + ?.fifthArgumentDot?.sixthArgumentDot val elem4 = firstArgumentDot?.secondArgumentDot?.thirdArgumentDot + firstArgumentDot?.secondArgumentDot?.thirdArgumentDot?.fourthArgumentDot @@ -31,15 +28,12 @@ val elem5 = firstArgumentDot()!!.secondArgumentDot()!! val elem6 = firstArgumentDot!!.secondArgumentDot!!.thirdArgumentDot()!! - .fourthArgumentDot!! -.fifthArgumentDot()!! -.sixthArgumentDot + .fourthArgumentDot!!.fifthArgumentDot()!!.sixthArgumentDot val elem7 = firstArgumentDot!!.secondArgumentDot()!! .thirdArgumentDot!! -.fourthArgumentDot()!! - .fifthArgumentDot!! +.fourthArgumentDot()!!.fifthArgumentDot!! .sixthArgumentDot @@ -55,15 +49,12 @@ val elem9 = firstArgumentDot().secondArgumentDot val elem10 = firstArgumentDot.secondArgumentDot() .thirdArgumentDot - .fourthArgumentDot -.fifthArgumentDot() -.sixthArgumentDot + .fourthArgumentDot.fifthArgumentDot().sixthArgumentDot val elem11 = firstArgumentDot.secondArgumentDot.thirdArgumentDot() .fourthArgumentDot - .fifthArgumentDot -.sixthArgumentDot + .fifthArgumentDot.sixthArgumentDot val elem12 = firstArgumentDot.secondArgumentDot.thirdArgumentDot + firstArgumentDot.secondArgumentDot().thirdArgumentDot.fourthArgumentDot From 3fb2d7ccc670c9d4c3c1456a7f88c823e4f2dce8 Mon Sep 17 00:00:00 2001 From: Nariman Abdullin Date: Mon, 20 Mar 2023 14:37:27 +0300 Subject: [PATCH 4/4] ### What's done: updated test to new functionality --- .../paragraph3/newlines/LongDotQualifiedExpressionExpected.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/diktat-rules/src/test/resources/test/paragraph3/newlines/LongDotQualifiedExpressionExpected.kt b/diktat-rules/src/test/resources/test/paragraph3/newlines/LongDotQualifiedExpressionExpected.kt index 76d115813a..534363c9a8 100644 --- a/diktat-rules/src/test/resources/test/paragraph3/newlines/LongDotQualifiedExpressionExpected.kt +++ b/diktat-rules/src/test/resources/test/paragraph3/newlines/LongDotQualifiedExpressionExpected.kt @@ -33,8 +33,8 @@ val elem6 = firstArgumentDot!!.secondArgumentDot!!.thirdArgumentDot()!! val elem7 = firstArgumentDot!!.secondArgumentDot()!! .thirdArgumentDot!! -.fourthArgumentDot()!!.fifthArgumentDot!! -.sixthArgumentDot +.fourthArgumentDot()!! + .fifthArgumentDot!!.sixthArgumentDot val elem8 = firstArgumentDot()!!.secondArgumentDot!!.thirdArgumentDot + firstArgumentDot!!.secondArgumentDot!!.thirdArgumentDot!!.fourthArgumentDot