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 01624e603a..df51b8b5f4 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 @@ -141,36 +141,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 +184,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 +232,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 +267,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 6df9da27dd..f1d0f092ea 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,16 +26,17 @@ 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 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" 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) @@ -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), @@ -680,7 +680,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 ) } @@ -826,10 +826,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) ) } @@ -856,9 +855,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) ) } @@ -984,7 +981,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 ) } @@ -1012,7 +1009,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) ) } @@ -1049,6 +1046,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 ) @@ -1064,7 +1062,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 ) @@ -1090,6 +1090,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), @@ -1150,12 +1151,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`() { 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..534363c9a8 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,16 +28,13 @@ val elem5 = firstArgumentDot()!!.secondArgumentDot()!! val elem6 = firstArgumentDot!!.secondArgumentDot!!.thirdArgumentDot()!! - .fourthArgumentDot!! -.fifthArgumentDot()!! -.sixthArgumentDot + .fourthArgumentDot!!.fifthArgumentDot()!!.sixthArgumentDot val elem7 = firstArgumentDot!!.secondArgumentDot()!! .thirdArgumentDot!! .fourthArgumentDot()!! - .fifthArgumentDot!! -.sixthArgumentDot + .fifthArgumentDot!!.sixthArgumentDot val elem8 = firstArgumentDot()!!.secondArgumentDot!!.thirdArgumentDot + firstArgumentDot!!.secondArgumentDot!!.thirdArgumentDot!!.fourthArgumentDot @@ -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