[pigeon][swift] Removes FlutterError in favor of PigeonError #6611
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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?, | ||
|
Contributor
There was a problem hiding this comment. is it open for developers to use a custom class name? why don't we just use
Contributor
Author
There was a problem hiding this comment.
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: _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(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( | ||
| SwiftOptions generatorOptions, Indent indent) { | ||
| indent.newln(); | ||
| indent.writeScoped( | ||
| 'private func createConnectionError(withChannelName channelName: String) -> ${_getErrorClassName(generatorOptions)} {', | ||
| '}', () { | ||
| indent.writeln( | ||
| '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(generatorOptions, indent); | ||
| } | ||
| if (hasFlutterApi) { | ||
| _writeCreateConnectionError(generatorOptions, indent); | ||
| } | ||
|
|
||
| _writePigeonError(generatorOptions, indent); | ||
|
||
| _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: _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(${_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(${_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,56 @@ 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 Flutter.'); | ||
| indent.writeln( | ||
| 'final class ${_getErrorClassName(generatorOptions)}: Error {'); | ||
| indent.nest(1, () { | ||
tarrinneal marked this conversation as resolved.
Outdated
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('init(flutterError: FlutterError) {'); | ||
| indent.nest(1, () { | ||
| indent.writeln('self.code = flutterError.code'); | ||
| indent.writeln('self.message = flutterError.message'); | ||
| indent.writeln('self.details = flutterError.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; | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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.