jextract/jni: support private(set) by not importing those setters (#809)

Previously we'd assume the setter was there and generate invalid code.

Author: ktoso

Samples/SwiftJavaExtractJNISampleApp/Sources/MySwiftLibrary/MySwiftClass.swift

@@ -44,6 +44,10 @@ public class MySwiftClass {
     }
   }
 
+  /// `public private(set)` should expose only a getter to Java; the setter
+  /// is unreachable so swift-java must not emit a setter thunk for it
+  public private(set) var privateSetCounter: Int64 = 7
+
   public static func method() {
   }
 

Samples/SwiftJavaExtractJNISampleApp/src/test/java/com/example/swift/MySwiftClassTest.java

@@ -17,6 +17,7 @@
 import org.junit.jupiter.api.Test;
 import org.swift.swiftkit.core.SwiftArena;
 
+import java.lang.reflect.Method;
 import java.util.Optional;
 import java.util.OptionalInt;
 import java.util.OptionalLong;
@@ -187,4 +188,17 @@ void toDebugStringTest() {
             assertEquals("debug: MySwiftClass(x: 20, y: 10)", c1.toDebugString());
         }
     }
+
+    @Test
+    void privateSetCounter_getterOnly() throws Exception {
+        try (var arena = SwiftArena.ofConfined()) {
+            MySwiftClass c = MySwiftClass.init(20, 10, arena);
+            assertEquals(7, c.getPrivateSetCounter());
+        }
+
+        // The setter must not be exposed to Java for `public private(set) var`
+        Method getter = MySwiftClass.class.getMethod("getPrivateSetCounter");
+        assertNotNull(getter);
+        assertThrows(NoSuchMethodException.class, () -> MySwiftClass.class.getMethod("setPrivateSetCounter", long.class));
+    }
 }

Sources/JExtractSwiftLib/FFM/CDeclLowering/FFMSwift2JavaGenerator+FunctionLowering.swift

@@ -59,7 +59,10 @@ extension FFMSwift2JavaGenerator {
     isSet: Bool,
     enclosingType: TypeSyntax? = nil,
   ) throws -> LoweredFunctionSignature? {
-    let supportedAccessors = decl.supportedAccessorKinds(binding: decl.bindings.first!)
+    let supportedAccessors = decl.supportedAccessorKinds(
+      binding: decl.bindings.first!,
+      minimumAccessLevel: self.config.effectiveMinimumInputAccessLevelMode,
+    )
     guard supportedAccessors.contains(isSet ? .set : .get) else {
       return nil
     }

Sources/SwiftExtract/Convenience/SwiftSyntax+Extensions.swift

@@ -94,6 +94,27 @@ extension DeclModifierSyntax {
   }
 }
 
