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

Overriden method incorrectly uses Java String Primitive #915

Closed
shivanshs9 opened this issue May 24, 2020 · 5 comments
Closed

Overriden method incorrectly uses Java String Primitive #915

shivanshs9 opened this issue May 24, 2020 · 5 comments

Comments

@shivanshs9
Copy link

Extension of #139

Input code

open fun x(vararg y: String): Boolean = true // using kotlin.String

Expected output code

override fun x(vararg y: String): Boolean { // using kotlin.String
    ...
}

Actual output code

import java.lang.String
import kotlin.Array
import kotlin.Boolean

override fun x(vararg y: Array<String>): Boolean { // using java.lang.String
    ...
}

It shouldn't have converted vararg of String to vararg of array of String. Moreover, due to incorrectly using String primitive, it fails compilation saying 'x' overrides nothing.

@shivanshs9
Copy link
Author

To get around the bug, I currently use the following hack.
Source - #236 (comment)

fun TypeName.javaToKotlinType(): TypeName = when (this) {
    is ParameterizedTypeName -> {
        (rawType.javaToKotlinType() as ClassName).parameterizedBy(
            *typeArguments.map {
                it.javaToKotlinType()
            }.toTypedArray()
        )
    }
    is WildcardTypeName -> {
        if (inTypes.isNotEmpty()) WildcardTypeName.consumerOf(inTypes[0].javaToKotlinType())
        else WildcardTypeName.producerOf(outTypes[0].javaToKotlinType())
    }

    else -> {
        val className = JavaToKotlinClassMap
            .mapJavaToKotlin(FqName(toString()))?.asSingleFqName()?.asString()
        if (className == null) this
        else ClassName.bestGuess(className)
    }
}

From my debugging, I know the kotlin metadata is lost for parameters otherwise it would've been easy to use something like the above hack only for kotlin stuff.

@Egorand
Copy link
Collaborator

Egorand commented May 24, 2020

Again, without seeing your code, it's unclear what API you're using to produce the output.

You should use the kotlin-metadata-specs API to get synthetic Kotlin types on the JVM. If you're using reflection or Mirror API then java.lang.String is what you'll get - kotlin.String is a type only known to the compiler, it's not present in the bytecode or the stubs that kapt produces for annotation processors.

We will likely deprecate reflection and Mirror-based API soon, as they often produce unexpected output due to the lossiness of representing Kotlin code on the JVM. Kotlin's @Metadata is the only correct way to introspect all of the information that's not present in bytecode or stubs.

@shivanshs9
Copy link
Author

The code is basically the same, just the method name different.

    FunSpec.overriding(
        RequestParser::class.getExecutableElement(
            "x",
            elementUtils
        )!!
    ).addStatement("...").build()

Okay, I checked out the kotlin-metadata-specs API and yeah, I agree Mirror API definitely won't go for a solution but a hack at the most. I went through KotlinPoetMetadataSpecs.kt and also experimented with ImmutableKmFunction and ImmutableKmClass. However, I couldn't find any API documentation to finally build ImmutableKmClass from, let's say MutableKmFunction. I guess the public API isn't ready to test yet?

@Egorand
Copy link
Collaborator

Egorand commented May 24, 2020

The API is experimental but is definitely ready for testing, e.g. Moshi's Kotlin codegen uses it. Feel free to try, provide feedback, and contribute if you'd like.

Kotlin only attaches a @Metadata annotation to the type declarations, hence you can't really extract all type information if all you have is an ExecutableElement representing a function. You'd have to locate the enclosing TypeElement first.

@Egorand
Copy link
Collaborator

Egorand commented May 25, 2020

Closing as this is not actionable.

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

No branches or pull requests

2 participants