BridgeJS: Warn on async throwing closures passed to JavaScript

Author: krodak

Plugins/BridgeJS/Sources/BridgeJSCore/Misc.swift

@@ -137,14 +137,21 @@ import SwiftSyntax
 import class Foundation.ProcessInfo
 
 public struct DiagnosticError: Error {
+    public enum Severity: String, Sendable {
+        case error
+        case warning
+    }
+
     public let node: Syntax
     public let message: String
     public let hint: String?
+    public let severity: Severity
 
-    public init(node: some SyntaxProtocol, message: String, hint: String? = nil) {
+    public init(node: some SyntaxProtocol, message: String, hint: String? = nil, severity: Severity = .error) {
         self.node = Syntax(node)
         self.message = message
         self.hint = hint
+        self.severity = severity
     }
 
     /// Formats the diagnostic error as a string.
@@ -166,12 +173,14 @@ public struct DiagnosticError: Error {
 
         let lineNumberWidth = max(3, String(lines.count).count)
 
+        let severityLabel = severity.rawValue
+        let severityColor = severity == .warning ? ANSI.boldYellow : ANSI.boldRed
         let header: String = {
             guard colorize else {
-                return "\(displayFileName):\(startLocation.line):\(startLocation.column): error: \(message)"
+                return "\(displayFileName):\(startLocation.line):\(startLocation.column): \(severityLabel): \(message)"
             }
             return
-                "\(displayFileName):\(startLocation.line):\(startLocation.column): \(ANSI.boldRed)error: \(ANSI.boldDefault)\(message)\(ANSI.reset)"
+                "\(displayFileName):\(startLocation.line):\(startLocation.column): \(severityColor)\(severityLabel): \(ANSI.boldDefault)\(message)\(ANSI.reset)"
         }()
 
         let highlightStartColumn = min(max(1, startLocation.column), mainLine.utf8.count + 1)
@@ -227,8 +236,8 @@ public struct DiagnosticError: Error {
         let pointerSpacing = max(0, highlightStartColumn - 1)
         let pointerMessage: String = {
             let pointer = String(repeating: " ", count: pointerSpacing) + "`- "
-            guard colorize else { return pointer + "error: \(message)" }
-            return pointer + "\(ANSI.boldRed)error: \(ANSI.boldDefault)\(message)\(ANSI.reset)"
+            guard colorize else { return pointer + "\(severityLabel): \(message)" }
+            return pointer + "\(severityColor)\(severityLabel): \(ANSI.boldDefault)\(message)\(ANSI.reset)"
         }()
         descriptionParts.append(
             Self.formatSourceLine(
@@ -304,6 +313,7 @@ public struct BridgeJSCoreDiagnosticError: Swift.Error, CustomStringConvertible
 private enum ANSI {
     static let reset = "\u{001B}[0;0m"
     static let boldRed = "\u{001B}[1;31m"
+    static let boldYellow = "\u{001B}[1;33m"
     static let boldDefault = "\u{001B}[1;39m"
     static let cyan = "\u{001B}[0;36m"
     static let underline = "\u{001B}[4;39m"

Plugins/BridgeJS/Sources/BridgeJSCore/SwiftToSkeleton.swift

@@ -24,6 +24,9 @@ public final class SwiftToSkeleton {
     private var sourceFiles: [(sourceFile: SourceFileSyntax, inputFilePath: String)] = []
     private var usedExternalModules = Set<String>()
 
+    /// Non-fatal diagnostics collected during `finalize()`. These do not fail the build.
+    public private(set) var warnings: [(file: String, diagnostic: DiagnosticError)] = []
+
     public init(
         progress: ProgressReporting,
         moduleName: String,
@@ -87,10 +90,15 @@ public final class SwiftToSkeleton {
             )
             importCollector.walk(sourceFile)
 
-            let importErrorsFatal = importCollector.errors.filter { !$0.message.contains("Unsupported type '") }
-            if !exportCollector.errors.isEmpty || !importErrorsFatal.isEmpty {
+            let exportErrors = exportCollector.errors.filter { $0.severity == .error }
+            let importErrorsFatal = importCollector.errors.filter {
+                $0.severity == .error && !$0.message.contains("Unsupported type '")
+            }
+            let fileWarnings = (exportCollector.errors + importCollector.errors).filter { $0.severity == .warning }
+            warnings.append(contentsOf: fileWarnings.map { (file: inputFilePath, diagnostic: $0) })
+            if !exportErrors.isEmpty || !importErrorsFatal.isEmpty {
                 perSourceErrors.append(
-                    (inputFilePath: inputFilePath, errors: exportCollector.errors + importErrorsFatal)
+                    (inputFilePath: inputFilePath, errors: exportErrors + importErrorsFatal)
                 )
             }
 
@@ -602,6 +610,37 @@ private enum ExportSwiftConstants {
     static let supportedRawTypes = SwiftEnumRawType.supportedTypeNames
 }
 
+/// Warns about Swift closures handed to JavaScript with an `async throws(JSException)` signature.
+/// Captureless closure values lose their thrown error at runtime due to a Swift compiler bug.
+private func asyncThrowsClosureWarning(node: some SyntaxProtocol) -> DiagnosticError {
+    DiagnosticError(
+        node: node,
+        message:
+            "async throwing closures passed to JavaScript may lose thrown errors due to a Swift compiler bug "
+            + "(swiftlang/swift#89320) unless the closure value captures state",
+        hint:
+            "Pass a closure that captures state, or see the BridgeJS closure documentation for details",
+        severity: .warning
+    )
+}
+
+extension BridgeType {
+    fileprivate var containsAsyncThrowsClosure: Bool {
+        switch self {
+        case .closure(let signature, _):
+            return signature.isAsync && signature.isThrows
+        case .nullable(let wrapped, _):
+            return wrapped.containsAsyncThrowsClosure
+        case .array(let element):
+            return element.containsAsyncThrowsClosure
+        case .dictionary(let value):
+            return value.containsAsyncThrowsClosure
+        default:
+            return false
+        }
+    }
+}
+
 extension AttributeSyntax {
     /// The attribute name as text when it is a simple identifier (e.g. "JS", "JSFunction").
     /// Prefer this over `attributeName.trimmedDescription` for name checks to avoid unnecessary string work.
@@ -1194,6 +1233,9 @@ private final class ExportSwiftAPICollector: SyntaxAnyVisitor {
 
             guard let type = resolvedType else { return nil }
             returnType = type
+            if returnType.containsAsyncThrowsClosure {
+                errors.append(asyncThrowsClosureWarning(node: returnClause.type))
+            }
         } else {
             returnType = .void
         }
@@ -2853,6 +2895,11 @@ private final class ImportSwiftMacrosAPICollector: SyntaxAnyVisitor {
             guard let bridgeType = withLookupErrors({ parent.lookupType(for: type, errors: &$0) }) else {
                 return nil
             }
+            if case .closure(let signature, useJSTypedClosure: true) = bridgeType,
+                signature.isAsync, signature.isThrows
+            {
+                errors.append(asyncThrowsClosureWarning(node: type))
+            }
             let nameToken = param.secondName ?? param.firstName
             let name = SwiftToSkeleton.normalizeIdentifier(nameToken.text)
             let labelToken = param.secondName == nil ? nil : param.firstName

Plugins/BridgeJS/Sources/BridgeJSTool/BridgeJSTool.swift

@@ -201,6 +201,9 @@ import BridgeJSUtilities
             let skeleton = try withSpan("SwiftToSkeleton.finalize") {
                 return try swiftToSkeleton.finalize()
             }
+            for (file, diagnostic) in swiftToSkeleton.warnings {
+                printStderr(diagnostic.formattedDescription(fileName: file))
+            }
 
             var exporter: ExportSwift?
             if let skeleton = skeleton.exported {

Plugins/BridgeJS/Sources/BridgeJSToolInternal/BridgeJSToolInternal.swift

@@ -60,6 +60,11 @@ import ArgumentParser
                 swiftToSkeleton.addSourceFile(sourceFile, inputFilePath: inputFile)
             }
             let skeleton = try swiftToSkeleton.finalize()
+            for (file, diagnostic) in swiftToSkeleton.warnings {
+                FileHandle.standardError.write(
+                    Data((diagnostic.formattedDescription(fileName: file, colorize: false) + "\n").utf8)
+                )
+            }
             let encoder = JSONEncoder()
             encoder.outputFormatting = [.prettyPrinted, .sortedKeys]
             let skeletonData = try encoder.encode(skeleton)

Plugins/BridgeJS/Tests/BridgeJSToolTests/ClosureAsyncThrowsWarningTests.swift

@@ -0,0 +1,122 @@
+import Foundation
+import SwiftParser
+import SwiftSyntax
+import Testing
+
+@testable import BridgeJSCore
+@testable import BridgeJSSkeleton
+
+@Suite struct ClosureAsyncThrowsWarningTests {
+    @Test
+    func warnsOnTypedAsyncThrowsClosureReturn() throws {
+        let result = try resolveApp(
+            source: """
+                @JS public func makeParser() -> JSTypedClosure<(String) async throws(JSException) -> String> {
+                    fatalError()
+                }
+                """
+        )
+        #expect(result.warnings.count == 1)
+        let warning = try #require(result.warnings.first)
+        #expect(warning.diagnostic.severity == .warning)
+        #expect(warning.diagnostic.message.contains("swiftlang/swift#89320"))
+    }
+
+    @Test
+    func warnsOnPlainAsyncThrowsClosureReturn() throws {
+        let result = try resolveApp(
+            source: """
+                @JS public func makeParser() -> (String) async throws(JSException) -> String {
+                    fatalError()
+                }
+                """
+        )
+        #expect(result.warnings.count == 1)
+        #expect(result.warnings.first?.diagnostic.severity == .warning)
+    }
+
+    @Test
+    func doesNotWarnOnAsyncThrowsClosureParameter() throws {
+        let result = try resolveApp(
+            source: """
+                @JS public func process(_ cb: (String) async throws(JSException) -> String) {}
+                """
+        )
+        #expect(result.warnings.isEmpty)
+    }
+
+    @Test
+    func doesNotWarnOnNonThrowingAsyncClosureReturn() throws {
+        let result = try resolveApp(
+            source: """
+                @JS public func makeParser() -> JSTypedClosure<(String) async -> String> {
+                    fatalError()
+                }
+                """
+        )
+        #expect(result.warnings.isEmpty)
+    }
+
+    @Test
+    func doesNotWarnOnSyncThrowsClosureReturn() throws {
+        let result = try resolveApp(
+            source: """
+                @JS public func makeParser() -> JSTypedClosure<(String) throws(JSException) -> String> {
+                    fatalError()
+                }
+                """
+        )
+        #expect(result.warnings.isEmpty)
+    }
+
+    @Test
+    func warnsOnTypedAsyncThrowsClosureImportParameter() throws {
+        let result = try resolveApp(
+            source: """
+                @JSFunction func register(
+                    _ cb: JSTypedClosure<(String) async throws(JSException) -> String>
+                ) throws(JSException)
+                """
+        )
+        #expect(result.warnings.count == 1)
+        #expect(result.warnings.first?.diagnostic.severity == .warning)
+    }
+
+    @Test
+    func warningDoesNotFailSkeletonResolution() throws {
+        let result = try resolveApp(
+            source: """
+                @JS public func makeParser() -> JSTypedClosure<(String) async throws(JSException) -> String> {
+                    fatalError()
+                }
+                """
+        )
+        let function = try #require(result.skeleton.exported?.functions.first(where: { $0.name == "makeParser" }))
+        guard case .closure(let signature, true) = function.returnType else {
+            Issue.record("Expected typed closure return type, got \(function.returnType)")
+            return
+        }
+        #expect(signature.isAsync)
+        #expect(signature.isThrows)
+    }
+
+    // MARK: - Utilities
+
+    private struct Resolution {
+        let skeleton: BridgeJSSkeleton
+        let warnings: [(file: String, diagnostic: DiagnosticError)]
+    }
+
+    private func resolveApp(source appSource: String) throws -> Resolution {
+        let swiftAPI = SwiftToSkeleton(
+            progress: .silent,
+            moduleName: "App",
+            exposeToGlobal: false,
+            externalModuleIndex: ExternalModuleIndex(dependencies: [])
+        )
+        let sourceFile = Parser.parse(source: appSource)
+        swiftAPI.addSourceFile(sourceFile, inputFilePath: "App.swift")
+        let skeleton = try swiftAPI.finalize()
+        return Resolution(skeleton: skeleton, warnings: swiftAPI.warnings)
+    }
+}

Sources/JavaScriptKit/Documentation.docc/Articles/BridgeJS/Bringing-Swift-Closures-to-JavaScript.md

@@ -78,7 +78,7 @@ const count = await fetchCount("/items"); // Promise<number>
 
 **Cancellation is a non-goal.** There is no propagation between a Swift `Task` and a JavaScript `Promise` in either direction.
 
-> Note: The reject path of async throwing typed closures is affected by a Swift compiler bug ([swiftlang/swift#89320](https://github.com/swiftlang/swift/issues/89320)). See <doc:Exporting-Swift-Closure> for details.
+> Note: The reject path of async throwing typed closures is affected by a Swift compiler bug ([swiftlang/swift#89320](https://github.com/swiftlang/swift/issues/89320)); BridgeJS emits a build-time warning for this signature. See <doc:Exporting-Swift-Closure> for details.
 
 ## Lifetime and release()
 

Sources/JavaScriptKit/Documentation.docc/Articles/BridgeJS/Exporting-Swift/Exporting-Swift-Closure.md

@@ -239,7 +239,7 @@ Notes:
 - The same `JavaScriptEventLoop.installGlobalExecutor()` requirement applies as for async functions; there is no special handling for closures.
 - **Cancellation is a non-goal.** There is no propagation between a Swift `Task` and a JavaScript `Promise` in either direction; cancelling one side does not cancel the other.
 
-> Warning: When an async throwing closure handed to JavaScript throws, the error is currently lost instead of rejecting the `Promise` with it, due to a Swift compiler bug on `wasm32` ([swiftlang/swift#89320](https://github.com/swiftlang/swift/issues/89320), fix in progress in [swiftlang/swift#89715](https://github.com/swiftlang/swift/pull/89715)). Closures that capture state are unaffected, as are throwing JavaScript callbacks passed into Swift.
+> Warning: When an async throwing closure handed to JavaScript throws, the error is currently lost instead of rejecting the `Promise` with it, due to a Swift compiler bug on `wasm32` ([swiftlang/swift#89320](https://github.com/swiftlang/swift/issues/89320), fix in progress in [swiftlang/swift#89715](https://github.com/swiftlang/swift/pull/89715)). Closures that capture state are unaffected, as are throwing JavaScript callbacks passed into Swift. BridgeJS emits a build-time warning for this signature.
 
 ## Supported Features