Skip to content

Commit e10b33e

Browse files
committed
[BridgeJS] Address PR feedback and refresh generated artifacts
- Make `accessLevel` decode-tolerant on imported skeleton structs (`ImportedFunctionSkeleton`, `ImportedConstructorSkeleton`, `ImportedGetterSkeleton`, `ImportedSetterSkeleton`, `ImportedTypeSkeleton`) by writing explicit `init(from:)` decoders that fall back to `.internal` when the key is missing. Without this, any pre-existing skeleton JSON without the new field fails decoding — the `build-examples` CI job hit `DecodingError.keyNotFound` for `accessLevel` against externally consumed skeletons. - Extract a private `recordSignature` helper so `visitClosure` and `recordInjectedSignature` share a single merge implementation. - Assert in `withAccessLevel(rawLevel:)` so unknown access strings ("open", "private", future schema additions) surface in debug builds instead of silently inheriting the outer level. - Document the `.internal` seeding assumption on `ClosureSignatureCollectorVisitor.init(moduleName:signatures:)`. - Regenerate the BridgeJS pre-generated artifacts under Benchmarks/, Examples/PlayBridgeJS/, Tests/BridgeJSIdentityTests/, and Tests/BridgeJSRuntimeTests/ via `./Utilities/bridge-js-generate.sh`, per CONTRIBUTING.md. The runtime-tests Swift output now emits `public init` on three `JSTypedClosure` extensions whose signatures surface through public exported types.
1 parent 648566b commit e10b33e

6 files changed

Lines changed: 279 additions & 10 deletions

File tree

