-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[pigeon][swift] Removes FlutterError in favor of PigeonError #6611
Changes from 19 commits
c510bd6
d8968d7
c2beea1
6a8a94c
b874f09
df102e3
b30a0cc
30ac050
92c97af
86d9168
20ac87a
800ba60
8fa6b75
4bd3dbc
fae49ea
b386a96
78c3bb6
f13a06a
63df915
428736d
88cd27d
a78cae7
4ed7852
a8ab4f8
877015d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -115,27 +115,25 @@ Future<bool> sendMessage(String messageText) { | |
### Swift | ||
|
||
This is the code that will use the generated Swift code to receive calls from Flutter. | ||
packages/pigeon/example/app/ios/Runner/AppDelegate.swift | ||
Unlike other languages, when throwing an error (both synchronous and asynchronous), use `PigeonError` instead of `FlutterError`, as `FlutterError` is not conforming to `Swift.Error`. | ||
Previously, a workaround was to declare an extension to `FlutterError` to conform to `Swift.Error`, but, as of Pigeon 19.0.0, `PigeonError` is provided for this purpose. Older code still using `FlutterError` will continue to work, but it is recommended to switch to `PigeonError` and remove the extension. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I don't think the second line is necessary. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
<?code-excerpt "ios/Runner/AppDelegate.swift (swift-class)"?> | ||
```swift | ||
// This extension of Error is required to do use FlutterError in any Swift code. | ||
extension FlutterError: Error {} | ||
|
||
private class PigeonApiImplementation: ExampleHostApi { | ||
func getHostLanguage() throws -> String { | ||
return "Swift" | ||
} | ||
|
||
func add(_ a: Int64, to b: Int64) throws -> Int64 { | ||
if a < 0 || b < 0 { | ||
throw FlutterError(code: "code", message: "message", details: "details") | ||
throw PigeonError(code: "code", message: "message", details: "details") | ||
} | ||
return a + b | ||
} | ||
|
||
func sendMessage(message: MessageData, completion: @escaping (Result<Bool, Error>) -> Void) { | ||
if message.code == Code.one { | ||
completion(.failure(FlutterError(code: "code", message: "message", details: "details"))) | ||
completion(.failure(PigeonError(code: "code", message: "message", details: "details"))) | ||
return | ||
} | ||
completion(.success(true)) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,16 +19,21 @@ class SwiftOptions { | |
/// Creates a [SwiftOptions] object | ||
const SwiftOptions({ | ||
this.copyrightHeader, | ||
this.errorClassName, | ||
}); | ||
|
||
/// A copyright header that will get prepended to generated code. | ||
final Iterable<String>? copyrightHeader; | ||
|
||
/// The name of the error class used for passing custom error parameters. | ||
final String? errorClassName; | ||
|
||
/// Creates a [SwiftOptions] from a Map representation where: | ||
/// `x = SwiftOptions.fromList(x.toMap())`. | ||
static SwiftOptions fromList(Map<String, Object> map) { | ||
return SwiftOptions( | ||
copyrightHeader: map['copyrightHeader'] as Iterable<String>?, | ||
errorClassName: map['errorClassName'] as String?, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is it open for developers to use a custom class name? why don't we just use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Generally, I believe calling pigeon once for each package (plugin, app, etc.) should suffice. In these scenarios, there's no need to concern ourselves with the generated class name. However, in this repo, pigeon is invoked multiple times for the same package, which results in multiple classes with the same name, ultimately causing a compilation error. Kotlin encounters a similar problem but has a solution for it. This Link provides a background of the issue as well as its resolution. I have therefore applied the same solution to the generation of Swift code. I believe overriding the error class name as an advanced usage, so I prefer not to integrate it with the command line option, similar to the approach taken by Kotlin. |
||
); | ||
} | ||
|
||
|
@@ -37,6 +42,7 @@ class SwiftOptions { | |
Map<String, Object> toMap() { | ||
final Map<String, Object> result = <String, Object>{ | ||
if (copyrightHeader != null) 'copyrightHeader': copyrightHeader!, | ||
if (errorClassName != null) 'errorClassName': errorClassName!, | ||
}; | ||
return result; | ||
} | ||
|
@@ -316,7 +322,7 @@ class SwiftGenerator extends StructuredGenerator<SwiftOptions> { | |
name: func.name, | ||
parameters: func.parameters, | ||
returnType: func.returnType, | ||
errorTypeName: 'FlutterError', | ||
errorTypeName: _getErrorClassName(generatorOptions), | ||
isAsynchronous: true, | ||
swiftFunction: func.swiftFunction, | ||
getParameterName: _getSafeArgumentName, | ||
|
@@ -350,6 +356,7 @@ class SwiftGenerator extends StructuredGenerator<SwiftOptions> { | |
indent, func.documentationComments, _docCommentSpec); | ||
_writeFlutterMethod( | ||
indent, | ||
generatorOptions: generatorOptions, | ||
name: func.name, | ||
channelName: makeChannelName(api, func, dartPackageName), | ||
parameters: func.parameters, | ||
|
@@ -614,7 +621,7 @@ class SwiftGenerator extends StructuredGenerator<SwiftOptions> { | |
}); | ||
} | ||
|
||
void _writeWrapError(Indent indent) { | ||
void _writeWrapError(SwiftOptions generatorOptions, Indent indent) { | ||
indent.newln(); | ||
indent.write('private func wrapError(_ error: Any) -> [Any?] '); | ||
indent.addScoped('{', '}', () { | ||
tarrinneal marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
@@ -627,6 +634,16 @@ class SwiftGenerator extends StructuredGenerator<SwiftOptions> { | |
indent.writeln('flutterError.details,'); | ||
}); | ||
}); | ||
indent.write( | ||
'if let pigeonError = error as? ${_getErrorClassName(generatorOptions)} '); | ||
indent.addScoped('{', '}', () { | ||
indent.write('return '); | ||
indent.addScoped('[', ']', () { | ||
indent.writeln('pigeonError.code,'); | ||
indent.writeln('pigeonError.message,'); | ||
indent.writeln('pigeonError.details,'); | ||
}); | ||
}); | ||
tarrinneal marked this conversation as resolved.
Show resolved
Hide resolved
|
||
indent.write('return '); | ||
indent.addScoped('[', ']', () { | ||
indent.writeln(r'"\(error)",'); | ||
|
@@ -645,13 +662,14 @@ private func nilOrValue<T>(_ value: Any?) -> T? { | |
}'''); | ||
} | ||
|
||
void _writeCreateConnectionError(Indent indent) { | ||
void _writeCreateConnectionError( | ||
SwiftOptions generatorOptions, Indent indent) { | ||
indent.newln(); | ||
indent.writeScoped( | ||
'private func createConnectionError(withChannelName channelName: String) -> FlutterError {', | ||
'private func createConnectionError(withChannelName channelName: String) -> ${_getErrorClassName(generatorOptions)} {', | ||
'}', () { | ||
indent.writeln( | ||
'return FlutterError(code: "channel-error", message: "Unable to establish connection on channel: \'\\(channelName)\'.", details: "")'); | ||
'return ${_getErrorClassName(generatorOptions)}(code: "channel-error", message: "Unable to establish connection on channel: \'\\(channelName)\'.", details: "")'); | ||
}); | ||
} | ||
|
||
|
@@ -671,17 +689,20 @@ private func nilOrValue<T>(_ value: Any?) -> T? { | |
|
||
if (hasHostApi) { | ||
_writeWrapResult(indent); | ||
_writeWrapError(indent); | ||
_writeWrapError(generatorOptions, indent); | ||
} | ||
if (hasFlutterApi) { | ||
_writeCreateConnectionError(indent); | ||
_writeCreateConnectionError(generatorOptions, indent); | ||
} | ||
|
||
_writePigeonError(generatorOptions, indent); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One last nit and we can land it, can you move the pigeon error above the other methods here? above `if (hasHostApi) {' There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
_writeIsNullish(indent); | ||
_writeNilOrValue(indent); | ||
} | ||
|
||
void _writeFlutterMethod( | ||
Indent indent, { | ||
required SwiftOptions generatorOptions, | ||
required String name, | ||
required String channelName, | ||
required List<Parameter> parameters, | ||
|
@@ -693,7 +714,7 @@ private func nilOrValue<T>(_ value: Any?) -> T? { | |
name: name, | ||
parameters: parameters, | ||
returnType: returnType, | ||
errorTypeName: 'FlutterError', | ||
errorTypeName: _getErrorClassName(generatorOptions), | ||
isAsynchronous: true, | ||
swiftFunction: swiftFunction, | ||
getParameterName: _getSafeArgumentName, | ||
|
@@ -735,12 +756,12 @@ private func nilOrValue<T>(_ value: Any?) -> T? { | |
indent.writeln('let message: String? = nilOrValue(listResponse[1])'); | ||
indent.writeln('let details: String? = nilOrValue(listResponse[2])'); | ||
indent.writeln( | ||
'completion(.failure(FlutterError(code: code, message: message, details: details)))'); | ||
'completion(.failure(${_getErrorClassName(generatorOptions)}(code: code, message: message, details: details)))'); | ||
}, addTrailingNewline: false); | ||
if (!returnType.isNullable && !returnType.isVoid) { | ||
indent.addScoped('else if listResponse[0] == nil {', '} ', () { | ||
indent.writeln( | ||
'completion(.failure(FlutterError(code: "null-error", message: "Flutter api returned null value for non-null return value.", details: "")))'); | ||
'completion(.failure(${_getErrorClassName(generatorOptions)}(code: "null-error", message: "Flutter api returned null value for non-null return value.", details: "")))'); | ||
}, addTrailingNewline: false); | ||
} | ||
indent.addScoped('else {', '}', () { | ||
|
@@ -870,11 +891,48 @@ private func nilOrValue<T>(_ value: Any?) -> T? { | |
indent.writeln('$varChannelName.setMessageHandler(nil)'); | ||
}); | ||
} | ||
|
||
void _writePigeonError(SwiftOptions generatorOptions, Indent indent) { | ||
indent.newln(); | ||
indent.writeln( | ||
'/// Error class for passing custom error details to Dart side.'); | ||
indent.writeln( | ||
'final class ${_getErrorClassName(generatorOptions)}: Error {'); | ||
indent.nest(1, () { | ||
tarrinneal marked this conversation as resolved.
Show resolved
Hide resolved
|
||
indent.writeln('let code: String'); | ||
indent.writeln('let message: String?'); | ||
indent.writeln('let details: Any?'); | ||
indent.newln(); | ||
indent.writeln('init(code: String, message: String?, details: Any?) {'); | ||
indent.nest(1, () { | ||
indent.writeln('self.code = code'); | ||
indent.writeln('self.message = message'); | ||
indent.writeln('self.details = details'); | ||
}); | ||
indent.writeln('}'); | ||
indent.newln(); | ||
indent.writeln('var localizedDescription: String {'); | ||
indent.nest(1, () { | ||
indent.writeln('return'); | ||
indent.nest(1, () { | ||
indent.writeln( | ||
'"${_getErrorClassName(generatorOptions)}(code: \\(code), message: \\(message ?? "<nil>"), details: \\(details ?? "<nil>")"'); | ||
}); | ||
}); | ||
indent.write('}'); | ||
}); | ||
indent.newln(); | ||
indent.write('}'); | ||
indent.newln(); | ||
} | ||
} | ||
|
||
/// Calculates the name of the codec that will be generated for [api]. | ||
String _getCodecName(Api api) => '${api.name}Codec'; | ||
|
||
String _getErrorClassName(SwiftOptions generatorOptions) => | ||
generatorOptions.errorClassName ?? 'PigeonError'; | ||
|
||
String _getArgumentName(int count, NamedType argument) => | ||
argument.name.isEmpty ? 'arg$count' : argument.name; | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be an empty line between these.
* **Breaking Change** [swift] Removes `FlutterError` in favor of `PigeonError`.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you update the changelog itself to match my comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah.. I missed that line. Changed it now.