properly handle Optional::ofNullable, de-dupe generic params; use SortedSet

Author: ktoso

Package.swift

@@ -209,6 +209,7 @@ let package = Package(
     .package(url: "https://github.com/apple/swift-argument-parser", from: "1.5.0"),
     .package(url: "https://github.com/apple/swift-system", from: "1.4.0"),
     .package(url: "https://github.com/apple/swift-log", from: "1.2.0"),
+    .package(url: "https://github.com/apple/swift-collections", .upToNextMinor(from: "1.3.0")), // primarily for ordered collections
 
 //    // FIXME: swift-subprocess stopped supporting 6.0 when it moved into a package;
 //    //        we'll need to drop 6.0 as well, but currently blocked on doing so by swiftpm plugin pending design questions
@@ -400,6 +401,7 @@ let package = Package(
       name: "SwiftJavaToolLib",
       dependencies: [
         .product(name: "Logging", package: "swift-log"),
+        .product(name: "OrderedCollections", package: "swift-collections"),
         .product(name: "SwiftBasicFormat", package: "swift-syntax"),
         .product(name: "SwiftSyntax", package: "swift-syntax"),
         .product(name: "SwiftSyntaxBuilder", package: "swift-syntax"),

Sources/SwiftJava/Macros.swift

@@ -150,11 +150,13 @@ public macro JavaMethod(
 /// The macro must be used within a specific JavaClass instance.
 ///
 /// ```swift
-/// @JavaMethod
+/// @JavaStaticMethod
 /// func sayHelloBack(_ i: Int32) -> Double
 /// ```
 @attached(body)
-public macro JavaStaticMethod() = #externalMacro(module: "SwiftJavaMacros", type: "JavaMethodMacro")
+public macro JavaStaticMethod(
+  genericResult: String? = nil
+) = #externalMacro(module: "SwiftJavaMacros", type: "JavaMethodMacro")
 
 /// Macro that marks extensions to specify that all of the @JavaMethod
 /// methods are implementations of Java methods spelled as `native`.

Sources/SwiftJava/generated/JavaLong.swift

@@ -140,9 +140,6 @@ extension JavaClass<JavaLong> {
   @JavaStaticMethod
   public func decode(_ arg0: String) throws -> JavaLong!
 
-  @JavaStaticMethod
-  public func highestOneBit(_ arg0: Int64) -> Int64
-
   @JavaStaticMethod
   public func sum(_ arg0: Int64, _ arg1: Int64) -> Int64
 
@@ -158,6 +155,9 @@ extension JavaClass<JavaLong> {
   @JavaStaticMethod
   public func toBinaryString(_ arg0: Int64) -> String
 
+  @JavaStaticMethod
+  public func highestOneBit(_ arg0: Int64) -> Int64
+
   @JavaStaticMethod
   public func lowestOneBit(_ arg0: Int64) -> Int64
 

Sources/SwiftJava/generated/JavaOptionalDouble.swift

@@ -16,10 +16,10 @@ open class JavaOptionalDouble: JavaObject {
   open func isEmpty() -> Bool
 
   @JavaMethod
-  open func isPresent() -> Bool
+  open func orElse(_ arg0: Double) -> Double
 
   @JavaMethod
-  open func orElse(_ arg0: Double) -> Double
+  open func isPresent() -> Bool
 
   @JavaMethod
   open func orElseThrow() -> Double

Sources/SwiftJavaToolLib/JavaClassTranslator.swift

@@ -15,6 +15,7 @@
 import SwiftJava
 import JavaLangReflect
 import SwiftSyntax
+import OrderedCollections
 import SwiftJavaConfigurationShared
 import Logging
 
@@ -606,25 +607,35 @@ extension JavaClassTranslator {
     return false
   }
 
-  /// Translates the given Java method into a Swift declaration.
-  package func renderMethod(
-    _ javaMethod: Method,
-    implementedInSwift: Bool,
-    genericParameters: [String] = [],
-    whereClause: String = ""
-  ) throws -> DeclSyntax {
-    // Map the generic params on the method.
-    var allGenericParameters = genericParameters
+  // TODO: make it more precise with the "origin" of the generic parameter (outer class etc)
+  func collectMethodGenericParameters(
+    genericParameters: [String],
+    method javaMethod: Method
+  ) -> OrderedSet<String> {
+    var allGenericParameters = OrderedSet(genericParameters)
     let typeParameters = javaMethod.getTypeParameters()
     if typeParameters.contains(where: {$0 != nil }) {
-      allGenericParameters += typeParameters.compactMap { typeParam in 
+      allGenericParameters.appending(contentsOf: typeParameters.compactMap { typeParam in 
         guard let typeParam else { return nil }
         guard genericParameterIsUsedInSignature(typeParam, in: javaMethod) else {
           return nil
         }
         return "\(typeParam.getTypeName()): AnyJavaObject"
-      }
+      })
     }
+
+    return allGenericParameters
+  }
+
+  /// Translates the given Java method into a Swift declaration.
+  package func renderMethod(
+    _ javaMethod: Method,
+    implementedInSwift: Bool,
+    genericParameters: [String] = [],
+    whereClause: String = ""
+  ) throws -> DeclSyntax {
+    // Map the generic params on the method.
+    let allGenericParameters = collectMethodGenericParameters(genericParameters: genericParameters, method: javaMethod)
     let genericParameterClauseStr = 
       if allGenericParameters.isEmpty {
         ""
@@ -657,7 +668,7 @@ extension JavaClassTranslator {
     let throwsStr = javaMethod.throwsCheckedException ? "throws" : ""
     let swiftMethodName = javaMethod.getName().escapedSwiftName
 
-    // Compute the '@...JavaMethod(...)' details
+    // Compute the parameters for '@...JavaMethod(...)'
     let methodAttribute: AttributeSyntax
       if implementedInSwift {
         methodAttribute = ""
@@ -674,6 +685,7 @@ extension JavaClassTranslator {
           parameters.append("genericResult: \"\(resultType)\"")
         }
         // TODO: generic parameters?
+        
         if !parameters.isEmpty {
           methodAttributeStr += "("
           methodAttributeStr.append(parameters.joined(separator: ", "))
@@ -690,6 +702,7 @@ extension JavaClassTranslator {
       ? "override "
       : ""
 
+    // FIXME: refactor this so we don't have to duplicate the method signatures
     if resultType.optionalWrappedType() != nil || parameters.contains(where: { $0.type.trimmedDescription.optionalWrappedType() != nil }) {
       let parameters = parameters.map { param -> (clause: FunctionParameterSyntax, passedArg: String) in
         let name = param.secondName!.trimmedDescription
@@ -711,15 +724,17 @@ extension JavaClassTranslator {
         }
 
 
-      return """
+      return 
+        """
         \(methodAttribute)\(raw: accessModifier)\(raw: overrideOpt)func \(raw: swiftMethodName)\(raw: genericParameterClauseStr)(\(raw: parametersStr))\(raw: throwsStr)\(raw: resultTypeStr)\(raw: whereClause)
         
         \(raw: accessModifier)\(raw: overrideOpt)func \(raw: swiftMethodName)Optional\(raw: genericParameterClauseStr)(\(raw: parameters.map(\.clause.description).joined(separator: ", ")))\(raw: throwsStr) -> \(raw: resultOptional)\(raw: whereClause) {
           \(body)
         }
         """
     } else {
-      return """
+      return 
+        """
         \(methodAttribute)\(raw: accessModifier)\(raw: overrideOpt)func \(raw: swiftMethodName)\(raw: genericParameterClauseStr)(\(raw: parametersStr))\(raw: throwsStr)\(raw: resultTypeStr)\(raw: whereClause)
         """
     }
@@ -900,6 +915,7 @@ struct MethodCollector {
 }
 
 // MARK: Utility functions
+
 extension JavaClassTranslator {
   /// Determine whether this method is an override of another Java
   /// method.
@@ -935,7 +951,7 @@ extension JavaClassTranslator {
           return true
         }
       } catch {
-        // FIXME: logging
+        log.warning("Failed to determine if method '\(method)' is an override, error: \(error)")
       }
     }
 

Sources/SwiftJavaToolLib/JavaTranslator.swift

@@ -138,12 +138,11 @@ extension JavaTranslator {
 
     // Special handle the case when the return type is the generic type of the method: `<T> T foo()`
 
-    // if isGenericJavaType(genericReturnType) {
-    //   print("[swift] generic method! \(method.getDeclaringClass().getName()).\(method.getName())")
-    //   getGenericJavaTypeOriginInfo(genericReturnType, from: method)
-    // }
-
-    return try getSwiftTypeNameAsString(method: method, genericReturnType!, preferValueTypes: preferValueTypes, outerOptional: outerOptional)
+    return try getSwiftTypeNameAsString(
+      method: method,
+      genericReturnType!, 
+      preferValueTypes: preferValueTypes, 
+      outerOptional: outerOptional)
   }
 
   /// Turn a Java type into a string.

Tests/SwiftJavaToolLibTests/WrapJavaTests/BasicWrapJavaTests.swift

@@ -0,0 +1,52 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2024 Apple Inc. and the Swift.org project authors
+// Licensed under Apache License v2.0
+//
+// See LICENSE.txt for license information
+// See CONTRIBUTORS.txt for the list of Swift.org project authors
+//
+// SPDX-License-Identifier: Apache-2.0
+//
+//===----------------------------------------------------------------------===//
+
+@_spi(Testing) import SwiftJava
+import SwiftJavaToolLib
+import JavaUtilJar
+import SwiftJavaShared
+import JavaNet
+import SwiftJavaConfigurationShared
+import _Subprocess
+import XCTest // NOTE: Workaround for https://github.com/swiftlang/swift-java/issues/43
+
+final class BasicWrapJavaTests: XCTestCase {
+
+  func testWrapJavaFromCompiledJavaSource() async throws {
+    let classpathURL = try await compileJava(
+      """
+      package com.example;
+
+      class ExampleSimpleClass {}
+      """)
+
+    try assertWrapJavaOutput(
+      javaClassNames: [
+        "com.example.ExampleSimpleClass"
+      ],
+      classpath: [classpathURL],
+      expectedChunks: [
+        """
+        import CSwiftJavaJNI
+        import SwiftJava
+        """,
+        """
+        @JavaClass("com.example.ExampleSimpleClass")
+        open class ExampleSimpleClass: JavaObject {
+        """
+      ]
+    )
+  }
+
+}

…wiftJavaToolLibTests/WrapJavaTests.swift …rapJavaTests/GenericsWrapJavaTests.swiftTests/SwiftJavaToolLibTests/WrapJavaTests.swift renamed to Tests/SwiftJavaToolLibTests/WrapJavaTests/GenericsWrapJavaTests.swift Tests/SwiftJavaToolLibTests/WrapJavaTests.swift renamed to Tests/SwiftJavaToolLibTests/WrapJavaTests/GenericsWrapJavaTests.swift

@@ -21,35 +21,8 @@ import SwiftJavaConfigurationShared
 import _Subprocess
 import XCTest // NOTE: Workaround for https://github.com/swiftlang/swift-java/issues/43
 
-final class WrapJavaTests: XCTestCase {
+final class GenericsWrapJavaTests: XCTestCase {
 
-  func testWrapJavaFromCompiledJavaSource() async throws {
-    let classpathURL = try await compileJava(
-      """
-      package com.example;
-
-      class ExampleSimpleClass {}
-      """)
-
-    try assertWrapJavaOutput(
-      javaClassNames: [
-        "com.example.ExampleSimpleClass"
-      ],
-      classpath: [classpathURL],
-      expectedChunks: [
-        """
-        import CSwiftJavaJNI
-        import SwiftJava
-        """,
-        """
-        @JavaClass("com.example.ExampleSimpleClass")
-        open class ExampleSimpleClass: JavaObject {
-        """
-      ]
-    )
-  }
-
-  // @Test
   func testWrapJavaGenericMethod_singleGeneric() async throws {
     let classpathURL = try await compileJava(
       """
@@ -271,7 +244,7 @@ final class WrapJavaTests: XCTestCase {
     )
   }
 
-  func testWrapJavaGenericMethodTypeErasure_returnType() async throws {
+  func test_wrapJava_genericMethodTypeErasure_returnType() async throws {
     let classpathURL = try await compileJava(
       """
       package com.example;
@@ -301,4 +274,37 @@ final class WrapJavaTests: XCTestCase {
       ]
     )
   }
-}
+  
+  func test_wrapJava_genericMethodTypeErasure_ofNullableOptional() async throws {
+    let classpathURL = try await compileJava(
+      """
+      package com.example;
+
+      final class Optional<T> {
+        public static <T> Optional<T> ofNullable(T value) { return null; }
+      }
+      """)
+
+    try assertWrapJavaOutput(
+      javaClassNames: [
+        "com.example.Optional"
+      ],
+      classpath: [classpathURL],
+      expectedChunks: [
+        """
+        @JavaClass("com.example.Optional")
+        open class Optional<T: AnyJavaObject>: JavaObject {
+
+        }
+        """,
+        """
+        extension JavaClass {
+          @JavaStaticMethod(genericResult: "Optional<T>!")
+          public func ofNullable<T: AnyJavaObject>(_ arg0: T?) -> Optional<T>! where ObjectType == Optional<T>
+        }
+        """
+      ]
+    )
+  }
+  
+}