Skip to content

Commit

Permalink
[pigeon] Fixes bug where Dart FlutterApis would assert that a nulla…
Browse files Browse the repository at this point in the history
…ble argument was nonnull (#1515)
  • Loading branch information
bparrishMines authored Apr 14, 2022
1 parent b3ab4a5 commit 4eda7ad
Show file tree
Hide file tree
Showing 19 changed files with 161 additions and 87 deletions.
3 changes: 3 additions & 0 deletions analysis_options.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ analyzer:
todo: ignore
# Turned off until null-safe rollout is complete.
unnecessary_null_comparison: ignore
exclude:
# Ignore generated files
- '**/*.mocks.dart' # Mockito @GenerateMocks

linter:
rules:
Expand Down
4 changes: 4 additions & 0 deletions packages/pigeon/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 2.0.4

* Fixes bug where Dart `FlutterApi`s would assert that a nullable argument was nonnull.

## 2.0.3

* Makes the generated Java Builder class final.
Expand Down
14 changes: 9 additions & 5 deletions packages/pigeon/lib/dart_generator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -338,13 +338,17 @@ void _writeFlutterApi(

indent.writeln(
'final $argType$nullTag $argName = ($argsArray[$count] as $genericArgType$nullTag)${castCall.isEmpty ? '' : '$nullTag$castCall'};');
indent.writeln(
'assert($argName != null, \'Argument for $channelName was null, expected non-null $argType.\');');
if (!arg.type.isNullable) {
indent.writeln(
'assert($argName != null, \'Argument for $channelName was null, expected non-null $argType.\');');
}
});
final Iterable<String> argNames =
indexMap(func.arguments, argNameFunc);
call =
'api.${func.name}(${argNames.map<String>((String x) => '$x$unwrapOperator').join(', ')})';
indexMap(func.arguments, (int index, NamedType field) {
final String name = _getSafeArgumentName(index, field);
return '$name${field.type.isNullable ? '' : unwrapOperator}';
});
call = 'api.${func.name}(${argNames.join(', ')})';
}
if (func.returnType.isVoid) {
if (isAsync) {
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 = '2.0.3';
const String pigeonVersion = '2.0.4';

/// Read all the content from [stdin] to a String.
String readStdin() {
Expand Down
77 changes: 55 additions & 22 deletions packages/pigeon/mock_handler_tester/test/message.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
//
// Autogenerated from Pigeon (v0.3.0), do not edit directly.
// Autogenerated from Pigeon (v2.0.4), do not edit directly.
// See also: https://pub.dev/packages/pigeon
// ignore_for_file: public_member_api_docs, non_constant_identifier_names, avoid_as, unused_import, unnecessary_parenthesis, prefer_null_aware_operators, omit_local_variable_types, unused_shown_name
// @dart = 2.12
Expand All @@ -19,6 +19,12 @@ enum RequestState {
}

class SearchRequest {
SearchRequest({
this.query,
this.anInt,
this.aBool,
});

String? query;
int? anInt;
bool? aBool;
Expand All @@ -33,14 +39,21 @@ class SearchRequest {

static SearchRequest decode(Object message) {
final Map<Object?, Object?> pigeonMap = message as Map<Object?, Object?>;
return SearchRequest()
..query = pigeonMap['query'] as String?
..anInt = pigeonMap['anInt'] as int?
..aBool = pigeonMap['aBool'] as bool?;
return SearchRequest(
query: pigeonMap['query'] as String?,
anInt: pigeonMap['anInt'] as int?,
aBool: pigeonMap['aBool'] as bool?,
);
}
}

class SearchReply {
SearchReply({
this.result,
this.error,
this.state,
});

String? result;
String? error;
RequestState? state;
Expand All @@ -55,16 +68,21 @@ class SearchReply {

static SearchReply decode(Object message) {
final Map<Object?, Object?> pigeonMap = message as Map<Object?, Object?>;
return SearchReply()
..result = pigeonMap['result'] as String?
..error = pigeonMap['error'] as String?
..state = pigeonMap['state'] != null
return SearchReply(
result: pigeonMap['result'] as String?,
error: pigeonMap['error'] as String?,
state: pigeonMap['state'] != null
? RequestState.values[pigeonMap['state']! as int]
: null;
: null,
);
}
}

class Nested {
Nested({
this.request,
});

SearchRequest? request;

Object encode() {
Expand All @@ -75,10 +93,11 @@ class Nested {

static Nested decode(Object message) {
final Map<Object?, Object?> pigeonMap = message as Map<Object?, Object?>;
return Nested()
..request = pigeonMap['request'] != null
return Nested(
request: pigeonMap['request'] != null
? SearchRequest.decode(pigeonMap['request']!)
: null;
: null,
);
}
}

Expand Down Expand Up @@ -132,7 +151,6 @@ class Api {
throw PlatformException(
code: 'channel-error',
message: 'Unable to establish connection on channel.',
details: null,
);
} else if (replyMap['error'] != null) {
final Map<Object?, Object?> error =
Expand All @@ -152,12 +170,11 @@ class Api {
'dev.flutter.pigeon.Api.search', codec,
binaryMessenger: _binaryMessenger);
final Map<Object?, Object?>? replyMap =
await channel.send(<Object>[arg_request]) as Map<Object?, Object?>?;
await channel.send(<Object?>[arg_request]) as Map<Object?, Object?>?;
if (replyMap == null) {
throw PlatformException(
code: 'channel-error',
message: 'Unable to establish connection on channel.',
details: null,
);
} else if (replyMap['error'] != null) {
final Map<Object?, Object?> error =
Expand All @@ -167,6 +184,11 @@ class Api {
message: error['message'] as String?,
details: error['details'],
);
} else if (replyMap['result'] == null) {
throw PlatformException(
code: 'null-error',
message: 'Host platform returned null value for non-null return value.',
);
} else {
return (replyMap['result'] as SearchReply?)!;
}
Expand All @@ -183,6 +205,9 @@ class _NestedApiCodec extends StandardMessageCodec {
} else if (value is SearchReply) {
buffer.putUint8(129);
writeValue(buffer, value.encode());
} else if (value is SearchRequest) {
buffer.putUint8(130);
writeValue(buffer, value.encode());
} else {
super.writeValue(buffer, value);
}
Expand All @@ -197,6 +222,9 @@ class _NestedApiCodec extends StandardMessageCodec {
case 129:
return SearchReply.decode(readValue(buffer)!);

case 130:
return SearchRequest.decode(readValue(buffer)!);

default:
return super.readValueOfType(type, buffer);
}
Expand All @@ -219,12 +247,11 @@ class NestedApi {
'dev.flutter.pigeon.NestedApi.search', codec,
binaryMessenger: _binaryMessenger);
final Map<Object?, Object?>? replyMap =
await channel.send(<Object>[arg_nested]) as Map<Object?, Object?>?;
await channel.send(<Object?>[arg_nested]) as Map<Object?, Object?>?;
if (replyMap == null) {
throw PlatformException(
code: 'channel-error',
message: 'Unable to establish connection on channel.',
details: null,
);
} else if (replyMap['error'] != null) {
final Map<Object?, Object?> error =
Expand All @@ -234,6 +261,11 @@ class NestedApi {
message: error['message'] as String?,
details: error['details'],
);
} else if (replyMap['result'] == null) {
throw PlatformException(
code: 'null-error',
message: 'Host platform returned null value for non-null return value.',
);
} else {
return (replyMap['result'] as SearchReply?)!;
}
Expand Down Expand Up @@ -274,18 +306,19 @@ abstract class FlutterSearchApi {
static const MessageCodec<Object?> codec = _FlutterSearchApiCodec();

SearchReply search(SearchRequest request);
static void setup(FlutterSearchApi? api) {
static void setup(FlutterSearchApi? api, {BinaryMessenger? binaryMessenger}) {
{
const BasicMessageChannel<Object?> channel = BasicMessageChannel<Object?>(
'dev.flutter.pigeon.FlutterSearchApi.search', codec);
final BasicMessageChannel<Object?> channel = BasicMessageChannel<Object?>(
'dev.flutter.pigeon.FlutterSearchApi.search', codec,
binaryMessenger: binaryMessenger);
if (api == null) {
channel.setMessageHandler(null);
} else {
channel.setMessageHandler((Object? message) async {
assert(message != null,
'Argument for dev.flutter.pigeon.FlutterSearchApi.search was null.');
final List<Object?> args = (message as List<Object?>?)!;
final SearchRequest? arg_request = args[0] as SearchRequest?;
final SearchRequest? arg_request = (args[0] as SearchRequest?);
assert(arg_request != null,
'Argument for dev.flutter.pigeon.FlutterSearchApi.search was null, expected non-null SearchRequest.');
final SearchReply output = api.search(arg_request!);
Expand Down
34 changes: 22 additions & 12 deletions packages/pigeon/mock_handler_tester/test/test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
//
// Autogenerated from Pigeon (v0.3.0), do not edit directly.
// Autogenerated from Pigeon (v2.0.4), do not edit directly.
// See also: https://pub.dev/packages/pigeon
// ignore_for_file: public_member_api_docs, non_constant_identifier_names, avoid_as, unused_import
// ignore_for_file: public_member_api_docs, non_constant_identifier_names, avoid_as, unused_import, unnecessary_parenthesis
// ignore_for_file: avoid_relative_lib_imports
// @dart = 2.12
import 'dart:async';
import 'dart:typed_data' show Uint8List, Int32List, Int64List, Float64List;
Expand Down Expand Up @@ -49,10 +50,11 @@ abstract class TestHostApi {

void initialize();
SearchReply search(SearchRequest request);
static void setup(TestHostApi? api) {
static void setup(TestHostApi? api, {BinaryMessenger? binaryMessenger}) {
{
const BasicMessageChannel<Object?> channel = BasicMessageChannel<Object?>(
'dev.flutter.pigeon.Api.initialize', codec);
final BasicMessageChannel<Object?> channel = BasicMessageChannel<Object?>(
'dev.flutter.pigeon.Api.initialize', codec,
binaryMessenger: binaryMessenger);
if (api == null) {
channel.setMockMessageHandler(null);
} else {
Expand All @@ -64,16 +66,17 @@ abstract class TestHostApi {
}
}
{
const BasicMessageChannel<Object?> channel =
BasicMessageChannel<Object?>('dev.flutter.pigeon.Api.search', codec);
final BasicMessageChannel<Object?> channel = BasicMessageChannel<Object?>(
'dev.flutter.pigeon.Api.search', codec,
binaryMessenger: binaryMessenger);
if (api == null) {
channel.setMockMessageHandler(null);
} else {
channel.setMockMessageHandler((Object? message) async {
assert(message != null,
'Argument for dev.flutter.pigeon.Api.search was null.');
final List<Object?> args = (message as List<Object?>?)!;
final SearchRequest? arg_request = args[0] as SearchRequest?;
final SearchRequest? arg_request = (args[0] as SearchRequest?);
assert(arg_request != null,
'Argument for dev.flutter.pigeon.Api.search was null, expected non-null SearchRequest.');
final SearchReply output = api.search(arg_request!);
Expand All @@ -94,6 +97,9 @@ class _TestNestedApiCodec extends StandardMessageCodec {
} else if (value is SearchReply) {
buffer.putUint8(129);
writeValue(buffer, value.encode());
} else if (value is SearchRequest) {
buffer.putUint8(130);
writeValue(buffer, value.encode());
} else {
super.writeValue(buffer, value);
}
Expand All @@ -108,6 +114,9 @@ class _TestNestedApiCodec extends StandardMessageCodec {
case 129:
return SearchReply.decode(readValue(buffer)!);

case 130:
return SearchRequest.decode(readValue(buffer)!);

default:
return super.readValueOfType(type, buffer);
}
Expand All @@ -118,18 +127,19 @@ abstract class TestNestedApi {
static const MessageCodec<Object?> codec = _TestNestedApiCodec();

SearchReply search(Nested nested);
static void setup(TestNestedApi? api) {
static void setup(TestNestedApi? api, {BinaryMessenger? binaryMessenger}) {
{
const BasicMessageChannel<Object?> channel = BasicMessageChannel<Object?>(
'dev.flutter.pigeon.NestedApi.search', codec);
final BasicMessageChannel<Object?> channel = BasicMessageChannel<Object?>(
'dev.flutter.pigeon.NestedApi.search', codec,
binaryMessenger: binaryMessenger);
if (api == null) {
channel.setMockMessageHandler(null);
} else {
channel.setMockMessageHandler((Object? message) async {
assert(message != null,
'Argument for dev.flutter.pigeon.NestedApi.search was null.');
final List<Object?> args = (message as List<Object?>?)!;
final Nested? arg_nested = args[0] as Nested?;
final Nested? arg_nested = (args[0] as Nested?);
assert(arg_nested != null,
'Argument for dev.flutter.pigeon.NestedApi.search was null, expected non-null Nested.');
final SearchReply output = api.search(arg_nested!);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
//
// Autogenerated from Pigeon (v1.0.19), do not edit directly.
// Autogenerated from Pigeon (v2.0.4), do not edit directly.
// See also: https://pub.dev/packages/pigeon
// ignore_for_file: public_member_api_docs, non_constant_identifier_names, avoid_as, unused_import, unnecessary_parenthesis, prefer_null_aware_operators, omit_local_variable_types, unused_shown_name
// @dart = 2.12
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
//
// Autogenerated from Pigeon (v1.0.19), do not edit directly.
// Autogenerated from Pigeon (v2.0.4), do not edit directly.
// See also: https://pub.dev/packages/pigeon
// ignore_for_file: public_member_api_docs, non_constant_identifier_names, avoid_as, unused_import, unnecessary_parenthesis, prefer_null_aware_operators, omit_local_variable_types, unused_shown_name
// @dart = 2.12
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
//
// Autogenerated from Pigeon (v1.0.19), do not edit directly.
// Autogenerated from Pigeon (v2.0.4), do not edit directly.
// See also: https://pub.dev/packages/pigeon
// ignore_for_file: public_member_api_docs, non_constant_identifier_names, avoid_as, unused_import, unnecessary_parenthesis, prefer_null_aware_operators, omit_local_variable_types, unused_shown_name
// @dart = 2.12
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
//
// Autogenerated from Pigeon (v2.0.2), do not edit directly.
// Autogenerated from Pigeon (v2.0.4), do not edit directly.
// See also: https://pub.dev/packages/pigeon
// ignore_for_file: public_member_api_docs, non_constant_identifier_names, avoid_as, unused_import, unnecessary_parenthesis, prefer_null_aware_operators, omit_local_variable_types, unused_shown_name
// @dart = 2.12
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
//
// Autogenerated from Pigeon (v1.0.19), do not edit directly.
// Autogenerated from Pigeon (v2.0.4), do not edit directly.
// See also: https://pub.dev/packages/pigeon
// ignore_for_file: public_member_api_docs, non_constant_identifier_names, avoid_as, unused_import, unnecessary_parenthesis, prefer_null_aware_operators, omit_local_variable_types, unused_shown_name
// @dart = 2.12
Expand Down
Loading

0 comments on commit 4eda7ad

Please sign in to comment.