Skip to content

Commit

Permalink
Kdoc fixer false-negative: not generating a Kdoc for several public f…
Browse files Browse the repository at this point in the history
…unctions

### What's done:
* fixed bug in MISSING_KDOC_ON_FUNCTION
Closes #965
  • Loading branch information
Cheshiriks committed Jul 16, 2021
1 parent a4ce78f commit 347b8ba
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,30 +29,29 @@ import org.cqfn.diktat.ruleset.utils.kDocTags
import org.cqfn.diktat.ruleset.utils.parameterNames
import org.cqfn.diktat.ruleset.utils.splitPathToDirs

import com.pinterest.ktlint.core.ast.ElementType.BINARY_EXPRESSION
import com.pinterest.ktlint.core.ast.ElementType
import com.pinterest.ktlint.core.ast.ElementType.BLOCK
import com.pinterest.ktlint.core.ast.ElementType.CALLABLE_REFERENCE_EXPRESSION
import com.pinterest.ktlint.core.ast.ElementType.CALL_EXPRESSION
import com.pinterest.ktlint.core.ast.ElementType.COLLECTION_LITERAL_EXPRESSION
import com.pinterest.ktlint.core.ast.ElementType.COLON
import com.pinterest.ktlint.core.ast.ElementType.DOT_QUALIFIED_EXPRESSION
import com.pinterest.ktlint.core.ast.ElementType.EQ
import com.pinterest.ktlint.core.ast.ElementType.FUN
import com.pinterest.ktlint.core.ast.ElementType.KDOC
import com.pinterest.ktlint.core.ast.ElementType.KDOC_SECTION
import com.pinterest.ktlint.core.ast.ElementType.KDOC_TAG_NAME
import com.pinterest.ktlint.core.ast.ElementType.KDOC_TEXT
import com.pinterest.ktlint.core.ast.ElementType.LAMBDA_EXPRESSION
import com.pinterest.ktlint.core.ast.ElementType.MODIFIER_LIST
import com.pinterest.ktlint.core.ast.ElementType.REFERENCE_EXPRESSION
import com.pinterest.ktlint.core.ast.ElementType.SAFE_ACCESS_EXPRESSION
import com.pinterest.ktlint.core.ast.ElementType.THIS_EXPRESSION
import com.pinterest.ktlint.core.ast.ElementType.THROW
import com.pinterest.ktlint.core.ast.ElementType.TYPE_REFERENCE
import com.pinterest.ktlint.core.ast.ElementType.WHEN_CONDITION_WITH_EXPRESSION
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.kdoc.psi.impl.KDocTag
import org.jetbrains.kotlin.psi.KtDotQualifiedExpression
import org.jetbrains.kotlin.psi.KtThrowExpression
import org.jetbrains.kotlin.psi.psiUtil.referenceExpression

Expand Down Expand Up @@ -143,11 +142,12 @@ class KdocMethods(configRules: List<RulesConfig>) : DiktatRule(
}

val explicitReturnType = node.findChildAfter(COLON, TYPE_REFERENCE)
val hasNotExpressionBodyTypes = allExpressionBodyTypes.any { node.hasChildOfType(it) }
val hasExplicitNotUnitReturnType = explicitReturnType != null && explicitReturnType.text != "Unit"
val hasExplicitUnitReturnType = explicitReturnType != null && explicitReturnType.text == "Unit"
val isFunWithExpressionBody = expressionBodyTypes.any { node.hasChildOfType(it) }
val isFunWithExpressionBody = node.hasChildOfType(EQ)
val hasReturnKdoc = kdocTags != null && kdocTags.hasKnownKdocTag(KDocKnownTag.RETURN)
return (hasExplicitNotUnitReturnType || isFunWithExpressionBody && !hasExplicitUnitReturnType) && !hasReturnKdoc
return (hasExplicitNotUnitReturnType || isFunWithExpressionBody && !hasExplicitUnitReturnType && hasNotExpressionBodyTypes) && !hasReturnKdoc
}

private fun getExplicitlyThrownExceptions(node: ASTNode): Set<String> {
Expand Down Expand Up @@ -251,12 +251,27 @@ class KdocMethods(configRules: List<RulesConfig>) : DiktatRule(
}
}

