Skip to content

Commit 4af1984

Browse files
kateinoigakukunkateinoigakukun
authored
BridgeJS: Fix macro test suites silently ignoring failures (#559)
* BridgeJS: Fix macro test suites silently ignoring failures * BridgeJS: Fix macro unit tests and macro implementations to pass tests
1 parent b0bf4a3 commit 4af1984

File tree

10 files changed

+347
-195
lines changed

10 files changed

+347
-195
lines changed

‎Plugins/BridgeJS/Package.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ let package = Package(
7272
name: "BridgeJSMacrosTests",
7373
dependencies: [
7474
"BridgeJSMacros",
75-
.product(name: "SwiftSyntaxMacrosTestSupport", package: "swift-syntax"),
75+
.product(name: "SwiftSyntaxMacrosGenericTestSupport", package: "swift-syntax"),
7676
]
7777
),
7878

‎Plugins/BridgeJS/Sources/BridgeJSMacros/JSClassMacro.swift‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,9 +66,16 @@ extension JSClassMacro: ExtensionMacro {
6666
conformingTo protocols: [TypeSyntax],
6767
in context: some MacroExpansionContext
6868
) throws -> [ExtensionDeclSyntax] {
69-
guard declaration.is(StructDeclSyntax.self) else { return [] }
69+
guard let structDecl = declaration.as(StructDeclSyntax.self) else { return [] }
7070
guard !protocols.isEmpty else { return [] }
7171

72+
// Do not add extension if the struct already conforms to _JSBridgedClass
73+
if let clause = structDecl.inheritanceClause,
74+
clause.inheritedTypes.contains(where: { $0.type.trimmed.description == "_JSBridgedClass" })
75+
{
76+
return []
77+
}
78+
7279
let conformanceList = protocols.map { $0.trimmed.description }.joined(separator: ", ")
7380
return [
7481
try ExtensionDeclSyntax("extension \(type.trimmed): \(raw: conformanceList) {}")

‎Plugins/BridgeJS/Sources/BridgeJSMacros/JSFunctionMacro.swift‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ extension JSFunctionMacro: BodyMacro {
4747
context.diagnose(
4848
Diagnostic(node: Syntax(declaration), message: JSMacroMessage.unsupportedDeclaration)
4949
)
50-
return []
50+
return [CodeBlockItemSyntax(stringLiteral: "fatalError(\"@JSFunction init must be inside a type\")")]
5151
}
5252

5353
let glueName = JSMacroHelper.glueName(baseName: "init", enclosingTypeName: enclosingTypeName)
@@ -70,3 +70,19 @@ extension JSFunctionMacro: BodyMacro {
7070
return []
7171
}
7272
}
73+
74+
extension JSFunctionMacro: PeerMacro {
75+
/// Emits a diagnostic when @JSFunction is applied to a declaration that is not a function or initializer.
76+
/// BodyMacro is only invoked for declarations with optional code blocks (e.g. functions, initializers),
77+
/// so for vars and other decls we need PeerMacro to run and diagnose.
78+
public static func expansion(
79+
of node: AttributeSyntax,
80+
providingPeersOf declaration: some DeclSyntaxProtocol,
81+
in context: some MacroExpansionContext
82+
) throws -> [DeclSyntax] {
83+
if declaration.is(FunctionDeclSyntax.self) { return [] }
84+
if declaration.is(InitializerDeclSyntax.self) { return [] }
85+
context.diagnose(Diagnostic(node: Syntax(declaration), message: JSMacroMessage.unsupportedDeclaration))
86+
return []
87+
}
88+
}

‎Plugins/BridgeJS/Sources/BridgeJSMacros/JSGetterMacro.swift‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,3 +60,20 @@ extension JSGetterMacro: AccessorMacro {
6060
]
6161
}
6262
}
63+
64+
extension JSGetterMacro: PeerMacro {
65+
/// Emits a diagnostic when @JSGetter is applied to a declaration that is not a variable (e.g. a function).
66+
/// AccessorMacro may not be invoked for non-property declarations. For variables with multiple
67+
/// bindings, the compiler emits its own diagnostic; we only diagnose non-variable decls here.
68+
public static func expansion(
69+
of node: AttributeSyntax,
70+
providingPeersOf declaration: some DeclSyntaxProtocol,
71+
in context: some MacroExpansionContext
72+
) throws -> [DeclSyntax] {
73+
guard declaration.is(VariableDeclSyntax.self) else {
74+
context.diagnose(Diagnostic(node: Syntax(declaration), message: JSMacroMessage.unsupportedVariable))
75+
return []
76+
}
77+
return []
78+
}
79+
}

