FIR checker: Report WRONG_MODIFIER_TARGET for suspend on

non-functional types.

compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirModifierUtils.kt

@@ -12,7 +12,6 @@ import com.intellij.util.diff.FlyweightCapableTreeStructure
 import org.jetbrains.kotlin.KtNodeTypes
 import org.jetbrains.kotlin.fir.*
 import org.jetbrains.kotlin.fir.analysis.checkers.getChildren
-import org.jetbrains.kotlin.fir.declarations.FirDeclaration
 import org.jetbrains.kotlin.lexer.KtModifierKeywordToken
 import org.jetbrains.kotlin.lexer.KtTokens
 import org.jetbrains.kotlin.psi.KtModifierList
@@ -95,6 +94,6 @@ internal fun FirSourceElement?.getModifierList(): FirModifierList? {
 
 internal operator fun FirModifierList?.contains(token: KtModifierKeywordToken): Boolean = this?.contains(token) == true
 
-internal fun FirDeclaration.getModifier(token: KtModifierKeywordToken): FirModifier<*>? = source.getModifierList()?.get(token)
+internal fun FirElement.getModifier(token: KtModifierKeywordToken): FirModifier<*>? = source.getModifierList()?.get(token)
 
-internal fun FirDeclaration.hasModifier(token: KtModifierKeywordToken): Boolean = token in source.getModifierList()
+internal fun FirElement.hasModifier(token: KtModifierKeywordToken): Boolean = token in source.getModifierList()

compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirSuspendOnInvalidTypeChecker.kt

@@ -0,0 +1,59 @@
+/*
+ * 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.declaration
+
+import org.jetbrains.kotlin.fir.FirElement
+import org.jetbrains.kotlin.fir.FirFakeSourceElementKind
+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.FirFile
+import org.jetbrains.kotlin.fir.types.FirFunctionTypeRef
+import org.jetbrains.kotlin.fir.types.FirResolvedTypeRef
+import org.jetbrains.kotlin.fir.types.FirTypeRefWithNullability
+import org.jetbrains.kotlin.fir.visitors.FirDefaultVisitorVoid
+import org.jetbrains.kotlin.lexer.KtTokens
+
+object FirSuspendOnInvalidTypeChecker : FirFileChecker() {
+    override fun check(declaration: FirFile, context: CheckerContext, reporter: DiagnosticReporter) {
+        // Traverse the entire file to look for source type refs (i.e., Fir(Dynamic|User|Function)TypeRef).
+        declaration.accept(object : FirDefaultVisitorVoid() {
+            override fun visitElement(element: FirElement) {
+                if (element.source?.kind is FirFakeSourceElementKind) return
+                element.acceptChildren(this)
+            }
+
+            override fun visitResolvedTypeRef(resolvedTypeRef: FirResolvedTypeRef) {
+                // We will only get to the source type refs by visiting delegatedTypeRef.
+                super.visitResolvedTypeRef(resolvedTypeRef)
+                resolvedTypeRef.delegatedTypeRef?.accept(this)
+            }
+
+            override fun visitTypeRefWithNullability(typeRefWithNullability: FirTypeRefWithNullability) {
+                // `suspend` is invalid for non-function types (i.e., FirDynamicTypeRef or FirUserTypeRef).
+                //
+                // It is also invalid for nullable function types, e.g., `suspend (() -> Int)?`.
+                // The correct way to denote a nullable suspend function type is `(suspend () -> Int)?`.
+                // In both cases, the source for the FirFunctionTypeRef is the TYPE_REFERENCE element.
+                // In the invalid case, the modifier list is in the source TYPE_REFERENCE element.
+                // But in the correct case, the modifier list is in the child NULLABLE_TYPE element.
+                // Therefore, if the FirFunctionTypeRef is marked nullable, it is invalid to HAVE the `suspend` modifier
+                // on the source element, even though it seems counter-intuitive.
+                val suspendModifier = typeRefWithNullability.getModifier(KtTokens.SUSPEND_KEYWORD) ?: return
+                if (typeRefWithNullability !is FirFunctionTypeRef || typeRefWithNullability.isMarkedNullable) {
+                    reporter.reportOn(
+                        suspendModifier.source,
+                        FirErrors.WRONG_MODIFIER_TARGET,
+                        suspendModifier.token,
+                        "non-functional type",
+                        context
+                    )
+                }
+            }
+        })
+    }
+}

compiler/fir/entrypoint/src/org/jetbrains/kotlin/fir/checkers/CommonDeclarationCheckers.kt

@@ -95,6 +95,7 @@ object CommonDeclarationCheckers : DeclarationCheckers() {
             FirKClassWithIncorrectTypeArgumentChecker,
             FirTopLevelFunctionsChecker,
             FirTopLevelPropertiesChecker,
+            FirSuspendOnInvalidTypeChecker,
         )
 
     override val controlFlowAnalyserCheckers: Set<FirControlFlowChecker>

compiler/testData/diagnostics/testsWithStdLib/coroutines/suspendFunctionType/modifierApplicability.fir.kt

@@ -6,12 +6,12 @@ interface SAM {
 
 typealias Test1 = suspend () -> Unit
 typealias Test2 = suspend Int.() -> Unit
-typealias Test3 = suspend Function0<Unit>
+typealias Test3 = <!WRONG_MODIFIER_TARGET!>suspend<!> Function0<Unit>
-typealias Test4 = suspend Action
+typealias Test4 = <!WRONG_MODIFIER_TARGET!>suspend<!> Action
 typealias Test5 = List<suspend () -> Unit>
-typealias Test6 = suspend List<() -> Unit>
+typealias Test6 = <!WRONG_MODIFIER_TARGET!>suspend<!> List<() -> Unit>
-typealias Test7 = suspend SAM
+typealias Test7 = <!WRONG_MODIFIER_TARGET!>suspend<!> SAM
-typealias Test8 = suspend kotlin.coroutines.SuspendFunction0<Unit>
+typealias Test8 = <!WRONG_MODIFIER_TARGET!>suspend<!> kotlin.coroutines.SuspendFunction0<Unit>
 typealias Test9 = suspend (() -> Unit) -> Unit
 typealias Test10 = suspend (suspend () -> Unit) -> Unit
 typealias Test11 = suspend () -> (suspend () -> Unit)
@@ -22,6 +22,7 @@ typealias Test15 = (@A() suspend () -> Unit)?
 typealias Test16 = (@A suspend () -> Unit)?
 typealias Test17 = @A suspend RS.() -> Unit
 typealias Test18 = (suspend () -> Unit)?
+typealias Test19 = (@A({ val x: <!WRONG_MODIFIER_TARGET!>suspend<!> String? = null; "" }()) suspend () -> Unit)?
 
 interface Supertype1 : suspend () -> Unit {
 
@@ -32,6 +33,6 @@ interface Supertype2 : suspend String.() -> Unit {
 }
 
 @Target(AnnotationTarget.TYPE)
-annotation class A
+annotation class A(val value: String = "")
 
 interface RS

compiler/testData/diagnostics/testsWithStdLib/coroutines/suspendFunctionType/modifierApplicability.kt

@@ -22,6 +22,7 @@ typealias Test15 = (@A() suspend () -> Unit)?
 typealias Test16 = (@A suspend () -> Unit)?
 typealias Test17 = @A suspend RS.() -> Unit
 typealias Test18 = (suspend () -> Unit)?
+typealias Test19 = (@A(<!ANNOTATION_ARGUMENT_MUST_BE_CONST!>{ val x: <!WRONG_MODIFIER_TARGET, WRONG_MODIFIER_TARGET!>suspend<!> String? = null; "" }()<!>) suspend () -> Unit)?
 
 interface Supertype1 : <!SUPERTYPE_IS_SUSPEND_FUNCTION_TYPE!>suspend () -> Unit<!> {
 
@@ -32,6 +33,6 @@ interface Supertype2 : <!SUPERTYPE_IS_SUSPEND_FUNCTION_TYPE!>suspend String.() -
 }
 
 @Target(AnnotationTarget.TYPE)
-annotation class A
+annotation class A(val value: String = "")
 
 interface RS

compiler/testData/diagnostics/testsWithStdLib/coroutines/suspendFunctionType/modifierApplicability.txt

@@ -1,7 +1,8 @@
 package
 
 @kotlin.annotation.Target(allowedTargets = {AnnotationTarget.TYPE}) public final annotation class A : kotlin.Annotation {
-    public constructor A()
+    public constructor A(/*0*/ value: kotlin.String = ...)
+    public final val value: kotlin.String
     public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
     public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
     public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
@@ -44,6 +45,7 @@ public typealias Test15 = (@A suspend () -> kotlin.Unit)?
 public typealias Test16 = (@A suspend () -> kotlin.Unit)?
 public typealias Test17 = (@A suspend RS.() -> kotlin.Unit)
 public typealias Test18 = (suspend () -> kotlin.Unit)?
+public typealias Test19 = (@A suspend () -> kotlin.Unit)?
 public typealias Test2 = suspend kotlin.Int.() -> kotlin.Unit
 public typealias Test3 = () -> kotlin.Unit
 public typealias Test4 = Action

compiler/testData/diagnostics/testsWithStdLib/coroutines/suspendFunctionType/nullableSuspendFunction.fir.kt

@@ -1,6 +1,6 @@
 val test1: (suspend () -> Unit)? = null
-val test2: suspend (() -> Unit)? = null
+val test2: <!WRONG_MODIFIER_TARGET!>suspend<!> (() -> Unit)? = null
-val test3: suspend ( (() -> Unit)? ) = null
+val test3: <!WRONG_MODIFIER_TARGET!>suspend<!> ( (() -> Unit)? ) = null
 
 fun foo() {
     test1?.invoke()