Skip to content

Commit

Permalink
Initial commit with KDoc formatting checks
Browse files Browse the repository at this point in the history
### What's done:
* Added check for number of white spaces after tags in KDocs
* Updated rules-config.json
  • Loading branch information
petertrr committed Jun 17, 2020
1 parent 5a42538 commit ce7e810
Show file tree
Hide file tree
Showing 19 changed files with 478 additions and 111 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,14 @@ enum class Warnings(private val id: Int, private val canBeAutoCorrected: Boolean
KDOC_WITHOUT_RETURN_TAG(23, true, "all methods which return values should have @return tag in KDoc"),
KDOC_WITHOUT_THROWS_TAG(24, true, "all methods which throw exceptions should have @throws tag in KDoc"),
BLANK_LINE_AFTER_KDOC(25, true, "there should be no empty line between Kdoc and code it is describing"),

KDOC_WRONG_SPACES_AFTER_TAG(26, true, "there should be exactly one white space after tag name in KDoc"),
KDOC_WRONG_TAGS_ORDER(27, true, "in KDoc standard tags are arranged in order @param, @return, @throws, but are"),
KDOC_NO_NEWLINE_AFTER_SPECIAL_TAGS(28, true, "in KDoc there should be exactly one empty line after special tags"),
KDOC_NO_EMPTY_TAGS(29, false, "no empty descriptions in tag blocks are allowed"),
KDOC_NO_DEPRECATED_TAG(30, false, "KDoc doesn't support @deprecated tag, use @Deprecated annotation instead"),

// ====== incorrect place and warn number ====
INCORRECT_PACKAGE_SEPARATOR(26, true, "package name parts should be separated only by dots - there should be no other symbols like underscores (_)")
INCORRECT_PACKAGE_SEPARATOR(31, true, "package name parts should be separated only by dots - there should be no other symbols like underscores (_)")
;

override fun ruleName(): String = this.name
Expand Down
Original file line number Diff line number Diff line change
@@ -1,21 +1,31 @@
package com.huawei.rri.fixbot.ruleset.huawei.rules

import com.huawei.rri.fixbot.ruleset.huawei.constants.Warnings.BLANK_LINE_AFTER_KDOC
import com.huawei.rri.fixbot.ruleset.huawei.utils.countSubStringOccurrences
import com.huawei.rri.fixbot.ruleset.huawei.utils.getFirstChildWithType
import com.huawei.rri.fixbot.ruleset.huawei.utils.leaveOnlyOneNewLine
import com.huawei.rri.fixbot.ruleset.huawei.constants.Warnings.*
import com.huawei.rri.fixbot.ruleset.huawei.utils.*
import com.pinterest.ktlint.core.KtLint
import com.pinterest.ktlint.core.Rule
import com.pinterest.ktlint.core.ast.ElementType
import com.pinterest.ktlint.core.ast.ElementType.CLASS
import com.pinterest.ktlint.core.ast.ElementType.FUN
import com.pinterest.ktlint.core.ast.ElementType.KDOC
import com.pinterest.ktlint.core.ast.ElementType.KDOC_LEADING_ASTERISK
import com.pinterest.ktlint.core.ast.ElementType.KDOC_TAG_NAME
import com.pinterest.ktlint.core.ast.ElementType.PROPERTY
import com.pinterest.ktlint.core.ast.ElementType.WHITE_SPACE
import com.pinterest.ktlint.core.ast.prevSibling
import config.rules.RulesConfig
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.LeafPsiElement
import org.jetbrains.kotlin.kdoc.parser.KDocKnownTag
import org.jetbrains.kotlin.kdoc.psi.impl.KDocTag

/**
* Formatting visitor for Kdoc:
* 1) removing all blank lines between Kdoc and the code it's declaring
* 2) ensuring there are no tags with empty content
* 3) ensuring there is only one white space between tag and it's body
* 4) ensuring tags @apiNote, @implSpec, @implNote have one empty line after their body
* 5) ensuring tags @param, @return, @throws are arranged in this order
*/
class KdocFormatting : Rule("kdoc-formatting") {

Expand All @@ -35,12 +45,119 @@ class KdocFormatting : Rule("kdoc-formatting") {
val declarationTypes = setOf(CLASS, FUN, PROPERTY)

if (declarationTypes.contains(node.elementType)) {
val kdoc = node.getFirstChildWithType(ElementType.KDOC)
val nodeAfterKdoc = kdoc?.treeNext
val name = node.getFirstChildWithType(ElementType.IDENTIFIER)
if (nodeAfterKdoc?.elementType == ElementType.WHITE_SPACE && nodeAfterKdoc.text.countSubStringOccurrences("\n") > 1) {
BLANK_LINE_AFTER_KDOC.warnAndFix(confiRules, emitWarn, isFixMode, name!!.text, nodeAfterKdoc.startOffset) {
nodeAfterKdoc.leaveOnlyOneNewLine()
checkBlankLineAfterKdoc(node)
}

if (node.elementType == KDOC) {
val kDocTags = node.kDocTags()

checkNoDeprecatedTag(kDocTags)
checkEmptyTags(kDocTags)
checkSpaceAfterTag(kDocTags)
checkBasicTagsOrder(node, kDocTags)
checkNewLineAfterSpecialTags(node)
}
}

private fun checkBlankLineAfterKdoc(node: ASTNode) {
val kdoc = node.getFirstChildWithType(KDOC)
val nodeAfterKdoc = kdoc?.treeNext
val name = node.getFirstChildWithType(ElementType.IDENTIFIER)
if (nodeAfterKdoc?.elementType == WHITE_SPACE && nodeAfterKdoc.text.countSubStringOccurrences("\n") > 1) {
BLANK_LINE_AFTER_KDOC.warnAndFix(confiRules, emitWarn, isFixMode, name!!.text, nodeAfterKdoc.startOffset) {
nodeAfterKdoc.leaveOnlyOneNewLine()
}
}
}

private fun checkNoDeprecatedTag(kDocTags: Collection<KDocTag>?) {
kDocTags?.find { it.name == "deprecated" }
?.let {
// fixme possible fix would be to remove tag and add annotation, but description should be kept
// also annotation supports different fields, which ideally should be properly filled
KDOC_NO_DEPRECATED_TAG.warn(confiRules, emitWarn, isFixMode, it.text, it.node.startOffset)
}
}

private fun checkEmptyTags(kDocTags: Collection<KDocTag>?) {
kDocTags?.filter {
it.getSubjectName() == null && it.getContent().isEmpty()
}?.forEach {
KDOC_NO_EMPTY_TAGS.warn(confiRules, emitWarn, isFixMode, "@${it.name!!}", it.node.startOffset)
}
}

private fun checkSpaceAfterTag(kDocTags: Collection<KDocTag>?) {
kDocTags?.filter {
it.node.findChildAfter(KDOC_TAG_NAME, WHITE_SPACE)?.text != " "
}?.forEach {
KDOC_WRONG_SPACES_AFTER_TAG.warnAndFix(confiRules, emitWarn, isFixMode,
"@${it.name!!}", it.node.startOffset) {
val whitespaceNode = it.node.findChildAfter(KDOC_TAG_NAME, WHITE_SPACE)
if (whitespaceNode != null) {
it.node.replaceChild(whitespaceNode, LeafPsiElement(WHITE_SPACE, " "))
}
}
}
}

private fun checkBasicTagsOrder(node: ASTNode, kDocTags: Collection<KDocTag>?) {
// basic tags which are present in current KDoc, in proper order
val basicTagsOrdered = listOf(KDocKnownTag.PARAM, KDocKnownTag.RETURN, KDocKnownTag.THROWS)
.filter { basicTag -> kDocTags?.find { it.knownTag == basicTag } != null }
val basicTags = kDocTags?.filter { basicTagsOrdered.contains(it.knownTag) }
val isTagsInCorrectOrder = basicTags?.map { it.knownTag }?.equals(basicTagsOrdered)

if (kDocTags != null && !isTagsInCorrectOrder!!) {
KDOC_WRONG_TAGS_ORDER.warnAndFix(confiRules, emitWarn, isFixMode,
basicTags.joinToString(", ") { "@${it.name}" }, basicTags.first().node.startOffset) {
val kDocSection = node.getFirstChildWithType(ElementType.KDOC_SECTION)!!
val basicTagChildren = kDocTags
.filter { basicTagsOrdered.contains(it.knownTag) }
.map { it.node }

basicTagsOrdered.forEachIndexed { index, tag ->
val tagNode = kDocTags.find { it.knownTag == tag }?.node
kDocSection.addChild(tagNode!!.copyElement(), basicTagChildren[index])
kDocSection.removeChild(basicTagChildren[index])
}
}
}
}

private fun checkNewLineAfterSpecialTags(node: ASTNode) {
val specialTagNames = listOf("implSpec", "implNote", "apiNote")

val presentSpecialTagNodes = node.getFirstChildWithType(ElementType.KDOC_SECTION)
?.getAllChildrenWithType(ElementType.KDOC_TAG)
?.filter { (it.psi as KDocTag).name in specialTagNames }

val poorlyFormattedTagNodes = presentSpecialTagNodes?.filterNot { specialTagNode ->
// empty line with just * followed by white space or end of block
specialTagNode.lastChildNode.elementType == KDOC_LEADING_ASTERISK
&& (specialTagNode.treeNext == null || specialTagNode.treeNext.elementType == WHITE_SPACE
&& specialTagNode.treeNext.text.count { it == '\n' } == 1)
// and with no empty line before
&& specialTagNode.lastChildNode.treePrev.elementType == WHITE_SPACE
&& specialTagNode.lastChildNode.treePrev.treePrev.elementType != KDOC_LEADING_ASTERISK
}

if (presentSpecialTagNodes != null && poorlyFormattedTagNodes!!.isNotEmpty()) {
KDOC_NO_NEWLINE_AFTER_SPECIAL_TAGS.warnAndFix(confiRules, emitWarn, isFixMode,
poorlyFormattedTagNodes.joinToString(", ") { "@${(it.psi as KDocTag).name!!}" },
poorlyFormattedTagNodes.first().startOffset) {
poorlyFormattedTagNodes.forEach { node ->
while (node.lastChildNode.elementType == KDOC_LEADING_ASTERISK
&& node.lastChildNode.treePrev.treePrev.elementType == KDOC_LEADING_ASTERISK) {
node.removeChild(node.lastChildNode) // KDOC_LEADING_ASTERISK
node.removeChild(node.lastChildNode) // WHITE_SPACE
}
if (node.lastChildNode.elementType != KDOC_LEADING_ASTERISK) {
val indent = node.prevSibling { it.elementType == WHITE_SPACE }
?.text?.substringAfter('\n')?.count { it == ' ' } ?: 0
node.addChild(LeafPsiElement(WHITE_SPACE, "\n${" ".repeat(indent)}"), null)
node.addChild(LeafPsiElement(KDOC_LEADING_ASTERISK, "*"), null)
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class KdocMethods : Rule("kdoc-methods") {

private fun checkSignatureDescription(node: ASTNode) {
val kDoc = node.getFirstChildWithType(KDOC)
val kDocTags = node.kDocTags()
val kDocTags = kDoc?.kDocTags()

val missingParameters = getMissingParameters(node, kDocTags)

Expand All @@ -55,7 +55,7 @@ class KdocMethods : Rule("kdoc-methods") {

if (paramCheckFailed) {
Warnings.KDOC_WITHOUT_PARAM_TAG.warnAndFix(confiRules, emitWarn, isFixMode, missingParameters.joinToString(), node.startOffset) {
// FixMe: add separate fix here if any parametr is missing
// FixMe: add separate fix here if any parameter is missing
}
}
if (returnCheckFailed) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
package com.huawei.rri.fixbot.ruleset.huawei.utils

import com.pinterest.ktlint.core.ast.ElementType
import org.jetbrains.kotlin.com.google.common.base.Preconditions
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import org.jetbrains.kotlin.kdoc.parser.KDocKnownTag
import org.jetbrains.kotlin.kdoc.psi.impl.KDocTag

fun ASTNode.kDocTags(): Collection<KDocTag>? {
val kDoc = this.getFirstChildWithType(ElementType.KDOC)
return kDoc?.getFirstChildWithType(ElementType.KDOC_SECTION)
Preconditions.checkArgument(this.elementType == ElementType.KDOC,
"kDoc tags can be retrieved only from KDOC node")
return this.getFirstChildWithType(ElementType.KDOC_SECTION)
?.getAllChildrenWithType(ElementType.KDOC_TAG)?.map { it.psi as KDocTag }
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,10 @@
package com.huawei.rri.fixbot.ruleset.huawei.chapter1

import com.huawei.rri.fixbot.ruleset.huawei.rules.IdentifierNaming
import com.pinterest.ktlint.core.KtLint
import com.pinterest.ktlint.core.Rule
import com.pinterest.ktlint.core.RuleSet
import com.huawei.rri.fixbot.ruleset.huawei.utils.format
import config.rules.RulesConfig
import org.assertj.core.api.Assertions.assertThat
import org.junit.Test
import com.huawei.rri.fixbot.ruleset.huawei.rules.PackageNaming
import config.rules.RulesConfig
import test_framework.processing.TestComparatorUnit

class IdentifierNamingFixTest {
Expand Down Expand Up @@ -36,20 +33,10 @@ class IdentifierNamingFixTest {

}

private fun format(text: String, fileName: String): String = IdentifierNaming().format(text, fileName)

private fun Rule.format(text: String, fileName: String): String {
return KtLint.format(
KtLint.Params(
text = text,
ruleSets = listOf(RuleSet("huawei-codestyle", this@format)),
fileName = fileName,
rulesConfigList = listOf(
RulesConfig("PACKAGE_NAME_INCORRECT", false, ""),
RulesConfig("PACKAGE_NAME_INCORRECT_PREFIX", false, "")
),
cb = { _, _ -> }
)
private fun format(text: String, fileName: String): String = IdentifierNaming().format(text, fileName,
listOf(
RulesConfig("PACKAGE_NAME_INCORRECT", false, ""),
RulesConfig("PACKAGE_NAME_INCORRECT_PREFIX", false, "")
)
}
)
}
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
package com.huawei.rri.fixbot.ruleset.huawei.chapter1

import com.pinterest.ktlint.core.KtLint
import com.pinterest.ktlint.core.Rule
import com.pinterest.ktlint.core.RuleSet
import com.huawei.rri.fixbot.ruleset.huawei.rules.PackageNaming
import com.huawei.rri.fixbot.ruleset.huawei.utils.format
import org.assertj.core.api.Assertions.assertThat
import org.junit.Test
import com.huawei.rri.fixbot.ruleset.huawei.rules.PackageNaming
import test_framework.processing.TestComparatorUnit

class PackageNamingFixTest {
val testComparatorUnit = TestComparatorUnit("test/paragraph1/naming/package", ::format)
private val testComparatorUnit = TestComparatorUnit("test/paragraph1/naming/package") { text, fileName ->
PackageNaming().format(text, fileName)
}

@Test
fun `incorrect case of package name (fix)`() {
Expand All @@ -34,18 +34,4 @@ class PackageNamingFixTest {
.compareFilesFromResources("FixUnderscoreExpected.kt", "FixUnderscoreTest.kt")
).isEqualTo(true)
}


private fun format(text: String, fileName: String): String = PackageNaming().format(text, fileName)

private fun Rule.format(text: String, fileName: String): String {
return KtLint.format(
KtLint.Params(
text = text,
ruleSets = listOf(RuleSet("huawei-codestyle", this@format)),
fileName = fileName,
cb = { _, _ -> }
)
)
}
}
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
package com.huawei.rri.fixbot.ruleset.huawei.chapter1

import com.pinterest.ktlint.core.KtLint
import com.pinterest.ktlint.core.Rule
import com.pinterest.ktlint.core.RuleSet
import com.huawei.rri.fixbot.ruleset.huawei.rules.PackageNaming
import com.huawei.rri.fixbot.ruleset.huawei.utils.format
import org.assertj.core.api.Assertions.assertThat
import org.junit.Test
import com.huawei.rri.fixbot.ruleset.huawei.rules.PackageNaming
import test_framework.processing.TestComparatorUnit

class PackagePathFixTest {
private val testComparatorUnitWithDomain = TestComparatorUnit("test/paragraph1/naming/package/src/main/kotlin/com/huawei/some/name", ::format)
private val testComparatorUnit = TestComparatorUnit("test/paragraph1/naming/package/src/main/kotlin/some", ::format)
private fun format(text: String, fileName: String): String = PackageNaming().format(text, fileName)

@Test
fun `fixing package name that differs from a path (fix)`() {
Expand Down Expand Up @@ -43,18 +42,4 @@ class PackagePathFixTest {
.compareFilesFromResources("FixMissingExpected.kt", "FixMissingTest.kt")
).isEqualTo(true)
}


private fun format(text: String, fileName: String): String = PackageNaming().format(text, fileName)

private fun Rule.format(text: String, fileName: String): String {
return KtLint.format(
KtLint.Params(
text = text,
ruleSets = listOf(RuleSet("huawei-codestyle", this@format)),
fileName = fileName,
cb = { _, _ -> }
)
)
}
}
Loading

0 comments on commit ce7e810

Please sign in to comment.