Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[package_info] Migrate to null safety #3398

Merged
bparrishMines merged 12 commits into from
Feb 4, 2021
Merged

[package_info] Migrate to null safety #3398

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
0021bef
migrate to null safety
bparrishMines Dec 28, 2020
c76b56c
update
bparrishMines Jan 6, 2021
3083760
Merge branch 'master' of github.com:flutter/plugins into package_info…
bparrishMines Jan 6, 2021
1d7b6fc
use min dependency and fix example warning
bparrishMines Jan 7, 2021
08b6bd2
null check future return value and use local variable
bparrishMines Jan 11, 2021
7092d00
seperate declaration and return statement
bparrishMines Jan 11, 2021
d2b1466
add to nnbd plugins list
bparrishMines Jan 13, 2021
a5f22e9
Merge branch 'master' of github.com:flutter/plugins into package_info…
bparrishMines Jan 16, 2021
29c6f8b
Merge branch 'master' of github.com:flutter/plugins into package_info…
bparrishMines Jan 25, 2021
c28e3fe
Merge branch 'master' of github.com:flutter/plugins into package_info…
bparrishMines Feb 3, 2021
9f1af8d
Merge branch 'master' of github.com:flutter/plugins into package_info…
bparrishMines Feb 4, 2021
bd08214
reverting example dir back to master
bparrishMines Feb 4, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
migrate to null safety
  • Loading branch information
@bparrishMines
bparrishMines committed Dec 28, 2020
commit 0021befcafc034be4b3f931ebe3ff90d6cd6bd40
4 changes: 4 additions & 0 deletions packages/package_info/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 0.5.0-nullsafety

* Migrate to null safety.

## 0.4.3+3

* Update Flutter SDK constraint.
Expand Down
19 changes: 9 additions & 10 deletions packages/package_info/lib/package_info.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,30 +24,29 @@ class PackageInfo {
/// See [fromPlatform] for the right API to get a [PackageInfo] that's
/// actually populated with real data.
PackageInfo({
this.appName,
this.packageName,
this.version,
this.buildNumber,
required this.appName,
required this.packageName,
required this.version,
required this.buildNumber,
});

static PackageInfo _fromPlatform;
static PackageInfo? _fromPlatform;

/// Retrieves package information from the platform.
/// The result is cached.
static Future<PackageInfo> fromPlatform() async {
if (_fromPlatform != null) {
return _fromPlatform;
return _fromPlatform!;
Copy link

@blasten blasten Jan 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't need _fromPlatform! since the analyzer is able to detect the enclosing _fromPlatform != null check.

Copy link
Contributor Author

@bparrishMines bparrishMines Jan 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, I was wondering about that too. When I remove it, I get this error

Screen Shot 2021-01-07 at 4 36 31 PM

I also made it synchronous just in case that was the problem. It seems like the static analysis is missing the null check. I'm probably missing something obvious, but have you seen this before?

Copy link

@blasten blasten Jan 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah yeah. @mehmetf just pointed out that the variable needs to be defined in the local scope

}

final Map<String, dynamic> map =
final Map<String, dynamic>? map =
await _kChannel.invokeMapMethod<String, dynamic>('getAll');
_fromPlatform = PackageInfo(
appName: map["appName"],
return _fromPlatform = PackageInfo(
Copy link

@blasten blasten Jan 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's probably a bit more readable to have this as two separate statements

appName: map!["appName"],
Copy link

@blasten blasten Jan 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is map! required here, but not below?

Copy link
Contributor Author

@bparrishMines bparrishMines Jan 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm new to null safety, but it looks like you only have to null check the value once within the same method.

Copy link

@blasten blasten Jan 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, what about:

final Map<String, dynamic> map = await _kChannel.invokeMapMethod<String, dynamic>('getAll')!;

Copy link
Contributor Author

@bparrishMines bparrishMines Jan 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried this as well, but that only null checks the Future and not the value the Future returns. You would have to add parentheses around the await. Something like:

final Map<String, dynamic> map = (await _kChannel.invokeMapMethod<String, dynamic>('getAll'))!;

I wasn't really sure if this was cleaner or not.

Copy link

@blasten blasten Jan 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

correct.

final Map<String, dynamic> map = (await _kChannel.invokeMapMethod<String, dynamic>('getAll'))!;

lgtm

packageName: map["packageName"],
version: map["version"],
buildNumber: map["buildNumber"],
);
return _fromPlatform;
}

/// The app name. `CFBundleDisplayName` on iOS, `application/label` on Android.
Expand Down
6 changes: 3 additions & 3 deletions packages/package_info/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ homepage: https://github.com/flutter/plugins/tree/master/packages/package_info
# 0.4.y+z is compatible with 1.0.0, if you land a breaking change bump
# the version to 2.0.0.
# See more details: https://github.com/flutter/flutter/wiki/Package-migration-to-1.0.0
version: 0.4.3+3
version: 0.5.0

flutter:
plugin:
Expand All @@ -29,8 +29,8 @@ dev_dependencies:
sdk: flutter
integration_test:
path: ../integration_test
pedantic: ^1.8.0
pedantic: ^1.10.0-nullsafety.3

environment:
sdk: ">=2.1.0 <3.0.0"
sdk: ">=2.12.0-0 <3.0.0"
flutter: ">=1.12.13+hotfix.5"
2 changes: 1 addition & 1 deletion packages/package_info/test/package_info_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ void main() {

const MethodChannel channel =
MethodChannel('plugins.flutter.io/package_info');
List<MethodCall> log;
late List<MethodCall> log;

channel.setMockMethodCallHandler((MethodCall methodCall) async {
log.add(methodCall);
Expand Down