Skip to content

Commit ae935a9

Browse files
wfltaylorwfltaylor
committed
Fix types in extensions bugs
1 parent 0b453a5 commit ae935a9

22 files changed

Lines changed: 1851 additions & 29 deletions

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

Lines changed: 42 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,39 @@ public final class SwiftToSkeleton {
139139
sourceFiles.append((sourceFile, inputFilePath))
140140
}
141141

142+
private func resolveDeferredExtensions(_ exportCollectors: [ExportSwiftAPICollector]) {
143+
var pendingExtensions = exportCollectors.flatMap { collector in
144+
collector.deferredExtensions.map { (owner: collector, declaration: $0) }
145+
}
146+
var previousCount: Int
147+
// An extended type might be defined in another extension, so keep resolving until no more progress is made.
148+
repeat {
149+
previousCount = pendingExtensions.count
150+
var nextPendingExtensions: [(owner: ExportSwiftAPICollector, declaration: ExtensionDeclSyntax)] = []
151+
for pending in pendingExtensions {
152+
if !resolveExtension(pending.declaration, in: exportCollectors) {
153+
nextPendingExtensions.append(pending)
154+
}
155+
}
156+
pendingExtensions = nextPendingExtensions
157+
} while pendingExtensions.count < previousCount
158+
for pending in pendingExtensions {
159+
pending.owner.diagnoseUnresolvedExtension(pending.declaration)
160+
}
161+
}
162+
163+
private func resolveExtension(
164+
_ declaration: ExtensionDeclSyntax,
165+
in exportCollectors: [ExportSwiftAPICollector]
166+
) -> Bool {
167+
for collector in exportCollectors {
168+
if collector.resolveExtension(declaration) {
169+
return true
170+
}
171+
}
172+
return false
173+
}
174+
142175
public func finalize() throws -> BridgeJSSkeleton {
143176
var perSourceErrors: [(inputFilePath: String, errors: [DiagnosticError])] = []
144177
var importedFiles: [ImportedFileSkeleton] = []
@@ -244,9 +277,7 @@ public final class SwiftToSkeleton {
244277
}
245278

246279
// Resolve extensions against all collectors. This needs to happen at this point so we can resolve both same file and cross file extensions.
247-
for source in exportCollectors {
248-
source.resolveDeferredExtensions(against: exportCollectors)
249-
}
280+
resolveDeferredExtensions(exportCollectors)
250281

251282
// We have to collect diagnostics after all deferred extensions are resolved, since they could generate some.
252283
for ((_, inputFilePath), exportCollector) in zip(sourceFiles, exportCollectors) {
@@ -1946,23 +1977,15 @@ private final class ExportSwiftAPICollector: SyntaxAnyVisitor {
19461977
return .skipChildren
19471978
}
19481979

1949-
func resolveDeferredExtensions(against collectors: [ExportSwiftAPICollector]) {
1950-
for ext in deferredExtensions {
1951-
var resolved = false
1952-
for collector in collectors {
1953-
if collector.resolveExtension(ext) {
1954-
resolved = true
1955-
break
1956-
}
1957-
}
1958-
if !resolved, containsJSAnnotatedDeclaration(ext.memberBlock.members) {
1959-
diagnose(
1960-
node: ext.extendedType,
1961-
message: "Unsupported type '\(ext.extendedType.trimmedDescription)'.",
1962-
hint: "You can only extend `@JS` annotated types defined in the same module"
1963-
)
1964-
}
1980+
func diagnoseUnresolvedExtension(_ ext: ExtensionDeclSyntax) {
1981+
guard containsJSAnnotatedDeclaration(ext.memberBlock.members) else {
1982+
return
19651983
}
1984+
diagnose(
1985+
node: ext.extendedType,
1986+
message: "Unsupported type '\(ext.extendedType.trimmedDescription)'.",
1987+
hint: "You can only extend `@JS` annotated types defined in the same module"
1988+
)
19661989
}
19671990

19681991
private func containsJSAnnotatedDeclaration(_ members: MemberBlockItemListSyntax) -> Bool {

‎Plugins/BridgeJS/Sources/BridgeJSLink/BridgeJSLink.swift‎

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1689,7 +1689,7 @@ public struct BridgeJSLink {
16891689
// Look up the enum to get its tsFullPath
16901690
for skeleton in exportedSkeletons {
16911691
for enumDef in skeleton.enums {
1692-
if enumDef.name == name || enumDef.swiftCallName == name {
1692+
if enumDef.swiftCallName == name {
16931693
// Use the stored tsFullPath which has the full namespace
16941694
switch type {
16951695
case .namespaceEnum:
@@ -1707,7 +1707,7 @@ public struct BridgeJSLink {
17071707
case .swiftStruct(let name):
17081708
for skeleton in exportedSkeletons {
17091709
for structDef in skeleton.structs {
1710-
if structDef.name == name || structDef.swiftCallName == name {
1710+
if structDef.swiftCallName == name {
17111711
return structDef.tsFullPath
17121712
}
17131713
}
@@ -1716,12 +1716,14 @@ public struct BridgeJSLink {
17161716
case .swiftHeapObject(let name):
17171717
for skeleton in exportedSkeletons {
17181718
for klass in skeleton.classes {
1719-
if klass.name == name || klass.swiftCallName == name {
1719+
if klass.swiftCallName == name {
17201720
return klass.name
17211721
}
17221722
}
17231723
}
17241724
return type.tsType
1725+
case .closure(let signature, _):
1726+
return signature.renderTSFunctionType { resolveTypeScriptType($0, exportedSkeletons: exportedSkeletons) }
17251727
case .alias(_, let underlying):
17261728
return resolveTypeScriptType(underlying, exportedSkeletons: exportedSkeletons)
17271729
case .nullable(let wrapped, let kind):
@@ -3761,7 +3763,8 @@ extension BridgeJSLink {
37613763
let abiName = getter.abiName(context: nil)
37623764
let funcLines = thunkBuilder.renderFunction(name: abiName)
37633765
if getter.from == nil {
3764-
importObjectBuilder.appendDts(["readonly \(renderTSPropertyName(jsName)): \(getter.type.tsType);"])
3766+
importObjectBuilder.appendDts(["readonly \(renderTSPropertyName(jsName)): \(getter.type.tsType);"]
3767+
)
37653768
}
37663769
importObjectBuilder.assignToImportObject(name: abiName, function: funcLines)
37673770
}
@@ -4230,6 +4233,17 @@ struct BridgeJSLinkError: Error {
42304233
let message: String
42314234
}
42324235

4236+
extension ClosureSignature {
4237+
fileprivate func renderTSFunctionType(renderType: (BridgeType) -> String) -> String {
4238+
let renderedParameters = parameters.enumerated().map { index, parameter in
4239+
"arg\(index): \(renderType(parameter))"
4240+
}.joined(separator: ", ")
4241+
let renderedReturnType = renderType(returnType)
4242+
let returnTypeWithEffect = isAsync ? "Promise<\(renderedReturnType)>" : renderedReturnType
4243+
return "(\(renderedParameters)) => \(returnTypeWithEffect)"
4244+
}
4245+
}
4246+
42334247
extension BridgeType {
42344248
var tsType: String {
42354249
switch self {
@@ -4271,12 +4285,7 @@ extension BridgeType {
42714285
case .swiftProtocol(let name):
42724286
return name
42734287
case .closure(let signature, _):
4274-
let paramTypes = signature.parameters.enumerated().map { index, param in
4275-
"arg\(index): \(param.tsType)"
4276-
}.joined(separator: ", ")
4277-
let returnTS =
4278-
signature.isAsync ? "Promise<\(signature.returnType.tsType)>" : signature.returnType.tsType
4279-
return "(\(paramTypes)) => \(returnTS)"
4288+
return signature.renderTSFunctionType { $0.tsType }
42804289
case .array(let elementType):
42814290
let inner = elementType.tsType
42824291
if inner.contains("|") || inner.contains("=>") {

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,30 @@ import Testing
358358
try snapshotCodegen(skeleton: skeleton, name: "CrossFileNestedTypeExtension")
359359
}
360360

361+
@Test
362+
func codegenCrossFileExtensionOrderIndependence() throws {
363+
let swiftAPI = SwiftToSkeleton(
364+
progress: .silent,
365+
moduleName: "TestModule",
366+
exposeToGlobal: false,
367+
externalModuleIndex: .empty
368+
)
369+
let memberURL = Self.multifileInputsDirectory.appendingPathComponent("CrossFileExtensionOrderMember.swift")
370+
swiftAPI.addSourceFile(
371+
Parser.parse(source: try String(contentsOf: memberURL, encoding: .utf8)),
372+
inputFilePath: "CrossFileExtensionOrderMember.swift"
373+
)
374+
let typeURL = Self.multifileInputsDirectory.appendingPathComponent("CrossFileExtensionOrderType.swift")
375+
swiftAPI.addSourceFile(
376+
Parser.parse(source: try String(contentsOf: typeURL, encoding: .utf8)),
377+
inputFilePath: "CrossFileExtensionOrderType.swift"
378+
)
379+
let skeleton = try swiftAPI.finalize()
380+
let record = skeleton.exported?.structs.first { $0.swiftCallName == "Archive.Record" }
381+
#expect(record?.methods.map(\.name) == ["describeRecord"])
382+
try snapshotCodegen(skeleton: skeleton, name: "CrossFileExtensionOrderIndependence")
383+
}
384+
361385
@Test
362386
func codegenSkipsEmptySkeletons() throws {
363387
let swiftAPI = SwiftToSkeleton(

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
@JS class Depot {
2+
@JS init() {}
3+
}
4+
5+
extension Depot.Crate {
6+
@JS func describeCrate() -> String {
7+
"Crate: " + label
8+
}
9+
}
10+
11+
extension Depot {
12+
@JS struct Crate {
13+
var label: String
14+
15+
@JS init(label: String) {
16+
self.label = label
17+
}
18+
}
19+
}

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
extension Archive.Record {
2+
@JS func describeRecord() -> String {
3+
"Record: " + label
4+
}
5+
}

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
@JS class Archive {
2+
@JS init() {}
3+
}
4+
5+
extension Archive {
6+
@JS struct Record {
7+
var label: String
8+
9+
@JS init(label: String) {
10+
self.label = label
11+
}
12+
}
13+
}

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,7 @@
77
@JS func makeBench() -> Workshop.Bench {
88
Workshop.Bench()
99
}
10+
11+
@JS func refitBench(_ bench: Workshop.Bench, _ transform: (Workshop.Bench) -> Workshop.Bench) -> Workshop.Bench {
12+
transform(bench)
13+
}

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
@JS enum Catalog {
2+
@JS struct Entry {
3+
var title: String
4+
5+
@JS init(title: String) {
6+
self.title = title
7+
}
8+
}
9+
}
10+
11+
@JS struct Entry {
12+
var identifier: Int
13+
14+
@JS init(identifier: Int) {
15+
self.identifier = identifier
16+
}
17+
}
18+
19+
@JS func takeEntry(_ entry: Entry) -> Entry {
20+
entry
21+
}
22+
23+
@JS func takeCatalogEntry(_ entry: Catalog.Entry) -> Catalog.Entry {
24+
entry
25+
}

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

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
{
2+
"exported" : {
3+
"aliases" : [
4+
5+
],
6+
"classes" : [
7+
{
8+
"constructor" : {
9+
"abiName" : "bjs_Archive_init",
10+
"effects" : {
11+
"isAsync" : false,
12+
"isStatic" : false,
13+
"isThrows" : false
14+
},
15+
"parameters" : [
16+
17+
]
18+
},
19+
"methods" : [
20+
21+
],
22+
"name" : "Archive",
23+
"properties" : [
24+
25+
],
26+
"swiftCallName" : "Archive"
27+
}
28+
],
29+
"enums" : [
30+
31+
],
32+
"exposeToGlobal" : false,
33+
"functions" : [
34+
35+
],
36+
"protocols" : [
37+
38+
],
39+
"structs" : [
40+
{
41+
"constructor" : {
42+
"abiName" : "bjs_Archive_Record_init",
43+
"effects" : {
44+
"isAsync" : false,
45+
"isStatic" : false,
46+
"isThrows" : false
47+
},
48+
"parameters" : [
49+
{
50+
"label" : "label",
51+
"name" : "label",
52+
"type" : {
53+
"string" : {
54+
55+
}
56+
}
57+
}
58+
]
59+
},
60+
"methods" : [
61+
{
62+
"abiName" : "bjs_Archive_Record_describeRecord",
63+
"effects" : {
64+
"isAsync" : false,
65+
"isStatic" : false,
66+
"isThrows" : false
67+
},
68+
"name" : "describeRecord",
69+
"parameters" : [
70+
71+
],
72+
"returnType" : {
73+
"string" : {
74+
75+
}
76+
}
77+
}
78+
],
79+
"name" : "Record",
80+
"namespace" : [
81+
"Archive"
82+
],
83+
"properties" : [
84+
{
85+
"isReadonly" : true,
86+
"isStatic" : false,
87+
"name" : "label",
88+
"namespace" : [
89+
"Archive"
90+
],
91+
"type" : {
92+
"string" : {
93+
94+
}
95+
}
96+
}
97+
],
98+
"swiftCallName" : "Archive.Record"
99+
}
100+
]
101+
},
102+
"moduleName" : "TestModule",
103+
"usedExternalModules" : [
104+
105+
]
106+
}

0 commit comments

Comments
 (0)