From 6de6a62cd3eb8e94f596c4145aff5f602c53d6d3 Mon Sep 17 00:00:00 2001 From: Peter Trifanov Date: Tue, 11 May 2021 18:09:59 +0300 Subject: [PATCH] Fix NPE in KdocFormatting rule (#876) ### What's done: * Updated logic * Added tests --- .../rules/chapter2/kdoc/KdocFormatting.kt | 40 +++++++++---------- .../cqfn/diktat/ruleset/utils/AstNodeUtils.kt | 13 ++++++ .../cqfn/diktat/ruleset/utils/KdocUtils.kt | 18 +++++++++ .../diktat/ruleset/smoke/DiktatSmokeTest.kt | 6 +++ .../KdocFormattingMultilineTagsExpected.kt | 22 ++++++++++ .../kotlin/KdocFormattingMultilineTagsTest.kt | 18 +++++++++ 6 files changed, 96 insertions(+), 21 deletions(-) create mode 100644 diktat-rules/src/test/resources/test/smoke/src/main/kotlin/KdocFormattingMultilineTagsExpected.kt create mode 100644 diktat-rules/src/test/resources/test/smoke/src/main/kotlin/KdocFormattingMultilineTagsTest.kt diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter2/kdoc/KdocFormatting.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter2/kdoc/KdocFormatting.kt index 0fabddb676..0425cbfaad 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter2/kdoc/KdocFormatting.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter2/kdoc/KdocFormatting.kt @@ -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 @@ -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 @@ -238,11 +239,9 @@ class KdocFormatting(configRules: List) : 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 { @@ -256,27 +255,26 @@ class KdocFormatting(configRules: List) : DiktatRule( } private fun checkEmptyLinesBetweenBasicTags(basicTags: List) { - 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() + } } } } 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 6b6359f8b5..e1f292178e 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 @@ -115,6 +115,19 @@ fun ASTNode.isCorrect() = this.findAllDescendantsWithSpecificType(TokenType.ERRO fun ASTNode.getAllChildrenWithType(elementType: IElementType): List = 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 = sequence { + var node = lastChildNode + while (node != null) { + yield(node) + node = node.treePrev + } +} + /** * Replaces the [beforeNode] of type [WHITE_SPACE] with the node with specified [text] * diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/KdocUtils.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/KdocUtils.kt index 37ba57eb2a..177be2abf5 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/KdocUtils.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/KdocUtils.kt @@ -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 @@ -33,6 +34,23 @@ fun ASTNode.kDocTags(): List { fun Iterable.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 * diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/smoke/DiktatSmokeTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/smoke/DiktatSmokeTest.kt index a766625e9c..90cbdd62b3 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/smoke/DiktatSmokeTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/smoke/DiktatSmokeTest.kt @@ -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 = mutableListOf() diff --git a/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/KdocFormattingMultilineTagsExpected.kt b/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/KdocFormattingMultilineTagsExpected.kt new file mode 100644 index 0000000000..d62b70bbe7 --- /dev/null +++ b/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/KdocFormattingMultilineTagsExpected.kt @@ -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 +} + diff --git a/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/KdocFormattingMultilineTagsTest.kt b/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/KdocFormattingMultilineTagsTest.kt new file mode 100644 index 0000000000..820e8a3c1d --- /dev/null +++ b/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/KdocFormattingMultilineTagsTest.kt @@ -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 +}