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

Properly indent comments inside multi-line Elvis expressions #1545

Merged
merged 7 commits into from
Nov 7, 2022
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ class SmartCastRule(configRules: List<RulesConfig>) : DiktatRule(
it.isLocal &&
it.hasInitializer() &&
it.name?.equals(getReferencedName())
?: false
?: false
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ class CompactInitialization(configRules: List<RulesConfig>) : DiktatRule(
(it.left as? KtDotQualifiedExpression)?.run {
(receiverExpression as? KtNameReferenceExpression)?.getReferencedName() == propertyName
}
?: false
?: false
}
.map {
// collect as an assignment associated with assigned field name
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,14 +97,14 @@ class DataClassesRule(configRules: List<RulesConfig>) : DiktatRule(
.none { it.elementType in badModifiers } &&
classBody?.getAllChildrenWithType(FUN)
?.isEmpty()
?: false &&
?: false &&
0x6675636b796f75676974687562 marked this conversation as resolved.
Show resolved Hide resolved
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
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,15 @@ 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
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
Expand All @@ -36,20 +40,25 @@ 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.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
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
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
Expand Down Expand Up @@ -824,6 +833,66 @@ 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 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 {
Fixed Show fixed Hide fixed
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
*/
fun PsiElement.isLongStringTemplateEntry(): Boolean =
node.elementType == LONG_STRING_TEMPLATE_ENTRY

private fun <T> Sequence<T>.takeWhileInclusive(pred: (T) -> Boolean): Sequence<T> {
var shouldContinue = true
return takeWhile {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ fun KtNameReferenceExpression.findLocalDeclaration(): KtProperty? = parents
it.isLocal &&
it.hasInitializer() &&
it.name?.equals(getReferencedName())
?: false
?: false
}
}
.firstOrNull()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ 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
import org.cqfn.diktat.ruleset.utils.lastIndent

import com.pinterest.ktlint.core.ast.ElementType.ARROW
Expand All @@ -17,9 +21,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
Expand All @@ -31,8 +34,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
Expand All @@ -47,13 +49,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
Expand Down Expand Up @@ -201,20 +201,27 @@ 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
}

private fun ASTNode.isElvisReferenceOrCommentBeforeElvis(): Boolean =
isElvisOperationReference() ||
isCommentBefore(ASTNode::isElvisOperationReference)

private fun ASTNode.isFromStringTemplate(): Boolean =
hasParent(LONG_STRING_TEMPLATE_ENTRY)

Expand All @@ -224,17 +231,74 @@ 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()) {
return CheckResult.from(indentError.actual, indentError.expected +
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>.() -> 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`
Expand All @@ -251,10 +315,38 @@ internal class DotCallChecker(config: IndentationConfig) : CustomIndentationChec
* .third()
* ```
*/
val parentIndent = whiteSpace.run {
parents.takeWhile { it is KtDotQualifiedExpression || it is KtSafeQualifiedExpression }.lastOrNull() ?: this
}.parentIndent() ?: 0
val expectedIndent = parentIndent + indentIncrement
val parentIndent = (parentExpressions.matchOrNull() ?: whiteSpace).parentIndent()
?: 0

val expectedIndent = when {
/*-
* 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.
*/
node.isElvisReferenceOrCommentBeforeElvis() &&
parentExpressions.any { it.node.isBooleanExpression() } -> parentIndent

/*-
* 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
}

return CheckResult.from(indentError.actual, expectedIndent, true)
}
return null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<KtClassBody>(true)?.getAllSearchResults(this))
// searching on the file level
// searching on the file level
?: (this.getParentOfType<KtFile>(true)!!.getAllSearchResults(this))
}

Expand Down Expand Up @@ -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)
}
}
Expand Down
Loading