-
Notifications
You must be signed in to change notification settings - Fork 48
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
Merged
Merged
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
de290a9
First pass
jrosen081 f18d327
Keep going
jrosen081 720c2cf
Add Tests for Subclasses
jrosen081 2f246e4
Fix issues with nested classes
jrosen081 2a37c20
Remove $ checks
jrosen081 647d993
Correctly support $
jrosen081 49c6818
remove tick
jrosen081 e12d92d
Add Subclass Generated
jrosen081 82e86fd
Update enums and tests
jrosen081 3e86f70
Fix incorrect comment
jrosen081 c0f6321
Some pr feedback
jrosen081 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 importDecls = translator.getImportDecls() | ||
|
||
let swiftFileText = """ | ||
|
@@ -247,11 +262,32 @@ struct JavaToSwift: ParsableCommand { | |
try writeContents( | ||
swiftFileText, | ||
to: swiftFileName, | ||
description: "Java class '\(javaClass.getCanonicalName())' translation" | ||
description: "Java class '\(javaClass.getName())' translation" | ||
) | ||
} | ||
} | ||
|
||
private func names(from javaClassNameOpt: String) -> (javaClassName: String, swiftName: String) { | ||
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. Thanks for pulling this out |
||
let javaClassName: String | ||
let swiftName: String | ||
if let equalLoc = javaClassNameOpt.firstIndex(of: "=") { | ||
let afterEqual = javaClassNameOpt.index(after: equalLoc) | ||
javaClassName = String(javaClassNameOpt[..<equalLoc]) | ||
swiftName = String(javaClassNameOpt[afterEqual...]) | ||
} else { | ||
if let dotLoc = javaClassNameOpt.lastIndex(of: ".") { | ||
let afterDot = javaClassNameOpt.index(after: dotLoc) | ||
swiftName = String(javaClassNameOpt[afterDot...]) | ||
} else { | ||
swiftName = javaClassNameOpt | ||
} | ||
|
||
javaClassName = javaClassNameOpt | ||
} | ||
|
||
return (javaClassName, swiftName) | ||
} | ||
|
||
mutating func writeContents(_ contents: String, to filename: String, description: String) throws { | ||
guard let outputDir = actualOutputDirectory else { | ||
print("// \(filename) - \(description)") | ||
|
@@ -298,11 +334,6 @@ struct JavaToSwift: ParsableCommand { | |
} | ||
} | ||
|
||
// TODO: For now, skip all nested classes. | ||
if entry.getName().contains("$") { | ||
continue | ||
} | ||
|
||
let javaCanonicalName = String(entry.getName().replacing("/", with: ".") | ||
.dropLast(".class".count)) | ||
configuration.classes[javaCanonicalName] = | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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
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? | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.config
to 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.swift
now has theAttributes.Name
type in it as well, i.e., the whole nested class definition is in one file. With the explicit recursion approach,Attributes.Name
ends up in a separate fileAttributes+Name.swift
. To do that, the config file needs to list out theAttributes$Name
class 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.config
and others that are discovered through the recursion. Thank you for working on nested classes!