mirror of
https://gitflic.ru/project/openide/openide.git
synced 2026-09-27 10:03:11 +07:00
IJPL-192063: Fix a false positive in IncorrectCancellationExceptionHandlingInspection
GitOrigin-RevId: d1dc891c45ef3c55d69286026c8acc12c7717bc3
This commit is contained in:
committed by
intellij-monorepo-bot
parent
78a70130fa
commit
ba5bb522c6
+41
-25
@@ -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<UParameter>): CaughtCeInfo? {
|
||||
val pceClass = findPceClass(holder.file.resolveScope) ?: return null
|
||||
private fun findSuspiciousCeCaughtParam(catchParameters: List<UParameter>, 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<UParameter>,
|
||||
pceClass: PsiClass,
|
||||
): Boolean {
|
||||
val tryExpression = catchClause.getParentOfType<UTryExpression>() ?: 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<PsiClass> {
|
||||
val factory = JavaPsiFacade.getElementFactory(pceClass.project)
|
||||
val pceClassType = factory.createType(pceClass)
|
||||
val collectProcessor = CommonProcessors.CollectProcessor<PsiType>()
|
||||
InheritanceUtil.processSuperTypes(pceClassType, false, collectProcessor)
|
||||
return collectProcessor.results
|
||||
.filterIsInstance<PsiClassType>()
|
||||
.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<PsiClass>,
|
||||
): 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)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
+3
-3
@@ -57,7 +57,7 @@ class IncorrectPceHandlingTests {
|
||||
void testDisjunctionTypesWhenPceIsFirst() {
|
||||
try {
|
||||
// anything
|
||||
} catch (ProcessCanceledException | IllegalStateException <error descr="'com.intellij.openapi.progress.ProcessCanceledException' must be rethrown">e</error>) {
|
||||
} catch (ProcessCanceledException | IllegalArgumentException <error descr="'com.intellij.openapi.progress.ProcessCanceledException' must be rethrown">e</error>) {
|
||||
<error descr="'com.intellij.openapi.progress.ProcessCanceledException' must not be logged">LOG.error("Error occurred: " + e.getMessage())</error>;
|
||||
}
|
||||
}
|
||||
@@ -65,7 +65,7 @@ class IncorrectPceHandlingTests {
|
||||
void testDisjunctionTypesWhenPceIsSecond() {
|
||||
try {
|
||||
// anything
|
||||
} catch (IllegalStateException | ProcessCanceledException <error descr="'com.intellij.openapi.progress.ProcessCanceledException' must be rethrown">e</error>) {
|
||||
} catch (IllegalArgumentException | ProcessCanceledException <error descr="'com.intellij.openapi.progress.ProcessCanceledException' must be rethrown">e</error>) {
|
||||
<error descr="'com.intellij.openapi.progress.ProcessCanceledException' must not be logged">LOG.error("Error occurred: " + e.getMessage())</error>;
|
||||
}
|
||||
}
|
||||
@@ -81,7 +81,7 @@ class IncorrectPceHandlingTests {
|
||||
void testPceInheritorSwallowedAndLoggerWhenDisjunctionTypeDefined() {
|
||||
try {
|
||||
// anything
|
||||
} catch (IllegalStateException | SubclassOfProcessCanceledException <error descr="'com.intellij.openapi.progress.ProcessCanceledException' inheritor must be rethrown">e</error>) {
|
||||
} catch (IllegalArgumentException | SubclassOfProcessCanceledException <error descr="'com.intellij.openapi.progress.ProcessCanceledException' inheritor must be rethrown">e</error>) {
|
||||
<error descr="'com.intellij.openapi.progress.ProcessCanceledException' inheritor must not be logged">LOG.error(e)</error>;
|
||||
}
|
||||
}
|
||||
|
||||
+16
@@ -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();
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
+19
@@ -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()
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
+8
-4
@@ -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<out Throwable>)
|
||||
""")
|
||||
""".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) {
|
||||
|
||||
+7
-2
@@ -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;
|
||||
|
||||
Reference in New Issue
Block a user