From b4c2eb5f196278cd24a50cd4a651ebec4b75a056 Mon Sep 17 00:00:00 2001 From: Frederik Haselmeier Date: Wed, 13 Mar 2024 09:01:03 +0100 Subject: [PATCH] [Kotlin] Replaced custom logic to test if an expression suspends with a pre-existing implementation KTIJ-29049 GitOrigin-RevId: 687086e40a0d777ab9d2433c9522afbd55abde68 --- .../RedundantSuspendModifierInspection.kt | 82 +++++-------------- .../SharedK1LocalInspectionTestGenerated.java | 10 +++ .../SharedK2LocalInspectionTestGenerated.java | 10 +++ .../suspendingLoopExtensionProperty.kt | 1 - .../suspendingLoopIteratorOtherClass.kt | 1 - .../suspendingLoopLocalExtensionProperty.kt | 1 - .../redundantSuspend/unresolvedFunction.kt | 4 + .../redundantSuspend/unresolvedProperty.kt | 4 + 8 files changed, 50 insertions(+), 63 deletions(-) create mode 100644 plugins/kotlin/code-insight/inspections-shared/tests/testData/inspectionsLocal/redundantSuspend/unresolvedFunction.kt create mode 100644 plugins/kotlin/code-insight/inspections-shared/tests/testData/inspectionsLocal/redundantSuspend/unresolvedProperty.kt diff --git a/plugins/kotlin/code-insight/inspections-shared/src/org/jetbrains/kotlin/idea/codeInsight/inspections/shared/RedundantSuspendModifierInspection.kt b/plugins/kotlin/code-insight/inspections-shared/src/org/jetbrains/kotlin/idea/codeInsight/inspections/shared/RedundantSuspendModifierInspection.kt index e7075b18bc25..273bd36becca 100644 --- a/plugins/kotlin/code-insight/inspections-shared/src/org/jetbrains/kotlin/idea/codeInsight/inspections/shared/RedundantSuspendModifierInspection.kt +++ b/plugins/kotlin/code-insight/inspections-shared/src/org/jetbrains/kotlin/idea/codeInsight/inspections/shared/RedundantSuspendModifierInspection.kt @@ -5,14 +5,19 @@ import com.intellij.codeInspection.IntentionWrapper import com.intellij.codeInspection.LocalInspectionToolSession import com.intellij.codeInspection.ProblemsHolder import com.intellij.psi.PsiElementVisitor +import com.intellij.psi.util.descendantsOfType import org.jetbrains.kotlin.analysis.api.KtAnalysisSession import org.jetbrains.kotlin.analysis.api.analyze -import org.jetbrains.kotlin.analysis.api.calls.* +import org.jetbrains.kotlin.analysis.api.calls.KtCallableMemberCall +import org.jetbrains.kotlin.analysis.api.calls.successfulCallOrNull import org.jetbrains.kotlin.analysis.api.symbols.KtCallableSymbol import org.jetbrains.kotlin.analysis.api.symbols.KtFunctionSymbol +import org.jetbrains.kotlin.analysis.api.symbols.KtKotlinPropertySymbol import org.jetbrains.kotlin.builtins.StandardNames import org.jetbrains.kotlin.config.LanguageFeature import org.jetbrains.kotlin.descriptors.Modality +import org.jetbrains.kotlin.idea.base.codeInsight.KotlinCallProcessor +import org.jetbrains.kotlin.idea.base.codeInsight.process import org.jetbrains.kotlin.idea.base.projectStructure.languageVersionSettings import org.jetbrains.kotlin.idea.base.resources.KotlinBundle import org.jetbrains.kotlin.idea.codeinsight.api.classic.inspections.AbstractKotlinInspection @@ -20,7 +25,9 @@ import org.jetbrains.kotlin.idea.codeinsight.utils.getFqNameIfPackageOrNonLocal import org.jetbrains.kotlin.idea.quickfix.RemoveModifierFixBase import org.jetbrains.kotlin.lexer.KtTokens import org.jetbrains.kotlin.name.Name -import org.jetbrains.kotlin.psi.* +import org.jetbrains.kotlin.psi.KtExpression +import org.jetbrains.kotlin.psi.KtNamedFunction +import org.jetbrains.kotlin.psi.namedFunctionVisitor import org.jetbrains.kotlin.psi.psiUtil.anyDescendantOfType internal class RedundantSuspendModifierInspection : AbstractKotlinInspection() { @@ -36,6 +43,7 @@ internal class RedundantSuspendModifierInspection : AbstractKotlinInspection() { val functionSymbol = function.getFunctionLikeSymbol() as? KtFunctionSymbol ?: return if (functionSymbol.modality == Modality.OPEN) return + if (function.hasUnresolvedCalls()) return if (function.hasSuspendOrUnresolvedCall(functionSymbol)) return holder.registerProblem( @@ -51,77 +59,31 @@ internal class RedundantSuspendModifierInspection : AbstractKotlinInspection() { context(KtAnalysisSession) private fun KtCallableSymbol.isSuspendSymbol(): Boolean { - // Currently, Kotlin does not support suspending properties except for accessing the coroutineContext - if (getFqNameIfPackageOrNonLocal() == coroutineContextFqName) { + if (this is KtKotlinPropertySymbol && getFqNameIfPackageOrNonLocal() == coroutineContextFqName) { return true } - return this is KtFunctionSymbol && isSuspend } context(KtAnalysisSession) - private fun KtExpression.resolveMemberFunction( - name: String, - psiFactory: KtPsiFactory, - context: KtExpression = this - ): KtFunctionCall<*>? { - val newExpression = psiFactory.createExpressionByPattern("$0.$1()", this, name) - val fragment = KtPsiFactory(project).createExpressionCodeFragment(newExpression.text, context) - val expression = fragment.firstChild as? KtExpression ?: return null - return expression.resolveCall()?.successfulFunctionCallOrNull() - } - - context(KtAnalysisSession) - private fun KtForExpression.isSuspendingLoopOrUnresolved(): Boolean { - val loopRangeExpression = loopRange ?: return true - val psiFactory = KtPsiFactory(project) - val iteratorFunction = loopRangeExpression.resolveMemberFunction("iterator", psiFactory) ?: return true - if (iteratorFunction.partiallyAppliedSymbol.symbol.isSuspendSymbol()) { - return true - } - val functionsToCheck = listOf("hasNext", "next") - for (f in functionsToCheck) { - val iteratorExpression = psiFactory.createExpressionByPattern("$0.iterator()", loopRangeExpression) - val resolvedFunction = iteratorExpression.resolveMemberFunction(f, psiFactory, loopRangeExpression) ?: return true - if (resolvedFunction.partiallyAppliedSymbol.symbol.isSuspendSymbol()) { - return true - } - } - return false - } - - context(KtAnalysisSession) - private fun KtCallInfo.isExternalSuspendOrUnresolved(selfSymbol: KtFunctionSymbol): Boolean { - val functionCall = successfulCallOrNull>() ?: return true - val symbol = functionCall.partiallyAppliedSymbol.symbol // Recursive call to itself, ignore - if (symbol == selfSymbol) return false - if (symbol.isSuspendSymbol()) return true - - return if (functionCall is KtCompoundVariableAccessCall) { - val compoundAccessSymbol = functionCall.compoundAccess.operationPartiallyAppliedSymbol.symbol - if (compoundAccessSymbol == selfSymbol) return false - compoundAccessSymbol.isSuspendSymbol() - } else { - false + private fun KtNamedFunction.hasUnresolvedCalls(): Boolean { + return anyDescendantOfType { expression -> + val resolvedCall = expression.resolveCall() ?: return@anyDescendantOfType false + resolvedCall.successfulCallOrNull>() == null } } context(KtAnalysisSession) private fun KtNamedFunction.hasSuspendOrUnresolvedCall(functionSymbol: KtFunctionSymbol): Boolean { - return anyDescendantOfType { expression -> - if (expression == this) return@anyDescendantOfType false - if (expression is KtForExpression) { - return@anyDescendantOfType expression.isSuspendingLoopOrUnresolved() + val allExpressions = descendantsOfType().toList() + var hasSuspendCall = false + val selfCallableId = functionSymbol.callableIdIfNonLocal + KotlinCallProcessor.process(allExpressions) { target -> + if (target.symbol.isSuspendSymbol() && target.symbol.callableIdIfNonLocal != selfCallableId) { + hasSuspendCall = true } - // If resolveCall returns null, we skip it (likely block/function/etc., not an actual expression we want to analyze) - val resolvedCall = expression.resolveCall() - ?: return@anyDescendantOfType false - // If we cannot resolve to anything or a singular call, then we do not know if this might be suspending or not - if (resolvedCall is KtErrorCallInfo) { - return@anyDescendantOfType true - } - resolvedCall.isExternalSuspendOrUnresolved(functionSymbol) } + return hasSuspendCall } } \ No newline at end of file diff --git a/plugins/kotlin/code-insight/inspections-shared/tests/k1/test/org/jetbrains/kotlin/idea/codeInsight/inspections/shared/SharedK1LocalInspectionTestGenerated.java b/plugins/kotlin/code-insight/inspections-shared/tests/k1/test/org/jetbrains/kotlin/idea/codeInsight/inspections/shared/SharedK1LocalInspectionTestGenerated.java index 64ef7cf30227..a26f1df9eed3 100644 --- a/plugins/kotlin/code-insight/inspections-shared/tests/k1/test/org/jetbrains/kotlin/idea/codeInsight/inspections/shared/SharedK1LocalInspectionTestGenerated.java +++ b/plugins/kotlin/code-insight/inspections-shared/tests/k1/test/org/jetbrains/kotlin/idea/codeInsight/inspections/shared/SharedK1LocalInspectionTestGenerated.java @@ -1113,6 +1113,16 @@ public abstract class SharedK1LocalInspectionTestGenerated extends AbstractShare public void testSuspendingLoopLocalExtensionProperty() throws Exception { runTest("../testData/inspectionsLocal/redundantSuspend/suspendingLoopLocalExtensionProperty.kt"); } + + @TestMetadata("unresolvedFunction.kt") + public void testUnresolvedFunction() throws Exception { + runTest("../testData/inspectionsLocal/redundantSuspend/unresolvedFunction.kt"); + } + + @TestMetadata("unresolvedProperty.kt") + public void testUnresolvedProperty() throws Exception { + runTest("../testData/inspectionsLocal/redundantSuspend/unresolvedProperty.kt"); + } } @RunWith(JUnit3RunnerWithInners.class) diff --git a/plugins/kotlin/code-insight/inspections-shared/tests/k2/test/org/jetbrains/kotlin/idea/k2/codeInsight/inspections/shared/SharedK2LocalInspectionTestGenerated.java b/plugins/kotlin/code-insight/inspections-shared/tests/k2/test/org/jetbrains/kotlin/idea/k2/codeInsight/inspections/shared/SharedK2LocalInspectionTestGenerated.java index 4a157dde1092..9ada3582bade 100644 --- a/plugins/kotlin/code-insight/inspections-shared/tests/k2/test/org/jetbrains/kotlin/idea/k2/codeInsight/inspections/shared/SharedK2LocalInspectionTestGenerated.java +++ b/plugins/kotlin/code-insight/inspections-shared/tests/k2/test/org/jetbrains/kotlin/idea/k2/codeInsight/inspections/shared/SharedK2LocalInspectionTestGenerated.java @@ -1113,6 +1113,16 @@ public abstract class SharedK2LocalInspectionTestGenerated extends AbstractShare public void testSuspendingLoopLocalExtensionProperty() throws Exception { runTest("../testData/inspectionsLocal/redundantSuspend/suspendingLoopLocalExtensionProperty.kt"); } + + @TestMetadata("unresolvedFunction.kt") + public void testUnresolvedFunction() throws Exception { + runTest("../testData/inspectionsLocal/redundantSuspend/unresolvedFunction.kt"); + } + + @TestMetadata("unresolvedProperty.kt") + public void testUnresolvedProperty() throws Exception { + runTest("../testData/inspectionsLocal/redundantSuspend/unresolvedProperty.kt"); + } } @RunWith(JUnit3RunnerWithInners.class) diff --git a/plugins/kotlin/code-insight/inspections-shared/tests/testData/inspectionsLocal/redundantSuspend/suspendingLoopExtensionProperty.kt b/plugins/kotlin/code-insight/inspections-shared/tests/testData/inspectionsLocal/redundantSuspend/suspendingLoopExtensionProperty.kt index 041d893b63aa..99b684df139a 100644 --- a/plugins/kotlin/code-insight/inspections-shared/tests/testData/inspectionsLocal/redundantSuspend/suspendingLoopExtensionProperty.kt +++ b/plugins/kotlin/code-insight/inspections-shared/tests/testData/inspectionsLocal/redundantSuspend/suspendingLoopExtensionProperty.kt @@ -1,4 +1,3 @@ -// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. // PROBLEM: none class SIterable { diff --git a/plugins/kotlin/code-insight/inspections-shared/tests/testData/inspectionsLocal/redundantSuspend/suspendingLoopIteratorOtherClass.kt b/plugins/kotlin/code-insight/inspections-shared/tests/testData/inspectionsLocal/redundantSuspend/suspendingLoopIteratorOtherClass.kt index 7081c75db56a..f99f26b3ddb6 100644 --- a/plugins/kotlin/code-insight/inspections-shared/tests/testData/inspectionsLocal/redundantSuspend/suspendingLoopIteratorOtherClass.kt +++ b/plugins/kotlin/code-insight/inspections-shared/tests/testData/inspectionsLocal/redundantSuspend/suspendingLoopIteratorOtherClass.kt @@ -1,4 +1,3 @@ -// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. // PROBLEM: none class OtherIterator { diff --git a/plugins/kotlin/code-insight/inspections-shared/tests/testData/inspectionsLocal/redundantSuspend/suspendingLoopLocalExtensionProperty.kt b/plugins/kotlin/code-insight/inspections-shared/tests/testData/inspectionsLocal/redundantSuspend/suspendingLoopLocalExtensionProperty.kt index ab4dc507033e..ea98c5e68cc0 100644 --- a/plugins/kotlin/code-insight/inspections-shared/tests/testData/inspectionsLocal/redundantSuspend/suspendingLoopLocalExtensionProperty.kt +++ b/plugins/kotlin/code-insight/inspections-shared/tests/testData/inspectionsLocal/redundantSuspend/suspendingLoopLocalExtensionProperty.kt @@ -1,4 +1,3 @@ -// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. // PROBLEM: none class SIterable { diff --git a/plugins/kotlin/code-insight/inspections-shared/tests/testData/inspectionsLocal/redundantSuspend/unresolvedFunction.kt b/plugins/kotlin/code-insight/inspections-shared/tests/testData/inspectionsLocal/redundantSuspend/unresolvedFunction.kt new file mode 100644 index 000000000000..e9ef8d04b6a6 --- /dev/null +++ b/plugins/kotlin/code-insight/inspections-shared/tests/testData/inspectionsLocal/redundantSuspend/unresolvedFunction.kt @@ -0,0 +1,4 @@ +// PROBLEM: none +suspend fun foo() { + test() +} \ No newline at end of file diff --git a/plugins/kotlin/code-insight/inspections-shared/tests/testData/inspectionsLocal/redundantSuspend/unresolvedProperty.kt b/plugins/kotlin/code-insight/inspections-shared/tests/testData/inspectionsLocal/redundantSuspend/unresolvedProperty.kt new file mode 100644 index 000000000000..125e71648146 --- /dev/null +++ b/plugins/kotlin/code-insight/inspections-shared/tests/testData/inspectionsLocal/redundantSuspend/unresolvedProperty.kt @@ -0,0 +1,4 @@ +// PROBLEM: none +suspend fun foo() { + test +} \ No newline at end of file