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.

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

Making dot qualified expression configurable #1420

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -141,36 +142,40 @@ class NewlinesRule(configRules: List<RulesConfig>) : 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 &&
Fixed Show fixed Hide fixed
nodeBeforeDotOrSafeAccess?.textContains('\n') != true
Fixed Show fixed Hide fixed
}
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)
}
}
}
Expand All @@ -180,8 +185,8 @@ class NewlinesRule(configRules: List<RulesConfig>) : DiktatRule(
/**
* Return false, if you find parent with types in list else return true
*/
private fun isNotFindParentNodeWithSpecificManyType(node: ASTNode, list: List<IElementType>): Boolean {
list.forEach { elem ->
private fun isNotFoundParentNodeOfTypes(node: ASTNode, types: List<IElementType>): Boolean {
types.forEach { elem ->
node.findParentNodeWithSpecificType(elem)?.let {
return false
}
Expand Down Expand Up @@ -228,7 +233,7 @@ class NewlinesRule(configRules: List<RulesConfig>) : 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()) {
Expand Down Expand Up @@ -263,9 +268,9 @@ class NewlinesRule(configRules: List<RulesConfig>) : 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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()} " +
Fixed Show fixed Hide fixed
"Dot qualified expression chain (more than ${maxCallsInOneLine}) should be split with newlines"
Fixed Show fixed Hide fixed
Fixed Show fixed Hide fixed
private val shouldBreakAfter = "${WRONG_NEWLINES.warnText()} need to break a line after and not before"
Fixed Show fixed Hide fixed
private val shouldBreakBefore = "${WRONG_NEWLINES.warnText()} need to break a line before and not after"
Fixed Show fixed Hide fixed
private val functionalStyleWarn = "${WRONG_NEWLINES.warnText()} need to follow functional style at"
Fixed Show fixed Hide fixed
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"
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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
)
}
Expand Down Expand Up @@ -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)
)
}

Expand All @@ -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)
)
}
Expand Down Expand Up @@ -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
)
}
Expand Down Expand Up @@ -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)
)
}
Expand Down Expand Up @@ -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
)
Expand All @@ -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
)
Expand All @@ -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),
Expand Down Expand Up @@ -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`() {
Expand Down