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

Fix NPE in KdocFormatting rule #876

Merged
merged 4 commits into from
May 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@ import org.cqfn.diktat.ruleset.utils.getAllChildrenWithType
import org.cqfn.diktat.ruleset.utils.getFirstChildWithType
import org.cqfn.diktat.ruleset.utils.getIdentifierName
import org.cqfn.diktat.ruleset.utils.hasChildMatching
import org.cqfn.diktat.ruleset.utils.hasTrailingNewlineInTagBody
import org.cqfn.diktat.ruleset.utils.kDocTags
import org.cqfn.diktat.ruleset.utils.leaveOnlyOneNewLine
import org.cqfn.diktat.ruleset.utils.reversedChildren

import com.pinterest.ktlint.core.ast.ElementType
import com.pinterest.ktlint.core.ast.ElementType.KDOC
Expand All @@ -39,7 +41,6 @@ import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.CompositeElement
import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.LeafPsiElement
import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.PsiWhiteSpaceImpl
import org.jetbrains.kotlin.com.intellij.psi.tree.IElementType
import org.jetbrains.kotlin.com.intellij.psi.tree.TokenSet
import org.jetbrains.kotlin.kdoc.parser.KDocKnownTag
import org.jetbrains.kotlin.kdoc.psi.impl.KDocTag
import org.jetbrains.kotlin.lexer.KtTokens
Expand Down Expand Up @@ -238,11 +239,9 @@ class KdocFormatting(configRules: List<RulesConfig>) : DiktatRule(
previousTag.addChild(treePrev.clone() as ASTNode, null)
previousTag.addChild(this.clone() as ASTNode, null)
}
?: run {
firstBasicTag.node.applyToPrevSibling(KDOC_LEADING_ASTERISK) {
treeParent.addChild(treePrev.clone() as ASTNode, this)
treeParent.addChild(this.clone() as ASTNode, treePrev)
}
?: firstBasicTag.node.applyToPrevSibling(KDOC_LEADING_ASTERISK) {
treeParent.addChild(treePrev.clone() as ASTNode, this)
treeParent.addChild(this.clone() as ASTNode, treePrev)
}
} else {
firstBasicTag.node.apply {
Expand All @@ -256,27 +255,26 @@ class KdocFormatting(configRules: List<RulesConfig>) : DiktatRule(
}

private fun checkEmptyLinesBetweenBasicTags(basicTags: List<KDocTag>) {
val tagsWithRedundantEmptyLines = basicTags.dropLast(1).filterNot { tag ->
val tagsWithRedundantEmptyLines = basicTags.dropLast(1).filter { tag ->
val nextWhiteSpace = tag.node.nextSibling { it.elementType == WHITE_SPACE }
val noEmptyKdocLines = tag
.node
.getChildren(TokenSet.create(KDOC_LEADING_ASTERISK))
.filter { it.treeNext == null || it.treeNext.elementType == WHITE_SPACE }
.count() == 0
nextWhiteSpace?.text?.count { it == '\n' } == 1 && noEmptyKdocLines
// either there is a trailing blank line in tag's body OR there are empty lines right after this tag
tag.hasTrailingNewlineInTagBody() || nextWhiteSpace?.text?.count { it == '\n' } != 1
}

tagsWithRedundantEmptyLines.forEach { tag ->
KDOC_NO_NEWLINES_BETWEEN_BASIC_TAGS.warnAndFix(configRules, emitWarn, isFixMode,
"@${tag.name}", tag.startOffset, tag.node) {
tag.node.nextSibling { it.elementType == WHITE_SPACE }?.leaveOnlyOneNewLine()
// the first asterisk before tag is not included inside KDOC_TAG node
// we look for the second and take its previous which should be WHITE_SPACE with newline
tag
.node
.getAllChildrenWithType(KDOC_LEADING_ASTERISK)
.firstOrNull()
?.let { tag.node.removeRange(it.treePrev, null) }
if (tag.hasTrailingNewlineInTagBody()) {
// if there is a blank line in tag's body, we remove it and everything after it, so that the next white space is kept in place
// we look for the last LEADING_ASTERISK and take its previous node which should be WHITE_SPACE with newline
tag.node.reversedChildren()
.takeWhile { it.elementType == WHITE_SPACE || it.elementType == KDOC_LEADING_ASTERISK }
.firstOrNull { it.elementType == KDOC_LEADING_ASTERISK }
?.let { tag.node.removeRange(it.treePrev, null) }
} else {
// otherwise we remove redundant blank lines from white space node after tag
tag.node.nextSibling { it.elementType == WHITE_SPACE }?.leaveOnlyOneNewLine()
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,19 @@ fun ASTNode.isCorrect() = this.findAllDescendantsWithSpecificType(TokenType.ERRO
fun ASTNode.getAllChildrenWithType(elementType: IElementType): List<ASTNode> =
this.getChildren(null).filter { it.elementType == elementType }

/**
* Generates a sequence of this ASTNode's children in reversed order
*
* @return a reevrsed sequence of children
*/
fun ASTNode.reversedChildren(): Sequence<ASTNode> = sequence {
petertrr marked this conversation as resolved.
Show resolved Hide resolved
var node = lastChildNode
while (node != null) {
yield(node)
node = node.treePrev
}
}

/**
* Replaces the [beforeNode] of type [WHITE_SPACE] with the node with specified [text]
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package org.cqfn.diktat.ruleset.utils
import com.pinterest.ktlint.core.ast.ElementType
import com.pinterest.ktlint.core.ast.ElementType.KDOC_SECTION
import com.pinterest.ktlint.core.ast.ElementType.WHITE_SPACE
import com.pinterest.ktlint.core.ast.isWhiteSpaceWithNewline
import com.pinterest.ktlint.core.ast.prevSibling
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.CompositeElement
Expand All @@ -33,6 +34,23 @@ fun ASTNode.kDocTags(): List<KDocTag> {
fun Iterable<KDocTag>.hasKnownKdocTag(knownTag: KDocKnownTag): Boolean =
this.find { it.knownTag == knownTag } != null

/**
* Checks for trailing newlines in tag's body. Handles cases, when there is no leading asterisk on an empty line:
* ```
* * @param param
*
* * @return
* ```
* as well as usual simple cases.
*
* @return true if there is a trailing newline
*/
fun KDocTag.hasTrailingNewlineInTagBody() = node.lastChildNode.isWhiteSpaceWithNewline() ||
node.reversedChildren()
.takeWhile { it.elementType == WHITE_SPACE || it.elementType == ElementType.KDOC_LEADING_ASTERISK }
.firstOrNull { it.elementType == ElementType.KDOC_LEADING_ASTERISK }
?.takeIf { it.treeNext == null || it.treeNext.elementType == WHITE_SPACE } != null

/**
* This method inserts a new tag into KDoc before specified another tag, aligning it with the rest of this KDoc
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,12 @@ class DiktatSmokeTest : FixTestBase("test/smoke/src/main/kotlin",
)
}

@Test
@Tag("DiktatRuleSetProvider")
fun `regression - should correctly handle tags with empty lines`() {
fixAndCompare("KdocFormattingMultilineTagsExpected.kt", "KdocFormattingMultilineTagsTest.kt")
}

companion object {
private const val DEFAULT_CONFIG_PATH = "../diktat-analysis.yml"
private val unfixedLintErrors: MutableList<LintError> = mutableListOf()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package org.cqfn.diktat

/**
* @param bar lorem ipsum
*
* dolor sit amet
* @return
*/
fun foo1(bar: Bar): Baz {
// placeholder
}

/**
* @param bar lorem ipsum
*
* dolor sit amet
* @return
*/
fun foo2(bar: Bar): Baz {
// placeholder
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/**
* @param bar lorem ipsum
*
* dolor sit amet
*/
fun foo1(bar: Bar): Baz {
// placeholder
}

/**
* @param bar lorem ipsum
*
* dolor sit amet
*
*/
fun foo2(bar: Bar): Baz {
// placeholder
}