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

NullPointerException on KDOC inspection #1334

Merged
merged 13 commits into from
Jun 2, 2022
Original file line number Diff line number Diff line change
Expand Up @@ -371,8 +371,8 @@ class IdentifierNaming(configRules: List<RulesConfig>) : DiktatRule(
* 4) backticks are prohibited in the naming of non-test methods
*/
@Suppress("UnsafeCallOnNullableType")
private fun checkFunctionName(node: ASTNode): List<ASTNode> {
val functionName = node.getIdentifierName()!!
private fun checkFunctionName(node: ASTNode): List<ASTNode>? {
val functionName = node.getIdentifierName() ?: return null

// basic check for camel case
if (!functionName.text.isLowerCamelCase()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import org.cqfn.diktat.ruleset.utils.hasKnownKdocTag
import org.cqfn.diktat.ruleset.utils.hasTestAnnotation
import org.cqfn.diktat.ruleset.utils.insertTagBefore
import org.cqfn.diktat.ruleset.utils.isAccessibleOutside
import org.cqfn.diktat.ruleset.utils.isAnonymousFunction
import org.cqfn.diktat.ruleset.utils.isGetterOrSetter
import org.cqfn.diktat.ruleset.utils.isLocatedInTest
import org.cqfn.diktat.ruleset.utils.isOverridden
Expand Down Expand Up @@ -88,7 +89,7 @@ class KdocMethods(configRules: List<RulesConfig>) : DiktatRule(
val config = configRules.getCommonConfiguration()
val filePath = node.getFilePath()
val isTestMethod = node.hasTestAnnotation() || isLocatedInTest(filePath.splitPathToDirs(), config.testAnchors)
if (!isTestMethod && !node.isStandardMethod() && !node.isSingleLineGetterOrSetter()) {
if (!isTestMethod && !node.isStandardMethod() && !node.isSingleLineGetterOrSetter() && !node.isAnonymousFunction()) {
checkSignatureDescription(node)
}
} else if (node.elementType == KDOC_SECTION) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import org.cqfn.diktat.ruleset.utils.findAllDescendantsWithSpecificType
import org.cqfn.diktat.ruleset.utils.getFirstChildWithType
import org.cqfn.diktat.ruleset.utils.hasChildOfType
import org.cqfn.diktat.ruleset.utils.hasParent
import org.cqfn.diktat.ruleset.utils.isAnonymousFunction

import com.pinterest.ktlint.core.ast.ElementType.CLASS_BODY
import com.pinterest.ktlint.core.ast.ElementType.FUN
Expand Down Expand Up @@ -65,7 +66,8 @@ class AvoidNestedFunctionsRule(configRules: List<RulesConfig>) : DiktatRule(
}

private fun isNestedFunction(node: ASTNode): Boolean =
node.hasParent(FUN) && node.hasFunParentUntil(CLASS_BODY) && !node.hasChildOfType(MODIFIER_LIST)
node.hasParent(FUN) && node.hasFunParentUntil(CLASS_BODY) && !node.hasChildOfType(MODIFIER_LIST) &&
!node.isAnonymousFunction()

private fun ASTNode.hasFunParentUntil(stopNode: IElementType): Boolean =
parents()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import com.pinterest.ktlint.core.ast.ElementType.EOL_COMMENT
import com.pinterest.ktlint.core.ast.ElementType.EQ
import com.pinterest.ktlint.core.ast.ElementType.FILE
import com.pinterest.ktlint.core.ast.ElementType.FILE_ANNOTATION_LIST
import com.pinterest.ktlint.core.ast.ElementType.FUN
import com.pinterest.ktlint.core.ast.ElementType.IMPORT_LIST
import com.pinterest.ktlint.core.ast.ElementType.INTERNAL_KEYWORD
import com.pinterest.ktlint.core.ast.ElementType.KDOC
Expand Down Expand Up @@ -159,6 +160,15 @@ fun ASTNode.replaceWhiteSpaceText(beforeNode: ASTNode, text: String) {
fun ASTNode.getFirstChildWithType(elementType: IElementType): ASTNode? =
this.findChildByType(elementType)

/**
* Checks if a function is anonymous
* throws exception if the node type is different from FUN
*/
fun ASTNode.isAnonymousFunction(): Boolean {
require(this.elementType == FUN)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can it be a part of checking of this function instead of assumption?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's needed here. We already everywhere have a check that this value is equal to FUN before calling this method

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so this function treats to node to be a FUN. it means that it's can be called only
node.elementType == FUN && node.isAnonymousFunction(), otherwise -- exception

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we then move it to KdocMethods.kt as private method, because it contains a specific logic for your cases only

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's the point. We do not want the developer to try to call this function on a different type of node, In this case, we will not know what to return

return this.getIdentifierName() == null
}

/**
* Checks if the symbols in this node are at the end of line
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,20 @@ class IdentifierNamingWarnTest : LintTestBase(::IdentifierNaming) {
lintMethod(code)
}

@Test
@Tag(WarningNames.GENERIC_NAME)
fun `anonymous function`() {
val code = """
package org.cqfn.diktat.test

fun foo() {
val sum: (Int) -> Int = fun(x): Int = x + x
}

""".trimIndent()
lintMethod(code)
}

@Test
@Tag(WarningNames.GENERIC_NAME)
fun `generic class - single capital letter, can be followed by a number (check - negative1)`() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,22 @@ class KdocMethodsTest : LintTestBase(::KdocMethods) {
lintMethod(validCode)
}

@Test
@Tag(WarningNames.MISSING_KDOC_TOP_LEVEL)
fun `anonymous function`() {
val code = """
package org.cqfn.diktat.test

fun foo() {
val sum: (Int) -> Int = fun(x): Int = x + x
}

""".trimIndent()
lintMethod(code,
LintError(3, 1, ruleId, "${MISSING_KDOC_ON_FUNCTION.warnText()} foo", false),
)
}

@Test
@Tags(
Tag(WarningNames.KDOC_WITHOUT_PARAM_TAG),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import org.cqfn.diktat.ruleset.rules.chapter5.AvoidNestedFunctionsRule
import org.cqfn.diktat.util.LintTestBase

import com.pinterest.ktlint.core.LintError
import generated.WarningNames
import generated.WarningNames.AVOID_NESTED_FUNCTIONS
import org.junit.jupiter.api.Tag
import org.junit.jupiter.api.Test
Expand Down Expand Up @@ -35,6 +36,20 @@ class AvoidNestedFunctionsWarnTest : LintTestBase(::AvoidNestedFunctionsRule) {
)
}

@Test
@Tag(AVOID_NESTED_FUNCTIONS)
fun `anonymous function`() {
val code = """
package org.cqfn.diktat.test

fun foo() {
val sum: (Int) -> Int = fun(x): Int = x + x
}

""".trimIndent()
lintMethod(code)
}

@Test
@Tag(AVOID_NESTED_FUNCTIONS)
fun `several nested functions`() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,12 @@ class DiktatSmokeTest : FixTestBase("test/smoke/src/main/kotlin",
fixAndCompareSmokeTest("DefaultPackageExpected.kt", "DefaultPackageTest.kt")
}

@Test
@Tag("DiktatRuleSetProvider")
fun `smoke test #8 - anonymous function`() {
fixAndCompareSmokeTest("Example8Expected.kt", "Example8Test.kt")
}

@Test
@Tag("DiktatRuleSetProvider")
fun `smoke test #7`() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package org.cqfn.diktat

fun foo() {
val sum: (Int, Int, Int,) -> Int = fun(
Copy link
Member

@orchestr7 orchestr7 Jun 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please also add a unit test (at list for checks), even duplicated. Not only smoke is needed

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added

x,
y,
z
): Int = x + y + x
println(sum(8, 8, 8))
}

fun boo() {
val message = fun() = println("Hello")
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package org.cqfn.diktat

fun foo() {
val sum: (Int, Int, Int,) -> Int = fun(
x,
y,
z
): Int {
return x + y + x
}
println(sum(8, 8, 8))
}

fun boo() {
val message = fun()=println("Hello")
}