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

Suggest using @property tag in KDoc #332

Merged
merged 14 commits into from
Oct 13, 2020
4 changes: 4 additions & 0 deletions diktat-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -254,5 +254,9 @@
configuration: {}
# Checks that floating-point values are not used in arithmetic expressions
- name: FLOAT_IN_ACCURATE_CALCULATIONS
enabled: true
configuration: {}
# Checks that property in constructor doesn't contains comment
- name: KDOC_NO_CONSTRUCTOR_PROPERTY
enabled: true
configuration: {}
4 changes: 3 additions & 1 deletion diktat-rules/src/main/kotlin/generated/WarningNames.kt
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ object WarningNames {

const val KDOC_NO_DEPRECATED_TAG: String = "KDOC_NO_DEPRECATED_TAG"

const val KDOC_NO_CONSTRUCTOR_PROPERTY: String = "KDOC_NO_CONSTRUCTOR_PROPERTY"

const val HEADER_WRONG_FORMAT: String = "HEADER_WRONG_FORMAT"

const val HEADER_MISSING_OR_WRONG_COPYRIGHT: String = "HEADER_MISSING_OR_WRONG_COPYRIGHT"
Expand Down Expand Up @@ -155,6 +157,6 @@ object WarningNames {
const val STRING_TEMPLATE_CURLY_BRACES: String = "STRING_TEMPLATE_CURLY_BRACES"

const val STRING_TEMPLATE_QUOTES: String = "STRING_TEMPLATE_QUOTES"

const val FLOAT_IN_ACCURATE_CALCULATIONS: String = "FLOAT_IN_ACCURATE_CALCULATIONS"
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ enum class Warnings(private val canBeAutoCorrected: Boolean, private val warn: S
KDOC_NO_NEWLINE_AFTER_SPECIAL_TAGS(true, "in KDoc there should be exactly one empty line after special tags"),
KDOC_NO_EMPTY_TAGS(false, "no empty descriptions in tag blocks are allowed"),
KDOC_NO_DEPRECATED_TAG(true, "KDoc doesn't support @deprecated tag, use @Deprecated annotation instead"),
KDOC_NO_CONSTRUCTOR_PROPERTY(true, "replace comment before property into KDoc"),
HEADER_WRONG_FORMAT(true, "file header comments should be properly formatted"),
HEADER_MISSING_OR_WRONG_COPYRIGHT(true, "file header comment must include copyright information inside a block comment"),
WRONG_COPYRIGHT_YEAR(true, "year defined in copyright and current year are different"),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,29 +1,43 @@
package org.cqfn.diktat.ruleset.rules.kdoc

import com.pinterest.ktlint.core.KtLint.FILE_PATH_USER_DATA_KEY
import com.pinterest.ktlint.core.Rule
import com.pinterest.ktlint.core.ast.ElementType.CLASS
import com.pinterest.ktlint.core.ast.ElementType.CLASS_BODY
import com.pinterest.ktlint.core.ast.ElementType.EOL_COMMENT
import com.pinterest.ktlint.core.ast.ElementType.FILE
import com.pinterest.ktlint.core.ast.ElementType.FUN
import com.pinterest.ktlint.core.ast.ElementType.IDENTIFIER
import com.pinterest.ktlint.core.ast.ElementType.KDOC
import com.pinterest.ktlint.core.ast.ElementType.KDOC_END
import com.pinterest.ktlint.core.ast.ElementType.KDOC_TEXT
import com.pinterest.ktlint.core.ast.ElementType.MODIFIER_LIST
import com.pinterest.ktlint.core.ast.ElementType.PRIMARY_CONSTRUCTOR
import com.pinterest.ktlint.core.ast.ElementType.PROPERTY
import com.pinterest.ktlint.core.ast.ElementType.VALUE_PARAMETER
import com.pinterest.ktlint.core.ast.ElementType.WHITE_SPACE
import com.pinterest.ktlint.core.ast.parent
import org.cqfn.diktat.common.config.rules.RulesConfig
import org.cqfn.diktat.common.config.rules.getCommonConfiguration
import org.cqfn.diktat.ruleset.constants.Warnings
import org.cqfn.diktat.ruleset.constants.Warnings.KDOC_NO_CONSTRUCTOR_PROPERTY
import org.cqfn.diktat.ruleset.constants.Warnings.MISSING_KDOC_CLASS_ELEMENTS
import org.cqfn.diktat.ruleset.constants.Warnings.MISSING_KDOC_TOP_LEVEL
import org.cqfn.diktat.ruleset.utils.*
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
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.TokenSet
import org.jetbrains.kotlin.kdoc.parser.KDocKnownTag
import org.jetbrains.kotlin.psi.psiUtil.parents

/**
* This rule checks the following features in KDocs:
* 1) All top-level (file level) functions and classes with public or internal access should have KDoc
* 2) All internal elements in class like class, property or function should be documented with KDoc
* 3) All property in constructor doesn't contain comment
*/
class KdocComments(private val configRules: List<RulesConfig>) : Rule("kdoc-comments") {

companion object {
private val statementsToDocument = TokenSet.create(CLASS, FUN, PROPERTY)
}
Expand All @@ -45,7 +59,83 @@ class KdocComments(private val configRules: List<RulesConfig>) : Rule("kdoc-comm
when (node.elementType) {
FILE -> checkTopLevelDoc(node)
CLASS -> checkClassElements(node)
VALUE_PARAMETER -> checkValueParameter(node)
}
}

@Suppress("UnsafeCallOnNullableType")
private fun checkValueParameter(node: ASTNode) {
if (node.parents().none { it.elementType == PRIMARY_CONSTRUCTOR }) return
val prevComment = if (node.treePrev.elementType == EOL_COMMENT) node.treePrev
else if (node.treePrev.elementType == WHITE_SPACE && node.treePrev.treePrev.elementType == EOL_COMMENT) node.treePrev.treePrev
else if (node.hasChildOfType(KDOC)) node.findChildByType(KDOC)!!
else return
val kDocBeforeClass = node.parent({ it.elementType == CLASS })!!.findChildByType(KDOC)
if (kDocBeforeClass != null)
checkKDocBeforeClass(node, kDocBeforeClass, prevComment)
else
createKDocWithProperty(node, prevComment)
}

@Suppress("UnsafeCallOnNullableType")
private fun createKDocWithProperty(node: ASTNode, prevComment: ASTNode) {
val modifier = node.getFirstChildWithType(MODIFIER_LIST)
KDOC_NO_CONSTRUCTOR_PROPERTY.warnAndFix(configRules, emitWarn, modifier.isAccessibleOutside(),
prevComment.text, prevComment.startOffset, node, modifier.isAccessibleOutside()) {
val classNode = node.parent({ it.elementType == CLASS })!!
val newKDocText = if (prevComment.elementType == KDOC) prevComment.text else {
"/**\n * ${prevComment.text.removePrefix("//")}\n */"
}
val newKDoc = KotlinParser().createNode(newKDocText).findChildByType(KDOC)!!
classNode.addChild(PsiWhiteSpaceImpl("\n"), classNode.firstChildNode)
classNode.addChild(newKDoc, classNode.firstChildNode)
if (prevComment.elementType == EOL_COMMENT) {
node.treeParent.removeRange(prevComment, node)
} else {
if (prevComment.treeNext.elementType == WHITE_SPACE)
node.removeChild(prevComment.treeNext)
node.removeChild(prevComment)
}
}

}

private fun checkKDocBeforeClass(node: ASTNode, kDocBeforeClass: ASTNode, prevComment: ASTNode) {
val propertyInClassKDoc = kDocBeforeClass.kDocTags()?.firstOrNull { it.knownTag == KDocKnownTag.PROPERTY && it.getSubjectName() == node.findChildByType(IDENTIFIER)!!.text }?.node
val propertyInLocalKDoc = if (prevComment.elementType == KDOC) prevComment.kDocTags()?.firstOrNull { it.knownTag == KDocKnownTag.PROPERTY && it.getSubjectName() == node.findChildByType(IDENTIFIER)!!.text }?.node else null
val isFixable = prevComment.elementType != KDOC || propertyInClassKDoc == null || propertyInLocalKDoc == null
KDOC_NO_CONSTRUCTOR_PROPERTY.warnAndFix(configRules, emitWarn, isFixable, prevComment.text, prevComment.startOffset, node, isFixable) {
if (prevComment.elementType == EOL_COMMENT)
handleCommentBefore(node, kDocBeforeClass, prevComment, propertyInClassKDoc)
else
handleKDocBefore(node, kDocBeforeClass, prevComment)
}
}

private fun handleKDocBefore(node: ASTNode, kDocBeforeClass: ASTNode, prevComment: ASTNode) {
val insertText = "${prevComment.text.removePrefix("/**").removeSuffix("*/").trim()}\n"
insertTextInKDoc(kDocBeforeClass, insertText)
if (prevComment.treeNext.elementType == WHITE_SPACE)
node.removeChild(prevComment.treeNext)
node.removeChild(prevComment)
}

@Suppress("UnsafeCallOnNullableType")
private fun handleCommentBefore(node: ASTNode, kDocBeforeClass: ASTNode, prevComment: ASTNode, propertyInClassKDoc: ASTNode?) {
if (propertyInClassKDoc != null) {
propertyInClassKDoc.addChild(LeafPsiElement(KDOC_TEXT, prevComment.text), null)
} else {
insertTextInKDoc(kDocBeforeClass, "* @property ${node.findChildByType(IDENTIFIER)!!.text} ${prevComment.text.removeRange(0, 2)}\n")
}
node.treeParent.removeRange(prevComment, node)
}

@Suppress("UnsafeCallOnNullableType")
private fun insertTextInKDoc(kDocBeforeClass: ASTNode, insertText: String) {
val allKDocText = kDocBeforeClass.text
val endKDoc = kDocBeforeClass.findChildByType(KDOC_END)!!.text
val newKDocText = StringBuilder(allKDocText).insert(allKDocText.indexOf(endKDoc), insertText).toString()
kDocBeforeClass.treeParent.replaceChild(kDocBeforeClass, KotlinParser().createNode(newKDocText).findChildByType(KDOC)!!)
}

private fun checkClassElements(node: ASTNode) {
Expand Down Expand Up @@ -77,7 +167,7 @@ class KdocComments(private val configRules: List<RulesConfig>) : Rule("kdoc-comm
val name = node.getIdentifierName()

if (modifier.isAccessibleOutside() && kdoc == null) {
warning.warn(configRules, emitWarn, isFixMode, name!!.text, node.startOffset,node)
warning.warn(configRules, emitWarn, isFixMode, name!!.text, node.startOffset, node)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,21 @@ 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.kdoc.parser.KDocKnownTag
import org.jetbrains.kotlin.kdoc.psi.api.KDoc
import org.jetbrains.kotlin.kdoc.psi.impl.KDocTag
import org.jetbrains.kotlin.utils.addToStdlib.ifNotEmpty

fun ASTNode.kDocTags(): Collection<KDocTag>? {
fun ASTNode.kDocTags(): List<KDocTag>? {
require(this.elementType == ElementType.KDOC) { "kDoc tags can be retrieved only from KDOC node" }
return this.getFirstChildWithType(KDOC_SECTION)
?.getAllChildrenWithType(ElementType.KDOC_TAG)?.map { it.psi as KDocTag }
return this.getAllChildrenWithType(KDOC_SECTION)
.map { sectionNode ->
sectionNode.getAllChildrenWithType(ElementType.KDOC_TAG)
.map { its -> its.psi as KDocTag }
}
.flatten()
}

fun Iterable<KDocTag>.hasKnownKDocTag(knownTag: KDocKnownTag): Boolean =
this.find { it.knownTag == knownTag } != null
this.find { it.knownTag == knownTag } != null

/**
* This method inserts a new tag into KDoc before specified another tag, aligning it with the rest of this KDoc
Expand Down
4 changes: 4 additions & 0 deletions diktat-rules/src/main/resources/diktat-analysis-huawei.yml
Original file line number Diff line number Diff line change
Expand Up @@ -254,5 +254,9 @@
configuration: {}
# Checks that floating-point values are not used in arithmetic expressions
- name: FLOAT_IN_ACCURATE_CALCULATIONS
enabled: true
configuration: {}
# Checks that property in constructor doesn't contains comment
- name: KDOC_NO_CONSTRUCTOR_PROPERTY
enabled: true
configuration: {}
4 changes: 4 additions & 0 deletions diktat-rules/src/main/resources/diktat-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -256,5 +256,9 @@
configuration: {}
# Checks that floating-point values are not used in arithmetic expressions
- name: FLOAT_IN_ACCURATE_CALCULATIONS
enabled: true
configuration: {}
# Checks that property in constructor doesn't contains comment
- name: KDOC_NO_CONSTRUCTOR_PROPERTY
enabled: true
configuration: {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package org.cqfn.diktat.ruleset.chapter2

import generated.WarningNames
import org.cqfn.diktat.ruleset.rules.kdoc.KdocComments
import org.cqfn.diktat.util.FixTestBase
import org.junit.jupiter.api.Tag
import org.junit.jupiter.api.Test

class KdocCommentsFixTest: FixTestBase("test/paragraph2/kdoc/", ::KdocComments) {


@Test
@Tag(WarningNames.KDOC_NO_CONSTRUCTOR_PROPERTY)
fun `check fix with class kdoc`() {
fixAndCompare("ConstructorCommentExpected.kt", "ConstructorCommentTest.kt")
}

@Test
@Tag(WarningNames.KDOC_NO_CONSTRUCTOR_PROPERTY)
fun `check fix without class kdoc`() {
fixAndCompare("ConstructorCommentNoKDocExpected.kt", "ConstructorCommentNoKDocTest.kt")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -203,4 +203,89 @@ class KdocWarnTest : LintTestBase(::KdocComments) {
""".trimMargin()
)
}

@Test
@Tag(WarningNames.KDOC_NO_CONSTRUCTOR_PROPERTY)
fun `check simple primary constructor with comment`() {
lintMethod(
"""
|/**
| * @property name d
| * @param adsf
| * @return something
| */
|class Example constructor (
| // short
| val name: String
|) {
|}
""".trimMargin(),
LintError(7, 4, ruleId, "${KDOC_NO_CONSTRUCTOR_PROPERTY.warnText()} // short", true)
)
}

@Test
@Tag(WarningNames.KDOC_NO_CONSTRUCTOR_PROPERTY)
fun `shouldn't trigger because not primary constructor`() {
lintMethod(
"""
|/**
| * @property name d
| * @property anotherName text
| */
|class Example {
| constructor(
| // name
| name: String,
| anotherName: String,
| OneMoreName: String
| )
|}
""".trimMargin()
)
}

@Test
@Tag(WarningNames.KDOC_NO_CONSTRUCTOR_PROPERTY)
fun `check another constructor`() {
lintMethod(
"""
|/**
| * @return some
| */
|class Example (
| //some descriptions
| name: String,
| anotherName: String,
| OneMoreName: String
| ) {
|}
""".trimMargin(),
LintError(5, 4, ruleId, "${KDOC_NO_CONSTRUCTOR_PROPERTY.warnText()} //some descriptions", true)
)
}

@Test
@Tag(WarningNames.KDOC_NO_CONSTRUCTOR_PROPERTY)
fun `check danother constructor`() {
lintMethod(
"""
|/**
| * @return some
| */
|class Example (
| /**
| * some descriptions
| * @return fdv
| */
|
| name: String,
| anotherName: String,
| OneMoreName: String
| ) {
|}
""".trimMargin(),
LintError(5, 4, ruleId, "${KDOC_NO_CONSTRUCTOR_PROPERTY.warnText()} /**...", true)
)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
package test.paragraph2.kdoc

/**
* Simple class
* @property name should replace
*/
class A constructor(
name: String
)

/**
* @property age age
* @property lastName Ivanov
*/
class A(
val age: Int,
val lastName: String
) {

}

class Out{
/**
* Inner class
* @property name Jack
*/
class In(
name: String
){}
}

/**
*
* some text
*/
class A (
name: String
){}

/**
* @property name another text
* some text
*/
class A (
name: String
){}

/**
* @property name another text
*/
class A (
/**
* @property name some text
*/
name: String
){}
Loading