From 736ac123740fb5f9ae2508a6a2fb2c383f5897b7 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Thu, 27 Dec 2018 15:59:50 +0100 Subject: [PATCH] Prohibit type parameters in class literals in annotation arguments #KT-27799 Fixed --- .../jetbrains/kotlin/diagnostics/Errors.java | 2 + .../rendering/DefaultErrorMessages.java | 2 + .../evaluate/ConstantExpressionEvaluator.kt | 48 +++++++++++++------ .../classLiteral/inAnnotationArguments.kt | 11 ++++- .../classLiteral/inAnnotationArguments.txt | 1 + .../inAnnotationArguments_noTypeParams.kt | 12 +++++ .../inAnnotationArguments_noTypeParams.txt | 11 +++++ .../checkers/DiagnosticsTestGenerated.java | 5 ++ .../DiagnosticsUsingJavacTestGenerated.java | 5 ++ .../kotlin/config/LanguageVersionSettings.kt | 1 + .../resolve/constants/constantValues.kt | 4 +- 11 files changed, 84 insertions(+), 18 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/classLiteral/inAnnotationArguments_noTypeParams.kt create mode 100644 compiler/testData/diagnostics/tests/classLiteral/inAnnotationArguments_noTypeParams.txt diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java index bb69bde693b..cba774061c4 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java @@ -231,6 +231,8 @@ public interface Errors { DiagnosticFactory0 ANNOTATION_ARGUMENT_MUST_BE_CONST = DiagnosticFactory0.create(ERROR); DiagnosticFactory0 ANNOTATION_ARGUMENT_MUST_BE_KCLASS_LITERAL = DiagnosticFactory0.create(ERROR); DiagnosticFactory0 ANNOTATION_ARGUMENT_MUST_BE_ENUM_CONST = DiagnosticFactory0.create(ERROR); + DiagnosticFactory0 ANNOTATION_ARGUMENT_KCLASS_LITERAL_OF_TYPE_PARAMETER = DiagnosticFactory0.create(WARNING); + DiagnosticFactory0 ANNOTATION_ARGUMENT_KCLASS_LITERAL_OF_TYPE_PARAMETER_ERROR = DiagnosticFactory0.create(ERROR); DiagnosticFactory0 ANNOTATION_PARAMETER_DEFAULT_VALUE_MUST_BE_CONSTANT = DiagnosticFactory0.create(ERROR); DiagnosticFactory0 ANNOTATIONS_ON_BLOCK_LEVEL_EXPRESSION_ON_THE_SAME_LINE = DiagnosticFactory0.create(WARNING); DiagnosticFactory0 ANNOTATION_USED_AS_ANNOTATION_ARGUMENT = DiagnosticFactory0.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 6857226c859..42da0f262c3 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java @@ -866,6 +866,8 @@ public class DefaultErrorMessages { MAP.put(ANNOTATION_ARGUMENT_MUST_BE_CONST, "An annotation argument must be a compile-time constant"); MAP.put(ANNOTATION_ARGUMENT_MUST_BE_ENUM_CONST, "An enum annotation argument must be a enum constant"); MAP.put(ANNOTATION_ARGUMENT_MUST_BE_KCLASS_LITERAL, "An annotation argument must be a class literal (T::class)"); + MAP.put(ANNOTATION_ARGUMENT_KCLASS_LITERAL_OF_TYPE_PARAMETER, "Type parameter in a class literal is not allowed in an annotation argument"); + MAP.put(ANNOTATION_ARGUMENT_KCLASS_LITERAL_OF_TYPE_PARAMETER_ERROR, "Type parameter in a class literal is deprecated in an annotation argument"); MAP.put(ANNOTATION_PARAMETER_DEFAULT_VALUE_MUST_BE_CONSTANT, "Default value of annotation parameter must be a compile-time constant"); MAP.put(ANNOTATIONS_ON_BLOCK_LEVEL_EXPRESSION_ON_THE_SAME_LINE, diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/constants/evaluate/ConstantExpressionEvaluator.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/constants/evaluate/ConstantExpressionEvaluator.kt index 5ca4badbdf7..1ca0eb65890 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/constants/evaluate/ConstantExpressionEvaluator.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/constants/evaluate/ConstantExpressionEvaluator.kt @@ -127,7 +127,13 @@ class ConstantExpressionEvaluator( val descriptor = expressionType.constructor.declarationDescriptor val diagnosticFactory = when { DescriptorUtils.isEnumClass(descriptor) -> Errors.ANNOTATION_ARGUMENT_MUST_BE_ENUM_CONST - descriptor is ClassDescriptor && KotlinBuiltIns.isKClass(descriptor) -> Errors.ANNOTATION_ARGUMENT_MUST_BE_KCLASS_LITERAL + descriptor is ClassDescriptor && KotlinBuiltIns.isKClass(descriptor) -> { + if (isTypeParameterOrArrayOfTypeParameter(expressionType.arguments.singleOrNull()?.type)) { + Errors.ANNOTATION_ARGUMENT_KCLASS_LITERAL_OF_TYPE_PARAMETER_ERROR + } else { + Errors.ANNOTATION_ARGUMENT_MUST_BE_KCLASS_LITERAL + } + } else -> Errors.ANNOTATION_ARGUMENT_MUST_BE_CONST } @@ -177,6 +183,8 @@ class ConstantExpressionEvaluator( } else { trace.report(Errors.ANNOTATION_ARGUMENT_MUST_BE_KCLASS_LITERAL.on(argumentExpression)) } + } else if (doubleColonLhs is DoubleColonLHS.Type && isTypeParameterOrArrayOfTypeParameter(doubleColonLhs.type)) { + trace.report(Errors.ANNOTATION_ARGUMENT_KCLASS_LITERAL_OF_TYPE_PARAMETER.on(argumentExpression)) } } } @@ -324,6 +332,13 @@ class ConstantExpressionEvaluator( fun getPossiblyErrorConstant(expression: KtExpression, bindingContext: BindingContext): CompileTimeConstant<*>? { return bindingContext.get(BindingContext.COMPILE_TIME_VALUE, expression) } + + internal fun isTypeParameterOrArrayOfTypeParameter(type: KotlinType?): Boolean = + when { + type == null -> false + KotlinBuiltIns.isArray(type) -> isTypeParameterOrArrayOfTypeParameter(type.arguments.singleOrNull()?.type) + else -> type.constructor.declarationDescriptor is TypeParameterDescriptor + } } } @@ -336,6 +351,7 @@ private class ConstantExpressionEvaluatorVisitor( private val constantExpressionEvaluator: ConstantExpressionEvaluator, private val trace: BindingTrace ) : KtVisitor?, KotlinType>() { + private val languageVersionSettings = constantExpressionEvaluator.languageVersionSettings private val builtIns = constantExpressionEvaluator.module.builtIns fun evaluate(expression: KtExpression, expectedType: KotlinType?): CompileTimeConstant<*>? { @@ -613,7 +629,8 @@ private class ConstantExpressionEvaluatorVisitor( trace.report(Errors.DIVISION_BY_ZERO.on(parentExpression)) if ((isIntegerType(argumentForReceiver.value) && isIntegerType(argumentForParameter.value)) || - !constantExpressionEvaluator.languageVersionSettings.supportsFeature(LanguageFeature.DivisionByZeroInConstantExpressions)) { + !languageVersionSettings.supportsFeature(LanguageFeature.DivisionByZeroInConstantExpressions) + ) { return ErrorValue.create("Division by zero").wrap() } } @@ -883,11 +900,19 @@ private class ConstantExpressionEvaluatorVisitor( } override fun visitClassLiteralExpression(expression: KtClassLiteralExpression, expectedType: KotlinType?): CompileTimeConstant<*>? { - val type = trace.getType(expression)!! - if (type.isError) return null - val descriptor = type.constructor.declarationDescriptor + val kClassType = trace.getType(expression)!! + if (kClassType.isError) return null + val descriptor = kClassType.constructor.declarationDescriptor if (descriptor !is ClassDescriptor || !KotlinBuiltIns.isKClass(descriptor)) return null - return KClassValue.create(type.arguments.first().type)?.wrap() + + val type = kClassType.arguments.singleOrNull()?.type ?: return null + if (languageVersionSettings.supportsFeature(LanguageFeature.ProhibitTypeParametersInClassLiteralsInAnnotationArguments) && + ConstantExpressionEvaluator.isTypeParameterOrArrayOfTypeParameter(type) + ) { + return null + } + + return KClassValue.create(type)?.wrap() } private fun resolveArguments(valueArguments: List, expectedType: KotlinType): List?> { @@ -983,12 +1008,7 @@ private class ConstantExpressionEvaluatorVisitor( parameters: CompileTimeConstant.Parameters, expectedType: KotlinType ): CompileTimeConstant<*>? { - if (parameters.isUnsignedNumberLiteral && - !checkAccessibilityOfUnsignedTypes( - constantExpressionEvaluator.module, - constantExpressionEvaluator.languageVersionSettings - ) - ) { + if (parameters.isUnsignedNumberLiteral && !checkAccessibilityOfUnsignedTypes()) { return UnsignedErrorValueTypeConstant(value, parameters) } @@ -1019,8 +1039,8 @@ private class ConstantExpressionEvaluatorVisitor( }.wrap(parameters) } - private fun checkAccessibilityOfUnsignedTypes(module: ModuleDescriptor, languageVersionSettings: LanguageVersionSettings): Boolean { - val uInt = module.findClassAcrossModuleDependencies(KotlinBuiltIns.FQ_NAMES.uInt) ?: return false + private fun checkAccessibilityOfUnsignedTypes(): Boolean { + val uInt = constantExpressionEvaluator.module.findClassAcrossModuleDependencies(KotlinBuiltIns.FQ_NAMES.uInt) ?: return false val accessibility = uInt.checkSinceKotlinVersionAccessibility(languageVersionSettings) // Case `NotAccessibleButWasExperimental` will be checked later in `checkExperimentalityOfConstantLiteral` return accessibility is SinceKotlinAccessibility.Accessible diff --git a/compiler/testData/diagnostics/tests/classLiteral/inAnnotationArguments.kt b/compiler/testData/diagnostics/tests/classLiteral/inAnnotationArguments.kt index 4757e47d376..e0613fb96ec 100644 --- a/compiler/testData/diagnostics/tests/classLiteral/inAnnotationArguments.kt +++ b/compiler/testData/diagnostics/tests/classLiteral/inAnnotationArguments.kt @@ -1,4 +1,4 @@ -// WITH_REFLECT +// !LANGUAGE: +ProhibitTypeParametersInClassLiteralsInAnnotationArguments import kotlin.reflect.KClass @@ -35,4 +35,11 @@ fun test7() {} fun test8() {} inline val T.test9 - get() = @Ann(T::class) object {} + get() = @AnnArray(arrayOf( + T::class, + Array::class, + Array>>::class + )) object {} + +inline val T.test10 + get() = @AnnArray([T::class]) object {} diff --git a/compiler/testData/diagnostics/tests/classLiteral/inAnnotationArguments.txt b/compiler/testData/diagnostics/tests/classLiteral/inAnnotationArguments.txt index 567e090119c..3ac66893540 100644 --- a/compiler/testData/diagnostics/tests/classLiteral/inAnnotationArguments.txt +++ b/compiler/testData/diagnostics/tests/classLiteral/inAnnotationArguments.txt @@ -1,5 +1,6 @@ package +public val T.test10: kotlin.Any public val T.test9: kotlin.Any public fun foo(): kotlin.String @Ann(k = kotlin.String::class) public fun test1(): kotlin.Unit diff --git a/compiler/testData/diagnostics/tests/classLiteral/inAnnotationArguments_noTypeParams.kt b/compiler/testData/diagnostics/tests/classLiteral/inAnnotationArguments_noTypeParams.kt new file mode 100644 index 00000000000..cbc2e51eeb1 --- /dev/null +++ b/compiler/testData/diagnostics/tests/classLiteral/inAnnotationArguments_noTypeParams.kt @@ -0,0 +1,12 @@ +// !LANGUAGE: -ProhibitTypeParametersInClassLiteralsInAnnotationArguments + +import kotlin.reflect.KClass + +annotation class Ann(vararg val k: KClass<*>) + +inline val T.test + get() = @Ann( + T::class, + Array::class, + Array>>::class + ) object {} diff --git a/compiler/testData/diagnostics/tests/classLiteral/inAnnotationArguments_noTypeParams.txt b/compiler/testData/diagnostics/tests/classLiteral/inAnnotationArguments_noTypeParams.txt new file mode 100644 index 00000000000..d3ec93f45ba --- /dev/null +++ b/compiler/testData/diagnostics/tests/classLiteral/inAnnotationArguments_noTypeParams.txt @@ -0,0 +1,11 @@ +package + +public val T.test: kotlin.Any + +public final annotation class Ann : kotlin.Annotation { + public constructor Ann(/*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 +} diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java index 54a27f3d161..2bcd0bd3a65 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java @@ -2991,6 +2991,11 @@ public class DiagnosticsTestGenerated extends AbstractDiagnosticsTest { runTest("compiler/testData/diagnostics/tests/classLiteral/inAnnotationArguments.kt"); } + @TestMetadata("inAnnotationArguments_noTypeParams.kt") + public void testInAnnotationArguments_noTypeParams() throws Exception { + runTest("compiler/testData/diagnostics/tests/classLiteral/inAnnotationArguments_noTypeParams.kt"); + } + @TestMetadata("integerValueType.kt") public void testIntegerValueType() throws Exception { runTest("compiler/testData/diagnostics/tests/classLiteral/integerValueType.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java index efd4ac29b82..5e3e8741fa2 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java @@ -2986,6 +2986,11 @@ public class DiagnosticsUsingJavacTestGenerated extends AbstractDiagnosticsUsing runTest("compiler/testData/diagnostics/tests/classLiteral/inAnnotationArguments.kt"); } + @TestMetadata("inAnnotationArguments_noTypeParams.kt") + public void testInAnnotationArguments_noTypeParams() throws Exception { + runTest("compiler/testData/diagnostics/tests/classLiteral/inAnnotationArguments_noTypeParams.kt"); + } + @TestMetadata("integerValueType.kt") public void testIntegerValueType() throws Exception { runTest("compiler/testData/diagnostics/tests/classLiteral/integerValueType.kt"); diff --git a/compiler/util/src/org/jetbrains/kotlin/config/LanguageVersionSettings.kt b/compiler/util/src/org/jetbrains/kotlin/config/LanguageVersionSettings.kt index c9e67df5d5a..e9b077bd465 100644 --- a/compiler/util/src/org/jetbrains/kotlin/config/LanguageVersionSettings.kt +++ b/compiler/util/src/org/jetbrains/kotlin/config/LanguageVersionSettings.kt @@ -100,6 +100,7 @@ enum class LanguageFeature( ProperInlineFromHigherPlatformDiagnostic(KOTLIN_1_4, kind = BUG_FIX), ProhibitRepeatedUseSiteTargetAnnotations(KOTLIN_1_4, kind = BUG_FIX), ProhibitUseSiteTargetAnnotationsOnSuperTypes(KOTLIN_1_4, kind = BUG_FIX), + ProhibitTypeParametersInClassLiteralsInAnnotationArguments(KOTLIN_1_4, kind = BUG_FIX), ProperVisibilityForCompanionObjectInstanceField(sinceVersion = null, kind = BUG_FIX), // Temporarily disabled, see KT-27084/KT-22379 diff --git a/core/descriptors/src/org/jetbrains/kotlin/resolve/constants/constantValues.kt b/core/descriptors/src/org/jetbrains/kotlin/resolve/constants/constantValues.kt index 5501e950f92..245fcbe71e9 100644 --- a/core/descriptors/src/org/jetbrains/kotlin/resolve/constants/constantValues.kt +++ b/core/descriptors/src/org/jetbrains/kotlin/resolve/constants/constantValues.kt @@ -214,10 +214,10 @@ class KClassValue(value: Value) : ConstantValue(value) { KClassValue(classId, arrayDimensions) } is TypeParameterDescriptor -> { - // This is possible if a reified type parameter is used in annotation on a local class / anonymous object. + // This is possible before 1.4 if a reified type parameter is used in annotation on a local class / anonymous object. // In JVM class file, we can't represent such literal properly, so we're writing java.lang.Object instead. // This has no effect on the compiler front-end or other back-ends, so we use kotlin.Any for simplicity here. - // Overall, it looks like such code should be disallowed: https://youtrack.jetbrains.com/issue/KT-27799 + // See LanguageFeature.ProhibitTypeParametersInClassLiteralsInAnnotationArguments KClassValue(ClassId.topLevel(KotlinBuiltIns.FQ_NAMES.any.toSafe()), 0) } else -> null