From 9995438b37b9847d0fffc316a7547395bf04fa13 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Fri, 27 Apr 2018 19:01:17 +0200 Subject: [PATCH] Forbid non-intended usages of Experimental and markers Experimental and UseExperimental can only be used as annotations or qualifiers (to allow "Experimental.Level.*"); experimental markers can only be used as annotations or qualifiers, or as left-hand side of a ::class literal in arguments to UseExperimental/WasExperimental. This is needed because we're not yet sure of the design of this feature and would like to retain the possibility to drop these declarations (Experimental, UseExperimental) altogether. If they were going to be used as types, it would be problematic because we can't simply delete something from stdlib, should deprecate it first. With this change, these declarations can only be used if the user has opted into using the experimental API somehow (for example, with `-Xuse-experimental=kotlin.Experimental`), so we won't stop conforming to our deprecation policy if we decide to remove these declarations in the future --- .../jetbrains/kotlin/diagnostics/Errors.java | 4 ++ .../rendering/DefaultErrorMessages.java | 3 + .../checkers/ExperimentalUsageChecker.kt | 56 ++++++++++++++++++- .../experimental/incorrectUseExperimental.kt | 4 +- .../experimental/incorrectUseExperimental.txt | 9 ++- .../experimental/usageNotAsAnnotation.kt | 54 ++++++++++++++++++ .../experimental/usageNotAsAnnotation.txt | 38 +++++++++++++ .../experimental/wasExperimental.kt | 8 +-- .../DiagnosticsTestWithStdLibGenerated.java | 5 ++ ...ticsTestWithStdLibUsingJavacGenerated.java | 5 ++ 10 files changed, 177 insertions(+), 9 deletions(-) create mode 100644 compiler/testData/diagnostics/testsWithStdLib/experimental/usageNotAsAnnotation.kt create mode 100644 compiler/testData/diagnostics/testsWithStdLib/experimental/usageNotAsAnnotation.txt diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java index f916e01e728..617fe35f263 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java @@ -241,6 +241,10 @@ public interface Errors { DiagnosticFactory2 EXPERIMENTAL_OVERRIDE_ERROR = DiagnosticFactory2.create(ERROR); DiagnosticFactory0 EXPERIMENTAL_IS_NOT_ENABLED = DiagnosticFactory0.create(ERROR); + DiagnosticFactory0 EXPERIMENTAL_CAN_ONLY_BE_USED_AS_ANNOTATION = DiagnosticFactory0.create(ERROR); + DiagnosticFactory0 + EXPERIMENTAL_MARKER_CAN_ONLY_BE_USED_AS_ANNOTATION_OR_ARGUMENT_IN_USE_EXPERIMENTAL = DiagnosticFactory0.create(ERROR); + DiagnosticFactory0 USE_EXPERIMENTAL_WITHOUT_ARGUMENTS = DiagnosticFactory0.create(WARNING); DiagnosticFactory1 USE_EXPERIMENTAL_ARGUMENT_IS_NOT_MARKER = DiagnosticFactory1.create(WARNING); DiagnosticFactory1 EXPERIMENTAL_ANNOTATION_WITH_WRONG_TARGET = 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 66d814d956c..4cc37ef2bdb 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java @@ -147,6 +147,9 @@ public class DefaultErrorMessages { MAP.put(EXPERIMENTAL_OVERRIDE_ERROR, "This declaration overrides experimental member of supertype ''{1}'' and must be annotated with ''@{0}''", TO_STRING, NAME); MAP.put(EXPERIMENTAL_IS_NOT_ENABLED, "This class can only be used with the compiler argument '-Xuse-experimental=kotlin.Experimental'"); + MAP.put(EXPERIMENTAL_CAN_ONLY_BE_USED_AS_ANNOTATION, "This class can only be used as an annotation"); + MAP.put(EXPERIMENTAL_MARKER_CAN_ONLY_BE_USED_AS_ANNOTATION_OR_ARGUMENT_IN_USE_EXPERIMENTAL, "This class can only be used as an annotation or as an argument to @UseExperimental"); + MAP.put(USE_EXPERIMENTAL_WITHOUT_ARGUMENTS, "@UseExperimental without any arguments has no effect"); MAP.put(USE_EXPERIMENTAL_ARGUMENT_IS_NOT_MARKER, "Annotation ''{0}'' is not an experimental API marker, therefore its usage in @UseExperimental is ignored", TO_STRING); MAP.put(EXPERIMENTAL_ANNOTATION_WITH_WRONG_TARGET, "Experimental annotation cannot be used on the following code elements: {0}. Please remove these targets", STRING); diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/ExperimentalUsageChecker.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/ExperimentalUsageChecker.kt index 8ef86c387ce..73d03f49eb7 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/ExperimentalUsageChecker.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/ExperimentalUsageChecker.kt @@ -26,9 +26,9 @@ import org.jetbrains.kotlin.diagnostics.Errors import org.jetbrains.kotlin.incremental.components.NoLookupLocation import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.name.Name -import org.jetbrains.kotlin.psi.KtAnnotated -import org.jetbrains.kotlin.psi.KtDeclaration -import org.jetbrains.kotlin.psi.KtNamedDeclaration +import org.jetbrains.kotlin.psi.* +import org.jetbrains.kotlin.psi.psiUtil.getParentOfType +import org.jetbrains.kotlin.psi.psiUtil.getTopmostParentQualifiedExpressionForSelector import org.jetbrains.kotlin.resolve.* import org.jetbrains.kotlin.resolve.calls.checkers.CallChecker import org.jetbrains.kotlin.resolve.calls.checkers.CallCheckerContext @@ -245,13 +245,63 @@ class ExperimentalUsageChecker(project: Project) : CallChecker { val experimentalities = targetDescriptor.loadExperimentalities(moduleAnnotationsResolver) reportNotAcceptedExperimentalities(experimentalities, element, context) + + val targetClass = when (targetDescriptor) { + is ClassDescriptor -> targetDescriptor + is TypeAliasDescriptor -> targetDescriptor.classDescriptor + else -> null + } + if (targetClass != null && targetClass.loadExperimentalityForMarkerAnnotation() != null) { + if (!element.isUsageAsAnnotationOrQualifier() && + !element.isUsageAsUseExperimentalArgument(context.trace.bindingContext) + ) { + context.trace.report( + Errors.EXPERIMENTAL_MARKER_CAN_ONLY_BE_USED_AS_ANNOTATION_OR_ARGUMENT_IN_USE_EXPERIMENTAL.on(element) + ) + } + } } private fun checkUsageOfKotlinExperimentalOrUseExperimental(element: PsiElement, context: CheckerContext) { if (EXPERIMENTAL_FQ_NAME.asString() !in context.languageVersionSettings.getFlag(AnalysisFlag.useExperimental)) { context.trace.report(Errors.EXPERIMENTAL_IS_NOT_ENABLED.on(element)) } + + if (!element.isUsageAsAnnotationOrQualifier()) { + context.trace.report(Errors.EXPERIMENTAL_CAN_ONLY_BE_USED_AS_ANNOTATION.on(element)) + } } + + private fun PsiElement.isUsageAsAnnotationOrQualifier(): Boolean { + val parent = parent + + if (parent is KtUserType) { + return parent.parent is KtTypeReference && + parent.parent.parent is KtConstructorCalleeExpression && + parent.parent.parent.parent is KtAnnotationEntry + } + + if (this is KtSimpleNameExpression) { + val qualifier = getTopmostParentQualifiedExpressionForSelector() ?: this + if ((qualifier.parent as? KtDotQualifiedExpression)?.receiverExpression == qualifier) { + return true + } + } + + if (getParentOfType(true) != null) return true + + return false + } + + private fun PsiElement.isUsageAsUseExperimentalArgument(bindingContext: BindingContext): Boolean = + parent is KtClassLiteralExpression && + parent.parent is KtValueArgument && + parent.parent.parent is KtValueArgumentList && + parent.parent.parent.parent.let { entry -> + entry is KtAnnotationEntry && bindingContext.get(BindingContext.ANNOTATION, entry)?.let { annotation -> + annotation.fqName == USE_EXPERIMENTAL_FQ_NAME || annotation.fqName == WAS_EXPERIMENTAL_FQ_NAME + } == true + } } class Overrides(project: Project) : DeclarationChecker { diff --git a/compiler/testData/diagnostics/testsWithStdLib/experimental/incorrectUseExperimental.kt b/compiler/testData/diagnostics/testsWithStdLib/experimental/incorrectUseExperimental.kt index 5c022771e41..9d10de7a486 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/experimental/incorrectUseExperimental.kt +++ b/compiler/testData/diagnostics/testsWithStdLib/experimental/incorrectUseExperimental.kt @@ -1,7 +1,9 @@ // !USE_EXPERIMENTAL: kotlin.Experimental +annotation class NotAMarker + @UseExperimental fun f1() {} -@UseExperimental(UseExperimental::class) +@UseExperimental(NotAMarker::class) fun f2() {} diff --git a/compiler/testData/diagnostics/testsWithStdLib/experimental/incorrectUseExperimental.txt b/compiler/testData/diagnostics/testsWithStdLib/experimental/incorrectUseExperimental.txt index b18c745498d..c210e94bb87 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/experimental/incorrectUseExperimental.txt +++ b/compiler/testData/diagnostics/testsWithStdLib/experimental/incorrectUseExperimental.txt @@ -1,4 +1,11 @@ package @kotlin.UseExperimental(markerClass = {}) public fun f1(): kotlin.Unit -@kotlin.UseExperimental(markerClass = {kotlin.UseExperimental::class}) public fun f2(): kotlin.Unit +@kotlin.UseExperimental(markerClass = {NotAMarker::class}) public fun f2(): kotlin.Unit + +public final annotation class NotAMarker : kotlin.Annotation { + public constructor NotAMarker() + 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 +} diff --git a/compiler/testData/diagnostics/testsWithStdLib/experimental/usageNotAsAnnotation.kt b/compiler/testData/diagnostics/testsWithStdLib/experimental/usageNotAsAnnotation.kt new file mode 100644 index 00000000000..e199348f55d --- /dev/null +++ b/compiler/testData/diagnostics/testsWithStdLib/experimental/usageNotAsAnnotation.kt @@ -0,0 +1,54 @@ +// !USE_EXPERIMENTAL: kotlin.Experimental +// !DIAGNOSTICS: -UNUSED_PARAMETER + +package test + +import kotlin.reflect.KClass + +// Usages in import should be OK +import kotlin.Experimental.Level.* +import kotlin.Experimental.Level +import kotlin.Experimental + +// Usages with FQ names should be OK + +@kotlin.Experimental(kotlin.Experimental.Level.ERROR) +annotation class M + + +// Usages as types should be errors + +fun f1(e: Experimental) {} +fun f2(u: UseExperimental?) {} + +typealias Experimental0 = Experimental +typealias UseExperimental0 = UseExperimental +fun f3(e: Experimental0 /* TODO */) {} +fun f4(u: UseExperimental0 /* TODO */) {} + + +// Usages as ::class literals should be errors + +annotation class VarargKClasses(vararg val k: KClass<*>) + +@VarargKClasses( + Experimental::class, + UseExperimental::class, + kotlin.Experimental::class, + kotlin.UseExperimental::class +) +fun f5() {} + + +// Usages of markers as types should be errors + +@Experimental +annotation class Marker + +fun f6(m: Marker) {} +fun f7(): List<Marker>? = null +fun f8(): test.Marker? = null + +typealias Marker0 = Marker + +fun f9(m: Marker0) {} diff --git a/compiler/testData/diagnostics/testsWithStdLib/experimental/usageNotAsAnnotation.txt b/compiler/testData/diagnostics/testsWithStdLib/experimental/usageNotAsAnnotation.txt new file mode 100644 index 00000000000..67662664779 --- /dev/null +++ b/compiler/testData/diagnostics/testsWithStdLib/experimental/usageNotAsAnnotation.txt @@ -0,0 +1,38 @@ +package + +package test { + public fun f1(/*0*/ e: kotlin.Experimental): kotlin.Unit + public fun f2(/*0*/ u: kotlin.UseExperimental?): kotlin.Unit + public fun f3(/*0*/ e: test.Experimental0 /* = kotlin.Experimental */): kotlin.Unit + public fun f4(/*0*/ u: test.UseExperimental0 /* = kotlin.UseExperimental */): kotlin.Unit + @test.VarargKClasses(k = {kotlin.Experimental::class, kotlin.UseExperimental::class, kotlin.Experimental::class, kotlin.UseExperimental::class}) public fun f5(): kotlin.Unit + public fun f6(/*0*/ m: test.Marker): kotlin.Unit + public fun f7(): kotlin.collections.List? + public fun f8(): test.Marker? + public fun f9(/*0*/ m: test.Marker0 /* = test.Marker */): kotlin.Unit + + @kotlin.Experimental(level = Level.ERROR) public final annotation class M : kotlin.Annotation { + public constructor M() + 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 + } + + @kotlin.Experimental public final annotation class Marker : kotlin.Annotation { + public constructor Marker() + 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 + } + + public final annotation class VarargKClasses : kotlin.Annotation { + public constructor VarargKClasses(/*0*/ vararg k: kotlin.reflect.KClass<*> /*kotlin.Array>*/) + public final val k: kotlin.Array> + 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 + } + public typealias Experimental0 = kotlin.Experimental + public typealias Marker0 = test.Marker + public typealias UseExperimental0 = kotlin.UseExperimental +} diff --git a/compiler/testData/diagnostics/testsWithStdLib/experimental/wasExperimental.kt b/compiler/testData/diagnostics/testsWithStdLib/experimental/wasExperimental.kt index 5df8444b7be..b50c25ef9ad 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/experimental/wasExperimental.kt +++ b/compiler/testData/diagnostics/testsWithStdLib/experimental/wasExperimental.kt @@ -1,6 +1,6 @@ // !API_VERSION: 1.2 // !USE_EXPERIMENTAL: kotlin.Experimental -// !DIAGNOSTICS: -INVISIBLE_MEMBER -INVISIBLE_REFERENCE -NEWER_VERSION_IN_SINCE_KOTLIN -UNUSED_PARAMETER +// !DIAGNOSTICS: -INVISIBLE_MEMBER -INVISIBLE_REFERENCE -NEWER_VERSION_IN_SINCE_KOTLIN @SinceKotlin("1.3") fun newPublishedFun() {} @@ -21,7 +21,7 @@ val newValExperimentalInThePast = "" @WasExperimental(Marker::class) class NewClassExperimentalInThePast -fun use1(c: NewClassExperimentalInThePast) { +fun use1() { newPublishedFun() newFunExperimentalInThePast() newValExperimentalInThePast @@ -29,7 +29,7 @@ fun use1(c: NewClassExperimentalInThePast) { } @UseExperimental(Marker::class) -fun use2(c: NewClassExperimentalInThePast) { +fun use2() { newPublishedFun() newFunExperimentalInThePast() newValExperimentalInThePast @@ -37,7 +37,7 @@ fun use2(c: NewClassExperimentalInThePast) { } @Marker -fun use3(c: NewClassExperimentalInThePast) { +fun use3() { newPublishedFun() newFunExperimentalInThePast() newValExperimentalInThePast diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestWithStdLibGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestWithStdLibGenerated.java index 1d94eb05276..55e905ed10b 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestWithStdLibGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestWithStdLibGenerated.java @@ -2172,6 +2172,11 @@ public class DiagnosticsTestWithStdLibGenerated extends AbstractDiagnosticsTestW runTest("compiler/testData/diagnostics/testsWithStdLib/experimental/typealias.kt"); } + @TestMetadata("usageNotAsAnnotation.kt") + public void testUsageNotAsAnnotation() throws Exception { + runTest("compiler/testData/diagnostics/testsWithStdLib/experimental/usageNotAsAnnotation.kt"); + } + @TestMetadata("useExperimentalOnFile.kt") public void testUseExperimentalOnFile() throws Exception { runTest("compiler/testData/diagnostics/testsWithStdLib/experimental/useExperimentalOnFile.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsTestWithStdLibUsingJavacGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsTestWithStdLibUsingJavacGenerated.java index 09ff9829b24..e953802d77b 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsTestWithStdLibUsingJavacGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsTestWithStdLibUsingJavacGenerated.java @@ -2172,6 +2172,11 @@ public class DiagnosticsTestWithStdLibUsingJavacGenerated extends AbstractDiagno runTest("compiler/testData/diagnostics/testsWithStdLib/experimental/typealias.kt"); } + @TestMetadata("usageNotAsAnnotation.kt") + public void testUsageNotAsAnnotation() throws Exception { + runTest("compiler/testData/diagnostics/testsWithStdLib/experimental/usageNotAsAnnotation.kt"); + } + @TestMetadata("useExperimentalOnFile.kt") public void testUseExperimentalOnFile() throws Exception { runTest("compiler/testData/diagnostics/testsWithStdLib/experimental/useExperimentalOnFile.kt");