-
Notifications
You must be signed in to change notification settings - Fork 68
Add Support for Nested Classes #115
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
Changes from 10 commits
de290a9
f18d327
720c2cf
2f246e4
2a37c20
647d993
49c6818
e12d92d
82e86fd
3e86f70
c0f6321
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -228,12 +228,27 @@ struct JavaToSwift: ParsableCommand { | |||||
|
|
||||||
| // Note that we will be translating this Java class, so it is a known class. | ||||||
| translator.translatedClasses[javaClassName] = (translatedSwiftName, nil, true) | ||||||
|
|
||||||
| var classes: [JavaClass<JavaObject>?] = javaClass.getClasses() | ||||||
|
|
||||||
| // Go through all subclasses to find all of the classes to translate | ||||||
| while let internalClass = classes.popLast() { | ||||||
| if let internalClass { | ||||||
| let (javaName, swiftName) = names(from: internalClass.getName()) | ||||||
| // If we have already been through this class, don't go through it again | ||||||
| guard translator.translatedClasses[javaName] == nil else { continue } | ||||||
| let currentClassName = swiftName | ||||||
| let currentSanitizedClassName = currentClassName.replacing("$", with: ".") | ||||||
| classes.append(contentsOf: internalClass.getClasses()) | ||||||
| translator.translatedClasses[javaName] = (currentSanitizedClassName, nil, true) | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| // Translate all of the Java classes into Swift classes. | ||||||
| for javaClass in javaClasses { | ||||||
| translator.startNewFile() | ||||||
| let swiftClassDecls = translator.translateClass(javaClass) | ||||||
| let swiftClassDecls = try! translator.translateClass(javaClass) | ||||||
|
||||||
| let swiftClassDecls = try! translator.translateClass(javaClass) | |
| let swiftClassDecls = try translator.translateClass(javaClass) |
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 was just following what was there before (there was a try! inside, I just moved it out). I'm fine making this a try
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.
Thanks for pulling this out
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -190,15 +190,24 @@ extension JavaTranslator { | |||||
| /// Translates the given Java class into the corresponding Swift type. This | ||||||
| /// can produce multiple declarations, such as a separate extension of | ||||||
| /// JavaClass to house static methods. | ||||||
| package func translateClass(_ javaClass: JavaClass<JavaObject>) -> [DeclSyntax] { | ||||||
| let fullName = javaClass.getCanonicalName() | ||||||
| let swiftTypeName = try! getSwiftTypeNameFromJavaClassName(fullName) | ||||||
| package func translateClass(_ javaClass: JavaClass<JavaObject>) throws -> [DeclSyntax] { | ||||||
| let fullName = javaClass.getName() | ||||||
| let swiftTypeName = try getSwiftTypeNameFromJavaClassName(fullName) | ||||||
| let (swiftParentType, swiftInnermostTypeName) = swiftTypeName.splitSwiftTypeName() | ||||||
|
|
||||||
| // If the swift parent type has not been translated, don't try to translate this one | ||||||
| if let swiftParentType, | ||||||
| !translatedClasses.contains(where: { _, value in value.swiftType == swiftParentType }) | ||||||
| { | ||||||
| logUntranslated("Unable to translate '\(fullName)' parent class: \(swiftParentType) not found") | ||||||
| return [] | ||||||
| } | ||||||
|
|
||||||
| // Superclass. | ||||||
| let extends: String | ||||||
| if !javaClass.isInterface(), | ||||||
| let superclass = javaClass.getSuperclass(), | ||||||
| superclass.getCanonicalName() != "java.lang.Object" | ||||||
| superclass.getName() != "java.lang.Object" | ||||||
| { | ||||||
| do { | ||||||
| extends = ", extends: \(try getSwiftTypeName(superclass).swiftName).self" | ||||||
|
|
@@ -265,7 +274,7 @@ extension JavaTranslator { | |||||
| ) | ||||||
|
|
||||||
| if !enumConstants.isEmpty { | ||||||
| let enumName = "\(swiftTypeName)Cases" | ||||||
| let enumName = "\(swiftTypeName.splitSwiftTypeName().name)Cases" | ||||||
|
||||||
| let enumName = "\(swiftTypeName.splitSwiftTypeName().name)Cases" | |
| let enumName = "\(swiftInnermostTypeName)Cases" |
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.
yep missed that
DougGregor marked this conversation as resolved.
Show resolved
Hide resolved
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -44,6 +44,9 @@ public struct Attributes { | |
| @JavaMethod | ||
| public func getValue(_ arg0: String) -> String | ||
|
|
||
| @JavaMethod | ||
| public func getValue(_ arg0: Attributes.Name?) -> String | ||
|
|
||
| @JavaMethod | ||
| public func isEmpty() -> Bool | ||
|
|
||
|
|
@@ -95,3 +98,92 @@ public struct Attributes { | |
| @JavaMethod | ||
| public func getOrDefault(_ arg0: JavaObject?, _ arg1: JavaObject?) -> JavaObject? | ||
| } | ||
| extension Attributes { | ||
| @JavaClass("java.util.jar.Attributes$Name") | ||
| public struct Name { | ||
|
Comment on lines
+101
to
+103
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a wonderful improvement, but I think it should be driven by adding to this module's |
||
| @JavaMethod | ||
| public init(_ arg0: String, environment: JNIEnvironment? = nil) | ||
|
|
||
| @JavaMethod | ||
| public func equals(_ arg0: JavaObject?) -> Bool | ||
|
|
||
| @JavaMethod | ||
| public func toString() -> String | ||
|
|
||
| @JavaMethod | ||
| public func hashCode() -> Int32 | ||
|
|
||
| @JavaMethod | ||
| public func getClass() -> JavaClass<JavaObject>? | ||
|
|
||
| @JavaMethod | ||
| public func notify() | ||
|
|
||
| @JavaMethod | ||
| public func notifyAll() | ||
|
|
||
| @JavaMethod | ||
| public func wait(_ arg0: Int64) throws | ||
|
|
||
| @JavaMethod | ||
| public func wait(_ arg0: Int64, _ arg1: Int32) throws | ||
|
|
||
| @JavaMethod | ||
| public func wait() throws | ||
| } | ||
| } | ||
| extension JavaClass<Attributes.Name> { | ||
| @JavaStaticField | ||
| public var MANIFEST_VERSION: Attributes.Name? | ||
|
|
||
| @JavaStaticField | ||
| public var SIGNATURE_VERSION: Attributes.Name? | ||
|
|
||
| @JavaStaticField | ||
| public var CONTENT_TYPE: Attributes.Name? | ||
|
|
||
| @JavaStaticField | ||
| public var CLASS_PATH: Attributes.Name? | ||
|
|
||
| @JavaStaticField | ||
| public var MAIN_CLASS: Attributes.Name? | ||
|
|
||
| @JavaStaticField | ||
| public var SEALED: Attributes.Name? | ||
|
|
||
| @JavaStaticField | ||
| public var EXTENSION_LIST: Attributes.Name? | ||
|
|
||
| @JavaStaticField | ||
| public var EXTENSION_NAME: Attributes.Name? | ||
|
|
||
| @JavaStaticField | ||
| public var EXTENSION_INSTALLATION: Attributes.Name? | ||
|
|
||
| @JavaStaticField | ||
| public var IMPLEMENTATION_TITLE: Attributes.Name? | ||
|
|
||
| @JavaStaticField | ||
| public var IMPLEMENTATION_VERSION: Attributes.Name? | ||
|
|
||
| @JavaStaticField | ||
| public var IMPLEMENTATION_VENDOR: Attributes.Name? | ||
|
|
||
| @JavaStaticField | ||
| public var IMPLEMENTATION_VENDOR_ID: Attributes.Name? | ||
|
|
||
| @JavaStaticField | ||
| public var IMPLEMENTATION_URL: Attributes.Name? | ||
|
|
||
| @JavaStaticField | ||
| public var SPECIFICATION_TITLE: Attributes.Name? | ||
|
|
||
| @JavaStaticField | ||
| public var SPECIFICATION_VERSION: Attributes.Name? | ||
|
|
||
| @JavaStaticField | ||
| public var SPECIFICATION_VENDOR: Attributes.Name? | ||
|
|
||
| @JavaStaticField | ||
| public var MULTI_RELEASE: Attributes.Name? | ||
| } | ||
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 don't think we should always recurse into nested classes. Rather, I think we should allow specification of nested classes in
Java2Swift.config, and teach the Jar walker that producesJava2Swift.configto walk nested classes.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 went back and forth on this a bit. I do like the idea of recurring down nested classes, due to the fact that the majority of uses of nested classes (at least in the JDK) are from the original class (i.e. methods that return nested classes from the main class)). I'm open to either, but like this as making a more complete class translation
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.
It does make the result more complete with less work on the config file, I agree. I have two concerns, but now I'm thinking that they aren't that important and can be addressed in the future.
One is that this means there is no way to disable the generation of the Swift import of a nested class: maybe we never need that, but at least for the moment I've avoided importing some Java classes because they trip up the translation in ways I wasn't able to immediately deal with. That said, if it became a problem, we could invent a way to do this.
The other is in how the output is structured. With this automatic recursion, the emitted file
Attributes.swiftnow has theAttributes.Nametype in it as well, i.e., the whole nested class definition is in one file. With the explicit recursion approach,Attributes.Nameends up in a separate fileAttributes+Name.swift. To do that, the config file needs to list out theAttributes$Nameclass as its own entry so that the Java2SwiftPlugin can know all of the files that will be emitted. It's more tidy for Swift, but it's not strictly necessary... and it would be relatively straightforward in the future to support being explicit in this way in addition to the recursion that you've implemented. For example, we could say that if you have an entry in the Java2Swift.config file for a nested class, we skip that nested class in the recursion because it's been handled at the top level already.That's all a very, very longwinded way of saying that I agree with you now and will likely implement the "explicit" mechanism I described on top of your PR. Let's merge away!
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 updated #118 to build on top of this. The Jar reader now emits nested classes (which is great), so we have to deal with some nested classes being explicitly-specified in
Java2Swift.configand others that are discovered through the recursion. Thank you for working on nested classes!