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 @@ -141,36 +141,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 +184,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 +232,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 +267,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,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"
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"
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)
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 @@ -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
)
}
Expand Down Expand Up @@ -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)
)
}

Expand All @@ -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)
)
}
Expand Down Expand Up @@ -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
)
}
Expand Down Expand Up @@ -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)
)
}
Expand Down Expand Up @@ -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
)
Expand All @@ -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
)
Expand All @@ -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),
Expand Down Expand Up @@ -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`() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down