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

Bugfixes in KdocComments rule #968

Merged
merged 6 commits into from
Jul 8, 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
3 changes: 3 additions & 0 deletions diktat-rules/src/main/kotlin/generated/WarningNames.kt
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,9 @@ public object WarningNames {

public const val KDOC_NO_CONSTRUCTOR_PROPERTY: String = "KDOC_NO_CONSTRUCTOR_PROPERTY"

public const val KDOC_NO_CLASS_BODY_PROPERTIES_IN_HEADER: String =
"KDOC_NO_CLASS_BODY_PROPERTIES_IN_HEADER"

public const val KDOC_EXTRA_PROPERTY: String = "KDOC_EXTRA_PROPERTY"

public const val KDOC_NO_CONSTRUCTOR_PROPERTY_WITH_COMMENT: String =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ enum class Warnings(
KDOC_NO_EMPTY_TAGS(false, "2.2.1", "no empty descriptions in tag blocks are allowed"),
KDOC_NO_DEPRECATED_TAG(true, "2.1.3", "KDoc doesn't support @deprecated tag, use @Deprecated annotation instead"),
KDOC_NO_CONSTRUCTOR_PROPERTY(true, "2.1.1", "all properties from the primary constructor should be documented in a @property tag in KDoc"),
KDOC_NO_CLASS_BODY_PROPERTIES_IN_HEADER(true, "2.1.1", "only properties from the primary constructor should be documented in a @property tag in class KDoc"),
KDOC_EXTRA_PROPERTY(false, "2.1.1", "There is property in KDoc which is not present in the class"),
KDOC_NO_CONSTRUCTOR_PROPERTY_WITH_COMMENT(true, "2.1.1", "replace comment before property with @property tag in class KDoc"),
KDOC_CONTAINS_DATE_OR_AUTHOR(false, "2.1.3", "KDoc should not contain creation date and author name"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ 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_EXTRA_PROPERTY
import org.cqfn.diktat.ruleset.constants.Warnings.KDOC_NO_CLASS_BODY_PROPERTIES_IN_HEADER
import org.cqfn.diktat.ruleset.constants.Warnings.KDOC_NO_CONSTRUCTOR_PROPERTY
import org.cqfn.diktat.ruleset.constants.Warnings.KDOC_NO_CONSTRUCTOR_PROPERTY_WITH_COMMENT
import org.cqfn.diktat.ruleset.constants.Warnings.MISSING_KDOC_CLASS_ELEMENTS
Expand Down Expand Up @@ -106,18 +107,12 @@ class KdocComments(configRules: List<RulesConfig>) : DiktatRule(
kdocBeforeClass?.let {
checkKdocBeforeClass(node, kdocBeforeClass, prevComment)
}
?: run {
kgevorkyan marked this conversation as resolved.
Show resolved Hide resolved
createKdocWithProperty(node, prevComment)
}
?: createKdocWithProperty(node, prevComment)
}
?: run {
kdocBeforeClass?.let {
checkBasicKdocBeforeClass(node, kdocBeforeClass)
}
?: run {
createKdocBasicKdoc(node)
}
?: kdocBeforeClass?.let {
checkBasicKdocBeforeClass(node, kdocBeforeClass)
}
?: createKdocBasicKdoc(node)
}

@Suppress("UnsafeCallOnNullableType")
Expand Down Expand Up @@ -151,6 +146,7 @@ class KdocComments(configRules: List<RulesConfig>) : DiktatRule(
null
}
if (prevComment.elementType == KDOC || prevComment.elementType == BLOCK_COMMENT) {
// there is a documentation before property that we can extract, and there is class KDoc, where we can move it to
handleKdocAndBlock(node, prevComment, kdocBeforeClass, propertyInClassKdoc, propertyInLocalKdoc)
} else {
KDOC_NO_CONSTRUCTOR_PROPERTY_WITH_COMMENT.warnAndFix(configRules, emitWarn, isFixMode, node.findChildByType(IDENTIFIER)!!.text, prevComment.startOffset, node) {
Expand Down Expand Up @@ -205,21 +201,19 @@ class KdocComments(configRules: List<RulesConfig>) : DiktatRule(
propertyInClassKdoc: ASTNode?,
propertyInLocalKdoc: ASTNode?) {
val kdocText = if (prevComment.elementType == KDOC) {
prevComment.text.removePrefix("/**").removeSuffix("*/")
prevComment.text.removePrefix("/**").removeSuffix("*/").trim('\n', ' ')
} else {
prevComment.text.removePrefix("/*").removeSuffix("*/")
prevComment.text.removePrefix("/*").removeSuffix("*/").trim('\n', ' ')
}
val isFixable = (propertyInClassKdoc != null && propertyInLocalKdoc != null) ||
(propertyInClassKdoc == null && propertyInLocalKdoc == null && kdocText
.replace("\n+".toRegex(), "")
.lines()
.size != 1)
KDOC_NO_CONSTRUCTOR_PROPERTY.warnAndFix(configRules, emitWarn, !isFixable, prevComment.text, prevComment.startOffset, node, !isFixable) {
if (propertyInClassKdoc == null && propertyInLocalKdoc == null) {
insertTextInKdoc(kdocBeforeClass, " * @property ${node.findChildByType(IDENTIFIER)!!.text} ${kdocText.replace("\n+".toRegex(), "").removePrefix("*")}")
} else {
insertTextInKdoc(kdocBeforeClass, "${kdocText.trim()}\n")
// if property is documented with KDoc, which has a property tag inside, then it can contain some additional more complicated
// structure, that will be hard to move automatically
val isFixable = propertyInLocalKdoc == null
KDOC_NO_CONSTRUCTOR_PROPERTY.warnAndFix(configRules, emitWarn, isFixable, prevComment.text, prevComment.startOffset, node, isFixable) {
propertyInClassKdoc?.let {
// local docs should be appended to docs in class
appendKdocTagContent(propertyInClassKdoc, "\n$kdocText")
}
?: insertTextInKdoc(kdocBeforeClass, " * @property ${node.findChildByType(IDENTIFIER)!!.text} ${kdocText.removePrefix("*")}\n")

if (prevComment.treeNext.elementType == WHITE_SPACE) {
node.removeChild(prevComment.treeNext)
Expand Down Expand Up @@ -256,6 +250,25 @@ class KdocComments(configRules: List<RulesConfig>) : DiktatRule(
kdocBeforeClass.treeParent.replaceChild(kdocBeforeClass, KotlinParser().createNode(newKdocText).findChildByType(KDOC)!!)
}

/**
* Append [content] to [kdocTagNode], e.g.
* (`@property foo bar`, "baz") -> `@property foo bar baz`
*/
private fun appendKdocTagContent(
kdocTagNode: ASTNode, content: String
) {
kdocTagNode.findChildByType(KDOC_TEXT)?.let {
kdocTagNode.replaceChild(
it,
LeafPsiElement(KDOC_TEXT, "${it.text}$content")
)
}
?: kdocTagNode.addChild(
LeafPsiElement(KDOC_TEXT, content),
null
)
}

private fun checkClassElements(node: ASTNode) {
val modifier = node.getFirstChildWithType(MODIFIER_LIST)
val classBody = node.getFirstChildWithType(CLASS_BODY)
Expand All @@ -267,7 +280,22 @@ class KdocComments(configRules: List<RulesConfig>) : DiktatRule(
.filterNot {
(it.elementType == FUN && it.isStandardMethod()) || (it.elementType == FUN && it.isOverridden()) || (it.elementType == PROPERTY && it.isOverridden())
}
.forEach { checkDoc(it, MISSING_KDOC_CLASS_ELEMENTS) }
.forEach { classElement ->
if (classElement.elementType == PROPERTY) {
// we check if property declared in class body is also documented in class header via `@property` tag
val classKdoc = node.getFirstChildWithType(KDOC)
val propertyInClassKdoc = classKdoc?.kDocTags()?.find {
it.knownTag == KDocKnownTag.PROPERTY && it.getSubjectName() == classElement.getIdentifierName()?.text
}
propertyInClassKdoc?.let {
// if property is documented as `@property`, then we suggest to move docs to the declaration inside the class body
KDOC_NO_CLASS_BODY_PROPERTIES_IN_HEADER.warn(configRules, emitWarn, isFixMode, classElement.text, classElement.startOffset, classElement)
return
}
}
// for everything else, we raise a missing kdoc warning
checkDoc(classElement, MISSING_KDOC_CLASS_ELEMENTS)
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ class KotlinParser {
* @return [ASTNode]
* @throws KotlinParseException if code is incorrect
*/
fun createNode(text: String, isPackage: Boolean = false) = makeNode(text, isPackage) ?: throw KotlinParseException("Your text is not valid")
fun createNode(text: String, isPackage: Boolean = false) = makeNode(text, isPackage) ?: throw KotlinParseException("Text is not valid: [$text]")

/**
* @param text kotlin code
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.cqfn.diktat.ruleset.chapter2

import org.cqfn.diktat.ruleset.constants.Warnings.KDOC_EXTRA_PROPERTY
import org.cqfn.diktat.ruleset.constants.Warnings.KDOC_NO_CLASS_BODY_PROPERTIES_IN_HEADER
import org.cqfn.diktat.ruleset.constants.Warnings.KDOC_NO_CONSTRUCTOR_PROPERTY
import org.cqfn.diktat.ruleset.constants.Warnings.KDOC_NO_CONSTRUCTOR_PROPERTY_WITH_COMMENT
import org.cqfn.diktat.ruleset.constants.Warnings.MISSING_KDOC_CLASS_ELEMENTS
Expand Down Expand Up @@ -474,4 +475,38 @@ class KdocCommentsWarnTest : LintTestBase(::KdocComments) {
LintError(3, 4, ruleId, "${KDOC_EXTRA_PROPERTY.warnText()} @property kek", false)
)
}

@Test
@Tag(WarningNames.KDOC_NO_CLASS_BODY_PROPERTIES_IN_HEADER)
fun `property described only in class KDoc`() {
lintMethod(
"""
|/**
| * @property foo lorem ipsum
| */
|class Example {
| val foo: Any
|}
""".trimMargin(),
LintError(5, 5, ruleId, "${KDOC_NO_CLASS_BODY_PROPERTIES_IN_HEADER.warnText()} val foo: Any")
)
}

@Test
fun `property described both in class KDoc and own KDoc`() {
lintMethod(
"""
|/**
| * @property foo lorem ipsum
| */
|class Example {
| /**
| * dolor sit amet
| */
| val foo: Any
|}
""".trimMargin(),
LintError(5, 5, ruleId, "${KDOC_NO_CLASS_BODY_PROPERTIES_IN_HEADER.warnText()} /**...")
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ package test.paragraph2.kdoc

/**
* @property name another text
* some text
*/
* some text
*/
class A (
var name: String
){}
Expand All @@ -20,8 +20,8 @@ class A (

/**
* @property name another text
* text
*/
* text
*/
class A (
val name: String
){}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class FileComparator {
private val actualResultList: List<String>
private val diffGenerator = DiffRowGenerator.create()
.showInlineDiffs(true)
.mergeOriginalRevised(true)
.mergeOriginalRevised(false)
.inlineDiffByWord(false)
.oldTag { start -> if (start) "[" else "]" }
.newTag { start -> if (start) "<" else ">" }
Expand Down Expand Up @@ -55,15 +55,19 @@ class FileComparator {
when (delta) {
is ChangeDelta -> diffGenerator
.generateDiffRows(delta.source.lines, delta.target.lines)
.joinToString(System.lineSeparator()) { it.oldLine }
.let { "[ChangeDelta, position ${delta.source.position}, lines: [$it]]" }
.joinToString(prefix = "ChangeDelta, position ${delta.source.position}, lines:\n", separator = "\n\n") {
"""-${it.oldLine}
|+${it.newLine}
|""".trimMargin()
}
.let { "ChangeDelta, position ${delta.source.position}, lines:\n$it" }
else -> delta.toString()
}
}

log.error("""
|Expected result from <${expectedResultFile.name}> and actual formatted are different.
|See difference below (for ChangeDelta [text] indicates removed text, <text> - inserted):
|See difference below:
|$joinedDeltas
""".trimMargin()
)
Expand Down
1 change: 1 addition & 0 deletions info/available-rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
| 2 | 2.2.1 | KDOC_NO_EMPTY_TAGS | Check: warns if KDoc tags have empty content | no | no | |
| 2 | 2.1.3 | KDOC_NO_DEPRECATED_TAG | Check: warns if `@deprecated` is used in KDoc<br>Fix: adds `@Deprecated` annotation with message, removes tag | yes | no | Annotation's `replaceWith` field can be filled too|
| 2 | 2.1.1 | KDOC_NO_CONSTRUCTOR_PROPERTY | Check: warns if there is no property tag inside KDoc before constructor | yes | no |
| 2 | 2.1.1 | KDOC_NO_CLASS_BODY_PROPERTIES_IN_HEADER | Check: warns if property is declared in class body but documented with property tag inside KDoc before constructor | yes | no |
| 2 | 2.1.1 | KDOC_NO_CONSTRUCTOR_PROPERTY_WITH_COMMENT| Check: warns if there is comment before property in constructor | yes | no |
| 2 | 2.2.1 | KDOC_CONTAINS_DATE_OR_AUTHOR | Check: warns if header KDoc contains `@author` tag.<br>Warns if `@since` tag contains version and not date. | no | no | Detect author by other patterns (e.g. 'created by' etc.)|
| 2 | 2.3.1 | KDOC_TRIVIAL_KDOC_ON_FUNCTION | Check: warns if KDoc contains single line with words 'return', 'get' or 'set' | no | no | |
Expand Down
89 changes: 87 additions & 2 deletions info/guide/diktat-coding-convention.md
Original file line number Diff line number Diff line change
Expand Up @@ -1130,8 +1130,93 @@ try {
<!-- =============================================================================== -->
### <a name="c3.5"></a> 3.5 Line length

Line length should be less than 120 symbols. The international code style prohibits `non-Latin` (`non-ASCII`) symbols.
(See [Identifiers](#r1.1.1)) However, if you still intend on using them, follow the following convention:
Line length should be less than 120 symbols. Otherwise, it should be split.

If `complex property` initializing is too long, it should be split:

**Invalid example:**
```kotlin
val complexProperty = 1 + 2 + 3 + 4
```
**Valid example:**
```kotlin
val complexProperty = 1 + 2
+ 3 + 4
```

If `annotation` is too long, it also should be split:

**Invalid example:**
```kotlin
@Query(value = "select * from table where age = 10", nativeQuery = true)
fun foo() {}
```
**Valid example:**
```kotlin
@Query(value = "select * from table " +
"where age = 10", nativeQuery = true)
fun foo() {}
```

Long one line `function` should be split:

**Invalid example:**
```kotlin
fun foo() = goo().write("TooLong")
```
**Valid example:**
```kotlin
fun foo() =
goo().write("TooLong")
```

Long `binary expression` should be split:

**Invalid example:**
```kotlin
if (( x > 100) || y < 100 && !isFoo()) {}
```

**Valid example:**
```kotlin
if (( x > 100) ||
y < 100 && !isFoo()) {}
```

`Eol comment` also can be split, but it depends on comment location.
If this comment is on the same line with code it should be on line before:

**Invalid example:**
```kotlin
fun foo() {
val name = "Nick" // this comment is too long
}
```
**Valid example:**
```kotlin
fun foo() {
// this comment is too long
val name = "Nick"
}
```

But if this comment is on new line - it should be split to several lines:

**Invalid example:**
```kotlin
// This comment is too long. It should be on two lines.
fun foo() {}
```

**Valid example:**
```kotlin
// This comment is too long.
// It should be on two lines.
fun foo() {}
```

The international code style prohibits `non-Latin` (`non-ASCII`) symbols. (See [Identifiers](#r1.1.1)) However, if you still intend on using them, follow
the following convention:

- One wide character occupies the width of two narrow characters.
The "wide" and "narrow" parts of a character are defined by its [east Asian width Unicode attribute](https://unicode.org/reports/tr11/).
Expand Down
1 change: 1 addition & 0 deletions info/rules-mapping.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
| MISSING_KDOC_CLASS_ELEMENTS | [2.1.1](guide/diktat-coding-convention.md#r2.1.1) | no | Comments |
| MISSING_KDOC_ON_FUNCTION | [2.1.1](guide/diktat-coding-convention.md#r2.1.1) | yes | Comments |
| KDOC_NO_CONSTRUCTOR_PROPERTY | [2.1.1](guide/diktat-coding-convention.md#r2.1.1) | yes | Comments |
| KDOC_NO_CLASS_BODY_PROPERTIES_IN_HEADER | [2.1.1](guide/diktat-coding-convention.md#r2.1.1) | yes | Comments |
| KDOC_EXTRA_PROPERTY | [2.1.1](guide/diktat-coding-convention.md#r2.1.1) | no | Comments |
| KDOC_NO_CONSTRUCTOR_PROPERTY_WITH_COMMENT | [2.1.1](guide/diktat-coding-convention.md#r2.1.1) | yes | Comments |
| KDOC_WITHOUT_PARAM_TAG | [2.1.2](guide/diktat-coding-convention.md#r2.1.2) | yes | Comments |
Expand Down