Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JVM] generic array type to TypeToken #4

Closed
wants to merge 2 commits into from

Conversation

romainbsl
Copy link
Member

No description provided.

internal class JVMGenericArrayTypeToken<T>(private val trueType: GenericArrayType) : JVMAbstractTypeToken<T>() {
override val jvmType: Type = trueType.genericComponentType
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why???
jvmType is supposed to return the JVM type representation of this type token.
This obviously breaks the contract, as it will return the JVM type of the array component.

Comment on lines +8 to +13
get() = when (jvmType) {
is Class<*> -> jvmType
is ParameterizedType -> jvmType.rawClass
else -> null
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above.

Should be:

Suggested change
private val rawType: Class<*>?
get() = when (jvmType) {
is Class<*> -> jvmType
is ParameterizedType -> jvmType.rawClass
else -> null
}
private val rawType: Class<*>?
get() {
val componentClass = typeToken(trueType.genericComponentType).getRaw().jvmType
return java.lang.reflect.Array.newInstance(componentClass, 0).javaClass
}

Comment on lines +23 to +28
when (jvmType) {
is Class<*> -> jvmType.typeParameters?.map { typeToken(it.bounds[0]) }?.toTypedArray() ?: emptyArray()
is ParameterizedType -> jvmType.actualTypeArguments.map { typeToken(it) }.toTypedArray()
else -> emptyArray()
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because you're using jvmType, you are not returning the generic params of the Array, but of its component.
You should simply return the component type.

@Suppress("UNCHECKED_CAST")
override fun isWildcard(): Boolean {
val realType = trueType as? ParameterizedType ?: return false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

trueType is GenericArrayType. It can never be ParameterizedType.

You need to check that the array's genericComponentType isn't wildcard.

Comment on lines +51 to +54
is ParameterizedType -> JVMParameterizedTypeToken<Any>(jvmType)
else -> null
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still no.
You are returning the parent of the component.
The parent of Array is just Any.

// Test typeToken of GenericArrayType -> Java primitive array
assertEquals(Byte::class.java, typeToken(GenericArrayType { java.lang.Byte.TYPE }).jvmType)
assertEquals(Int::class.java, typeToken(GenericArrayType { Integer.TYPE }).jvmType)
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There needs to be more tests.
How do you represent a generic array type using erased ?

We need a way to express the type Array<List<String>> using erased. Maybe create an erasedArray function?

There need to be tests that validates that:

assertEquals(generic<Array<List<String>>>, erasedArray(erasedComp(List::class, String::class)))

By the way, there is absolutely no reason those tests are JVM only.

Comment on lines +40 to +47
assertEquals(List::class.java, (generic<Array<List<String>>>().getRaw() as JVMClassTypeToken).jvmType)
assertEquals(List::class.java, (generic<Array<List<String>>>().jvmType as ParameterizedType).rawType)
// Test generic parameters (when Array<ParameterizedType>)
assertEquals(String::class.java, generic<Array<List<String>>>().getGenericParameters()[0].jvmType)
assertEquals(String::class.java, ((generic<Array<List<String>>>().jvmType as ParameterizedType).actualTypeArguments[0] as WildcardType).upperBounds[0])
// Test typeToken of GenericArrayType -> Java primitive array
assertEquals(Byte::class.java, typeToken(GenericArrayType { java.lang.Byte.TYPE }).jvmType)
assertEquals(Int::class.java, typeToken(GenericArrayType { Integer.TYPE }).jvmType)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests are plainly incorrect.

@romainbsl romainbsl force-pushed the master branch 2 times, most recently from a02b034 to 422ec3c Compare June 26, 2020 13:31
@romainbsl romainbsl closed this Nov 2, 2020
@romainbsl romainbsl deleted the feature/generic_array_type branch November 2, 2020 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants