Skip to content

Commit 8414bf8

Browse files
committed
wrap-java: correct importing nested types
We cannot just blindly visit all getClasses() because this includes types form super classes as well. This is useful in general, but in this context we specifically want types which are nested in this exact declaration, not its parens.
1 parent 99ae030 commit 8414bf8

File tree

4 files changed

+64
-17
lines changed

4 files changed

+64
-17
lines changed

Sources/SwiftJava/String+Extensions.swift

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,7 @@
1515
import Foundation
1616

1717
extension String {
18-
/// For a String that's of the form java.util.Vector, return the "Vector"
19-
/// part.
18+
/// For a String that's of the form java.util.Vector, return the "Vector" part.
2019
package var defaultSwiftNameForJavaClass: String {
2120
if let dotLoc = lastIndex(of: ".") {
2221
let afterDot = index(after: dotLoc)

Sources/SwiftJavaTool/Commands/WrapJavaCommand.swift

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -180,28 +180,39 @@ extension SwiftJava.WrapJavaCommand {
180180
// of classes to be translated if they were already specified.
181181
var allClassesToVisit = javaClasses
182182
var currentClassIndex: Int = 0
183-
while currentClassIndex < allClassesToVisit.count {
183+
outerClassLoop: while currentClassIndex < allClassesToVisit.count {
184184
defer {
185185
currentClassIndex += 1
186186
}
187187

188-
// The current class we're in.
188+
// The current top-level class we're in.
189189
let currentClass = allClassesToVisit[currentClassIndex]
190+
let currentClassName = currentClass.getName()
190191
guard let currentSwiftName = translator.translatedClasses[currentClass.getName()]?.swiftType else {
191192
continue
192193
}
193194

194-
// Find all of the nested classes that weren't explicitly translated
195-
// already.
196-
let nestedClasses: [JavaClass<JavaObject>] = currentClass.getClasses().compactMap { nestedClass in
197-
guard let nestedClass else { return nil }
195+
// Find all of the nested classes that weren't explicitly translated already.
196+
let nestedAndSuperclassNestedClasses = currentClass.getClasses() // watch out, this includes nested types from superclasses
197+
let nestedClasses: [JavaClass<JavaObject>] = nestedAndSuperclassNestedClasses.compactMap { nestedClass in
198+
guard let nestedClass else {
199+
return nil
200+
}
198201

199202
// If this is a local class, we're done.
200203
let javaClassName = nestedClass.getName()
201204
if javaClassName.isLocalJavaClass {
202205
return nil
203206
}
204207

208+
// We only want to visit and import types which are explicitly inside this decl,
209+
// and NOT any of the types contained in the super classes. That would violate our "current class"
210+
// nesting, because those are *actually* nested in the other class, not "the current one" (i.e. in a super class).
211+
guard javaClassName.hasPrefix(currentClassName) else {
212+
log.trace("Skip super-class nested class '\(javaClassName)', is not member of \(currentClassName). Will be visited independently.")
213+
return nil
214+
}
215+
205216
// If we have an inclusive filter, import only types from it
206217
for include in config.filterInclude ?? [] {
207218
guard javaClassName.starts(with: include) else {
@@ -227,7 +238,9 @@ extension SwiftJava.WrapJavaCommand {
227238
.defaultSwiftNameForJavaClass
228239

229240
let swiftName = "\(currentSwiftName).\(swiftUnqualifiedName)"
230-
translator.translatedClasses[javaClassName] = SwiftTypeName(module: nil, name: swiftName)
241+
let translatedSwiftName = SwiftTypeName(module: nil, name: swiftName)
242+
translator.translatedClasses[javaClassName] = translatedSwiftName
243+
log.debug("Record translated Java class '\(javaClassName)' -> \(translatedSwiftName)")
231244
return nestedClass
232245
}
233246

Sources/SwiftJavaToolLib/JavaTranslator+Validation.swift

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,10 @@ package extension JavaTranslator {
5858
package var description: String {
5959
switch self {
6060
case .multipleClassesMappedToSameName(let swiftToJavaMapping):
61-
"""
62-
The following Java classes were mapped to the same Swift type name:
63-
\(swiftToJavaMapping.map(mappingDescription(mapping:)).joined(separator: "\n"))
64-
"""
61+
"""
62+
The following Java classes were mapped to the same Swift type name:
63+
\(swiftToJavaMapping.map(mappingDescription(mapping:)).joined(separator: "\n"))
64+
"""
6565
}
6666
}
6767

@@ -72,10 +72,6 @@ package extension JavaTranslator {
7272
}
7373
}
7474
func validateClassConfiguration() throws(ValidationError) {
75-
// for a in translatedClasses {
76-
// print("MAPPING = \(a.key) -> \(a.value.swiftModule?.escapedSwiftName ?? "").\(a.value.swiftType.escapedSwiftName)")
77-
// }
78-
7975
// Group all classes by swift name
8076
let groupedDictionary: [SwiftTypeName: [(JavaFullyQualifiedTypeName, SwiftTypeName)]] = Dictionary(grouping: translatedClasses, by: {
8177
// SwiftTypeName(swiftType: $0.value.swiftType, swiftModule: $0.value.swiftModule)

Tests/SwiftJavaToolLibTests/WrapJavaTests/BasicWrapJavaTests.swift

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,4 +49,43 @@ final class BasicWrapJavaTests: XCTestCase {
4949
)
5050
}
5151

52+
func test_wrapJava_doNotDupeImportNestedClassesFromSuperclassAutomatically() async throws {
53+
let classpathURL = try await compileJava(
54+
"""
55+
package com.example;
56+
57+
class SuperClass {
58+
class SuperNested {}
59+
}
60+
61+
class ExampleSimpleClass {
62+
class SimpleNested {}
63+
}
64+
""")
65+
66+
try assertWrapJavaOutput(
67+
javaClassNames: [
68+
"com.example.SuperClass",
69+
"com.example.SuperClass$SuperNested",
70+
"com.example.ExampleSimpleClass",
71+
],
72+
classpath: [classpathURL],
73+
expectedChunks: [
74+
"""
75+
@JavaClass("com.example.SuperClass")
76+
open class SuperClass: JavaObject {
77+
""",
78+
// FIXME: the mapping configuration could be used to nest this properly but today we don't automatically?
79+
"""
80+
@JavaClass("com.example.SuperClass$SuperNested")
81+
open class SuperNested: JavaObject {
82+
""",
83+
"""
84+
@JavaClass("com.example.SuperClass")
85+
open class SuperClass: JavaObject {
86+
""",
87+
]
88+
)
89+
}
90+
5291
}

0 commit comments

Comments
 (0)