Skip to content

Commit

Permalink
Fix bug when parsing DartOptions.copyrightHeader with `@ConfigurePi…
Browse files Browse the repository at this point in the history
…geon` (flutter#672)
  • Loading branch information
bparrishMines authored Feb 10, 2022
1 parent a61bdb8 commit 28b5bbd
Show file tree
Hide file tree
Showing 8 changed files with 93 additions and 22 deletions.
4 changes: 4 additions & 0 deletions packages/pigeon/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 1.0.18

* [front-end] Fix error caused by parsing `copyrightHeaders` passed to options in `@ConfigurePigeon`.

## 1.0.17

* [dart_test] Adds missing linter ignores.
Expand Down
4 changes: 3 additions & 1 deletion packages/pigeon/lib/dart_generator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@ class DartOptions {
/// Creates a [DartOptions] from a Map representation where:
/// `x = DartOptions.fromMap(x.toMap())`.
static DartOptions fromMap(Map<String, Object> map) {
final Iterable<dynamic>? copyrightHeader =
map['copyrightHeader'] as Iterable<dynamic>?;
return DartOptions(
isNullSafe: map['isNullSafe'] as bool? ?? true,
copyrightHeader: map['copyrightHeader'] as Iterable<String>?,
copyrightHeader: copyrightHeader?.cast<String>(),
);
}

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 @@ -8,7 +8,7 @@ import 'dart:mirrors';
import 'ast.dart';

/// The current version of pigeon. This must match the version in pubspec.yaml.
const String pigeonVersion = '1.0.17';
const String pigeonVersion = '1.0.18';

/// Read all the content from [stdin] to a String.
String readStdin() {
Expand Down
4 changes: 3 additions & 1 deletion packages/pigeon/lib/java_generator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,12 @@ class JavaOptions {
/// Creates a [JavaOptions] from a Map representation where:
/// `x = JavaOptions.fromMap(x.toMap())`.
static JavaOptions fromMap(Map<String, Object> map) {
final Iterable<dynamic>? copyrightHeader =
map['copyrightHeader'] as Iterable<dynamic>?;
return JavaOptions(
className: map['className'] as String?,
package: map['package'] as String?,
copyrightHeader: map['copyrightHeader'] as Iterable<String>?,
copyrightHeader: copyrightHeader?.cast<String>(),
);
}

Expand Down
4 changes: 3 additions & 1 deletion packages/pigeon/lib/objc_generator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,12 @@ class ObjcOptions {
/// Creates a [ObjcOptions] from a Map representation where:
/// `x = ObjcOptions.fromMap(x.toMap())`.
static ObjcOptions fromMap(Map<String, Object> map) {
final Iterable<dynamic>? copyrightHeader =
map['copyrightHeader'] as Iterable<dynamic>?;
return ObjcOptions(
header: map['header'] as String?,
prefix: map['prefix'] as String?,
copyrightHeader: map['copyrightHeader'] as Iterable<String>?,
copyrightHeader: copyrightHeader?.cast<String>(),
);
}

Expand Down
50 changes: 33 additions & 17 deletions packages/pigeon/lib/pigeon_lib.dart
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,19 @@ class _RootBuilder extends dart_ast_visitor.RecursiveAstVisitor<Object?> {
return expression.value!;
} else if (expression is dart_ast.BooleanLiteral) {
return expression.value;
} else if (expression is dart_ast.ListLiteral) {
final List<dynamic> list = <dynamic>[];
for (final dart_ast.CollectionElement element in expression.elements) {
if (element is dart_ast.Expression) {
list.add(_expressionToMap(element));
} else {
_errors.add(Error(
message: 'expected Expression but found $element',
lineNumber: _calculateLineNumber(source, element.offset),
));
}
}
return list;
} else {
_errors.add(Error(
message:
Expand Down Expand Up @@ -1086,6 +1099,20 @@ options:

final ParseResults parseResults =
pigeon.parseFile(options.input!, sdkPath: sdkPath);

if (parseResults.errors.isNotEmpty) {
final List<Error> errors = <Error>[];
for (final Error err in parseResults.errors) {
errors.add(Error(
message: err.message,
filename: options.input,
lineNumber: err.lineNumber));
}

printErrors(errors);
return 1;
}

if (parseResults.pigeonOptions != null) {
options = PigeonOptions.fromMap(
mergeMaps(options.toMap(), parseResults.pigeonOptions!));
Expand All @@ -1096,32 +1123,21 @@ options:
return 1;
}

final List<Error> errors = <Error>[];
if (options.objcHeaderOut != null) {
options = options.merge(PigeonOptions(
objcOptions: options.objcOptions!.merge(
ObjcOptions(header: path.basename(options.objcHeaderOut!)))));
}

for (final Error err in parseResults.errors) {
errors.add(Error(
message: err.message,
filename: options.input,
lineNumber: err.lineNumber));
}
if (errors.isEmpty) {
for (final Generator generator in safeGenerators) {
final IOSink? sink = generator.shouldGenerate(options);
if (sink != null) {
generator.generate(sink, options, parseResults.root);
await sink.flush();
}
for (final Generator generator in safeGenerators) {
final IOSink? sink = generator.shouldGenerate(options);
if (sink != null) {
generator.generate(sink, options, parseResults.root);
await sink.flush();
}
}

printErrors(errors);

return errors.isNotEmpty ? 1 : 0;
return 0;
}

/// Print a list of errors to stderr.
Expand Down
2 changes: 1 addition & 1 deletion packages/pigeon/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: pigeon
description: Code generator tool to make communication between Flutter and the host platform type-safe and easier.
repository: https://github.com/flutter/packages/tree/main/packages/pigeon
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3Apigeon
version: 1.0.17 # This must match the version in lib/generator_tools.dart
version: 1.0.18 # This must match the version in lib/generator_tools.dart

environment:
sdk: ">=2.12.0 <3.0.0"
Expand Down
45 changes: 45 additions & 0 deletions packages/pigeon/test/pigeon_lib_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1004,4 +1004,49 @@ abstract class HostApiBridge {
expect(results.root.enums.length, 1);
expect(results.root.enums[0].name, 'MessageKey');
});

test('@ConfigurePigeon JavaOptions.copyrightHeader', () {
const String code = '''
@ConfigurePigeon(PigeonOptions(
javaOptions: JavaOptions(copyrightHeader: <String>['A', 'Header']),
))
class Message {
int? id;
}
''';

final ParseResults results = _parseSource(code);
final PigeonOptions options = PigeonOptions.fromMap(results.pigeonOptions!);
expect(options.javaOptions!.copyrightHeader, <String>['A', 'Header']);
});

test('@ConfigurePigeon DartOptions.copyrightHeader', () {
const String code = '''
@ConfigurePigeon(PigeonOptions(
dartOptions: DartOptions(copyrightHeader: <String>['A', 'Header']),
))
class Message {
int? id;
}
''';

final ParseResults results = _parseSource(code);
final PigeonOptions options = PigeonOptions.fromMap(results.pigeonOptions!);
expect(options.dartOptions!.copyrightHeader, <String>['A', 'Header']);
});

test('@ConfigurePigeon ObjcOptions.copyrightHeader', () {
const String code = '''
@ConfigurePigeon(PigeonOptions(
objcOptions: ObjcOptions(copyrightHeader: <String>['A', 'Header']),
))
class Message {
int? id;
}
''';

final ParseResults results = _parseSource(code);
final PigeonOptions options = PigeonOptions.fromMap(results.pigeonOptions!);
expect(options.objcOptions!.copyrightHeader, <String>['A', 'Header']);
});
}

0 comments on commit 28b5bbd

Please sign in to comment.