From 5edfc0f5cdac029ed6478c46793c17b70fa43bd7 Mon Sep 17 00:00:00 2001 From: Denis Zharkov Date: Mon, 19 Feb 2018 14:14:09 +0300 Subject: [PATCH] Prohibit labeled returns inside kotlin.suspend argument lambdas Probably, it would be more correct to skip such lambdas when resolving the returns' references, but it'd be more complicated and still useless since non-local returns are impossible in such lambdas (relevant parameter is noinline) #KT-22900 Fixed --- .../jetbrains/kotlin/diagnostics/Errors.java | 1 + .../rendering/DefaultErrorMessages.java | 1 + .../org/jetbrains/kotlin/psi/KtPsiUtil.java | 5 ++++ .../LambdaWithSuspendModifierCallChecker.kt | 1 + .../ControlStructureTypingVisitor.java | 19 +++++++++++++ .../returnLabelForBuiltInSuspend.kt | 27 +++++++++++++++++++ .../DiagnosticsTestWithStdLibGenerated.java | 6 +++++ ...ticsTestWithStdLibUsingJavacGenerated.java | 6 +++++ 8 files changed, 66 insertions(+) create mode 100644 compiler/testData/diagnostics/testsWithStdLib/coroutines/returnLabelForBuiltInSuspend.kt diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java index d2fb9fd560b..80276a153d5 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java @@ -996,6 +996,7 @@ public interface Errors { DiagnosticFactory1 ILLEGAL_SUSPEND_PROPERTY_ACCESS = DiagnosticFactory1.create(ERROR); DiagnosticFactory0 ILLEGAL_RESTRICTED_SUSPENDING_FUNCTION_CALL = DiagnosticFactory0.create(ERROR); DiagnosticFactory0 NON_MODIFIER_FORM_FOR_BUILT_IN_SUSPEND = DiagnosticFactory0.create(ERROR); + DiagnosticFactory0 RETURN_FOR_BUILT_IN_SUSPEND = DiagnosticFactory0.create(ERROR); DiagnosticFactory0 MODIFIER_FORM_FOR_NON_BUILT_IN_SUSPEND = DiagnosticFactory0.create(WARNING); DiagnosticFactory1> PLUGIN_ERROR = DiagnosticFactory1.create(ERROR); diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java index 9fc588c458d..123f8ab8c8d 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java @@ -897,6 +897,7 @@ public class DefaultErrorMessages { MAP.put(ILLEGAL_SUSPEND_PROPERTY_ACCESS, "Suspend property ''{0}'' should be accessed only from a coroutine or suspend function", NAME); MAP.put(ILLEGAL_RESTRICTED_SUSPENDING_FUNCTION_CALL, "Restricted suspending functions can only invoke member or extension suspending functions on their restricted coroutine scope"); MAP.put(NON_MODIFIER_FORM_FOR_BUILT_IN_SUSPEND, "''suspend'' function can only be called in a form of modifier of a lambda: suspend { ... }"); + MAP.put(RETURN_FOR_BUILT_IN_SUSPEND, "Using implicit label for this lambda is prohibited"); MAP.put(MODIFIER_FORM_FOR_NON_BUILT_IN_SUSPEND, "Calls having a form of ''suspend {}'' are deprecated because ''suspend'' in the context will have a meaning of a modifier. Add empty argument list to the call: ''suspend() { ... }''"); MAP.put(PLUGIN_ERROR, "{0}", (d, c) -> d.getText()); diff --git a/compiler/frontend/src/org/jetbrains/kotlin/psi/KtPsiUtil.java b/compiler/frontend/src/org/jetbrains/kotlin/psi/KtPsiUtil.java index d04a00447b5..057a608be3e 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/psi/KtPsiUtil.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/psi/KtPsiUtil.java @@ -920,6 +920,11 @@ public class KtPsiUtil { return null; } + public static boolean isLabeledFunctionLiteral(@NotNull KtFunctionLiteral functionLiteral) { + // KtFunctionLiteral -> KtLambdaExpression -> KtLabeledExpression + return functionLiteral.getParent().getParent() instanceof KtLabeledExpression; + } + @Nullable public static KtExpression getLastElementDeparenthesized( @Nullable KtExpression expression, diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/checkers/LambdaWithSuspendModifierCallChecker.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/checkers/LambdaWithSuspendModifierCallChecker.kt index f1c7d2a4e55..a05d399c64d 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/checkers/LambdaWithSuspendModifierCallChecker.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/checkers/LambdaWithSuspendModifierCallChecker.kt @@ -19,6 +19,7 @@ import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameOrNull import org.jetbrains.kotlin.utils.addToStdlib.safeAs object LambdaWithSuspendModifierCallChecker : CallChecker { + @JvmField val KOTLIN_SUSPEND_BUILT_IN_FUNCTION_FQ_NAME = FqName("kotlin.suspend") override fun check(resolvedCall: ResolvedCall<*>, reportOn: PsiElement, context: CallCheckerContext) { diff --git a/compiler/frontend/src/org/jetbrains/kotlin/types/expressions/ControlStructureTypingVisitor.java b/compiler/frontend/src/org/jetbrains/kotlin/types/expressions/ControlStructureTypingVisitor.java index 0a9341a418c..1d680be354b 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/types/expressions/ControlStructureTypingVisitor.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/types/expressions/ControlStructureTypingVisitor.java @@ -31,6 +31,8 @@ import org.jetbrains.kotlin.resolve.BindingContextUtils; import org.jetbrains.kotlin.resolve.ModifierCheckerCore; import org.jetbrains.kotlin.resolve.ModifiersChecker; import org.jetbrains.kotlin.resolve.calls.ArgumentTypeResolver; +import org.jetbrains.kotlin.resolve.calls.callUtil.CallUtilKt; +import org.jetbrains.kotlin.resolve.calls.checkers.LambdaWithSuspendModifierCallChecker; import org.jetbrains.kotlin.resolve.calls.model.MutableDataFlowInfoForArguments; import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall; import org.jetbrains.kotlin.resolve.calls.smartcasts.DataFlowInfo; @@ -55,6 +57,7 @@ import org.jetbrains.kotlin.types.expressions.typeInfoFactory.TypeInfoFactoryKt; import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Objects; import static org.jetbrains.kotlin.diagnostics.Errors.*; import static org.jetbrains.kotlin.resolve.BindingContext.*; @@ -663,6 +666,22 @@ public class ControlStructureTypingVisitor extends ExpressionTypingVisitor { context.trace.report(RETURN_NOT_ALLOWED.on(expression)); resultType = ErrorUtils.createErrorType(RETURN_NOT_ALLOWED_MESSAGE); } + else if (labelTargetElement instanceof KtFunctionLiteral + && Objects.equals(expression.getLabelName(), "suspend")) { + KtExpression callExpression = KtPsiUtil.getParentCallIfPresent((KtFunction) labelTargetElement); + ResolvedCall resolvedCall = + CallUtilKt.getResolvedCall(callExpression, context.trace.getBindingContext()); + + if (resolvedCall != null && + !KtPsiUtil.isLabeledFunctionLiteral((KtFunctionLiteral) labelTargetElement) && + Objects.equals( + DescriptorUtilsKt.fqNameOrNull(resolvedCall.getResultingDescriptor()), + LambdaWithSuspendModifierCallChecker.KOTLIN_SUSPEND_BUILT_IN_FUNCTION_FQ_NAME + ) + ) { + context.trace.report(RETURN_FOR_BUILT_IN_SUSPEND.on(expression)); + } + } } } diff --git a/compiler/testData/diagnostics/testsWithStdLib/coroutines/returnLabelForBuiltInSuspend.kt b/compiler/testData/diagnostics/testsWithStdLib/coroutines/returnLabelForBuiltInSuspend.kt new file mode 100644 index 00000000000..2e329b65e0c --- /dev/null +++ b/compiler/testData/diagnostics/testsWithStdLib/coroutines/returnLabelForBuiltInSuspend.kt @@ -0,0 +1,27 @@ +// SKIP_TXT +fun bar() { + suspend { + return@suspend + } + + suspend { + run { + return@suspend + } + } + + suspend l@{ + return@l + } + + suspend suspend@{ + return@suspend + } + + val x = suspend@{ + suspend { + // Might be resolved to outer lambda, but doesn't make sense because suspend-lambdas here is noinline + return@suspend + } + } +} diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestWithStdLibGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestWithStdLibGenerated.java index 716bf4f6a36..9144ffbf087 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestWithStdLibGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestWithStdLibGenerated.java @@ -1374,6 +1374,12 @@ public class DiagnosticsTestWithStdLibGenerated extends AbstractDiagnosticsTestW doTest(fileName); } + @TestMetadata("returnLabelForBuiltInSuspend.kt") + public void testReturnLabelForBuiltInSuspend() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/testsWithStdLib/coroutines/returnLabelForBuiltInSuspend.kt"); + doTest(fileName); + } + @TestMetadata("suspendApplicability.kt") public void testSuspendApplicability() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/testsWithStdLib/coroutines/suspendApplicability.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsTestWithStdLibUsingJavacGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsTestWithStdLibUsingJavacGenerated.java index cf9122e5d6b..c1f1d4529a3 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsTestWithStdLibUsingJavacGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsTestWithStdLibUsingJavacGenerated.java @@ -1374,6 +1374,12 @@ public class DiagnosticsTestWithStdLibUsingJavacGenerated extends AbstractDiagno doTest(fileName); } + @TestMetadata("returnLabelForBuiltInSuspend.kt") + public void testReturnLabelForBuiltInSuspend() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/testsWithStdLib/coroutines/returnLabelForBuiltInSuspend.kt"); + doTest(fileName); + } + @TestMetadata("suspendApplicability.kt") public void testSuspendApplicability() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/testsWithStdLib/coroutines/suspendApplicability.kt");