From d14f27da43bd04a2187c57adfc7c19ce64c0a900 Mon Sep 17 00:00:00 2001 From: Daria Pleshkova <81246075+diphtongue@users.noreply.github.com> Date: Fri, 15 Dec 2023 11:55:51 +0300 Subject: [PATCH] `KDOC_WITHOUT_THROWS_TAG` add `@throws` annotation when throw inside 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 --- .../rules/chapter2/kdoc/KdocMethods.kt | 33 ++++++++ .../ruleset/chapter2/KdocMethodsFixTest.kt | 6 ++ .../ruleset/chapter2/KdocMethodsTest.kt | 75 +++++++++++++++++++ .../methods/KdocWithoutThrowsTagExpected.kt | 22 ++++++ .../methods/KdocWithoutThrowsTagTested.kt | 21 ++++++ 5 files changed, 157 insertions(+) create mode 100644 diktat-rules/src/test/resources/test/paragraph2/kdoc/package/src/main/kotlin/com/saveourtool/diktat/kdoc/methods/KdocWithoutThrowsTagExpected.kt create mode 100644 diktat-rules/src/test/resources/test/paragraph2/kdoc/package/src/main/kotlin/com/saveourtool/diktat/kdoc/methods/KdocWithoutThrowsTagTested.kt diff --git a/diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter2/kdoc/KdocMethods.kt b/diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter2/kdoc/KdocMethods.kt index 3e8d96494e..c5b69d0a64 100644 --- a/diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter2/kdoc/KdocMethods.kt +++ b/diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter2/kdoc/KdocMethods.kt @@ -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 @@ -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 @@ -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 @@ -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, @@ -197,11 +202,39 @@ class KdocMethods(configRules: List) : 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 { 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` diff --git a/diktat-rules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter2/KdocMethodsFixTest.kt b/diktat-rules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter2/KdocMethodsFixTest.kt index d5fade6004..93fa42bc35 100644 --- a/diktat-rules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter2/KdocMethodsFixTest.kt +++ b/diktat-rules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter2/KdocMethodsFixTest.kt @@ -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") + } } diff --git a/diktat-rules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter2/KdocMethodsTest.kt b/diktat-rules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter2/KdocMethodsTest.kt index 94c439e65d..0908afd7ee 100644 --- a/diktat-rules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter2/KdocMethodsTest.kt +++ b/diktat-rules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter2/KdocMethodsTest.kt @@ -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`() { diff --git a/diktat-rules/src/test/resources/test/paragraph2/kdoc/package/src/main/kotlin/com/saveourtool/diktat/kdoc/methods/KdocWithoutThrowsTagExpected.kt b/diktat-rules/src/test/resources/test/paragraph2/kdoc/package/src/main/kotlin/com/saveourtool/diktat/kdoc/methods/KdocWithoutThrowsTagExpected.kt new file mode 100644 index 0000000000..3af3dc8699 --- /dev/null +++ b/diktat-rules/src/test/resources/test/paragraph2/kdoc/package/src/main/kotlin/com/saveourtool/diktat/kdoc/methods/KdocWithoutThrowsTagExpected.kt @@ -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() + } +} diff --git a/diktat-rules/src/test/resources/test/paragraph2/kdoc/package/src/main/kotlin/com/saveourtool/diktat/kdoc/methods/KdocWithoutThrowsTagTested.kt b/diktat-rules/src/test/resources/test/paragraph2/kdoc/package/src/main/kotlin/com/saveourtool/diktat/kdoc/methods/KdocWithoutThrowsTagTested.kt new file mode 100644 index 0000000000..5cd21a66e8 --- /dev/null +++ b/diktat-rules/src/test/resources/test/paragraph2/kdoc/package/src/main/kotlin/com/saveourtool/diktat/kdoc/methods/KdocWithoutThrowsTagTested.kt @@ -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() + } +}