From e6a1b96c53e838e46afc219a79c1dd21b06dac5f Mon Sep 17 00:00:00 2001 From: Mikhail Glukhikh Date: Wed, 28 Nov 2018 16:25:59 +0300 Subject: [PATCH] Redundant async inspection: support case with GlobalScope Partial implementation of KT-28504 --- .../collections/SimplifyCallChainFix.kt | 21 +++++++++--------- .../coroutines/RedundantAsyncInspection.kt | 22 ++++++++++++++++--- .../coroutines/redundantAsync/globalScope.kt | 2 -- .../redundantAsync/globalScope.kt.after | 7 ++++++ .../redundantAsync/globalScopeNoContext.kt | 7 ++++++ .../globalScopeNoContext.kt.after | 7 ++++++ .../globalScopeNoContextNoPackage.kt | 8 +++++++ .../globalScopeNoContextNoPackage.kt.after | 10 +++++++++ .../LocalInspectionTestGenerated.java | 10 +++++++++ 9 files changed, 79 insertions(+), 15 deletions(-) create mode 100644 idea/testData/inspectionsLocal/coroutines/redundantAsync/globalScope.kt.after create mode 100644 idea/testData/inspectionsLocal/coroutines/redundantAsync/globalScopeNoContext.kt create mode 100644 idea/testData/inspectionsLocal/coroutines/redundantAsync/globalScopeNoContext.kt.after create mode 100644 idea/testData/inspectionsLocal/coroutines/redundantAsync/globalScopeNoContextNoPackage.kt create mode 100644 idea/testData/inspectionsLocal/coroutines/redundantAsync/globalScopeNoContextNoPackage.kt.after diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/collections/SimplifyCallChainFix.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/collections/SimplifyCallChainFix.kt index d9ce2c4ad5c..bdf25b3c628 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/collections/SimplifyCallChainFix.kt +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/collections/SimplifyCallChainFix.kt @@ -25,7 +25,7 @@ import org.jetbrains.kotlin.name.Name import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.psi.psiUtil.PsiChildRange -class SimplifyCallChainFix(private val newCallText: String) : LocalQuickFix { +class SimplifyCallChainFix(private val newCallText: String, private val removeReceiverOfFirstCall: Boolean = false) : LocalQuickFix { private val shortenedText = newCallText.split("(").joinToString(separator = "(") { it.substringAfterLast(".") } @@ -34,20 +34,21 @@ class SimplifyCallChainFix(private val newCallText: String) : LocalQuickFix { override fun getFamilyName() = name - private fun apply(secondQualifiedExpression: KtQualifiedExpression) { - val factory = KtPsiFactory(secondQualifiedExpression) - val firstExpression = secondQualifiedExpression.receiverExpression + fun apply(qualifiedExpression: KtQualifiedExpression) { + val factory = KtPsiFactory(qualifiedExpression) + val firstExpression = qualifiedExpression.receiverExpression - val operationSign = when (firstExpression) { + val operationSign = if (removeReceiverOfFirstCall) "" else when (firstExpression) { is KtSafeQualifiedExpression -> "?." is KtQualifiedExpression -> "." else -> "" } - val receiverExpression = (firstExpression as? KtQualifiedExpression)?.receiverExpression + val receiverExpressionOrEmptyString: Any = + if (!removeReceiverOfFirstCall && firstExpression is KtQualifiedExpression) firstExpression.receiverExpression else "" val firstCallExpression = AbstractCallChainChecker.getCallExpression(firstExpression) ?: return - val secondCallExpression = secondQualifiedExpression.selectorExpression as? KtCallExpression ?: return + val secondCallExpression = qualifiedExpression.selectorExpression as? KtCallExpression ?: return val lastArgumentName = if (newCallText.startsWith("joinTo")) Name.identifier("transform") else null if (lastArgumentName != null) { @@ -82,7 +83,7 @@ class SimplifyCallChainFix(private val newCallText: String) : LocalQuickFix { val newQualifiedExpression = if (lambdaExpression != null) factory.createExpressionByPattern( "$0$1$2 $3 $4", - receiverExpression ?: "", + receiverExpressionOrEmptyString, operationSign, newCallText, argumentsText, @@ -90,13 +91,13 @@ class SimplifyCallChainFix(private val newCallText: String) : LocalQuickFix { ) else factory.createExpressionByPattern( "$0$1$2 $3", - receiverExpression ?: "", + receiverExpressionOrEmptyString, operationSign, newCallText, argumentsText ) - val result = secondQualifiedExpression.replaced(newQualifiedExpression) + val result = qualifiedExpression.replaced(newQualifiedExpression) ShortenReferences.DEFAULT.process(result) } diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/coroutines/RedundantAsyncInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/coroutines/RedundantAsyncInspection.kt index 60ccccee38d..d4436bff1af 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/coroutines/RedundantAsyncInspection.kt +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/coroutines/RedundantAsyncInspection.kt @@ -7,20 +7,23 @@ package org.jetbrains.kotlin.idea.inspections.coroutines import com.intellij.codeInspection.ProblemHighlightType import com.intellij.codeInspection.ProblemsHolder +import org.jetbrains.kotlin.idea.caches.resolve.analyze import org.jetbrains.kotlin.idea.inspections.collections.AbstractCallChainChecker import org.jetbrains.kotlin.idea.inspections.collections.SimplifyCallChainFix +import org.jetbrains.kotlin.psi.KtNameReferenceExpression import org.jetbrains.kotlin.psi.KtQualifiedExpression import org.jetbrains.kotlin.psi.psiUtil.startOffset import org.jetbrains.kotlin.psi.qualifiedExpressionVisitor +import org.jetbrains.kotlin.resolve.BindingContext import org.jetbrains.kotlin.resolve.calls.model.DefaultValueArgument +import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameSafe +import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode class RedundantAsyncInspection : AbstractCallChainChecker() { private fun generateConversion(expression: KtQualifiedExpression): Conversion? { var defaultContext: Boolean? = null var defaultStart: Boolean? = null - // Temporary forbid cases with explicit scope - if (expression.receiverExpression is KtQualifiedExpression) return null var conversion = findQualifiedConversion(expression, conversionGroups) check@{ _, firstResolvedCall, _, _ -> for ((parameterDescriptor, valueArgument) in firstResolvedCall.valueArguments) { @@ -36,6 +39,17 @@ class RedundantAsyncInspection : AbstractCallChainChecker() { defaultStart ?: return null if (defaultContext!! && !defaultStart!!) return null + val receiverExpression = expression.receiverExpression + val scopeExpression = (receiverExpression as? KtQualifiedExpression)?.receiverExpression + if (scopeExpression != null) { + val context = scopeExpression.analyze(BodyResolveMode.PARTIAL) + val scopeDescriptor = (scopeExpression as? KtNameReferenceExpression)?.let { context[BindingContext.REFERENCE_TARGET, it] } + if (scopeDescriptor?.fqNameSafe?.toString() != globalScope) { + // Temporary forbid cases with explicit non-global scope + return null + } + } + if (defaultContext!! && defaultStart!!) { conversion = conversion.withArgumentList( if (conversion === conversions[0]) { @@ -58,7 +72,7 @@ class RedundantAsyncInspection : AbstractCallChainChecker() { "Redundant 'async' call may be reduced to '$fullReplacement'", ProblemHighlightType.GENERIC_ERROR_OR_WARNING, isOnTheFly, - SimplifyCallChainFix(fullReplacement) + SimplifyCallChainFix(fullReplacement, removeReceiverOfFirstCall = true) ) holder.registerProblem(descriptor) }) @@ -79,6 +93,8 @@ class RedundantAsyncInspection : AbstractCallChainChecker() { ) ) + private const val globalScope = "kotlinx.coroutines.GlobalScope" + private const val defaultAsyncArgument = "$COROUTINE_PACKAGE.Dispatchers.Default" private const val defaultAsyncArgumentExperimental = "$COROUTINE_EXPERIMENTAL_PACKAGE.DefaultDispatcher" diff --git a/idea/testData/inspectionsLocal/coroutines/redundantAsync/globalScope.kt b/idea/testData/inspectionsLocal/coroutines/redundantAsync/globalScope.kt index 7020fd8a82f..0e3cc0e3d6e 100644 --- a/idea/testData/inspectionsLocal/coroutines/redundantAsync/globalScope.kt +++ b/idea/testData/inspectionsLocal/coroutines/redundantAsync/globalScope.kt @@ -1,9 +1,7 @@ // WITH_RUNTIME -// PROBLEM: none package kotlinx.coroutines suspend fun test(ctx: CoroutineContext) { - // Does not work yet (1.3.11) GlobalScope.async(ctx) { 42 }.await() } \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/coroutines/redundantAsync/globalScope.kt.after b/idea/testData/inspectionsLocal/coroutines/redundantAsync/globalScope.kt.after new file mode 100644 index 00000000000..273a7dac96e --- /dev/null +++ b/idea/testData/inspectionsLocal/coroutines/redundantAsync/globalScope.kt.after @@ -0,0 +1,7 @@ +// WITH_RUNTIME + +package kotlinx.coroutines + +suspend fun test(ctx: CoroutineContext) { + withContext(ctx) { 42 } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/coroutines/redundantAsync/globalScopeNoContext.kt b/idea/testData/inspectionsLocal/coroutines/redundantAsync/globalScopeNoContext.kt new file mode 100644 index 00000000000..284e9d8c578 --- /dev/null +++ b/idea/testData/inspectionsLocal/coroutines/redundantAsync/globalScopeNoContext.kt @@ -0,0 +1,7 @@ +// WITH_RUNTIME + +package kotlinx.coroutines + +suspend fun test() { + GlobalScope.async() { 42 }.await() +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/coroutines/redundantAsync/globalScopeNoContext.kt.after b/idea/testData/inspectionsLocal/coroutines/redundantAsync/globalScopeNoContext.kt.after new file mode 100644 index 00000000000..c1fa61e4d99 --- /dev/null +++ b/idea/testData/inspectionsLocal/coroutines/redundantAsync/globalScopeNoContext.kt.after @@ -0,0 +1,7 @@ +// WITH_RUNTIME + +package kotlinx.coroutines + +suspend fun test() { + withContext(Dispatchers.Default) { 42 } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/coroutines/redundantAsync/globalScopeNoContextNoPackage.kt b/idea/testData/inspectionsLocal/coroutines/redundantAsync/globalScopeNoContextNoPackage.kt new file mode 100644 index 00000000000..9a4da53363f --- /dev/null +++ b/idea/testData/inspectionsLocal/coroutines/redundantAsync/globalScopeNoContextNoPackage.kt @@ -0,0 +1,8 @@ +// WITH_RUNTIME + +import kotlinx.coroutines.GlobalScope +import kotlinx.coroutines.async + +suspend fun test() { + GlobalScope.async() { 42 }.await() +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/coroutines/redundantAsync/globalScopeNoContextNoPackage.kt.after b/idea/testData/inspectionsLocal/coroutines/redundantAsync/globalScopeNoContextNoPackage.kt.after new file mode 100644 index 00000000000..9a19a7afd78 --- /dev/null +++ b/idea/testData/inspectionsLocal/coroutines/redundantAsync/globalScopeNoContextNoPackage.kt.after @@ -0,0 +1,10 @@ +// WITH_RUNTIME + +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.GlobalScope +import kotlinx.coroutines.async +import kotlinx.coroutines.withContext + +suspend fun test() { + withContext(Dispatchers.Default) { 42 } +} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java index 597b20c7e94..9d09e86503d 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java @@ -2080,6 +2080,16 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest { runTest("idea/testData/inspectionsLocal/coroutines/redundantAsync/globalScope.kt"); } + @TestMetadata("globalScopeNoContext.kt") + public void testGlobalScopeNoContext() throws Exception { + runTest("idea/testData/inspectionsLocal/coroutines/redundantAsync/globalScopeNoContext.kt"); + } + + @TestMetadata("globalScopeNoContextNoPackage.kt") + public void testGlobalScopeNoContextNoPackage() throws Exception { + runTest("idea/testData/inspectionsLocal/coroutines/redundantAsync/globalScopeNoContextNoPackage.kt"); + } + @TestMetadata("simple.kt") public void testSimple() throws Exception { runTest("idea/testData/inspectionsLocal/coroutines/redundantAsync/simple.kt");