Skip to content

Commit

Permalink
KDOC_WITHOUT_THROWS_TAG add @throws annotation when throw inside …
Browse files Browse the repository at this point in the history
…try-catch (#1862)

### What's done:
- Fixed `KDOC_WITHOUT_THROWS_TAG` rule when it adds a @throws annotation to the function, which has `throw` inside try-catch block
- Added tests, using real exceptions

Closes #1718
  • Loading branch information
diphtongue authored Dec 15, 2023
1 parent 02c1a7c commit d14f27d
Show file tree
Hide file tree
Showing 5 changed files with 157 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import com.saveourtool.diktat.ruleset.utils.KotlinParser
import com.saveourtool.diktat.ruleset.utils.appendNewlineMergingWhiteSpace
import com.saveourtool.diktat.ruleset.utils.findAllDescendantsWithSpecificType
import com.saveourtool.diktat.ruleset.utils.findChildAfter
import com.saveourtool.diktat.ruleset.utils.findParentNodeWithSpecificType
import com.saveourtool.diktat.ruleset.utils.getAllChildrenWithType
import com.saveourtool.diktat.ruleset.utils.getBodyLines
import com.saveourtool.diktat.ruleset.utils.getFilePath
import com.saveourtool.diktat.ruleset.utils.getFirstChildWithType
Expand Down Expand Up @@ -40,6 +42,7 @@ import org.jetbrains.kotlin.KtNodeTypes.MODIFIER_LIST
import org.jetbrains.kotlin.KtNodeTypes.REFERENCE_EXPRESSION
import org.jetbrains.kotlin.KtNodeTypes.THIS_EXPRESSION
import org.jetbrains.kotlin.KtNodeTypes.THROW
import org.jetbrains.kotlin.KtNodeTypes.TRY
import org.jetbrains.kotlin.KtNodeTypes.TYPE_REFERENCE
import org.jetbrains.kotlin.com.intellij.lang.ASTFactory
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
Expand All @@ -54,6 +57,7 @@ import org.jetbrains.kotlin.kdoc.psi.impl.KDocTag
import org.jetbrains.kotlin.lexer.KtTokens
import org.jetbrains.kotlin.lexer.KtTokens.COLON
import org.jetbrains.kotlin.lexer.KtTokens.EQ
import org.jetbrains.kotlin.lexer.KtTokens.IDENTIFIER
import org.jetbrains.kotlin.lexer.KtTokens.WHITE_SPACE
import org.jetbrains.kotlin.psi.KtCallExpression
import org.jetbrains.kotlin.psi.KtCatchClause
Expand All @@ -62,6 +66,7 @@ import org.jetbrains.kotlin.psi.KtFunction
import org.jetbrains.kotlin.psi.KtThrowExpression
import org.jetbrains.kotlin.psi.psiUtil.collectDescendantsOfType
import org.jetbrains.kotlin.psi.psiUtil.referenceExpression
import java.lang.Class.forName

/**
* This rule checks that whenever the method has arguments, return value, can throw exceptions,
Expand Down Expand Up @@ -197,11 +202,39 @@ class KdocMethods(configRules: List<RulesConfig>) : DiktatRule(
&& !hasReturnKdoc && !isReferenceExpressionWithSameName
}

private fun isThrowInTryCatchBlock(node: ASTNode?): Boolean {
node ?: return false
val parent = node.findParentNodeWithSpecificType(TRY)
val nodeName = node.findAllDescendantsWithSpecificType(IDENTIFIER).firstOrNull() ?: return false

if (parent?.elementType == TRY) {
val catchNodes = parent?.getAllChildrenWithType(CATCH)
val findNodeWithMatchingCatch = catchNodes?.firstOrNull { catchNode ->
val matchingNodeForCatchNode = catchNode.findAllDescendantsWithSpecificType(IDENTIFIER)
.firstOrNull { catchNodeName ->
nodeName.text == catchNodeName.text || try {
val nodeClass = forName("java.lang.${nodeName.text}")
val nodeInstance = nodeClass.getDeclaredConstructor().newInstance()
val catchNodeClass = forName("java.lang.${catchNodeName.text}")
catchNodeClass.isInstance(nodeInstance)
} catch (e: ClassNotFoundException) {
false
}
}
matchingNodeForCatchNode != null
}
return findNodeWithMatchingCatch != null
}
return false
}

private fun getExplicitlyThrownExceptions(node: ASTNode): Set<String> {
val codeBlock = node.getFirstChildWithType(BLOCK)
val throwKeywords = codeBlock?.findAllDescendantsWithSpecificType(THROW)

return throwKeywords
?.asSequence()
?.filter { !isThrowInTryCatchBlock(it) }
?.map { it.psi as KtThrowExpression }
?.filter {
// we only take freshly created exceptions here: `throw IAE("stuff")` vs `throw e`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,4 +61,10 @@ class KdocMethodsFixTest : FixTestBase("test/paragraph2/kdoc/package/src/main/ko
fun `KdocMethods rule should reformat code (full example)`() {
fixAndCompare("KdocMethodsFullExpected.kt", "KdocMethodsFullTested.kt")
}

@Test
@Tag(WarningNames.KDOC_WITHOUT_THROWS_TAG)
fun `Should add throws tag only for throw without catch`() {
fixAndCompare("KdocWithoutThrowsTagExpected.kt", "KdocWithoutThrowsTagTested.kt")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,81 @@ class KdocMethodsTest : LintTestBase(::KdocMethods) {
)
}

@Test
@Tag(WarningNames.KDOC_WITHOUT_THROWS_TAG)
fun `No warning when throw has matching catch`() {
lintMethod(
"""
/**
* Test method
* @param a: Int - dummy integer
* @return doubled value
*/
fun foo(a: Int): Int {
try {
if (a < 0)
throw NumberFormatExecption()
} catch (e: ArrayIndexOutOfBounds) {
print(1)
} catch (e: NullPointerException) {
print(2)
} catch (e: NumberFormatExecption) {
print(3)
}
return 2 * a
}
""".trimIndent())
}

@Test
@Tag(WarningNames.KDOC_WITHOUT_THROWS_TAG)
fun `Warning when throw doesn't have matching catch`() {
lintMethod(
"""
/**
* Test method
* @param a: Int - dummy integer
* @return doubled value
*/
fun foo(a: Int): Int {
try {
if (a < 0)
throw NumberFormatException()
throw NullPointerException()
throw NoSuchElementException()
} catch (e: NoSuchElementException) {
print(1)
} catch (e: IllegalArgumentException) {
print(2)
}
return 2 * a
}
""".trimIndent(),
DiktatError(1, 1, ruleId, "${KDOC_WITHOUT_THROWS_TAG.warnText()} foo (NullPointerException)", true))
}

@Test
@Tag(WarningNames.KDOC_WITHOUT_THROWS_TAG)
fun `No warning when throw has matching catch, which is parent exception to throw`() {
lintMethod(
"""
/**
* Test method
* @param a: Int - dummy integer
* @return doubled value
*/
fun foo(a: Int): Int {
try {
if (a < 0)
throw NumberFormatException()
} catch (e: IllegalArgumentException) {
print(1)
}
return 2 * a
}
""".trimIndent())
}

@Test
@Tag(WarningNames.MISSING_KDOC_TOP_LEVEL)
fun `do not force documentation on standard methods`() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package test.paragraph2.kdoc.`package`.src.main.kotlin.com.saveourtool.diktat.kdoc.methods

/**
* @param onSuccess
* @param onFailure
* @throws ArrayIndexOutOfBounds
*/
fun parseInputNumber(onSuccess: (number: Int) -> Unit, onFailure: () -> Unit) {
try {
val input: Int = binding.inputEditText.text.toString().toInt()
if (input < 0)
throw NumberFormatException()
throw ArrayIndexOutOfBounds()
throw NullPointerException()

onSuccess(input)
} catch (e: IllegalArgumentException) {
onFailure()
} catch (e: NullPointerException) {
onFailure()
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package test.paragraph2.kdoc.`package`.src.main.kotlin.com.saveourtool.diktat.kdoc.methods

/**
* @param onSuccess
* @param onFailure
*/
fun parseInputNumber(onSuccess: (number: Int) -> Unit, onFailure: () -> Unit) {
try {
val input: Int = binding.inputEditText.text.toString().toInt()
if (input < 0)
throw NumberFormatException()
throw ArrayIndexOutOfBounds()
throw NullPointerException()

onSuccess(input)
} catch (e: IllegalArgumentException) {
onFailure()
} catch (e: NullPointerException) {
onFailure()
}
}

0 comments on commit d14f27d

Please sign in to comment.