diff --git a/Plugins/BridgeJS/Sources/BridgeJSCore/SwiftToSkeleton.swift b/Plugins/BridgeJS/Sources/BridgeJSCore/SwiftToSkeleton.swift index f39ac16f8..aa696ec31 100644 --- a/Plugins/BridgeJS/Sources/BridgeJSCore/SwiftToSkeleton.swift +++ b/Plugins/BridgeJS/Sources/BridgeJSCore/SwiftToSkeleton.swift @@ -1198,6 +1198,14 @@ private final class ExportSwiftAPICollector: SyntaxAnyVisitor { className: classNameForABI ) + if let mutatingModifier = node.modifiers.first(where: { $0.name.tokenKind == .keyword(.mutating) }) { + diagnose( + node: mutatingModifier, + message: "@JS does not support mutating struct methods: mutations to 'self' cannot be propagated back to JavaScript", + hint: "Remove the mutating keyword or redesign the API to return the updated value instead" + ) + return nil + } guard let effects = collectEffects(signature: node.signature, isStatic: isStatic) else { return nil } @@ -1522,7 +1530,7 @@ private final class ExportSwiftAPICollector: SyntaxAnyVisitor { } } - /// Walks extension members under the matching type’s state, returning whether the type was found. + /// Walks extension members under the matching type's state, returning whether the type was found. /// /// Note: The lookup scans dictionaries keyed by `makeKey(name:namespace:)`, matching only by /// plain name. If two types share a name but differ by namespace, `.first(where:)` picks diff --git a/Plugins/BridgeJS/Sources/BridgeJSSkeleton/BridgeJSSkeleton.swift b/Plugins/BridgeJS/Sources/BridgeJSSkeleton/BridgeJSSkeleton.swift index 346b7333b..d132888b3 100644 --- a/Plugins/BridgeJS/Sources/BridgeJSSkeleton/BridgeJSSkeleton.swift +++ b/Plugins/BridgeJS/Sources/BridgeJSSkeleton/BridgeJSSkeleton.swift @@ -635,11 +635,35 @@ public struct Effects: Codable, Equatable, Sendable { public var isAsync: Bool public var isThrows: Bool public var isStatic: Bool + public var isMutating: Bool - public init(isAsync: Bool, isThrows: Bool, isStatic: Bool = false) { + public init(isAsync: Bool, isThrows: Bool, isStatic: Bool = false, isMutating: Bool = false) { self.isAsync = isAsync self.isThrows = isThrows self.isStatic = isStatic + self.isMutating = isMutating + } + + private enum CodingKeys: String, CodingKey { + case isAsync, isThrows, isStatic, isMutating + } + + public init(from decoder: any Decoder) throws { + let container = try decoder.container(keyedBy: CodingKeys.self) + self.isAsync = try container.decode(Bool.self, forKey: .isAsync) + self.isThrows = try container.decode(Bool.self, forKey: .isThrows) + self.isStatic = try container.decode(Bool.self, forKey: .isStatic) + self.isMutating = try container.decodeIfPresent(Bool.self, forKey: .isMutating) ?? false + } + + public func encode(to encoder: any Encoder) throws { + var container = encoder.container(keyedBy: CodingKeys.self) + try container.encode(isAsync, forKey: .isAsync) + try container.encode(isThrows, forKey: .isThrows) + try container.encode(isStatic, forKey: .isStatic) + if isMutating { + try container.encode(isMutating, forKey: .isMutating) + } } } diff --git a/Plugins/BridgeJS/Tests/BridgeJSToolTests/DiagnosticsTests.swift b/Plugins/BridgeJS/Tests/BridgeJSToolTests/DiagnosticsTests.swift index e71a1f84e..8acc9ea13 100644 --- a/Plugins/BridgeJS/Tests/BridgeJSToolTests/DiagnosticsTests.swift +++ b/Plugins/BridgeJS/Tests/BridgeJSToolTests/DiagnosticsTests.swift @@ -319,4 +319,97 @@ import Testing // No line 3 in source, so output must not show a " 3 |" context line after the pointer #expect(!description.contains(" 3 |")) } + + // MARK: - Mutating struct method diagnostic + + /// `@JS` on a `mutating` struct method cannot be supported: the JS-side + /// bridge calls a Swift function whose `self` was lowered across the WASM + /// boundary, so mutations to `self` cannot be propagated back to the + /// JavaScript caller. The codegen detects the `mutating` modifier and + /// emits an explicit diagnostic pointing the user at the right fix, + /// rather than silently producing a thunk that would discard their + /// mutations at runtime. + @Test + func mutatingStructMethodEmitsDiagnostic() throws { + let source = """ + @JS struct Counter { + var number: Int + + @JS public mutating func increment() { + number += 1 + } + } + """ + let swiftAPI = SwiftToSkeleton( + progress: .silent, + moduleName: "TestModule", + exposeToGlobal: false, + externalModuleIndex: .empty + ) + swiftAPI.addSourceFile(Parser.parse(source: source), inputFilePath: "test.swift") + #expect(throws: BridgeJSCoreDiagnosticError.self) { + _ = try swiftAPI.finalize() + } + } + + @Test + func mutatingStructMethodDiagnosticMessageAndHint() throws { + let source = """ + @JS struct Counter { + var number: Int + + @JS public mutating func increment() { + number += 1 + } + } + """ + let swiftAPI = SwiftToSkeleton( + progress: .silent, + moduleName: "TestModule", + exposeToGlobal: false, + externalModuleIndex: .empty + ) + swiftAPI.addSourceFile(Parser.parse(source: source), inputFilePath: "test.swift") + do { + _ = try swiftAPI.finalize() + Issue.record("Expected finalize() to throw for a mutating @JS struct method") + } catch let error as BridgeJSCoreDiagnosticError { + let allMessages = error.diagnostics.map { $0.diagnostic.message }.joined(separator: "\n") + #expect( + allMessages.contains( + "@JS does not support mutating struct methods: mutations to 'self' cannot be propagated back to JavaScript" + ) + ) + let allHints = error.diagnostics.compactMap { $0.diagnostic.hint }.joined(separator: "\n") + #expect( + allHints.contains( + "Remove the mutating keyword or redesign the API to return the updated value instead" + ) + ) + } + } + + @Test + func nonMutatingStructMethodSucceeds() throws { + // Regression guard: an otherwise-identical struct method WITHOUT the + // `mutating` modifier should still pass through the codegen cleanly. + let source = """ + @JS struct Counter { + var number: Int + + @JS public func describe() -> String { + return "Counter(\\(number))" + } + } + """ + let swiftAPI = SwiftToSkeleton( + progress: .silent, + moduleName: "TestModule", + exposeToGlobal: false, + externalModuleIndex: .empty + ) + swiftAPI.addSourceFile(Parser.parse(source: source), inputFilePath: "test.swift") + let skeleton = try swiftAPI.finalize() + #expect(skeleton.exported != nil) + } } diff --git a/Plugins/BridgeJS/Tests/BridgeJSToolTests/Inputs/MacroSwift/MutatingStructMethod.swift b/Plugins/BridgeJS/Tests/BridgeJSToolTests/Inputs/MacroSwift/MutatingStructMethod.swift new file mode 100644 index 000000000..23d2e6538 --- /dev/null +++ b/Plugins/BridgeJS/Tests/BridgeJSToolTests/Inputs/MacroSwift/MutatingStructMethod.swift @@ -0,0 +1,12 @@ +@JS struct Counter { + var number: Int +} + +extension Counter { + @JS public mutating func increment() { + number += 1 + } + @JS public mutating func add(_ value: Int) { + number += value + } +} diff --git a/Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSCodegenTests/MutatingStructMethod.json b/Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSCodegenTests/MutatingStructMethod.json new file mode 100644 index 000000000..3e360fd00 --- /dev/null +++ b/Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSCodegenTests/MutatingStructMethod.json @@ -0,0 +1,45 @@ +{ + "exported" : { + "classes" : [ + + ], + "enums" : [ + + ], + "exposeToGlobal" : false, + "functions" : [ + + ], + "protocols" : [ + + ], + "structs" : [ + { + "methods" : [ + + ], + "name" : "Counter", + "properties" : [ + { + "isReadonly" : true, + "isStatic" : false, + "name" : "number", + "type" : { + "integer" : { + "_0" : { + "isSigned" : true, + "width" : "word" + } + } + } + } + ], + "swiftCallName" : "Counter" + } + ] + }, + "moduleName" : "TestModule", + "usedExternalModules" : [ + + ] +} \ No newline at end of file diff --git a/Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSCodegenTests/MutatingStructMethod.swift b/Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSCodegenTests/MutatingStructMethod.swift new file mode 100644 index 000000000..c77ec562f --- /dev/null +++ b/Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSCodegenTests/MutatingStructMethod.swift @@ -0,0 +1,45 @@ +extension Counter: _BridgedSwiftStruct { + @_spi(BridgeJS) @_transparent public static func bridgeJSStackPop() -> Counter { + let number = Int.bridgeJSStackPop() + return Counter(number: number) + } + + @_spi(BridgeJS) @_transparent public consuming func bridgeJSStackPush() { + self.number.bridgeJSStackPush() + } + + init(unsafelyCopying jsObject: JSObject) { + _bjs_struct_lower_Counter(jsObject.bridgeJSLowerParameter()) + self = Self.bridgeJSStackPop() + } + + func toJSObject() -> JSObject { + let __bjs_self = self + __bjs_self.bridgeJSStackPush() + return JSObject(id: UInt32(bitPattern: _bjs_struct_lift_Counter())) + } +} + +#if arch(wasm32) +@_extern(wasm, module: "bjs", name: "swift_js_struct_lower_Counter") +fileprivate func _bjs_struct_lower_Counter_extern(_ objectId: Int32) -> Void +#else +fileprivate func _bjs_struct_lower_Counter_extern(_ objectId: Int32) -> Void { + fatalError("Only available on WebAssembly") +} +#endif +@inline(never) fileprivate func _bjs_struct_lower_Counter(_ objectId: Int32) -> Void { + return _bjs_struct_lower_Counter_extern(objectId) +} + +#if arch(wasm32) +@_extern(wasm, module: "bjs", name: "swift_js_struct_lift_Counter") +fileprivate func _bjs_struct_lift_Counter_extern() -> Int32 +#else +fileprivate func _bjs_struct_lift_Counter_extern() -> Int32 { + fatalError("Only available on WebAssembly") +} +#endif +@inline(never) fileprivate func _bjs_struct_lift_Counter() -> Int32 { + return _bjs_struct_lift_Counter_extern() +} \ No newline at end of file diff --git a/Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSLinkTests/MutatingStructMethod.d.ts b/Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSLinkTests/MutatingStructMethod.d.ts new file mode 100644 index 000000000..25d90921a --- /dev/null +++ b/Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSLinkTests/MutatingStructMethod.d.ts @@ -0,0 +1,20 @@ +// NOTICE: This is auto-generated code by BridgeJS from JavaScriptKit, +// DO NOT EDIT. +// +// To update this file, just rebuild your project or run +// `swift package bridge-js`. + +export interface Counter { + number: number; +} +export type Exports = { +} +export type Imports = { +} +export function createInstantiator(options: { + imports: Imports; +}, swift: any): Promise<{ + addImports: (importObject: WebAssembly.Imports) => void; + setInstance: (instance: WebAssembly.Instance) => void; + createExports: (instance: WebAssembly.Instance) => Exports; +}>; \ No newline at end of file diff --git a/Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSLinkTests/MutatingStructMethod.js b/Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSLinkTests/MutatingStructMethod.js new file mode 100644 index 000000000..0b11f90fa --- /dev/null +++ b/Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSLinkTests/MutatingStructMethod.js @@ -0,0 +1,230 @@ +// NOTICE: This is auto-generated code by BridgeJS from JavaScriptKit, +// DO NOT EDIT. +// +// To update this file, just rebuild your project or run +// `swift package bridge-js`. + +export async function createInstantiator(options, swift) { + let instance; + let memory; + let setException; + let decodeString; + const textDecoder = new TextDecoder("utf-8"); + const textEncoder = new TextEncoder("utf-8"); + let tmpRetString; + let tmpRetBytes; + let tmpRetException; + let tmpRetOptionalBool; + let tmpRetOptionalInt; + let tmpRetOptionalFloat; + let tmpRetOptionalDouble; + let tmpRetOptionalHeapObject; + let strStack = []; + let i32Stack = []; + let i64Stack = []; + let f32Stack = []; + let f64Stack = []; + let ptrStack = []; + const enumHelpers = {}; + const structHelpers = {}; + + let _exports = null; + let bjs = null; + const __bjs_createCounterHelpers = () => ({ + lower: (value) => { + i32Stack.push((value.number | 0)); + }, + lift: () => { + const int = i32Stack.pop(); + return { number: int }; + } + }); + + return { + /** + * @param {WebAssembly.Imports} importObject + */ + addImports: (importObject, importsContext) => { + bjs = {}; + importObject["bjs"] = bjs; + bjs["swift_js_return_string"] = function(ptr, len) { + tmpRetString = decodeString(ptr, len); + } + bjs["swift_js_init_memory"] = function(sourceId, bytesPtr) { + const source = swift.memory.getObject(sourceId); + swift.memory.release(sourceId); + const bytes = new Uint8Array(memory.buffer, bytesPtr); + bytes.set(source); + } + bjs["swift_js_make_js_string"] = function(ptr, len) { + return swift.memory.retain(decodeString(ptr, len)); + } + bjs["swift_js_init_memory_with_result"] = function(ptr, len) { + const target = new Uint8Array(memory.buffer, ptr, len); + target.set(tmpRetBytes); + tmpRetBytes = undefined; + } + bjs["swift_js_throw"] = function(id) { + tmpRetException = swift.memory.retainByRef(id); + } + bjs["swift_js_retain"] = function(id) { + return swift.memory.retainByRef(id); + } + bjs["swift_js_release"] = function(id) { + swift.memory.release(id); + } + bjs["swift_js_push_i32"] = function(v) { + i32Stack.push(v | 0); + } + bjs["swift_js_push_f32"] = function(v) { + f32Stack.push(Math.fround(v)); + } + bjs["swift_js_push_f64"] = function(v) { + f64Stack.push(v); + } + bjs["swift_js_push_string"] = function(ptr, len) { + const value = decodeString(ptr, len); + strStack.push(value); + } + bjs["swift_js_pop_i32"] = function() { + return i32Stack.pop(); + } + bjs["swift_js_pop_f32"] = function() { + return f32Stack.pop(); + } + bjs["swift_js_pop_f64"] = function() { + return f64Stack.pop(); + } + bjs["swift_js_push_pointer"] = function(pointer) { + ptrStack.push(pointer); + } + bjs["swift_js_pop_pointer"] = function() { + return ptrStack.pop(); + } + bjs["swift_js_push_i64"] = function(v) { + i64Stack.push(v); + } + bjs["swift_js_pop_i64"] = function() { + return i64Stack.pop(); + } + bjs["swift_js_struct_lower_Counter"] = function(objectId) { + structHelpers.Counter.lower(swift.memory.getObject(objectId)); + } + bjs["swift_js_struct_lift_Counter"] = function() { + const value = structHelpers.Counter.lift(); + return swift.memory.retain(value); + } + bjs["swift_js_return_optional_bool"] = function(isSome, value) { + if (isSome === 0) { + tmpRetOptionalBool = null; + } else { + tmpRetOptionalBool = value !== 0; + } + } + bjs["swift_js_return_optional_int"] = function(isSome, value) { + if (isSome === 0) { + tmpRetOptionalInt = null; + } else { + tmpRetOptionalInt = value | 0; + } + } + bjs["swift_js_return_optional_float"] = function(isSome, value) { + if (isSome === 0) { + tmpRetOptionalFloat = null; + } else { + tmpRetOptionalFloat = Math.fround(value); + } + } + bjs["swift_js_return_optional_double"] = function(isSome, value) { + if (isSome === 0) { + tmpRetOptionalDouble = null; + } else { + tmpRetOptionalDouble = value; + } + } + bjs["swift_js_return_optional_string"] = function(isSome, ptr, len) { + if (isSome === 0) { + tmpRetString = null; + } else { + tmpRetString = decodeString(ptr, len); + } + } + bjs["swift_js_return_optional_object"] = function(isSome, objectId) { + if (isSome === 0) { + tmpRetString = null; + } else { + tmpRetString = swift.memory.getObject(objectId); + } + } + bjs["swift_js_return_optional_heap_object"] = function(isSome, pointer) { + if (isSome === 0) { + tmpRetOptionalHeapObject = null; + } else { + tmpRetOptionalHeapObject = pointer; + } + } + bjs["swift_js_get_optional_int_presence"] = function() { + return tmpRetOptionalInt != null ? 1 : 0; + } + bjs["swift_js_get_optional_int_value"] = function() { + const value = tmpRetOptionalInt; + tmpRetOptionalInt = undefined; + return value; + } + bjs["swift_js_get_optional_string"] = function() { + const str = tmpRetString; + tmpRetString = undefined; + if (str == null) { + return -1; + } else { + const bytes = textEncoder.encode(str); + tmpRetBytes = bytes; + return bytes.length; + } + } + bjs["swift_js_get_optional_float_presence"] = function() { + return tmpRetOptionalFloat != null ? 1 : 0; + } + bjs["swift_js_get_optional_float_value"] = function() { + const value = tmpRetOptionalFloat; + tmpRetOptionalFloat = undefined; + return value; + } + bjs["swift_js_get_optional_double_presence"] = function() { + return tmpRetOptionalDouble != null ? 1 : 0; + } + bjs["swift_js_get_optional_double_value"] = function() { + const value = tmpRetOptionalDouble; + tmpRetOptionalDouble = undefined; + return value; + } + bjs["swift_js_get_optional_heap_object_pointer"] = function() { + const pointer = tmpRetOptionalHeapObject; + tmpRetOptionalHeapObject = undefined; + return pointer || 0; + } + bjs["swift_js_closure_unregister"] = function(funcRef) {} + }, + setInstance: (i) => { + instance = i; + memory = instance.exports.memory; + + decodeString = (ptr, len) => { const bytes = new Uint8Array(memory.buffer, ptr >>> 0, len >>> 0); return textDecoder.decode(bytes); } + + setException = (error) => { + instance.exports._swift_js_exception.value = swift.memory.retain(error) + } + }, + /** @param {WebAssembly.Instance} instance */ + createExports: (instance) => { + const js = swift.memory.heap; + const CounterHelpers = __bjs_createCounterHelpers(); + structHelpers.Counter = CounterHelpers; + + const exports = { + }; + _exports = exports; + return exports; + }, + } +} \ No newline at end of file