Skip to content

Commit

Permalink
[pigeon] Allows kotlin generator to skip error class generation (flut…
Browse files Browse the repository at this point in the history
…ter#6183)

Allows kotlin generator to skip error class generation by creating `includeErrorClass` argument.

This may be considered only a partial fix, as it requires another pigeon file with the class generated rather than creating a new file with the class.

This solution seems to me to solve the problem well enough as creating another file is a non-trivial task and there is already a working solution - renaming the error class per file.

fixes: flutter#142099
  • Loading branch information
tarrinneal authored Feb 27, 2024
1 parent e07eb50 commit 679bdd7
Show file tree
Hide file tree
Showing 8 changed files with 86 additions and 100 deletions.
3 changes: 2 additions & 1 deletion packages/pigeon/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
## NEXT
## 17.1.0

* [kotlin] Adds `includeErrorClass` to `KotlinOptions`.
* Updates minimum supported SDK version to Flutter 3.13/Dart 3.1.

## 17.0.0
Expand Down
2 changes: 1 addition & 1 deletion packages/pigeon/lib/generator_tools.dart
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import 'ast.dart';
/// The current version of pigeon.
///
/// This must match the version in pubspec.yaml.
const String pigeonVersion = '17.0.0';
const String pigeonVersion = '17.1.0';

/// Read all the content from [stdin] to a String.
String readStdin() {
Expand Down
35 changes: 23 additions & 12 deletions packages/pigeon/lib/kotlin_generator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ class KotlinOptions {
this.package,
this.copyrightHeader,
this.errorClassName,
this.includeErrorClass = true,
});

/// The package where the generated class will live.
Expand All @@ -43,13 +44,20 @@ class KotlinOptions {
/// The name of the error class used for passing custom error parameters.
final String? errorClassName;

/// Whether to include the error class in generation.
///
/// This should only ever be set to false if you have another generated
/// Kotlin file in the same directory.
final bool includeErrorClass;

/// Creates a [KotlinOptions] from a Map representation where:
/// `x = KotlinOptions.fromMap(x.toMap())`.
static KotlinOptions fromMap(Map<String, Object> map) {
return KotlinOptions(
package: map['package'] as String?,
copyrightHeader: map['copyrightHeader'] as Iterable<String>?,
errorClassName: map['errorClassName'] as String?,
includeErrorClass: map['includeErrorClass'] as bool? ?? true,
);
}

Expand All @@ -60,6 +68,7 @@ class KotlinOptions {
if (package != null) 'package': package!,
if (copyrightHeader != null) 'copyrightHeader': copyrightHeader!,
if (errorClassName != null) 'errorClassName': errorClassName!,
'includeErrorClass': includeErrorClass,
};
return result;
}
Expand Down Expand Up @@ -298,7 +307,7 @@ class KotlinGenerator extends StructuredGenerator<KotlinOptions> {
addDocumentationComments(
indent, field.documentationComments, _docCommentSpec);
indent.write(
'val ${field.name}: ${_nullsafeKotlinTypeForDartType(field.type)}');
'val ${field.name}: ${_nullSafeKotlinTypeForDartType(field.type)}');
final String defaultNil = field.type.isNullable ? ' = null' : '';
indent.add(defaultNil);
}
Expand Down Expand Up @@ -361,16 +370,15 @@ class KotlinGenerator extends StructuredGenerator<KotlinOptions> {
});
});

final String errorClassName = _getErrorClassName(generatorOptions);
for (final Method method in api.methods) {
_writeFlutterMethod(
indent,
generatorOptions: generatorOptions,
name: method.name,
parameters: method.parameters,
returnType: method.returnType,
channelName: makeChannelName(api, method, dartPackageName),
documentationComments: method.documentationComments,
errorClassName: errorClassName,
dartPackageName: dartPackageName,
);
}
Expand Down Expand Up @@ -588,7 +596,9 @@ class KotlinGenerator extends StructuredGenerator<KotlinOptions> {
if (hasFlutterApi) {
_writeCreateConnectionError(generatorOptions, indent);
}
_writeErrorClass(generatorOptions, indent);
if (generatorOptions.includeErrorClass) {
_writeErrorClass(generatorOptions, indent);
}
}

