From 612390e2ac76d983993e67bb77318c636465af34 Mon Sep 17 00:00:00 2001 From: Andrey Shcheglov Date: Tue, 1 Nov 2022 16:19:55 +0300 Subject: [PATCH 1/7] Properly indent comments inside multi-line Elvis expressions ### What's done: * Now, EOL and block comments within multi-line Elvis expressions have the same indentation level as the immediately following Elvis operator (`?:`), e.g.: ```kotlin fun foo() { findProperty("dockerNetwork") as String? // https://docs.docker.com/compose/networking/ ?: "default" } ``` * This fixes #1532. --- .../cqfn/diktat/ruleset/utils/AstNodeUtils.kt | 38 +++++ .../ruleset/utils/indentation/Checkers.kt | 75 +++++++-- .../chapter3/spaces/IndentationRuleTest.kt | 147 ++++++++++++++++++ 3 files changed, 243 insertions(+), 17 deletions(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt index bf2ea5a8b1..a20cf479b6 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt @@ -24,7 +24,10 @@ import com.pinterest.ktlint.core.ast.ElementType.ANNOTATED_EXPRESSION import com.pinterest.ktlint.core.ast.ElementType.ANNOTATION_ENTRY import com.pinterest.ktlint.core.ast.ElementType.BINARY_EXPRESSION import com.pinterest.ktlint.core.ast.ElementType.BLOCK_COMMENT +import com.pinterest.ktlint.core.ast.ElementType.CALL_EXPRESSION import com.pinterest.ktlint.core.ast.ElementType.CONST_KEYWORD +import com.pinterest.ktlint.core.ast.ElementType.DOT +import com.pinterest.ktlint.core.ast.ElementType.ELVIS import com.pinterest.ktlint.core.ast.ElementType.EOL_COMMENT import com.pinterest.ktlint.core.ast.ElementType.EQ import com.pinterest.ktlint.core.ast.ElementType.FILE @@ -36,6 +39,7 @@ import com.pinterest.ktlint.core.ast.ElementType.KDOC import com.pinterest.ktlint.core.ast.ElementType.LAMBDA_EXPRESSION import com.pinterest.ktlint.core.ast.ElementType.LATEINIT_KEYWORD import com.pinterest.ktlint.core.ast.ElementType.LBRACE +import com.pinterest.ktlint.core.ast.ElementType.LONG_STRING_TEMPLATE_ENTRY import com.pinterest.ktlint.core.ast.ElementType.MODIFIER_LIST import com.pinterest.ktlint.core.ast.ElementType.OPERATION_REFERENCE import com.pinterest.ktlint.core.ast.ElementType.OVERRIDE_KEYWORD @@ -43,6 +47,8 @@ import com.pinterest.ktlint.core.ast.ElementType.PARENTHESIZED import com.pinterest.ktlint.core.ast.ElementType.PRIVATE_KEYWORD import com.pinterest.ktlint.core.ast.ElementType.PROTECTED_KEYWORD import com.pinterest.ktlint.core.ast.ElementType.PUBLIC_KEYWORD +import com.pinterest.ktlint.core.ast.ElementType.REFERENCE_EXPRESSION +import com.pinterest.ktlint.core.ast.ElementType.SAFE_ACCESS import com.pinterest.ktlint.core.ast.ElementType.WHITE_SPACE import com.pinterest.ktlint.core.ast.isLeaf import com.pinterest.ktlint.core.ast.isPartOfComment @@ -50,6 +56,7 @@ import com.pinterest.ktlint.core.ast.isRoot import com.pinterest.ktlint.core.ast.isWhiteSpace import com.pinterest.ktlint.core.ast.parent import org.jetbrains.kotlin.com.intellij.lang.ASTNode +import org.jetbrains.kotlin.com.intellij.psi.PsiElement import org.jetbrains.kotlin.com.intellij.psi.TokenType import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.LeafPsiElement import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.PsiWhiteSpaceImpl @@ -824,6 +831,37 @@ fun ASTNode.takeByChainOfTypes(vararg types: IElementType): ASTNode? { return node } +/** + * @return whether this node is a dot (`.`, `.?`) before a function call or a + * property reference. + * @since 1.2.4 + */ +fun ASTNode.isDotBeforeCallOrReference(): Boolean = + elementType in sequenceOf(DOT, SAFE_ACCESS) && + treeNext.elementType in sequenceOf(CALL_EXPRESSION, REFERENCE_EXPRESSION) + +/** + * @return whether this node is an _Elvis_ operation reference (i.e. an + * [OPERATION_REFERENCE] which holds [ELVIS] is its only child). + * @see OPERATION_REFERENCE + * @see ELVIS + * @since 1.2.4 + */ +fun ASTNode.isElvisOperationReference(): Boolean = + treeParent.elementType == BINARY_EXPRESSION && + elementType == OPERATION_REFERENCE && + run { + val children = children().toList() + children.size == 1 && children[0].elementType == ELVIS + } + +/** + * @return whether this PSI element is a long string template entry. + * @since 1.2.4 + */ +fun PsiElement.isLongStringTemplateEntry(): Boolean = + node.elementType == LONG_STRING_TEMPLATE_ENTRY + private fun Sequence.takeWhileInclusive(pred: (T) -> Boolean): Sequence { var shouldContinue = true return takeWhile { diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/indentation/Checkers.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/indentation/Checkers.kt index e869c7247f..ab61000b01 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/indentation/Checkers.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/indentation/Checkers.kt @@ -8,6 +8,9 @@ import org.cqfn.diktat.ruleset.rules.chapter3.files.IndentationAmount import org.cqfn.diktat.ruleset.rules.chapter3.files.IndentationAmount.SINGLE import org.cqfn.diktat.ruleset.rules.chapter3.files.IndentationError import org.cqfn.diktat.ruleset.utils.hasParent +import org.cqfn.diktat.ruleset.utils.isDotBeforeCallOrReference +import org.cqfn.diktat.ruleset.utils.isElvisOperationReference +import org.cqfn.diktat.ruleset.utils.isLongStringTemplateEntry import org.cqfn.diktat.ruleset.utils.lastIndent import com.pinterest.ktlint.core.ast.ElementType.ARROW @@ -17,9 +20,8 @@ import com.pinterest.ktlint.core.ast.ElementType.BINARY_EXPRESSION import com.pinterest.ktlint.core.ast.ElementType.BINARY_WITH_TYPE import com.pinterest.ktlint.core.ast.ElementType.BLOCK_COMMENT import com.pinterest.ktlint.core.ast.ElementType.BODY -import com.pinterest.ktlint.core.ast.ElementType.CALL_EXPRESSION import com.pinterest.ktlint.core.ast.ElementType.COLON -import com.pinterest.ktlint.core.ast.ElementType.DOT +import com.pinterest.ktlint.core.ast.ElementType.DOT_QUALIFIED_EXPRESSION import com.pinterest.ktlint.core.ast.ElementType.ELSE import com.pinterest.ktlint.core.ast.ElementType.ELVIS import com.pinterest.ktlint.core.ast.ElementType.EOL_COMMENT @@ -31,8 +33,7 @@ import com.pinterest.ktlint.core.ast.ElementType.KDOC_SECTION import com.pinterest.ktlint.core.ast.ElementType.LONG_STRING_TEMPLATE_ENTRY import com.pinterest.ktlint.core.ast.ElementType.LPAR import com.pinterest.ktlint.core.ast.ElementType.OPERATION_REFERENCE -import com.pinterest.ktlint.core.ast.ElementType.REFERENCE_EXPRESSION -import com.pinterest.ktlint.core.ast.ElementType.SAFE_ACCESS +import com.pinterest.ktlint.core.ast.ElementType.SAFE_ACCESS_EXPRESSION import com.pinterest.ktlint.core.ast.ElementType.SUPER_TYPE_LIST import com.pinterest.ktlint.core.ast.ElementType.THEN import com.pinterest.ktlint.core.ast.ElementType.VALUE_ARGUMENT @@ -47,13 +48,11 @@ import org.jetbrains.kotlin.com.intellij.lang.ASTNode import org.jetbrains.kotlin.com.intellij.psi.PsiElement import org.jetbrains.kotlin.com.intellij.psi.PsiWhiteSpace import org.jetbrains.kotlin.psi.KtBlockExpression -import org.jetbrains.kotlin.psi.KtDotQualifiedExpression import org.jetbrains.kotlin.psi.KtFile import org.jetbrains.kotlin.psi.KtIfExpression import org.jetbrains.kotlin.psi.KtLoopExpression import org.jetbrains.kotlin.psi.KtProperty import org.jetbrains.kotlin.psi.KtPropertyAccessor -import org.jetbrains.kotlin.psi.KtSafeQualifiedExpression import org.jetbrains.kotlin.psi.KtWhenEntry import org.jetbrains.kotlin.psi.psiUtil.parents import org.jetbrains.kotlin.psi.psiUtil.parentsWithSelf @@ -201,16 +200,19 @@ internal class SuperTypeListChecker(config: IndentationConfig) : CustomIndentati * Same is true for safe calls (`?.`) and elvis operator (`?:`). */ internal class DotCallChecker(config: IndentationConfig) : CustomIndentationChecker(config) { - private fun ASTNode.isDotBeforeCallOrReference() = elementType.let { it == DOT || it == SAFE_ACCESS } && - treeNext.elementType.let { it == CALL_EXPRESSION || it == REFERENCE_EXPRESSION } - - private fun ASTNode.isCommentBeforeDot(): Boolean { - if (elementType == EOL_COMMENT || elementType == BLOCK_COMMENT) { + /** + * @param nextNodePredicate the predicate which the next non-comment + * non-whitespace node should satisfy. + * @return `true` if this is a comment node which is immediately preceding + * the node specified by [nextNodePredicate]. + */ + private fun ASTNode.isCommentBefore(nextNodePredicate: ASTNode.() -> Boolean): Boolean { + if (elementType in sequenceOf(EOL_COMMENT, BLOCK_COMMENT)) { var nextNode: ASTNode? = treeNext - while (nextNode != null && (nextNode.elementType == WHITE_SPACE || nextNode.elementType == EOL_COMMENT)) { + while (nextNode != null && nextNode.elementType in sequenceOf(WHITE_SPACE, EOL_COMMENT)) { nextNode = nextNode.treeNext } - return nextNode?.isDotBeforeCallOrReference() ?: false + return nextNode?.nextNodePredicate() ?: false } return false } @@ -224,10 +226,18 @@ internal class DotCallChecker(config: IndentationConfig) : CustomIndentationChec .node .takeIf { nextNode -> (nextNode.isDotBeforeCallOrReference() || - nextNode.elementType == OPERATION_REFERENCE && nextNode.firstChildNode.elementType.let { type -> - type == ELVIS || type == IS_EXPRESSION || type == AS_KEYWORD || type == AS_SAFE - } || nextNode.isCommentBeforeDot()) && whiteSpace.parents.none { it.node.elementType == LONG_STRING_TEMPLATE_ENTRY } + nextNode.elementType == OPERATION_REFERENCE && + nextNode.firstChildNode.elementType in sequenceOf(ELVIS, IS_EXPRESSION, AS_KEYWORD, AS_SAFE) || + nextNode.isCommentBefore(ASTNode::isDotBeforeCallOrReference) || + nextNode.isCommentBefore(ASTNode::isElvisOperationReference)) && + whiteSpace.parents.none(PsiElement::isLongStringTemplateEntry) } + /*- + * Here, `node` is any of: + * + * - a `DOT` or a `SAFE_ACCESS`, + * - an `OPERATION_REFERENCE` with `ELVIS` as the only child, or + */ ?.let { node -> val indentIncrement = IndentationAmount.valueOf(configuration.extendedIndentBeforeDot) if (node.isFromStringTemplate()) { @@ -252,7 +262,38 @@ internal class DotCallChecker(config: IndentationConfig) : CustomIndentationChec * ``` */ val parentIndent = whiteSpace.run { - parents.takeWhile { it is KtDotQualifiedExpression || it is KtSafeQualifiedExpression }.lastOrNull() ?: this + /*- + * The list of parents of this whitespace node, + * nearest-to-farthest order + * (the farthest parent is the file node). + */ + parents.takeWhile { parent -> + val parentType = parent.node.elementType + + when { + /* + * #1532, 1.2.4+: if this is an Elvis operator + * (OPERATION_REFERENCE -> ELVIS), or an EOL or a + * block comment which immediately precedes this + * Elvis operator, then the indent of the parent + * binary expression should be used as a base for + * the increment. + */ + node.isElvisOperationReference() -> parentType == BINARY_EXPRESSION + node.isCommentBefore(ASTNode::isElvisOperationReference) -> parentType == BINARY_EXPRESSION + + /* + * Pre-1.2.4 behaviour, all other cases: the indent + * of the parent dot-qualified or safe-access + * expression should be used as a base for the + * increment. + */ + else -> parentType in sequenceOf( + DOT_QUALIFIED_EXPRESSION, + SAFE_ACCESS_EXPRESSION, + ) + } + }.lastOrNull() ?: this }.parentIndent() ?: 0 val expectedIndent = parentIndent + indentIncrement return CheckResult.from(indentError.actual, expectedIndent, true) diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/spaces/IndentationRuleTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/spaces/IndentationRuleTest.kt index b7931600f0..c18ed19233 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/spaces/IndentationRuleTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/spaces/IndentationRuleTest.kt @@ -2133,4 +2133,151 @@ class IndentationRuleTest { extendedIndentBeforeDot = TRUE)) fun `case 2`() = Unit } + + @Nested + @TestMethodOrder(NaturalDisplayName::class) + inner class `Comments inside binary expressions` { + @IndentationTest( + IndentedSourceCode( + """ + val x: Int = functionCall() + // This is a comment + ?: 42 + """), + singleConfiguration = true) + fun `case 1`() = Unit + + @IndentationTest( + IndentedSourceCode( + """ + val x: Int = functionCall() as Int? + // This is a comment + ?: 42 + """), + singleConfiguration = true) + fun `case 2`() = Unit + + @IndentationTest( + IndentedSourceCode( + """ + val x: Int = null as Int? + // This is a comment + ?: 42 + """), + singleConfiguration = true) + fun `case 3`() = Unit + + @IndentationTest( + IndentedSourceCode( + """ + val x: Int = 42 as Int? + // This is a comment + ?: 42 + """), + singleConfiguration = true) + fun `case 4`() = Unit + + @IndentationTest( + IndentedSourceCode( + """ + val x: Boolean = functionCall() + /* + * This is a block comment + */ + ?: true + """), + singleConfiguration = true) + fun `case 5`() = Unit + + @IndentationTest( + IndentedSourceCode( + """ + val x: Boolean = functionCall() as Boolean? + /* + * This is a block comment + */ + ?: true + """), + singleConfiguration = true) + fun `case 6`() = Unit + + @IndentationTest( + IndentedSourceCode( + """ + val x: Boolean = null as Boolean? + /* + * This is a block comment + */ + ?: true + """), + singleConfiguration = true) + fun `case 7`() = Unit + + @IndentationTest( + IndentedSourceCode( + """ + val x: Boolean = true as Boolean? + /* + * This is a block comment + */ + ?: true + """), + singleConfiguration = true) + fun `case 8`() = Unit + + @IndentationTest( + IndentedSourceCode( + """ + fun f(): String { + return functionCall() + // This is a comment + // This is the 2nd line of the comment + ?: "default value" + } + """), + singleConfiguration = true) + fun `case 9`() = Unit + + @IndentationTest( + IndentedSourceCode( + """ + fun f(): String { + return functionCall() as String? + // This is a comment + // This is the 2nd line of the comment + ?: "default value" + } + """), + singleConfiguration = true) + fun `case 10`() = Unit + + @IndentationTest( + IndentedSourceCode( + """ + fun f(): String { + return null as String? + // This is a comment + // This is the 2nd line of the comment + ?: "default value" + } + """), + singleConfiguration = true) + fun `case 11`() = Unit + + @IndentationTest( + IndentedSourceCode( + """ + fun f(): String { + return "return value" as String? + // This is a comment + // This is the 2nd line of the comment + ?: "default value" + // This is a comment + // This is the 2nd line of the comment + ?: "unreachable code" + } + """), + singleConfiguration = true) + fun `case 12`() = Unit + } } From 676e763403d428e8ef5d625ae7cc925c85927b46 Mon Sep 17 00:00:00 2001 From: Andrey Shcheglov Date: Tue, 1 Nov 2022 17:16:45 +0300 Subject: [PATCH 2/7] Make Diktat happy --- .../org/cqfn/diktat/ruleset/utils/search/VariablesSearch.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesSearch.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesSearch.kt index b9ade13e28..5eb082e16c 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesSearch.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesSearch.kt @@ -65,9 +65,9 @@ abstract class VariablesSearch(val node: ASTNode, .let { declarationScope -> // searching in the scope with declaration (in the context) declarationScope?.getAllSearchResults(this) - // searching on the class level in class body + // searching on the class level in class body ?: (this.getParentOfType(true)?.getAllSearchResults(this)) - // searching on the file level + // searching on the file level ?: (this.getParentOfType(true)!!.getAllSearchResults(this)) } From fd3440e82e83902f4998468777248b887c47b6a6 Mon Sep 17 00:00:00 2001 From: Andrey Shcheglov Date: Tue, 1 Nov 2022 20:41:39 +0300 Subject: [PATCH 3/7] Don't indent Elvis expressions enclosed into binary expressions --- .../cqfn/diktat/ruleset/utils/AstNodeUtils.kt | 31 +++ .../ruleset/utils/indentation/Checkers.kt | 113 +++++++--- .../chapter3/spaces/IndentationRuleTest.kt | 198 ++++++++++++++++++ 3 files changed, 311 insertions(+), 31 deletions(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt index a20cf479b6..abce5997e5 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt @@ -20,6 +20,7 @@ import org.cqfn.diktat.ruleset.rules.chapter1.PackageNaming import com.pinterest.ktlint.core.KtLint import com.pinterest.ktlint.core.ast.ElementType +import com.pinterest.ktlint.core.ast.ElementType.ANDAND import com.pinterest.ktlint.core.ast.ElementType.ANNOTATED_EXPRESSION import com.pinterest.ktlint.core.ast.ElementType.ANNOTATION_ENTRY import com.pinterest.ktlint.core.ast.ElementType.BINARY_EXPRESSION @@ -42,6 +43,7 @@ import com.pinterest.ktlint.core.ast.ElementType.LBRACE import com.pinterest.ktlint.core.ast.ElementType.LONG_STRING_TEMPLATE_ENTRY import com.pinterest.ktlint.core.ast.ElementType.MODIFIER_LIST import com.pinterest.ktlint.core.ast.ElementType.OPERATION_REFERENCE +import com.pinterest.ktlint.core.ast.ElementType.OROR import com.pinterest.ktlint.core.ast.ElementType.OVERRIDE_KEYWORD import com.pinterest.ktlint.core.ast.ElementType.PARENTHESIZED import com.pinterest.ktlint.core.ast.ElementType.PRIVATE_KEYWORD @@ -855,6 +857,35 @@ fun ASTNode.isElvisOperationReference(): Boolean = children.size == 1 && children[0].elementType == ELVIS } +/** + * @return whether this node is a whitespace or a comment. + * @since 1.2.4 + */ +fun ASTNode.isWhiteSpaceOrComment(): Boolean = + isWhiteSpace() || elementType in commentType + +/** + * @return whether this node is a boolean expression (i.e. a [BINARY_EXPRESSION] + * holding an [OPERATION_REFERENCE] which, in turn, holds either [ANDAND] or + * [OROR] is its only child). + * @see BINARY_EXPRESSION + * @see OPERATION_REFERENCE + * @see ANDAND + * @see OROR + * @since 1.2.4 + */ +fun ASTNode.isBooleanExpression(): Boolean = + elementType == BINARY_EXPRESSION && run { + val operationAndArgs = children().filterNot(ASTNode::isWhiteSpaceOrComment).toList() + operationAndArgs.size == 3 && run { + val operationReference = operationAndArgs[1] + operationReference.elementType == OPERATION_REFERENCE && run { + val operations = operationReference.children().toList() + operations.size == 1 && operations[0].elementType in sequenceOf(ANDAND, OROR) + } + } + } + /** * @return whether this PSI element is a long string template entry. * @since 1.2.4 diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/indentation/Checkers.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/indentation/Checkers.kt index ab61000b01..db6f95e5af 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/indentation/Checkers.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/indentation/Checkers.kt @@ -8,6 +8,7 @@ import org.cqfn.diktat.ruleset.rules.chapter3.files.IndentationAmount import org.cqfn.diktat.ruleset.rules.chapter3.files.IndentationAmount.SINGLE import org.cqfn.diktat.ruleset.rules.chapter3.files.IndentationError import org.cqfn.diktat.ruleset.utils.hasParent +import org.cqfn.diktat.ruleset.utils.isBooleanExpression import org.cqfn.diktat.ruleset.utils.isDotBeforeCallOrReference import org.cqfn.diktat.ruleset.utils.isElvisOperationReference import org.cqfn.diktat.ruleset.utils.isLongStringTemplateEntry @@ -217,6 +218,10 @@ internal class DotCallChecker(config: IndentationConfig) : CustomIndentationChec return false } + private fun ASTNode.isElvisReferenceOrCommentBeforeElvis(): Boolean = + isElvisOperationReference() || + isCommentBefore(ASTNode::isElvisOperationReference) + private fun ASTNode.isFromStringTemplate(): Boolean = hasParent(LONG_STRING_TEMPLATE_ENTRY) @@ -245,6 +250,55 @@ internal class DotCallChecker(config: IndentationConfig) : CustomIndentationChec indentIncrement, true) } + /*- + * The list of immediate parents of this whitespace node, + * nearest-to-farthest order + * (the farthest parent is the file node). + */ + val parentExpressions = whiteSpace.parents.takeWhile { parent -> + val parentType = parent.node.elementType + + when { + /* + * #1532, 1.2.4+: if this is an Elvis operator + * (OPERATION_REFERENCE -> ELVIS), or an EOL or a + * block comment which immediately precedes this + * Elvis operator, then the indent of the parent + * binary expression should be used as a base for + * the increment. + */ + node.isElvisReferenceOrCommentBeforeElvis() -> parentType == BINARY_EXPRESSION + + /* + * Pre-1.2.4 behaviour, all other cases: the indent + * of the parent dot-qualified or safe-access + * expression should be used as a base for the + * increment. + */ + else -> parentType in sequenceOf( + DOT_QUALIFIED_EXPRESSION, + SAFE_ACCESS_EXPRESSION, + ) + } + }.toList() + + /* + * Selects from the matching parent nodes. + */ + val matchOrNull: Iterable.() -> PsiElement? = { + when { + /* + * Selects nearest. + */ + node.isElvisReferenceOrCommentBeforeElvis() -> firstOrNull() + + /* + * Selects farthest. + */ + else -> lastOrNull() + } + } + // we need to get indent before the first expression in calls chain /*- * If the parent indent (the one before a `DOT_QUALIFIED_EXPRESSION` @@ -261,41 +315,38 @@ internal class DotCallChecker(config: IndentationConfig) : CustomIndentationChec * .third() * ``` */ - val parentIndent = whiteSpace.run { + val parentIndent = (parentExpressions.matchOrNull() ?: whiteSpace).parentIndent() + ?: 0 + + val expectedIndent = when { /*- - * The list of parents of this whitespace node, - * nearest-to-farthest order - * (the farthest parent is the file node). + * Don't indent Elvis expressions (and the corresponding comments) + * which are nested inside boolean expressions: + * + * ```kotlin + * val x = true && + * "" + * ?.isEmpty() + * ?: true + * ``` + * + * This is a special case, and this is how IDEA formats source code. */ - parents.takeWhile { parent -> - val parentType = parent.node.elementType + node.isElvisReferenceOrCommentBeforeElvis() && + parentExpressions.any { it.node.isBooleanExpression() } -> parentIndent - when { - /* - * #1532, 1.2.4+: if this is an Elvis operator - * (OPERATION_REFERENCE -> ELVIS), or an EOL or a - * block comment which immediately precedes this - * Elvis operator, then the indent of the parent - * binary expression should be used as a base for - * the increment. - */ - node.isElvisOperationReference() -> parentType == BINARY_EXPRESSION - node.isCommentBefore(ASTNode::isElvisOperationReference) -> parentType == BINARY_EXPRESSION + /*- + * All other cases (dot-qualified, safe-access, Elvis). + * Expression parts are indented regularly, e.g.: + * + * ```kotlin + * val a = null as Boolean? + * ?: true + * ``` + */ + else -> parentIndent + indentIncrement + } - /* - * Pre-1.2.4 behaviour, all other cases: the indent - * of the parent dot-qualified or safe-access - * expression should be used as a base for the - * increment. - */ - else -> parentType in sequenceOf( - DOT_QUALIFIED_EXPRESSION, - SAFE_ACCESS_EXPRESSION, - ) - } - }.lastOrNull() ?: this - }.parentIndent() ?: 0 - val expectedIndent = parentIndent + indentIncrement return CheckResult.from(indentError.actual, expectedIndent, true) } return null diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/spaces/IndentationRuleTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/spaces/IndentationRuleTest.kt index c18ed19233..be7dedd305 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/spaces/IndentationRuleTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/spaces/IndentationRuleTest.kt @@ -2280,4 +2280,202 @@ class IndentationRuleTest { singleConfiguration = true) fun `case 12`() = Unit } + + @Nested + @TestMethodOrder(NaturalDisplayName::class) + inner class `Multi-line Elvis expressions` { + @IndentationTest( + IndentedSourceCode( + """ + val elvisExpressionInsideBinaryExpressionA = true && + "" + ?.trim() + ?.trim() + ?.trim() + ?.isEmpty() + ?: true + """), + singleConfiguration = true) + fun `case 1`() = Unit + + @IndentationTest( + IndentedSourceCode( + """ + val elvisExpressionInsideBinaryExpressionB = false || + "" + .trim() + .trim() + .trim() + .isEmpty() + ?: true + """), + singleConfiguration = true) + fun `case 2`() = Unit + + @IndentationTest( + IndentedSourceCode( + """ + val elvisExpressionInsideBinaryExpressionC = true && + null as Boolean? + ?: true + """), + singleConfiguration = true) + fun `case 3`() = Unit + + @IndentationTest( + IndentedSourceCode( + """ + val elvisExpressionInsideBinaryExpressionD = false || + (null as Boolean?) + ?: true + """), + singleConfiguration = true) + fun `case 4`() = Unit + + @IndentationTest( + IndentedSourceCode( + """ + val elvisExpressionInsideBinaryExpressionE = true && + (42 as? Boolean) + ?: true + """), + singleConfiguration = true) + fun `case 5`() = Unit + + /** + * _Elvis_ after a _safe-access_ expression should have the same + * indentation level as the previous function calls. + */ + @IndentationTest( + IndentedSourceCode( + """ + val elvisAfterSafeAccess = "" + ?.trim() + ?.trim() + ?.trim() + ?.isEmpty() + ?: "" + """), + singleConfiguration = true) + fun `case 6`() = Unit + + /** + * _Elvis_ after a _dot-qualified_ expression should have the same + * indentation level as the previous function calls. + */ + @IndentationTest( + IndentedSourceCode( + """ + val elvisAfterDotQualified = "" + .trim() + .trim() + .trim() + .isEmpty() + ?: "" + """), + singleConfiguration = true) + fun `case 7`() = Unit + + @IndentationTest( + IndentedSourceCode( + """ + fun f(): Boolean { + return list.getChildren(null) + .none { it.elementType in badModifiers } && + classBody?.getAllChildrenWithType(FUN) + ?.isEmpty() + ?: false && + getFirstChildWithType(SUPER_TYPE_LIST) == null + } + """), + singleConfiguration = true) + fun `case 8`() = Unit + + @IndentationTest( + IndentedSourceCode( + """ + fun f(): Boolean { + return classBody?.getFirstChildWithType(FUN) == null && + getFirstChildWithType(SUPER_TYPE_LIST) == null && + // if there is any prop with logic in accessor then don't recommend to convert class to data class + classBody?.let(::areGoodProps) + ?: true + } + """), + singleConfiguration = true) + fun `case 9`() = Unit + + @IndentationTest( + IndentedSourceCode( + """ + fun f(): Boolean { + return block.getChildrenOfType() + .any { it.nameAsName == property.nameAsName && expression.node.isGoingAfter(it.node) } || + block.parent + .let { it as? KtFunctionLiteral } + ?.valueParameters + ?.any { it.nameAsName == property.nameAsName } + ?: false + } + """), + singleConfiguration = true) + fun `case 10`() = Unit + + @IndentationTest( + IndentedSourceCode( + """ + fun f(): Boolean { + return blockExpression + .statements + .takeWhile { !it.isAncestor(this, true) } + .mapNotNull { it as? KtProperty } + .find { + it.isLocal && + it.hasInitializer() && + it.name?.equals(getReferencedName()) + ?: false + } + } + """), + singleConfiguration = true) + fun `case 11`() = Unit + + @IndentationTest( + IndentedSourceCode( + """ + fun f(): Any { + return siblings(forward = true, withItself = false) + .filterNot { it.node.isPartOfComment() || it is PsiWhiteSpace } + .takeWhile { + // statements like `name.field = value` where name == propertyName + it is KtBinaryExpression && it.node.findChildByType(OPERATION_REFERENCE)?.findChildByType(EQ) != null && + (it.left as? KtDotQualifiedExpression)?.run { + (receiverExpression as? KtNameReferenceExpression)?.getReferencedName() == propertyName + } + ?: false + } + } + """), + singleConfiguration = true) + fun `case 12`() = Unit + + @IndentationTest( + IndentedSourceCode( + """ + fun f(): Any { + return blockExpression + .statements + .takeWhile { !it.isAncestor(this, true) } + .mapNotNull { it as? KtProperty } + .find { + it.isLocal && + it.hasInitializer() && + it.name?.equals(getReferencedName()) + ?: false + } + } + """), + singleConfiguration = true) + fun `case 13`() = Unit + } } From 427bb422fa07298c46f9199bf7f72089fba34a4c Mon Sep 17 00:00:00 2001 From: Andrey Shcheglov Date: Tue, 1 Nov 2022 21:00:30 +0300 Subject: [PATCH 4/7] Update test data --- .../src/main/kotlin/ManyLineTransformInLongLineExpected.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/diktat-ruleset/src/test/resources/test/smoke/src/main/kotlin/ManyLineTransformInLongLineExpected.kt b/diktat-ruleset/src/test/resources/test/smoke/src/main/kotlin/ManyLineTransformInLongLineExpected.kt index bfde8a2237..d6c5617213 100644 --- a/diktat-ruleset/src/test/resources/test/smoke/src/main/kotlin/ManyLineTransformInLongLineExpected.kt +++ b/diktat-ruleset/src/test/resources/test/smoke/src/main/kotlin/ManyLineTransformInLongLineExpected.kt @@ -2,9 +2,9 @@ package org.cqfn.diktat fun foo() { (1 or 2 or 3 or 4 or 5 or 6 or 7 or 8 or 9 or 10 or 11 or 12 or 13 or 14 or 15 or 16 or 17 or 18 or 19 or 20 or 21 or 22 or 23 or 24 or 25 or 26 or 27 or 28 or 29 or 30 or 31 - ?: 32 or 33 or 34 or 35 or 36 or 37 or 38 or 39 ?: 40 or 41 or 42 or 43 or 44 or 45 or 46 or 47 or 48 or 49 or 50 or 51 or 52 or 53 + 54 or 55 or 56 or 57 or 58 or 59 - ?: 60 or 61 or 62 or 63 or 64 or 65 or 66 - 67 or 68 or 69 or 70 or 1 + 2 or 3 or 4 or 5 or 6 or 7 or 8 or 9 or 10 or 11 or 12 or 13 or 14 or 15 or 16 or 17 or 18 + 19 - - 20 or 21 or 22 or 23 or 24 or 25 or 26 or 27 or 28 or 29 or 30 or 31 or 32 or 33 or 34 or 35 or 36 or 37 or 38 or 39 or 40 or 41 || 42 or 43 or 44 or 45 or 46 or 47 && 48 or 49 || + ?: 32 or 33 or 34 or 35 or 36 or 37 or 38 or 39 ?: 40 or 41 or 42 or 43 or 44 or 45 or 46 or 47 or 48 or 49 or 50 or 51 or 52 or 53 + 54 or 55 or 56 or 57 or 58 or 59 + ?: 60 or 61 or 62 or 63 or 64 or 65 or 66 - 67 or 68 or 69 or 70 or 1 + 2 or 3 or 4 or 5 or 6 or 7 or 8 or 9 or 10 or 11 or 12 or 13 or 14 or 15 or 16 or 17 or 18 + 19 - + 20 or 21 or 22 or 23 or 24 or 25 or 26 or 27 or 28 or 29 or 30 or 31 or 32 or 33 or 34 or 35 or 36 or 37 or 38 or 39 or 40 or 41 || 42 or 43 or 44 or 45 or 46 or 47 && 48 or 49 || 50 or 51 or 52 or 53 or 54 or 55 or 56 or 57 or 58 or 59 or 60 or 61 or 62 or 63 or 64 or 65 or 66 or 67 or 68 or 69 or 70 or 1 or 2 or 3 or 4 or 5 or 6 or 7 or 8 or 9 or 10 or 11 or 12 or 13 or 14 or 15 or 16 or 17 or 18 or 19 or 20 or 21 or 22 or 23 or 24 or 25 or 26 or 27 or 28 or 29 or 30 or 31 or 32 or 33 or 34 or 35 or 36 or 37 or 38 or 39 or 40 or 41 or 42 or 43 or 44 or 45 or 46 or 47 or 48 or 49 or 50 or 51 or 52 or 53 or 54 or 55 or 56 or 57 or 58 or 59 or 60 or 61 or 62 or 63 or 64 or 65 or 66 or 67 or 68 or 69 or 70) From d9bad9fb97f23bb0fca91f60a806d4009f95aac9 Mon Sep 17 00:00:00 2001 From: Andrey Shcheglov Date: Tue, 1 Nov 2022 21:09:13 +0300 Subject: [PATCH 5/7] Reformat the code --- .../org/cqfn/diktat/ruleset/rules/chapter4/SmartCastRule.kt | 2 +- .../ruleset/rules/chapter6/classes/CompactInitialization.kt | 2 +- .../diktat/ruleset/rules/chapter6/classes/DataClassesRule.kt | 4 ++-- .../src/main/kotlin/org/cqfn/diktat/ruleset/utils/PsiUtils.kt | 2 +- .../org/cqfn/diktat/ruleset/utils/search/VariablesSearch.kt | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter4/SmartCastRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter4/SmartCastRule.kt index 943fc6678d..d6cd25d394 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter4/SmartCastRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter4/SmartCastRule.kt @@ -225,7 +225,7 @@ class SmartCastRule(configRules: List) : DiktatRule( it.isLocal && it.hasInitializer() && it.name?.equals(getReferencedName()) - ?: false + ?: false } } diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter6/classes/CompactInitialization.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter6/classes/CompactInitialization.kt index b57627a447..192ce0e612 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter6/classes/CompactInitialization.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter6/classes/CompactInitialization.kt @@ -72,7 +72,7 @@ class CompactInitialization(configRules: List) : DiktatRule( (it.left as? KtDotQualifiedExpression)?.run { (receiverExpression as? KtNameReferenceExpression)?.getReferencedName() == propertyName } - ?: false + ?: false } .map { // collect as an assignment associated with assigned field name diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter6/classes/DataClassesRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter6/classes/DataClassesRule.kt index 02cd7b3af4..0919c61ba5 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter6/classes/DataClassesRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter6/classes/DataClassesRule.kt @@ -97,14 +97,14 @@ class DataClassesRule(configRules: List) : DiktatRule( .none { it.elementType in badModifiers } && classBody?.getAllChildrenWithType(FUN) ?.isEmpty() - ?: false && + ?: false && getFirstChildWithType(SUPER_TYPE_LIST) == null } return classBody?.getFirstChildWithType(FUN) == null && getFirstChildWithType(SUPER_TYPE_LIST) == null && // if there is any prop with logic in accessor then don't recommend to convert class to data class classBody?.let(::areGoodProps) - ?: true + ?: true } /** diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/PsiUtils.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/PsiUtils.kt index 58dffd1fc9..2c24fe2975 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/PsiUtils.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/PsiUtils.kt @@ -67,7 +67,7 @@ fun KtNameReferenceExpression.findLocalDeclaration(): KtProperty? = parents it.isLocal && it.hasInitializer() && it.name?.equals(getReferencedName()) - ?: false + ?: false } } .firstOrNull() diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesSearch.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesSearch.kt index 5eb082e16c..0f5f811f6d 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesSearch.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesSearch.kt @@ -107,7 +107,7 @@ abstract class VariablesSearch(val node: ASTNode, .let { it as? KtFunctionLiteral } ?.valueParameters ?.any { it.nameAsName == property.nameAsName } - ?: false + ?: false // FixMe: also see very strange behavior of Kotlin in tests (disabled) } } From febe6761946fe138ca5567eea3bbdf88e434ebab Mon Sep 17 00:00:00 2001 From: Andrey Shcheglov Date: Tue, 1 Nov 2022 21:17:58 +0300 Subject: [PATCH 6/7] Make Diktat and Detekt happy --- .../kotlin/org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt | 4 ++++ .../org/cqfn/diktat/ruleset/utils/indentation/Checkers.kt | 5 ++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt index abce5997e5..b396daae2d 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt @@ -874,6 +874,10 @@ fun ASTNode.isWhiteSpaceOrComment(): Boolean = * @see OROR * @since 1.2.4 */ +@Suppress( + "MAGIC_NUMBER", + "MagicNumber", +) fun ASTNode.isBooleanExpression(): Boolean = elementType == BINARY_EXPRESSION && run { val operationAndArgs = children().filterNot(ASTNode::isWhiteSpaceOrComment).toList() diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/indentation/Checkers.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/indentation/Checkers.kt index db6f95e5af..c474302224 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/indentation/Checkers.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/indentation/Checkers.kt @@ -225,7 +225,10 @@ internal class DotCallChecker(config: IndentationConfig) : CustomIndentationChec private fun ASTNode.isFromStringTemplate(): Boolean = hasParent(LONG_STRING_TEMPLATE_ENTRY) - @Suppress("ComplexMethod") + @Suppress( + "ComplexMethod", + "TOO_LONG_FUNCTION", + ) override fun checkNode(whiteSpace: PsiWhiteSpace, indentError: IndentationError): CheckResult? { whiteSpace.nextSibling .node From 1e36180c89f8923e39a1e51e60637e7e6d05a753 Mon Sep 17 00:00:00 2001 From: Andrey Shcheglov Date: Wed, 2 Nov 2022 02:21:58 +0300 Subject: [PATCH 7/7] Make Diktat happy --- .../rules/chapter4/calculations/AccurateCalculationsRule.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter4/calculations/AccurateCalculationsRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter4/calculations/AccurateCalculationsRule.kt index f406a1fa05..12d207d849 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter4/calculations/AccurateCalculationsRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter4/calculations/AccurateCalculationsRule.kt @@ -55,7 +55,7 @@ class AccurateCalculationsRule(configRules: List) : DiktatRule( .singleOrNull() ?.let { it.getArgumentExpression() as? KtCallExpression } ?.isAbsOfFloat() - ?: false || + ?: false || (receiverExpression as? KtCallExpression).isAbsOfFloat() } ?: false