From bfa800bc68c2635abb1e1cc9b11f9f0a0ae5d931 Mon Sep 17 00:00:00 2001 From: Konrad Malawski Date: Tue, 30 Jun 2026 16:10:29 +0900 Subject: [PATCH] jextract/jni: support private(set) by not importing those setters Previously we'd assume the setter was there and generate invalid code. --- .../Sources/MySwiftLibrary/MySwiftClass.swift | 4 + .../com/example/swift/MySwiftClassTest.java | 14 ++ ...Swift2JavaGenerator+FunctionLowering.swift | 5 +- .../Convenience/SwiftSyntax+Extensions.swift | 21 +++ .../SwiftExtract/SwiftAnalysisVisitor.swift | 5 +- .../SwiftTypes/SwiftFunctionSignature.swift | 15 +- .../JNI/JNIVariablesTests.swift | 134 ++++++++++++++++++ .../VariableImportTests.swift | 31 ++++ 8 files changed, 225 insertions(+), 4 deletions(-) diff --git a/Samples/SwiftJavaExtractJNISampleApp/Sources/MySwiftLibrary/MySwiftClass.swift b/Samples/SwiftJavaExtractJNISampleApp/Sources/MySwiftLibrary/MySwiftClass.swift index 099aa5ca0..9a2c3368b 100644 --- a/Samples/SwiftJavaExtractJNISampleApp/Sources/MySwiftLibrary/MySwiftClass.swift +++ b/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() { } diff --git a/Samples/SwiftJavaExtractJNISampleApp/src/test/java/com/example/swift/MySwiftClassTest.java b/Samples/SwiftJavaExtractJNISampleApp/src/test/java/com/example/swift/MySwiftClassTest.java index a1f8a4529..e69ac9a14 100644 --- a/Samples/SwiftJavaExtractJNISampleApp/src/test/java/com/example/swift/MySwiftClassTest.java +++ b/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)); + } } \ No newline at end of file diff --git a/Sources/JExtractSwiftLib/FFM/CDeclLowering/FFMSwift2JavaGenerator+FunctionLowering.swift b/Sources/JExtractSwiftLib/FFM/CDeclLowering/FFMSwift2JavaGenerator+FunctionLowering.swift index e6a22786a..f1c50b0ea 100644 --- a/Sources/JExtractSwiftLib/FFM/CDeclLowering/FFMSwift2JavaGenerator+FunctionLowering.swift +++ b/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 } diff --git a/Sources/SwiftExtract/Convenience/SwiftSyntax+Extensions.swift b/Sources/SwiftExtract/Convenience/SwiftSyntax+Extensions.swift index e54c1c3eb..5c07c2018 100644 --- a/Sources/SwiftExtract/Convenience/SwiftSyntax+Extensions.swift +++ b/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) { diff --git a/Sources/SwiftExtract/SwiftAnalysisVisitor.swift b/Sources/SwiftExtract/SwiftAnalysisVisitor.swift index 3c9e79d1a..76de54d70 100644 --- a/Sources/SwiftExtract/SwiftAnalysisVisitor.swift +++ b/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), diff --git a/Sources/SwiftExtract/SwiftTypes/SwiftFunctionSignature.swift b/Sources/SwiftExtract/SwiftTypes/SwiftFunctionSignature.swift index 15a147a08..559cf9985 100644 --- a/Sources/SwiftExtract/SwiftTypes/SwiftFunctionSignature.swift +++ b/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] } } diff --git a/Tests/JExtractSwiftTests/JNI/JNIVariablesTests.swift b/Tests/JExtractSwiftTests/JNI/JNIVariablesTests.swift index 11eb08bfb..45a4ed94f 100644 --- a/Tests/JExtractSwiftTests/JNI/JNIVariablesTests.swift +++ b/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!, 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); + """, + ] + ) + } } diff --git a/Tests/JExtractSwiftTests/VariableImportTests.swift b/Tests/JExtractSwiftTests/VariableImportTests.swift index 76c1a42bd..f2ffde174 100644 --- a/Tests/JExtractSwiftTests/VariableImportTests.swift +++ b/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", + ] + ) + } }