From ba5bb522c603c70c42b0200c5f9f6cf7963ea35f Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Tue, 24 Jun 2025 21:20:30 +0200 Subject: [PATCH] IJPL-192063: Fix a false positive in IncorrectCancellationExceptionHandlingInspection GitOrigin-RevId: d1dc891c45ef3c55d69286026c8acc12c7717bc3 --- ...CancellationExceptionHandlingInspection.kt | 66 ++++++++++++------- .../IncorrectPceHandlingTests.java | 6 +- ...HandlingWhenMultipleCatchClausesTests.java | 16 +++++ ...PceHandlingWhenPceCaughtImplicitlyTests.kt | 19 ++++++ ...CanceledExceptionHandlingInspectionTest.kt | 12 ++-- ...tionExceptionHandlingInspectionTestBase.kt | 9 ++- 6 files changed, 94 insertions(+), 34 deletions(-) diff --git a/plugins/devkit/devkit-core/src/inspections/IncorrectCancellationExceptionHandlingInspection.kt b/plugins/devkit/devkit-core/src/inspections/IncorrectCancellationExceptionHandlingInspection.kt index 0b5cec3c52b2..06db20b9c742 100644 --- a/plugins/devkit/devkit-core/src/inspections/IncorrectCancellationExceptionHandlingInspection.kt +++ b/plugins/devkit/devkit-core/src/inspections/IncorrectCancellationExceptionHandlingInspection.kt @@ -12,6 +12,7 @@ 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 com.intellij.util.CommonProcessors import org.jetbrains.annotations.ApiStatus import org.jetbrains.idea.devkit.DevKitBundle.message import org.jetbrains.uast.* @@ -19,9 +20,6 @@ import org.jetbrains.uast.visitor.AbstractUastNonRecursiveVisitor import org.jetbrains.uast.visitor.AbstractUastVisitor private const val PCE_CLASS_NAME = "com.intellij.openapi.progress.ProcessCanceledException" -private const val RUNTIME_EXCEPTION_CLASS_NAME = "java.lang.RuntimeException" -private const val EXCEPTION_CLASS_NAME = "java.lang.Exception" -private const val THROWABLE_CLASS_NAME = "java.lang.Throwable" internal class IncorrectCancellationExceptionHandlingInspection : DevKitUastInspectionBase() { @@ -29,19 +27,19 @@ internal class IncorrectCancellationExceptionHandlingInspection : DevKitUastInsp return create(holder.file.language, object : AbstractUastNonRecursiveVisitor() { override fun visitCatchClause(node: UCatchClause): Boolean { + val pceClass = findPceClass(holder.file.resolveScope) ?: return super.visitCatchClause(node) val catchParameters = node.parameters - val caughtCeInfo = findSuspiciousCeCaughtParam(catchParameters) + val caughtCeInfo = findSuspiciousCeCaughtParam(catchParameters, pceClass) if (caughtCeInfo != null) { inspectIncorrectCeHandling(node, caughtCeInfo) } else { - inspectGenericThrowableIfAnyOfTryStatementsThrowsCe(node, catchParameters) + inspectGenericThrowableIfAnyOfTryStatementsThrowsCe(node, catchParameters, pceClass) } return super.visitCatchClause(node) } - private fun findSuspiciousCeCaughtParam(catchParameters: List): CaughtCeInfo? { - val pceClass = findPceClass(holder.file.resolveScope) ?: return null + private fun findSuspiciousCeCaughtParam(catchParameters: List, pceClass: PsiClass): CaughtCeInfo? { for (catchParameter in catchParameters) { // language-specific check: val checker = cancellationExceptionHandlingChecker(catchParameter.lang) @@ -73,10 +71,10 @@ internal class IncorrectCancellationExceptionHandlingInspection : DevKitUastInsp return PCE_CLASS_NAME == (this as? PsiClassType)?.resolve()?.qualifiedName } - private fun PsiType.isInheritorOrSelf(ceClass: PsiClass): Boolean { + private fun PsiType.isInheritorOrSelf(psiClass: PsiClass): Boolean { val psiClassType = this as? PsiClassType ?: return false - val psiClass = psiClassType.resolve() ?: return false - return InheritanceUtil.isInheritorOrSelf(psiClass, ceClass, true) + val checkedPsiClass = psiClassType.resolve() ?: return false + return InheritanceUtil.isInheritorOrSelf(checkedPsiClass, psiClass, true) } private fun inspectIncorrectCeHandling(node: UCatchClause, caughtCeInfo: CaughtCeInfo) { @@ -99,19 +97,22 @@ internal class IncorrectCancellationExceptionHandlingInspection : DevKitUastInsp private fun inspectGenericThrowableIfAnyOfTryStatementsThrowsCe( catchClause: UCatchClause, catchParameters: List, + pceClass: PsiClass, ): Boolean { val tryExpression = catchClause.getParentOfType() ?: return super.visitCatchClause(catchClause) if (tryExpression.containsCatchClauseForType(PCE_CLASS_NAME) || tryExpression.checkContainsSuspiciousCeCatchClause()) { // Cancellation exception will be caught by the explicit catch clause return super.visitCatchClause(catchClause) } + val pceSuperTypeClasses = getPceThrowableSuperTypeClasses(pceClass) val caughtGenericThrowableParam = catchParameters.firstOrNull { - it.type.isClassType(RUNTIME_EXCEPTION_CLASS_NAME) || - it.type.isClassType(EXCEPTION_CLASS_NAME) || - it.type.isClassType(THROWABLE_CLASS_NAME) + pceSuperTypeClasses.any { pceSuperTypeClass -> + val pceSuperTypeQualifiedName = pceSuperTypeClass.qualifiedName ?: return@any false + it.type.isClassType(pceSuperTypeQualifiedName) + } } if (caughtGenericThrowableParam != null) { - if (tryExpression.containsMoreSpecificCatchClause(caughtGenericThrowableParam)) { + if (tryExpression.containsMoreSpecificCatchClause(caughtGenericThrowableParam, pceSuperTypeClasses)) { // Cancellation exception will be caught by catch clause with a more specific type, so do not report return super.visitCatchClause(catchClause) } @@ -130,6 +131,17 @@ internal class IncorrectCancellationExceptionHandlingInspection : DevKitUastInsp return super.visitCatchClause(catchClause) } + private fun getPceThrowableSuperTypeClasses(pceClass: PsiClass): List { + val factory = JavaPsiFacade.getElementFactory(pceClass.project) + val pceClassType = factory.createType(pceClass) + val collectProcessor = CommonProcessors.CollectProcessor() + InheritanceUtil.processSuperTypes(pceClassType, false, collectProcessor) + return collectProcessor.results + .filterIsInstance() + .filter { InheritanceUtil.isInheritor(it, "java.lang.Throwable") } + .mapNotNull { it.resolve() } + } + private fun UTryExpression.checkContainsSuspiciousCeCatchClause(): Boolean { val sourcePsi = this.sourcePsi ?: return false return cancellationExceptionHandlingChecker(this.lang)?.containsSuspiciousCeCatchClause(sourcePsi) == true @@ -245,23 +257,27 @@ internal class IncorrectCancellationExceptionHandlingInspection : DevKitUastInsp return loggingExpression } - private fun PsiType.isClassType(fullyQualifiedClassName: String): Boolean { + private fun PsiType.isClassType(qualifiedName: String): Boolean { if (this is PsiDisjunctionType) { - return this.disjunctions.any { PsiTypesUtil.classNameEquals(it, fullyQualifiedClassName) } + return this.disjunctions.any { PsiTypesUtil.classNameEquals(it, qualifiedName) } } - return PsiTypesUtil.classNameEquals(this, fullyQualifiedClassName) + return PsiTypesUtil.classNameEquals(this, qualifiedName) } - private fun UTryExpression.containsCatchClauseForType(fullyQualifiedClassName: String): Boolean { - return this.catchClauses.any { clause -> clause.parameters.any { it.type.isClassType(fullyQualifiedClassName) } } + private fun UTryExpression.containsCatchClauseForType(qualifiedName: String): Boolean { + return this.catchClauses.any { clause -> clause.parameters.any { it.type.isClassType(qualifiedName) } } } - private fun UTryExpression.containsMoreSpecificCatchClause(param: UParameter): Boolean { - return when ((param.type as? PsiClassType)?.resolve()?.qualifiedName) { - java.lang.Throwable::class.java.name -> - this.containsCatchClauseForType(EXCEPTION_CLASS_NAME) || this.containsCatchClauseForType(RUNTIME_EXCEPTION_CLASS_NAME) - java.lang.Exception::class.java.name -> this.containsCatchClauseForType(RUNTIME_EXCEPTION_CLASS_NAME) - else -> false + private fun UTryExpression.containsMoreSpecificCatchClause( + param: UParameter, + pceSuperTypeClasses: Collection, + ): Boolean { + val parameterType = param.type as? PsiClassType ?: return false + val parameterTypeQualifiedName = parameterType.resolve()?.qualifiedName ?: return false + val subclassesOfCheckedType = pceSuperTypeClasses.filter { InheritanceUtil.isInheritor(it, true, parameterTypeQualifiedName) } + return subclassesOfCheckedType.any { + val qualifiedName = it.qualifiedName ?: return@any false + this.containsCatchClauseForType(qualifiedName) } } diff --git a/plugins/devkit/devkit-java-tests/testData/inspections/incorrectCeHandling/IncorrectPceHandlingTests.java b/plugins/devkit/devkit-java-tests/testData/inspections/incorrectCeHandling/IncorrectPceHandlingTests.java index da677e3e8fcc..4b0ead9da5e2 100644 --- a/plugins/devkit/devkit-java-tests/testData/inspections/incorrectCeHandling/IncorrectPceHandlingTests.java +++ b/plugins/devkit/devkit-java-tests/testData/inspections/incorrectCeHandling/IncorrectPceHandlingTests.java @@ -57,7 +57,7 @@ class IncorrectPceHandlingTests { void testDisjunctionTypesWhenPceIsFirst() { try { // anything - } catch (ProcessCanceledException | IllegalStateException e) { + } catch (ProcessCanceledException | IllegalArgumentException e) { LOG.error("Error occurred: " + e.getMessage()); } } @@ -65,7 +65,7 @@ class IncorrectPceHandlingTests { void testDisjunctionTypesWhenPceIsSecond() { try { // anything - } catch (IllegalStateException | ProcessCanceledException e) { + } catch (IllegalArgumentException | ProcessCanceledException e) { LOG.error("Error occurred: " + e.getMessage()); } } @@ -81,7 +81,7 @@ class IncorrectPceHandlingTests { void testPceInheritorSwallowedAndLoggerWhenDisjunctionTypeDefined() { try { // anything - } catch (IllegalStateException | SubclassOfProcessCanceledException e) { + } catch (IllegalArgumentException | SubclassOfProcessCanceledException e) { LOG.error(e); } } diff --git a/plugins/devkit/devkit-java-tests/testData/inspections/incorrectCeHandling/IncorrectPceHandlingWhenMultipleCatchClausesTests.java b/plugins/devkit/devkit-java-tests/testData/inspections/incorrectCeHandling/IncorrectPceHandlingWhenMultipleCatchClausesTests.java index ae4c0daef29e..2943bc9acaac 100644 --- a/plugins/devkit/devkit-java-tests/testData/inspections/incorrectCeHandling/IncorrectPceHandlingWhenMultipleCatchClausesTests.java +++ b/plugins/devkit/devkit-java-tests/testData/inspections/incorrectCeHandling/IncorrectPceHandlingWhenMultipleCatchClausesTests.java @@ -1,6 +1,7 @@ import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.progress.ProcessCanceledException; import com.example.SubclassOfProcessCanceledException; +import java.util.concurrent.CancellationException; class IncorrectPceHandlingWhenMultipleCatchClausesTests { private static final Logger LOG = Logger.getInstance(IncorrectPceHandlingWhenMultipleCatchClausesTests.class); @@ -144,4 +145,19 @@ class IncorrectPceHandlingWhenMultipleCatchClausesTests { } } + // IJPL-192063 + public static void foo() { + try { + bar(); + } catch (CancellationException e) { + throw e; + } catch (Exception e) { + e.printStackTrace(); + } + } + + static void bar() throws ProcessCanceledException { + throw new ProcessCanceledException(); + } + } diff --git a/plugins/devkit/devkit-kotlin-tests/testData/inspections/incorrectCeHandling/IncorrectPceHandlingWhenPceCaughtImplicitlyTests.kt b/plugins/devkit/devkit-kotlin-tests/testData/inspections/incorrectCeHandling/IncorrectPceHandlingWhenPceCaughtImplicitlyTests.kt index abfceb3fbcb2..ec06f344df28 100644 --- a/plugins/devkit/devkit-kotlin-tests/testData/inspections/incorrectCeHandling/IncorrectPceHandlingWhenPceCaughtImplicitlyTests.kt +++ b/plugins/devkit/devkit-kotlin-tests/testData/inspections/incorrectCeHandling/IncorrectPceHandlingWhenPceCaughtImplicitlyTests.kt @@ -1,6 +1,7 @@ import com.example.SubclassOfProcessCanceledException import com.intellij.openapi.diagnostic.Logger import com.intellij.openapi.progress.ProcessCanceledException +import kotlin.coroutines.cancellation.CancellationException private val LOG = Logger.getInstance("any") @@ -224,4 +225,22 @@ class IncorrectPceHandlingWhenPceCaughtImplicitlyTests { } } + // IJPL-192063 + fun foo() { + try { + bar() + } + catch (e: CancellationException) { + throw e + } + catch (e: java.lang.Exception) { + e.printStackTrace() + } + } + + @kotlin.Throws(com.intellij.openapi.progress.ProcessCanceledException::class) + fun bar() { + throw com.intellij.openapi.progress.ProcessCanceledException() + } + } 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 344dba3056f7..8b3267dce481 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-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +// Copyright 2000-2025 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 @@ -17,18 +17,22 @@ abstract class KtIncorrectCancellationExceptionHandlingInspectionTestBase : Inco @Target(AnnotationTarget.FUNCTION, AnnotationTarget.PROPERTY_GETTER, AnnotationTarget.PROPERTY_SETTER, AnnotationTarget.CONSTRUCTOR) @Retention(AnnotationRetention.SOURCE) annotation class Throws(vararg val exceptionClasses: KClass) - """) + """.trimIndent()) addKotlinFile("Throws.kt", """ package kotlin @SinceKotlin("1.4") actual typealias Throws = kotlin.jvm.Throws - """) + """.trimIndent()) + addKotlinFile("CancellationException.kt", """ + package kotlin.coroutines.cancellation + actual typealias CancellationException = java.util.concurrent.CancellationException + """.trimIndent()) addKotlinFile("SubclassOfProcessCanceledException.kt", """ package com.example import com.intellij.openapi.progress.ProcessCanceledException class SubclassOfProcessCanceledException : ProcessCanceledException() - """) + """.trimIndent()) } protected fun addKotlinFile(relativePath: String, @Language("kotlin") fileText: String) { diff --git a/plugins/devkit/devkit-tests/testSrc/org/jetbrains/idea/devkit/inspections/IncorrectCancellationExceptionHandlingInspectionTestBase.kt b/plugins/devkit/devkit-tests/testSrc/org/jetbrains/idea/devkit/inspections/IncorrectCancellationExceptionHandlingInspectionTestBase.kt index 57c109b9cce3..db6fd7b509eb 100644 --- a/plugins/devkit/devkit-tests/testSrc/org/jetbrains/idea/devkit/inspections/IncorrectCancellationExceptionHandlingInspectionTestBase.kt +++ b/plugins/devkit/devkit-tests/testSrc/org/jetbrains/idea/devkit/inspections/IncorrectCancellationExceptionHandlingInspectionTestBase.kt @@ -1,4 +1,4 @@ -// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +// Copyright 2000-2025 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. package org.jetbrains.idea.devkit.inspections import com.intellij.testFramework.fixtures.LightJavaCodeInsightFixtureTestCase @@ -8,9 +8,14 @@ abstract class IncorrectCancellationExceptionHandlingInspectionTestBase : LightJ override fun setUp() { super.setUp() myFixture.enableInspections(IncorrectCancellationExceptionHandlingInspection()) + myFixture.addClass(""" + package java.util.concurrent; + public class CancellationException extends IllegalStateException {} + """.trimIndent()) myFixture.addClass(""" package com.intellij.openapi.progress; - public class ProcessCanceledException extends RuntimeException {} + import java.util.concurrent.CancellationException; + public class ProcessCanceledException extends CancellationException {} """.trimIndent()) myFixture.addClass(""" package com.intellij.openapi.diagnostic;