From b88913af1d40d5ccee3380b0cbfff67deab0d40c Mon Sep 17 00:00:00 2001 From: Mark Punzalan Date: Fri, 9 Apr 2021 08:19:49 +0000 Subject: [PATCH] FIR checker: Report WRONG_MODIFIER_TARGET for `suspend` on non-functional types. --- .../checkers/declaration/FirModifierUtils.kt | 5 +- .../FirSuspendOnInvalidTypeChecker.kt | 59 +++++++++++++++++++ .../fir/checkers/CommonDeclarationCheckers.kt | 1 + .../modifierApplicability.fir.kt | 13 ++-- .../modifierApplicability.kt | 3 +- .../modifierApplicability.txt | 4 +- .../nullableSuspendFunction.fir.kt | 4 +- 7 files changed, 76 insertions(+), 13 deletions(-) create mode 100644 compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirSuspendOnInvalidTypeChecker.kt diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirModifierUtils.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirModifierUtils.kt index 3f36c99f680..feb646d1012 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirModifierUtils.kt +++ b/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() diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirSuspendOnInvalidTypeChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirSuspendOnInvalidTypeChecker.kt new file mode 100644 index 00000000000..c537f2d2335 --- /dev/null +++ b/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 + ) + } + } + }) + } +} \ No newline at end of file diff --git a/compiler/fir/entrypoint/src/org/jetbrains/kotlin/fir/checkers/CommonDeclarationCheckers.kt b/compiler/fir/entrypoint/src/org/jetbrains/kotlin/fir/checkers/CommonDeclarationCheckers.kt index c546ed99cfe..fb2ab5655ca 100644 --- a/compiler/fir/entrypoint/src/org/jetbrains/kotlin/fir/checkers/CommonDeclarationCheckers.kt +++ b/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 diff --git a/compiler/testData/diagnostics/testsWithStdLib/coroutines/suspendFunctionType/modifierApplicability.fir.kt b/compiler/testData/diagnostics/testsWithStdLib/coroutines/suspendFunctionType/modifierApplicability.fir.kt index f5cd4c8888b..f0ea930bea2 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/coroutines/suspendFunctionType/modifierApplicability.fir.kt +++ b/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 -typealias Test4 = suspend Action +typealias Test3 = suspend Function0 +typealias Test4 = suspend Action typealias Test5 = List Unit> -typealias Test6 = suspend List<() -> Unit> -typealias Test7 = suspend SAM -typealias Test8 = suspend kotlin.coroutines.SuspendFunction0 +typealias Test6 = suspend List<() -> Unit> +typealias Test7 = suspend SAM +typealias Test8 = suspend kotlin.coroutines.SuspendFunction0 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: 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 diff --git a/compiler/testData/diagnostics/testsWithStdLib/coroutines/suspendFunctionType/modifierApplicability.kt b/compiler/testData/diagnostics/testsWithStdLib/coroutines/suspendFunctionType/modifierApplicability.kt index 29ca315a5f2..a53d7511a3b 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/coroutines/suspendFunctionType/modifierApplicability.kt +++ b/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({ val x: suspend String? = null; "" }()) suspend () -> Unit)? interface Supertype1 : suspend () -> Unit { @@ -32,6 +33,6 @@ interface Supertype2 : suspend String.() - } @Target(AnnotationTarget.TYPE) -annotation class A +annotation class A(val value: String = "") interface RS diff --git a/compiler/testData/diagnostics/testsWithStdLib/coroutines/suspendFunctionType/modifierApplicability.txt b/compiler/testData/diagnostics/testsWithStdLib/coroutines/suspendFunctionType/modifierApplicability.txt index 9b64d81285d..8303b04304c 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/coroutines/suspendFunctionType/modifierApplicability.txt +++ b/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 diff --git a/compiler/testData/diagnostics/testsWithStdLib/coroutines/suspendFunctionType/nullableSuspendFunction.fir.kt b/compiler/testData/diagnostics/testsWithStdLib/coroutines/suspendFunctionType/nullableSuspendFunction.fir.kt index b572f13731c..afbac3c9797 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/coroutines/suspendFunctionType/nullableSuspendFunction.fir.kt +++ b/compiler/testData/diagnostics/testsWithStdLib/coroutines/suspendFunctionType/nullableSuspendFunction.fir.kt @@ -1,6 +1,6 @@ val test1: (suspend () -> Unit)? = null -val test2: suspend (() -> Unit)? = null -val test3: suspend ( (() -> Unit)? ) = null +val test2: suspend (() -> Unit)? = null +val test3: suspend ( (() -> Unit)? ) = null fun foo() { test1?.invoke()