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

Kdoc fixer false-negative: not generating a Kdoc for several public functions #989

Merged
merged 7 commits into from
Jul 19, 2021
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,8 @@ class HeaderCommentRule(configRules: List<RulesConfig>) : DiktatRule(

/**
* Whether copyright text is present in the configuration
*
* @return true if config has "copyrightText"
*/
internal fun hasCopyrightText() = config.keys.contains("copyrightText")

Expand Down
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,26 @@ class KdocMethods(configRules: List<RulesConfig>) : DiktatRule(
}
}

private fun ASTNode.isSingleLineGetterOrSetter() = isGetterOrSetter() && (expressionBodyTypes.any { hasChildOfType(it) } || getBodyLines().size == 1)
private fun ASTNode.isSingleLineGetterOrSetter(): Boolean {
val dotQualifiedExp = this.findChildByType(DOT_QUALIFIED_EXPRESSION)?.psi?.let { it as KtDotQualifiedExpression }
val isThisExpression = dotQualifiedExp != null && dotQualifiedExp.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 @@ -303,6 +303,7 @@ class IndentationRule(configRules: List<RulesConfig>) : DiktatRule(

/**
* @param token a token that caused indentation increment, for example an opening brace
* @return Unit
*/
fun storeIncrementingToken(token: IElementType) = token
.also { require(it in increasingTokens) { "Only tokens that increase indentation should be passed to this method" } }
Expand Down Expand Up @@ -342,6 +343,7 @@ class IndentationRule(configRules: List<RulesConfig>) : DiktatRule(
* @param initiator a node that caused exceptional indentation
* @param indent an additional indent
* @param includeLastChild whether the last child node should be included in the range affected by exceptional indentation
* @return true if add exception in exceptionalIndents
*/
fun addException(
initiator: ASTNode,
Expand All @@ -351,6 +353,7 @@ class IndentationRule(configRules: List<RulesConfig>) : DiktatRule(

/**
* @param astNode the node which is used to determine whether exceptinoal indents are still active
* @return boolean result
*/
fun checkAndReset(astNode: ASTNode) = exceptionalIndents.retainAll { it.isActive(astNode) }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ class KotlinParser {

/**
* @param text kotlin code
* @return [KtPrimaryConstructor]
*/
fun createPrimaryConstructor(text: String) = ktPsiFactory.createPrimaryConstructor(text)

Expand Down
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)
}
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ class TestArgumentsReader(

/**
* Whether all tests should be run
*
* @return true if command has option "all"
*/
fun shouldRunAllTests() = cmd.hasOption("all")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ open class TestCompare : TestBase {

/**
* Get result of the test execution
*
* @return list stdOut
*/
protected open fun getExecutionResult() = testResult.stdOut
}