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

Java annotations for Kotlin sugar #110

Open
JakeWharton opened this Issue May 10, 2018 · 20 comments

Comments

Projects
None yet
6 participants
@JakeWharton

JakeWharton commented May 10, 2018

Discussion of the proposal in #111

Summary

This proposal adds annotations which enable the use of more Kotlin language
features like extension functions, extension properties, default values, and
property names for Java code.

  • @ExtensionFunction / @ExtensionProperty - Turn a static method with at
    least one argument into an extension function or an extension property.
  • @ParameterName - An explicit name for parameters.
  • @DefaultValue - Default parameter values.
@fvasco

This comment has been minimized.

fvasco commented May 10, 2018

Should we consider @Suspend for coroutine support or @Internal for internal modifier?

@ice1000

This comment has been minimized.

ice1000 commented May 11, 2018

@fvasco @Suspend is painful, since it requires the compiler to do CPS conversions (which is not supported by javac), and you'll need an annotation processor (and of course, an extremely complicated one) for it.

And BTW @Internal is a great idea.

@fvasco

This comment has been minimized.

fvasco commented May 11, 2018

@ice1000 suspend is a regular Continuation parameter, it is not really different then extension's one.

I consider KtName more "painful", in the example we have defined a better name for copyOf, but toImmutableSet method is undocumented in JavaDoc.
Looking for rootCause how should I can found getRootCause in Throwables class?
JavaDoc is not organized for Kotlin syntax.

My only consideration against suspend annotation is a really specific Kotlin type of Continuation.
Async libraries use often a different type for CPS argument, AsyncHandler for AWS API, Handler for Vert.x, OperationCompletionListener for Memcached client, Promise in JS, and si on.

Therefore a really common code pattern is:

suspend fun <T> myAwait(block: (MyHandler<T>) -> Unit): T =
        suspendCoroutine { cont ->
            val handler = ContinuationMyHandler<T>(cont)
            block(handler)
        }

followed by the code

val result = myAwait{ handler -> myClass.myMethod(p1, p2, p3, handler) }

Permitting the suspend annotation allows many sympathetic libraries to define

@KtSuspend("mypackage.ContinuationMyHandler")
public void myMethod(String p1, int p2, int p3, MyHandler callback)

so the kotlin code becomes

val result = myClass.myMethod(p1, p2, p3)
@fvasco

This comment has been minimized.

fvasco commented May 11, 2018

Another tiny consideration: using @KtSuspend("mypackage.MyHandlerContinuation") allows every library to ship code with Koltin support but without any dependencies to Kotlin library (*) (the handler is referenced as a constant string).

So every Java developer has no issue.
Every Kotlin developer must include a specific library containing mypackage.MyHandlerContinuation class.

  • I hope that this annotation has packaged in a dedicated bundle suitable for Java projects.
@ice1000

This comment has been minimized.

ice1000 commented May 11, 2018

So you mean @KtSuspend requires the user to manually pass the Continuation instance? That sounds much more reasonable.

@fvasco

This comment has been minimized.

fvasco commented May 11, 2018

Maybe is better provide with a @KtSuspend annotation an another one, @KtContinuationAdapter to specify an eventually wrapper (ie mypackage.ContinuationMyHandler), this annotation can be applicable to parameters, methods, types or packages.

@KtSuspend is applicable to all function returning void with at least one parameter, the last one must have the type Continuation.
The class referenced by the @KtContinuationAdapter annotation parameter must have a constractor with one argument of type Continuation and that class must implement the method continuation interface, ie:

@KtContinuationAdapter("ContinuationMyHandler")
class AsyncByteBuffer {

  @KtSuspend
  void load(int bytes, MyHandler myHandler) { ... }

}

class ContinuationMyHandler implements MyHandler {

  ContinuationMyHandler(Continuation continuation) { ... }

  ...

}
@JakeWharton

This comment has been minimized.

JakeWharton commented May 12, 2018

We are not going to pursue either of those suggestions as part of this proposal. Coroutines aren't stable and the annotations in this proposal are targeted at library APIs where internal isn't useful.

@fvasco

This comment has been minimized.

fvasco commented May 12, 2018

Hi @JakeWharton
I don't agree with your considerations.

If this KEEP becomes stable before coroutine then it may contain experimental annotation, else the coroutine will be defined and the discussed part might be valid (the above annotation requires only a suspend keyword and a Continuation interface).

Finally the @KtInternal annotation shares the same motivation of internal visibility modifier, regardeless the target libraries.

This KEEP is interesting not only for well-know public libraries, but it is applicable to every project with mixed Java-Kotlin code.

@JakeWharton

This comment has been minimized.

JakeWharton commented May 12, 2018

The proposal is intentionally limited in scope to increase its chance of success, as evidenced by the last section where we enumerate a few other language features that could easily be included. Attempting to map every Kotlin language feature back into some shape of Java source is an easy and fast way for this proposal to never get implemented.