‎Plugins/BridgeJS/Sources/BridgeJSMacros/JSSetterMacro.swift‎

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,23 @@ extension JSSetterMacro: BodyMacro {
2525
let rawFunctionName = JSMacroHelper.stripBackticks(functionName)
2626
guard rawFunctionName.hasPrefix("set"), rawFunctionName.count > 3 else {
2727
context.diagnose(Diagnostic(node: Syntax(declaration), message: JSMacroMessage.invalidSetterName))
28-
return []
28+
return [
29+
CodeBlockItemSyntax(
30+
stringLiteral:
31+
"fatalError(\"@JSSetter function name must start with 'set' followed by a property name\")"
32+
)
33+
]
2934
}
3035

3136
let propertyName = String(rawFunctionName.dropFirst(3))
3237
guard !propertyName.isEmpty else {
3338
context.diagnose(Diagnostic(node: Syntax(declaration), message: JSMacroMessage.invalidSetterName))
34-
return []
39+
return [
40+
CodeBlockItemSyntax(
41+
stringLiteral:
42+
"fatalError(\"@JSSetter function name must start with 'set' followed by a property name\")"
43+
)
44+
]
3545
}
3646

3747
// Convert first character to lowercase (e.g., "Foo" -> "foo")
@@ -56,7 +66,11 @@ extension JSSetterMacro: BodyMacro {
5666
let parameters = functionDecl.signature.parameterClause.parameters
5767
guard let firstParam = parameters.first else {
5868
context.diagnose(Diagnostic(node: Syntax(declaration), message: JSMacroMessage.setterRequiresParameter))
59-
return []
69+
return [
70+
CodeBlockItemSyntax(
71+
stringLiteral: "fatalError(\"@JSSetter function must have at least one parameter\")"
72+
)
73+
]
6074
}
6175

6276
let paramName = firstParam.secondName ?? firstParam.firstName
@@ -69,3 +83,21 @@ extension JSSetterMacro: BodyMacro {
6983
return [CodeBlockItemSyntax(stringLiteral: "try \(call)")]
7084
}
7185
}
86+
87+
extension JSSetterMacro: PeerMacro {
88+
/// Emits a diagnostic when @JSSetter is applied to a declaration that is not a function.
89+
/// BodyMacro is only invoked for declarations with optional code blocks (e.g. functions).
90+
public static func expansion(
91+
of node: AttributeSyntax,
92+
providingPeersOf declaration: some DeclSyntaxProtocol,
93+
in context: some MacroExpansionContext
94+
) throws -> [DeclSyntax] {
95+
guard declaration.is(FunctionDeclSyntax.self) else {
96+
context.diagnose(
97+
Diagnostic(node: Syntax(declaration), message: JSMacroMessage.unsupportedSetterDeclaration)
98+
)
99+
return []
100+
}
101+
return []
102+
}
103+
}

‎Plugins/BridgeJS/Tests/BridgeJSMacrosTests/JSClassMacroTests.swift‎

Lines changed: 28 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
import SwiftDiagnostics
22
import SwiftSyntax
33
import SwiftSyntaxMacroExpansion
4-
import SwiftSyntaxMacros
5-
import SwiftSyntaxMacrosTestSupport
4+
import SwiftSyntaxMacrosGenericTestSupport
65
import Testing
76
import BridgeJSMacros
87

@@ -13,14 +12,15 @@ import BridgeJSMacros
1312
]
1413

