Fix types in extensions bugs

Author: wfltaylor

Plugins/BridgeJS/Sources/BridgeJSCore/SwiftToSkeleton.swift

@@ -139,6 +139,39 @@ public final class SwiftToSkeleton {
         sourceFiles.append((sourceFile, inputFilePath))
     }
 
+    private func resolveDeferredExtensions(_ exportCollectors: [ExportSwiftAPICollector]) {
+        var pendingExtensions = exportCollectors.flatMap { collector in
+            collector.deferredExtensions.map { (owner: collector, declaration: $0) }
+        }
+        var previousCount: Int
+        // An extended type might be defined in another extension, so keep resolving until no more progress is made.
+        repeat {
+            previousCount = pendingExtensions.count
+            var nextPendingExtensions: [(owner: ExportSwiftAPICollector, declaration: ExtensionDeclSyntax)] = []
+            for pending in pendingExtensions {
+                if !resolveExtension(pending.declaration, in: exportCollectors) {
+                    nextPendingExtensions.append(pending)
+                }
+            }
+            pendingExtensions = nextPendingExtensions
+        } while pendingExtensions.count < previousCount
+        for pending in pendingExtensions {
+            pending.owner.diagnoseUnresolvedExtension(pending.declaration)
+        }
+    }
+
+    private func resolveExtension(
+        _ declaration: ExtensionDeclSyntax,
+        in exportCollectors: [ExportSwiftAPICollector]
+    ) -> Bool {
+        for collector in exportCollectors {
+            if collector.resolveExtension(declaration) {
+                return true
+            }
+        }
+        return false
+    }
+
     public func finalize() throws -> BridgeJSSkeleton {
         var perSourceErrors: [(inputFilePath: String, errors: [DiagnosticError])] = []
         var importedFiles: [ImportedFileSkeleton] = []
@@ -244,9 +277,7 @@ public final class SwiftToSkeleton {
         }
 
         // Resolve extensions against all collectors. This needs to happen at this point so we can resolve both same file and cross file extensions.
-        for source in exportCollectors {
-            source.resolveDeferredExtensions(against: exportCollectors)
-        }
+        resolveDeferredExtensions(exportCollectors)
 
         // We have to collect diagnostics after all deferred extensions are resolved, since they could generate some.
         for ((_, inputFilePath), exportCollector) in zip(sourceFiles, exportCollectors) {
@@ -1946,23 +1977,15 @@ private final class ExportSwiftAPICollector: SyntaxAnyVisitor {
         return .skipChildren
     }
 
-    func resolveDeferredExtensions(against collectors: [ExportSwiftAPICollector]) {
-        for ext in deferredExtensions {
-            var resolved = false
-            for collector in collectors {
-                if collector.resolveExtension(ext) {
-                    resolved = true
-                    break
-                }
-            }
-            if !resolved, containsJSAnnotatedDeclaration(ext.memberBlock.members) {
-                diagnose(
-                    node: ext.extendedType,
-                    message: "Unsupported type '\(ext.extendedType.trimmedDescription)'.",
-                    hint: "You can only extend `@JS` annotated types defined in the same module"
-                )
-            }
+    func diagnoseUnresolvedExtension(_ ext: ExtensionDeclSyntax) {
+        guard containsJSAnnotatedDeclaration(ext.memberBlock.members) else {
+            return
         }
+        diagnose(
+            node: ext.extendedType,
+            message: "Unsupported type '\(ext.extendedType.trimmedDescription)'.",
+            hint: "You can only extend `@JS` annotated types defined in the same module"
+        )
     }
 
     private func containsJSAnnotatedDeclaration(_ members: MemberBlockItemListSyntax) -> Bool {

Plugins/BridgeJS/Sources/BridgeJSLink/BridgeJSLink.swift

@@ -1689,7 +1689,7 @@ public struct BridgeJSLink {
             // Look up the enum to get its tsFullPath
             for skeleton in exportedSkeletons {
                 for enumDef in skeleton.enums {
-                    if enumDef.name == name || enumDef.swiftCallName == name {
+                    if enumDef.swiftCallName == name {
                         // Use the stored tsFullPath which has the full namespace
                         switch type {
                         case .namespaceEnum:
@@ -1707,7 +1707,7 @@ public struct BridgeJSLink {
         case .swiftStruct(let name):
             for skeleton in exportedSkeletons {
                 for structDef in skeleton.structs {
-                    if structDef.name == name || structDef.swiftCallName == name {
+                    if structDef.swiftCallName == name {
                         return structDef.tsFullPath
                     }
                 }
@@ -1716,12 +1716,14 @@ public struct BridgeJSLink {
         case .swiftHeapObject(let name):
             for skeleton in exportedSkeletons {
                 for klass in skeleton.classes {
-                    if klass.name == name || klass.swiftCallName == name {
+                    if klass.swiftCallName == name {
                         return klass.name
                     }
                 }
             }
             return type.tsType
+        case .closure(let signature, _):
+            return signature.renderTSFunctionType { resolveTypeScriptType($0, exportedSkeletons: exportedSkeletons) }
         case .alias(_, let underlying):
             return resolveTypeScriptType(underlying, exportedSkeletons: exportedSkeletons)
         case .nullable(let wrapped, let kind):
@@ -3761,7 +3763,8 @@ extension BridgeJSLink {
         let abiName = getter.abiName(context: nil)
         let funcLines = thunkBuilder.renderFunction(name: abiName)
         if getter.from == nil {
-            importObjectBuilder.appendDts(["readonly \(renderTSPropertyName(jsName)): \(getter.type.tsType);"])
+            importObjectBuilder.appendDts(["readonly \(renderTSPropertyName(jsName)): \(getter.type.tsType);"]
+            )
         }
         importObjectBuilder.assignToImportObject(name: abiName, function: funcLines)
     }
@@ -4230,6 +4233,17 @@ struct BridgeJSLinkError: Error {
     let message: String
 }
 
+extension ClosureSignature {
+    fileprivate func renderTSFunctionType(renderType: (BridgeType) -> String) -> String {
+        let renderedParameters = parameters.enumerated().map { index, parameter in
+            "arg\(index): \(renderType(parameter))"
+        }.joined(separator: ", ")
+        let renderedReturnType = renderType(returnType)
+        let returnTypeWithEffect = isAsync ? "Promise<\(renderedReturnType)>" : renderedReturnType
+        return "(\(renderedParameters)) => \(returnTypeWithEffect)"
+    }
+}
+
 extension BridgeType {
     var tsType: String {
         switch self {
@@ -4271,12 +4285,7 @@ extension BridgeType {
         case .swiftProtocol(let name):
             return name
         case .closure(let signature, _):
-            let paramTypes = signature.parameters.enumerated().map { index, param in
-                "arg\(index): \(param.tsType)"
-            }.joined(separator: ", ")
-            let returnTS =
-                signature.isAsync ? "Promise<\(signature.returnType.tsType)>" : signature.returnType.tsType
-            return "(\(paramTypes)) => \(returnTS)"
+            return signature.renderTSFunctionType { $0.tsType }
         case .array(let elementType):
             let inner = elementType.tsType
             if inner.contains("|") || inner.contains("=>") {

Plugins/BridgeJS/Tests/BridgeJSToolTests/BridgeJSCodegenTests.swift

@@ -358,6 +358,30 @@ import Testing
         try snapshotCodegen(skeleton: skeleton, name: "CrossFileNestedTypeExtension")
     }
 
+    @Test
+    func codegenCrossFileExtensionOrderIndependence() throws {
+        let swiftAPI = SwiftToSkeleton(
+            progress: .silent,
+            moduleName: "TestModule",
+            exposeToGlobal: false,
+            externalModuleIndex: .empty
+        )
+        let memberURL = Self.multifileInputsDirectory.appendingPathComponent("CrossFileExtensionOrderMember.swift")
+        swiftAPI.addSourceFile(
+            Parser.parse(source: try String(contentsOf: memberURL, encoding: .utf8)),
+            inputFilePath: "CrossFileExtensionOrderMember.swift"
+        )
+        let typeURL = Self.multifileInputsDirectory.appendingPathComponent("CrossFileExtensionOrderType.swift")
+        swiftAPI.addSourceFile(
+            Parser.parse(source: try String(contentsOf: typeURL, encoding: .utf8)),
+            inputFilePath: "CrossFileExtensionOrderType.swift"
+        )
+        let skeleton = try swiftAPI.finalize()
+        let record = skeleton.exported?.structs.first { $0.swiftCallName == "Archive.Record" }
+        #expect(record?.methods.map(\.name) == ["describeRecord"])
+        try snapshotCodegen(skeleton: skeleton, name: "CrossFileExtensionOrderIndependence")
+    }
+
     @Test
     func codegenSkipsEmptySkeletons() throws {
         let swiftAPI = SwiftToSkeleton(

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

@@ -0,0 +1,19 @@
+@JS class Depot {
+    @JS init() {}
+}
+
+extension Depot.Crate {
+    @JS func describeCrate() -> String {
+        "Crate: " + label
+    }
+}
+
+extension Depot {
+    @JS struct Crate {
+        var label: String
+
+        @JS init(label: String) {
+            self.label = label
+        }
+    }
+}

Plugins/BridgeJS/Tests/BridgeJSToolTests/Inputs/MacroSwift/Multifile/CrossFileExtensionOrderMember.swift

@@ -0,0 +1,5 @@
+extension Archive.Record {
+    @JS func describeRecord() -> String {
+        "Record: " + label
+    }
+}

Plugins/BridgeJS/Tests/BridgeJSToolTests/Inputs/MacroSwift/Multifile/CrossFileExtensionOrderType.swift

@@ -0,0 +1,13 @@
+@JS class Archive {
+    @JS init() {}
+}
+
+extension Archive {
+    @JS struct Record {
+        var label: String
+
+        @JS init(label: String) {
+            self.label = label
+        }
+    }
+}

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

@@ -7,3 +7,7 @@
 @JS func makeBench() -> Workshop.Bench {
     Workshop.Bench()
 }
+
+@JS func refitBench(_ bench: Workshop.Bench, _ transform: (Workshop.Bench) -> Workshop.Bench) -> Workshop.Bench {
+    transform(bench)
+}

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

@@ -0,0 +1,25 @@
+@JS enum Catalog {
+    @JS struct Entry {
+        var title: String
+
+        @JS init(title: String) {
+            self.title = title
+        }
+    }
+}
+
+@JS struct Entry {
+    var identifier: Int
+
+    @JS init(identifier: Int) {
+        self.identifier = identifier
+    }
+}
+
+@JS func takeEntry(_ entry: Entry) -> Entry {
+    entry
+}
+
+@JS func takeCatalogEntry(_ entry: Catalog.Entry) -> Catalog.Entry {
+    entry
+}

Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSCodegenTests/CrossFileExtensionOrderIndependence.json

@@ -0,0 +1,106 @@
+{
+  "exported" : {
+    "aliases" : [
+
+    ],
+    "classes" : [
+      {
+        "constructor" : {
+          "abiName" : "bjs_Archive_init",
+          "effects" : {
+            "isAsync" : false,
+            "isStatic" : false,
+            "isThrows" : false
+          },
+          "parameters" : [
+
+          ]
+        },
+        "methods" : [
+
+        ],
+        "name" : "Archive",
+        "properties" : [
+
+        ],
+        "swiftCallName" : "Archive"
+      }
+    ],
+    "enums" : [
+
+    ],
+    "exposeToGlobal" : false,
+    "functions" : [
+
+    ],
+    "protocols" : [
+
+    ],
+    "structs" : [
+      {
+        "constructor" : {
+          "abiName" : "bjs_Archive_Record_init",
+          "effects" : {
+            "isAsync" : false,
+            "isStatic" : false,
+            "isThrows" : false
+          },
+          "parameters" : [
+            {
+              "label" : "label",
+              "name" : "label",
+              "type" : {
+                "string" : {
+
+                }
+              }
+            }
+          ]
+        },
+        "methods" : [
+          {
+            "abiName" : "bjs_Archive_Record_describeRecord",
+            "effects" : {
+              "isAsync" : false,
+              "isStatic" : false,
+              "isThrows" : false
+            },
+            "name" : "describeRecord",
+            "parameters" : [
+
+            ],
+            "returnType" : {
+              "string" : {
+
+              }
+            }
+          }
+        ],
+        "name" : "Record",
+        "namespace" : [
+          "Archive"
+        ],
+        "properties" : [
+          {
+            "isReadonly" : true,
+            "isStatic" : false,
+            "name" : "label",
+            "namespace" : [
+              "Archive"
+            ],
+            "type" : {
+              "string" : {
+
+              }
+            }
+          }
+        ],
+        "swiftCallName" : "Archive.Record"
+      }
+    ]
+  },
+  "moduleName" : "TestModule",
+  "usedExternalModules" : [
+
+  ]
+}