From 143c3ccb95f93299233ade88c24b2fa2b9b29abf Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Fri, 4 May 2018 17:06:26 +0200 Subject: [PATCH] Disallow usages of experimental markers as qualifiers Only allow explicit imports of markers, but not of nested classes declared in them --- .../checkers/ExperimentalUsageChecker.kt | 13 +++++----- .../experimental/usageNotAsAnnotation.kt | 25 ++++++++++++++++++- .../experimental/usageNotAsAnnotation.txt | 16 ++++++++++++ 3 files changed, 47 insertions(+), 7 deletions(-) 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 89ead0f60ca..c485a8b5405 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/ExperimentalUsageChecker.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/ExperimentalUsageChecker.kt @@ -26,7 +26,6 @@ import org.jetbrains.kotlin.incremental.components.NoLookupLocation import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.name.Name 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 @@ -246,7 +245,7 @@ class ExperimentalUsageChecker(project: Project) : CallChecker { else -> null } if (targetClass != null && targetClass.loadExperimentalityForMarkerAnnotation() != null) { - if (!element.isUsageAsAnnotationOrQualifier() && + if (!element.isUsageAsAnnotationOrImport() && !element.isUsageAsUseExperimentalArgument(context.trace.bindingContext) ) { context.trace.report( @@ -261,12 +260,12 @@ class ExperimentalUsageChecker(project: Project) : CallChecker { context.trace.report(Errors.EXPERIMENTAL_IS_NOT_ENABLED.on(element)) } - if (!element.isUsageAsAnnotationOrQualifier()) { + if (!element.isUsageAsAnnotationOrImport() && !element.isUsageAsQualifier()) { context.trace.report(Errors.EXPERIMENTAL_CAN_ONLY_BE_USED_AS_ANNOTATION.on(element)) } } - private fun PsiElement.isUsageAsAnnotationOrQualifier(): Boolean { + private fun PsiElement.isUsageAsAnnotationOrImport(): Boolean { val parent = parent if (parent is KtUserType) { @@ -275,6 +274,10 @@ class ExperimentalUsageChecker(project: Project) : CallChecker { parent.parent.parent.parent is KtAnnotationEntry } + return parent is KtDotQualifiedExpression && parent.parent is KtImportDirective + } + + private fun PsiElement.isUsageAsQualifier(): Boolean { if (this is KtSimpleNameExpression) { val qualifier = getTopmostParentQualifiedExpressionForSelector() ?: this if ((qualifier.parent as? KtDotQualifiedExpression)?.receiverExpression == qualifier) { @@ -282,8 +285,6 @@ class ExperimentalUsageChecker(project: Project) : CallChecker { } } - if (getParentOfType(true) != null) return true - return false } diff --git a/compiler/testData/diagnostics/testsWithStdLib/experimental/usageNotAsAnnotation.kt b/compiler/testData/diagnostics/testsWithStdLib/experimental/usageNotAsAnnotation.kt index e199348f55d..6dd19ba3273 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/experimental/usageNotAsAnnotation.kt +++ b/compiler/testData/diagnostics/testsWithStdLib/experimental/usageNotAsAnnotation.kt @@ -1,5 +1,7 @@ +// !LANGUAGE: +NestedClassesInAnnotations // !USE_EXPERIMENTAL: kotlin.Experimental // !DIAGNOSTICS: -UNUSED_PARAMETER +// FILE: api.kt package test @@ -43,7 +45,13 @@ fun f5() {} // Usages of markers as types should be errors @Experimental -annotation class Marker +annotation class Marker { + class NestedClass + + companion object { + const val value = 42 + } +} fun f6(m: Marker) {} fun f7(): List<Marker>? = null @@ -52,3 +60,18 @@ fun f8(): test.Marker fun f9(m: Marker0) {} + + +// Usages of markers as qualifiers are errors as well (we can lift this restriction for select cases) + +fun f10(m: Marker.NestedClass) { + Marker.value +} + +// FILE: usage-from-other-file.kt + +// Usages of markers in import statements should be OK, but not as qualifiers to import their nested classes + +import test.Marker +import test.Marker.NestedClass +import test.Marker.Companion diff --git a/compiler/testData/diagnostics/testsWithStdLib/experimental/usageNotAsAnnotation.txt b/compiler/testData/diagnostics/testsWithStdLib/experimental/usageNotAsAnnotation.txt index 67662664779..e667fa24982 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/experimental/usageNotAsAnnotation.txt +++ b/compiler/testData/diagnostics/testsWithStdLib/experimental/usageNotAsAnnotation.txt @@ -2,6 +2,7 @@ package package test { public fun f1(/*0*/ e: kotlin.Experimental): kotlin.Unit + public fun f10(/*0*/ m: test.Marker.NestedClass): 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 @@ -23,6 +24,21 @@ package test { 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 companion object Companion { + private constructor Companion() + public const final val value: kotlin.Int = 42 + 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 class NestedClass { + public constructor NestedClass() + 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 {