static void _writeMethodDeclaration(
Expand All @@ -606,7 +616,7 @@ class KotlinGenerator extends StructuredGenerator<KotlinOptions> {
final List<String> argSignature = <String>[];
if (parameters.isNotEmpty) {
final Iterable<String> argTypes = parameters
.map((NamedType e) => _nullsafeKotlinTypeForDartType(e.type));
.map((NamedType e) => _nullSafeKotlinTypeForDartType(e.type));
final Iterable<String> argNames = indexMap(parameters, getArgumentName);
argSignature.addAll(
map2(
Expand All @@ -620,7 +630,7 @@ class KotlinGenerator extends StructuredGenerator<KotlinOptions> {
}

final String returnTypeString =
returnType.isVoid ? '' : _nullsafeKotlinTypeForDartType(returnType);
returnType.isVoid ? '' : _nullSafeKotlinTypeForDartType(returnType);

final String resultType = returnType.isVoid ? 'Unit' : returnTypeString;
addDocumentationComments(indent, documentationComments, _docCommentSpec);
Expand Down Expand Up @@ -700,7 +710,7 @@ class KotlinGenerator extends StructuredGenerator<KotlinOptions> {
indent.write('$call ');
final String resultType = returnType.isVoid
? 'Unit'
: _nullsafeKotlinTypeForDartType(returnType);
: _nullSafeKotlinTypeForDartType(returnType);
indent.addScoped('{ result: Result<$resultType> ->', '}', () {
indent.writeln('val error = result.exceptionOrNull()');
indent.writeScoped('if (error != null) {', '}', () {
Expand Down Expand Up @@ -751,11 +761,11 @@ class KotlinGenerator extends StructuredGenerator<KotlinOptions> {

void _writeFlutterMethod(
Indent indent, {
required KotlinOptions generatorOptions,
required String name,
required List<Parameter> parameters,
required TypeDeclaration returnType,
required String channelName,
required String errorClassName,
required String dartPackageName,
List<String> documentationComments = const <String>[],
int? minApiRequirement,
Expand All @@ -778,6 +788,7 @@ class KotlinGenerator extends StructuredGenerator<KotlinOptions> {
getArgumentName: _getSafeArgumentName,
);

final String errorClassName = _getErrorClassName(generatorOptions);
indent.addScoped('{', '}', () {
onWriteBody(
indent,
Expand Down Expand Up @@ -910,9 +921,9 @@ String _kotlinTypeForBuiltinGenericDartType(TypeDeclaration type) {
} else {
switch (type.baseName) {
case 'List':
return 'List<${_nullsafeKotlinTypeForDartType(type.typeArguments.first)}>';
return 'List<${_nullSafeKotlinTypeForDartType(type.typeArguments.first)}>';
case 'Map':
return 'Map<${_nullsafeKotlinTypeForDartType(type.typeArguments.first)}, ${_nullsafeKotlinTypeForDartType(type.typeArguments.last)}>';
return 'Map<${_nullSafeKotlinTypeForDartType(type.typeArguments.first)}, ${_nullSafeKotlinTypeForDartType(type.typeArguments.last)}>';
default:
return '${type.baseName}<${_flattenTypeArguments(type.typeArguments)}>';
}
Expand Down Expand Up @@ -946,7 +957,7 @@ String _kotlinTypeForDartType(TypeDeclaration type) {
return _kotlinTypeForBuiltinDartType(type) ?? type.baseName;
}

String _nullsafeKotlinTypeForDartType(TypeDeclaration type) {
String _nullSafeKotlinTypeForDartType(TypeDeclaration type) {
final String nullSafe = type.isNullable ? '?' : '';
return '${_kotlinTypeForDartType(type)}$nullSafe';
}
Expand All @@ -969,7 +980,7 @@ String _cast(Indent indent, String variable, {required TypeDeclaration type}) {
}
return '${type.baseName}.ofRaw($variable as Int)!!';
}
return '$variable as ${_nullsafeKotlinTypeForDartType(type)}';
return '$variable as ${_nullSafeKotlinTypeForDartType(type)}';
}

String _castInt(bool isNullable) {
Expand Down
1 change: 1 addition & 0 deletions packages/pigeon/lib/pigeon_lib.dart
Original file line number Diff line number Diff line change
Expand Up @@ -776,6 +776,7 @@ class KotlinGeneratorAdapter implements GeneratorAdapter {
options.kotlinOptions ?? const KotlinOptions();
kotlinOptions = kotlinOptions.merge(KotlinOptions(
errorClassName: kotlinOptions.errorClassName ?? 'FlutterError',
includeErrorClass: kotlinOptions.includeErrorClass,
copyrightHeader: options.copyrightHeader != null
? _lineReader(
path.posix.join(options.basePath ?? '', options.copyrightHeader))
Expand Down
Loading

0 comments on commit 679bdd7

Please sign in to comment.