Skip to content

Commit 62382f7

Browse files
krodakkrodak
authored
Merge pull request swiftwasm#812 from PassiveLogic/fix/protocol-conformance-audit
BridgeJS: Diagnose unsupported protocol conformers before lowering
2 parents cadafdc + 712c3a7 commit 62382f7

12 files changed

Lines changed: 283 additions & 45 deletions

File tree

‎Plugins/BridgeJS/Sources/BridgeJSCore/ClosureCodegen.swift‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -182,15 +182,15 @@ public struct ClosureCodegen {
182182
} else {
183183
printer.write("let result = \(closureCallExpr)")
184184
switch signature.returnType {
185-
case .swiftProtocol:
185+
case .swiftProtocol(let protocolName):
186186
printer.write(
187-
"return (result as! _BridgedSwiftProtocolExportable).bridgeJSLowerAsProtocolReturn()"
187+
"return _bridgeJSUnwrapProtocolExportable(result, \"\(protocolName)\").bridgeJSLowerAsProtocolReturn()"
188188
)
189-
case .nullable(.swiftProtocol, _):
189+
case .nullable(.swiftProtocol(let protocolName), _):
190190
printer.write("if let result {")
191191
printer.indent {
192192
printer.write(
193-
"_swift_js_return_optional_object(1, (result as! _BridgedSwiftProtocolExportable).bridgeJSLowerAsProtocolReturn())"
193+
"_swift_js_return_optional_object(1, _bridgeJSUnwrapProtocolExportable(result, \"\(protocolName)\").bridgeJSLowerAsProtocolReturn())"
194194
)
195195
}
196196
printer.write("} else {")

‎Plugins/BridgeJS/Sources/BridgeJSCore/ExportSwift.swift‎

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -262,8 +262,8 @@ public class ExportSwift {
262262

263263
private func protocolCastSuffix(for returnType: BridgeType) -> (prefix: String, suffix: String) {
264264
switch returnType {
265-
case .swiftProtocol:
266-
return ("", " as! _BridgedSwiftProtocolExportable")
265+
case .swiftProtocol(let name):
266+
return ("_bridgeJSUnwrapProtocolExportable(", ", \"\(name)\")")
267267
default:
268268
return ("", "")
269269
}
@@ -424,11 +424,11 @@ public class ExportSwift {
424424
}
425425
case .swiftProtocol:
426426
append("return ret.bridgeJSLowerAsProtocolReturn()")
427-
case .nullable(.swiftProtocol, _):
427+
case .nullable(.swiftProtocol(let protocolName), _):
428428
append(
429429
"""
430430
if let ret {
431-
_swift_js_return_optional_object(1, (ret as! _BridgedSwiftProtocolExportable).bridgeJSLowerAsProtocolReturn())
431+
_swift_js_return_optional_object(1, _bridgeJSUnwrapProtocolExportable(ret, "\(raw: protocolName)").bridgeJSLowerAsProtocolReturn())
432432
} else {
433433
_swift_js_return_optional_object(0, 0)
434434
}
@@ -993,9 +993,9 @@ struct StackCodegen {
993993
return ["\(raw: accessor).bridgeJSStackPush()"]
994994
case .jsObject(_?):
995995
return ["\(raw: accessor).jsObject.bridgeJSStackPush()"]
996-
case .swiftProtocol:
996+
case .swiftProtocol(let protocolName):
997997
return [
998-
"_swift_js_push_i32((\(raw: accessor) as! _BridgedSwiftProtocolExportable).bridgeJSLowerAsProtocolReturn())"
998+
"_swift_js_push_i32(_bridgeJSUnwrapProtocolExportable(\(raw: accessor), \"\(raw: protocolName)\").bridgeJSLowerAsProtocolReturn())"
999999
]
10001000
case .void, .namespaceEnum:
10011001
return []
@@ -1016,8 +1016,8 @@ struct StackCodegen {
10161016
varPrefix: String
10171017
) -> [CodeBlockItemSyntax] {
10181018
switch elementType {
1019-
case .swiftProtocol:
1020-
return lowerProtocolArrayStatements(accessor: accessor, varPrefix: varPrefix)
1019+
case .swiftProtocol(let protocolName):
1020+
return lowerProtocolArrayStatements(protocolName: protocolName, accessor: accessor, varPrefix: varPrefix)
10211021
case .void, .namespaceEnum:
10221022
fatalError("Invalid array element type: \(elementType)")
10231023
default:
@@ -1026,14 +1026,15 @@ struct StackCodegen {
10261026
}
10271027

10281028
private func lowerProtocolArrayStatements(
1029+
protocolName: String,
10291030
accessor: String,
10301031
varPrefix: String
10311032
) -> [CodeBlockItemSyntax] {
10321033
let elemVar = "__bjs_elem_\(varPrefix)"
10331034
return [
10341035
"""
10351036
for \(raw: elemVar) in \(raw: accessor) {
1036-
_swift_js_push_i32((\(raw: elemVar) as! _BridgedSwiftProtocolExportable).bridgeJSLowerAsProtocolReturn())
1037+
_swift_js_push_i32(_bridgeJSUnwrapProtocolExportable(\(raw: elemVar), "\(raw: protocolName)").bridgeJSLowerAsProtocolReturn())
10371038
}
10381039
""",
10391040
"_swift_js_push_i32(Int32(\(raw: accessor).count))",
@@ -1048,8 +1049,12 @@ struct StackCodegen {
10481049
switch valueType {
10491050
case .jsObject(let className?) where className != "JSObject":
10501051
return ["\(raw: accessor).mapValues { $0.jsObject }.bridgeJSStackPush()"]
1051-
case .swiftProtocol:
1052-
return lowerProtocolDictionaryStatements(accessor: accessor, varPrefix: varPrefix)
1052+
case .swiftProtocol(let protocolName):
1053+
return lowerProtocolDictionaryStatements(
1054+
protocolName: protocolName,
1055+
accessor: accessor,
1056+
varPrefix: varPrefix
1057+
)
10531058
case .nullable, .closure:
10541059
return lowerDictionaryStatementsInline(
10551060
valueType: valueType,
@@ -1107,6 +1112,7 @@ struct StackCodegen {
11071112
}
11081113

11091114
private func lowerProtocolDictionaryStatements(
1115+
protocolName: String,
11101116
accessor: String,
11111117
varPrefix: String
11121118
) -> [CodeBlockItemSyntax] {
@@ -1115,7 +1121,7 @@ struct StackCodegen {
11151121
"""
11161122
for \(raw: pairVar) in \(raw: accessor) {
11171123
\(raw: pairVar).key.bridgeJSStackPush()
1118-
_swift_js_push_i32((\(raw: pairVar).value as! _BridgedSwiftProtocolExportable).bridgeJSLowerAsProtocolReturn())
1124+
_swift_js_push_i32(_bridgeJSUnwrapProtocolExportable(\(raw: pairVar).value, "\(raw: protocolName)").bridgeJSLowerAsProtocolReturn())
11191125
}
11201126
""",
11211127
"_swift_js_push_i32(Int32(\(raw: accessor).count))",

‎Plugins/BridgeJS/Sources/BridgeJSCore/ImportTS.swift‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -188,12 +188,12 @@ public struct ImportTS {
188188
}
189189
)
190190
)
191-
} else if case .nullable(.swiftProtocol, _) = param.type, context == .exportSwift {
191+
} else if case .nullable(.swiftProtocol(let protocolName), _) = param.type, context == .exportSwift {
192192
body.write("let \(pattern): (Int32, Int32)")
193193
body.write("if let \(param.name) {")
194194
body.indent {
195195
body.write(
196-
"\(pattern) = (1, (\(param.name) as! _BridgedSwiftProtocolExportable).bridgeJSLowerAsProtocolReturn())"
196+
"\(pattern) = (1, _bridgeJSUnwrapProtocolExportable(\(param.name), \"\(protocolName)\").bridgeJSLowerAsProtocolReturn())"
197197
)
198198
}
199199
body.write("} else {")
@@ -203,9 +203,9 @@ public struct ImportTS {
203203
body.write("}")
204204
} else {
205205
let initializerExpr: ExprSyntax
206-
if case .swiftProtocol = param.type, context == .exportSwift {
206+
if case .swiftProtocol(let protocolName) = param.type, context == .exportSwift {
207207
initializerExpr = ExprSyntax(
208-
"(\(raw: param.name) as! _BridgedSwiftProtocolExportable).bridgeJSLowerAsProtocolReturn()"
208+
"_bridgeJSUnwrapProtocolExportable(\(raw: param.name), \"\(raw: protocolName)\").bridgeJSLowerAsProtocolReturn()"
209209
)
210210
} else {
211211
initializerExpr = ExprSyntax("\(raw: param.name).bridgeJSLowerParameter()")

‎Plugins/BridgeJS/Sources/BridgeJSCore/SwiftToSkeleton.swift‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,8 @@ public final class SwiftToSkeleton {
295295
collector.finalize(&exported)
296296
}
297297

298+
perSourceErrors.append(contentsOf: diagnoseProtocolConformances(in: exported))
299+
298300
if !perSourceErrors.isEmpty {
299301
let diagnostics = perSourceErrors.flatMap { inputFilePath, errors in
300302
errors.map { (file: inputFilePath, diagnostic: $0) }
@@ -318,6 +320,99 @@ public final class SwiftToSkeleton {
318320
)
319321
}
320322

323+
private func diagnoseProtocolConformances(
324+
in exported: ExportedSkeleton
325+
) -> [(inputFilePath: String, errors: [DiagnosticError])] {
326+
var loweredProtocols: Set<String> = []
327+
func collect(_ type: BridgeType, loweredBySwift: Bool) {
328+
switch type {
329+
case .swiftProtocol(let name):
330+
if loweredBySwift { loweredProtocols.insert(name) }
331+
case .array(let element), .dictionary(let element), .nullable(let element, _), .alias(_, let element):
332+
collect(element, loweredBySwift: loweredBySwift)
333+
case .closure(let signature, _):
334+
for parameter in signature.parameters {
335+
collect(parameter, loweredBySwift: !loweredBySwift)
336+
}
337+
collect(signature.returnType, loweredBySwift: loweredBySwift)
338+
default:
339+
break
340+
}
341+
}
342+
func collect(_ function: ExportedFunction, loweredReturn: Bool = true) {
343+
for parameter in function.parameters { collect(parameter.type, loweredBySwift: !loweredReturn) }
344+
collect(function.returnType, loweredBySwift: loweredReturn)
345+
}
346+
347+
for function in exported.functions + exported.classes.flatMap(\.methods)
348+
+ exported.structs.flatMap(\.methods) + exported.enums.flatMap(\.staticMethods)
349+
{
350+
collect(function)
351+
}
352+
for constructor in exported.classes.compactMap(\.constructor) + exported.structs.compactMap(\.constructor) {
353+
for parameter in constructor.parameters { collect(parameter.type, loweredBySwift: false) }
354+
}
355+
for property in exported.classes.flatMap(\.properties) + exported.enums.flatMap(\.staticProperties) {
356+
collect(property.type, loweredBySwift: true)
357+
if !property.isReadonly { collect(property.type, loweredBySwift: false) }
358+
}
359+
for property in exported.structs.flatMap(\.properties) {
360+
collect(property.type, loweredBySwift: true)
361+
if !property.isStatic || !property.isReadonly { collect(property.type, loweredBySwift: false) }
362+
}
363+
for value in exported.enums.flatMap(\.cases).flatMap(\.associatedValues) {
364+
collect(value.type, loweredBySwift: true)
365+
}
366+
for protocolDef in exported.protocols {
367+
for method in protocolDef.methods { collect(method, loweredReturn: false) }
368+
for property in protocolDef.properties {
369+
collect(property.type, loweredBySwift: false)
370+
if !property.isReadonly { collect(property.type, loweredBySwift: true) }
371+
}
372+
}
373+
374+
guard !loweredProtocols.isEmpty else { return [] }
375+
var diagnostics: [(inputFilePath: String, errors: [DiagnosticError])] = []
376+
for declaration in typeDeclResolver.declarationsWithInheritance where !declaration.is(ProtocolDeclSyntax.self) {
377+
let extendedType = declaration.as(ExtensionDeclSyntax.self)?.extendedType
378+
let target = extendedType.flatMap { typeDeclResolver.resolveExtensionTarget($0) }
379+
let classDecl = declaration.as(ClassDeclSyntax.self) ?? target?.as(ClassDeclSyntax.self)
380+
if classDecl?.attributes.hasJSAttribute() == true { continue }
381+
if let extendedType, target == nil {
382+
var errors: [DiagnosticError] = []
383+
if case .swiftHeapObject = resolveExternal(for: extendedType, errors: &errors) { continue }
384+
}
385+
guard
386+
let name = declaration.asProtocol(NamedDeclSyntax.self)?.name.text ?? extendedType?.trimmedDescription,
387+
let inputFilePath = sourceFiles.first(where: { $0.sourceFile.id == declaration.root.id })?.inputFilePath
388+
else { continue }
389+
for inherited in declaration.inheritanceClause?.inheritedTypes ?? [] {
390+
guard let protocolDecl = typeDeclResolver.resolve(inherited.type)?.as(ProtocolDeclSyntax.self),
391+
protocolDecl.attributes.hasJSAttribute(), loweredProtocols.contains(protocolDecl.name.text)
392+
else { continue }
393+
let protocolName = protocolDecl.name.text
394+
diagnostics.append(
395+
(
396+
inputFilePath,
397+
[
398+
DiagnosticError(
399+
node: declaration,
400+
message:
401+
"'\(name)' conforms to '\(protocolName)', a @JS protocol that exported APIs "
402+
+ "bridge to JavaScript, but '\(name)' is not a '@JS class'. Passing it to "
403+
+ "JavaScript as 'any \(protocolName)' would trap at runtime.",
404+
hint:
405+
"Mark '\(name)' as a '@JS class' so it can cross the bridge, or avoid using "
406+
+ "'\(protocolName)' as an existential in exported APIs."
407+
)
408+
]
409+
)
410+
)
411+
}
412+
}
413+
return diagnostics
414+
}
415+
321416
private static let jsTypedArrayTypealiasNames: [String: String] = [
322417
"Int8": "JSInt8Array",
323418
"UInt8": "JSUint8Array",

‎Plugins/BridgeJS/Sources/BridgeJSCore/TypeDeclResolver.swift‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ class TypeDeclResolver {
99
typealias QualifiedName = [String]
1010
private var typeDeclByQualifiedName: [QualifiedName: TypeDecl] = [:]
1111
private var typeAliasByQualifiedName: [QualifiedName: TypeAliasDeclSyntax] = [:]
12+
private(set) var declarationsWithInheritance: [any DeclGroupSyntax] = []
1213

1314
enum Error: Swift.Error {
1415
case typeNotFound(QualifiedName)
@@ -24,6 +25,9 @@ class TypeDeclResolver {
2425
}
2526

2627
func visitNominalDecl(_ node: TypeDecl) -> SyntaxVisitorContinueKind {
28+
if node.inheritanceClause != nil {
29+
resolver.declarationsWithInheritance.append(node)
30+
}
2731
let name = node.name.text
2832
let qualifiedName = scope + [name]
2933
resolver.typeDeclByQualifiedName[qualifiedName] = node
@@ -74,6 +78,9 @@ class TypeDeclResolver {
7478
}
7579

7680
override func visit(_ node: ExtensionDeclSyntax) -> SyntaxVisitorContinueKind {
81+
if node.inheritanceClause != nil {
82+
resolver.declarationsWithInheritance.append(node)
83+
}
7784
guard let components = node.memberScopeComponents else {
7885
return .skipChildren
7986
}

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -487,6 +487,27 @@ import Testing
487487
}
488488
}
489489

490+
@Test func exportedClassConformanceInAnotherModule() throws {
491+
let core = try makeSkeleton(
492+
"""
493+
@JS public class MyImpl {
494+
@JS public init() {}
495+
@JS public func ok() -> Int { 42 }
496+
}
497+
""",
498+
moduleName: "Core"
499+
)
500+
_ = try makeSkeleton(
501+
"""
502+
import Core
503+
@JS protocol P { func ok() -> Int }
504+
extension MyImpl: P {}
505+
@JS func get() -> P { MyImpl() }
506+
""",
507+
dependencies: [(moduleName: "Core", skeleton: core)]
508+
)
509+
}
510+
490511
// MARK: - Utillites
491512

492513
private func resolveApp(

0 commit comments

Comments
 (0)