private fun ASTNode.isSingleLineGetterOrSetter() = isGetterOrSetter() && (expressionBodyTypes.any { hasChildOfType(it) } || getBodyLines().size == 1)
private fun ASTNode.isSingleLineGetterOrSetter(): Boolean {
val isDotQualifiedExp = this.findChildByType(DOT_QUALIFIED_EXPRESSION)?.psi
val isThisExpression = isDotQualifiedExp != null && (isDotQualifiedExp as KtDotQualifiedExpression).receiverExpression.node.elementType == THIS_EXPRESSION
val isExpressionBodyTypes = expressionBodyTypes.any { hasChildOfType(it) }
return isGetterOrSetter() && (isExpressionBodyTypes || getBodyLines().size == 1 || isThisExpression)
}

companion object {
// expression body of function can have a lot of 'ElementType's, this list might be not full
private val expressionBodyTypes = setOf(BINARY_EXPRESSION, CALL_EXPRESSION, LAMBDA_EXPRESSION, REFERENCE_EXPRESSION,
CALLABLE_REFERENCE_EXPRESSION, SAFE_ACCESS_EXPRESSION, WHEN_CONDITION_WITH_EXPRESSION, COLLECTION_LITERAL_EXPRESSION)
private val expressionBodyTypes = setOf(CALL_EXPRESSION, REFERENCE_EXPRESSION)
private val allExpressionBodyTypes = setOf(
DOT_QUALIFIED_EXPRESSION,
CALL_EXPRESSION,
REFERENCE_EXPRESSION,
ElementType.BINARY_EXPRESSION,
ElementType.LAMBDA_EXPRESSION,
ElementType.CALLABLE_REFERENCE_EXPRESSION,
ElementType.SAFE_ACCESS_EXPRESSION,
ElementType.WHEN_CONDITION_WITH_EXPRESSION,
ElementType.COLLECTION_LITERAL_EXPRESSION
)
private val uselessKdocRegex = """^([rR]eturn|[gGsS]et)[s]?\s+\w+(\s+\w+)?$""".toRegex()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@ class KdocMethodsFixTest : FixTestBase("test/paragraph2/kdoc/package/src/main/ko
fixAndCompare("MissingKdocWithModifiersExpected.kt", "MissingKdocWithModifiersTest.kt")
}

@Test
@Tag(WarningNames.MISSING_KDOC_ON_FUNCTION)
fun `KDoc should be for function with single line body`() {
fixAndCompare("MissingKdocOnFunctionExpected.kt", "MissingKdocOnFunctionTest.kt")
}

@Test
@Tag(WarningNames.KDOC_EMPTY_KDOC)
fun `Rule should not suggest empty KDoc templates`() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ class KdocMethodsTest : LintTestBase(::KdocMethods) {
| this.x = x
| }
|
| fun getX() {
| fun getX(): Type {
| return x
| }
|
Expand All @@ -309,7 +309,6 @@ class KdocMethodsTest : LintTestBase(::KdocMethods) {
| }
|}
""".trimMargin(),
LintError(10, 5, ruleId, "${MISSING_KDOC_ON_FUNCTION.warnText()} getY", false),
LintError(12, 5, ruleId, "${MISSING_KDOC_ON_FUNCTION.warnText()} setY", true),
LintError(17, 5, ruleId, "${MISSING_KDOC_ON_FUNCTION.warnText()} getZ", true)
)
Expand Down Expand Up @@ -368,4 +367,17 @@ class KdocMethodsTest : LintTestBase(::KdocMethods) {
LintError(1, 1, ruleId, "${MISSING_KDOC_ON_FUNCTION.warnText()} foo", true)
)
}

@Test
@Tag(WarningNames.MISSING_KDOC_ON_FUNCTION)
fun `KDoc should be for function with single line body`() {
lintMethod(
"""
|fun hasNoChildren() = children.size == 0
|fun getFirstChild() = children.elementAtOrNull(0)
""".trimMargin(),
LintError(1, 1, ruleId, "${MISSING_KDOC_ON_FUNCTION.warnText()} hasNoChildren", true),
LintError(2, 1, ruleId, "${MISSING_KDOC_ON_FUNCTION.warnText()} getFirstChild", true)
)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package test.paragraph2.kdoc

class Example {
/**
* @return
*/
fun hasNoChildren() = children.size == 0

/**
* @return
*/
fun getFirstChild() = children.elementAtOrNull(0)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package test.paragraph2.kdoc

class Example {
fun hasNoChildren() = children.size == 0

fun getFirstChild() = children.elementAtOrNull(0)
}

0 comments on commit 347b8ba

Please sign in to comment.