diff --git a/idea/resources/inspectionDescriptions/RedundantSuspendModifier.html b/idea/resources/inspectionDescriptions/RedundantSuspendModifier.html new file mode 100644 index 00000000000..9ba4c57da34 --- /dev/null +++ b/idea/resources/inspectionDescriptions/RedundantSuspendModifier.html @@ -0,0 +1,5 @@ + + +This inspection reports 'suspend' modifier as redundant if no other suspend functions are called inside + + diff --git a/idea/src/META-INF/plugin.xml b/idea/src/META-INF/plugin.xml index 712ebd8a16d..562b81ca492 100644 --- a/idea/src/META-INF/plugin.xml +++ b/idea/src/META-INF/plugin.xml @@ -2691,6 +2691,16 @@ level="WEAK WARNING" language="kotlin" /> + + + diff --git a/idea/src/org/jetbrains/kotlin/idea/highlighter/KotlinSuspendCallLineMarkerProvider.kt b/idea/src/org/jetbrains/kotlin/idea/highlighter/KotlinSuspendCallLineMarkerProvider.kt index b23c03a95f3..920e7e7d7cd 100644 --- a/idea/src/org/jetbrains/kotlin/idea/highlighter/KotlinSuspendCallLineMarkerProvider.kt +++ b/idea/src/org/jetbrains/kotlin/idea/highlighter/KotlinSuspendCallLineMarkerProvider.kt @@ -19,6 +19,7 @@ package org.jetbrains.kotlin.idea.highlighter import com.intellij.codeHighlighting.Pass import com.intellij.codeInsight.daemon.LineMarkerInfo import com.intellij.codeInsight.daemon.LineMarkerProvider +import com.intellij.openapi.actionSystem.AnAction import com.intellij.openapi.editor.markup.GutterIconRenderer import com.intellij.openapi.progress.ProgressManager import com.intellij.psi.PsiElement @@ -43,19 +44,12 @@ class KotlinSuspendCallLineMarkerProvider : LineMarkerProvider { ) { override fun createGutterRenderer(): GutterIconRenderer? { return object : LineMarkerInfo.LineMarkerGutterIconRenderer(this) { - override fun getClickAction() = null + override fun getClickAction(): AnAction? = null } } } - override fun getLineMarkerInfo(element: PsiElement) = null - - private fun isValidCandidateExpression(expression: KtExpression): Boolean { - if (expression is KtOperationReferenceExpression || expression is KtForExpression) return true - val parent = expression.parent - if (parent is KtCallExpression && parent.calleeExpression == expression) return true - return false - } + override fun getLineMarkerInfo(element: PsiElement): LineMarkerInfo<*>? = null override fun collectSlowLineMarkers( elements: MutableList, @@ -67,27 +61,34 @@ class KotlinSuspendCallLineMarkerProvider : LineMarkerProvider { ProgressManager.checkCanceled() if (element !is KtExpression) continue - if (!isValidCandidateExpression(element)) continue - val lineNumber = element.getLineNumber() if (lineNumber in markedLineNumbers) continue - - val bindingContext = element.analyze(BodyResolveMode.PARTIAL) - val resolvedCall = if (element is KtForExpression) { - bindingContext[BindingContext.LOOP_RANGE_NEXT_RESOLVED_CALL, element.loopRange] - } else { - element.getResolvedCall(bindingContext) - } ?: continue - val calleeDescriptor = resolvedCall.resultingDescriptor as? FunctionDescriptor ?: continue - if (!calleeDescriptor.isSuspend) continue + if (!element.hasSuspendCalls()) continue markedLineNumbers += lineNumber result += if (element is KtForExpression) { SuspendCallMarkerInfo(element.loopRange!!, "Suspending iteration") - } else { SuspendCallMarkerInfo(element, "Suspend function call") } } } } + +private fun KtExpression.isValidCandidateExpression(): Boolean { + if (this is KtOperationReferenceExpression || this is KtForExpression) return true + val parent = parent + if (parent is KtCallExpression && parent.calleeExpression == this) return true + return false +} + +fun KtExpression.hasSuspendCalls(bindingContext: BindingContext = analyze(BodyResolveMode.PARTIAL)): Boolean { + if (!isValidCandidateExpression()) return false + + val resolvedCall = if (this is KtForExpression) { + bindingContext[BindingContext.LOOP_RANGE_NEXT_RESOLVED_CALL, loopRange] + } else { + this.getResolvedCall(bindingContext) + } ?: return false + return (resolvedCall.resultingDescriptor as? FunctionDescriptor)?.isSuspend == true +} diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantSuspendModifierInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantSuspendModifierInspection.kt new file mode 100644 index 00000000000..0fb756799b5 --- /dev/null +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantSuspendModifierInspection.kt @@ -0,0 +1,60 @@ +/* + * Copyright 2010-2017 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.jetbrains.kotlin.idea.inspections + +import com.intellij.codeInspection.IntentionWrapper +import com.intellij.codeInspection.LocalInspectionToolSession +import com.intellij.codeInspection.ProblemHighlightType +import com.intellij.codeInspection.ProblemsHolder +import com.intellij.psi.PsiElementVisitor +import org.jetbrains.kotlin.config.LanguageFeature +import org.jetbrains.kotlin.idea.caches.resolve.analyzeFully +import org.jetbrains.kotlin.idea.highlighter.hasSuspendCalls +import org.jetbrains.kotlin.idea.project.languageVersionSettings +import org.jetbrains.kotlin.idea.quickfix.RemoveModifierFix +import org.jetbrains.kotlin.lexer.KtTokens +import org.jetbrains.kotlin.psi.KtExpression +import org.jetbrains.kotlin.psi.KtNamedFunction +import org.jetbrains.kotlin.psi.KtVisitorVoid +import org.jetbrains.kotlin.psi.psiUtil.anyDescendantOfType + +class RedundantSuspendModifierInspection : AbstractKotlinInspection() { + override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean, session: LocalInspectionToolSession): PsiElementVisitor { + return object : KtVisitorVoid() { + override fun visitNamedFunction(function: KtNamedFunction) { + super.visitNamedFunction(function) + if (!function.languageVersionSettings.supportsFeature(LanguageFeature.Coroutines)) return + + val suspendModifier = function.modifierList?.getModifier(KtTokens.SUSPEND_KEYWORD) ?: return + if (!function.hasBody()) return + + val context = function.analyzeFully() + + if (function.anyDescendantOfType { it.hasSuspendCalls(context) }) { + return + } + + holder.registerProblem(suspendModifier, + "Redundant 'suspend' modifier", + ProblemHighlightType.LIKE_UNUSED_SYMBOL, + IntentionWrapper(RemoveModifierFix(function, KtTokens.SUSPEND_KEYWORD, isRedundant = true), + function.containingFile)) + + } + } + } +} diff --git a/idea/testData/inspections/redundantSuspendModifier/inspectionData/expected.xml b/idea/testData/inspections/redundantSuspendModifier/inspectionData/expected.xml new file mode 100644 index 00000000000..82c8d6a142c --- /dev/null +++ b/idea/testData/inspections/redundantSuspendModifier/inspectionData/expected.xml @@ -0,0 +1,67 @@ + + + test.kt + 13 + light_idea_test_case + + Redundant 'suspend' modifier + Redundant 'suspend' modifier + + + test.kt + 33 + light_idea_test_case + + Redundant 'suspend' modifier + Redundant 'suspend' modifier + + + test.kt + 35 + light_idea_test_case + + Redundant 'suspend' modifier + Redundant 'suspend' modifier + + + test.kt + 40 + light_idea_test_case + + Redundant 'suspend' modifier + Redundant 'suspend' modifier + + + + test.kt + 57 + light_idea_test_case + + Redundant 'suspend' modifier + Redundant 'suspend' modifier + + + operators.kt + 5 + light_idea_test_case + + Redundant 'suspend' modifier + Redundant 'suspend' modifier + + + operators.kt + 23 + light_idea_test_case + + Redundant 'suspend' modifier + Redundant 'suspend' modifier + + + operators.kt + 37 + light_idea_test_case + + Redundant 'suspend' modifier + Redundant 'suspend' modifier + + \ No newline at end of file diff --git a/idea/testData/inspections/redundantSuspendModifier/inspectionData/inspections.test b/idea/testData/inspections/redundantSuspendModifier/inspectionData/inspections.test new file mode 100644 index 00000000000..f87aff07270 --- /dev/null +++ b/idea/testData/inspections/redundantSuspendModifier/inspectionData/inspections.test @@ -0,0 +1 @@ +// INSPECTION_CLASS: org.jetbrains.kotlin.idea.inspections.RedundantSuspendModifierInspection diff --git a/idea/testData/inspections/redundantSuspendModifier/operators.kt b/idea/testData/inspections/redundantSuspendModifier/operators.kt new file mode 100644 index 00000000000..405f1f3ea73 --- /dev/null +++ b/idea/testData/inspections/redundantSuspendModifier/operators.kt @@ -0,0 +1,43 @@ +// WITH_RUNTIME + +class A(val x: Int) { + // Redundant + suspend operator fun plus(a: A): A { + return A(x + a.x) + } +} + +// Not redundant +suspend fun foo(a1: A, a2: A): A { + return a1 + a2 +} +// Not redundant +suspend fun bar(a1: A, a2: A): A { + var result = a1 + result += a2 + return result +} + +class B(var x: Int) { + // Redundant + suspend operator fun minusAssign(b: B) { + x -= b.x + } +} + +// Not redundant +suspend fun foo(b1: B, b2: B): B { + val result = b1 + result -= b2 + return result +} + +class C(val x: Int, val y: Int) { + // Redundant + suspend operator fun invoke() = x + y +} + +// Not redundant +suspend fun foo(c1: C, c2: C): Int { + return c1() + c2() +} \ No newline at end of file diff --git a/idea/testData/inspections/redundantSuspendModifier/test.kt b/idea/testData/inspections/redundantSuspendModifier/test.kt new file mode 100644 index 00000000000..09509bf8d52 --- /dev/null +++ b/idea/testData/inspections/redundantSuspendModifier/test.kt @@ -0,0 +1,69 @@ +// WITH_RUNTIME + +fun coroutine(block: suspend () -> Unit) {} + +// Not redundant +suspend fun rootSuspend() { + coroutine { + empty() + } +} + +// Redundant +suspend fun empty() {} + +suspend fun suspendUser() = rootSuspend() // not redundant + +open class My { + // Not redundant + open suspend fun baseSuspend() { + rootSuspend() + } +} + +class Your : My() { + override fun baseSuspend() { + + } +} + +class SIterable { + operator fun iterator() = this + // Redundant + suspend operator fun hasNext(): Boolean = false + // Redundant + suspend operator fun next(): Int = 0 +} + +class SIterator { + // Redundant + suspend operator fun iterator() = this + operator fun hasNext(): Boolean = false + operator fun next(): Int = 0 + +} + +// Not redundant +suspend fun foo() { + val iterable = SIterable() + coroutine { + for (x in iterable) { + println(x) + } + } +} + +// Not redundant +suspend fun bar() { + val iterator = SIterator() + coroutine { + for (x in iterator) { + println(x) + } + } +} + +interface Suspended { + // Not redundant + suspend fun bar() +} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/codeInsight/InspectionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/codeInsight/InspectionTestGenerated.java index e13e197d39d..16e4d39eacb 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/codeInsight/InspectionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/codeInsight/InspectionTestGenerated.java @@ -323,6 +323,12 @@ public class InspectionTestGenerated extends AbstractInspectionTest { doTest(fileName); } + @TestMetadata("redundantSuspendModifier/inspectionData/inspections.test") + public void testRedundantSuspendModifier_inspectionData_Inspections_test() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspections/redundantSuspendModifier/inspectionData/inspections.test"); + doTest(fileName); + } + @TestMetadata("redundantUnitReturnType/inspectionData/inspections.test") public void testRedundantUnitReturnType_inspectionData_Inspections_test() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspections/redundantUnitReturnType/inspectionData/inspections.test");