From cb6b9a9a0caeada9fd7ca333bc07931f0a9bf69a Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Fri, 20 Sep 2024 08:24:56 +0200 Subject: [PATCH] IJPL-162560: IncorrectProcessCanceledExceptionHandlingInspection: Add support for CancellationException in coroutine context GitOrigin-RevId: aeaa49f2a8e27af62a3ec826b3cece3a4e0c447b --- ...rrectProcessCanceledExceptionHandling.html | 48 +++- .../resources/intellij.devkit.core.xml | 7 +- .../messages/DevKitBundle.properties | 18 +- ...cessCanceledExceptionHandlingInspection.kt | 216 +++++++++++------ .../IncorrectPceHandlingTests.java | 36 +-- ...HandlingWhenMultipleCatchClausesTests.java | 36 +-- ...eHandlingWhenPceCaughtImplicitlyTests.java | 72 +++--- ...rrectCeHandlingInSuspendingLambdasTests.kt | 91 +++++++ .../IncorrectCeHandlingTests.kt | 73 ++++++ ...CeHandlingWhenMultipleCatchClausesTests.kt | 179 ++++++++++++++ ...tCeHandlingWhenPceCaughtImplicitlyTests.kt | 227 ++++++++++++++++++ .../IncorrectPceHandlingTests.kt | 24 +- ...ceHandlingWhenMultipleCatchClausesTests.kt | 36 +-- ...PceHandlingWhenPceCaughtImplicitlyTests.kt | 56 ++--- ...CanceledExceptionHandlingInspectionTest.kt | 88 ++++++- .../resources/intellij.kotlin.devkit.xml | 3 + .../KtCancellationExceptionHandlingChecker.kt | 77 ++++++ 17 files changed, 1054 insertions(+), 233 deletions(-) create mode 100644 plugins/devkit/devkit-kotlin-tests/testData/inspections/incorrectPceHandling/IncorrectCeHandlingInSuspendingLambdasTests.kt create mode 100644 plugins/devkit/devkit-kotlin-tests/testData/inspections/incorrectPceHandling/IncorrectCeHandlingTests.kt create mode 100644 plugins/devkit/devkit-kotlin-tests/testData/inspections/incorrectPceHandling/IncorrectCeHandlingWhenMultipleCatchClausesTests.kt create mode 100644 plugins/devkit/devkit-kotlin-tests/testData/inspections/incorrectPceHandling/IncorrectCeHandlingWhenPceCaughtImplicitlyTests.kt create mode 100644 plugins/devkit/intellij.kotlin.devkit/src/inspections/KtCancellationExceptionHandlingChecker.kt diff --git a/plugins/devkit/devkit-core/resources/inspectionDescriptions/IncorrectProcessCanceledExceptionHandling.html b/plugins/devkit/devkit-core/resources/inspectionDescriptions/IncorrectProcessCanceledExceptionHandling.html index 15f46b88b1af..5becbb9fdc7d 100644 --- a/plugins/devkit/devkit-core/resources/inspectionDescriptions/IncorrectProcessCanceledExceptionHandling.html +++ b/plugins/devkit/devkit-core/resources/inspectionDescriptions/IncorrectProcessCanceledExceptionHandling.html @@ -1,18 +1,24 @@ -Reports ProcessCanceledExceptions handled in an incorrect way. +Reports ProcessCanceledException and CancellationException handled incorrectly.

ProcessCanceledException and its inheritors must not be caught, swallowed, logged, or handled in any way. - Instead, it must be rethrown so that the infrastructure can handle it correctly. + Instead, it must be rethrown so that the IntelliJ Platform infrastructure can handle it correctly. +

+

+ CancellationException must not be caught, swallowed, logged, or handled in any way in coroutine context. + Instead, it must be rethrown so that the coroutines infrastructure can handle it correctly.

- Inspection reports both explicit ProcessCanceledException or its inheritors catching, - as well as catching RuntimeException, Exception and Throwable covering ProcessCanceledException. + Inspection reports both explicit ProcessCanceledException (including inheritors) and CancellationExceptioncatching, + as well as catching RuntimeException, Exception and Throwable covering cancellation exceptions.

-

Example:

+

Examples:

+ +

ProcessCanceledException:


 // bad:
@@ -39,6 +45,38 @@ try {
 }
+

CancellationException:

+

+// bad:
+cs.launch {
+  try {
+    // ...
+  } catch (e: CancellationException) { // exception should not be swallowed
+  }
+}
+
+// bad:
+suspend fun suspendingFunction() {
+  try {
+    // ...
+  } catch (ProcessCanceledException e) {
+    LOG.error("Error occurred", e); // exception should not be logged
+    throw e;
+  }
+}
+
+
+// good:
+cs.launch {
+  try {
+    // ...
+  } catch (ProcessCanceledException e) {
+    // additional actions
+    throw e;
+  }
+}
+
+

New in 2023.2