It is a non-goal to solve Java-Kotlin interop for every project as you state.

@fvasco

This comment has been minimized.

fvasco commented May 12, 2018

@JakeWharton thank you for quick response,
your point of view is reasonable.

The idea behind this KEEP is really interesting, but the most articulate discussed feature, the @KtSuspend annotation, is a great boost for long standing opened issue like this vert-x3/vertx-lang-kotlin#41
Already mentioned libraries and more share the same problem.

I not am an Android developer, I am developing a fully asynchronous server application. I consider asynchronous libraries an emerging trend for server-side develop (see Java NIO, Netty, Spring, AWS Java client, Vert.x, ...), so simplify further the Kotlin interoperation with existing Java libraries is a great benefit.

Can you consider to introduce an optional part for this KEEP or do you consider a better option extending using a new one?

@gildor

This comment has been minimized.

Contributor

gildor commented May 12, 2018

@fvasco @KtSuspend looks like much more advanced feature with a lot of non clear parts such as adpaters.
Also coroutines are experimental, so it's one more not clear case (continuation interface will be changed, maybe make sense to wait for this)

If you see good use cases mabe make sense to create a new KEEP

@gildor

This comment has been minimized.

Contributor

gildor commented May 12, 2018

I don't think that internal for Java code is good idea. If some class/method available from Java for some reason no need to overcomplicate it and provide additional visibility rules just for Kotlin.
And probably it shouldn't be a part of this proposal.

@fvasco

This comment has been minimized.

fvasco commented May 12, 2018

Hi @gildor,
Coroutines are experimental, it is true, but some libraries are already propose an experimental support for it, so personally I do not consider a real argument against this.

I exposed all reasonable motivation for @KtSuspend annotation, maybe @vietj can expose better consideration about this.

Await coroutine stabilization is a JetBrain choice, so I should consider that this KEEP should be included in Kotlin 1.2 (right?).
My idea is to provide a @KtSuspend in Kotlin 1.3, so when the coroutine will be stable.

However if your consider interesting this idea and you would examine in deep the not clear parts then I will open a dedicated KEEP.

@opatry

This comment has been minimized.

opatry commented May 15, 2018

Please, 🙏 do not put Kotlin related names in annotations that might be used in standard Java libraries.
What I want to create my own language and provide such custom and "colored" annotations? Should every Java libraries authors pollute their own code with @KtName, @FooName, @MyName etc.?
Same is for @DefaultValue("myNamingConvention"), what if I do not choose such naming convention in my own language?

This should be defined through a JSR with generic semantics that Kotlin might choose to handle afterwards.

For instance, @Inject (javax.inject.Inject) reflects this, @NonNull from Android don't which is unfortunate, despite the availability of @NotNull (javax.validation.constraints.NotNull).

Correct me if I'm wrong.

Still, I think this initiative is great and trying to improve Java/Kotlin interoperability goes in the right direction, I'm just worried about the implementation and chosen names.

@JakeWharton

This comment has been minimized.

JakeWharton commented May 15, 2018

The names have already changed, which I'll update soon, but being Kotlin-specific is not changing.

Otherwise, your own argument works against you:

what if I do not choose such naming convention in my own language?

Any semantics you try to define in some annotations which are common to every JVM language (an impossible task in and of itself, surely) can be trivially undermined by inventing your own language whose semantics are different.

@opatry

This comment has been minimized.

opatry commented May 15, 2018

@Name with javax.* package seems reasonable and generic enough to be compliant with other JVM languages, @DefaultValue seems harder and might be avoided IMO.

@JakeWharton

This comment has been minimized.

JakeWharton commented May 15, 2018

Just updated the proposal based on 2018-05-10 JetBrains and Google/Android meeting.

Change summary:

  1. Rename @KtName to @ParameterName, scope only to parameters
  2. Add name property to @ExtensionFunction and @ExtensionProperty

See the commit message for some raw notes.

@fvasco

This comment has been minimized.

fvasco commented May 16, 2018

How import Java extensions?

Ie: for the Java class

class Util {
   @ExtensionProperty String ca = "ca"
   @ExtensionFunction void cm(String p) { }

   @ExtensionProperty static String sa = "sa"
   @ExtensionFunction static void sm(String p) {}
}

should the follow code be valid?

import Util

with(Util()) {
   ca.cm()
}
import Util.sa
import Util.sm
// or import Util.*

sa.sm()

Explicit import made easy understand the related Java classes.

@JakeWharton

This comment has been minimized.

JakeWharton commented May 16, 2018

I think this is ultimately for the compiler team to decide, but we talked about requiring an import because otherwise they have to parse a lot of Java to find the extension.

(Also worth noting that @ExtensionProperty only works on methods where the first parameter is the receiver type.)

@SUPERCILEX

This comment has been minimized.

SUPERCILEX commented Jul 18, 2018

Any status updates from the JetBrains team? This would be extremely useful...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment