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

Warn if param or property in kdoc present several times #1547

Merged
merged 27 commits into from
Nov 9, 2022
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-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -450,6 +450,9 @@
# Checks that property in KDoc present in class
- name: KDOC_EXTRA_PROPERTY
enabled: true
# There's a property in KDoc which is already present
- name: KDOC_DUPLICATE_PROPERTY
enabled: true
nulls marked this conversation as resolved.
Show resolved Hide resolved
# Checks that KDoc in constructor has property tag but with comment inside constructor
- name: KDOC_NO_CONSTRUCTOR_PROPERTY_WITH_COMMENT
enabled: true
Expand Down
2 changes: 2 additions & 0 deletions diktat-rules/src/main/kotlin/generated/WarningNames.kt
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ public object WarningNames {

public const val KDOC_EXTRA_PROPERTY: String = "KDOC_EXTRA_PROPERTY"

public const val KDOC_DUPLICATE_PROPERTY: String = "KDOC_DUPLICATE_PROPERTY"

public const val KDOC_NO_CONSTRUCTOR_PROPERTY_WITH_COMMENT: String =
"KDOC_NO_CONSTRUCTOR_PROPERTY_WITH_COMMENT"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ enum class Warnings(
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_DUPLICATE_PROPERTY(false, "2.1.1", "There's a property in KDoc which is already present"),
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"),
HEADER_WRONG_FORMAT(true, "2.2.1", "file header comments should be properly formatted"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package org.cqfn.diktat.ruleset.rules.chapter2.kdoc
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_DUPLICATE_PROPERTY
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
Expand Down Expand Up @@ -54,7 +55,9 @@ class KdocComments(configRules: List<RulesConfig>) : DiktatRule(
NAME_ID,
configRules,
listOf(KDOC_EXTRA_PROPERTY, KDOC_NO_CONSTRUCTOR_PROPERTY,
KDOC_NO_CONSTRUCTOR_PROPERTY_WITH_COMMENT, MISSING_KDOC_CLASS_ELEMENTS, MISSING_KDOC_TOP_LEVEL)
KDOC_NO_CONSTRUCTOR_PROPERTY_WITH_COMMENT, MISSING_KDOC_CLASS_ELEMENTS, MISSING_KDOC_TOP_LEVEL,
KDOC_DUPLICATE_PROPERTY
)
) {
private val config by lazy { configRules.getCommonConfiguration() }

Expand Down Expand Up @@ -97,6 +100,7 @@ class KdocComments(configRules: List<RulesConfig>) : DiktatRule(
val kdocBeforeClass = node
?.parent({ it.elementType == CLASS })
?.findChildByType(KDOC) ?: return

val propertiesInKdoc = kdocBeforeClass
.kDocTags()
.filter { it.knownTag == KDocKnownTag.PROPERTY }
Expand Down Expand Up @@ -267,6 +271,17 @@ class KdocComments(configRules: List<RulesConfig>) : DiktatRule(
node.treeParent.removeChildMergingSurroundingWhitespaces(prevComment)
}

private fun checkDuplicateProperties(kdoc: ASTNode) {
val propertiesAndParams = kdoc.kDocTags()
.filter { it.knownTag == KDocKnownTag.PROPERTY || it.knownTag == KDocKnownTag.PARAM }
val traversedNodes: MutableSet<String?> = mutableSetOf()
propertiesAndParams.forEach { property ->
if (!traversedNodes.add(property.getSubjectName())) {
KDOC_DUPLICATE_PROPERTY.warn(configRules, emitWarn, isFixMode, property.text, property.node.startOffset, kdoc)
}
}
}

@Suppress("UnsafeCallOnNullableType")
private fun insertTextInKdoc(kdocBeforeClass: ASTNode, insertText: String) {
val allKdocText = kdocBeforeClass.text
Expand Down Expand Up @@ -338,7 +353,9 @@ class KdocComments(configRules: List<RulesConfig>) : DiktatRule(
// if there is an annotation before function, AST is a bit more complex, so we need to look for Kdoc among
// children of modifier list
val kdoc = node.getFirstChildWithType(KDOC) ?: node.getFirstChildWithType(MODIFIER_LIST)?.getFirstChildWithType(KDOC)

kdoc?.let {
checkDuplicateProperties(kdoc)
}
val name = node.getIdentifierName()
val isModifierAccessibleOutsideOrActual = node.getFirstChildWithType(MODIFIER_LIST).run {
isAccessibleOutside() && this?.hasChildOfType(ElementType.ACTUAL_KEYWORD) != true
Expand Down
3 changes: 3 additions & 0 deletions diktat-rules/src/main/resources/diktat-analysis-huawei.yml
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,9 @@
# Checks that property in KDoc present in class
- name: KDOC_EXTRA_PROPERTY
enabled: true
# There's a property in KDoc which is already present
- name: KDOC_DUPLICATE_PROPERTY
enabled: true
# Checks that KDoc in constructor has property tag but with comment inside constructor
- name: KDOC_NO_CONSTRUCTOR_PROPERTY_WITH_COMMENT
enabled: true
Expand Down
3 changes: 3 additions & 0 deletions diktat-rules/src/main/resources/diktat-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,9 @@
# Checks that property in KDoc present in class
- name: KDOC_EXTRA_PROPERTY
enabled: true
# There's a property in KDoc which is already present
- name: KDOC_DUPLICATE_PROPERTY
enabled: true
# Checks that KDoc in constructor has property tag but with comment inside constructor
- name: KDOC_NO_CONSTRUCTOR_PROPERTY_WITH_COMMENT
enabled: true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package org.cqfn.diktat.ruleset.chapter2

import org.cqfn.diktat.common.config.rules.DIKTAT_RULE_SET_ID
import org.cqfn.diktat.ruleset.constants.Warnings
import org.cqfn.diktat.ruleset.constants.Warnings.KDOC_DUPLICATE_PROPERTY
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
Expand All @@ -17,6 +18,7 @@ import org.junit.jupiter.api.Tag
import org.junit.jupiter.api.Tags
import org.junit.jupiter.api.Test

@Suppress("LargeClass")
class KdocCommentsWarnTest : LintTestBase(::KdocComments) {
private val ruleId: String = "$DIKTAT_RULE_SET_ID:${KdocComments.NAME_ID}"

Expand Down Expand Up @@ -619,4 +621,80 @@ class KdocCommentsWarnTest : LintTestBase(::KdocComments) {
""".trimMargin()
)
}

@Test
fun `should warn if there are duplicate tags 1`() {
lintMethod(
"""
|/**
| * @param field1 description1
| * @param field2 description2
| * @param field2
| */
|fun foo(field1: Long, field2: Int) {
| //
|}
""".trimMargin(),
LintError(4, 4, ruleId, "${KDOC_DUPLICATE_PROPERTY.warnText()} @param field2"),
)
}

@Test
fun `should warn if there are duplicate tags 2`() {
lintMethod(
"""
|/**
| * @property field1
| * @property field2
| * @property field2
| */
|@Serializable
|data class DataClass(
| val field1: String,
| val field2: String,
|)
""".trimMargin(),
LintError(4, 4, ruleId, "${KDOC_DUPLICATE_PROPERTY.warnText()} @property field2"),
)
}

@Test
fun `should warn if there are duplicate tags 3`() {
lintMethod(
"""
|/**
| * @property field1
| * @property field2
| * @param field2
| */
|@Serializable
|data class DataClass(
| val field1: String,
| val field2: String,
|)
""".trimMargin(),
LintError(4, 4, ruleId, "${KDOC_DUPLICATE_PROPERTY.warnText()} @param field2"),
)
}

@Test
fun `should warn if there are duplicate tags 4`() {
lintMethod(
"""
|/**
| * @property field1
| * @property field1
| * @property field2
| * @param field2
| */
|@Serializable
|data class DataClass(
| val field1: String,
| val field2: String,
|)
""".trimMargin(),
LintError(3, 4, ruleId, "${KDOC_DUPLICATE_PROPERTY.warnText()} @property field1"),
LintError(5, 4, ruleId, "${KDOC_DUPLICATE_PROPERTY.warnText()} @param field2"),
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ package org.cqfn.diktat.test.framework.common
/**
* Class that keeps the result of executed command
*
* @param stdOut standard output
* @param stdErr error stream
* @property stdOut content from stdout stream
* @property stdErr content from stderr stream
*/
Expand Down
1 change: 1 addition & 0 deletions info/available-rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
| 1 | 1.6.2 | FUNCTION_BOOLEAN_PREFIX | Check: functions/methods that return boolean should have a special prefix, such as "is/should/etc."<br>Fix: | yes | no | Recursively update usages of this function in the project. Aggressive fix - what if the new name will not be valid? |
| 2 | 2.1.1 | MISSING_KDOC_TOP_LEVEL | Check: warns at a file level internal or public class or if the function has a missing KDoc.<br>Fix: no | no | no | Support extension for setters/getters. Support extension for method "override". |
| 2 | 2.1.1 | KDOC_EXTRA_PROPERTY | Check: warn if there is a property in KDoc that is not present in the class. | no | no | - |
| 2 | 2.1.1 | KDOC_DUPLICATE_PROPERTY | Check: warn if there's a property in KDoc which is already present. | no | no | - |
| 2 | 2.1.1 | MISSING_KDOC_CLASS_ELEMENTS | Check: warns if accessible internal elements (protected, public, internal) in a class are not documented.<br>Fix: no | no | no | Maybe exception cases for setters and getters are needed; no sense in adding KDoc to them. |
| 2 | 2.1.1 | MISSING_KDOC_ON_FUNCTION | Check: warns if accessible function doesn't have KDoc.<br>Fix: adds KDoc template if it is not empty. | yes | no | |
| 2 | 2.1.1 | KDOC_NO_CONSTRUCTOR_PROPERTY | Check: warns if there is no property tag inside KDoc before the constructor. | yes | no | |
Expand Down
3 changes: 2 additions & 1 deletion info/rules-mapping.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
| 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_DUPLICATE_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 |
| KDOC_WITHOUT_RETURN_TAG | [2.1.2](guide/diktat-coding-convention.md#r2.1.2) | yes | Comments |
Expand Down Expand Up @@ -126,4 +127,4 @@
| USE_LAST_INDEX | [6.2.4](guide/diktat-coding-convention.md#r6.2.4) | yes | Classes |
| AVOID_USING_UTILITY_CLASS | [6.4.1](guide/diktat-coding-convention.md#r6.4.1) | no | Classes |
| OBJECT_IS_PREFERRED | [6.4.2](guide/diktat-coding-convention.md#r6.4.2) | yes | Classes |
| RUN_IN_SCRIPT | [6.5.1](guide/diktat-coding-convention.md#r6.5.1) | yes | Classes |
| RUN_IN_SCRIPT | [6.5.1](guide/diktat-coding-convention.md#r6.5.1) | yes | Classes |