Skip to content

Commit c488158

Browse files
kateinoigakukunkateinoigakukun
committed
BridgeJS: tag both snippet and module origins in the skeleton
Encode `.snippet` as `{"kind":"snippet","path":...}` alongside the existing tagged module form, so the JSON mirrors the Swift cases and a snippet path can no longer encode into a shape that fails to decode.
1 parent 452fd36 commit c488158

5 files changed

Lines changed: 118 additions & 57 deletions

File tree

‎Plugins/BridgeJS/Sources/BridgeJSSkeleton/BridgeJSSkeleton.swift‎

Lines changed: 56 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1131,75 +1131,90 @@ private struct AsyncClosureReturnTypeCollector: BridgeSkeletonVisitor {
11311131
/// - `module`: Read from an external ECMAScript module named by a bare specifier
11321132
/// that the JavaScript host resolves (e.g. `node:path`, `lodash/fp`).
11331133
///
1134-
/// The JSON encoding keeps `.global` as `"global"` and a snippet as its plain path
1135-
/// string; only `.module` uses a tagged object. That keeps checked-in skeletons
1136-
/// stable and avoids colliding with the `"global"` sentinel, since `global` is
1137-
/// itself a valid package name.
1134+
/// `.global` encodes as the string `"global"`; the other two encode as tagged
1135+
/// objects that name their kind, so the JSON mirrors the Swift cases and is
1136+
/// readable without knowing that a `/`-prefixed string means one thing and any
1137+
/// other string means another. A plain string is never used for a specifier,
1138+
/// which also avoids colliding with the `"global"` sentinel — `global` is itself
1139+
/// a valid npm package name.
11381140
public enum JSImportFrom: Codable, Equatable, Sendable {
11391141
case global
11401142
case snippet(String)
11411143
case module(String)
11421144

11431145
private enum CodingKeys: String, CodingKey {
1144-
case kind, specifier
1146+
case kind, path, specifier
11451147
}
11461148

11471149
public init(from decoder: any Decoder) throws {
11481150
if let container = try? decoder.singleValueContainer(), let value = try? container.decode(String.self) {
1149-
if value == "global" {
1150-
self = .global
1151-
return
1152-
}
1153-
guard value.hasPrefix("/"), !value.split(separator: "/").contains("..") else {
1151+
guard value == "global" else {
11541152
throw DecodingError.dataCorruptedError(
11551153
in: container,
1156-
debugDescription: "Unknown import origin '\(value)'. Expected \"global\" or a rooted snippet path."
1154+
debugDescription: "Unknown import origin '\(value)'. Expected \"global\"."
11571155
)
11581156
}
1159-
self = .snippet(value)
1157+
self = .global
11601158
return
11611159
}
11621160
let container = try decoder.container(keyedBy: CodingKeys.self)
11631161
let kind = try container.decode(String.self, forKey: .kind)
1164-
guard kind == "module" else {
1162+
switch kind {
1163+
case "snippet":
1164+
let path = try container.decode(String.self, forKey: .path)
1165+
// A snippet names a file we resolve inside the Swift target, so it must be
1166+
// rooted there and must not traverse out of it.
1167+
guard path.hasPrefix("/"), !path.split(separator: "/").contains("..") else {
1168+
throw DecodingError.dataCorruptedError(
1169+
forKey: .path,
1170+
in: container,
1171+
debugDescription: "Snippet path '\(path)' must start with '/' and must not contain '..'."
1172+
)
1173+
}
1174+
self = .snippet(path)
1175+
case "module":
1176+
let specifier = try container.decode(String.self, forKey: .specifier)
1177+
// A bare specifier is resolved by the JavaScript host, so almost anything is
1178+
// legal, but the shapes that can never work are rejected here as well as at
1179+
// parse time: an empty specifier, a relative one, and a rooted path.
1180+
guard !specifier.isEmpty else {
1181+
throw DecodingError.dataCorruptedError(
1182+
forKey: .specifier,
1183+
in: container,
1184+
debugDescription: "Module specifier must not be empty."
1185+
)
1186+
}
1187+
guard !specifier.hasPrefix("."), !specifier.hasPrefix("/") else {
1188+
throw DecodingError.dataCorruptedError(
1189+
forKey: .specifier,
1190+
in: container,
1191+
debugDescription: "Module specifier '\(specifier)' must not be a path. Use a snippet instead."
1192+
)
1193+
}
1194+
self = .module(specifier)
1195+
default:
11651196
throw DecodingError.dataCorruptedError(
11661197
forKey: .kind,
11671198
in: container,
1168-
debugDescription: "Unknown import origin kind '\(kind)'. Expected \"module\"."
1169-
)
1170-
}
1171-
let specifier = try container.decode(String.self, forKey: .specifier)
1172-
// A bare specifier is resolved by the JavaScript host, so almost anything is
1173-
// legal, but the shapes that can never work are rejected here as well as at
1174-
// parse time: an empty specifier, a relative one, and a rooted path (which is
1175-
// a snippet and must be encoded as a plain string instead).
1176-
guard !specifier.isEmpty else {
1177-
throw DecodingError.dataCorruptedError(
1178-
forKey: .specifier,
1179-
in: container,
1180-
debugDescription: "Module specifier must not be empty."
1181-
)
1182-
}
1183-
guard !specifier.hasPrefix("."), !specifier.hasPrefix("/") else {
1184-
throw DecodingError.dataCorruptedError(
1185-
forKey: .specifier,
1186-
in: container,
1187-
debugDescription:
1188-
"Module specifier '\(specifier)' must not be a path. Rooted snippet paths are encoded as a plain string."
1199+
debugDescription: "Unknown import origin kind '\(kind)'. Expected \"snippet\" or \"module\"."
11891200
)
11901201
}
1191-
self = .module(specifier)
11921202
}
11931203

11941204
public func encode(to encoder: any Encoder) throws {
1195-
guard let specifier = moduleSpecifier else {
1205+
switch self {
1206+
case .global:
11961207
var container = encoder.singleValueContainer()
1197-
try container.encode(snippetPath ?? "global")
1198-
return
1208+
try container.encode("global")
1209+
case .snippet(let path):
1210+
var container = encoder.container(keyedBy: CodingKeys.self)
1211+
try container.encode("snippet", forKey: .kind)
1212+
try container.encode(path, forKey: .path)
1213+
case .module(let specifier):
1214+
var container = encoder.container(keyedBy: CodingKeys.self)
1215+
try container.encode("module", forKey: .kind)
1216+
try container.encode(specifier, forKey: .specifier)
11991217
}
1200-
var container = encoder.container(keyedBy: CodingKeys.self)
1201-
try container.encode("module", forKey: .kind)
1202-
try container.encode(specifier, forKey: .specifier)
12031218
}
12041219

12051220
/// The path of a target-local JavaScript file, rooted at the Swift target directory.

‎Plugins/BridgeJS/Tests/BridgeJSToolTests/BridgeJSCodegenTests.swift‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,16 +50,20 @@ import Testing
5050
#expect(throws: DecodingError.self) {
5151
try JSONDecoder().decode(JSImportFrom.self, from: Data(#""module.js""#.utf8))
5252
}
53+
// A bare path string is no longer an origin at all; snippets are tagged.
54+
#expect(throws: DecodingError.self) {
55+
try JSONDecoder().decode(JSImportFrom.self, from: Data(#""/Modules/utils.mjs""#.utf8))
56+
}
5357
}
5458

5559
@Test(arguments: [
5660
JSImportFrom.global,
5761
JSImportFrom.snippet("/Modules/utils.mjs"),
5862
JSImportFrom.module("node:path"),
5963
JSImportFrom.module("@scope/package/sub"),
60-
// A package literally named "global" is why bare specifiers are encoded as a
61-
// tagged object rather than a plain string: a plain string would be
62-
// indistinguishable from the `.global` sentinel.
64+
// A package literally named "global" is why specifiers are encoded as tagged
65+
// objects rather than plain strings: a plain string would be indistinguishable
66+
// from the `.global` sentinel.
6367
JSImportFrom.module("global"),
6468
JSImportFrom.module("#internal"),
6569
JSImportFrom.module("https://esm.sh/lodash@4"),
@@ -70,11 +74,17 @@ import Testing
7074
}
7175

7276
@Test
73-
func legacyStringFormIsPreservedForGlobalAndLocalPaths() throws {
77+
func originsEncodeToTheirDocumentedShapes() throws {
7478
let encoder = JSONEncoder()
79+
encoder.outputFormatting = [.sortedKeys]
7580
#expect(String(data: try encoder.encode(JSImportFrom.global), encoding: .utf8) == #""global""#)
7681
#expect(
77-
String(data: try encoder.encode(JSImportFrom.snippet("/a.js")), encoding: .utf8) == #""\/a.js""#
82+
String(data: try encoder.encode(JSImportFrom.snippet("/a.js")), encoding: .utf8)
83+
== #"{"kind":"snippet","path":"\/a.js"}"#
84+
)
85+
#expect(
86+
String(data: try encoder.encode(JSImportFrom.module("node:path")), encoding: .utf8)
87+
== #"{"kind":"module","specifier":"node:path"}"#
7888
)
7989
}
8090

@@ -94,6 +104,9 @@ import Testing
94104
#"{"kind": "module", "specifier": ""}"#,
95105
#"{"kind": "module", "specifier": "./relative.mjs"}"#,
96106
#"{"kind": "module", "specifier": "/../../escape.mjs"}"#,
107+
#"{"kind": "snippet", "path": "node:path"}"#,
108+
#"{"kind": "snippet", "path": "/../../escape.mjs"}"#,
109+
#"{"kind": "snippet", "path": "relative.mjs"}"#,
97110
])
98111
func invalidKeyedJSImportFromFailsToDecode(json: String) {
99112
#expect(throws: DecodingError.self) {

‎Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSCodegenTests/JSImportBareModule.json‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,10 @@
114114
},
115115
{
116116
"accessLevel" : "internal",
117-
"from" : "\/Modules\/DefaultExport.mjs",
117+
"from" : {
118+
"kind" : "snippet",
119+
"path" : "\/Modules\/DefaultExport.mjs"
120+
},
118121
"jsName" : "default",
119122
"name" : "localDefaultExport",
120123
"type" : {

‎Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSCodegenTests/JSImportModule.json‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,10 @@
1010
"isStatic" : false,
1111
"isThrows" : true
1212
},
13-
"from" : "\/Modules\/JSImportModule.mjs",
13+
"from" : {
14+
"kind" : "snippet",
15+
"path" : "\/Modules\/JSImportModule.mjs"
16+
},
1417
"name" : "moduleAdd",
1518
"parameters" : [
1619
{
@@ -52,7 +55,10 @@
5255
"isStatic" : false,
5356
"isThrows" : true
5457
},
55-
"from" : "\/Modules\/JSImportModule.mjs",
58+
"from" : {
59+
"kind" : "snippet",
60+
"path" : "\/Modules\/JSImportModule.mjs"
61+
},
5662
"jsName" : "renamedFunction",
5763
"name" : "moduleRenamed",
5864
"parameters" : [
@@ -68,7 +74,10 @@
6874
"globalGetters" : [
6975
{
7076
"accessLevel" : "internal",
71-
"from" : "\/Modules\/JSImportModule.mjs",
77+
"from" : {
78+
"kind" : "snippet",
79+
"path" : "\/Modules\/JSImportModule.mjs"
80+
},
7281
"jsName" : "version",
7382
"name" : "moduleVersion",
7483
"type" : {
@@ -97,7 +106,10 @@
97106
}
98107
]
99108
},
100-
"from" : "\/Modules\/ModuleCounter.mjs",
109+
"from" : {
110+
"kind" : "snippet",
111+
"path" : "\/Modules\/ModuleCounter.mjs"
112+
},
101113
"getters" : [
102114
{
103115
"accessLevel" : "internal",

‎Tests/BridgeJSRuntimeTests/Generated/JavaScript/BridgeJS.json‎

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24246,7 +24246,10 @@
2424624246
"globalGetters" : [
2424724247
{
2424824248
"accessLevel" : "internal",
24249-
"from" : "\/Modules\/DefaultExport.mjs",
24249+
"from" : {
24250+
"kind" : "snippet",
24251+
"path" : "\/Modules\/DefaultExport.mjs"
24252+
},
2425024253
"jsName" : "default",
2425124254
"name" : "defaultExport",
2425224255
"type" : {
@@ -24313,7 +24316,10 @@
2431324316
"isStatic" : false,
2431424317
"isThrows" : true
2431524318
},
24316-
"from" : "\/Modules\/JSImportModule.mjs",
24319+
"from" : {
24320+
"kind" : "snippet",
24321+
"path" : "\/Modules\/JSImportModule.mjs"
24322+
},
2431724323
"name" : "moduleAdd",
2431824324
"parameters" : [
2431924325
{
@@ -24355,7 +24361,10 @@
2435524361
"isStatic" : false,
2435624362
"isThrows" : true
2435724363
},
24358-
"from" : "\/Modules\/JSImportModule.mjs",
24364+
"from" : {
24365+
"kind" : "snippet",
24366+
"path" : "\/Modules\/JSImportModule.mjs"
24367+
},
2435924368
"jsName" : "renamedFunction",
2436024369
"name" : "moduleRenamed",
2436124370
"parameters" : [
@@ -24374,7 +24383,10 @@
2437424383
"isStatic" : false,
2437524384
"isThrows" : true
2437624385
},
24377-
"from" : "\/Modules\/JSImportModule.mjs",
24386+
"from" : {
24387+
"kind" : "snippet",
24388+
"path" : "\/Modules\/JSImportModule.mjs"
24389+
},
2437824390
"name" : "moduleThrow",
2437924391
"parameters" : [
2438024392

@@ -24389,7 +24401,10 @@
2438924401
"globalGetters" : [
2439024402
{
2439124403
"accessLevel" : "internal",
24392-
"from" : "\/Modules\/JSImportModule.mjs",
24404+
"from" : {
24405+
"kind" : "snippet",
24406+
"path" : "\/Modules\/JSImportModule.mjs"
24407+
},
2439324408
"jsName" : "version",
2439424409
"name" : "moduleVersion",
2439524410
"type" : {
@@ -24418,7 +24433,10 @@
2441824433
}
2441924434
]
2442024435
},
24421-
"from" : "\/Modules\/ModuleCounter.mjs",
24436+
"from" : {
24437+
"kind" : "snippet",
24438+
"path" : "\/Modules\/ModuleCounter.mjs"
24439+
},
2442224440
"getters" : [
2442324441
{
2442424442
"accessLevel" : "internal",

0 commit comments

Comments
 (0)