Benchmarks/Sources/Generated/JavaScript/BridgeJS.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3339,6 +3339,7 @@
33393339
{
33403340
"functions" : [
33413341
{
3342+
"accessLevel" : "internal",
33423343
"effects" : {
33433344
"isAsync" : false,
33443345
"isStatic" : false,
@@ -3355,6 +3356,7 @@
33553356
}
33563357
},
33573358
{
3359+
"accessLevel" : "internal",
33583360
"effects" : {
33593361
"isAsync" : false,
33603362
"isStatic" : false,
@@ -3378,6 +3380,7 @@
33783380
}
33793381
},
33803382
{
3383+
"accessLevel" : "internal",
33813384
"effects" : {
33823385
"isAsync" : false,
33833386
"isStatic" : false,

Examples/PlayBridgeJS/Sources/PlayBridgeJS/Generated/JavaScript/BridgeJS.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,7 @@
242242
{
243243
"functions" : [
244244
{
245+
"accessLevel" : "internal",
245246
"effects" : {
246247
"isAsync" : false,
247248
"isStatic" : false,
@@ -260,11 +261,13 @@
260261
],
261262
"types" : [
262263
{
264+
"accessLevel" : "internal",
263265
"getters" : [
264266

265267
],
266268
"methods" : [
267269
{
270+
"accessLevel" : "internal",
268271
"effects" : {
269272
"isAsync" : false,
270273
"isStatic" : false,

Plugins/BridgeJS/Sources/BridgeJSSkeleton/BridgeJSSkeleton.swift

Lines changed: 104 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -612,12 +612,22 @@ public struct BridgeSkeletonWalker<Visitor: BridgeSkeletonVisitor> {
612612
}
613613

614614
/// String-typed convenience: maps `"public"`/`"package"`/`"internal"` from
615-
/// `Exported*.explicitAccessControl` to the typed enum.
615+
/// `Exported*.explicitAccessControl` to the typed enum. Unknown strings
616+
/// (e.g. `"open"`, `"private"`) hit the assert in debug builds and inherit
617+
/// the outer level in release — the `@JSExport` macros reject those cases
618+
/// upstream, so this is a defensive guard against future schema drift.
616619
private mutating func withAccessLevel(
617620
_ rawLevel: String?,
618621
_ body: (inout BridgeSkeletonWalker) -> Void
619622
) {
620-
withAccessLevel(rawLevel.flatMap(BridgeJSAccessLevel.init(rawValue:)), body)
623+
let level: BridgeJSAccessLevel?
624+
if let rawLevel {
625+
level = BridgeJSAccessLevel(rawValue: rawLevel)
626+
assert(level != nil, "Unexpected access level string: \(rawLevel)")
627+
} else {
628+
level = nil
629+
}
630+
withAccessLevel(level, body)
621631
}
622632
}
623633

@@ -1063,6 +1073,22 @@ public struct ImportedFunctionSkeleton: Codable {
10631073
self.accessLevel = accessLevel
10641074
}
10651075

1076+
private enum CodingKeys: String, CodingKey {
1077+
case name, jsName, from, parameters, returnType, effects, documentation, accessLevel
1078+
}
1079+
1080+
public init(from decoder: any Decoder) throws {
1081+
let container = try decoder.container(keyedBy: CodingKeys.self)
1082+
self.name = try container.decode(String.self, forKey: .name)
1083+
self.jsName = try container.decodeIfPresent(String.self, forKey: .jsName)
1084+
self.from = try container.decodeIfPresent(JSImportFrom.self, forKey: .from)
1085+
self.parameters = try container.decode([Parameter].self, forKey: .parameters)
1086+
self.returnType = try container.decode(BridgeType.self, forKey: .returnType)
1087+
self.effects = try container.decode(Effects.self, forKey: .effects)
1088+
self.documentation = try container.decodeIfPresent(String.self, forKey: .documentation)
1089+
self.accessLevel = try container.decodeIfPresent(BridgeJSAccessLevel.self, forKey: .accessLevel) ?? .internal
1090+
}
1091+
10661092
public func abiName(context: ImportedTypeSkeleton?) -> String {
10671093
return abiName(context: context, operation: nil)
10681094
}
@@ -1087,6 +1113,16 @@ public struct ImportedConstructorSkeleton: Codable {
10871113
self.accessLevel = accessLevel
10881114
}
10891115

1116+
private enum CodingKeys: String, CodingKey {
1117+
case parameters, accessLevel
1118+
}
1119+
1120+
public init(from decoder: any Decoder) throws {
1121+
let container = try decoder.container(keyedBy: CodingKeys.self)
1122+
self.parameters = try container.decode([Parameter].self, forKey: .parameters)
1123+
self.accessLevel = try container.decodeIfPresent(BridgeJSAccessLevel.self, forKey: .accessLevel) ?? .internal
1124+
}
1125+
10901126
public func abiName(context: ImportedTypeSkeleton) -> String {
10911127
return ABINameGenerator.generateImportedABIName(
10921128
baseName: "init",
@@ -1126,6 +1162,21 @@ public struct ImportedGetterSkeleton: Codable {
11261162
self.accessLevel = accessLevel
11271163
}
11281164

1165+
private enum CodingKeys: String, CodingKey {
1166+
case name, jsName, from, type, documentation, functionName, accessLevel
1167+
}
1168+
1169+
public init(from decoder: any Decoder) throws {
1170+
let container = try decoder.container(keyedBy: CodingKeys.self)
1171+
self.name = try container.decode(String.self, forKey: .name)
1172+
self.jsName = try container.decodeIfPresent(String.self, forKey: .jsName)
1173+
self.from = try container.decodeIfPresent(JSImportFrom.self, forKey: .from)
1174+
self.type = try container.decode(BridgeType.self, forKey: .type)
1175+
self.documentation = try container.decodeIfPresent(String.self, forKey: .documentation)
1176+
self.functionName = try container.decodeIfPresent(String.self, forKey: .functionName)
1177+
self.accessLevel = try container.decodeIfPresent(BridgeJSAccessLevel.self, forKey: .accessLevel) ?? .internal
1178+
}
1179+
11291180
public func abiName(context: ImportedTypeSkeleton?) -> String {
11301181
if let functionName = functionName {
11311182
return ABINameGenerator.generateImportedABIName(
@@ -1169,6 +1220,20 @@ public struct ImportedSetterSkeleton: Codable {
11691220
self.accessLevel = accessLevel
11701221
}
11711222

1223+
private enum CodingKeys: String, CodingKey {
1224+
case name, jsName, type, documentation, functionName, accessLevel
1225+
}
1226+
1227+
public init(from decoder: any Decoder) throws {
1228+
let container = try decoder.container(keyedBy: CodingKeys.self)
1229+
self.name = try container.decode(String.self, forKey: .name)
1230+
self.jsName = try container.decodeIfPresent(String.self, forKey: .jsName)
1231+
self.type = try container.decode(BridgeType.self, forKey: .type)
1232+
self.documentation = try container.decodeIfPresent(String.self, forKey: .documentation)
1233+
self.functionName = try container.decodeIfPresent(String.self, forKey: .functionName)
1234+
self.accessLevel = try container.decodeIfPresent(BridgeJSAccessLevel.self, forKey: .accessLevel) ?? .internal
1235+
}
1236+
11721237
public func abiName(context: ImportedTypeSkeleton?) -> String {
11731238
if let functionName = functionName {
11741239
return ABINameGenerator.generateImportedABIName(
@@ -1224,6 +1289,24 @@ public struct ImportedTypeSkeleton: Codable {
12241289
self.documentation = documentation
12251290
self.accessLevel = accessLevel
12261291
}
1292+
1293+
private enum CodingKeys: String, CodingKey {
1294+
case name, jsName, from, constructor, methods, staticMethods, getters, setters, documentation, accessLevel
1295+
}
1296+
1297+
public init(from decoder: any Decoder) throws {
1298+
let container = try decoder.container(keyedBy: CodingKeys.self)
1299+
self.name = try container.decode(String.self, forKey: .name)
1300+
self.jsName = try container.decodeIfPresent(String.self, forKey: .jsName)
1301+
self.from = try container.decodeIfPresent(JSImportFrom.self, forKey: .from)
1302+
self.constructor = try container.decodeIfPresent(ImportedConstructorSkeleton.self, forKey: .constructor)
1303+
self.methods = try container.decode([ImportedFunctionSkeleton].self, forKey: .methods)
1304+
self.staticMethods = try container.decode([ImportedFunctionSkeleton].self, forKey: .staticMethods)
1305+
self.getters = try container.decode([ImportedGetterSkeleton].self, forKey: .getters)
1306+
self.setters = try container.decode([ImportedSetterSkeleton].self, forKey: .setters)
1307+
self.documentation = try container.decodeIfPresent(String.self, forKey: .documentation)
1308+
self.accessLevel = try container.decodeIfPresent(BridgeJSAccessLevel.self, forKey: .accessLevel) ?? .internal
1309+
}
12271310
}
12281311

12291312
public struct ImportedFileSkeleton: Codable {
@@ -1298,6 +1381,12 @@ public struct ClosureSignatureCollectorVisitor: BridgeSkeletonVisitor {
12981381
public var signatures: Set<ClosureSignature> { Set(signatureAccessLevels.keys) }
12991382
let moduleName: String
13001383

1384+
/// Convenience for callers that only need to seed signatures without
1385+
/// access metadata (e.g. exported-side walking, where closure init access
1386+
/// is irrelevant because the synthesized init isn't surfaced to consumers).
1387+
/// All seeded signatures default to `.internal`; if a seeded signature is
1388+
/// later observed with a more permissive access level, the merge in
1389+
/// `recordSignature` upgrades it.
13011390
public init(moduleName: String, signatures: Set<ClosureSignature> = []) {
13021391
self.moduleName = moduleName
13031392
for signature in signatures {
@@ -1309,6 +1398,18 @@ public struct ClosureSignatureCollectorVisitor: BridgeSkeletonVisitor {
13091398
_ signature: ClosureSignature,
13101399
useJSTypedClosure: Bool,
13111400
accessLevel: BridgeJSAccessLevel
1401+
) {
1402+
recordSignature(signature, accessLevel: accessLevel)
1403+
}
1404+
1405+
/// Insert `signature` at `accessLevel`, or upgrade the existing level to
1406+
/// the more permissive of the two. Centralizing the merge here keeps
1407+
/// `visitClosure` and `recordInjectedSignature` in lockstep — if the
1408+
/// merge policy ever needs to change (e.g. adding a diagnostic for
1409+
/// conflicting levels), there's only one place to update.
1410+
private mutating func recordSignature(
1411+
_ signature: ClosureSignature,
1412+
accessLevel: BridgeJSAccessLevel
13121413
) {
13131414
if let existing = signatureAccessLevels[signature] {
13141415
signatureAccessLevels[signature] = max(existing, accessLevel)
@@ -1367,11 +1468,7 @@ public struct ClosureSignatureCollectorVisitor: BridgeSkeletonVisitor {
13671468
_ signature: ClosureSignature,
13681469
for function: ImportedFunctionSkeleton
13691470
) {
1370-
if let existing = signatureAccessLevels[signature] {
1371-
signatureAccessLevels[signature] = max(existing, function.accessLevel)
1372-
} else {
1373-
signatureAccessLevels[signature] = function.accessLevel
1374-
}
1471+
recordSignature(signature, accessLevel: function.accessLevel)
13751472
}
13761473
}
13771474

Tests/BridgeJSIdentityTests/Generated/JavaScript/BridgeJS.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -410,6 +410,7 @@
410410
],
411411
"types" : [
412412
{
413+
"accessLevel" : "internal",
413414
"getters" : [
414415

415416
],
@@ -422,6 +423,7 @@
422423
],
423424
"staticMethods" : [
424425
{
426+
"accessLevel" : "internal",
425427
"effects" : {
426428
"isAsync" : false,
427429
"isStatic" : false,

Tests/BridgeJSRuntimeTests/Generated/BridgeJS.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -369,7 +369,7 @@ private enum _BJS_Closure_20BridgeJSRuntimeTests7GreeterC_SS {
369369
}
370370

371371
extension JSTypedClosure where Signature == (Greeter) -> String {
372-
init(fileID: StaticString = #fileID, line: UInt32 = #line, _ body: @escaping (Greeter) -> String) {
372+
public init(fileID: StaticString = #fileID, line: UInt32 = #line, _ body: @escaping (Greeter) -> String) {
373373
self.init(
374374
makeClosure: make_swift_closure_BridgeJSRuntimeTests_20BridgeJSRuntimeTests7GreeterC_SS,
375375
body: body,
@@ -687,7 +687,7 @@ private enum _BJS_Closure_20BridgeJSRuntimeTestsSS_7GreeterC {
687687
}
688688

689689
extension JSTypedClosure where Signature == (String) -> Greeter {
690-
init(fileID: StaticString = #fileID, line: UInt32 = #line, _ body: @escaping (String) -> Greeter) {
690+
public init(fileID: StaticString = #fileID, line: UInt32 = #line, _ body: @escaping (String) -> Greeter) {
691691
self.init(
692692
makeClosure: make_swift_closure_BridgeJSRuntimeTests_20BridgeJSRuntimeTestsSS_7GreeterC,
693693
body: body,
@@ -753,7 +753,7 @@ private enum _BJS_Closure_20BridgeJSRuntimeTestsSS_SS {
753753
}
754754

755755
extension JSTypedClosure where Signature == (String) -> String {
756-
init(fileID: StaticString = #fileID, line: UInt32 = #line, _ body: @escaping (String) -> String) {
756+
public init(fileID: StaticString = #fileID, line: UInt32 = #line, _ body: @escaping (String) -> String) {
757757
self.init(
758758
makeClosure: make_swift_closure_BridgeJSRuntimeTests_20BridgeJSRuntimeTestsSS_SS,
759759
body: body,

0 commit comments

Comments
 (0)