Skip to content

Commit eab885b

Browse files
stuartmorgan-gstuartmorgan-g
authored
[google_maps_flutter] Fix handling of non-nullable invokeMethod return types (flutter#3754)
During the null-safety migration I accepted the auto-migrator use of as Future<T> to handle invokeMethod<T> returning a T?. I didn't realize that as does not actually do that kind of casting, and will fail with "type 'Future<T?>' is not a subtype of type 'Future<T>' in type cast". There were no tests that exercised these methods in any way, so automated tests didn't catch the bug. This adds a minimal test that calls all of the non-void methods to ensure that they don't explode (and a TODO to backfill full unit tests of the entire method channel). Fixes flutter/flutter#78426 Fixes flutter/flutter#78856
1 parent c3dc31f commit eab885b

5 files changed

Lines changed: 92 additions & 30 deletions

File tree

‎packages/google_maps_flutter/google_maps_flutter_platform_interface/CHANGELOG.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
1+
## 2.0.3
2+
3+
* Fix type issues in `isMarkerInfoWindowShown` and `getZoomLevel` introduced
4+
in the null safety migration.
5+
16
## 2.0.2
27

38
* Mark constructors for CameraUpdate, CircleId, MapsObjectId, MarkerId, PolygonId, PolylineId and TileOverlayId as const

‎packages/google_maps_flutter/google_maps_flutter_platform_interface/lib/src/method_channel/method_channel_google_maps_flutter.dart‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -62,15 +62,22 @@ class MethodChannelGoogleMapsFlutter extends GoogleMapsFlutterPlatform {
6262
// Keep a collection of mapId to a map of TileOverlays.
6363
final Map<int, Map<TileOverlayId, TileOverlay>> _tileOverlays = {};
6464

65-
@override
66-
Future<void> init(int mapId) {
65+
/// Returns the channel for [mapId], creating it if it doesn't already exist.
66+
@visibleForTesting
67+
MethodChannel ensureChannelInitialized(int mapId) {
6768
MethodChannel? channel = _channels[mapId];
6869
if (channel == null) {
6970
channel = MethodChannel('plugins.flutter.io/google_maps_$mapId');
7071
channel.setMethodCallHandler(
7172
(MethodCall call) => _handleMethodCall(call, mapId));
7273
_channels[mapId] = channel;
7374
}
75+
return channel;
76+
}
77+
78+
@override
79+
Future<void> init(int mapId) {
80+
MethodChannel channel = ensureChannelInitialized(mapId);
7481
return channel.invokeMethod<void>('map#waitForMap');
7582
}
7683

@@ -414,18 +421,17 @@ class MethodChannelGoogleMapsFlutter extends GoogleMapsFlutterPlatform {
414421
Future<bool> isMarkerInfoWindowShown(
415422
MarkerId markerId, {
416423
required int mapId,
417-
}) {
424+
}) async {
418425
assert(markerId != null);
419-
return channel(mapId).invokeMethod<bool>('markers#isInfoWindowShown',
420-
<String, String>{'markerId': markerId.value}) as Future<bool>;
426+
return (await channel(mapId).invokeMethod<bool>('markers#isInfoWindowShown',
427+
<String, String>{'markerId': markerId.value}))!;
421428
}
422429

423430
@override
424431
Future<double> getZoomLevel({
425432
required int mapId,
426-
}) {
427-
return channel(mapId).invokeMethod<double>('map#getZoomLevel')
428-
as Future<double>;
433+
}) async {
434+
return (await channel(mapId).invokeMethod<double>('map#getZoomLevel'))!;
429435
}
430436

431437
@override

‎packages/google_maps_flutter/google_maps_flutter_platform_interface/pubspec.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ description: A common platform interface for the google_maps_flutter plugin.
33
homepage: https://github.com/flutter/plugins/tree/master/packages/google_maps_flutter/google_maps_flutter_platform_interface
44
# NOTE: We strongly prefer non-breaking changes, even at the expense of a
55
# less-clean API. See https://flutter.dev/go/platform-interface-breaking-changes
6-
version: 2.0.2
6+
version: 2.0.3
77

88
dependencies:
99
flutter:

‎packages/google_maps_flutter/google_maps_flutter_platform_interface/test/method_channel/method_channel_google_maps_flutter_test.dart‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
// Copyright 2013 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
import 'package:flutter/services.dart';
6+
import 'package:flutter_test/flutter_test.dart';
7+
8+
import 'package:google_maps_flutter_platform_interface/src/method_channel/method_channel_google_maps_flutter.dart';
9+
import 'package:google_maps_flutter_platform_interface/google_maps_flutter_platform_interface.dart';
10+
11+
void main() {
12+
TestWidgetsFlutterBinding.ensureInitialized();
13+
14+
group('$MethodChannelGoogleMapsFlutter', () {
15+
late List<String> log;
16+
17+
setUp(() async {
18+
log = <String>[];
19+
});
20+
21+
/// Initializes a map with the given ID and canned responses, logging all
22+
/// calls to [log].
23+
void configureMockMap(
24+
MethodChannelGoogleMapsFlutter maps, {
25+
required int mapId,
26+
required Future<dynamic>? Function(MethodCall call) handler,
27+
}) {
28+
maps
29+
.ensureChannelInitialized(mapId)
30+
.setMockMethodCallHandler((MethodCall methodCall) {
31+
log.add(methodCall.method);
32+
return handler(methodCall);
33+
});
34+
}
35+
36+
// Calls each method that uses invokeMethod with a return type other than
37+
// void to ensure that the casting/nullability handling succeeds.
38+
//
39+
// TODO(stuartmorgan): Remove this once there is real test coverage of
40+
// each method, since that would cover this issue.
41+
test('non-void invokeMethods handle types correctly', () async {
42+
const int mapId = 0;
43+
final MethodChannelGoogleMapsFlutter maps =
44+
MethodChannelGoogleMapsFlutter();
45+
configureMockMap(maps, mapId: mapId,
46+
handler: (MethodCall methodCall) async {
47+
switch (methodCall.method) {
48+
case 'map#getLatLng':
49+
return <dynamic>[1.0, 2.0];
50+
case 'markers#isInfoWindowShown':
51+
return true;
52+
case 'map#getZoomLevel':
53+
return 2.5;
54+
case 'map#takeSnapshot':
55+
return null;
56+
}
57+
});
58+
59+
await maps.getLatLng(ScreenCoordinate(x: 0, y: 0), mapId: mapId);
60+
await maps.isMarkerInfoWindowShown(MarkerId(''), mapId: mapId);
61+
await maps.getZoomLevel(mapId: mapId);
62+
await maps.takeSnapshot(mapId: mapId);
63+
// Check that all the invokeMethod calls happened.
64+
expect(log, <String>[
65+
'map#getLatLng',
66+
'markers#isInfoWindowShown',
67+
'map#getZoomLevel',
68+
'map#takeSnapshot',
69+
]);
70+
});
71+
});
72+
}

‎packages/google_maps_flutter/google_maps_flutter_platform_interface/test/platform_interface/google_maps_flutter_platform_test.dart‎

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
// found in the LICENSE file.
44

55
import 'package:mockito/mockito.dart';
6-
import 'package:flutter/services.dart';
76
import 'package:flutter_test/flutter_test.dart';
87
import 'package:plugin_platform_interface/plugin_platform_interface.dart';
98

@@ -36,26 +35,6 @@ void main() {
3635
GoogleMapsFlutterPlatform.instance = ExtendsGoogleMapsFlutterPlatform();
3736
});
3837
});
39-
40-
group('$MethodChannelGoogleMapsFlutter', () {
41-
const MethodChannel channel =
42-
MethodChannel('plugins.flutter.io/google_maps_flutter');
43-
final List<MethodCall> log = <MethodCall>[];
44-
channel.setMockMethodCallHandler((MethodCall methodCall) async {
45-
log.add(methodCall);
46-
});
47-
48-
tearDown(() {
49-
log.clear();
50-
});
51-
52-
test('foo', () async {
53-
expect(
54-
log,
55-
<Matcher>[],
56-
);
57-
});
58-
});
5938
}
6039

6140
class GoogleMapsFlutterPlatformMock extends Mock

0 commit comments

Comments
 (0)