Skip to content

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 11 commits into from
Oct 26, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 38 additions & 7 deletions Sources/Java2Swift/JavaToSwift.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Comment on lines +235 to +244
Copy link
Member

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 produces Java2Swift.config to walk nested classes.

Copy link
Contributor Author

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

Copy link
Member

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 the Attributes.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 file Attributes+Name.swift. To do that, the config file needs to list out the Attributes$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!

Copy link
Member

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!

}
}

// 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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the be a try rather than a try!?

Suggested change
let swiftClassDecls = try! translator.translateClass(javaClass)
let swiftClassDecls = try translator.translateClass(javaClass)

Copy link
Contributor Author

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

let importDecls = translator.getImportDecls()

let swiftFileText = """
Expand All @@ -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) {
Copy link
Member

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

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)")
Expand Down Expand Up @@ -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] =
Expand Down
40 changes: 31 additions & 9 deletions Sources/Java2SwiftLib/JavaTranslator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -265,7 +274,7 @@ extension JavaTranslator {
)

if !enumConstants.isEmpty {
let enumName = "\(swiftTypeName)Cases"
let enumName = "\(swiftTypeName.splitSwiftTypeName().name)Cases"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be swiftInnermostTypeName, right?

Suggested change
let enumName = "\(swiftTypeName.splitSwiftTypeName().name)Cases"
let enumName = "\(swiftInnermostTypeName)Cases"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep missed that

members.append(
contentsOf: translateToEnumValue(name: enumName, enumFields: enumConstants)
)
Expand All @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 =
"""
Expand All @@ -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
)
// Translate static members.
var staticMembers: [DeclSyntax] = []

Expand Down Expand Up @@ -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
Expand Down
92 changes: 92 additions & 0 deletions Sources/JavaKitJar/generated/Attributes.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The 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

"java.util.jar.Attributes$Name" : "Attributes.Name",

to this module's Java2Swift.config.

@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?
}
41 changes: 36 additions & 5 deletions Tests/Java2SwiftTests/Java2SwiftTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ var jvm: JavaVirtualMachine {
}

@JavaClass("java.time.Month")
public struct JavaMonth {

}
public struct JavaMonth {}
@JavaClass("java.lang.ProcessBuilder")
struct ProcessBuilder {}

class Java2SwiftTests: XCTestCase {
func testJavaLangObjectMapping() throws {
Expand Down Expand Up @@ -114,6 +114,36 @@ class Java2SwiftTests: XCTestCase {
]
)
}

func testNestedSubclasses() async throws {
try assertTranslatedClass(
ProcessBuilder.self,
swiftTypeName: "ProcessBuilder",
translatedClasses: [
"java.lang.ProcessBuilder": ("ProcessBuilder", nil, true),
"java.lang.ProcessBuilder$Redirect": ("ProcessBuilder.Redirect", nil, true),
"java.lang.ProcessBuilder$Redirect$Type": ("ProcessBuilder.Redirect.Type", nil, true),
],
expectedChunks: [
"import JavaKit",
"""
@JavaMethod
public func redirectInput() -> ProcessBuilder.Redirect?
""",
"""
extension ProcessBuilder {
@JavaClass("java.lang.ProcessBuilder$Redirect")
public struct Redirect {
""",
"""
extension ProcessBuilder.Redirect {
@JavaClass("java.lang.ProcessBuilder$Redirect$Type")
public struct Type {
"""
]
)
}

}

@JavaClass("java.util.ArrayList")
Expand Down Expand Up @@ -145,9 +175,10 @@ func assertTranslatedClass<JavaClassType: AnyJavaObject>(
translator.translatedClasses = translatedClasses
translator.translatedClasses[javaType.fullJavaClassName] = (swiftTypeName, nil, true)


translator.startNewFile()
let translatedDecls = translator.translateClass(
try JavaClass<JavaObject>(
let translatedDecls = try translator.translateClass(
JavaClass<JavaObject>(
javaThis: javaType.getJNIClass(in: environment),
environment: environment)
)
Expand Down