Skip to content

Commit 5d086b6

Browse files
tsapetatsapeta
authored
Normalize implicitly-unwrapped optional types before splicing into expressions (#23)
1 parent 5cf4aad commit 5d086b6

5 files changed

Lines changed: 178 additions & 7 deletions

File tree

‎apple/Sources/ExpoModulesMacros/DecorateModuleBuilder.swift‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,8 @@ internal struct JSFunction {
127127
if let accessor = fastDecodeAccessor(for: type) {
128128
return "let arg\(index) = try arguments.unownedValue(at: \(index)).\(accessor)()"
129129
}
130-
return "let arg\(index) = try \(type).getDynamicType().cast(jsValue: arguments[\(index)], appContext: appContext) as! \(type)"
130+
let exprType = expressionType(type)
131+
return "let arg\(index) = try \(exprType).getDynamicType().cast(jsValue: arguments[\(index)], appContext: appContext) as! \(exprType)"
131132
}
132133

133134
/// The `self.<name>(...)` call for the given arity. Slots `0..<arity` are passed their decoded
@@ -184,7 +185,7 @@ internal struct JSFunction {
184185
if fastDecodeAccessor(for: returnType) != nil {
185186
return ["return result.toJavaScriptValue(in: runtime)"]
186187
}
187-
return ["return try \(returnType).getDynamicType().castToJS(result, appContext: appContext, in: runtime)"]
188+
return ["return try \(expressionType(returnType)).getDynamicType().castToJS(result, appContext: appContext, in: runtime)"]
188189
}
189190

190191
/// The `setProperty` statement that installs this function on the JS object. The decode-call-encode
@@ -295,7 +296,7 @@ internal struct JSProperty {
295296
getEncode = "return self.\(swiftName).toJavaScriptValue(in: runtime)"
296297
} else if let valueType {
297298
getEncode =
298-
"return try \(valueType).getDynamicType().castToJS(self.\(swiftName), appContext: appContext, in: runtime)"
299+
"return try \(expressionType(valueType)).getDynamicType().castToJS(self.\(swiftName), appContext: appContext, in: runtime)"
299300
} else {
300301
// No known type: fall back to converting whatever `self.<name>` is. This only happens when the
301302
// declaration has neither an annotation nor a literal default, which is rare for a stored var.
@@ -311,8 +312,9 @@ internal struct JSProperty {
311312
if let accessor = fastDecodeAccessor(for: valueType) {
312313
setDecode = "self.\(swiftName) = try arguments.unownedValue(at: 0).\(accessor)()"
313314
} else {
315+
let exprType = expressionType(valueType)
314316
setDecode =
315-
"self.\(swiftName) = try \(valueType).getDynamicType().cast(jsValue: arguments[0], appContext: appContext) as! \(valueType)"
317+
"self.\(swiftName) = try \(exprType).getDynamicType().cast(jsValue: arguments[0], appContext: appContext) as! \(exprType)"
316318
}
317319
lines.append(
318320
accessorClosure(descriptorName, "set", usesAppContext: usesAppContext, body: "\(setDecode)\nreturn .undefined"))

‎apple/Sources/ExpoModulesMacros/MacroHelpers.swift‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,3 +245,14 @@ extension AttributeListSyntax {
245245
return nil
246246
}
247247
}
248+
249+
/// A type spelled so it's valid in expression position (before `.getDynamicType()` or after `as!`).
250+
/// Implicitly-unwrapped optionals (`T!`) are only allowed in type-annotation position, so a trailing
251+
/// `!` is rewritten to `?` (`T!` and `T?` are both `Optional<T>`, which the dynamic-type / cast layer
252+
/// treats identically). Other type spellings pass through unchanged.
253+
internal func expressionType(_ type: String) -> String {
254+
guard type.hasSuffix("!") else {
255+
return type
256+
}
257+
return type.dropLast() + "?"
258+
}

‎apple/Sources/ExpoModulesMacros/RecordMacro.swift‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,8 @@ private func jsObjectReadLines(properties: [RecordProperty]) -> [String] {
316316
var lines: [String] = []
317317
for property in properties {
318318
let valueVar = "\(property.name)JSValue"
319-
let cast = "try \(property.type).getDynamicType().cast(jsValue: \(valueVar), appContext: appContext) as! \(property.type)"
319+
let exprType = expressionType(property.type)
320+
let cast = "try \(exprType).getDynamicType().cast(jsValue: \(valueVar), appContext: appContext) as! \(exprType)"
320321
lines.append(" let \(valueVar) = object.getProperty(\"\(property.name)\")")
321322
if property.isRequired {
322323
lines.append(" guard !\(valueVar).isUndefined() else {")
@@ -337,7 +338,8 @@ private func dictionaryReadLines(properties: [RecordProperty]) -> [String] {
337338
var lines: [String] = []
338339
for property in properties {
339340
let valueVar = "\(property.name)Value"
340-
let cast = "try \(property.type).getDynamicType().cast(\(valueVar), appContext: appContext) as! \(property.type)"
341+
let exprType = expressionType(property.type)
342+
let cast = "try \(exprType).getDynamicType().cast(\(valueVar), appContext: appContext) as! \(exprType)"
341343
lines.append(" let \(valueVar) = dictionary[\"\(property.name)\"]")
342344
if property.isRequired {
343345
lines.append(" guard let \(valueVar) else {")
@@ -399,7 +401,7 @@ private func toObjectMethod(properties: [RecordProperty], inheritsRecord: Bool)
399401
lines.append(" let object = try appContext.runtime.createObject()")
400402
}
401403
for property in properties {
402-
lines.append(" object.setProperty(\"\(property.name)\", value: try \(property.type).getDynamicType().convertToJS(self.\(property.name), appContext: appContext))")
404+
lines.append(" object.setProperty(\"\(property.name)\", value: try \(expressionType(property.type)).getDynamicType().convertToJS(self.\(property.name), appContext: appContext))")
403405
}
404406
lines.append(" return object")
405407
let body = lines.joined(separator: "\n")

‎apple/Tests/ExpoModulesMacrosTests/ExpoModuleMacroTests.swift‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,52 @@ struct ExpoModuleMacroTests {
335335
)
336336
}
337337

338+
@Test
339+
func `Implicitly-unwrapped optional return normalizes to optional in the cast expression`() {
340+
assertExpansion(
341+
"""
342+
@ExpoModule
343+
final class MyModule: Module {
344+
@JS
345+
func make(count: Int) -> MyRecord! { nil }
346+
}
347+
""",
348+
expandedSource: """
349+
final class MyModule: Module {
350+
@JavaScriptActor
351+
func make(count: Int) -> MyRecord! { nil }
352+
353+
private func _assertTypesConformance_make() {
354+
func make<T: AnyArgument>(_: T.Type) {
355+
}
356+
make(MyRecord.self)
357+
}
358+
359+
public static let _jsName = "MyModule"
360+
361+
public func _synthesizedDefinition() -> [AnyDefinition] {
362+
return []
363+
}
364+
365+
@JavaScriptActor
366+
public func _decorateModule(object: borrowing JavaScriptObject, in runtime: JavaScriptRuntime, appContext: AppContext) throws {
367+
object.setProperty("make") { [weak appContext, self] (this: borrowing JavaScriptUnownedValue, arguments: consuming JavaScriptValuesBuffer) in
368+
guard let appContext else {
369+
throw Exceptions.AppContextLost()
370+
}
371+
guard arguments.count == 1 else {
372+
throw Exceptions.ArgumentsRangeMismatch((functionName: "make", received: arguments.count, required: 1, maximum: 1))
373+
}
374+
let arg0 = try arguments.unownedValue(at: 0).asInt()
375+
let result = self.make(count: arg0)
376+
return try MyRecord?.getDynamicType().castToJS(result, appContext: appContext, in: runtime)
377+
}
378+
}
379+
}
380+
"""
381+
)
382+
}
383+
338384
@Test
339385
func `Static function emits a static conformance-assertion peer`() {
340386
assertExpansion(
@@ -613,6 +659,57 @@ struct ExpoModuleMacroTests {
613659
)
614660
}
615661
662+
@Test
663+
func `Implicitly-unwrapped optional property normalizes the type to optional in the cast expressions`() {
664+
assertExpansion(
665+
"""
666+
@ExpoModule
667+
final class MyModule: Module {
668+
@JS
669+
var config: MyRecord!
670+
}
671+
""",
672+
expandedSource: """
673+
final class MyModule: Module {
674+
@JavaScriptActor
675+
var config: MyRecord!
676+
677+
private func _assertTypesConformance_config() {
678+
func config<T: AnyArgument>(_: T.Type) {
679+
}
680+
config(MyRecord.self)
681+
}
682+
683+
public static let _jsName = "MyModule"
684+
685+
public func _synthesizedDefinition() -> [AnyDefinition] {
686+
return []
687+
}
688+
689+
@JavaScriptActor
690+
public func _decorateModule(object: borrowing JavaScriptObject, in runtime: JavaScriptRuntime, appContext: AppContext) throws {
691+
let configDescriptor = runtime.createObject()
692+
configDescriptor.setProperty("enumerable", value: true)
693+
configDescriptor.setProperty("get") { [weak appContext, self] (this: borrowing JavaScriptUnownedValue, arguments: consuming JavaScriptValuesBuffer) in
694+
guard let appContext else {
695+
throw Exceptions.AppContextLost()
696+
}
697+
return try MyRecord?.getDynamicType().castToJS(self.config, appContext: appContext, in: runtime)
698+
}
699+
configDescriptor.setProperty("set") { [weak appContext, self] (this: borrowing JavaScriptUnownedValue, arguments: consuming JavaScriptValuesBuffer) in
700+
guard let appContext else {
701+
throw Exceptions.AppContextLost()
702+
}
703+
self.config = try MyRecord?.getDynamicType().cast(jsValue: arguments[0], appContext: appContext) as! MyRecord?
704+
return .undefined
705+
}
706+
object.defineProperty("config", descriptor: configDescriptor)
707+
}
708+
}
709+
"""
710+
)
711+
}
712+
616713
@Test
617714
func `Mixed members: only @JS-marked ones are picked up`() {
618715
assertExpansion(

‎apple/Tests/ExpoModulesMacrosTests/RecordMacroTests.swift‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,65 @@ struct RecordMacroTests {
115115
)
116116
}
117117

118+
@Test
119+
func `Implicitly-unwrapped optional property normalizes to optional in cast and convert expressions`() {
120+
assertExpansion(
121+
"""
122+
@Record
123+
struct Options {
124+
var owner: MyRecord!
125+
}
126+
""",
127+
expandedSource: """
128+
struct Options {
129+
var owner: MyRecord!
130+
131+
private func _assertTypesConformance() {
132+
func owner<T: AnyArgument>(_: T.Type) {
133+
}
134+
owner(MyRecord.self)
135+
}
136+
137+
public init() {
138+
}
139+
140+
public init(owner: MyRecord! = nil) {
141+
self.owner = owner
142+
}
143+
144+
@JavaScriptActor
145+
public static func from(object: borrowing JavaScriptObject, appContext: AppContext) throws -> Self {
146+
let ownerJSValue = object.getProperty("owner")
147+
let owner: MyRecord! = (ownerJSValue.isUndefined() || ownerJSValue.isNull()) ? nil : try MyRecord?.getDynamicType().cast(jsValue: ownerJSValue, appContext: appContext) as! MyRecord?
148+
return Self(owner: owner)
149+
}
150+
151+
public static func from(dictionary: [String: Any], appContext: AppContext) throws -> Self {
152+
let ownerValue = dictionary["owner"]
153+
let owner: MyRecord! = (ownerValue == nil || ownerValue! is NSNull) ? nil : try MyRecord?.getDynamicType().cast(ownerValue, appContext: appContext) as! MyRecord?
154+
return Self(owner: owner)
155+
}
156+
157+
public func toDictionary(appContext: AppContext? = nil) -> [String: Any] {
158+
var dictionary: [String: Any] = [:]
159+
dictionary["owner"] = self.owner
160+
return dictionary
161+
}
162+
163+
@JavaScriptActor
164+
public func toObject(appContext: AppContext) throws -> JavaScriptObject {
165+
let object = try appContext.runtime.createObject()
166+
object.setProperty("owner", value: try MyRecord?.getDynamicType().convertToJS(self.owner, appContext: appContext))
167+
return object
168+
}
169+
}
170+
171+
extension Options: Record {
172+
}
173+
"""
174+
)
175+
}
176+
118177
@Test
119178
func `Non-primitive properties are checked in a single conformance-assertion peer`() {
120179
assertExpansion(

0 commit comments

Comments
 (0)