+extension AccessLevelMode {
+  package func matches(_ modifier: DeclModifierSyntax) -> Bool {
+    switch (self, modifier.name.tokenKind) {
+    case (.public, .keyword(.public)),
+      (.public, .keyword(.open)):
+      return true
+    case (.package, .keyword(.public)),
+      (.package, .keyword(.open)),
+      (.package, .keyword(.package)):
+      return true
+    case (.internal, .keyword(.public)),
+      (.internal, .keyword(.open)),
+      (.internal, .keyword(.package)),
+      (.internal, .keyword(.internal)):
+      return true
+    default:
+      return false
+    }
+  }
+}
+
 extension WithModifiersSyntax {
   package func isPublic(in type: NominalTypeDeclSyntaxNode?) -> Bool {
     if let protocolDecl = type?.as(ProtocolDeclSyntax.self) {

Sources/SwiftExtract/SwiftAnalysisVisitor.swift

@@ -286,7 +286,10 @@ final class SwiftAnalysisVisitor {
     self.log.debug("Import variable: \(node.kind) '\(node.qualifiedNameForDebug)'")
 
     do {
-      let supportedAccessors = node.supportedAccessorKinds(binding: binding)
+      let supportedAccessors = node.supportedAccessorKinds(
+        binding: binding,
+        minimumAccessLevel: config.effectiveMinimumInputAccessLevelMode,
+      )
       if supportedAccessors.contains(.get) {
         try importAccessor(
           from: DeclSyntax(node),

Sources/SwiftExtract/SwiftTypes/SwiftFunctionSignature.swift

@@ -458,8 +458,12 @@ extension VariableDeclSyntax {
   /// Determine what operations (i.e. get and/or set) supported in this `VariableDeclSyntax`
   ///
   /// - Parameters:
-  ///   - binding the pattern binding in this declaration.
-  public func supportedAccessorKinds(binding: PatternBindingSyntax) -> AccessorBlockSyntax.SupportedAccessorKinds {
+  ///   - binding: the pattern binding in this declaration.
+  ///   - minimumAccessLevel: the minimum access level being extracted
+  public func supportedAccessorKinds(
+    binding: PatternBindingSyntax,
+    minimumAccessLevel: AccessLevelMode
+  ) -> AccessorBlockSyntax.SupportedAccessorKinds {
     if self.bindingSpecifier.tokenKind == .keyword(.let) {
       return [.get]
     }
@@ -468,6 +472,13 @@ extension VariableDeclSyntax {
       return accessorBlock.supportedAccessorKinds()
     }
 
+    // Account for private(set) and similar modifiers
+    for modifier in self.modifiers where modifier.detail?.detail.text == "set" {
+      if !minimumAccessLevel.matches(modifier) {
+        return [.get]
+      }
+    }
+
     return [.get, .set]
   }
 }

Tests/JExtractSwiftTests/JNI/JNIVariablesTests.swift

@@ -13,6 +13,7 @@
 //===----------------------------------------------------------------------===//
 
 import JExtractSwiftLib
+import SwiftJavaConfigurationShared
 import Testing
 
 @Suite
@@ -35,6 +36,8 @@ struct JNIVariablesTests {
       }
       public var someBoolean: Bool
       public var isBoolean: Bool
+      public private(set) var privateSetCounter: Int64
+      public internal(set) var internalSetCounter: Int64
     }
     """
 
@@ -447,4 +450,135 @@ struct JNIVariablesTests {
       ]
     )
   }
+
+  @Test
+  func privateSetCounter_javaBindings() throws {
+    try assertOutput(
+      input: membersSource,
+      .jni,
+      .java,
+      detectChunkByInitialLines: 8,
+      expectedChunks: [
+        """
+        /**
+         * Downcall to Swift:
+         * {@snippet lang=swift :
+         * public private(set) var privateSetCounter: Int64
+         * }
+         */
+         public long getPrivateSetCounter() {
+           return MyClass.$getPrivateSetCounter(this.$memoryAddress());
+         }
+        """,
+        """
+        private static native long $getPrivateSetCounter(long selfPointer);
+        """,
+      ],
+      notExpectedChunks: [
+        "setPrivateSetCounter",
+        "$setPrivateSetCounter",
+      ]
+    )
+  }
+
+  @Test
+  func privateSetCounter_swiftThunks() throws {
+    try assertOutput(
+      input: membersSource,
+      .jni,
+      .swift,
+      detectChunkByInitialLines: 1,
+      expectedChunks: [
+        """
+        @_cdecl("Java_com_example_swift_MyClass__00024getPrivateSetCounter__J")
+        public func Java_com_example_swift_MyClass__00024getPrivateSetCounter__J(environment: UnsafeMutablePointer<JNIEnv?>!, thisClass: jclass, selfPointer: jlong) -> jlong {
+          ...
+          return selfPointer$.pointee.privateSetCounter.getJNILocalRefValue(in: environment)
+        }
+        """
+      ],
+      notExpectedChunks: [
+        "setPrivateSetCounter"
+      ]
+    )
+  }
+
+  @Test
+  func internalSetCounter_javaBindings() throws {
+    try assertOutput(
+      input: membersSource,
+      .jni,
+      .java,
+      detectChunkByInitialLines: 8,
+      expectedChunks: [
+        """
+        /**
+         * Downcall to Swift:
+         * {@snippet lang=swift :
+         * public internal(set) var internalSetCounter: Int64
+         * }
+         */
+         public long getInternalSetCounter() {
+           return MyClass.$getInternalSetCounter(this.$memoryAddress());
+         }
+        """,
+        """
+        private static native long $getInternalSetCounter(long selfPointer);
+        """,
+      ],
+      notExpectedChunks: [
+        "setInternalSetCounter",
+        "$setInternalSetCounter",
+      ]
+    )
+  }
+
+  @Test
+  func packageSet_javaBindings_atPackageLevel() throws {
+    let src =
+      """
+      public class MyClass {
+        public package(set) var packageSetCounter: Int64
+      }
+      """
+    var config = Configuration()
+    config.minimumInputAccessLevelMode = .package
+    try assertOutput(
+      input: src,
+      config: config,
+      .jni,
+      .java,
+      detectChunkByInitialLines: 8,
+      expectedChunks: [
+        """
+        /**
+         * Downcall to Swift:
+         * {@snippet lang=swift :
+         * public package(set) var packageSetCounter: Int64
+         * }
+         */
+         public long getPackageSetCounter() {
+           return MyClass.$getPackageSetCounter(this.$memoryAddress());
+         }
+        """,
+        """
+        /**
+         * Downcall to Swift:
+         * {@snippet lang=swift :
+         * public package(set) var packageSetCounter: Int64
+         * }
+         */
+         public void setPackageSetCounter(long newValue) {
+           MyClass.$setPackageSetCounter(newValue, this.$memoryAddress());
+         }
+        """,
+        """
+        private static native long $getPackageSetCounter(long selfPointer);
+        """,
+        """
+        private static native void $setPackageSetCounter(long newValue, long selfPointer);
+        """,
+      ]
+    )
+  }
 }

Tests/JExtractSwiftTests/VariableImportTests.swift

@@ -123,4 +123,35 @@ final class VariableImportTests {
       ]
     )
   }
+
+  let class_privateSetInterfaceFile =
+    """
+    public class MySwiftClass {
+      public private(set) var counterInt: Int
+    }
+    """
+
+  @Test("Import: public private(set) var counterInt: Int emits only the getter")
+  func variable_int_privateSet() throws {
+    try assertOutput(
+      input: class_privateSetInterfaceFile,
+      .ffm,
+      .java,
+      swiftModuleName: "FakeModule",
+      detectChunkByInitialLines: 1,
+      expectedChunks: [
+        """
+        public long getCounterInt() throws SwiftIntegerOverflowException {
+          $ensureAlive();
+          long result$checked = swiftjava_FakeModule_MySwiftClass_counterInt$get.call(this.$memorySegment());
+          ...
+        }
+        """
+      ],
+      notExpectedChunks: [
+        "swiftjava_FakeModule_MySwiftClass_counterInt$set",
+        "setCounterInt",
+      ]
+    )
+  }
 }