Add Support for Static Fields to Java2Swift tool #61
Conversation
ktoso
left a comment
ktoso
left a comment
There was a problem hiding this comment.
Looks good to me but I'll give @DougGregor a chance to answer the question asked 👍
| extension JavaClass<HelloSwift> { | ||
| @JavaField | ||
| @JavaStaticField | ||
| var initialValue: Double |
There was a problem hiding this comment.
should this be a static var?
There was a problem hiding this comment.
It's an instance in JavaClass similar to how static methods are instance methods on their Java class. I was thinking of how to get this be a static var, but everything needs the JNIEnvironment (from my understanding), which is an instance var on all Java structs
There was a problem hiding this comment.
Right. Java static methods/vars become non-static members on the appropriately-specialized JavaClass instance.
There was a problem hiding this comment.
For bonus points, one could have the @JavaStaticField macro detect the static and produce an error diagnosing the problem, but it's not really necessary. Folks shouldn't have to write these macro uses themselves.
There was a problem hiding this comment.
Hmmm do we have access to the surrounding context of a macro? I know you did some work there, but I lost where that ended up.
There was a problem hiding this comment.
Yeah, the lexicalContext of the macro captures this information. To be very clear, I don't think this diagnostic is something that should block your PR from being merged. I just wanted to note that we can implement these kinds of checks in the macro implementation if we want.
DougGregor
left a comment
DougGregor
left a comment
There was a problem hiding this comment.
This looks good to me, with the one minor request to the log message to make it easier to figure out when static fields aren't getting translated.
| extension JavaClass<HelloSwift> { | ||
| @JavaField | ||
| @JavaStaticField | ||
| var initialValue: Double |
There was a problem hiding this comment.
For bonus points, one could have the @JavaStaticField macro detect the static and produce an error diagnosing the problem, but it's not really necessary. Folks shouldn't have to write these macro uses themselves.
|
Nice thank you! |
|
Issue for the follow up : #65 |
|
#73 makes it possible to write unit tests for Java2Swift, so long as you can find some class in the JDK that has the kind of thing you want to test. |
3 participants
Closes #39
This adds support to adding static fields to the Java2Swift tool. The issue recommends adding a
@JavaStaticFieldmacro, but it looks like due to the fact that the subscript created by theJavaFieldmacro works for static properties when defined on aJavaClass, so I'm not sure a@JavaStaticFieldreally buys anything here, so I'm open to removing it.Same testing as in #55 (since that one also adds a static String).
I also noticed that we had a
staticMethodsarray, but were re-requesting all methods in theJava2Swifttool when handling static methods, so I updated it to just use thestaticMethodsarray to simplify the code a bit.