1514
@Test func emptyStruct() {
16-
assertMacroExpansion(
15+
TestSupport.assertMacroExpansion(
1716
"""
1817
@JSClass
1918
struct MyClass {
2019
}
2120
""",
2221
expandedSource: """
2322
struct MyClass {
23+
2424
let jsObject: JSObject
2525
2626
init(unsafelyWrapping jsObject: JSObject) {
@@ -32,12 +32,12 @@ import BridgeJSMacros
3232
}
3333
""",
3434
macroSpecs: macroSpecs,
35-
indentationWidth: indentationWidth
35+
indentationWidth: indentationWidth,
3636
)
3737
}
3838

3939
@Test func structWithExistingJSObject() {
40-
assertMacroExpansion(
40+
TestSupport.assertMacroExpansion(
4141
"""
4242
@JSClass
4343
struct MyClass {
@@ -62,7 +62,7 @@ import BridgeJSMacros
6262
}
6363

6464
@Test func structWithExistingInit() {
65-
assertMacroExpansion(
65+
TestSupport.assertMacroExpansion(
6666
"""
6767
@JSClass
6868
struct MyClass {
@@ -73,11 +73,11 @@ import BridgeJSMacros
7373
""",
7474
expandedSource: """
7575
struct MyClass {
76-
let jsObject: JSObject
77-
7876
init(unsafelyWrapping jsObject: JSObject) {
7977
self.jsObject = jsObject
8078
}
79+
80+
let jsObject: JSObject
8181
}
8282
8383
extension MyClass: _JSBridgedClass {
@@ -89,7 +89,7 @@ import BridgeJSMacros
8989
}
9090

9191
@Test func structWithBothExisting() {
92-
assertMacroExpansion(
92+
TestSupport.assertMacroExpansion(
9393
"""
9494
@JSClass
9595
struct MyClass {
@@ -118,7 +118,7 @@ import BridgeJSMacros
118118
}
119119

120120
@Test func structWithMembers() {
121-
assertMacroExpansion(
121+
TestSupport.assertMacroExpansion(
122122
"""
123123
@JSClass
124124
struct MyClass {
@@ -127,10 +127,10 @@ import BridgeJSMacros
127127
""",
128128
expandedSource: """
129129
struct MyClass {
130-
let jsObject: JSObject
131-
132130
var name: String
133131
132+
let jsObject: JSObject
133+
134134
init(unsafelyWrapping jsObject: JSObject) {
135135
self.jsObject = jsObject
136136
}
@@ -145,7 +145,7 @@ import BridgeJSMacros
145145
}
146146

147147
@Test func _class() {
148-
assertMacroExpansion(
148+
TestSupport.assertMacroExpansion(
149149
"""
150150
@JSClass
151151
class MyClass {
@@ -168,7 +168,7 @@ import BridgeJSMacros
168168
}
169169

170170
@Test func _enum() {
171-
assertMacroExpansion(
171+
TestSupport.assertMacroExpansion(
172172
"""
173173
@JSClass
174174
enum MyEnum {
@@ -191,7 +191,7 @@ import BridgeJSMacros
191191
}
192192

193193
@Test func _actor() {
194-
assertMacroExpansion(
194+
TestSupport.assertMacroExpansion(
195195
"""
196196
@JSClass
197197
actor MyActor {
@@ -214,7 +214,7 @@ import BridgeJSMacros
214214
}
215215

216216
@Test func structWithDifferentJSObjectName() {
217-
assertMacroExpansion(
217+
TestSupport.assertMacroExpansion(
218218
"""
219219
@JSClass
220220
struct MyClass {
@@ -223,10 +223,10 @@ import BridgeJSMacros
223223
""",
224224
expandedSource: """
225225
struct MyClass {
226-
let jsObject: JSObject
227-
228226
var otherProperty: String
229227
228+
let jsObject: JSObject
229+
230230
init(unsafelyWrapping jsObject: JSObject) {
231231
self.jsObject = jsObject
232232
}
@@ -241,7 +241,7 @@ import BridgeJSMacros
241241
}
242242

243243
@Test func structWithDifferentInit() {
244-
assertMacroExpansion(
244+
TestSupport.assertMacroExpansion(
245245
"""
246246
@JSClass
247247
struct MyClass {
@@ -251,11 +251,11 @@ import BridgeJSMacros
251251
""",
252252
expandedSource: """
253253
struct MyClass {
254-
let jsObject: JSObject
255-
256254
init(name: String) {
257255
}
258256
257+
let jsObject: JSObject
258+
259259
init(unsafelyWrapping jsObject: JSObject) {
260260
self.jsObject = jsObject
261261
}
@@ -270,7 +270,7 @@ import BridgeJSMacros
270270
}
271271

272272
@Test func structWithMultipleMembers() {
273-
assertMacroExpansion(
273+
TestSupport.assertMacroExpansion(
274274
"""
275275
@JSClass
276276
struct MyClass {
@@ -280,11 +280,11 @@ import BridgeJSMacros
280280
""",
281281
expandedSource: """
282282
struct MyClass {
283-
let jsObject: JSObject
284-
285283
var name: String
286284
var age: Int
287285
286+
let jsObject: JSObject
287+
288288
init(unsafelyWrapping jsObject: JSObject) {
289289
self.jsObject = jsObject
290290
}
@@ -299,7 +299,7 @@ import BridgeJSMacros
299299
}
300300

301301
@Test func structWithComment() {
302-
assertMacroExpansion(
302+
TestSupport.assertMacroExpansion(
303303
"""
304304
/// Documentation comment
305305
@JSClass
@@ -309,6 +309,7 @@ import BridgeJSMacros
309309
expandedSource: """
310310
/// Documentation comment
311311
struct MyClass {
312+
312313
let jsObject: JSObject
313314
314315
init(unsafelyWrapping jsObject: JSObject) {
@@ -325,14 +326,15 @@ import BridgeJSMacros
325326
}
326327

327328
@Test func structAlreadyConforms() {
328-
assertMacroExpansion(
329+
TestSupport.assertMacroExpansion(
329330
"""
330331
@JSClass
331332
struct MyClass: _JSBridgedClass {
332333
}
333334
""",
334335
expandedSource: """
335336
struct MyClass: _JSBridgedClass {
337+
336338
let jsObject: JSObject
337339
338340
init(unsafelyWrapping jsObject: JSObject) {

0 commit comments

Comments
 (0)