IDEA-330739: Devkit: extend "Cancellation check in loops" inspection

- Added support for forEach() and similar methods, and ContainerUtil.process()

GitOrigin-RevId: fc9b66f3584cf94d603d9a84825f4142de92544d
This commit is contained in:
Karol Lewandowski
2024-02-08 19:47:32 +01:00
committed by intellij-monorepo-bot
parent cfb343476f
commit bf33f56ded
40 changed files with 1090 additions and 120 deletions

View File

@@ -1,25 +1,39 @@
<html>
<body>
Reports <code>forEach</code> loops with missing cancellation checks.
Reports loops, <code>forEach</code>-like methods, and <code>ContainerUtil.process()</code> with missing cancellation checks.
<p>Runs only within the methods with <code>com.intellij.util.concurrency.annotations.RequiresReadLock</code> annotation.</p>
<p><b>Example:</b></p>
<pre><code lang="kotlin">
for (item in items) {
ProgressManager.checkCanceled() // should be present in the first line
...
}
@RequiresReadLock
fun doSomething() {
...
for (item in items) {
ProgressManager.checkCanceled() // should be present in the first line
...
}
items.forEach {
ProgressManager.checkCanceled() // should be present in the first line
...
}
...
}
</code></pre>
<p>
In case of nested loops with nothing in between:
</p>
<p>In the case of nested loops with nothing in between:</p>
<pre><code lang="kotlin">
for (item in items) {
// nothing in between
for (inner in item.inners) {
ProgressManager.checkCanceled() // should be present in the first line of the inner loop only
...
}
@RequiresReadLock
fun doSomething() {
...
for (item in items) {
// nothing in between
for (inner in item.inners) {
ProgressManager.checkCanceled() // should be present in the first line of the inner loop only
...
}
}
...
}
</code></pre>
<p>
In blocking context <code>com.intellij.openapi.progress.ProgressManager#checkCanceled</code> should be used,
@@ -30,4 +44,4 @@ Reports <code>forEach</code> loops with missing cancellation checks.
</p>
</body>
<p><small>New in 2023.1</small>
</html>
</html>

View File

@@ -1,16 +1,35 @@
// 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.inspections
import com.intellij.codeInsight.AnnotationUtil
import com.intellij.codeInspection.ProblemsHolder
import com.intellij.psi.CommonClassNames.*
import com.intellij.psi.PsiElement
import com.intellij.psi.PsiElementVisitor
import com.intellij.uast.UastHintedVisitorAdapter
import com.intellij.util.concurrency.annotations.RequiresReadLock
import com.intellij.util.containers.ContainerUtil
import com.siyeh.ig.callMatcher.CallMatcher
import org.jetbrains.idea.devkit.DevKitBundle
import org.jetbrains.idea.devkit.inspections.quickfix.CancellationCheckInLoopsFixProviders
import org.jetbrains.uast.*
import org.jetbrains.uast.visitor.AbstractUastNonRecursiveVisitor
private val javaLoopMethods: CallMatcher = CallMatcher.anyOf(
CallMatcher.instanceCall(JAVA_LANG_ITERABLE, "forEach").parameterTypes(JAVA_UTIL_FUNCTION_CONSUMER),
CallMatcher.instanceCall(JAVA_UTIL_ITERATOR, "forEachRemaining").parameterTypes(JAVA_UTIL_FUNCTION_CONSUMER),
CallMatcher.instanceCall(JAVA_UTIL_STREAM_STREAM, "forEach", "forEachOrdered").parameterTypes(JAVA_UTIL_FUNCTION_CONSUMER), // TODO: IntStream, etc.?
CallMatcher.instanceCall(JAVA_UTIL_MAP, "forEach").parameterTypes("java.util.function.BiConsumer"),
CallMatcher.staticCall(ContainerUtil::class.java.name, "process")
)
private val kotlinLoopMethods: CallMatcher = CallMatcher.anyOf(
CallMatcher.staticCall("kotlin.collections.ArraysKt___ArraysKt", "forEach", "forEachIndexed"),
CallMatcher.staticCall("kotlin.collections.CollectionsKt___CollectionsKt", "forEach", "forEachIndexed"),
CallMatcher.staticCall("kotlin.collections.CollectionsKt__IteratorsKt", "forEach"),
CallMatcher.staticCall("kotlin.collections.MapsKt___MapsKt", "forEach"),
CallMatcher.staticCall("kotlin.sequences.SequencesKt___SequencesKt", "forEach", "forEachIndexed")
)
internal class CancellationCheckInLoopsInspection : DevKitUastInspectionBase() {
@@ -39,8 +58,16 @@ internal class CancellationCheckInLoopsInspection : DevKitUastInspectionBase() {
inspectLoopExpression(node, checkProvider, holder)
return true
}
override fun visitCallExpression(node: UCallExpression): Boolean {
if (node.isLoopMethodCall()) {
val lambdaExpression = node.valueArguments.filterIsInstance<ULambdaExpression>().firstOrNull() ?: return true
inspectLoopExpression(lambdaExpression, checkProvider, holder)
}
return true
}
},
arrayOf(ULoopExpression::class.java)
arrayOf(ULoopExpression::class.java, UCallExpression::class.java)
)
}
@@ -48,24 +75,31 @@ internal class CancellationCheckInLoopsInspection : DevKitUastInspectionBase() {
* If the first expression in a loop is not another loop, finds the right cancellation check based on the context of a loop,
* and if it's missing, registers the problem.
*/
private fun inspectLoopExpression(loopExpression: ULoopExpression,
private fun inspectLoopExpression(loopOrLambdaExpression: UExpression,
checkProvider: CancellationCheckProvider,
holder: ProblemsHolder) {
val sourcePsi = loopExpression.sourcePsi ?: return
if (loopOrLambdaExpression !is ULoopExpression && loopOrLambdaExpression !is ULambdaExpression) return
if (!shouldBeRunOn(loopOrLambdaExpression)) return
if (!shouldBeRunOn(loopExpression)) return
val callContext = when (loopOrLambdaExpression) {
is ULoopExpression -> loopOrLambdaExpression.sourcePsi
is ULambdaExpression -> loopOrLambdaExpression.getParentOfType<UCallExpression>()?.sourcePsi
else -> null
} ?: return
val firstExpressionInLoop = loopExpression.bodyExpressions.firstOrNull()
val firstExpressionInLoop = loopOrLambdaExpression.bodyExpressions.firstOrNull()?.let {
if (it is UReturnExpression) it.returnExpression else it // fix for Kotlin implicit return in forEach functions
}
// Don't insert a check between nested loops if there is nothing in between
if (firstExpressionInLoop is ULoopExpression) return
if (firstExpressionInLoop?.isLoopExpressionOrLoopMethodCall() == true) return
val cancellationCheckFqn = checkProvider.findCancellationCheckCall(sourcePsi)
val cancellationCheckFqn = checkProvider.findCancellationCheckCall(callContext)
val firstExpressionInLoopSourcePsi = firstExpressionInLoop?.sourcePsi
if (firstExpressionInLoopSourcePsi != null && checkProvider.isCancellationCheckCall(firstExpressionInLoopSourcePsi,
cancellationCheckFqn)) return
val anchor = sourcePsi.firstChild
val anchor = getAnchor(loopOrLambdaExpression) ?: return
val fixProvider = CancellationCheckInLoopsFixProviders.forLanguage(holder.file.language) ?: return
val fixes = fixProvider.getFixes(anchor, cancellationCheckFqn)
holder.registerProblem(
@@ -83,13 +117,39 @@ internal class CancellationCheckInLoopsInspection : DevKitUastInspectionBase() {
return AnnotationUtil.isAnnotated(containingMethod.javaPsi, RequiresReadLock::class.java.canonicalName, AnnotationUtil.CHECK_HIERARCHY)
}
private val ULoopExpression.bodyExpressions: List<UExpression>
private fun UExpression.isLoopExpressionOrLoopMethodCall(): Boolean {
if (this is ULoopExpression) return true
val call = when (this) {
is UCallExpression -> this
is UQualifiedReferenceExpression -> this.selector
else -> return false
} as UCallExpression
return call.isLoopMethodCall()
}
private fun UCallExpression.isLoopMethodCall(): Boolean {
return javaLoopMethods.uCallMatches(this) || kotlinLoopMethods.uCallMatches(this)
}
private val UExpression.bodyExpressions: List<UExpression>
get() {
return when (val loopBody = body) {
val loopBody = when (this) {
is ULoopExpression -> this.body
is ULambdaExpression -> this.body
else -> return emptyList()
}
return when (loopBody) {
is UBlockExpression -> loopBody.expressions
else -> listOf(body)
else -> listOf(loopBody)
}
}
}
private fun getAnchor(loopOrLambdaExpression: UExpression): PsiElement? {
return when (loopOrLambdaExpression) {
is ULoopExpression -> loopOrLambdaExpression.sourcePsi?.firstChild
is ULambdaExpression -> loopOrLambdaExpression.getParentOfType<UCallExpression>()?.methodIdentifier?.sourcePsi
else -> null
}
}
}

View File

@@ -1,7 +1,7 @@
// 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.inspections.quickfix
import com.intellij.codeInsight.BlockUtils.expandSingleStatementToBlockStatement
import com.intellij.codeInsight.BlockUtils
import com.intellij.codeInspection.LocalQuickFix
import com.intellij.codeInspection.LocalQuickFixOnPsiElement
import com.intellij.lang.LanguageExtension
@@ -11,6 +11,7 @@ import com.intellij.openapi.util.IntellijInternalApi
import com.intellij.psi.*
import com.intellij.psi.codeStyle.JavaCodeStyleManager
import com.intellij.psi.util.parentOfType
import com.intellij.util.CommonJavaRefactoringUtil
import org.jetbrains.annotations.ApiStatus
import org.jetbrains.idea.devkit.DevKitBundle
@@ -25,48 +26,67 @@ internal object CancellationCheckInLoopsFixProviders : LanguageExtension<Cancell
@ApiStatus.Internal
interface CancellationCheckInLoopsFixProvider {
fun getFixes(loopKeyword: PsiElement, cancellationCheckFqn: String): List<LocalQuickFix>
fun getFixes(anchor: PsiElement, cancellationCheckFqn: String): List<LocalQuickFix>
}
internal class JavaCancellationCheckInLoopsFixProvider : CancellationCheckInLoopsFixProvider {
override fun getFixes(loopKeyword: PsiElement, cancellationCheckFqn: String): List<LocalQuickFix> {
return listOf(InsertCancellationCheckFix(cancellationCheckFqn, loopKeyword))
override fun getFixes(anchor: PsiElement, cancellationCheckFqn: String): List<LocalQuickFix> {
return when (anchor) {
is PsiIdentifier -> listOf(InsertCancellationCheckInLoopMethodFix(cancellationCheckFqn, anchor))
else -> listOf(InsertCancellationCheckInLoopFix(cancellationCheckFqn, anchor))
}
}
}
internal class InsertCancellationCheckFix(
private val cancellationCheckCallFqn: String,
loopKeyword: PsiElement,
) : LocalQuickFixOnPsiElement(loopKeyword) {
override fun getFamilyName(): String = DevKitBundle.message("inspection.insert.cancellation.check.fix.message")
override fun getText(): String = familyName
internal class InsertCancellationCheckInLoopFix(cancellationCheckCallFqn: String, loopKeyword: PsiElement)
: AbstractInsertCancellationCheckInLoopFix(cancellationCheckCallFqn, loopKeyword) {
override fun isAvailable(project: Project, file: PsiFile, startElement: PsiElement, endElement: PsiElement): Boolean {
return startElement.parentOfType<PsiLoopStatement>() != null
}
override fun getCodeBlock(startElement: PsiElement): PsiCodeBlock? {
val loopStatement = startElement.parentOfType<PsiLoopStatement>() ?: return null
val body = loopStatement.body ?: return null
return BlockUtils.expandSingleStatementToBlockStatement(body).parentOfType<PsiBlockStatement>(withSelf = true)?.codeBlock
}
}
internal class InsertCancellationCheckInLoopMethodFix(cancellationCheckCallFqn: String, methodIdentifier: PsiElement)
: AbstractInsertCancellationCheckInLoopFix(cancellationCheckCallFqn, methodIdentifier) {
override fun getCodeBlock(startElement: PsiElement): PsiCodeBlock? {
val callExpression = startElement.parentOfType<PsiMethodCallExpression>() ?: return null
val lambdaExpression = callExpression.argumentList.expressions.filterIsInstance<PsiLambdaExpression>().firstOrNull() ?: return null
return CommonJavaRefactoringUtil.expandExpressionLambdaToCodeBlock(lambdaExpression)
}
}
internal abstract class AbstractInsertCancellationCheckInLoopFix(
private val cancellationCheckCallFqn: String,
anchor: PsiElement,
) : LocalQuickFixOnPsiElement(anchor) {
override fun getFamilyName(): String = DevKitBundle.message("inspection.insert.cancellation.check.fix.message")
override fun getText(): String = familyName
override fun invoke(project: Project, file: PsiFile, startElement: PsiElement, endElement: PsiElement) {
val loopStatement = startElement.parentOfType<PsiLoopStatement>() ?: return
val factory = PsiElementFactory.getInstance(project)
val cancellationCheckText = "${cancellationCheckCallFqn}();"
val cancellationCheckStatement = factory.createStatementFromText(cancellationCheckText, loopStatement)
val body = loopStatement.body ?: return
val bodyBlock = expandSingleStatementToBlockStatement(body).parentOfType<PsiBlockStatement>(withSelf = true)
val insertedElement = bodyBlock?.codeBlock?.addBefore(cancellationCheckStatement, bodyBlock.codeBlock.firstBodyElement)
insertedElement?.let {
val codeBlock = getCodeBlock(startElement) ?: return
val cancellationCheckStatement = createCancellationCheckStatement(project, startElement)
codeBlock.addBefore(cancellationCheckStatement, codeBlock.firstBodyElement)?.also {
JavaCodeStyleManager.getInstance(project).shortenClassReferences(it)
}
}
abstract fun getCodeBlock(startElement: PsiElement): PsiCodeBlock?
private fun createCancellationCheckStatement(project: Project, startElement: PsiElement): PsiStatement {
val psiElementFactory = PsiElementFactory.getInstance(project)
val cancellationCheckText = "${cancellationCheckCallFqn}();"
return psiElementFactory.createStatementFromText(cancellationCheckText, startElement)
}
}

View File

@@ -0,0 +1,79 @@
package inspections.cancellationCheckInLoops;
import com.intellij.util.concurrency.annotations.RequiresReadLock;
import com.intellij.openapi.progress.ProgressManager;
import com.intellij.util.containers.ContainerUtil;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.stream.Stream;
import static inspections.cancellationCheckInLoops.Foo.doSomething;
class Clazz {
@RequiresReadLock
public static void foo(Iterable<String> iterable, Iterator<String> iterator, List<String> list, Map<String, String> map, Stream<String> stream) {
ContainerUtil.<warning descr="Cancellation check 'com.intellij.openapi.progress.ProgressManager.checkCanceled' should be the first statement in a loop body">process</warning>(list, e -> {
doSomething();
return true;
});
iterable.<warning descr="Cancellation check 'com.intellij.openapi.progress.ProgressManager.checkCanceled' should be the first statement in a loop body">forEach</warning>(e -> {
doSomething();
});
iterator.<warning descr="Cancellation check 'com.intellij.openapi.progress.ProgressManager.checkCanceled' should be the first statement in a loop body">forEachRemaining</warning>(e -> {
doSomething();
});
map.<warning descr="Cancellation check 'com.intellij.openapi.progress.ProgressManager.checkCanceled' should be the first statement in a loop body">forEach</warning>((k, v) -> {
doSomething();
});
stream.<warning descr="Cancellation check 'com.intellij.openapi.progress.ProgressManager.checkCanceled' should be the first statement in a loop body">forEach</warning>(e -> {
doSomething();
});
stream.<warning descr="Cancellation check 'com.intellij.openapi.progress.ProgressManager.checkCanceled' should be the first statement in a loop body">forEachOrdered</warning>(e -> {
doSomething();
});
stream.<warning descr="Cancellation check 'com.intellij.openapi.progress.ProgressManager.checkCanceled' should be the first statement in a loop body">forEach</warning>(e -> doSomething());
iterable.<warning descr="Cancellation check 'com.intellij.openapi.progress.ProgressManager.checkCanceled' should be the first statement in a loop body">forEach</warning>(e -> {
// empty block
});
}
// No read lock required
public static void foo2(Iterable<String> iterable, Iterator<String> iterator, List<String> list, Map<String, String> map, Stream<String> stream) {
ContainerUtil.process(list, e -> {
doSomething();
return true;
});
iterable.forEach(e -> {
doSomething();
});
iterator.forEachRemaining(e -> {
doSomething();
});
map.forEach((k, v) -> {
doSomething();
});
stream.forEach(e -> {
doSomething();
});
stream.forEachOrdered(e -> {
doSomething();
});
stream.forEach(e -> doSomething());
iterable.forEach(e -> {
// empty block
});
}
}

View File

@@ -0,0 +1,36 @@
package inspections.cancellationCheckInLoops;
import com.intellij.util.concurrency.annotations.RequiresReadLock;
import com.intellij.util.containers.ContainerUtil;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.stream.Stream;
import static inspections.cancellationCheckInLoops.Foo.doSomething;
class Clazz {
@RequiresReadLock
public static void foo(Iterable<String> iterable, Iterator<String> iterator, List<String> list) {
ContainerUtil.process(list, a -> {
iterable.<warning descr="Cancellation check 'com.intellij.openapi.progress.ProgressManager.checkCanceled' should be the first statement in a loop body">forEach</warning>(b -> {
doSomething();
});
return true;
});
for (String a : iterable) {
iterator.<warning descr="Cancellation check 'com.intellij.openapi.progress.ProgressManager.checkCanceled' should be the first statement in a loop body">forEachRemaining</warning>(b -> {
doSomething();
});
}
iterable.forEach(a -> {
<warning descr="Cancellation check 'com.intellij.openapi.progress.ProgressManager.checkCanceled' should be the first statement in a loop body">for</warning> (String b : iterable) {
doSomething();
}
});
}
}

View File

@@ -0,0 +1,20 @@
package inspections.cancellationCheckInLoops;
import com.intellij.openapi.progress.ProgressManager;
import com.intellij.util.Processor;
import com.intellij.util.concurrency.annotations.RequiresReadLock;
import com.intellij.util.containers.ContainerUtil;
import kotlin.collections.ArraysKt;
import static inspections.cancellationCheckInLoops.Foo.doSomething;
class Clazz {
@RequiresReadLock
public static void foo(String[] array) {
ArraysKt.<warning descr="Cancellation check 'com.intellij.openapi.progress.ProgressManager.checkCanceled' should be the first statement in a loop body">for<caret>Each</warning>(array, e -> {
doSomething();
return null;
});
}
}

View File

@@ -0,0 +1,21 @@
package inspections.cancellationCheckInLoops;
import com.intellij.openapi.progress.ProgressManager;
import com.intellij.util.Processor;
import com.intellij.util.concurrency.annotations.RequiresReadLock;
import com.intellij.util.containers.ContainerUtil;
import kotlin.collections.ArraysKt;
import static inspections.cancellationCheckInLoops.Foo.doSomething;
class Clazz {
@RequiresReadLock
public static void foo(String[] array) {
ArraysKt.forEach(array, e -> {
ProgressManager.checkCanceled();
doSomething();
return null;
});
}
}

View File

@@ -0,0 +1,19 @@
package inspections.cancellationCheckInLoops;
import com.intellij.openapi.progress.ProgressManager;
import com.intellij.util.Processor;
import com.intellij.util.concurrency.annotations.RequiresReadLock;
import com.intellij.util.containers.ContainerUtil;
import static inspections.cancellationCheckInLoops.Foo.doSomething;
class Clazz {
@RequiresReadLock
public static void foo(Iterable<String> list) {
ContainerUtil.<warning descr="Cancellation check 'com.intellij.openapi.progress.ProgressManager.checkCanceled' should be the first statement in a loop body">pro<caret>cess</warning>(list, e -> {
doSomething();
return true;
});
}
}

View File

@@ -0,0 +1,20 @@
package inspections.cancellationCheckInLoops;
import com.intellij.openapi.progress.ProgressManager;
import com.intellij.util.Processor;
import com.intellij.util.concurrency.annotations.RequiresReadLock;
import com.intellij.util.containers.ContainerUtil;
import static inspections.cancellationCheckInLoops.Foo.doSomething;
class Clazz {
@RequiresReadLock
public static void foo(Iterable<String> list) {
ContainerUtil.process(list, e -> {
ProgressManager.checkCanceled();
doSomething();
return true;
});
}
}

View File

@@ -0,0 +1,16 @@
package inspections.cancellationCheckInLoops;
import com.intellij.util.concurrency.annotations.RequiresReadLock;
import java.util.List;
import static inspections.cancellationCheckInLoops.Foo.doSomething;
class Clazz {
@RequiresReadLock
public static void foo(List<String> iterable) {
iterable.<warning descr="Cancellation check 'com.intellij.openapi.progress.ProgressManager.checkCanceled' should be the first statement in a loop body">for<caret>Each</warning>(e -> {
doSomething();
});
}
}

View File

@@ -0,0 +1,18 @@
package inspections.cancellationCheckInLoops;
import com.intellij.openapi.progress.ProgressManager;
import com.intellij.util.concurrency.annotations.RequiresReadLock;
import java.util.List;
import static inspections.cancellationCheckInLoops.Foo.doSomething;
class Clazz {
@RequiresReadLock
public static void foo(List<String> iterable) {
iterable.forEach(e -> {
ProgressManager.checkCanceled();
doSomething();
});
}
}

View File

@@ -0,0 +1,16 @@
package inspections.cancellationCheckInLoops;
import com.intellij.util.concurrency.annotations.RequiresReadLock;
import java.util.Iterator;
import static inspections.cancellationCheckInLoops.Foo.doSomething;
class Clazz {
@RequiresReadLock
public static void foo(Iterator<String> iterator) {
iterator.<warning descr="Cancellation check 'com.intellij.openapi.progress.ProgressManager.checkCanceled' should be the first statement in a loop body">for<caret>EachRemaining</warning>(e -> {
doSomething();
});
}
}

View File

@@ -0,0 +1,18 @@
package inspections.cancellationCheckInLoops;
import com.intellij.openapi.progress.ProgressManager;
import com.intellij.util.concurrency.annotations.RequiresReadLock;
import java.util.Iterator;
import static inspections.cancellationCheckInLoops.Foo.doSomething;
class Clazz {
@RequiresReadLock
public static void foo(Iterator<String> iterator) {
iterator.forEachRemaining(e -> {
ProgressManager.checkCanceled();
doSomething();
});
}
}

View File

@@ -0,0 +1,16 @@
package inspections.cancellationCheckInLoops;
import com.intellij.util.concurrency.annotations.RequiresReadLock;
import java.util.Map;
import static inspections.cancellationCheckInLoops.Foo.doSomething;
class Clazz {
@RequiresReadLock
public static void foo(Map<String, String> map) {
map.<warning descr="Cancellation check 'com.intellij.openapi.progress.ProgressManager.checkCanceled' should be the first statement in a loop body">for<caret>Each</warning>((k, v) -> {
doSomething();
});
}
}

View File

@@ -0,0 +1,18 @@
package inspections.cancellationCheckInLoops;
import com.intellij.openapi.progress.ProgressManager;
import com.intellij.util.concurrency.annotations.RequiresReadLock;
import java.util.Map;
import static inspections.cancellationCheckInLoops.Foo.doSomething;
class Clazz {
@RequiresReadLock
public static void foo(Map<String, String> map) {
map.forEach((k, v) -> {
ProgressManager.checkCanceled();
doSomething();
});
}
}

View File

@@ -0,0 +1,16 @@
package inspections.cancellationCheckInLoops;
import com.intellij.util.concurrency.annotations.RequiresReadLock;
import java.util.stream.Stream;
import static inspections.cancellationCheckInLoops.Foo.doSomething;
class Clazz {
@RequiresReadLock
public static void foo(Stream<String> stream) {
stream.<warning descr="Cancellation check 'com.intellij.openapi.progress.ProgressManager.checkCanceled' should be the first statement in a loop body">for<caret>Each</warning>(e -> {
doSomething();
});
}
}

View File

@@ -0,0 +1,18 @@
package inspections.cancellationCheckInLoops;
import com.intellij.openapi.progress.ProgressManager;
import com.intellij.util.concurrency.annotations.RequiresReadLock;
import java.util.stream.Stream;
import static inspections.cancellationCheckInLoops.Foo.doSomething;
class Clazz {
@RequiresReadLock
public static void foo(Stream<String> stream) {
stream.forEach(e -> {
ProgressManager.checkCanceled();
doSomething();
});
}
}

View File

@@ -0,0 +1,16 @@
package inspections.cancellationCheckInLoops;
import com.intellij.util.concurrency.annotations.RequiresReadLock;
import java.util.stream.Stream;
import static inspections.cancellationCheckInLoops.Foo.doSomething;
class Clazz {
@RequiresReadLock
public static void foo(Stream<String> stream) {
stream.<warning descr="Cancellation check 'com.intellij.openapi.progress.ProgressManager.checkCanceled' should be the first statement in a loop body">for<caret>EachOrdered</warning>(e -> {
doSomething();
});
}
}

View File

@@ -0,0 +1,18 @@
package inspections.cancellationCheckInLoops;
import com.intellij.openapi.progress.ProgressManager;
import com.intellij.util.concurrency.annotations.RequiresReadLock;
import java.util.stream.Stream;
import static inspections.cancellationCheckInLoops.Foo.doSomething;
class Clazz {
@RequiresReadLock
public static void foo(Stream<String> stream) {
stream.forEachOrdered(e -> {
ProgressManager.checkCanceled();
doSomething();
});
}
}

View File

@@ -0,0 +1,15 @@
package inspections.cancellationCheckInLoops;
import com.intellij.util.concurrency.annotations.RequiresReadLock;
import java.util.List;
class Clazz {
@RequiresReadLock
public static void foo(List<String> iterable) {
iterable.<warning descr="Cancellation check 'com.intellij.openapi.progress.ProgressManager.checkCanceled' should be the first statement in a loop body">for<caret>Each</warning>(e -> {
// empty block
});
}
}

View File

@@ -0,0 +1,17 @@
package inspections.cancellationCheckInLoops;
import com.intellij.openapi.progress.ProgressManager;
import com.intellij.util.concurrency.annotations.RequiresReadLock;
import java.util.List;
class Clazz {
@RequiresReadLock
public static void foo(List<String> iterable) {
iterable.forEach(e -> {
ProgressManager.checkCanceled();
// empty block
});
}
}

View File

@@ -0,0 +1,24 @@
package inspections.cancellationCheckInLoops;
import com.intellij.util.concurrency.annotations.RequiresReadLock;
import com.intellij.util.containers.ContainerUtil;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.stream.Stream;
import static inspections.cancellationCheckInLoops.Foo.doSomething;
class Clazz {
@RequiresReadLock
public static void foo(Iterable<String> iterable, List<String> list) {
ContainerUtil.process(list, a -> {
iterable.<warning descr="Cancellation check 'com.intellij.openapi.progress.ProgressManager.checkCanceled' should be the first statement in a loop body">for<caret>Each</warning>(b -> {
doSomething();
});
return true;
});
}
}

View File

@@ -0,0 +1,26 @@
package inspections.cancellationCheckInLoops;
import com.intellij.openapi.progress.ProgressManager;
import com.intellij.util.concurrency.annotations.RequiresReadLock;
import com.intellij.util.containers.ContainerUtil;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.stream.Stream;
import static inspections.cancellationCheckInLoops.Foo.doSomething;
class Clazz {
@RequiresReadLock
public static void foo(Iterable<String> iterable, List<String> list) {
ContainerUtil.process(list, a -> {
iterable.forEach(b -> {
ProgressManager.checkCanceled();
doSomething();
});
return true;
});
}
}

View File

@@ -0,0 +1,14 @@
package inspections.cancellationCheckInLoops;
import com.intellij.util.concurrency.annotations.RequiresReadLock;
import java.util.stream.Stream;
import static inspections.cancellationCheckInLoops.Foo.doSomething;
class Clazz {
@RequiresReadLock
public static void foo(Stream<String> stream) {
stream.<warning descr="Cancellation check 'com.intellij.openapi.progress.ProgressManager.checkCanceled' should be the first statement in a loop body">for<caret>Each</warning>(e -> doSomething());
}
}

View File

@@ -0,0 +1,18 @@
package inspections.cancellationCheckInLoops;
import com.intellij.openapi.progress.ProgressManager;
import com.intellij.util.concurrency.annotations.RequiresReadLock;
import java.util.stream.Stream;
import static inspections.cancellationCheckInLoops.Foo.doSomething;
class Clazz {
@RequiresReadLock
public static void foo(Stream<String> stream) {
stream.forEach(e -> {
ProgressManager.checkCanceled();
doSomething();
});
}
}

View File

@@ -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.inspections
import com.intellij.testFramework.TestDataPath
@@ -47,4 +47,12 @@ class CancellationCheckInLoopsInspectionTest : CancellationCheckInLoopsInspectio
doTest()
}
fun testLoopMethods() {
doTest()
}
fun testNestedLoopMethods() {
doTest()
}
}

View File

@@ -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.inspections.quickfix
import com.intellij.testFramework.TestDataPath
@@ -6,7 +6,6 @@ import org.jetbrains.idea.devkit.DevKitBundle
import org.jetbrains.idea.devkit.DevkitJavaTestsUtil
import org.jetbrains.idea.devkit.inspections.CancellationCheckInLoopsInspectionTestBase
@TestDataPath("\$CONTENT_ROOT/testData/inspections/insertCancellationCheckFix")
class InsertCancellationCheckFixTest : CancellationCheckInLoopsInspectionTestBase() {
@@ -16,6 +15,8 @@ class InsertCancellationCheckFixTest : CancellationCheckInLoopsInspectionTestBas
private val fixName = DevKitBundle.message("inspection.insert.cancellation.check.fix.message")
// loops:
fun testBlockDoWhileLoop() {
doTest(fixName)
}
@@ -31,7 +32,7 @@ class InsertCancellationCheckFixTest : CancellationCheckInLoopsInspectionTestBas
fun testBlockWhileLoop() {
doTest(fixName)
}
fun testEmptyDoWhileLoop() {
doTest(fixName)
}
@@ -76,4 +77,48 @@ class InsertCancellationCheckFixTest : CancellationCheckInLoopsInspectionTestBas
doTest(fixName)
}
// loop methods:
fun testBlockIterableForEachMethod() {
doTest(fixName)
}
fun testBlockIteratorForEachRemainingMethod() {
doTest(fixName)
}
fun testBlockStreamForEachMethod() {
doTest(fixName)
}
fun testBlockStreamForEachOrderedMethod() {
doTest(fixName)
}
fun testBlockMapForEachMethod() {
doTest(fixName)
}
fun testEmptyIterableForEachMethod() {
doTest(fixName)
}
fun testSingleStreamForEachMethod() {
doTest(fixName)
}
fun testBlockContainerUtilProcessMethod() {
doTest(fixName)
}
fun testNestedLoopMethods() {
doTest(fixName)
}
// Kotlin methods:
fun testBlockArraysKtForEachMethod() {
doTest(fixName)
}
}

View File

@@ -0,0 +1,100 @@
package org.jetbrains.idea.devkit.inspections
import com.intellij.openapi.progress.ProgressManager
import com.intellij.util.concurrency.annotations.RequiresReadLock
import com.intellij.util.containers.ContainerUtil
import inspections.cancellationCheckInLoops.Foo.doSomething
import java.util.function.Consumer
@Suppress("UNUSED_ANONYMOUS_PARAMETER")
class Testing {
@Suppress("UNUSED_PARAMETER")
private fun withSuspendLambda(l: suspend () -> Any) { }
@RequiresReadLock
private fun foo1(array: Array<String>, list: List<String>, map: Map<String, String>, iterator: Iterator<String>, sequence: Sequence<String>) {
array.<warning descr="Cancellation check 'com.intellij.openapi.progress.ProgressManager.checkCanceled' should be the first statement in a loop body">forEach</warning> { doSomething() }
array.<warning descr="Cancellation check 'com.intellij.openapi.progress.ProgressManager.checkCanceled' should be the first statement in a loop body">forEachIndexed</warning> { index, s -> doSomething() }
list.<warning descr="Cancellation check 'com.intellij.openapi.progress.ProgressManager.checkCanceled' should be the first statement in a loop body">forEach</warning> { doSomething() }
list.<warning descr="Cancellation check 'com.intellij.openapi.progress.ProgressManager.checkCanceled' should be the first statement in a loop body">forEachIndexed</warning> { index, s -> doSomething() }
sequence.<warning descr="Cancellation check 'com.intellij.openapi.progress.ProgressManager.checkCanceled' should be the first statement in a loop body">forEach</warning> { doSomething() }
sequence.<warning descr="Cancellation check 'com.intellij.openapi.progress.ProgressManager.checkCanceled' should be the first statement in a loop body">forEachIndexed</warning> { index, s -> doSomething() }
map.<warning descr="Cancellation check 'com.intellij.openapi.progress.ProgressManager.checkCanceled' should be the first statement in a loop body">forEach</warning> { e -> doSomething() }
map.<warning descr="Cancellation check 'com.intellij.openapi.progress.ProgressManager.checkCanceled' should be the first statement in a loop body">forEach</warning> { k, v -> doSomething() }
iterator.<warning descr="Cancellation check 'com.intellij.openapi.progress.ProgressManager.checkCanceled' should be the first statement in a loop body">forEach</warning> { doSomething() }
iterator.<warning descr="Cancellation check 'com.intellij.openapi.progress.ProgressManager.checkCanceled' should be the first statement in a loop body">forEachRemaining</warning> { doSomething() }
ContainerUtil.<warning descr="Cancellation check 'com.intellij.openapi.progress.ProgressManager.checkCanceled' should be the first statement in a loop body">process</warning>(list) {
true
}
}
@RequiresReadLock
fun foo2(array: Array<String>, list: List<String>, map: Map<String, String>, iterator: Iterator<String>, sequence: Sequence<String>) {
withSuspendLambda {
array.<warning descr="Cancellation check 'com.intellij.openapi.progress.checkCancelled' should be the first statement in a loop body">forEach</warning> { doSomething() }
array.<warning descr="Cancellation check 'com.intellij.openapi.progress.checkCancelled' should be the first statement in a loop body">forEachIndexed</warning> { index, s -> doSomething() }
list.<warning descr="Cancellation check 'com.intellij.openapi.progress.checkCancelled' should be the first statement in a loop body">forEach</warning> { doSomething() }
list.<warning descr="Cancellation check 'com.intellij.openapi.progress.checkCancelled' should be the first statement in a loop body">forEachIndexed</warning> { index, s -> doSomething() }
sequence.<warning descr="Cancellation check 'com.intellij.openapi.progress.checkCancelled' should be the first statement in a loop body">forEach</warning> { doSomething() }
sequence.<warning descr="Cancellation check 'com.intellij.openapi.progress.checkCancelled' should be the first statement in a loop body">forEachIndexed</warning> { index, s -> doSomething() }
map.<warning descr="Cancellation check 'com.intellij.openapi.progress.checkCancelled' should be the first statement in a loop body">forEach</warning> { e -> doSomething() }
map.<warning descr="Cancellation check 'com.intellij.openapi.progress.checkCancelled' should be the first statement in a loop body">forEach</warning> { k, v -> doSomething() }
iterator.<warning descr="Cancellation check 'com.intellij.openapi.progress.checkCancelled' should be the first statement in a loop body">forEach</warning> { doSomething() }
iterator.<warning descr="Cancellation check 'com.intellij.openapi.progress.checkCancelled' should be the first statement in a loop body">forEachRemaining</warning> { doSomething() }
ContainerUtil.<warning descr="Cancellation check 'com.intellij.openapi.progress.checkCancelled' should be the first statement in a loop body">process</warning>(list) {
true
}
}
}
@RequiresReadLock
suspend fun foo3(array: Array<String>, list: List<String>, map: Map<String, String>, iterator: Iterator<String>, sequence: Sequence<String>) {
array.<warning descr="Cancellation check 'com.intellij.openapi.progress.checkCancelled' should be the first statement in a loop body">forEach</warning> { doSomething() }
array.<warning descr="Cancellation check 'com.intellij.openapi.progress.checkCancelled' should be the first statement in a loop body">forEachIndexed</warning> { index, s -> doSomething() }
list.<warning descr="Cancellation check 'com.intellij.openapi.progress.checkCancelled' should be the first statement in a loop body">forEach</warning> { doSomething() }
list.<warning descr="Cancellation check 'com.intellij.openapi.progress.checkCancelled' should be the first statement in a loop body">forEachIndexed</warning> { index, s -> doSomething() }
sequence.<warning descr="Cancellation check 'com.intellij.openapi.progress.checkCancelled' should be the first statement in a loop body">forEach</warning> { doSomething() }
sequence.<warning descr="Cancellation check 'com.intellij.openapi.progress.checkCancelled' should be the first statement in a loop body">forEachIndexed</warning> { index, s -> doSomething() }
map.<warning descr="Cancellation check 'com.intellij.openapi.progress.checkCancelled' should be the first statement in a loop body">forEach</warning> { e -> doSomething() }
map.<warning descr="Cancellation check 'com.intellij.openapi.progress.checkCancelled' should be the first statement in a loop body">forEach</warning> { k, v -> doSomething() }
iterator.<warning descr="Cancellation check 'com.intellij.openapi.progress.checkCancelled' should be the first statement in a loop body">forEach</warning> { doSomething() }
iterator.<warning descr="Cancellation check 'com.intellij.openapi.progress.checkCancelled' should be the first statement in a loop body">forEachRemaining</warning> { doSomething() }
ContainerUtil.<warning descr="Cancellation check 'com.intellij.openapi.progress.checkCancelled' should be the first statement in a loop body">process</warning>(list) {
true
}
}
// no @RequiresReadLock
private fun foo4(array: Array<String>) {
array.forEach { doSomething() }
}
// no @RequiresReadLock
fun foo5(list: List<String>) {
withSuspendLambda {
list.forEach { doSomething() }
}
}
// no @RequiresReadLock
suspend fun foo6(sequence: Sequence<String>) {
sequence.forEachIndexed { index, s -> doSomething() }
}
}

View File

@@ -0,0 +1,66 @@
package org.jetbrains.idea.devkit.inspections
import com.intellij.openapi.progress.ProgressManager
import com.intellij.util.concurrency.annotations.RequiresReadLock
import com.intellij.util.containers.ContainerUtil
import inspections.cancellationCheckInLoops.Foo.doSomething
import java.util.function.Consumer
@Suppress("UNUSED_PARAMETER", "UNUSED_ANONYMOUS_PARAMETER")
class Testing {
@Suppress("UNUSED_PARAMETER")
private fun withSuspendLambda(l: suspend () -> Any) { }
@RequiresReadLock
private fun foo1(array: Array<String>, list: List<String>, map: Map<String, String>, iterator: Iterator<String>, sequence: Sequence<String>) {
array.forEach {
list.<warning descr="Cancellation check 'com.intellij.openapi.progress.ProgressManager.checkCanceled' should be the first statement in a loop body">forEach</warning> { doSomething() }
}
for (i in 1..10) {
list.<warning descr="Cancellation check 'com.intellij.openapi.progress.ProgressManager.checkCanceled' should be the first statement in a loop body">forEachIndexed</warning> { index, s -> doSomething() }
}
sequence.forEach {
<warning descr="Cancellation check 'com.intellij.openapi.progress.ProgressManager.checkCanceled' should be the first statement in a loop body">for</warning> (i in 1..10) {
doSomething()
}
}
}
@RequiresReadLock
fun foo2(array: Array<String>, list: List<String>, map: Map<String, String>, iterator: Iterator<String>, sequence: Sequence<String>) {
withSuspendLambda {
array.forEach {
list.<warning descr="Cancellation check 'com.intellij.openapi.progress.checkCancelled' should be the first statement in a loop body">forEach</warning> { doSomething() }
}
for (i in 1..10) {
list.<warning descr="Cancellation check 'com.intellij.openapi.progress.checkCancelled' should be the first statement in a loop body">forEachIndexed</warning> { index, s -> doSomething() }
}
sequence.forEach {
<warning descr="Cancellation check 'com.intellij.openapi.progress.checkCancelled' should be the first statement in a loop body">for</warning> (i in 1..10) {
doSomething()
}
}
}
}
@RequiresReadLock
suspend fun foo3(array: Array<String>, list: List<String>, map: Map<String, String>, iterator: Iterator<String>, sequence: Sequence<String>) {
array.forEach {
list.<warning descr="Cancellation check 'com.intellij.openapi.progress.checkCancelled' should be the first statement in a loop body">forEach</warning> { doSomething() }
}
for (i in 1..10) {
list.<warning descr="Cancellation check 'com.intellij.openapi.progress.checkCancelled' should be the first statement in a loop body">forEachIndexed</warning> { index, s -> doSomething() }
}
sequence.forEach {
<warning descr="Cancellation check 'com.intellij.openapi.progress.checkCancelled' should be the first statement in a loop body">for</warning> (i in 1..10) {
doSomething()
}
}
}
}

View File

@@ -0,0 +1,11 @@
package inspections.insertCancellationCheckFix
import com.intellij.util.concurrency.annotations.RequiresReadLock
import inspections.cancellationCheckInLoops.Foo.doSomething
@RequiresReadLock
fun main(list: Iterable<String>) {
list.<warning descr="Cancellation check 'com.intellij.openapi.progress.ProgressManager.checkCanceled' should be the first statement in a loop body">for<caret>Each</warning> {
doSomething()
}
}

View File

@@ -0,0 +1,13 @@
package inspections.insertCancellationCheckFix
import com.intellij.openapi.progress.ProgressManager
import com.intellij.util.concurrency.annotations.RequiresReadLock
import inspections.cancellationCheckInLoops.Foo.doSomething
@RequiresReadLock
fun main(list: Iterable<String>) {
list.forEach {
ProgressManager.checkCanceled()
doSomething()
}
}

View File

@@ -0,0 +1,9 @@
package inspections.insertCancellationCheckFix
import com.intellij.util.concurrency.annotations.RequiresReadLock
import inspections.cancellationCheckInLoops.Foo.doSomething
@RequiresReadLock
fun main(iterator: Iterator<String>) {
iterator.<warning descr="Cancellation check 'com.intellij.openapi.progress.ProgressManager.checkCanceled' should be the first statement in a loop body">for<caret>EachRemaining</warning> {}
}

View File

@@ -0,0 +1,12 @@
package inspections.insertCancellationCheckFix
import com.intellij.openapi.progress.ProgressManager
import com.intellij.util.concurrency.annotations.RequiresReadLock
import inspections.cancellationCheckInLoops.Foo.doSomething
@RequiresReadLock
fun main(iterator: Iterator<String>) {
iterator.forEachRemaining {
ProgressManager.checkCanceled()
}
}

View File

@@ -0,0 +1,19 @@
package inspections.insertCancellationCheckFix
import com.intellij.util.concurrency.annotations.RequiresReadLock
import inspections.cancellationCheckInLoops.Foo.doSomething
@Suppress("UNUSED_PARAMETER")
fun withSuspendLambda(l: suspend () -> Any) { }
@RequiresReadLock
@Suppress("UNUSED_ANONYMOUS_PARAMETER")
fun test(map: Map<String, String>) {
withSuspendLambda {
map.<warning descr="Cancellation check 'com.intellij.openapi.progress.checkCancelled' should be the first statement in a loop body">for<caret>Each</warning> { k, v ->
// comments
doSomething()
}
}
}

View File

@@ -0,0 +1,21 @@
package inspections.insertCancellationCheckFix
import com.intellij.openapi.progress.checkCancelled
import com.intellij.util.concurrency.annotations.RequiresReadLock
import inspections.cancellationCheckInLoops.Foo.doSomething
@Suppress("UNUSED_PARAMETER")
fun withSuspendLambda(l: suspend () -> Any) { }
@RequiresReadLock
@Suppress("UNUSED_ANONYMOUS_PARAMETER")
fun test(map: Map<String, String>) {
withSuspendLambda {
map.forEach { k, v ->
checkCancelled()
// comments
doSomething()
}
}
}

View File

@@ -1,11 +1,10 @@
// 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
import org.jetbrains.idea.devkit.inspections.CancellationCheckInLoopsInspectionTestBase
import org.jetbrains.idea.devkit.kotlin.DevkitKtTestsUtil
@TestDataPath("\$CONTENT_ROOT/testData/inspections/cancellationCheckInLoops")
class KtCancellationCheckInLoopsInspectionTest : CancellationCheckInLoopsInspectionTestBase() {
@@ -61,4 +60,12 @@ class KtCancellationCheckInLoopsInspectionTest : CancellationCheckInLoopsInspect
doTest()
}
fun testLoopMethods() {
doTest()
}
fun testNestedLoopMethods() {
doTest()
}
}

View File

@@ -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.quickfix
import com.intellij.testFramework.TestDataPath
@@ -6,7 +6,6 @@ import org.jetbrains.idea.devkit.DevKitBundle
import org.jetbrains.idea.devkit.inspections.CancellationCheckInLoopsInspectionTestBase
import org.jetbrains.idea.devkit.kotlin.DevkitKtTestsUtil
@TestDataPath("\$CONTENT_ROOT/testData/inspections/insertCancellationCheckFix")
class KtInsertCancellationCheckFixTest : CancellationCheckInLoopsInspectionTestBase() {
@@ -27,7 +26,9 @@ class KtInsertCancellationCheckFixTest : CancellationCheckInLoopsInspectionTestB
}
private val fixName = DevKitBundle.message("inspection.insert.cancellation.check.fix.message")
// loops:
fun testBlockDoWhileLoop() {
doTest(fixName)
}
@@ -88,4 +89,18 @@ class KtInsertCancellationCheckFixTest : CancellationCheckInLoopsInspectionTestB
doTest(fixName)
}
// loop methods:
fun testBlockIterableForEachMethod() {
doTest(fixName)
}
fun testEmptyIteratorForEachRemainingMethod() {
doTest(fixName)
}
fun testSuspendingMapForMethod() {
doTest(fixName)
}
}

