Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.
Merged
4 changes: 4 additions & 0 deletions packages/device_info/device_info/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 0.5.0-nullsafety

* Migrate to null safety.

## 0.4.2+9

* Update android compileSdkVersion to 29.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ import 'package:integration_test/integration_test.dart';
void main() {
IntegrationTestWidgetsFlutterBinding.ensureInitialized();

IosDeviceInfo iosInfo;
AndroidDeviceInfo androidInfo;
IosDeviceInfo? iosInfo;
AndroidDeviceInfo? androidInfo;

setUpAll(() async {
final DeviceInfoPlugin deviceInfoPlugin = DeviceInfoPlugin();
Expand All @@ -24,9 +24,9 @@ void main() {

testWidgets('Can get non-null device model', (WidgetTester tester) async {
if (Platform.isIOS) {
expect(iosInfo.model, isNotNull);
expect(iosInfo?.model, isNotNull);
} else if (Platform.isAndroid) {
expect(androidInfo.model, isNotNull);
expect(androidInfo?.model, isNotNull);
}
});
}
2 changes: 1 addition & 1 deletion packages/device_info/device_info/example/lib/main.dart
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class _MyAppState extends State<MyApp> {
}

Future<void> initPlatformState() async {
Map<String, dynamic> deviceData;
Map<String, dynamic> deviceData = <String, dynamic>{};

try {
if (Platform.isAndroid) {
Expand Down
5 changes: 4 additions & 1 deletion packages/device_info/device_info/example/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ dev_dependencies:
sdk: flutter
integration_test:
path: ../../../integration_test
pedantic: ^1.8.0
pedantic: ^1.10.0-nullsafety.1

flutter:
uses-material-design: true

environment:
sdk: '>=2.10.0-56.0.dev <3.0.0'
4 changes: 2 additions & 2 deletions packages/device_info/device_info/lib/device_info.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class DeviceInfoPlugin {
DeviceInfoPlugin();

/// This information does not change from call to call. Cache it.
AndroidDeviceInfo _cachedAndroidDeviceInfo;
AndroidDeviceInfo? _cachedAndroidDeviceInfo;

/// Information derived from `android.os.Build`.
///
Expand All @@ -25,7 +25,7 @@ class DeviceInfoPlugin {
await DeviceInfoPlatform.instance.androidInfo();

/// This information does not change from call to call. Cache it.
IosDeviceInfo _cachedIosDeviceInfo;
IosDeviceInfo? _cachedIosDeviceInfo;

/// Information derived from `UIDevice`.
///
Expand Down
14 changes: 9 additions & 5 deletions packages/device_info/device_info/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ homepage: https://github.com/flutter/plugins/tree/master/packages/device_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.2+9
version: 0.5.0-nullsafety

# Don't publish this package until null safety is stable or the package is in the allow list.
publish_to: none

flutter:
plugin:
Expand All @@ -19,14 +22,15 @@ flutter:
dependencies:
flutter:
sdk: flutter
device_info_platform_interface: ^1.0.0
device_info_platform_interface:
path: ../device_info_platform_interface

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We are going with git dependencies, but feel free to change it after this PR is merged.


dev_dependencies:
test: ^1.3.0
test: ^1.10.0-nullsafety.1
flutter_test:
sdk: flutter
pedantic: ^1.8.0
pedantic: ^1.10.0-nullsafety.1

environment:
sdk: ">=2.1.0<3.0.0"
sdk: '>=2.10.0-56.0.dev <3.0.0'
flutter: ">=1.12.13+hotfix.5 <2.0.0"
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 1.1.0-nullsafety

* Migrate to null safety.

## 1.0.1

- Documentation typo fixed.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,27 +8,27 @@
class AndroidDeviceInfo {
/// Android device Info class.
AndroidDeviceInfo({
this.version,
this.board,
this.bootloader,
this.brand,
this.device,
this.display,
this.fingerprint,
this.hardware,
this.host,
this.id,
this.manufacturer,
this.model,
this.product,
List<String> supported32BitAbis,
List<String> supported64BitAbis,
List<String> supportedAbis,
this.tags,
this.type,
this.isPhysicalDevice,
this.androidId,
List<String> systemFeatures,
required this.version,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We have forked this package internally and my migration followed a different path. I am not a fan of making all these fields required for two reasons:

  • Constructing the object (for tests) becomes painful. Consider an app test that only needs to set manufacturer. They now have to initialize all the other fields in their tests or use the obscure Map constructor.
  • Semantically, these fields are not required to exist. They can be empty. To reflect that, we are initializing them to empty string. As an app developer, I would much rather have a signal that a field is nullable rather than having it be an empty string which does not provide compile-time safety.

In my migration, I marked all these fields as nullable. However, this failed tests because gallery uses this code and assumes they are not nullable.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Semantically, these fields are not required to exist. They can be empty. To reflect that, we are initializing them to empty string.

If it's referring to usage of Map, this is a limitation of the implementation.

When looking at this, we were trying to answer the question of what is a valid AndroidDeviceInfo based on the information available from the platform. If something changes in future versions of Android, then that is a breaking change to the API exposed to Dart.

In my opinion, if everything is nullable by default, then the API becomes some sort of "best-effort". "we think version is available, but we can't guarantee that".

This was the rationale behind it. That said, could you point to the cases where testing was painful?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we were trying to answer the question of what is a valid AndroidDeviceInfo based on the information available from the platform.

That means these fields are not supposed to be nullable. In that case, it would be far better to construct them via map values like so:

   someField: map[key]!,
   // rather than
   // someField: map[key] ?? '',

The code, today, reads like any one of these values can be non-existent and that's OK.

The test pain is a secondary reason. If these are truly non-nullable values, you can always expose a forTest constructor that effectively does what your map constructor is doing.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

someField: map[key]! makes sense to me. We should fix it. This is an area where I think Pigeon could help, but that is a different discussion 😃

required this.board,
required this.bootloader,
required this.brand,
required this.device,
required this.display,
required this.fingerprint,
required this.hardware,
required this.host,
required this.id,
required this.manufacturer,
required this.model,
required this.product,
required List<String> supported32BitAbis,
required List<String> supported64BitAbis,
required List<String> supportedAbis,
required this.tags,
required this.type,
required this.isPhysicalDevice,
required this.androidId,
required List<String> systemFeatures,
}) : supported32BitAbis = List<String>.unmodifiable(supported32BitAbis),
supported64BitAbis = List<String>.unmodifiable(supported64BitAbis),
supportedAbis = List<String>.unmodifiable(supportedAbis),
Expand Down Expand Up @@ -115,25 +115,25 @@ class AndroidDeviceInfo {
return AndroidDeviceInfo(
version: AndroidBuildVersion._fromMap(
map['version']?.cast<String, dynamic>() ?? {}),
board: map['board'],
bootloader: map['bootloader'],
brand: map['brand'],
device: map['device'],
display: map['display'],
fingerprint: map['fingerprint'],
hardware: map['hardware'],
host: map['host'],
id: map['id'],
manufacturer: map['manufacturer'],
model: map['model'],
product: map['product'],
board: map['board'] ?? '',
bootloader: map['bootloader'] ?? '',
brand: map['brand'] ?? '',
device: map['device'] ?? '',
display: map['display'] ?? '',
fingerprint: map['fingerprint'] ?? '',
hardware: map['hardware'] ?? '',
host: map['host'] ?? '',
id: map['id'] ?? '',
manufacturer: map['manufacturer'] ?? '',
model: map['model'] ?? '',
product: map['product'] ?? '',
supported32BitAbis: _fromList(map['supported32BitAbis'] ?? []),
supported64BitAbis: _fromList(map['supported64BitAbis'] ?? []),
supportedAbis: _fromList(map['supportedAbis'] ?? []),
tags: map['tags'],
type: map['type'],
isPhysicalDevice: map['isPhysicalDevice'],
androidId: map['androidId'],
tags: map['tags'] ?? '',
type: map['type'] ?? '',
isPhysicalDevice: map['isPhysicalDevice'] ?? '',
androidId: map['androidId'] ?? '',
systemFeatures: _fromList(map['systemFeatures'] ?? []),
);
}
Expand All @@ -151,13 +151,13 @@ class AndroidDeviceInfo {
/// See: https://developer.android.com/reference/android/os/Build.VERSION.html
class AndroidBuildVersion {
AndroidBuildVersion._({
this.baseOS,
this.codename,
this.incremental,
this.previewSdkInt,
this.release,
this.sdkInt,
this.securityPatch,
required this.baseOS,
required this.codename,
required this.incremental,
required this.previewSdkInt,
required this.release,
required this.sdkInt,
required this.securityPatch,
});

/// The base OS build the product is based on.
Expand Down Expand Up @@ -186,13 +186,13 @@ class AndroidBuildVersion {
/// Deserializes from the map message received from [_kChannel].
static AndroidBuildVersion _fromMap(Map<String, dynamic> map) {
return AndroidBuildVersion._(
baseOS: map['baseOS'],
codename: map['codename'],
incremental: map['incremental'],
previewSdkInt: map['previewSdkInt'],
release: map['release'],
sdkInt: map['sdkInt'],
securityPatch: map['securityPatch'],
baseOS: map['baseOS'] ?? '',
codename: map['codename'] ?? '',
incremental: map['incremental'] ?? '',
previewSdkInt: map['previewSdkInt'] ?? '',
release: map['release'] ?? '',
sdkInt: map['sdkInt'] ?? '',
securityPatch: map['securityPatch'] ?? '',
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@
class IosDeviceInfo {
/// IOS device info class.
IosDeviceInfo({
this.name,
this.systemName,
this.systemVersion,
this.model,
this.localizedModel,
this.identifierForVendor,
this.isPhysicalDevice,
this.utsname,
required this.name,
required this.systemName,
required this.systemVersion,
required this.model,
required this.localizedModel,
required this.identifierForVendor,
required this.isPhysicalDevice,
required this.utsname,
});

/// Device name.
Expand Down Expand Up @@ -45,12 +45,12 @@ class IosDeviceInfo {
/// Deserializes from the map message received from [_kChannel].
static IosDeviceInfo fromMap(Map<String, dynamic> map) {
return IosDeviceInfo(
name: map['name'],
systemName: map['systemName'],
systemVersion: map['systemVersion'],
model: map['model'],
localizedModel: map['localizedModel'],
identifierForVendor: map['identifierForVendor'],
name: map['name'] ?? '',
systemName: map['systemName'] ?? '',
systemVersion: map['systemVersion'] ?? '',
model: map['model'] ?? '',
localizedModel: map['localizedModel'] ?? '',
identifierForVendor: map['identifierForVendor'] ?? '',
isPhysicalDevice: map['isPhysicalDevice'] == 'true',
utsname:
IosUtsname._fromMap(map['utsname']?.cast<String, dynamic>() ?? {}),
Expand All @@ -62,11 +62,11 @@ class IosDeviceInfo {
/// See http://pubs.opengroup.org/onlinepubs/7908799/xsh/sysutsname.h.html for details.
class IosUtsname {
IosUtsname._({
this.sysname,
this.nodename,
this.release,
this.version,
this.machine,
required this.sysname,
required this.nodename,
required this.release,
required this.version,
required this.machine,
});

/// Operating system name.
Expand All @@ -87,11 +87,11 @@ class IosUtsname {
/// Deserializes from the map message received from [_kChannel].
static IosUtsname _fromMap(Map<String, dynamic> map) {
return IosUtsname._(
sysname: map['sysname'],
nodename: map['nodename'],
release: map['release'],
version: map['version'],
machine: map['machine'],
sysname: map['sysname'] ?? '',
nodename: map['nodename'] ?? '',
release: map['release'] ?? '',
version: map['version'] ?? '',
machine: map['machine'] ?? '',
);
}
}
16 changes: 10 additions & 6 deletions packages/device_info/device_info_platform_interface/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,24 @@ description: A common platform interface for the device_info plugin.
homepage: https://github.com/flutter/plugins/tree/master/packages/device_info
# NOTE: We strongly prefer non-breaking changes, even at the expense of a
# less-clean API. See https://flutter.dev/go/platform-interface-breaking-changes
version: 1.0.1
version: 1.1.0-nullsafety

# Don't publish this package until null safety is stable or the package is in the allow list.
publish_to: none

dependencies:
flutter:
sdk: flutter
meta: ^1.1.8
plugin_platform_interface: ^1.0.2
meta: ^1.3.0-nullsafety.3
plugin_platform_interface:
path: ../../plugin_platform_interface

dev_dependencies:
flutter_test:
sdk: flutter
mockito: ^4.1.1
pedantic: ^1.8.0
test: ^1.10.0-nullsafety.1
pedantic: ^1.10.0-nullsafety.1

environment:
sdk: ">=2.7.0 <3.0.0"
sdk: '>=2.10.0-56.0.dev <3.0.0'
flutter: ">=1.9.1+hotfix.4 <2.0.0"
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ void main() {
TestWidgetsFlutterBinding.ensureInitialized();

group("$MethodChannelDeviceInfo", () {
MethodChannelDeviceInfo methodChannelDeviceInfo;
late MethodChannelDeviceInfo methodChannelDeviceInfo;

setUp(() async {
methodChannelDeviceInfo = MethodChannelDeviceInfo();
Expand Down