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

Fixed KdocComments for types in generic classes #1832

Merged
merged 4 commits into from
Nov 29, 2023
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
1 change: 1 addition & 0 deletions diktat-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,7 @@
configuration:
isParamTagsForParameters: false # create param tags for parameters without val or var
isParamTagsForPrivateProperties: false # create param tags for private properties
isParamTagsForGenericTypes: false # create param tags for generic types
# Checks that property in KDoc present in class
- name: KDOC_EXTRA_PROPERTY
enabled: true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import org.jetbrains.kotlin.KtNodeTypes.FUN
import org.jetbrains.kotlin.KtNodeTypes.MODIFIER_LIST
import org.jetbrains.kotlin.KtNodeTypes.PRIMARY_CONSTRUCTOR
import org.jetbrains.kotlin.KtNodeTypes.PROPERTY
import org.jetbrains.kotlin.KtNodeTypes.TYPE_PARAMETER
import org.jetbrains.kotlin.KtNodeTypes.TYPE_PARAMETER_LIST
import org.jetbrains.kotlin.KtNodeTypes.TYPE_REFERENCE
import org.jetbrains.kotlin.KtNodeTypes.VALUE_PARAMETER
import org.jetbrains.kotlin.KtNodeTypes.VALUE_PARAMETER_LIST
Expand Down Expand Up @@ -76,8 +78,6 @@ class KdocComments(configRules: List<RulesConfig>) : DiktatRule(
when (node.elementType) {
KtFileElementType.INSTANCE -> checkTopLevelDoc(node)
CLASS -> checkClassElements(node)
VALUE_PARAMETER -> checkValueParameter(node)
PRIMARY_CONSTRUCTOR -> checkParameterList(node.findChildByType(VALUE_PARAMETER_LIST))
BLOCK -> checkKdocPresent(node)
else -> {
// this is a generated else block
Expand All @@ -102,28 +102,42 @@ class KdocComments(configRules: List<RulesConfig>) : DiktatRule(
}
}

private fun checkParameterList(node: ASTNode?) {
val kdocBeforeClass = node
?.parent { it.elementType == CLASS }
?.findChildByType(KDOC) ?: return
private fun checkParameterList(
classNode: ASTNode,
typeParameterListNode: ASTNode?,
valueParameterListNode: ASTNode?
) {
if (typeParameterListNode == null && valueParameterListNode == null) {
return
}

val kdocBeforeClass = classNode.findChildByType(KDOC)
?: return

val parametersInKdoc = kdocBeforeClass
.kDocTags()
.filter { it.knownTag == KDocKnownTag.PROPERTY || it.knownTag == KDocKnownTag.PARAM }

val parametersInConstructor = node
.findChildrenMatching { it.elementType == VALUE_PARAMETER }
.mapNotNull { it.findChildByType(IDENTIFIER)?.text }
val getParametersFromListNode = { parameterListNode: ASTNode? ->
parameterListNode?.let { node ->
val parameterType = if (parameterListNode.elementType == TYPE_PARAMETER_LIST) TYPE_PARAMETER else VALUE_PARAMETER

node.findChildrenMatching { it.elementType == parameterType }
.mapNotNull { it.findChildByType(IDENTIFIER)?.text }
} ?: emptyList()
}

val parametersInTypeParameterList = getParametersFromListNode(typeParameterListNode)
val parametersInValueParameterList = getParametersFromListNode(valueParameterListNode)

parametersInKdoc
.filterNot { it.getSubjectName() == null || it.getSubjectName() in parametersInConstructor }
.forEach { KDOC_EXTRA_PROPERTY.warn(configRules, emitWarn, it.text, it.node.startOffset, node) }
.filter { it.getSubjectName() != null && it.getSubjectName() !in (parametersInTypeParameterList + parametersInValueParameterList) }
.forEach { KDOC_EXTRA_PROPERTY.warn(configRules, emitWarn, it.text, it.node.startOffset, classNode) }
}

@Suppress("UnsafeCallOnNullableType", "ComplexMethod")
private fun checkValueParameter(valueParameterNode: ASTNode) {
if (valueParameterNode.parents().none { it.elementType == PRIMARY_CONSTRUCTOR } ||
valueParameterNode.parents().any { it.elementType == TYPE_REFERENCE }) {
private fun checkValueParameter(classNode: ASTNode, valueParameterNode: ASTNode) {
if (valueParameterNode.parents().any { it.elementType == TYPE_REFERENCE }) {
return
}

Expand All @@ -139,7 +153,7 @@ class KdocComments(configRules: List<RulesConfig>) : DiktatRule(
null
}

val kdocBeforeClass = valueParameterNode.parent { it.elementType == CLASS }!!.findChildByType(KDOC)
val kdocBeforeClass = classNode.findChildByType(KDOC)
val isParamTagNeeded = (!valueParameterNode.hasChildOfType(VAL_KEYWORD) && !valueParameterNode.hasChildOfType(VAR_KEYWORD)) ||
!valueParameterNode.getFirstChildWithType(MODIFIER_LIST).isAccessibleOutside()

Expand All @@ -155,6 +169,15 @@ class KdocComments(configRules: List<RulesConfig>) : DiktatRule(
?: createKdocBasicKdoc(valueParameterNode, isParamTagNeeded)
}

@Suppress("UnsafeCallOnNullableType", "ComplexMethod")
private fun checkTypeParameter(classNode: ASTNode, typeParameterNode: ASTNode) {
val kdocBeforeClass = classNode.findChildByType(KDOC)

kdocBeforeClass?.let {
checkBasicKdocBeforeClass(typeParameterNode, kdocBeforeClass, true)
} ?: createKdocBasicKdoc(typeParameterNode, true)
}

@Suppress("UnsafeCallOnNullableType", "ComplexMethod")
private fun checkBasicKdocBeforeClass(
node: ASTNode,
Expand Down Expand Up @@ -182,10 +205,7 @@ class KdocComments(configRules: List<RulesConfig>) : DiktatRule(
}
}
} ?: run {
val isParameter = !node.hasChildOfType(VAL_KEYWORD) && !node.hasChildOfType(VAR_KEYWORD)
val isPrivateProperty = !node.getFirstChildWithType(MODIFIER_LIST).isAccessibleOutside()

if (!isParamTagNeeded || (isParameter && configuration.isParamTagsForParameters) || (isPrivateProperty && configuration.isParamTagsForPrivateProperties)) {
if (isNeedToWarn(node, isParamTagNeeded)) {
val warningText = if (isParamTagNeeded) "add param <$parameterName> to KDoc" else "add property <$parameterName> to KDoc"

KDOC_NO_CONSTRUCTOR_PROPERTY.warnAndFix(configRules, emitWarn, isFixMode, warningText, node.startOffset, node) {
Expand All @@ -196,6 +216,17 @@ class KdocComments(configRules: List<RulesConfig>) : DiktatRule(
}
}

private fun isNeedToWarn(node: ASTNode, isParamTagNeeded: Boolean): Boolean {
val isParameter = node.elementType == VALUE_PARAMETER && !node.hasChildOfType(VAL_KEYWORD) && !node.hasChildOfType(VAR_KEYWORD)
val isPrivateProperty = node.elementType == VALUE_PARAMETER && !node.getFirstChildWithType(MODIFIER_LIST).isAccessibleOutside()
val isTypeParameterNode = node.elementType == TYPE_PARAMETER

return !isParamTagNeeded ||
(isParameter && configuration.isParamTagsForParameters) ||
(isPrivateProperty && configuration.isParamTagsForPrivateProperties) ||
(isTypeParameterNode && configuration.isParamTagsForGenericTypes)
}

private fun replaceWrongTagInClassKdoc(
kdocBeforeClass: ASTNode,
parameterName: String,
Expand Down Expand Up @@ -271,10 +302,7 @@ class KdocComments(configRules: List<RulesConfig>) : DiktatRule(

@Suppress("UnsafeCallOnNullableType")
private fun createKdocBasicKdoc(node: ASTNode, isParamTagNeeded: Boolean) {
val isParameter = !node.hasChildOfType(VAL_KEYWORD) && !node.hasChildOfType(VAR_KEYWORD)
val isPrivateProperty = !node.getFirstChildWithType(MODIFIER_LIST).isAccessibleOutside()

if (!isParamTagNeeded || (isParameter && configuration.isParamTagsForParameters) || (isPrivateProperty && configuration.isParamTagsForPrivateProperties)) {
if (isNeedToWarn(node, isParamTagNeeded)) {
val parameterName = node.findChildByType(IDENTIFIER)!!.text
val warningText = if (isParamTagNeeded) "add param <$parameterName> to KDoc" else "add property <$parameterName> to KDoc"

Expand All @@ -295,10 +323,7 @@ class KdocComments(configRules: List<RulesConfig>) : DiktatRule(
prevComment: ASTNode,
isParamTagNeeded: Boolean
) {
val isParameter = !node.hasChildOfType(VAL_KEYWORD) && !node.hasChildOfType(VAR_KEYWORD)
val isPrivateProperty = !node.getFirstChildWithType(MODIFIER_LIST).isAccessibleOutside()

if (!isParamTagNeeded || (isParameter && configuration.isParamTagsForParameters) || (isPrivateProperty && configuration.isParamTagsForPrivateProperties)) {
if (isNeedToWarn(node, isParamTagNeeded)) {
val parameterName = node.findChildByType(IDENTIFIER)!!.text
val classNode = node.parent { it.elementType == CLASS }!!

Expand Down Expand Up @@ -409,10 +434,7 @@ class KdocComments(configRules: List<RulesConfig>) : DiktatRule(
}
}
?: run {
val isParameter = !node.hasChildOfType(VAL_KEYWORD) && !node.hasChildOfType(VAR_KEYWORD)
val isPrivateProperty = !node.getFirstChildWithType(MODIFIER_LIST).isAccessibleOutside()

if (!isParamTagNeeded || (isParameter && configuration.isParamTagsForParameters) || (isPrivateProperty && configuration.isParamTagsForPrivateProperties)) {
if (isNeedToWarn(node, isParamTagNeeded)) {
KDOC_NO_CONSTRUCTOR_PROPERTY_WITH_COMMENT.warnOnlyOrWarnAndFix(configRules, emitWarn, warningText, prevComment.startOffset, node, isFixable, isFixMode) {
val newKdocText = if (isParamTagNeeded) "* @param $parameterName$commentText\n " else "* @property $parameterName$commentText\n "
insertTextInKdoc(kdocBeforeClass, checkOneNewLineAfterKdocClassDescription(kdocBeforeClass, newKdocText, isFirstTagInKdoc))
Expand Down Expand Up @@ -462,10 +484,7 @@ class KdocComments(configRules: List<RulesConfig>) : DiktatRule(
}
}
?: run {
val isParameter = !node.hasChildOfType(VAL_KEYWORD) && !node.hasChildOfType(VAR_KEYWORD)
val isPrivateProperty = !node.getFirstChildWithType(MODIFIER_LIST).isAccessibleOutside()

if (!isParamTagNeeded || (isParameter && configuration.isParamTagsForParameters) || (isPrivateProperty && configuration.isParamTagsForPrivateProperties)) {
if (isNeedToWarn(node, isParamTagNeeded)) {
KDOC_NO_CONSTRUCTOR_PROPERTY_WITH_COMMENT.warnAndFix(configRules, emitWarn, isFixMode, warningText, prevComment.startOffset, node) {
val newKdocText = if (isParamTagNeeded) {
"* @param $parameterName ${createClassKdocTextFromEolComment(prevComment)}\n "
Expand Down Expand Up @@ -540,12 +559,25 @@ class KdocComments(configRules: List<RulesConfig>) : DiktatRule(
} ?: kdocTagNode.addChild(LeafPsiElement(KDocTokens.TEXT, content), null)
}

private fun checkClassElements(node: ASTNode) {
val modifier = node.getFirstChildWithType(MODIFIER_LIST)
val classBody = node.getFirstChildWithType(CLASS_BODY)
private fun checkClassElements(classNode: ASTNode) {
val modifierList = classNode.getFirstChildWithType(MODIFIER_LIST)
val typeParameterList = classNode.getFirstChildWithType(TYPE_PARAMETER_LIST)
val valueParameterList = classNode.getFirstChildWithType(PRIMARY_CONSTRUCTOR)?.getFirstChildWithType(VALUE_PARAMETER_LIST)
val classBody = classNode.getFirstChildWithType(CLASS_BODY)
val classKdoc = classNode.getFirstChildWithType(KDOC)

checkParameterList(classNode, typeParameterList, valueParameterList)

typeParameterList
?.findChildrenMatching { it.elementType == TYPE_PARAMETER }
?.forEach { checkTypeParameter(classNode, it) }

valueParameterList
?.findChildrenMatching { it.elementType == VALUE_PARAMETER }
?.forEach { checkValueParameter(classNode, it) }

// if parent class is public or internal than we can check it's internal code elements
if (classBody != null && modifier.isAccessibleOutside()) {
if (classBody != null && modifierList.isAccessibleOutside()) {
classBody
.getChildren(statementsToDocument)
.filterNot {
Expand All @@ -555,7 +587,6 @@ class KdocComments(configRules: List<RulesConfig>) : DiktatRule(
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
}
Expand Down Expand Up @@ -614,6 +645,11 @@ class KdocComments(configRules: List<RulesConfig>) : DiktatRule(
* Create param tags for private properties
*/
val isParamTagsForPrivateProperties = config["isParamTagsForPrivateProperties"]?.toBoolean() ?: true

/**
* Create param tags for generic types
*/
val isParamTagsForGenericTypes = config["isParamTagsForGenericTypes"]?.toBoolean() ?: true
}

companion object {
Expand Down
1 change: 1 addition & 0 deletions diktat-rules/src/main/resources/diktat-analysis-huawei.yml
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,7 @@
configuration:
isParamTagsForParameters: false # create param tags for parameters without val or var
isParamTagsForPrivateProperties: false # create param tags for private properties
isParamTagsForGenericTypes: false # create param tags for generic types
# Checks that property in KDoc present in class
- name: KDOC_EXTRA_PROPERTY
enabled: true
Expand Down
1 change: 1 addition & 0 deletions diktat-rules/src/main/resources/diktat-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,7 @@
configuration:
isParamTagsForParameters: true # create param tags for parameters without val or var
isParamTagsForPrivateProperties: true # create param tags for private properties
isParamTagsForGenericTypes: true # create param tags for generic types
# Checks that the long lambda has parameters
- name: TOO_MANY_LINES_IN_LAMBDA
enabled: true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -817,4 +817,67 @@ class KdocCommentsWarnTest : LintTestBase(::KdocComments) {
DiktatError(5, 4, ruleId, "${KDOC_DUPLICATE_PROPERTY.warnText()} @param field2"),
)
}

@Test
@Tag(WarningNames.KDOC_EXTRA_PROPERTY)
fun `shouldn't warn extra property on generic type`() {
lintMethod(
"""
|/**
| * S3 implementation of Storage
| *
| * @param s3Operations [S3Operations] to operate with S3
| * @param K type of key
| */
|abstract class AbstractReactiveStorage<K : Any> constructor(
| s3Operations: S3Operations,
|) : ReactiveStorage<K> {
| // abcd
|}
""".trimMargin()
)
}

@Test
@Tag(WarningNames.KDOC_NO_CONSTRUCTOR_PROPERTY)
fun `should trigger on generic type`() {
lintMethod(
"""
|/**
| * S3 implementation of Storage
| *
| * @param s3Operations [S3Operations] to operate with S3
| */
|abstract class AbstractReactiveStorage<K : Any, P: Any>(
| s3Operations: S3Operations,
|) : ReactiveStorage<K>, AnotherStorage<P> {
| // abcd
|}
""".trimMargin(),
DiktatError(6, 40, ruleId, "${KDOC_NO_CONSTRUCTOR_PROPERTY.warnText()} add param <K> to KDoc", true),
DiktatError(6, 49, ruleId, "${KDOC_NO_CONSTRUCTOR_PROPERTY.warnText()} add param <P> to KDoc", true),
)
}

@Test
@Tag(WarningNames.KDOC_NO_CONSTRUCTOR_PROPERTY)
fun `change property to param in kdoc for generic type`() {
lintMethod(
"""
|/**
| * S3 implementation of Storage
| *
| * @param s3Operations [S3Operations] to operate with S3
| * @property K type of key
| */
|abstract class AbstractReactiveStorage<K : Any, P: Any>(
| s3Operations: S3Operations,
|) : ReactiveStorage<K>, AnotherStorage<P> {
| // abcd
|}
""".trimMargin(),
DiktatError(7, 40, ruleId, "${KDOC_NO_CONSTRUCTOR_PROPERTY.warnText()} change `@property` tag to `@param` tag for <K> to KDoc", true),
DiktatError(7, 49, ruleId, "${KDOC_NO_CONSTRUCTOR_PROPERTY.warnText()} add param <P> to KDoc", true),
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ class A constructor(
* class
* comment
*
* @param K
* @property openName single-line comment
* @property openLastName
* block
Expand All @@ -196,7 +197,7 @@ class A constructor(
* kdoc property
* comment
*/
open class B constructor(
open class B<K : Any> constructor(
open val openName: String,
open val openLastName: String,
open val openBirthDate: String,
Expand All @@ -212,6 +213,9 @@ open class B constructor(
* class
* comment
*
* @param K
* @param P
* @param G
* @param privateName single-line comment
* @property protectedName single-line comment
* @property internalName single-line comment
Expand Down Expand Up @@ -255,7 +259,7 @@ open class B constructor(
* kdoc property
* comment
*/
class A constructor(
class A<K : Any, P: Any, G: Any> constructor(
private val privateName: String,
protected val protectedName: String,
internal val internalName: String,
Expand Down Expand Up @@ -304,4 +308,4 @@ class A constructor(
* comment
*/
paramAddr: String,
) : B() {}
) : B<K>(), C<P>, D<G> {}
Loading
Loading