Enable null safety by default in templates#78619
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
flutter-dashboard
bot
added
the
tool
|
@munificent can you check to see if my changes are ideomatic? |
goderbauer
left a comment
goderbauer
left a comment
There was a problem hiding this comment.
Look like some tool tests are failing.
packages/flutter_tools/templates/module/common/lib/main.dart.tmpl
Outdated
Show resolved
Hide resolved
|
|
||
| static Future<String> get platformVersion async { | ||
| final String version = await _channel.invokeMethod('getPlatformVersion'); | ||
| static Future<String?> get platformVersion async { |
There was a problem hiding this comment.
Should this actually be allowed to return null? Or is null always an error case (e.g. plugin wasn't available).
There was a problem hiding this comment.
If I'm reading this correctly, then MethodChannel can never return null, but OptionalMethodChannel can.
https://github.com/flutter/flutter/blob/master/packages/flutter/lib/src/services/platform_channel.dart#L330
I think we know in MethodChannel that T is never null (as per missingOk: false), so we could change MethodChannel.invokeMethod to return Future<T> not Future<T?>?
There was a problem hiding this comment.
This seems to work:
diff --git a/packages/flutter/lib/src/services/platform_channel.dart b/packages/flutter/lib/src/services/platform_channel.dart
index c9bad0e210..bd07c68829 100644
--- a/packages/flutter/lib/src/services/platform_channel.dart
+++ b/packages/flutter/lib/src/services/platform_channel.dart
@@ -327,8 +327,8 @@ class MethodChannel {
/// * <https://api.flutter.dev/javadoc/io/flutter/plugin/common/MethodCall.html>
/// for how to access method call arguments on Android.
@optionalTypeArgs
- Future<T?> invokeMethod<T>(String method, [ dynamic arguments ]) {
- return _invokeMethod<T>(method, missingOk: false, arguments: arguments);
+ Future<T> invokeMethod<T>(String method, [ dynamic arguments ]) {
+ return _invokeMethod<T>(method, missingOk: false, arguments: arguments) as T;
}Who know this code best and can comment?
There was a problem hiding this comment.
Looks like changing the interface requires a larger refactoring...
munificent
left a comment
munificent
left a comment
There was a problem hiding this comment.
Dart changes look idiomatic to me.
|
CI is now passing (the red X is the Flutter Dashboard being red). |
|
|
||
| static Future<String> get platformVersion async { | ||
| final String version = await _channel.invokeMethod('getPlatformVersion'); | ||
| static Future<String?> get platformVersion async { |
There was a problem hiding this comment.
Looks like changing the interface requires a larger refactoring...
Co-authored-by: Michael Goderbauer <[email protected]>
6 participants
Enable Dart null safety (https://dart.dev/null-safety) by default in all
flutter createtemplates.