FIR: introduce ILLEGAL_SUSPEND_FUNCTION_CALL & PROPERTY_ACCESS diagnostics

2021-07-20 16:36:43 +03:00
parent e4992176c1
commit 8f1d07084b
33 changed files with 147 additions and 222 deletions
@@ -1182,6 +1182,15 @@ object DIAGNOSTICS_LIST : DiagnosticList("FirErrors") {

        val OPERATOR_RENAMED_ON_IMPORT by error<KtImportDirective>(PositioningStrategy.IMPORT_LAST_NAME)
    }
+
+    val SUSPEND by object : DiagnosticGroup("Suspend errors") {
+        val ILLEGAL_SUSPEND_FUNCTION_CALL by error<PsiElement>(PositioningStrategy.REFERENCED_NAME_BY_QUALIFIED) {
+            parameter<Symbol>("suspendCallable")
+        }
+        val ILLEGAL_SUSPEND_PROPERTY_ACCESS by error<PsiElement>(PositioningStrategy.REFERENCED_NAME_BY_QUALIFIED) {
+            parameter<Symbol>("suspendCallable")
+        }
+    }
 }

 private val exposedVisibilityDiagnosticInit: DiagnosticBuilder.() -> Unit = {
@@ -607,4 +607,8 @@ object FirErrors {
    val CONFLICTING_IMPORT by error1<KtImportDirective, Name>(SourceElementPositioningStrategies.IMPORT_LAST_NAME)
    val OPERATOR_RENAMED_ON_IMPORT by error0<KtImportDirective>(SourceElementPositioningStrategies.IMPORT_LAST_NAME)

+    // Suspend errors
+    val ILLEGAL_SUSPEND_FUNCTION_CALL by error1<PsiElement, FirBasedSymbol<*>>(SourceElementPositioningStrategies.REFERENCED_NAME_BY_QUALIFIED)
+    val ILLEGAL_SUSPEND_PROPERTY_ACCESS by error1<PsiElement, FirBasedSymbol<*>>(SourceElementPositioningStrategies.REFERENCED_NAME_BY_QUALIFIED)
+
 }
@@ -41,7 +41,8 @@ object CommonExpressionCheckers : ExpressionCheckers() {
            FirSealedClassConstructorCallChecker,
            FirUninitializedEnumChecker,
            FirFunInterfaceConstructorReferenceChecker,
-            FirReifiedChecker
+            FirReifiedChecker,
+            FirSuspendCallChecker,
        )

    override val functionCallCheckers: Set<FirFunctionCallChecker>
@@ -0,0 +1,63 @@
+/*
+ * Copyright 2010-2021 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file.
+ */
+
+package org.jetbrains.kotlin.fir.analysis.checkers.expression
+
+import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext
+import org.jetbrains.kotlin.fir.analysis.diagnostics.DiagnosticReporter
+import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors
+import org.jetbrains.kotlin.fir.analysis.diagnostics.reportOn
+import org.jetbrains.kotlin.fir.declarations.*
+import org.jetbrains.kotlin.fir.declarations.utils.isSuspend
+import org.jetbrains.kotlin.fir.expressions.FirQualifiedAccessExpression
+import org.jetbrains.kotlin.fir.references.FirResolvedCallableReference
+import org.jetbrains.kotlin.fir.references.FirResolvedNamedReference
+import org.jetbrains.kotlin.fir.resolve.inference.isSuspendFunctionType
+import org.jetbrains.kotlin.fir.symbols.SymbolInternals
+import org.jetbrains.kotlin.fir.symbols.ensureResolved
+import org.jetbrains.kotlin.fir.symbols.impl.FirCallableSymbol
+import org.jetbrains.kotlin.fir.types.coneType
+import org.jetbrains.kotlin.resolve.calls.checkers.COROUTINE_CONTEXT_1_2_20_FQ_NAME
+import org.jetbrains.kotlin.resolve.calls.checkers.COROUTINE_CONTEXT_1_2_30_FQ_NAME
+import org.jetbrains.kotlin.resolve.calls.checkers.COROUTINE_CONTEXT_1_3_FQ_NAME
+
+object FirSuspendCallChecker : FirQualifiedAccessExpressionChecker() {
+    private val SUSPEND_PROPERTIES_FQ_NAMES = setOf(
+        COROUTINE_CONTEXT_1_2_20_FQ_NAME, COROUTINE_CONTEXT_1_2_30_FQ_NAME, COROUTINE_CONTEXT_1_3_FQ_NAME
+    )
+
+    @OptIn(SymbolInternals::class)
+    override fun check(expression: FirQualifiedAccessExpression, context: CheckerContext, reporter: DiagnosticReporter) {
+        val reference = expression.calleeReference as? FirResolvedNamedReference ?: return
+        if (reference is FirResolvedCallableReference) return
+        val symbol = reference.resolvedSymbol as? FirCallableSymbol ?: return
+        symbol.ensureResolved(FirResolvePhase.STATUS)
+        val fir = symbol.fir as? FirMemberDeclaration ?: return
+        when (fir) {
+            is FirSimpleFunction -> if (!fir.isSuspend) return
+            is FirProperty -> if (symbol.callableId.asSingleFqName() !in SUSPEND_PROPERTIES_FQ_NAMES) return
+            else -> return
+        }
+        val enclosingSuspendFunction = findEnclosingSuspendFunction(context)
+        if (enclosingSuspendFunction == null) {
+            when (fir) {
+                is FirSimpleFunction -> reporter.reportOn(expression.source, FirErrors.ILLEGAL_SUSPEND_FUNCTION_CALL, symbol, context)
+                is FirProperty -> reporter.reportOn(expression.source, FirErrors.ILLEGAL_SUSPEND_PROPERTY_ACCESS, symbol, context)
+                else -> {
+                }
+            }
+        }
+    }
+
+    private fun findEnclosingSuspendFunction(context: CheckerContext): FirFunction? {
+        return context.containingDeclarations.lastOrNull {
+            when (it) {
+                is FirAnonymousFunction -> it.typeRef.coneType.isSuspendFunctionType(context.session)
+                is FirSimpleFunction -> it.isSuspend
+                else -> false
+            }
+        } as? FirFunction
+    }
+}
@@ -191,6 +191,8 @@ import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.HAS_NEXT_FUNCTION
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.ILLEGAL_CONST_EXPRESSION
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.ILLEGAL_KOTLIN_VERSION_STRING_VALUE
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.ILLEGAL_SELECTOR
+import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.ILLEGAL_SUSPEND_FUNCTION_CALL
+import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.ILLEGAL_SUSPEND_PROPERTY_ACCESS
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.ILLEGAL_UNDERSCORE
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.IMPLEMENTATION_BY_DELEGATION_IN_EXPECT_CLASS
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.INAPPLICABLE_CANDIDATE
@@ -1477,6 +1479,18 @@ class FirDefaultErrorMessages {
            )
            map.put(OPERATOR_RENAMED_ON_IMPORT, "Operator renamed to a different operator on import")

+            // Suspend
+            map.put(
+                ILLEGAL_SUSPEND_FUNCTION_CALL,
+                "Suspend function ''{0}'' should be called only from a coroutine or another suspend function",
+                SYMBOL
+            )
+            map.put(
+                ILLEGAL_SUSPEND_PROPERTY_ACCESS,
+                "Suspend property ''{0}'' should be accessed only from a coroutine or suspend function",
+                SYMBOL
+            )
+
            // Extended checkers group
            map.put(REDUNDANT_VISIBILITY_MODIFIER, "Redundant visibility modifier")
            map.put(REDUNDANT_MODALITY_MODIFIER, "Redundant modality modifier")
@@ -1,8 +0,0 @@
-// !LANGUAGE: +Coroutines
-// SKIP_TXT
-
-import kotlin.reflect.KSuspendFunction0
-
-fun test(c: KSuspendFunction0<Unit>) {
-    c()
-}
@@ -1,3 +1,4 @@
+// FIR_IDENTICAL
 // !LANGUAGE: +Coroutines
 // SKIP_TXT

@@ -1,19 +0,0 @@
-// !DIAGNOSTICS: -UNUSED_PARAMETER
-
-fun foo(x: () -> Int) {}
-fun foo(x: suspend () -> Int) {}
-
-fun usualCall(): Int = 42
-suspend fun suspendCall(): Int = 42
-
-// it's important to have ambiguity in these cases to introduce overload resolution by suspend modifier in future
-fun test1() {
-    <!OVERLOAD_RESOLUTION_AMBIGUITY!>foo<!> { usualCall() }
-    <!OVERLOAD_RESOLUTION_AMBIGUITY!>foo<!> { suspendCall() }
-}
-
-// candidate without suspend conversions is more specific
-fun test2(f: () -> Int, g: suspend () -> Int) {
-    foo(f)
-    foo(g)
-}
@@ -1,3 +1,4 @@
+// FIR_IDENTICAL
 // !DIAGNOSTICS: -UNUSED_PARAMETER

 fun foo(x: () -> Int) {}
@@ -1,29 +0,0 @@
-// !LANGUAGE: +UnrestrictedBuilderInference
-// !DIAGNOSTICS: -UNUSED_PARAMETER
-
-class Builder<T> {
-    suspend fun add(t: T) {}
-}
-
-fun <S> build(g: suspend Builder<S>.() -> Unit): List<S> = TODO()
-fun <S> wrongBuild(g: Builder<S>.() -> Unit): List<S> = TODO()
-
-fun <S> Builder<S>.extensionAdd(s: S) {}
-
-suspend fun <S> Builder<S>.safeExtensionAdd(s: S) {}
-
-val member = <!NEW_INFERENCE_NO_INFORMATION_FOR_PARAMETER!>build<!> {
-    add(42)
-}
-
-val memberWithoutAnn = <!NEW_INFERENCE_NO_INFORMATION_FOR_PARAMETER!>wrongBuild<!> {
-    add(42)
-}
-
-val extension = <!NEW_INFERENCE_NO_INFORMATION_FOR_PARAMETER!>build<!> {
-    extensionAdd("foo")
-}
-
-val safeExtension = <!NEW_INFERENCE_NO_INFORMATION_FOR_PARAMETER!>build<!> {
-    safeExtensionAdd("foo")
-}
@@ -1,3 +1,4 @@
+// FIR_IDENTICAL
 // !LANGUAGE: +UnrestrictedBuilderInference
 // !DIAGNOSTICS: -UNUSED_PARAMETER

@@ -1,53 +0,0 @@
-// !DIAGNOSTICS: -UNUSED_PARAMETER
-// SKIP_TXT
-
-import kotlin.coroutines.*
-
-fun ordinal() {
-    kotlin.coroutines.coroutineContext
-    coroutineContext
-}
-
-suspend fun named() {
-    kotlin.coroutines.coroutineContext
-    coroutineContext
-}
-
-class A {
-    val coroutineContextNew = kotlin.coroutines.coroutineContext
-    val context = coroutineContext
-}
-
-class Controller {
-    fun ordinal() {
-        kotlin.coroutines.coroutineContext
-        coroutineContext
-    }
-
-    suspend fun named() {
-        kotlin.coroutines.coroutineContext
-        coroutineContext
-    }
-
-    suspend fun severalArgs(s: String, a: Any) {
-        kotlin.coroutines.coroutineContext
-        coroutineContext
-    }
-}
-
-fun builder(c: () -> CoroutineContext) = {}
-fun builderSuspend(c: suspend () -> CoroutineContext) = {}
-
-fun builderSeveralArgs(c: (Int, Int, Int) -> CoroutineContext) = {}
-fun builderSuspendSeveralArgs(c: suspend (Int, Int, Int) -> CoroutineContext) = {}
-
-fun test() {
-    builder { kotlin.coroutines.coroutineContext }
-    builder { coroutineContext }
-    builderSuspend { kotlin.coroutines.coroutineContext }
-    builderSuspend { coroutineContext }
-    builderSeveralArgs { _, _, _ -> kotlin.coroutines.coroutineContext }
-    builderSeveralArgs { _, _, _ -> coroutineContext }
-    builderSuspendSeveralArgs { _, _, _ -> kotlin.coroutines.coroutineContext }
-    builderSuspendSeveralArgs { _, _, _ -> coroutineContext }
-}
@@ -1,3 +1,4 @@
+// FIR_IDENTICAL
 // !DIAGNOSTICS: -UNUSED_PARAMETER
 // SKIP_TXT

@@ -1,22 +0,0 @@
-import Host.bar
-
-object Host {
-    suspend fun bar() {}
-}
-
-suspend fun foo() {}
-
-fun noSuspend() {
-    foo()
-    bar()
-}
-
-class A {
-    init {
-        foo()
-        bar()
-    }
-}
-
-val x = foo()
-val y = bar()
@@ -1,3 +1,4 @@
+// FIR_IDENTICAL
 import Host.bar

 object Host {
@@ -1,5 +1,5 @@
 fun bar(d: Delegate): String {
-    val x: String by d
+    val x: String by <!ILLEGAL_SUSPEND_FUNCTION_CALL!>d<!>
    return x
 }

@@ -1,27 +0,0 @@
-// !USE_EXPERIMENTAL: kotlin.RequiresOptIn
-// !LANGUAGE: +NewInference
-
-import kotlin.experimental.ExperimentalTypeInference
-
-interface In<in E> {
-    fun send(element: E)
-}
-
-class InImpl<E> : In<E> {
-    override fun send(element: E) {}
-}
-
-@OptIn(ExperimentalTypeInference::class)
-public fun <T> builder(@BuilderInference block: In<T>.() -> Unit) {
-    InImpl<T>().block()
-}
-
-suspend fun yield() {}
-
-fun test() {
-    builder {
-        send(run {
-            yield() // No error but `yield` is not inside "suspension" context actually
-        })
-    }
-}
@@ -1,3 +1,4 @@
+// FIR_IDENTICAL
 // !USE_EXPERIMENTAL: kotlin.RequiresOptIn
 // !LANGUAGE: +NewInference

@@ -28,6 +28,6 @@ suspend fun calculate() = "OK"

 fun box() {
    test {
-        calculate()
+        <!ILLEGAL_SUSPEND_FUNCTION_CALL!>calculate<!>()
    }
 }
@@ -18,7 +18,7 @@ interface SuspendRunnable {
 // suspend calls possible inside lambda matching to the parameter

 inline fun test(crossinline c: suspend () -> Unit)  {
-    c()
+    <!ILLEGAL_SUSPEND_FUNCTION_CALL!>c<!>()
    val o = object : SuspendRunnable {
        override suspend fun run() {
            c()
@@ -27,6 +27,6 @@ suspend fun calculate() = "OK"

 fun box() {
    test {
-        calculate()
+        <!ILLEGAL_SUSPEND_FUNCTION_CALL!>calculate<!>()
    }
 }
@@ -1,37 +0,0 @@
-// !DIAGNOSTICS: -UNUSED_VARIABLE -UNUSED_PARAMETER -NOTHING_TO_INLINE
-// SKIP_TXT
-// WITH_COROUTINES
-import kotlin.coroutines.*
-import kotlin.coroutines.intrinsics.*
-import helpers.*
-
-interface SuspendRunnable {
-    suspend fun run()
-}
-
-// Function is NOT suspend
-// parameter is noinline
-// parameter is suspend
-// Block is NOT allowed to be called inside the body of owner inline function
-// Block is allowed to be called from nested classes/lambdas (as common crossinlines)
-// It is possible to call startCoroutine on the parameter
-// suspend calls possible inside lambda matching to the parameter
-
-inline fun test(noinline c: suspend () -> Unit)  {
-    c()
-    val o = object : SuspendRunnable {
-        override suspend fun run() {
-            c()
-        }
-    }
-    val l: suspend () -> Unit = { c() }
-    c.startCoroutine(EmptyContinuation)
-}
-
-suspend fun calculate() = "OK"
-
-fun box() {
-    test {
-        calculate()
-    }
-}
@@ -1,3 +1,4 @@
+// FIR_IDENTICAL
 // !DIAGNOSTICS: -UNUSED_VARIABLE -UNUSED_PARAMETER -NOTHING_TO_INLINE
 // SKIP_TXT
 // WITH_COROUTINES
@@ -18,7 +18,7 @@ interface SuspendRunnable {
 // suspend calls possible inside lambda matching to the parameter

 inline fun test(c: suspend () -> Unit) {
-    c()
+    <!ILLEGAL_SUSPEND_FUNCTION_CALL!>c<!>()
    val o = object: SuspendRunnable {
        override suspend fun run() {
            c()
@@ -1,10 +0,0 @@
-suspend fun foo1(q: suspend () -> Unit) = q()
-suspend fun foo2(x: suspend (Int) -> String) = x(1)
-
-
-suspend fun foo3(y: suspend String.(Int) -> Double) = "".y(1)
-suspend fun String.foo4(y: suspend String.(Int) -> Double) = "".y(1)
-
-fun noSuspend(x: suspend (Int) -> String) {
-    x(1)
-}
@@ -1,3 +1,4 @@
+// FIR_IDENTICAL
 suspend fun foo1(q: suspend () -> Unit) = q()
 suspend fun foo2(x: suspend (Int) -> String) = x(1)

@@ -1,2 +0,0 @@
-fun test1(sfn: suspend () -> Unit) = sfn()
-fun test2(sfn: suspend () -> Unit) = sfn.invoke()
@@ -1,2 +1,3 @@
+// FIR_IDENTICAL
 fun test1(sfn: suspend () -> Unit) = <!ILLEGAL_SUSPEND_FUNCTION_CALL!>sfn<!>()
 fun test2(sfn: suspend () -> Unit) = sfn.<!ILLEGAL_SUSPEND_FUNCTION_CALL!>invoke<!>()
@@ -1,9 +0,0 @@
-val test1: (suspend () -> Unit)? = null
-val test2: <!WRONG_MODIFIER_TARGET!>suspend<!> (() -> Unit)? = null
-val test3: <!WRONG_MODIFIER_TARGET!>suspend<!> ( (() -> Unit)? ) = null
-
-fun foo() {
-    test1?.invoke()
-    test2?.invoke()
-    test3?.invoke()
-}
@@ -1,3 +1,4 @@
+// FIR_IDENTICAL
 val test1: (suspend () -> Unit)? = null
 val test2: <!WRONG_MODIFIER_TARGET!>suspend<!> (() -> Unit)? = null
 val test3: <!WRONG_MODIFIER_TARGET!>suspend<!> ( (() -> Unit)? ) = null
@@ -3142,6 +3142,20 @@ internal val KT_DIAGNOSTIC_CONVERTER = KtDiagnosticConverterBuilder.buildConvert
            token,
        )
    }
+    add(FirErrors.ILLEGAL_SUSPEND_FUNCTION_CALL) { firDiagnostic ->
+        IllegalSuspendFunctionCallImpl(
+            firSymbolBuilder.buildSymbol(firDiagnostic.a.fir),
+            firDiagnostic as FirPsiDiagnostic,
+            token,
+        )
+    }
+    add(FirErrors.ILLEGAL_SUSPEND_PROPERTY_ACCESS) { firDiagnostic ->
+        IllegalSuspendPropertyAccessImpl(
+            firSymbolBuilder.buildSymbol(firDiagnostic.a.fir),
+            firDiagnostic as FirPsiDiagnostic,
+            token,
+        )
+    }
    add(FirJvmErrors.CONFLICTING_JVM_DECLARATIONS) { firDiagnostic ->
        ConflictingJvmDeclarationsImpl(
            firDiagnostic as FirPsiDiagnostic,
@@ -2197,6 +2197,16 @@ sealed class KtFirDiagnostic<PSI : PsiElement> : KtDiagnosticWithPsi<PSI> {
        override val diagnosticClass get() = OperatorRenamedOnImport::class
    }

+    abstract class IllegalSuspendFunctionCall : KtFirDiagnostic<PsiElement>() {
+        override val diagnosticClass get() = IllegalSuspendFunctionCall::class
+        abstract val suspendCallable: KtSymbol
+    }
+
+    abstract class IllegalSuspendPropertyAccess : KtFirDiagnostic<PsiElement>() {
+        override val diagnosticClass get() = IllegalSuspendPropertyAccess::class
+        abstract val suspendCallable: KtSymbol
+    }
+
    abstract class ConflictingJvmDeclarations : KtFirDiagnostic<PsiElement>() {
        override val diagnosticClass get() = ConflictingJvmDeclarations::class
    }
@@ -3545,6 +3545,22 @@ internal class OperatorRenamedOnImportImpl(
    override val firDiagnostic: FirPsiDiagnostic by weakRef(firDiagnostic)
 }

+internal class IllegalSuspendFunctionCallImpl(
+    override val suspendCallable: KtSymbol,
+    firDiagnostic: FirPsiDiagnostic,
+    override val token: ValidityToken,
+) : KtFirDiagnostic.IllegalSuspendFunctionCall(), KtAbstractFirDiagnostic<PsiElement> {
+    override val firDiagnostic: FirPsiDiagnostic by weakRef(firDiagnostic)
+}
+
+internal class IllegalSuspendPropertyAccessImpl(
+    override val suspendCallable: KtSymbol,
+    firDiagnostic: FirPsiDiagnostic,
+    override val token: ValidityToken,
+) : KtFirDiagnostic.IllegalSuspendPropertyAccess(), KtAbstractFirDiagnostic<PsiElement> {
+    override val firDiagnostic: FirPsiDiagnostic by weakRef(firDiagnostic)
+}
+
 internal class ConflictingJvmDeclarationsImpl(
    firDiagnostic: FirPsiDiagnostic,
    override val token: ValidityToken,