View File

@@ -1,17 +1,31 @@
// 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.inspections
import com.intellij.openapi.module.Module
import com.intellij.openapi.roots.ContentEntry
import com.intellij.openapi.roots.ModifiableRootModel
import com.intellij.pom.java.LanguageLevel
import com.intellij.testFramework.fixtures.kotlin.KotlinTester
import org.jetbrains.idea.devkit.inspections.quickfix.LightDevKitInspectionFixTestBase
abstract class CancellationCheckInLoopsInspectionTestBase : LightDevKitInspectionFixTestBase() {
private val projectDescriptor = object : ProjectDescriptor(LanguageLevel.HIGHEST) {
override fun configureModule(module: Module, model: ModifiableRootModel, contentEntry: ContentEntry) {
super.configureModule(module, model, contentEntry)
KotlinTester.configureKotlinStdLib(model)
}
}
override fun getProjectDescriptor() = projectDescriptor
override fun setUp() {
super.setUp()
myFixture.addClass("""
package com.intellij.util.concurrency.annotations;
public @interface RequiresReadLock { }
""".trimIndent()
""".trimIndent()
)
myFixture.addClass("""
@@ -20,7 +34,7 @@ abstract class CancellationCheckInLoopsInspectionTestBase : LightDevKitInspectio
public final class Foo {
public static void doSomething() { }
}
""".trimIndent()
""".trimIndent()
)
myFixture.addClass("""
@@ -29,7 +43,31 @@ abstract class CancellationCheckInLoopsInspectionTestBase : LightDevKitInspectio
public abstract class ProgressManager {
public static void checkCanceled() { }
}
""".trimIndent()
""".trimIndent()
)
myFixture.addClass("""
package com.intellij.util;
@FunctionalInterface
public interface Processor<T> {
boolean process(T t);
}
""".trimIndent()
)
myFixture.addClass("""
package com.intellij.util.containers;
import com.intellij.util.Processor;
public final class ContainerUtil {
public static <T> boolean process(Iterable<? extends T> iterable, Processor<? super T> processor) {return true;}
public static <T> boolean process(List<? extends T> list, Processor<? super T> processor) {return true;}
public static <T> boolean process(T [] iterable, Processor<? super T> processor) {return true;}
public static <T> boolean process(Iterator<? extends T> iterator, Processor<? super T> processor) {return true;}
}
""".trimIndent()
)
myFixture.enableInspections(CancellationCheckInLoopsInspection())

View File

@@ -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.codeInspection.LocalQuickFix
@@ -16,44 +16,34 @@ import org.jetbrains.kotlin.idea.codeInsight.intentions.shared.AddBracesIntentio
import org.jetbrains.kotlin.idea.codeinsight.utils.isRedundantSemicolon
import org.jetbrains.kotlin.lexer.KtTokens
import org.jetbrains.kotlin.psi.KtBlockExpression
import org.jetbrains.kotlin.psi.KtCallExpression
import org.jetbrains.kotlin.psi.KtExpression
import org.jetbrains.kotlin.psi.KtLambdaExpression
import org.jetbrains.kotlin.psi.KtLoopExpression
import org.jetbrains.kotlin.psi.KtPsiFactory
import org.jetbrains.kotlin.psi.KtPsiUtil.findChildByType
internal class KtCancellationCheckInLoopsFixProvider : CancellationCheckInLoopsFixProvider {
override fun getFixes(loopKeyword: PsiElement, cancellationCheckFqn: String): List<LocalQuickFix> {
return listOf(KtInsertCancellationCheckFix(cancellationCheckFqn, loopKeyword))
override fun getFixes(anchor: PsiElement, cancellationCheckFqn: String): List<LocalQuickFix> {
return when {
anchor.node.elementType == KtTokens.IDENTIFIER -> listOf(KtInsertCancellationCheckInLoopMethodFix(cancellationCheckFqn, anchor))
else -> listOf(KtInsertCancellationCheckInLoopFix(cancellationCheckFqn, anchor))
}
}
}
internal class KtInsertCancellationCheckFix(
private val cancellationCheckCallFqn: String,
loopKeyword: PsiElement,
) : LocalQuickFixOnPsiElement(loopKeyword) {
override fun getFamilyName(): String = DevKitBundle.message("inspection.insert.cancellation.check.fix.message")
override fun getText(): String = familyName
override fun isAvailable(project: Project, file: PsiFile, startElement: PsiElement, endElement: PsiElement): Boolean {
return startElement.parentOfType<KtLoopExpression>() != null
internal class KtInsertCancellationCheckInLoopFix(cancellationCheckCallFqn: String, loopKeyword: PsiElement)
: AbstractKtInsertCancellationCheckFix(cancellationCheckCallFqn, loopKeyword) {
override fun getBlockExpression(startElement: PsiElement): KtBlockExpression? {
val loopStatement = startElement.parentOfType<KtLoopExpression>() ?: return null
return loopStatement.getOrCreateBodyBlock(startElement.project)
}
override fun invoke(project: Project, file: PsiFile, startElement: PsiElement, endElement: PsiElement) {
val loopStatement = startElement.parentOfType<KtLoopExpression>() ?: return
val factory = KtPsiFactory(project)
val cancellationCheckExpression = factory.createExpression("${cancellationCheckCallFqn}()")
val bodyBlock = loopStatement.getOrCreateBodyBlock(project)
bodyBlock?.addExpressionToFirstLine(cancellationCheckExpression)?.let {
ShortenReferencesFacility.getInstance().shorten(it)
}
override fun addExpression(bodyBlock: KtBlockExpression, cancellationCheckExpression: KtExpression): KtExpression? {
val project = bodyBlock.project
CodeStyleManager.getInstance(project).reformat(bodyBlock)
return bodyBlock.addAfter(cancellationCheckExpression, bodyBlock.lBrace) as? KtExpression
}
private fun KtLoopExpression.getOrCreateBodyBlock(project: Project): KtBlockExpression? {
@@ -82,10 +72,47 @@ internal class KtInsertCancellationCheckFix(
}
}
private fun KtBlockExpression.addExpressionToFirstLine(expression: KtExpression): KtExpression? {
// otherwise the code might become incorrect in case of poor formatting before inserting an expression (e.g. missing new lines)
CodeStyleManager.getInstance(project).reformat(this)
return addAfter(expression, lBrace) as? KtExpression
override fun isAvailable(project: Project, file: PsiFile, startElement: PsiElement, endElement: PsiElement): Boolean {
return startElement.parentOfType<KtLoopExpression>() != null
}
}
internal class KtInsertCancellationCheckInLoopMethodFix(cancellationCheckCallFqn: String, loopKeyword: PsiElement)
: AbstractKtInsertCancellationCheckFix(cancellationCheckCallFqn, loopKeyword) {
override fun getBlockExpression(startElement: PsiElement): KtBlockExpression? {
return startElement.parentOfType<KtCallExpression>()?.lambdaArguments?.firstOrNull()?.getLambdaExpression()?.bodyExpression
}
override fun addExpression(bodyBlock: KtBlockExpression, cancellationCheckExpression: KtExpression): KtExpression? {
val project = bodyBlock.project
val insertedExpression = bodyBlock.addAfter(cancellationCheckExpression, bodyBlock.lBrace) as? KtExpression
insertedExpression?.parent?.addAfter(KtPsiFactory(project).createNewLine(), insertedExpression)
bodyBlock.parentOfType<KtLambdaExpression>()?.let {
CodeStyleManager.getInstance(project).reformat(it)
}
return insertedExpression
}
}
internal abstract class AbstractKtInsertCancellationCheckFix(
private val cancellationCheckCallFqn: String,
loopKeyword: PsiElement,
) : LocalQuickFixOnPsiElement(loopKeyword) {
override fun getFamilyName(): String = DevKitBundle.message("inspection.insert.cancellation.check.fix.message")
override fun getText(): String = familyName
override fun invoke(project: Project, file: PsiFile, startElement: PsiElement, endElement: PsiElement) {
val bodyBlock = getBlockExpression(startElement) ?: return
val cancellationCheckExpression = KtPsiFactory(project).createExpression("${cancellationCheckCallFqn}()")
addExpression(bodyBlock, cancellationCheckExpression)?.let {
ShortenReferencesFacility.getInstance().shorten(it)
}
}
abstract fun getBlockExpression(startElement: PsiElement): KtBlockExpression?
abstract fun addExpression(bodyBlock: KtBlockExpression, cancellationCheckExpression: KtExpression): KtExpression?
}

View File

@@ -1,41 +1,47 @@
// 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.util
import com.intellij.psi.PsiElement
import org.jetbrains.idea.devkit.kotlin.util.ExecutionContextType.*
import org.jetbrains.kotlin.analysis.api.analyze
import org.jetbrains.kotlin.analysis.api.calls.singleFunctionCallOrNull
import org.jetbrains.kotlin.lexer.KtTokens
import org.jetbrains.kotlin.psi.*
import org.jetbrains.kotlin.psi.psiUtil.getParentOfType
import org.jetbrains.kotlin.psi.psiUtil.getParentOfTypes
import org.jetbrains.kotlin.psi.psiUtil.getParentOfTypesAndPredicate
import org.jetbrains.kotlin.psi.psiUtil.getStrictParentOfType
internal fun getContext(element: PsiElement): ExecutionContextType {
val containingLambda = element.getParentOfType<KtLambdaExpression>(false)
val suspendingLambda = element.findParentSuspendingLambda()
if (suspendingLambda != null) {
return SUSPENDING_LAMBDA
}
// if lambda is null, check the containing method
if (containingLambda == null) {
val containingMethod = element.getParentOfType<KtNamedFunction>(false)
return if (containingMethod.hasSuspendModifier()) ExecutionContextType.SUSPENDING_FUNCTION else ExecutionContextType.BLOCKING
}
val containingMethod = element.getParentOfType<KtNamedFunction>(false)
return if (containingMethod.hasSuspendModifier()) SUSPENDING_FUNCTION else BLOCKING
}
// otherwise, check containing argument (whether the corresponding parameter has `suspend` modifier)
val containingArgument = containingLambda.getParentOfType<KtValueArgument>(true, KtCallableDeclaration::class.java)
if (containingArgument != null) {
val callExpression = containingArgument.getStrictParentOfType<KtCallExpression>() ?: return ExecutionContextType.BLOCKING
analyze(callExpression) {
val functionCall = callExpression.resolveCall()?.singleFunctionCallOrNull() ?: return ExecutionContextType.BLOCKING
val lambdaArgumentType = functionCall.argumentMapping[containingLambda]?.returnType ?: return ExecutionContextType.BLOCKING
return if (lambdaArgumentType.isSuspendFunctionType) ExecutionContextType.SUSPENDING_LAMBDA else ExecutionContextType.BLOCKING
private fun PsiElement.findParentSuspendingLambda(): KtLambdaExpression? {
return this.getParentOfTypesAndPredicate(false, KtLambdaExpression::class.java) {
// check containing argument (whether the corresponding parameter has `suspend` modifier)
val containingArgument = it.getParentOfType<KtValueArgument>(true, KtCallableDeclaration::class.java)
if (containingArgument != null) {
val callExpression = containingArgument.getStrictParentOfType<KtCallExpression>() ?: return@getParentOfTypesAndPredicate false
analyze(callExpression) {
val functionCall = callExpression.resolveCall()?.singleFunctionCallOrNull() ?: return@getParentOfTypesAndPredicate false
val lambdaArgumentType = functionCall.argumentMapping[it]?.returnType ?: return@getParentOfTypesAndPredicate false
return@getParentOfTypesAndPredicate lambdaArgumentType.isSuspendFunctionType
}
}
}
// otherwise, check if it's a property or a function
val containingPropertyOrFunction: KtCallableDeclaration? = containingLambda.getParentOfTypes(true, KtProperty::class.java,
KtNamedFunction::class.java)
if (containingPropertyOrFunction?.typeReference.hasSuspendModifier()) return ExecutionContextType.SUSPENDING_LAMBDA
return if (containingPropertyOrFunction.hasSuspendModifier()) ExecutionContextType.SUSPENDING_LAMBDA else ExecutionContextType.BLOCKING
// otherwise, check if it's a property or a function
val containingPropertyOrFunction: KtCallableDeclaration? = it.getParentOfTypes(true, KtProperty::class.java,
KtNamedFunction::class.java)
if (containingPropertyOrFunction?.typeReference.hasSuspendModifier()) return@getParentOfTypesAndPredicate true
return@getParentOfTypesAndPredicate containingPropertyOrFunction.hasSuspendModifier()
}
}
private fun KtModifierListOwner?.hasSuspendModifier(): Boolean {