-
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
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) | ||||||
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. Shouldn't the be a
Suggested change
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. 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 |
||||||
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] = | ||||||
|
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" | ||||||
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 can be
Suggested change
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. yep missed that |
||||||
members.append( | ||||||
contentsOf: translateToEnumValue(name: enumName, enumFields: enumConstants) | ||||||
) | ||||||
|
@@ -278,7 +287,7 @@ extension JavaTranslator { | |||||
do { | ||||||
let implementedInSwift = constructor.isNative && | ||||||
constructor.getDeclaringClass()!.equals(javaClass.as(JavaObject.self)!) && | ||||||
swiftNativeImplementations.contains(javaClass.getCanonicalName()) | ||||||
swiftNativeImplementations.contains(javaClass.getName()) | ||||||
|
||||||
let translated = try translateConstructor( | ||||||
constructor, | ||||||
|
@@ -312,7 +321,7 @@ extension JavaTranslator { | |||||
|
||||||
let implementedInSwift = method.isNative && | ||||||
method.getDeclaringClass()!.equals(javaClass.as(JavaObject.self)!) && | ||||||
swiftNativeImplementations.contains(javaClass.getCanonicalName()) | ||||||
swiftNativeImplementations.contains(javaClass.getName()) | ||||||
|
||||||
// Translate the method if we can. | ||||||
do { | ||||||
|
@@ -357,7 +366,6 @@ extension JavaTranslator { | |||||
} | ||||||
|
||||||
// Emit the struct declaration describing the java class. | ||||||
let (swiftParentType, swiftInnermostTypeName) = swiftTypeName.splitSwiftTypeName() | ||||||
let classOrInterface: String = javaClass.isInterface() ? "JavaInterface" : "JavaClass"; | ||||||
var classDecl = | ||||||
""" | ||||||
|
@@ -383,6 +391,20 @@ extension JavaTranslator { | |||||
|
||||||
topLevelDecls.append(classDecl) | ||||||
|
||||||
let subClassDecls = javaClass.getClasses().compactMap { | ||||||
$0.flatMap { clazz in | ||||||
do { | ||||||
return try translateClass(clazz) | ||||||
} catch { | ||||||
logUntranslated("Unable to translate '\(fullName)' subclass '\(clazz.getName())': \(error)") | ||||||
return nil | ||||||
} | ||||||
} | ||||||
}.flatMap(\.self) | ||||||
|
||||||
topLevelDecls.append( | ||||||
contentsOf: subClassDecls | ||||||
) | ||||||
DougGregor marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
// Translate static members. | ||||||
var staticMembers: [DeclSyntax] = [] | ||||||
|
||||||
|
@@ -438,7 +460,7 @@ extension JavaTranslator { | |||||
// Members that are native and will instead go into a NativeMethods | ||||||
// protocol. | ||||||
var nativeMembers: [DeclSyntax] = [] | ||||||
if swiftNativeImplementations.contains(javaClass.getCanonicalName()) { | ||||||
if swiftNativeImplementations.contains(javaClass.getName()) { | ||||||
nativeMembers.append( | ||||||
contentsOf: javaClass.getDeclaredMethods().compactMap { | ||||||
$0.flatMap { method in | ||||||
|
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? | ||
} |
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!