-
Notifications
You must be signed in to change notification settings - Fork 67
java2swift: Add Support for Enum Cases #76
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
Conversation
| return nil | ||
|
|
||
| if field.isEnumConstant() { | ||
| return "public static let \(raw: field.getName()) = try! JavaClass<Self>(in: JavaVirtualMachine.shared().environment()).\(raw: field.getName())!" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we've going to use JavaVirtualMachine here, we need to also make sure to import JavaKitVM in the generated code. There's a set of imports in the JavaTranslator that you would need to add to.
I do worry a bit about calling JavaVirtualMachine.shared() here in something that looks like a constant, and I wonder if we should consider alternative ways to model Java enums.
|
@jrosen081 your items (2) and (3) have me wondering if we should try different ways of bringing Java enum constants into Swift. For example, we could collect all of the enum cases and put them into a synthesized Swift enum, e.g., /* assuming this is java.time.Month */
extension Month {
public enum EnumConstant {
case JANUARY
case FEBRUARY
// ...
}
public init(_ constant: EnumConstant, environment: JNIEnvironment) { ... }
public var enumConstant: EnumConstant { get }
}Then we could add matching operators for switch and such as needed. Perhaps even factor some of these operations out into a protocol to which Java enum classes would be made to conform. We would no longer require the virtual machine instance nor the What do you think? I'm open to other ideas, too! |
I like it. I think we would need |
New update based on the proposal, my one thought would be to update the |
|
@jrosen081 I definitely like this direction better. Should we also add an initializer that takes a I'm mixed on the idea of converting |
New initializer: It is not ideal imo to be both |
Perhaps it should be neither? The Irrespective of our decisions on the above, I'm really liking this direction! Once you're happy with it, I recommend regenerating the java2swift sources with |
6c5f632 to
4d039b2
Compare
Didn't think of it like that. Went with a
I ran it, but nothing ended up changing (I don't think we are converting any enums right now) |
DougGregor
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left one small comment. This looks great!
| package static let defaultTranslatedClasses: [String: (swiftType: String, swiftModule: String?, isOptional: Bool)] = [ | ||
| "java.lang.Class": ("JavaClass", "JavaKit", true), | ||
| "java.lang.String": ("String", "JavaKit", false), | ||
| "java.lang.Object": ("JavaObject", "JavaKit", true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this change shouldn't be necessary. java.lang.Object is already listed in Java2Swift.config for the JavaKit target, so the only way this could be needed is if something is missing a JavaKit dependency.
Okay! I'm sure we'll find some more enums as we bring more of the Java base types into the JavaKit modules. |
|
LGTM! Looking good, thanks for the contribution! :-) |
|
@jrosen081 merged, thank you! |
Closes #21
Adds a few nice pieces of support for enums:
.APRILfor april static property on Month).There are a few things we could improve here related to the static properties that I'm interested to see if we should update:
try!which is likely not great. If the programmer already is using thejava-swiftwork, they likely have the JVM all set up, so this should be fine, but if not, we might want to remove the static properties or come up with another way of creating the environment statically that is not failing (if that is even possible)Open Question the I'm unsure of:
Equatableconformance for these using theequalsfunction, so that we can get a good implementation of~=for switch statements?~=implementation itself (possibly also usingequals)?