android / android-ktx

A set of Kotlin extensions for Android app development.
https://android.github.io/android-ktx/core-ktx/
7.47k stars 563 forks source link

checkApi and updateApi ignore methods with reified generics #235

Open hluhovskyi opened 6 years ago

hluhovskyi commented 6 years ago

Following method is ignored by checkApi as well as by updateApi:

inline fun <reified T> doSomething() {
}

As follows Context.systemService is absent in current.txt.

JakeWharton commented 6 years ago

If memory serves, this is a UAST bug.

tnorbye commented 6 years ago

Finally got around to investigating this, and I'm realizing that it's not actually a UAST bug.

It looks like what the Kotlin compiler does with reified methods like that is to turn them private! If I access your method as a Kotlin extension method from Kotlin, it works and compiles: context.systemService() // Compiles

However, if I try to access it from Java as a util method, it fails: ContextKt.systemService(context); // Doesn't compile with this message from javac: error: systemService(Context) has private access in ContextKt where T is a type-variable: T extends Object declared in method systemService(Context)

So, the AST marks this method as private. And when metalava is visiting the API of the class, it sees that the method isn't public and therefore drops it.

Until now I've made the metalava signature files reflect the Java-observable API (e.g. a property exploded into its getters and setters etc). But this is a pretty clear example of a case where it needs to diverge from that and actually list the Kotlin-only-accessible part of the API. I'm thinking I should just switch the signatures over (for Kotlin-compilation-units) to more directly reflect Kotlin. The main downside to this is when Java code is replaced by equivalent Kotlin code; it will be a big visible change in the signatures of the class, which it really shouldn't be. I'm going to think about it, but please chime in if you have strong opinions one way or the other!

JakeWharton commented 6 years ago

It sounds like we might need to track both projections of the API. Not here, where we only care about the Kotlin view, but for regular support libraries.

For example, the difference between a getter function and a property defined in Kotlin is very different call-site semantics when called from Kotlin. From the Java side, though, both are the same getter method.

On Tue, Mar 13, 2018, 11:02 PM Tor Norbye notifications@github.com wrote:

Finally got around to investigating this, and I'm realizing that it's not actually a UAST bug.

It looks like what the Kotlin compiler does with reified methods like that is to turn them private! If I access your method as a Kotlin extension method from Kotlin, it works and compiles: context.systemService() // Compiles

However, if I try to access it from Java as a util method, it fails: ContextKt.systemService(context); // Doesn't compile with this message from javac: error: systemService(Context) has private access in ContextKt where T is a type-variable: T extends Object declared in method systemService(Context)

So, the AST marks this method as private. And when metalava is visiting the API of the class, it sees that the method isn't public and therefore drops it.

Until now I've made the metalava signature files reflect the Java-observable API (e.g. a property exploded into its getters and setters etc). But this is a pretty clear example of a case where it needs to diverge from that and actually list the Kotlin-only-accessible part of the API. I'm thinking I should just switch the signatures over (for Kotlin-compilation-units) to more directly reflect Kotlin. The main downside to this is when Java code is replaced by equivalent Kotlin code; it will be a big visible change in the signatures of the class, which it really shouldn't be. I'm going to think about it, but please chime in if you have strong opinions one way or the other!

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/android/android-ktx/issues/235#issuecomment-372889182, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEEEd-9OQo0Pp6_tnVi4J03d--AhHmDks5teIgqgaJpZM4R71Y- .