Skip to content

Commit 55fdaf8

Browse files
committed
BridgeJS: Enforce throws(JSException) for @js protocol properties
1 parent a070ba0 commit 55fdaf8

File tree

13 files changed

+1863
-432
lines changed

13 files changed

+1863
-432
lines changed

Plugins/BridgeJS/Sources/BridgeJSCore/ClosureCodegen.swift

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,11 @@ public struct ClosureCodegen {
6161

6262
// Generate the call and return value lifting
6363
try builder.call(returnType: signature.returnType)
64+
65+
if signature.isThrows {
66+
builder.body.append("if let error = _swift_js_take_exception() { throw error }")
67+
}
68+
6469
try builder.liftReturnValue(returnType: signature.returnType)
6570

6671
// Get the body code

Plugins/BridgeJS/Sources/BridgeJSCore/ExportSwift.swift

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1447,7 +1447,7 @@ struct ProtocolCodegen {
14471447
let getterBuilder = ImportTS.CallJSEmission(
14481448
moduleName: moduleName,
14491449
abiName: getterAbiName,
1450-
context: .exportSwift
1450+
context: property.effects.isThrows ? .exportSwiftProtocol : .exportSwift
14511451
)
14521452
try getterBuilder.lowerParameter(param: Parameter(label: nil, name: "jsObject", type: .jsObject(nil)))
14531453
try getterBuilder.call(returnType: property.type)
@@ -1466,6 +1466,12 @@ struct ProtocolCodegen {
14661466
)
14671467

14681468
if property.isReadonly {
1469+
// Build effect specifiers for getter if property throws
1470+
let getterEffects =
1471+
property.effects.isThrows || property.effects.isAsync
1472+
? ImportTS.buildAccessorEffect(throws: property.effects.isThrows, async: property.effects.isAsync)
1473+
: nil
1474+
14691475
let propertyDecl = VariableDeclSyntax(
14701476
bindingSpecifier: .keyword(.var),
14711477
bindings: PatternBindingListSyntax {
@@ -1479,6 +1485,7 @@ struct ProtocolCodegen {
14791485
AccessorDeclListSyntax {
14801486
AccessorDeclSyntax(
14811487
accessorSpecifier: .keyword(.get),
1488+
effectSpecifiers: getterEffects,
14821489
body: getterBuilder.getBody()
14831490
)
14841491
}

Plugins/BridgeJS/Sources/BridgeJSCore/SwiftToSkeleton.swift

Lines changed: 71 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1057,6 +1057,29 @@ private final class ExportSwiftAPICollector: SyntaxAnyVisitor {
10571057
return Effects(isAsync: isAsync, isThrows: isThrows, isStatic: isStatic)
10581058
}
10591059

1060+
private func collectEffectsFromAccessor(_ accessor: AccessorDeclSyntax) -> Effects? {
1061+
let isAsync = accessor.effectSpecifiers?.asyncSpecifier != nil
1062+
var isThrows = false
1063+
if let throwsClause = accessor.effectSpecifiers?.throwsClause {
1064+
guard let thrownType = throwsClause.type else {
1065+
diagnose(
1066+
node: throwsClause,
1067+
message: "Thrown type is not specified, only JSException is supported for now"
1068+
)
1069+
return nil
1070+
}
1071+
guard thrownType.trimmedDescription == "JSException" else {
1072+
diagnose(
1073+
node: throwsClause,
1074+
message: "Only JSException is supported for thrown type, got \(thrownType.trimmedDescription)"
1075+
)
1076+
return nil
1077+
}
1078+
isThrows = true
1079+
}
1080+
return Effects(isAsync: isAsync, isThrows: isThrows, isStatic: false)
1081+
}
1082+
10601083
private func extractNamespace(
10611084
from jsAttribute: AttributeSyntax
10621085
) -> [String]? {
@@ -1654,18 +1677,62 @@ private final class ExportSwiftAPICollector: SyntaxAnyVisitor {
16541677
guard let accessorBlock = binding.accessorBlock else {
16551678
diagnose(
16561679
node: binding,
1657-
message: "Protocol property must specify { get } or { get set }",
1658-
hint: "Add { get } for readonly or { get set } for readwrite property"
1680+
message: "Protocol property must specify { get throws(JSException) }",
1681+
hint: "Add { get throws(JSException) } for the property accessor"
16591682
)
16601683
continue
16611684
}
16621685

1663-
let isReadonly = hasOnlyGetter(accessorBlock)
1686+
// Find the getter accessor
1687+
let getterAccessor: AccessorDeclSyntax?
1688+
switch accessorBlock.accessors {
1689+
case .accessors(let accessors):
1690+
getterAccessor = accessors.first { $0.accessorSpecifier.tokenKind == .keyword(.get) }
1691+
case .getter:
1692+
diagnose(
1693+
node: accessorBlock,
1694+
message: "@JS protocol property getter must declare throws(JSException)",
1695+
hint: "Use { get throws(JSException) } syntax"
1696+
)
1697+
continue
1698+
}
1699+
1700+
guard let getter = getterAccessor else {
1701+
diagnose(node: accessorBlock, message: "Protocol property must have a getter")
1702+
continue
1703+
}
1704+
1705+
// Check for setter - not allowed with throwing getter
1706+
if case .accessors(let accessors) = accessorBlock.accessors {
1707+
if accessors.contains(where: { $0.accessorSpecifier.tokenKind == .keyword(.set) }) {
1708+
diagnose(
1709+
node: accessorBlock,
1710+
message: "@JS protocol cannot have { get set } properties",
1711+
hint:
1712+
"Use readonly property with setter method: var \(propertyName): \(typeAnnotation.type.trimmedDescription) { get throws(JSException) } and func set\(propertyName.capitalized)(_ value: \(typeAnnotation.type.trimmedDescription)) throws(JSException)"
1713+
)
1714+
continue
1715+
}
1716+
}
1717+
1718+
guard let effects = collectEffectsFromAccessor(getter) else {
1719+
continue
1720+
}
1721+
1722+
guard effects.isThrows else {
1723+
diagnose(
1724+
node: getter,
1725+
message: "@JS protocol property getter must be throws",
1726+
hint: "Declare the getter as 'get throws(JSException)'"
1727+
)
1728+
continue
1729+
}
16641730

16651731
let exportedProperty = ExportedProtocolProperty(
16661732
name: propertyName,
16671733
type: propertyType,
1668-
isReadonly: isReadonly
1734+
isReadonly: true, // Always readonly since { get set } with throws is not allowed
1735+
effects: effects
16691736
)
16701737

16711738
if var currentProtocol = exportedProtocolByName[protocolKey] {

Plugins/BridgeJS/Sources/BridgeJSLink/BridgeJSLink.swift

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3281,14 +3281,40 @@ extension BridgeJSLink {
32813281
for param in method.parameters {
32823282
try thunkBuilder.liftParameter(param: param)
32833283
}
3284-
let returnExpr = try thunkBuilder.callMethod(name: method.name, returnType: method.returnType)
3284+
3285+
// Check if this is a setter method (setFoo pattern with single parameter and void return)
3286+
// If so, convert to property assignment for JavaScript compatibility
3287+
let returnExpr: String?
3288+
if let propertyName = extractSetterPropertyName(from: method.name),
3289+
method.parameters.count == 1,
3290+
method.returnType == .void
3291+
{
3292+
thunkBuilder.callPropertySetter(name: propertyName, returnType: method.returnType)
3293+
returnExpr = nil
3294+
} else {
3295+
returnExpr = try thunkBuilder.callMethod(name: method.name, returnType: method.returnType)
3296+
}
3297+
32853298
let funcLines = thunkBuilder.renderFunction(
32863299
name: method.abiName,
32873300
returnExpr: returnExpr,
32883301
returnType: method.returnType
32893302
)
32903303
importObjectBuilder.assignToImportObject(name: method.abiName, function: funcLines)
32913304
}
3305+
3306+
/// Extracts property name from a setter method name (e.g., "setFoo" -> "foo")
3307+
/// Returns nil if the method name doesn't match the setter pattern
3308+
private func extractSetterPropertyName(from methodName: String) -> String? {
3309+
guard methodName.hasPrefix("set"), methodName.count > 3 else {
3310+
return nil
3311+
}
3312+
let propertyName = String(methodName.dropFirst(3))
3313+
guard !propertyName.isEmpty else {
3314+
return nil
3315+
}
3316+
return propertyName.prefix(1).lowercased() + propertyName.dropFirst()
3317+
}
32923318
}
32933319

32943320
/// Utility enum for generating default value representations in JavaScript/TypeScript

Plugins/BridgeJS/Sources/BridgeJSSkeleton/BridgeJSSkeleton.swift

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -406,15 +406,18 @@ public struct ExportedProtocolProperty: Codable, Equatable, Sendable {
406406
public let name: String
407407
public let type: BridgeType
408408
public let isReadonly: Bool
409+
public let effects: Effects
409410

410411
public init(
411412
name: String,
412413
type: BridgeType,
413-
isReadonly: Bool
414+
isReadonly: Bool,
415+
effects: Effects
414416
) {
415417
self.name = name
416418
self.type = type
417419
self.isReadonly = isReadonly
420+
self.effects = effects
418421
}
419422
}
420423

Plugins/BridgeJS/Tests/BridgeJSToolTests/Inputs/MacroSwift/Protocol.swift

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -36,17 +36,27 @@ import JavaScriptKit
3636
}
3737

3838
@JS protocol MyViewControllerDelegate {
39-
var eventCount: Int { get set }
40-
var delegateName: String { get }
41-
var optionalName: String? { get set }
42-
var optionalRawEnum: ExampleEnum? { get set }
43-
var rawStringEnum: ExampleEnum { get set }
44-
var result: Result { get set }
45-
var optionalResult: Result? { get set }
46-
var direction: Direction { get set }
47-
var directionOptional: Direction? { get set }
48-
var priority: Priority { get set }
49-
var priorityOptional: Priority? { get set }
39+
var eventCount: Int { get throws(JSException) }
40+
var delegateName: String { get throws(JSException) }
41+
var optionalName: String? { get throws(JSException) }
42+
var optionalRawEnum: ExampleEnum? { get throws(JSException) }
43+
var rawStringEnum: ExampleEnum { get throws(JSException) }
44+
var result: Result { get throws(JSException) }
45+
var optionalResult: Result? { get throws(JSException) }
46+
var direction: Direction { get throws(JSException) }
47+
var directionOptional: Direction? { get throws(JSException) }
48+
var priority: Priority { get throws(JSException) }
49+
var priorityOptional: Priority? { get throws(JSException) }
50+
func setEventCount(_ value: Int) throws(JSException)
51+
func setOptionalName(_ value: String?) throws(JSException)
52+
func setOptionalRawEnum(_ value: ExampleEnum?) throws(JSException)
53+
func setRawStringEnum(_ value: ExampleEnum) throws(JSException)
54+
func setResult(_ value: Result) throws(JSException)
55+
func setOptionalResult(_ value: Result?) throws(JSException)
56+
func setDirection(_ value: Direction) throws(JSException)
57+
func setDirectionOptional(_ value: Direction?) throws(JSException)
58+
func setPriority(_ value: Priority) throws(JSException)
59+
func setPriorityOptional(_ value: Priority?) throws(JSException)
5060
func onSomethingHappened() throws(JSException)
5161
func onValueChanged(_ value: String) throws(JSException)
5262
func onCountUpdated(count: Int) throws(JSException) -> Bool

0 commit comments

Comments
 (0)