azalea/kotlin-fork
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Check expected type of annotation parameter correctly in AnnotationDeserializer

Browse Source 
The main idea of this refactoring is to separate two usages of
`AnnotationDeserializer.resolveValue`: the one where we load annotation
argument values, and the one where we load constant values of properties
for JS/Native/Common
(`AnnotationAndConstantLoaderImpl.loadPropertyConstant`).

In the latter case, `expectedType` is the type of the property and it
can be a supertype of the actual value (e.g. see `arrayConst` in
compiler/testData/serialization/builtinsSerializer/compileTimeConstants.kt).
But in the former case, we need to check that the value conforms to the
expected type and disregard it if it's not the case, which is possible
if the annotation was recompiled separately.

 #KT-28927
This commit is contained in:
Alexander Udalov
2019-05-29 18:38:56 +02:00
parent c666b60ca3
commit 43cb17cdd5
6 changed files with 84 additions and 95 deletions
Download Patch File Download Diff File Expand all files Collapse all files
compiler/fir/resolve/src/org/jetbrains/kotlin/fir/deserialization/AbstractAnnotationDeserializer.kt
+2 -24
View File
@@ -142,29 +142,7 @@ abstract class AbstractAnnotationDeserializer(
)
}
// ARRAY -> {
// val expectedIsArray = KotlinBuiltIns.isArray(expectedType) || KotlinBuiltIns.isPrimitiveArray(expectedType)
// val arrayElements = value.arrayElementList
//
// val actualArrayType =
// if (arrayElements.isNotEmpty()) {
// val actualElementType = resolveArrayElementType(arrayElements.first(), nameResolver)
// builtIns.getPrimitiveArrayKotlinTypeByPrimitiveKotlinType(actualElementType)
// ?: builtIns.getArrayType(Variance.INVARIANT, actualElementType)
// } else {
// // In the case of empty array, no element has the element type, so we fall back to the expected type, if any.
// // This is not very accurate when annotation class has been changed without recompiling clients,
// // but should not in fact matter because the value is empty anyway
// if (expectedIsArray) expectedType else builtIns.getArrayType(Variance.INVARIANT, builtIns.anyType)
// }
//
// val expectedElementType = builtIns.getArrayElementType(if (expectedIsArray) expectedType else actualArrayType)
//
// ConstantValueFactory.createArrayValue(
// arrayElements.map {
// resolveValue(expectedElementType, it, nameResolver)
// },
// actualArrayType
// )
// TODO: see AnnotationDeserializer
// }
// else -> error("Unsupported annotation argument type: ${value.type} (expected $expectedType)")
else -> return null
@@ -173,4 +151,4 @@ abstract class AbstractAnnotationDeserializer(
}
private fun <T> const(kind: IrConstKind<T>, value: T) = FirConstExpressionImpl(session, null, kind, value)
}
}
compiler/testData/codegen/box/reflection/annotations/onTypes/classLiteralWithExpectedType.kt
Vendored
+24
View File
@@ -0,0 +1,24 @@
// TARGET_BACKEND: JVM
// WITH_REFLECT
import kotlin.reflect.KClass
import kotlin.test.assertEquals
@Target(AnnotationTarget.TYPE)
annotation class Anno(
val k1: KClass<out CharSequence>,
val k2: KClass<in String>,
val ka: Array<KClass<out Number>>
)
fun f(): @Anno(String::class, CharSequence::class, [Double::class, Long::class, Int::class]) Unit {}
fun box(): String {
assertEquals(
"[@Anno(k1=class java.lang.String, k2=interface java.lang.CharSequence, " +
"ka=[class java.lang.Double, class java.lang.Long, class java.lang.Integer])]",
::f.returnType.annotations.toString()
)
return "OK"
}
compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java
Generated
+5
View File
@@ -20008,6 +20008,11 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest {
KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/reflection/annotations/onTypes"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.JVM, true);
}
@TestMetadata("classLiteralWithExpectedType.kt")
public void testClassLiteralWithExpectedType() throws Exception {
runTest("compiler/testData/codegen/box/reflection/annotations/onTypes/classLiteralWithExpectedType.kt");
}
@TestMetadata("differentArgumentTypes.kt")
public void testDifferentArgumentTypes() throws Exception {
runTest("compiler/testData/codegen/box/reflection/annotations/onTypes/differentArgumentTypes.kt");
compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java
Generated
+5
View File
@@ -20008,6 +20008,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes
KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/reflection/annotations/onTypes"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.JVM, true);
}
@TestMetadata("classLiteralWithExpectedType.kt")
public void testClassLiteralWithExpectedType() throws Exception {
runTest("compiler/testData/codegen/box/reflection/annotations/onTypes/classLiteralWithExpectedType.kt");
}
@TestMetadata("differentArgumentTypes.kt")
public void testDifferentArgumentTypes() throws Exception {
runTest("compiler/testData/codegen/box/reflection/annotations/onTypes/differentArgumentTypes.kt");
compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java
Generated
+5
View File
@@ -20013,6 +20013,11 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes
KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/reflection/annotations/onTypes"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.JVM_IR, true);
}
@TestMetadata("classLiteralWithExpectedType.kt")
public void testClassLiteralWithExpectedType() throws Exception {
runTest("compiler/testData/codegen/box/reflection/annotations/onTypes/classLiteralWithExpectedType.kt");
}
@TestMetadata("differentArgumentTypes.kt")
public void testDifferentArgumentTypes() throws Exception {
runTest("compiler/testData/codegen/box/reflection/annotations/onTypes/differentArgumentTypes.kt");
core/deserialization/src/org/jetbrains/kotlin/serialization/deserialization/AnnotationDeserializer.kt
+43 -71
View File
@@ -20,7 +20,6 @@ import org.jetbrains.kotlin.builtins.KotlinBuiltIns
import org.jetbrains.kotlin.descriptors.*
import org.jetbrains.kotlin.descriptors.annotations.AnnotationDescriptor
import org.jetbrains.kotlin.descriptors.annotations.AnnotationDescriptorImpl
import org.jetbrains.kotlin.descriptors.annotations.Annotations
import org.jetbrains.kotlin.metadata.ProtoBuf.Annotation
import org.jetbrains.kotlin.metadata.ProtoBuf.Annotation.Argument
import org.jetbrains.kotlin.metadata.ProtoBuf.Annotation.Argument.Value
@@ -31,8 +30,9 @@ import org.jetbrains.kotlin.name.ClassId
import org.jetbrains.kotlin.name.Name
import org.jetbrains.kotlin.resolve.DescriptorUtils
import org.jetbrains.kotlin.resolve.constants.*
import org.jetbrains.kotlin.types.*
import org.jetbrains.kotlin.types.typeUtil.isSubtypeOf
import org.jetbrains.kotlin.resolve.descriptorUtil.classId
import org.jetbrains.kotlin.types.ErrorUtils
import org.jetbrains.kotlin.types.KotlinType
class AnnotationDeserializer(private val module: ModuleDescriptor, private val notFoundClasses: NotFoundClasses) {
private val builtIns: KotlinBuiltIns
@@ -59,13 +59,19 @@ class AnnotationDeserializer(private val module: ModuleDescriptor, private val n
nameResolver: NameResolver
): Pair<Name, ConstantValue<*>>? {
val parameter = parameterByName[nameResolver.getName(proto.nameId)] ?: return null
return Pair(nameResolver.getName(proto.nameId), resolveValue(parameter.type, proto.value, nameResolver))
return Pair(nameResolver.getName(proto.nameId), resolveValueAndCheckExpectedType(parameter.type, proto.value, nameResolver))
}
private fun resolveValueAndCheckExpectedType(expectedType: KotlinType, value: Value, nameResolver: NameResolver): ConstantValue<*> {
return resolveValue(expectedType, value, nameResolver).takeIf {
doesValueConformToExpectedType(it, expectedType, value)
} ?: ErrorValue.create("Unexpected argument value: actual type ${value.type} != expected type $expectedType")
}
fun resolveValue(expectedType: KotlinType, value: Value, nameResolver: NameResolver): ConstantValue<*> {
val isUnsigned = Flags.IS_UNSIGNED.get(value.flags)
val result: ConstantValue<*> = when (value.type) {
return when (value.type) {
Type.BYTE -> value.intValue.toByte().letIf(isUnsigned, ::UByteValue, ::ByteValue)
Type.CHAR -> CharValue(value.intValue.toChar())
Type.SHORT -> value.intValue.toShort().letIf(isUnsigned, ::UShortValue, ::ShortValue)
@@ -74,81 +80,47 @@ class AnnotationDeserializer(private val module: ModuleDescriptor, private val n
Type.FLOAT -> FloatValue(value.floatValue)
Type.DOUBLE -> DoubleValue(value.doubleValue)
Type.BOOLEAN -> BooleanValue(value.intValue != 0L)
Type.STRING -> {
StringValue(nameResolver.getString(value.stringValue))
}
Type.CLASS -> {
KClassValue(nameResolver.getClassId(value.classId), value.arrayDimensionCount)
}
Type.ENUM -> {
EnumValue(nameResolver.getClassId(value.classId), nameResolver.getName(value.enumValueId))
}
Type.ANNOTATION -> {
AnnotationValue(deserializeAnnotation(value.annotation, nameResolver))
}
Type.ARRAY -> {
val expectedIsArray = KotlinBuiltIns.isArray(expectedType) || KotlinBuiltIns.isPrimitiveArray(expectedType)
val arrayElements = value.arrayElementList
val actualArrayType =
if (arrayElements.isNotEmpty()) {
val actualElementType = resolveArrayElementType(arrayElements.first(), nameResolver)
builtIns.getPrimitiveArrayKotlinTypeByPrimitiveKotlinType(actualElementType)
?: builtIns.getArrayType(Variance.INVARIANT, actualElementType)
} else {
// In the case of empty array, no element has the element type, so we fall back to the expected type, if any.
// This is not very accurate when annotation class has been changed without recompiling clients,
// but should not in fact matter because the value is empty anyway
if (expectedIsArray) expectedType else builtIns.getArrayType(Variance.INVARIANT, builtIns.anyType)
}
val expectedElementType = builtIns.getArrayElementType(if (expectedIsArray) expectedType else actualArrayType)
ConstantValueFactory.createArrayValue(
arrayElements.map {
resolveValue(expectedElementType, it, nameResolver)
},
actualArrayType
)
}
Type.STRING -> StringValue(nameResolver.getString(value.stringValue))
Type.CLASS -> KClassValue(nameResolver.getClassId(value.classId), value.arrayDimensionCount)
Type.ENUM -> EnumValue(nameResolver.getClassId(value.classId), nameResolver.getName(value.enumValueId))
Type.ANNOTATION -> AnnotationValue(deserializeAnnotation(value.annotation, nameResolver))
Type.ARRAY -> ConstantValueFactory.createArrayValue(
value.arrayElementList.map { resolveValue(builtIns.anyType, it, nameResolver) },
expectedType
)
else -> error("Unsupported annotation argument type: ${value.type} (expected $expectedType)")
}
}
return if (result.getType(module).isSubtypeOf(expectedType)) {
result
} else {
// This usually means that an annotation class has been changed incompatibly without recompiling clients
ErrorValue.create(
"Unexpected argument value: type ${result.getType(module)} is not a subtype of $expectedType (value.type = ${value.type})"
)
// This method returns false if the actual value loaded from an annotation argument does not conform to the expected type of the
// corresponding parameter in the annotation class. This usually means that the annotation class has been changed incompatibly
// without recompiling clients, in which case we prefer not to load the annotation argument value at all, to avoid constructing
// an incorrect model and breaking some assumptions in the compiler.
private fun doesValueConformToExpectedType(result: ConstantValue<*>, expectedType: KotlinType, value: Value): Boolean {
return when (value.type) {
Type.CLASS -> {
val expectedClass = expectedType.constructor.declarationDescriptor as? ClassDescriptor
// We could also check that the class value's type is a subtype of the expected type, but loading the definition of the
// referenced class here is undesirable and may even be incorrect (because the module might be different at the
// destination where these constant values are read). This can lead to slightly incorrect model in some edge cases.
expectedClass == null || KotlinBuiltIns.isKClass(expectedClass)
}
Type.ARRAY -> {
check(result is ArrayValue && result.value.size == value.arrayElementList.size) {
"Deserialized ArrayValue should have the same number of elements as the original array value: $result"
}
val expectedElementType = builtIns.getArrayElementType(expectedType)
result.value.indices.all { i ->
doesValueConformToExpectedType(result.value[i], expectedElementType, value.getArrayElement(i))
}
}
else -> result.getType(module) == expectedType
}
}
private inline fun <T, R> T.letIf(predicate: Boolean, f: (T) -> R, g: (T) -> R): R =
if (predicate) f(this) else g(this)
private fun resolveArrayElementType(value: Value, nameResolver: NameResolver): SimpleType =
with(builtIns) {
when (value.type) {
Type.BYTE -> byteType
Type.CHAR -> charType
Type.SHORT -> shortType
Type.INT -> intType
Type.LONG -> longType
Type.FLOAT -> floatType
Type.DOUBLE -> doubleType
Type.BOOLEAN -> booleanType
Type.STRING -> stringType
Type.CLASS -> KotlinTypeFactory.simpleNotNullType(
Annotations.EMPTY, kClass, listOf(StarProjectionImpl(kClass.declaredTypeParameters.single()))
)
Type.ENUM -> resolveClass(nameResolver.getClassId(value.classId)).defaultType
Type.ANNOTATION -> resolveClass(nameResolver.getClassId(value.annotation.id)).defaultType
Type.ARRAY -> error("Array of arrays is impossible")
else -> error("Unknown type: ${value.type}")
}
}
private fun resolveClass(classId: ClassId): ClassDescriptor {
return module.findNonGenericClassAcrossDependencies(classId, notFoundClasses)
}