diff --git a/plugins/devkit/devkit-core/resources/intellij.devkit.core.xml b/plugins/devkit/devkit-core/resources/intellij.devkit.core.xml index b218c8dd2041..e9e0633c2593 100644 --- a/plugins/devkit/devkit-core/resources/intellij.devkit.core.xml +++ b/plugins/devkit/devkit-core/resources/intellij.devkit.core.xml @@ -444,7 +444,7 @@ groupPathKey="inspections.group.path" groupKey="inspections.group.code" enabledByDefault="false" level="ERROR" implementationClass="org.jetbrains.idea.devkit.inspections.IncorrectProcessCanceledExceptionHandlingInspection" - key="inspections.incorrect.process.canceled.exception.handling.display.name"/> + key="inspections.incorrect.cancellation.exception.handling.display.name"/> + + + diff --git a/plugins/devkit/devkit-core/resources/messages/DevKitBundle.properties b/plugins/devkit/devkit-core/resources/messages/DevKitBundle.properties index a1896c1fce77..0c99285a6af9 100644 --- a/plugins/devkit/devkit-core/resources/messages/DevKitBundle.properties +++ b/plugins/devkit/devkit-core/resources/messages/DevKitBundle.properties @@ -658,17 +658,17 @@ inspections.calling.method.should.be.rbc.annotated=Calling method should be anno inspections.calling.method.should.be.rbc.annotated.message=Calling method should be annotated with '@RequiresBlockingContext' inspections.calling.method.should.be.rbc.annotated.annotate.fix=Annotate calling method with '@RequiresBlockingContext' -inspections.incorrect.process.canceled.exception.handling.display.name='ProcessCanceledException' handled incorrectly -inspections.incorrect.process.canceled.exception.handling.name.not.rethrown='ProcessCanceledException' must be rethrown -inspections.incorrect.process.canceled.exception.handling.name.logged='ProcessCanceledException' must not be logged -inspections.incorrect.process.canceled.exception.inheritor.handling.name.not.rethrown='ProcessCanceledException' inheritor must be rethrown -inspections.incorrect.process.canceled.exception.inheritor.handling.name.logged='ProcessCanceledException' inheritor must not be logged +inspections.incorrect.cancellation.exception.handling.display.name=Cancellation exception handled incorrectly +inspections.incorrect.cancellation.exception.handling.name.not.rethrown=''{0}'' must be rethrown +inspections.incorrect.cancellation.exception.handling.name.logged=''{0}'' must not be logged +inspections.incorrect.cancellation.exception.inheritor.handling.name.not.rethrown=''{0}'' inheritor must be rethrown +inspections.incorrect.cancellation.exception.inheritor.handling.name.logged=''{0}'' inheritor must not be logged -inspections.incorrect.implicit.process.canceled.exception.handling.name.not.rethrown=''ProcessCanceledException'' must be rethrown. ''ProcessCanceledException'' is thrown by ''{0}()''. -inspections.incorrect.implicit.process.canceled.exception.handling.name.logged=''ProcessCanceledException'' must not be logged. ''ProcessCanceledException'' is thrown by ''{0}()''. +inspections.incorrect.implicit.cancellation.exception.handling.name.not.rethrown=''{0}'' must be rethrown. It is thrown by ''{1}()''. +inspections.incorrect.implicit.cancellation.exception.handling.name.logged=''{0}'' must not be logged. It is thrown by ''{1}()''. -inspections.incorrect.implicit.process.canceled.exception.inheritor.handling.name.not.rethrown=''ProcessCanceledException'' inheritor must be rethrown. ''ProcessCanceledException'' is thrown by ''{0}()''. -inspections.incorrect.implicit.process.canceled.exception.inheritor.handling.name.logged=''ProcessCanceledException'' inheritor must not be logged. ''ProcessCanceledException'' is thrown by ''{0}()''. +inspections.incorrect.implicit.cancellation.exception.inheritor.handling.name.not.rethrown=''{0}'' inheritor must be rethrown. It is thrown by ''{1}()''. +inspections.incorrect.implicit.cancellation.exception.inheritor.handling.name.logged=''{0}'' inheritor must not be logged. It is thrown by ''{1}()''. update.ide.from.sources=Update &IDE from sources update.ide.from.sources.option=from sources inspections.meta.information.unknown.inspection.id=Unknown inspection id ''{0}'' diff --git a/plugins/devkit/devkit-core/src/inspections/IncorrectProcessCanceledExceptionHandlingInspection.kt b/plugins/devkit/devkit-core/src/inspections/IncorrectProcessCanceledExceptionHandlingInspection.kt index c84198cbde1f..87b6e733e4af 100644 --- a/plugins/devkit/devkit-core/src/inspections/IncorrectProcessCanceledExceptionHandlingInspection.kt +++ b/plugins/devkit/devkit-core/src/inspections/IncorrectProcessCanceledExceptionHandlingInspection.kt @@ -3,14 +3,19 @@ package org.jetbrains.idea.devkit.inspections import com.intellij.codeInspection.ProblemsHolder import com.intellij.codeInspection.registerUProblem +import com.intellij.lang.Language +import com.intellij.lang.LanguageExtension import com.intellij.openapi.diagnostic.Logger +import com.intellij.openapi.extensions.ExtensionPointName import com.intellij.openapi.progress.ProcessCanceledException +import com.intellij.openapi.util.IntellijInternalApi import com.intellij.psi.* import com.intellij.psi.search.GlobalSearchScope import com.intellij.psi.util.InheritanceUtil import com.intellij.psi.util.PsiTypesUtil import com.intellij.uast.UastHintedVisitorAdapter.Companion.create -import org.jetbrains.idea.devkit.DevKitBundle +import org.jetbrains.annotations.ApiStatus +import org.jetbrains.idea.devkit.DevKitBundle.message import org.jetbrains.uast.* import org.jetbrains.uast.visitor.AbstractUastNonRecursiveVisitor import org.jetbrains.uast.visitor.AbstractUastVisitor @@ -38,13 +43,22 @@ internal class IncorrectProcessCanceledExceptionHandlingInspection : DevKitUastI private fun findSuspiciousCeCaughtParam(catchParameters: List): CaughtCeInfo? { val ceClasses = findCeClasses(holder.file.resolveScope) ?: return null for (catchParameter in catchParameters) { + // language-specific check: + val checker = cancellationExceptionHandlingChecker(catchParameter.lang) + val sourcePsi = catchParameter.sourcePsi + if (sourcePsi != null && checker?.isSuspicious(sourcePsi) == true) { + return CaughtCeInfo(checker.getCeName(), false, catchParameter) + } + // general UAST check: val type = catchParameter.type - val caughtExceptionTypes = if (type is PsiDisjunctionType) type.disjunctions else listOf(type) + val caughtExceptionTypes = (if (type is PsiDisjunctionType) type.disjunctions else listOf(type)).filterIsInstance() for (caughtExceptionType in caughtExceptionTypes) { - if (ceClasses.any { caughtExceptionType.isInheritorOrSelf(it) }) { + val baseCeClass = ceClasses.firstOrNull { caughtExceptionType.isInheritorOrSelf(it) } + if (baseCeClass != null) { return CaughtCeInfo( - catchParameter, - caughtExceptionType.isCancellationExceptionClass() + baseCeClass.qualifiedName!!, + !caughtExceptionType.isCancellationExceptionClass(), + catchParameter ) } } @@ -59,7 +73,7 @@ internal class IncorrectProcessCanceledExceptionHandlingInspection : DevKitUastI } private fun PsiType.isCancellationExceptionClass(): Boolean { - return ceClassNames.any { it != (this as? PsiClassType)?.resolve()?.qualifiedName } + return ceClassNames.any { it == (this as? PsiClassType)?.resolve()?.qualifiedName } } private fun PsiType.isInheritorOrSelf(ceClass: PsiClass): Boolean { @@ -72,15 +86,15 @@ internal class IncorrectProcessCanceledExceptionHandlingInspection : DevKitUastI val catchBody = node.body if (!ceIsRethrown(catchBody, caughtCeInfo.parameter)) { val message = if (caughtCeInfo.isInheritor) - DevKitBundle.message("inspections.incorrect.process.canceled.exception.inheritor.handling.name.not.rethrown") - else DevKitBundle.message("inspections.incorrect.process.canceled.exception.handling.name.not.rethrown") + message("inspections.incorrect.cancellation.exception.inheritor.handling.name.not.rethrown", caughtCeInfo.baseCeClassName) + else message("inspections.incorrect.cancellation.exception.handling.name.not.rethrown", caughtCeInfo.baseCeClassName) holder.registerUProblem(caughtCeInfo.parameter, message) } val loggingExpression = findCeLoggingExpression(catchBody, caughtCeInfo.parameter) if (loggingExpression != null) { val message = if (caughtCeInfo.isInheritor) - DevKitBundle.message("inspections.incorrect.process.canceled.exception.inheritor.handling.name.logged") - else DevKitBundle.message("inspections.incorrect.process.canceled.exception.handling.name.logged") + message("inspections.incorrect.cancellation.exception.inheritor.handling.name.logged", caughtCeInfo.baseCeClassName) + else message("inspections.incorrect.cancellation.exception.handling.name.logged", caughtCeInfo.baseCeClassName) holder.registerUProblem(loggingExpression, message) } } @@ -90,7 +104,7 @@ internal class IncorrectProcessCanceledExceptionHandlingInspection : DevKitUastI catchParameters: List, ): Boolean { val tryExpression = catchClause.getParentOfType() ?: return super.visitCatchClause(catchClause) - if (tryExpression.containsCatchClauseForType()) { + if (tryExpression.containsCatchClauseForType() || tryExpression.checkContainsSuspiciousCeCatchClause()) { // Cancellation exception will be caught by the explicit catch clause return super.visitCatchClause(catchClause) } @@ -102,35 +116,99 @@ internal class IncorrectProcessCanceledExceptionHandlingInspection : DevKitUastI // Cancellation exception will be caught by catch clause with a more specific type, so do not report return super.visitCatchClause(catchClause) } - val (ceThrowingExpression, isCeInheritorThrown) = findCeThrowingExpression(tryExpression) - if (ceThrowingExpression != null) { - inspectIncorrectImplicitCeHandling(catchClause, caughtGenericThrowableParam, isCeInheritorThrown, ceThrowingExpression) + + // lang-specific check: + val langSpecificCaughtCeInfo = findLangSpecificCeThrowingExpressionInfo(tryExpression, caughtGenericThrowableParam) + if (langSpecificCaughtCeInfo != null) { + inspectIncorrectImplicitCeHandling(catchClause, langSpecificCaughtCeInfo) + } + // UAST check: + val caughtCeInfo = findCeThrowingExpressionInfo(tryExpression, caughtGenericThrowableParam) + if (caughtCeInfo != null) { + inspectIncorrectImplicitCeHandling(catchClause, caughtCeInfo) } } return super.visitCatchClause(catchClause) } - private fun inspectIncorrectImplicitCeHandling( - node: UCatchClause, - caughtParam: UParameter, - isCeInheritor: Boolean, - ceThrowingExpression: UCallExpression, - ) { - val catchBody = node.body - if (!ceIsRethrown(catchBody, caughtParam)) { - val methodName = ceThrowingExpression.methodName ?: return - val message = if (isCeInheritor) - DevKitBundle.message("inspections.incorrect.implicit.process.canceled.exception.inheritor.handling.name.not.rethrown", - methodName) - else DevKitBundle.message("inspections.incorrect.implicit.process.canceled.exception.handling.name.not.rethrown", methodName) - holder.registerUProblem(caughtParam, message) + private fun UTryExpression.checkContainsSuspiciousCeCatchClause(): Boolean { + val sourcePsi = this.sourcePsi ?: return false + return cancellationExceptionHandlingChecker(this.lang)?.containsSuspiciousCeCatchClause(sourcePsi) == true + } + + private fun findLangSpecificCeThrowingExpressionInfo(tryExpression: UTryExpression, caughtGenericThrowableParam: UParameter): CaughtCeInfo? { + val ceHandlingChecker = cancellationExceptionHandlingChecker(tryExpression.lang) + val ceThrowingExpressionName = ceHandlingChecker?.findCeThrowingExpressionName(tryExpression.sourcePsi!!) ?: return null + return CaughtCeInfo(ceHandlingChecker.getCeName(), false, caughtGenericThrowableParam, ceThrowingExpressionName) + } + + // it searches only for explicit `throws` declarations in directly called methods. + private fun findCeThrowingExpressionInfo(tryExpression: UTryExpression, caughtGenericThrowableParam: UParameter): CaughtCeInfo? { + var caughtCeInfo: CaughtCeInfo? = null + val tryClause = tryExpression.tryClause + val resolveScope = tryClause.sourcePsi?.resolveScope ?: return null + val ceClasses = findCeClasses(resolveScope) ?: return null + tryClause.accept(object : AbstractUastVisitor() { + override fun visitCallExpression(node: UCallExpression): Boolean { + if (caughtCeInfo == null) { + val calledMethod = node.resolveToUElementOfType() ?: return super.visitCallExpression(node) + for (throwsType in calledMethod.javaPsi.throwsTypes) { + val throwsClassType = throwsType as? PsiClassType ?: continue + val baseThrowsCeClass = ceClasses.firstOrNull { throwsClassType.isInheritorOrSelf(it) } + if (baseThrowsCeClass != null) { + if (!isInNestedTryCatchBlock(node, tryExpression, throwsClassType)) { + caughtCeInfo = CaughtCeInfo( + baseThrowsCeClass.qualifiedName!!, + !throwsClassType.isCancellationExceptionClass(), + caughtGenericThrowableParam, + node.methodName + ) + return super.visitCallExpression(node) + } + } + } + } + return super.visitCallExpression(node) + } + }) + return caughtCeInfo + } + + private fun isInNestedTryCatchBlock( + expression: UCallExpression, + checkedTryExpression: UTryExpression, + thrownExceptionType: PsiClassType, + ): Boolean { + val thrownExceptionClass = thrownExceptionType.resolve() ?: return false + val parentTryExpression = expression.getParentOfType() ?: return false + if (parentTryExpression == checkedTryExpression) return false + return parentTryExpression.catchClauses.any { catchClause -> + catchClause.types.any anyType@{ type -> + return@anyType (if (type is PsiDisjunctionType) type.disjunctions else listOf(type)) + .any { it.isInheritorOrSelf(thrownExceptionClass) } + } } - val loggingExpression = findCeLoggingExpression(catchBody, caughtParam) + } + + private fun inspectIncorrectImplicitCeHandling(catchClause: UCatchClause, caughtCeInfo: CaughtCeInfo) { + val catchBody = catchClause.body + if (!ceIsRethrown(catchBody, caughtCeInfo.parameter)) { + val methodName = caughtCeInfo.ceThrowingMethodName ?: return + val message = if (caughtCeInfo.isInheritor) + message("inspections.incorrect.implicit.cancellation.exception.inheritor.handling.name.not.rethrown", + caughtCeInfo.baseCeClassName, methodName) + else message("inspections.incorrect.implicit.cancellation.exception.handling.name.not.rethrown", + caughtCeInfo.baseCeClassName, methodName) + holder.registerUProblem(caughtCeInfo.parameter, message) + } + val loggingExpression = findCeLoggingExpression(catchBody, caughtCeInfo.parameter) if (loggingExpression != null) { - val methodName = ceThrowingExpression.methodName ?: return - val message = if (isCeInheritor) - DevKitBundle.message("inspections.incorrect.implicit.process.canceled.exception.inheritor.handling.name.logged", methodName) - else DevKitBundle.message("inspections.incorrect.implicit.process.canceled.exception.handling.name.logged", methodName) + val methodName = caughtCeInfo.ceThrowingMethodName ?: return + val message = if (caughtCeInfo.isInheritor) + message("inspections.incorrect.implicit.cancellation.exception.inheritor.handling.name.logged", + caughtCeInfo.baseCeClassName, methodName) + else message("inspections.incorrect.implicit.cancellation.exception.handling.name.logged", + caughtCeInfo.baseCeClassName, methodName) holder.registerUProblem(loggingExpression, message) } } @@ -166,34 +244,6 @@ internal class IncorrectProcessCanceledExceptionHandlingInspection : DevKitUastI return loggingExpression } - // it searches only for explicit `throws` declarations in directly called methods. - private fun findCeThrowingExpression(tryExpression: UTryExpression): Pair { - val tryClause = tryExpression.tryClause - var ceThrowingExpression: UCallExpression? = null - var isCeInheritorCaught = false - val resolveScope = tryClause.sourcePsi?.resolveScope ?: return Pair(null, false) - val ceClasses = findCeClasses(resolveScope) ?: return Pair(null, false) - tryClause.accept(object : AbstractUastVisitor() { - override fun visitCallExpression(node: UCallExpression): Boolean { - if (ceThrowingExpression == null) { - val calledMethod = node.resolveToUElementOfType() ?: return super.visitCallExpression(node) - for (throwsType in calledMethod.javaPsi.throwsTypes) { - val psiClassType = throwsType as? PsiClassType ?: continue - if (ceClasses.any { psiClassType.isInheritorOrSelf(it) }) { - if (!isInNestedTryCatchBlock(node, tryExpression, psiClassType)) { - ceThrowingExpression = node - isCeInheritorCaught = psiClassType.isCancellationExceptionClass() - return super.visitCallExpression(node) - } - } - } - } - return super.visitCallExpression(node) - } - }) - return Pair(ceThrowingExpression, isCeInheritorCaught) - } - private inline fun PsiType.isClassType(): Boolean { if (this is PsiDisjunctionType) { return this.disjunctions.any { PsiTypesUtil.classNameEquals(it, T::class.java.name) } @@ -214,29 +264,35 @@ internal class IncorrectProcessCanceledExceptionHandlingInspection : DevKitUastI } } - private fun isInNestedTryCatchBlock( - expression: UCallExpression, - checkedTryExpression: UTryExpression, - thrownExceptionType: PsiClassType, - ): Boolean { - val thrownExceptionClass = thrownExceptionType.resolve() ?: return false - val parentTryExpression = expression.getParentOfType() ?: return false - if (parentTryExpression == checkedTryExpression) return false - return parentTryExpression.catchClauses.any { catchClause -> - catchClause.types.any anyType@{ type -> - if (type is PsiDisjunctionType) { - return@anyType type.disjunctions.any { it.isInheritorOrSelf(thrownExceptionClass) } - } - return@anyType type.isInheritorOrSelf(thrownExceptionClass) - } - } - } - }, arrayOf(UCatchClause::class.java)) } private class CaughtCeInfo( - val parameter: UParameter, + val baseCeClassName: String, val isInheritor: Boolean, + val parameter: UParameter, + val ceThrowingMethodName: String? = null, ) } + +// allow additional languages to contribute language-specific checks +// (used by Kotlin at least for suspending context cancellation handling): + +private val EP_NAME: ExtensionPointName = + ExtensionPointName.Companion.create("DevKit.lang.cancellationExceptionHandlingChecker") + +private object CancellationExceptionHandlingCheckers : + LanguageExtension(EP_NAME.name) + +private fun cancellationExceptionHandlingChecker(language: Language): CancellationExceptionHandlingChecker? { + return CancellationExceptionHandlingCheckers.forLanguage(language) +} + +@IntellijInternalApi +@ApiStatus.Internal +interface CancellationExceptionHandlingChecker { + fun isSuspicious(catchParameter: PsiElement): Boolean + fun getCeName(): String + fun containsSuspiciousCeCatchClause(tryExpression: PsiElement): Boolean + fun findCeThrowingExpressionName(tryExpression: PsiElement): String? +} diff --git a/plugins/devkit/devkit-java-tests/testData/inspections/incorrectPceHandling/IncorrectPceHandlingTests.java b/plugins/devkit/devkit-java-tests/testData/inspections/incorrectPceHandling/IncorrectPceHandlingTests.java index c97210c2a027..da677e3e8fcc 100644 --- a/plugins/devkit/devkit-java-tests/testData/inspections/incorrectPceHandling/IncorrectPceHandlingTests.java +++ b/plugins/devkit/devkit-java-tests/testData/inspections/incorrectPceHandling/IncorrectPceHandlingTests.java @@ -8,7 +8,7 @@ class IncorrectPceHandlingTests { void testPceSwallowed() { try { // anything - } catch (ProcessCanceledException e) { + } catch (ProcessCanceledException e) { // exception swallowed } } @@ -17,7 +17,7 @@ class IncorrectPceHandlingTests { try { // anything } catch (ProcessCanceledException e) { - LOG.error(e); + LOG.error(e); throw e; } } @@ -25,64 +25,64 @@ class IncorrectPceHandlingTests { void testSwallowedAndLoggedWithMessageOnInfoLevel() { try { // anything - } catch (ProcessCanceledException e) { - LOG.info("Error occured", e); + } catch (ProcessCanceledException e) { + LOG.info("Error occured", e); } } void testSwallowedAndLoggedOnInfoLevel() { try { // anything - } catch (ProcessCanceledException e) { - LOG.info(e); + } catch (ProcessCanceledException e) { + LOG.info(e); } } void testSwallowedAndLoggedOnErrorLevel() { try { // anything - } catch (ProcessCanceledException e) { - LOG.error(e); + } catch (ProcessCanceledException e) { + LOG.error(e); } } void testSwallowedAndOnlyExceptionMessageLogged() { try { // anything - } catch (ProcessCanceledException e) { - LOG.error("Error occurred: " + e.getMessage()); + } catch (ProcessCanceledException e) { + LOG.error("Error occurred: " + e.getMessage()); } } void testDisjunctionTypesWhenPceIsFirst() { try { // anything - } catch (ProcessCanceledException | IllegalStateException e) { - LOG.error("Error occurred: " + e.getMessage()); + } catch (ProcessCanceledException | IllegalStateException e) { + LOG.error("Error occurred: " + e.getMessage()); } } void testDisjunctionTypesWhenPceIsSecond() { try { // anything - } catch (IllegalStateException | ProcessCanceledException e) { - LOG.error("Error occurred: " + e.getMessage()); + } catch (IllegalStateException | ProcessCanceledException e) { + LOG.error("Error occurred: " + e.getMessage()); } } void testPceInheritorSwallowedAndLogger() { try { // anything - } catch (SubclassOfProcessCanceledException e) { - LOG.error(e); + } catch (SubclassOfProcessCanceledException e) { + LOG.error(e); } } void testPceInheritorSwallowedAndLoggerWhenDisjunctionTypeDefined() { try { // anything - } catch (IllegalStateException | SubclassOfProcessCanceledException e) { - LOG.error(e); + } catch (IllegalStateException | SubclassOfProcessCanceledException e) { + LOG.error(e); } } diff --git a/plugins/devkit/devkit-java-tests/testData/inspections/incorrectPceHandling/IncorrectPceHandlingWhenMultipleCatchClausesTests.java b/plugins/devkit/devkit-java-tests/testData/inspections/incorrectPceHandling/IncorrectPceHandlingWhenMultipleCatchClausesTests.java index 024e7fd6adef..ae4c0daef29e 100644 --- a/plugins/devkit/devkit-java-tests/testData/inspections/incorrectPceHandling/IncorrectPceHandlingWhenMultipleCatchClausesTests.java +++ b/plugins/devkit/devkit-java-tests/testData/inspections/incorrectPceHandling/IncorrectPceHandlingWhenMultipleCatchClausesTests.java @@ -8,7 +8,7 @@ class IncorrectPceHandlingWhenMultipleCatchClausesTests { void testPceSwallowed() { try { // anything - } catch (ProcessCanceledException e) { + } catch (ProcessCanceledException e) { // exception swallowed } catch (Exception e) { // exception swallowed @@ -19,7 +19,7 @@ class IncorrectPceHandlingWhenMultipleCatchClausesTests { try { // anything } catch (ProcessCanceledException e) { - LOG.error(e); + LOG.error(e); throw e; } catch (Exception e) { // exception swallowed @@ -29,8 +29,8 @@ class IncorrectPceHandlingWhenMultipleCatchClausesTests { void testSwallowedAndLoggedWithMessageOnInfoLevel() { try { // anything - } catch (ProcessCanceledException e) { - LOG.info("Error occured", e); + } catch (ProcessCanceledException e) { + LOG.info("Error occured", e); } catch (Exception e) { LOG.info("Error occured", e); } @@ -39,8 +39,8 @@ class IncorrectPceHandlingWhenMultipleCatchClausesTests { void testSwallowedAndLoggedOnInfoLevel() { try { // anything - } catch (ProcessCanceledException e) { - LOG.info(e); + } catch (ProcessCanceledException e) { + LOG.info(e); } catch (Exception e) { LOG.info(e); } @@ -49,8 +49,8 @@ class IncorrectPceHandlingWhenMultipleCatchClausesTests { void testSwallowedAndLoggedOnErrorLevel() { try { // anything - } catch (ProcessCanceledException e) { - LOG.error(e); + } catch (ProcessCanceledException e) { + LOG.error(e); } catch (Exception e) { LOG.error(e); } @@ -59,8 +59,8 @@ class IncorrectPceHandlingWhenMultipleCatchClausesTests { void testSwallowedAndOnlyExceptionMessageLogged() { try { // anything - } catch (ProcessCanceledException e) { - LOG.error("Error occurred: " + e.getMessage()); + } catch (ProcessCanceledException e) { + LOG.error("Error occurred: " + e.getMessage()); } catch (Exception e) { LOG.error("Error occurred: " + e.getMessage()); } @@ -69,7 +69,7 @@ class IncorrectPceHandlingWhenMultipleCatchClausesTests { void testPceSwallowedAndMultipleGenericCatchClauses() { try { // anything - } catch (ProcessCanceledException e) { + } catch (ProcessCanceledException e) { // exception swallowed } catch (RuntimeException e) { // exception swallowed @@ -84,7 +84,7 @@ class IncorrectPceHandlingWhenMultipleCatchClausesTests { try { // anything } catch (ProcessCanceledException e) { - LOG.error(e); + LOG.error(e); throw e; } catch (RuntimeException e) { LOG.error(e); @@ -98,9 +98,9 @@ class IncorrectPceHandlingWhenMultipleCatchClausesTests { void testPceInheritorSwallowedAndMultipleGenericCatchClauses() { try { // anything - } catch (SubclassOfProcessCanceledException e) { + } catch (SubclassOfProcessCanceledException e) { // exception swallowed - } catch (ProcessCanceledException e) { + } catch (ProcessCanceledException e) { // exception swallowed } catch (RuntimeException e) { // exception swallowed @@ -115,10 +115,10 @@ class IncorrectPceHandlingWhenMultipleCatchClausesTests { try { // anything } catch (SubclassOfProcessCanceledException e) { - LOG.error(e); + LOG.error(e); throw e; } catch (ProcessCanceledException e) { - LOG.error(e); + LOG.error(e); throw e; } catch (RuntimeException e) { LOG.error(e); @@ -135,8 +135,8 @@ class IncorrectPceHandlingWhenMultipleCatchClausesTests { try { // anything } - catch (ProcessCanceledException e) { - LOG.error(e); + catch (ProcessCanceledException e) { + LOG.error(e); } } catch (Throwable e) { diff --git a/plugins/devkit/devkit-java-tests/testData/inspections/incorrectPceHandling/IncorrectPceHandlingWhenPceCaughtImplicitlyTests.java b/plugins/devkit/devkit-java-tests/testData/inspections/incorrectPceHandling/IncorrectPceHandlingWhenPceCaughtImplicitlyTests.java index e13a6b4c7d2f..49ea7f2d275b 100644 --- a/plugins/devkit/devkit-java-tests/testData/inspections/incorrectPceHandling/IncorrectPceHandlingWhenPceCaughtImplicitlyTests.java +++ b/plugins/devkit/devkit-java-tests/testData/inspections/incorrectPceHandling/IncorrectPceHandlingWhenPceCaughtImplicitlyTests.java @@ -14,7 +14,7 @@ class IncorrectPceHandlingWhenPceCaughtImplicitlyTests { void testPceSwallowed() { try { throwPce(); - } catch (Exception e) { + } catch (Exception e) { // exception swallowed } } @@ -23,7 +23,7 @@ class IncorrectPceHandlingWhenPceCaughtImplicitlyTests { try { throwPce(); } catch (Exception e) { - LOG.error(e); + LOG.error(e); throw e; } } @@ -31,55 +31,55 @@ class IncorrectPceHandlingWhenPceCaughtImplicitlyTests { void testSwallowedAndLoggedWithMessageOnInfoLevel() { try { throwPce(); - } catch (Exception e) { - LOG.info("Error occured", e); + } catch (Exception e) { + LOG.info("Error occured", e); } } void testSwallowedAndLoggedOnInfoLevel() { try { throwPce(); - } catch (Exception e) { - LOG.info(e); + } catch (Exception e) { + LOG.info(e); } } void testSwallowedAndLoggedOnErrorLevel() { try { throwPce(); - } catch (Exception e) { - LOG.error(e); + } catch (Exception e) { + LOG.error(e); } } void testSwallowedAndOnlyExceptionMessageLogged() { try { throwPce(); - } catch (Exception e) { - LOG.error("Error occurred: " + e.getMessage()); + } catch (Exception e) { + LOG.error("Error occurred: " + e.getMessage()); } } void testDisjunctionTypesWhenPceIsFirst() { try { throwPce(); - } catch (RuntimeException | Error e) { - LOG.error("Error occurred: " + e.getMessage()); + } catch (RuntimeException | Error e) { + LOG.error("Error occurred: " + e.getMessage()); } } void testDisjunctionTypesWhenPceIsSecond() { try { throwPce(); - } catch (Error | RuntimeException e) { - LOG.error("Error occurred: " + e.getMessage()); + } catch (Error | RuntimeException e) { + LOG.error("Error occurred: " + e.getMessage()); } } void testPceSwallowedAndMultipleGenericCatchClauses() { try { throwPce(); - } catch (RuntimeException e) { + } catch (RuntimeException e) { // exception swallowed } catch (Exception e) { // exception swallowed @@ -92,7 +92,7 @@ class IncorrectPceHandlingWhenPceCaughtImplicitlyTests { try { throwPce(); } catch (RuntimeException e) { - LOG.error(e); + LOG.error(e); throw e; } catch (Exception e) { LOG.error(e); @@ -109,8 +109,8 @@ class IncorrectPceHandlingWhenPceCaughtImplicitlyTests { try { throwPce(); } - catch (ProcessCanceledException e) { - LOG.error(e); + catch (ProcessCanceledException e) { + LOG.error(e); } } catch (Throwable e) { @@ -129,7 +129,7 @@ class IncorrectPceHandlingWhenPceCaughtImplicitlyTests { void testPceInheritorSwallowed() { try { throwPceInheritor(); - } catch (Exception e) { + } catch (Exception e) { // exception swallowed } } @@ -138,7 +138,7 @@ class IncorrectPceHandlingWhenPceCaughtImplicitlyTests { try { throwPceInheritor(); } catch (Exception e) { - LOG.error(e); + LOG.error(e); throw e; } } @@ -146,55 +146,55 @@ class IncorrectPceHandlingWhenPceCaughtImplicitlyTests { void testPceInheritorSwallowedAndLoggedWithMessageOnInfoLevel() { try { throwPceInheritor(); - } catch (Exception e) { - LOG.info("Error occured", e); + } catch (Exception e) { + LOG.info("Error occured", e); } } void testPceInheritorSwallowedAndLoggedOnInfoLevel() { try { throwPceInheritor(); - } catch (Exception e) { - LOG.info(e); + } catch (Exception e) { + LOG.info(e); } } void testPceInheritorSwallowedAndLoggedOnErrorLevel() { try { throwPceInheritor(); - } catch (Exception e) { - LOG.error(e); + } catch (Exception e) { + LOG.error(e); } } void testPceInheritorSwallowedAndOnlyExceptionMessageLogged() { try { throwPceInheritor(); - } catch (Exception e) { - LOG.error("Error occurred: " + e.getMessage()); + } catch (Exception e) { + LOG.error("Error occurred: " + e.getMessage()); } } void testDisjunctionTypesWhenPceInheritorIsFirst() { try { throwPceInheritor(); - } catch (RuntimeException | Error e) { - LOG.error("Error occurred: " + e.getMessage()); + } catch (RuntimeException | Error e) { + LOG.error("Error occurred: " + e.getMessage()); } } void testDisjunctionTypesWhenPceInheritorIsSecond() { try { throwPceInheritor(); - } catch (Error | RuntimeException e) { - LOG.error("Error occurred: " + e.getMessage()); + } catch (Error | RuntimeException e) { + LOG.error("Error occurred: " + e.getMessage()); } } void testPceInheritorSwallowedAndMultipleGenericCatchClauses() { try { throwPceInheritor(); - } catch (RuntimeException e) { + } catch (RuntimeException e) { // exception swallowed } catch (Exception e) { // exception swallowed @@ -207,7 +207,7 @@ class IncorrectPceHandlingWhenPceCaughtImplicitlyTests { try { throwPceInheritor(); } catch (RuntimeException e) { - LOG.error(e); + LOG.error(e); throw e; } catch (Exception e) { LOG.error(e); @@ -224,8 +224,8 @@ class IncorrectPceHandlingWhenPceCaughtImplicitlyTests { try { throwPceInheritor(); } - catch (SubclassOfProcessCanceledException e) { - LOG.error(e); + catch (SubclassOfProcessCanceledException e) { + LOG.error(e); } } catch (Throwable e) { diff --git a/plugins/devkit/devkit-kotlin-tests/testData/inspections/incorrectPceHandling/IncorrectCeHandlingInSuspendingLambdasTests.kt b/plugins/devkit/devkit-kotlin-tests/testData/inspections/incorrectPceHandling/IncorrectCeHandlingInSuspendingLambdasTests.kt new file mode 100644 index 000000000000..d97f29277dfd --- /dev/null +++ b/plugins/devkit/devkit-kotlin-tests/testData/inspections/incorrectPceHandling/IncorrectCeHandlingInSuspendingLambdasTests.kt @@ -0,0 +1,91 @@ +import com.example.SubclassOfCancellationException +import com.intellij.openapi.diagnostic.Logger +import kotlinx.coroutines.CancellationException + +private val LOG = Logger.getInstance("any") + +fun launch(block: suspend () -> Unit) { + // ... +} + +class IncorrectCeHandlingInSuspendingLambdasTests { + fun testCeSwallowed() { + launch { + try { + // anything + } + catch (e: CancellationException) { + // exception swallowed + } + } + } + + fun testCeLogged() { + launch { + try { + // anything + } + catch (e: CancellationException) { + LOG.error(e) + throw e + } + } + } + + fun testSwallowedAndLoggedWithMessageOnInfoLevel() { + launch { + try { + // anything + } + catch (e: CancellationException) { + LOG.info("Error occured", e) + } + } + } + + fun testSwallowedAndLoggedOnInfoLevel() { + launch { + try { + // anything + } + catch (e: CancellationException) { + LOG.info(e) + } + } + } + + fun testSwallowedAndLoggedOnErrorLevel() { + launch { + try { + // anything + } + catch (e: CancellationException) { + LOG.error(e) + } + } + } + + fun testSwallowedAndOnlyExceptionMessageLogged() { + launch { + try { + // anything + } + catch (e: CancellationException) { + LOG.error("Error occurred: " + e.message) + } + } + } + + // should not report subclasses + fun testCeInheritorSwallowedAndLogger() { + launch { + try { + // anything + } + catch (e: SubclassOfCancellationException) { + LOG.error(e) + } + } + } + +} \ No newline at end of file diff --git a/plugins/devkit/devkit-kotlin-tests/testData/inspections/incorrectPceHandling/IncorrectCeHandlingTests.kt b/plugins/devkit/devkit-kotlin-tests/testData/inspections/incorrectPceHandling/IncorrectCeHandlingTests.kt new file mode 100644 index 000000000000..b630872987de --- /dev/null +++ b/plugins/devkit/devkit-kotlin-tests/testData/inspections/incorrectPceHandling/IncorrectCeHandlingTests.kt @@ -0,0 +1,73 @@ +import com.example.SubclassOfCancellationException +import com.intellij.openapi.diagnostic.Logger +import kotlinx.coroutines.CancellationException + +private val LOG = Logger.getInstance("any") + +class IncorrectCeHandlingTests { + suspend fun testCeSwallowed() { + try { + // anything + } + catch (e: CancellationException) { + // exception swallowed + } + } + + suspend fun testCeLogged() { + try { + // anything + } + catch (e: CancellationException) { + LOG.error(e) + throw e + } + } + + suspend fun testSwallowedAndLoggedWithMessageOnInfoLevel() { + try { + // anything + } + catch (e: CancellationException) { + LOG.info("Error occured", e) + } + } + + suspend fun testSwallowedAndLoggedOnInfoLevel() { + try { + // anything + } + catch (e: CancellationException) { + LOG.info(e) + } + } + + suspend fun testSwallowedAndLoggedOnErrorLevel() { + try { + // anything + } + catch (e: CancellationException) { + LOG.error(e) + } + } + + suspend fun testSwallowedAndOnlyExceptionMessageLogged() { + try { + // anything + } + catch (e: CancellationException) { + LOG.error("Error occurred: " + e.message) + } + } + + // should not report subclasses + suspend fun testCeInheritorSwallowedAndLogger() { + try { + // anything + } + catch (e: SubclassOfCancellationException) { + LOG.error(e) + } + } + +} \ No newline at end of file diff --git a/plugins/devkit/devkit-kotlin-tests/testData/inspections/incorrectPceHandling/IncorrectCeHandlingWhenMultipleCatchClausesTests.kt b/plugins/devkit/devkit-kotlin-tests/testData/inspections/incorrectPceHandling/IncorrectCeHandlingWhenMultipleCatchClausesTests.kt new file mode 100644 index 000000000000..96a17c573a87 --- /dev/null +++ b/plugins/devkit/devkit-kotlin-tests/testData/inspections/incorrectPceHandling/IncorrectCeHandlingWhenMultipleCatchClausesTests.kt @@ -0,0 +1,179 @@ +import com.example.SubclassOfCancellationException +import com.intellij.openapi.diagnostic.Logger +import kotlinx.coroutines.CancellationException + +private val LOG = Logger.getInstance("any") + +class IncorrectCeHandlingWhenMultipleCatchClausesTests { + suspend fun testCeSwallowed() { + try { + // anything + } + catch (e: CancellationException) { + // exception swallowed + } + catch (e: Exception) { + // exception swallowed + } + } + + suspend fun testCeLogged() { + try { + // anything + } + catch (e: CancellationException) { + LOG.error(e) + throw e + } + catch (e: Exception) { + // exception swallowed + } + } + + suspend fun testSwallowedAndLoggedWithMessageOnInfoLevel() { + try { + // anything + } + catch (e: CancellationException) { + LOG.info("Error occured", e) + } + catch (e: Exception) { + LOG.info("Error occured", e) + } + } + + suspend fun testSwallowedAndLoggedOnInfoLevel() { + try { + // anything + } + catch (e: CancellationException) { + LOG.info(e) + } + catch (e: Exception) { + LOG.info(e) + } + } + + suspend fun testSwallowedAndLoggedOnErrorLevel() { + try { + // anything + } + catch (e: CancellationException) { + LOG.error(e) + } + catch (e: Exception) { + LOG.error(e) + } + } + + suspend fun testSwallowedAndOnlyExceptionMessageLogged() { + try { + // anything + } + catch (e: CancellationException) { + LOG.error("Error occurred: " + e.message) + } + catch (e: Exception) { + LOG.error("Error occurred: " + e.message) + } + } + + suspend fun testCeSwallowedAndMultipleGenericCatchClauses() { + try { + // anything + } + catch (e: CancellationException) { + // exception swallowed + } + catch (e: RuntimeException) { + // exception swallowed + } + catch (e: Exception) { + // exception swallowed + } + catch (e: Throwable) { + // exception swallowed + } + } + + suspend fun testCeLoggedAndMultipleGenericCatchClauses() { + try { + // anything + } + catch (e: CancellationException) { + LOG.error(e) + throw e + } + catch (e: RuntimeException) { + LOG.error(e) + } + catch (e: Exception) { + LOG.error(e) + } + catch (e: Throwable) { + LOG.error(e) + } + } + + suspend fun testCeInheritorSwallowedAndMultipleGenericCatchClauses() { + try { + // anything + } + // should not report subclasses of CancellationException: + catch (e: SubclassOfCancellationException) { + // exception swallowed + } + catch (e: CancellationException) { + // exception swallowed + } + catch (e: RuntimeException) { + // exception swallowed + } + catch (e: Exception) { + // exception swallowed + } + catch (e: Throwable) { + // exception swallowed + } + } + + suspend fun testCeInheritorLoggedAndMultipleGenericCatchClauses() { + try { + // anything + } + // subclasses should not be reported: + catch (e: SubclassOfCancellationException) { + LOG.error(e) + throw e + } + catch (e: CancellationException) { + LOG.error(e) + throw e + } + catch (e: RuntimeException) { + LOG.error(e) + } + catch (e: Exception) { + LOG.error(e) + } + catch (e: Throwable) { + LOG.error(e) + } + } + + suspend fun testNotHandlingOuterTryIfNestedCatchesCe() { + try { + // anything + try { + // anything + } + catch (e: CancellationException) { + LOG.error(e) + } + } + catch (e: Throwable) { + LOG.error(e) + } + } + +} \ No newline at end of file diff --git a/plugins/devkit/devkit-kotlin-tests/testData/inspections/incorrectPceHandling/IncorrectCeHandlingWhenPceCaughtImplicitlyTests.kt b/plugins/devkit/devkit-kotlin-tests/testData/inspections/incorrectPceHandling/IncorrectCeHandlingWhenPceCaughtImplicitlyTests.kt new file mode 100644 index 000000000000..4d6d99153e0e --- /dev/null +++ b/plugins/devkit/devkit-kotlin-tests/testData/inspections/incorrectPceHandling/IncorrectCeHandlingWhenPceCaughtImplicitlyTests.kt @@ -0,0 +1,227 @@ +import com.example.SubclassOfCancellationException +import com.intellij.openapi.diagnostic.Logger +import kotlinx.coroutines.CancellationException + +private val LOG = Logger.getInstance("any") + +class IncorrectCeHandlingWhenCeCaughtImplicitlyTests { + + // tests for CancellationException + @Throws(CancellationException::class) + fun throwCe() { + // anything + } + + fun testCeSwallowed() { + try { + throwCe() + } + catch (e: Exception) { + // exception swallowed + } + } + + fun testCeLogged() { + try { + throwCe() + } + catch (e: Exception) { + LOG.error(e) + throw e + } + } + + fun testSwallowedAndLoggedWithMessageOnInfoLevel() { + try { + throwCe() + } + catch (e: Exception) { + LOG.info("Error occured", e) + } + } + + fun testSwallowedAndLoggedOnInfoLevel() { + try { + throwCe() + } + catch (e: Exception) { + LOG.info(e) + } + } + + fun testSwallowedAndLoggedOnErrorLevel() { + try { + throwCe() + } + catch (e: Exception) { + LOG.error(e) + } + } + + fun testSwallowedAndOnlyExceptionMessageLogged() { + try { + throwCe() + } + catch (e: Exception) { + LOG.error("Error occurred: " + e.message) + } + } + + fun testCeSwallowedAndMultipleGenericCatchClauses() { + try { + throwCe() + } + catch (e: RuntimeException) { + // exception swallowed + } + catch (e: Exception) { + // exception swallowed + } + catch (e: Throwable) { + // exception swallowed + } + } + + fun testCeLoggedAndMultipleGenericCatchClauses() { + try { + throwCe() + } + catch (e: RuntimeException) { + LOG.error(e) + throw e + } + catch (e: Exception) { + LOG.error(e) + throw e + } + catch (e: Throwable) { + LOG.error(e) + throw e + } + } + + fun testNotHandlingOuterTryIfNestedCatchesCe() { + try { + // anything + try { + throwCe() + } + catch (e: CancellationException) { + LOG.error(e) + } + } + catch (e: Throwable) { + LOG.error(e) + } + } + + // tests for CancellationException inheritor; they should not be reported + @Throws(SubclassOfCancellationException::class) + fun throwCeInheritor() { + // anything + } + + fun testCeInheritorSwallowed() { + try { + throwCeInheritor() + } + catch (e: Exception) { + // exception swallowed + } + } + + fun testCeInheritorLogged() { + try { + throwCeInheritor() + } + catch (e: Exception) { + LOG.error(e) + throw e + } + } + + fun testCeInheritorSwallowedAndLoggedWithMessageOnInfoLevel() { + try { + throwCeInheritor() + } + catch (e: Exception) { + LOG.info("Error occured", e) + } + } + + fun testCeInheritorSwallowedAndLoggedOnInfoLevel() { + try { + throwCeInheritor() + } + catch (e: Exception) { + LOG.info(e) + } + } + + fun testCeInheritorSwallowedAndLoggedOnErrorLevel() { + try { + throwCeInheritor() + } + catch (e: Exception) { + LOG.error(e) + } + } + + fun testCeInheritorSwallowedAndOnlyExceptionMessageLogged() { + try { + throwCeInheritor() + } + catch (e: Exception) { + LOG.error("Error occurred: " + e.message) + } + } + + fun testCeInheritorSwallowedAndMultipleGenericCatchClauses() { + try { + throwCeInheritor() + } + catch (e: RuntimeException) { + // exception swallowed + } + catch (e: Exception) { + // exception swallowed + } + catch (e: Throwable) { + // exception swallowed + } + } + + fun testCeInheritorLoggedAndMultipleGenericCatchClauses() { + try { + throwCeInheritor() + } + catch (e: RuntimeException) { + LOG.error(e) + throw e + } + catch (e: Exception) { + LOG.error(e) + throw e + } + catch (e: Throwable) { + LOG.error(e) + throw e + } + } + + fun testNotHandlingOuterTryIfNestedCatchesCeInheritor() { + try { + // anything + try { + throwCeInheritor() + } + catch (e: SubclassOfCancellationException) { + LOG.error(e) + } + } + catch (e: Throwable) { + LOG.error(e) + } + } + +} diff --git a/plugins/devkit/devkit-kotlin-tests/testData/inspections/incorrectPceHandling/IncorrectPceHandlingTests.kt b/plugins/devkit/devkit-kotlin-tests/testData/inspections/incorrectPceHandling/IncorrectPceHandlingTests.kt index 4b189ddefb23..ce750d8b7b10 100644 --- a/plugins/devkit/devkit-kotlin-tests/testData/inspections/incorrectPceHandling/IncorrectPceHandlingTests.kt +++ b/plugins/devkit/devkit-kotlin-tests/testData/inspections/incorrectPceHandling/IncorrectPceHandlingTests.kt @@ -9,7 +9,7 @@ class IncorrectPceHandlingTests { try { // anything } - catch (e: ProcessCanceledException) { + catch (e: ProcessCanceledException) { // exception swallowed } } @@ -19,7 +19,7 @@ class IncorrectPceHandlingTests { // anything } catch (e: ProcessCanceledException) { - LOG.error(e) + LOG.error(e) throw e } } @@ -28,8 +28,8 @@ class IncorrectPceHandlingTests { try { // anything } - catch (e: ProcessCanceledException) { - LOG.info("Error occured", e) + catch (e: ProcessCanceledException) { + LOG.info("Error occured", e) } } @@ -37,8 +37,8 @@ class IncorrectPceHandlingTests { try { // anything } - catch (e: ProcessCanceledException) { - LOG.info(e) + catch (e: ProcessCanceledException) { + LOG.info(e) } } @@ -46,8 +46,8 @@ class IncorrectPceHandlingTests { try { // anything } - catch (e: ProcessCanceledException) { - LOG.error(e) + catch (e: ProcessCanceledException) { + LOG.error(e) } } @@ -55,8 +55,8 @@ class IncorrectPceHandlingTests { try { // anything } - catch (e: ProcessCanceledException) { - LOG.error("Error occurred: " + e.message) + catch (e: ProcessCanceledException) { + LOG.error("Error occurred: " + e.message) } } @@ -64,8 +64,8 @@ class IncorrectPceHandlingTests { try { // anything } - catch (e: SubclassOfProcessCanceledException) { - LOG.error(e) + catch (e: SubclassOfProcessCanceledException) { + LOG.error(e) } } diff --git a/plugins/devkit/devkit-kotlin-tests/testData/inspections/incorrectPceHandling/IncorrectPceHandlingWhenMultipleCatchClausesTests.kt b/plugins/devkit/devkit-kotlin-tests/testData/inspections/incorrectPceHandling/IncorrectPceHandlingWhenMultipleCatchClausesTests.kt index cf5b3f8049bb..7953dbf559cb 100644 --- a/plugins/devkit/devkit-kotlin-tests/testData/inspections/incorrectPceHandling/IncorrectPceHandlingWhenMultipleCatchClausesTests.kt +++ b/plugins/devkit/devkit-kotlin-tests/testData/inspections/incorrectPceHandling/IncorrectPceHandlingWhenMultipleCatchClausesTests.kt @@ -9,7 +9,7 @@ class IncorrectPceHandlingWhenMultipleCatchClausesTests { try { // anything } - catch (e: ProcessCanceledException) { + catch (e: ProcessCanceledException) { // exception swallowed } catch (e: Exception) { @@ -22,7 +22,7 @@ class IncorrectPceHandlingWhenMultipleCatchClausesTests { // anything } catch (e: ProcessCanceledException) { - LOG.error(e) + LOG.error(e) throw e } catch (e: Exception) { @@ -34,8 +34,8 @@ class IncorrectPceHandlingWhenMultipleCatchClausesTests { try { // anything } - catch (e: ProcessCanceledException) { - LOG.info("Error occured", e) + catch (e: ProcessCanceledException) { + LOG.info("Error occured", e) } catch (e: Exception) { LOG.info("Error occured", e) @@ -46,8 +46,8 @@ class IncorrectPceHandlingWhenMultipleCatchClausesTests { try { // anything } - catch (e: ProcessCanceledException) { - LOG.info(e) + catch (e: ProcessCanceledException) { + LOG.info(e) } catch (e: Exception) { LOG.info(e) @@ -58,8 +58,8 @@ class IncorrectPceHandlingWhenMultipleCatchClausesTests { try { // anything } - catch (e: ProcessCanceledException) { - LOG.error(e) + catch (e: ProcessCanceledException) { + LOG.error(e) } catch (e: Exception) { LOG.error(e) @@ -70,8 +70,8 @@ class IncorrectPceHandlingWhenMultipleCatchClausesTests { try { // anything } - catch (e: ProcessCanceledException) { - LOG.error("Error occurred: " + e.message) + catch (e: ProcessCanceledException) { + LOG.error("Error occurred: " + e.message) } catch (e: Exception) { LOG.error("Error occurred: " + e.message) @@ -82,7 +82,7 @@ class IncorrectPceHandlingWhenMultipleCatchClausesTests { try { // anything } - catch (e: ProcessCanceledException) { + catch (e: ProcessCanceledException) { // exception swallowed } catch (e: RuntimeException) { @@ -101,7 +101,7 @@ class IncorrectPceHandlingWhenMultipleCatchClausesTests { // anything } catch (e: ProcessCanceledException) { - LOG.error(e) + LOG.error(e) throw e } catch (e: RuntimeException) { @@ -119,10 +119,10 @@ class IncorrectPceHandlingWhenMultipleCatchClausesTests { try { // anything } - catch (e: SubclassOfProcessCanceledException) { + catch (e: SubclassOfProcessCanceledException) { // exception swallowed } - catch (e: ProcessCanceledException) { + catch (e: ProcessCanceledException) { // exception swallowed } catch (e: RuntimeException) { @@ -141,11 +141,11 @@ class IncorrectPceHandlingWhenMultipleCatchClausesTests { // anything } catch (e: SubclassOfProcessCanceledException) { - LOG.error(e) + LOG.error(e) throw e } catch (e: ProcessCanceledException) { - LOG.error(e) + LOG.error(e) throw e } catch (e: RuntimeException) { @@ -165,8 +165,8 @@ class IncorrectPceHandlingWhenMultipleCatchClausesTests { try { // anything } - catch (e: ProcessCanceledException) { - LOG.error(e) + catch (e: ProcessCanceledException) { + LOG.error(e) } } catch (e: Throwable) { diff --git a/plugins/devkit/devkit-kotlin-tests/testData/inspections/incorrectPceHandling/IncorrectPceHandlingWhenPceCaughtImplicitlyTests.kt b/plugins/devkit/devkit-kotlin-tests/testData/inspections/incorrectPceHandling/IncorrectPceHandlingWhenPceCaughtImplicitlyTests.kt index c14ecbffcb55..abfceb3fbcb2 100644 --- a/plugins/devkit/devkit-kotlin-tests/testData/inspections/incorrectPceHandling/IncorrectPceHandlingWhenPceCaughtImplicitlyTests.kt +++ b/plugins/devkit/devkit-kotlin-tests/testData/inspections/incorrectPceHandling/IncorrectPceHandlingWhenPceCaughtImplicitlyTests.kt @@ -16,7 +16,7 @@ class IncorrectPceHandlingWhenPceCaughtImplicitlyTests { try { throwPce() } - catch (e: Exception) { + catch (e: Exception) { // exception swallowed } } @@ -26,7 +26,7 @@ class IncorrectPceHandlingWhenPceCaughtImplicitlyTests { throwPce() } catch (e: Exception) { - LOG.error(e) + LOG.error(e) throw e } } @@ -35,8 +35,8 @@ class IncorrectPceHandlingWhenPceCaughtImplicitlyTests { try { throwPce() } - catch (e: Exception) { - LOG.info("Error occured", e) + catch (e: Exception) { + LOG.info("Error occured", e) } } @@ -44,8 +44,8 @@ class IncorrectPceHandlingWhenPceCaughtImplicitlyTests { try { throwPce() } - catch (e: Exception) { - LOG.info(e) + catch (e: Exception) { + LOG.info(e) } } @@ -53,8 +53,8 @@ class IncorrectPceHandlingWhenPceCaughtImplicitlyTests { try { throwPce() } - catch (e: Exception) { - LOG.error(e) + catch (e: Exception) { + LOG.error(e) } } @@ -62,8 +62,8 @@ class IncorrectPceHandlingWhenPceCaughtImplicitlyTests { try { throwPce() } - catch (e: Exception) { - LOG.error("Error occurred: " + e.message) + catch (e: Exception) { + LOG.error("Error occurred: " + e.message) } } @@ -71,7 +71,7 @@ class IncorrectPceHandlingWhenPceCaughtImplicitlyTests { try { throwPce() } - catch (e: RuntimeException) { + catch (e: RuntimeException) { // exception swallowed } catch (e: Exception) { @@ -87,7 +87,7 @@ class IncorrectPceHandlingWhenPceCaughtImplicitlyTests { throwPce() } catch (e: RuntimeException) { - LOG.error(e) + LOG.error(e) throw e } catch (e: Exception) { @@ -106,8 +106,8 @@ class IncorrectPceHandlingWhenPceCaughtImplicitlyTests { try { throwPce() } - catch (e: ProcessCanceledException) { - LOG.error(e) + catch (e: ProcessCanceledException) { + LOG.error(e) } } catch (e: Throwable) { @@ -125,7 +125,7 @@ class IncorrectPceHandlingWhenPceCaughtImplicitlyTests { try { throwPceInheritor() } - catch (e: Exception) { + catch (e: Exception) { // exception swallowed } } @@ -135,7 +135,7 @@ class IncorrectPceHandlingWhenPceCaughtImplicitlyTests { throwPceInheritor() } catch (e: Exception) { - LOG.error(e) + LOG.error(e) throw e } } @@ -144,8 +144,8 @@ class IncorrectPceHandlingWhenPceCaughtImplicitlyTests { try { throwPceInheritor() } - catch (e: Exception) { - LOG.info("Error occured", e) + catch (e: Exception) { + LOG.info("Error occured", e) } } @@ -153,8 +153,8 @@ class IncorrectPceHandlingWhenPceCaughtImplicitlyTests { try { throwPceInheritor() } - catch (e: Exception) { - LOG.info(e) + catch (e: Exception) { + LOG.info(e) } } @@ -162,8 +162,8 @@ class IncorrectPceHandlingWhenPceCaughtImplicitlyTests { try { throwPceInheritor() } - catch (e: Exception) { - LOG.error(e) + catch (e: Exception) { + LOG.error(e) } } @@ -171,8 +171,8 @@ class IncorrectPceHandlingWhenPceCaughtImplicitlyTests { try { throwPceInheritor() } - catch (e: Exception) { - LOG.error("Error occurred: " + e.message) + catch (e: Exception) { + LOG.error("Error occurred: " + e.message) } } @@ -180,7 +180,7 @@ class IncorrectPceHandlingWhenPceCaughtImplicitlyTests { try { throwPceInheritor() } - catch (e: RuntimeException) { + catch (e: RuntimeException) { // exception swallowed } catch (e: Exception) { @@ -196,7 +196,7 @@ class IncorrectPceHandlingWhenPceCaughtImplicitlyTests { throwPceInheritor() } catch (e: RuntimeException) { - LOG.error(e) + LOG.error(e) throw e } catch (e: Exception) { @@ -215,8 +215,8 @@ class IncorrectPceHandlingWhenPceCaughtImplicitlyTests { try { throwPceInheritor() } - catch (e: SubclassOfProcessCanceledException) { - LOG.error(e) + catch (e: SubclassOfProcessCanceledException) { + LOG.error(e) } } catch (e: Throwable) { diff --git a/plugins/devkit/devkit-kotlin-tests/testSrc/org/jetbrains/idea/devkit/kotlin/inspections/KtIncorrectProcessCanceledExceptionHandlingInspectionTest.kt b/plugins/devkit/devkit-kotlin-tests/testSrc/org/jetbrains/idea/devkit/kotlin/inspections/KtIncorrectProcessCanceledExceptionHandlingInspectionTest.kt index b253347571bf..503491e4786d 100644 --- a/plugins/devkit/devkit-kotlin-tests/testSrc/org/jetbrains/idea/devkit/kotlin/inspections/KtIncorrectProcessCanceledExceptionHandlingInspectionTest.kt +++ b/plugins/devkit/devkit-kotlin-tests/testSrc/org/jetbrains/idea/devkit/kotlin/inspections/KtIncorrectProcessCanceledExceptionHandlingInspectionTest.kt @@ -1,4 +1,4 @@ -// Copyright 2000-2023 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. package org.jetbrains.idea.devkit.kotlin.inspections import com.intellij.testFramework.TestDataPath @@ -6,12 +6,11 @@ import org.intellij.lang.annotations.Language import org.jetbrains.idea.devkit.inspections.IncorrectProcessCanceledExceptionHandlingInspectionTestBase import org.jetbrains.idea.devkit.kotlin.DevkitKtTestsUtil -@TestDataPath("\$CONTENT_ROOT/testData/inspections/incorrectPceHandling") -class KtIncorrectProcessCanceledExceptionHandlingInspectionTest : IncorrectProcessCanceledExceptionHandlingInspectionTestBase() { +abstract class KtIncorrectProcessCanceledExceptionHandlingInspectionTestBase : IncorrectProcessCanceledExceptionHandlingInspectionTestBase() { override fun setUp() { super.setUp() - addKotlinFile("JvmPlatformAnnotations.kt", """ + addKotlinFile("JvmPlatformAnnotations.kt", """ package kotlin.jvm import kotlin.reflect.KClass @@ -19,7 +18,7 @@ class KtIncorrectProcessCanceledExceptionHandlingInspectionTest : IncorrectProce @Retention(AnnotationRetention.SOURCE) annotation class Throws(vararg val exceptionClasses: KClass) """) - addKotlinFile("Throws.kt", """ + addKotlinFile("Throws.kt", """ package kotlin @SinceKotlin("1.4") @@ -32,10 +31,19 @@ class KtIncorrectProcessCanceledExceptionHandlingInspectionTest : IncorrectProce """) } - private fun addKotlinFile(relativePath: String, @Language("kotlin") fileText: String) { + protected fun addKotlinFile(relativePath: String, @Language("kotlin") fileText: String) { myFixture.addFileToProject(relativePath, fileText) } + override fun getBasePath() = DevkitKtTestsUtil.TESTDATA_PATH + "inspections/incorrectPceHandling" + + override fun getFileExtension() = "kt" + +} + +@TestDataPath("\$CONTENT_ROOT/testData/inspections/incorrectPceHandling") +class KtIncorrectProcessCanceledExceptionHandlingInspectionTest : KtIncorrectProcessCanceledExceptionHandlingInspectionTestBase() { + fun testIncorrectPceHandlingTests() { doTest() } @@ -47,9 +55,73 @@ class KtIncorrectProcessCanceledExceptionHandlingInspectionTest : IncorrectProce fun testIncorrectPceHandlingWhenPceCaughtImplicitlyTests() { doTest() } +} - override fun getBasePath() = DevkitKtTestsUtil.TESTDATA_PATH + "inspections/incorrectPceHandling" - override fun getFileExtension() = "kt" +@TestDataPath("\$CONTENT_ROOT/testData/inspections/incorrectPceHandling") +class KtIncorrectCancellationExceptionHandlingInspectionTest : KtIncorrectProcessCanceledExceptionHandlingInspectionTestBase() { + + private val USE_K2_KEY = "idea.kotlin.plugin.use.k2" + private var previousK2Property: String? = null + + override fun setUp() { + previousK2Property = System.getProperty(USE_K2_KEY) + System.setProperty(USE_K2_KEY, "true") + super.setUp() + myFixture.addClass(""" + package java.util.concurrent; + public class CancellationException extends IllegalStateException { + public CancellationException() {} + public CancellationException(String message) { super(message); } + } + """.trimIndent() + ) + addKotlinFile("CancellationException.kt", """ + package kotlinx.coroutines + + @SinceKotlin("1.4") + typealias CancellationException = java.util.concurrent.CancellationException + """) + addKotlinFile("SubclassOfCancellationException.kt", """ + package com.example + import kotlinx.coroutines.CancellationException + class SubclassOfCancellationException : CancellationException() + """) + } + + override fun tearDown() { + try { + val prevK2Property = previousK2Property + if (prevK2Property != null) { + System.setProperty(USE_K2_KEY, prevK2Property) + } + else { + System.clearProperty(USE_K2_KEY) + } + } + catch (e: Throwable) { + addSuppressedException(e) + } + finally { + super.tearDown() + } + } + + fun testIncorrectCeHandlingTests() { + doTest() + } + + fun testIncorrectCeHandlingInSuspendingLambdasTests() { + doTest() + } + + fun testIncorrectCeHandlingWhenMultipleCatchClausesTests() { + doTest() + } + + // TODO: disabled - for some reason @Throws cannot be resolved in test data + /*fun testIncorrectCeHandlingWhenPceCaughtImplicitlyTests() { + doTest() + }*/ } diff --git a/plugins/devkit/intellij.kotlin.devkit/resources/intellij.kotlin.devkit.xml b/plugins/devkit/intellij.kotlin.devkit/resources/intellij.kotlin.devkit.xml index 4ce211bf963e..e8fe1956dd3f 100644 --- a/plugins/devkit/intellij.kotlin.devkit/resources/intellij.kotlin.devkit.xml +++ b/plugins/devkit/intellij.kotlin.devkit/resources/intellij.kotlin.devkit.xml @@ -86,5 +86,8 @@ + diff --git a/plugins/devkit/intellij.kotlin.devkit/src/inspections/KtCancellationExceptionHandlingChecker.kt b/plugins/devkit/intellij.kotlin.devkit/src/inspections/KtCancellationExceptionHandlingChecker.kt new file mode 100644 index 000000000000..1e8d896e2429 --- /dev/null +++ b/plugins/devkit/intellij.kotlin.devkit/src/inspections/KtCancellationExceptionHandlingChecker.kt @@ -0,0 +1,77 @@ +// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +package org.jetbrains.idea.devkit.kotlin.inspections + +import com.intellij.psi.PsiElement +import org.jetbrains.idea.devkit.inspections.CancellationExceptionHandlingChecker +import org.jetbrains.idea.devkit.kotlin.util.getContext +import org.jetbrains.kotlin.analysis.api.KaSession +import org.jetbrains.kotlin.analysis.api.analyze +import org.jetbrains.kotlin.analysis.api.annotations.KaAnnotationValue +import org.jetbrains.kotlin.analysis.api.symbols.KaCallableSymbol +import org.jetbrains.kotlin.analysis.api.symbols.name +import org.jetbrains.kotlin.analysis.api.types.KaType +import org.jetbrains.kotlin.idea.references.mainReference +import org.jetbrains.kotlin.name.ClassId +import org.jetbrains.kotlin.psi.KtCallExpression +import org.jetbrains.kotlin.psi.KtParameter +import org.jetbrains.kotlin.psi.KtTryExpression + +private val cancellationExceptionClassId = ClassId.fromString("kotlinx/coroutines/CancellationException") +private val throwsClassId = ClassId.fromString("kotlin/jvm/Throws") + +internal class KtCancellationExceptionHandlingChecker : CancellationExceptionHandlingChecker { + + override fun isSuspicious(catchParameter: PsiElement): Boolean { + val parameter = catchParameter as? KtParameter ?: return false + if (getContext(parameter).isSuspending()) { + analyze(parameter) { + val catchParameterType = parameter.typeReference?.type ?: return false + val cancellationException = buildClassType(cancellationExceptionClassId) + return catchParameterType.semanticallyEquals(cancellationException) + } + } + return false + } + + override fun getCeName(): String { + return "kotlinx.coroutines.CancellationException" + } + + override fun containsSuspiciousCeCatchClause(tryExpression: PsiElement): Boolean { + val parameter = tryExpression as? KtTryExpression ?: return false + if (getContext(parameter).isSuspending()) { + return analyze(parameter) { + parameter.catchClauses + .any { it.catchParameter?.expressionType?.semanticallyEquals(buildCancellationExceptionType()) == true } + } + } + return false + } + + override fun findCeThrowingExpressionName(tryExpression: PsiElement): String? { + val ktTryExpression = tryExpression as? KtTryExpression ?: return null + for (expression in ktTryExpression.tryBlock.statements.filterIsInstance()) { + analyze(expression) { + // workaround with calleeExpression as expression.mainReference?.resolveToSymbol() doesn't work + val symbol = expression.calleeExpression?.mainReference?.resolveToSymbol() as? KaCallableSymbol ?: return@analyze + if (throwsCe(symbol)) { + return symbol.name?.asString() + } + } + } + return null + } + + private fun KaSession.throwsCe(symbol: KaCallableSymbol): Boolean { + val exceptionClasses = symbol.annotations.firstOrNull { it.classId == throwsClassId } + ?.arguments?.firstOrNull { it.name.asString() == "exceptionClasses" } + ?.expression as? KaAnnotationValue.ArrayValue ?: return false + return exceptionClasses.values + .filterIsInstance() + .any { it.type.semanticallyEquals(buildCancellationExceptionType()) } + } + + private fun KaSession.buildCancellationExceptionType(): KaType = + buildClassType(cancellationExceptionClassId) + +}