Merge pull request #1656 from devoncarew/gen_refactoring

General flutter_tools refactoring

packages/flutter_tools/lib/flutter_tools.dart

@@ -2,16 +2,14 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-library flutter_tools;
-
 import 'dart:async';
 
 import 'package:archive/archive.dart';
 
 import 'src/flx.dart' as flx;
 
-/// Assembles a Flutter .flx file from a pre-existing manifest descriptor
-/// and a pre-compiled snapshot.
+/// Assembles a Flutter .flx file from a pre-existing manifest descriptor and a
+/// pre-compiled snapshot.
 Future<int> assembleFlx({
   Map manifestDescriptor: const {},
   ArchiveFile snapshotFile: null,

packages/flutter_tools/lib/src/android/adb.dart

@@ -19,7 +19,7 @@ class Adb {
 
   final String adbPath;
 
-  Map<String, String> _idToNameCache = <String, String>{};
+  final Map<String, String> _idToNameCache = <String, String>{};
 
   bool exists() {
     try {

packages/flutter_tools/lib/src/commands/apk.dart

@@ -366,7 +366,7 @@ int _buildApk(
     ensureDirectoryExists(finalApk.path);
     builder.align(unalignedApk, finalApk);
 
-    printStatus('APK generated: ${finalApk.path}');
+    printStatus('Generated APK to ${finalApk.path}.');
 
     return 0;
   } finally {
@@ -520,7 +520,7 @@ Future<ApplicationPackageStore> buildAll(
     // TODO(mpcomplete): Temporary hack. We only support the apk builder atm.
     if (package == applicationPackages.android) {
       if (!FileSystemEntity.isFileSync(_kDefaultAndroidManifestPath)) {
-        printStatus('Using pre-built SkyShell.apk');
+        printStatus('Using pre-built SkyShell.apk.');
         continue;
       }
 

packages/flutter_tools/lib/src/commands/daemon.dart

@@ -362,7 +362,7 @@ class AndroidDeviceDiscovery {
       if (androidDevice == null) {
         // device added
         androidDevice = new AndroidDevice(
-          id: device.id,
+          device.id,
           productID: device.productID,
           modelID: device.modelID,
           deviceCodeName: device.deviceCodeName,
@@ -385,11 +385,6 @@ class AndroidDeviceDiscovery {
     for (AndroidDevice device in currentDevices) {
       _devices.remove(device.id);
 
-      // I don't know the purpose of this cache or if it's a good idea. We should
-      // probably have a DeviceManager singleton class to coordinate known devices
-      // and different device discovery mechanisms.
-      Device.removeFromCache(device.id);
-
       removedController.add(device);
     }
   }
@@ -424,7 +419,7 @@ class IOSSimulatorDeviceDiscovery {
 
       if (androidDevice == null) {
         // device added
-        androidDevice = new IOSSimulator(id: device.udid, name: device.name);
+        androidDevice = new IOSSimulator(device.udid, name: device.name);
         _devices[androidDevice.id] = androidDevice;
         addedController.add(androidDevice);
       } else {
@@ -436,8 +431,6 @@ class IOSSimulatorDeviceDiscovery {
     for (IOSSimulator device in currentDevices) {
       _devices.remove(device.id);
 
-      Device.removeFromCache(device.id);
-
       removedController.add(device);
     }
   }

packages/flutter_tools/lib/src/device.dart

@@ -66,18 +66,9 @@ abstract class DeviceDiscovery {
 }
 
 abstract class Device {
-  final String id;
-  static Map<String, Device> _deviceCache = {};
-
-  static Device unique(String id, Device constructor(String id)) {
-    return _deviceCache.putIfAbsent(id, () => constructor(id));
-  }
-
-  static void removeFromCache(String id) {
-    _deviceCache.remove(id);
-  }
+  Device(this.id);
 
-  Device.fromId(this.id);
+  final String id;
 
   String get name;
 
@@ -133,6 +124,12 @@ abstract class DeviceLogReader {
 
 // TODO(devoncarew): Unify this with [DeviceManager].
 class DeviceStore {
+  DeviceStore({
+    this.android,
+    this.iOS,
+    this.iOSSimulator
+  });
+
   final AndroidDevice android;
   final IOSDevice iOS;
   final IOSSimulator iOSSimulator;
@@ -148,12 +145,6 @@ class DeviceStore {
     return result;
   }
 
-  DeviceStore({
-    this.android,
-    this.iOS,
-    this.iOSSimulator
-  });
-
   static Device _deviceForConfig(BuildConfiguration config, List<Device> devices) {
     Device device = null;
 
@@ -194,10 +185,6 @@ class DeviceStore {
         case TargetPlatform.iOSSimulator:
           assert(iOSSimulator == null);
           iOSSimulator = _deviceForConfig(config, IOSSimulator.getAttachedDevices());
-          if (iOSSimulator == null) {
-            // Creates a simulator with the default identifier
-            iOSSimulator = new IOSSimulator();
-          }
           break;
         case TargetPlatform.mac:
         case TargetPlatform.linux:

packages/flutter_tools/lib/src/flx.dart

@@ -33,8 +33,8 @@ Map<String, double> _kIconDensities = {
   'xxhdpi' : 3.0,
   'xxxhdpi' : 4.0
 };
-const List<String> _kThemes = const ['white', 'black'];
-const List<int> _kSizes = const [18, 24, 36, 48];
+const List<String> _kThemes = const <String>['white', 'black'];
+const List<int> _kSizes = const <int>[18, 24, 36, 48];
 
 class _Asset {
   final String source;

packages/flutter_tools/lib/src/ios/device_ios.dart

@@ -47,7 +47,19 @@ class IOSSimulatorDiscovery extends DeviceDiscovery {
 }
 
 class IOSDevice extends Device {
-  static final String defaultDeviceID = 'default_ios_id';
+  IOSDevice(String id, { this.name }) : super(id) {
+    _installerPath = _checkForCommand('ideviceinstaller');
+    _listerPath = _checkForCommand('idevice_id');
+    _informerPath = _checkForCommand('ideviceinfo');
+    _debuggerPath = _checkForCommand('idevicedebug');
+    _loggerPath = _checkForCommand('idevicesyslog');
+    _pusherPath = _checkForCommand(
+        'ios-deploy',
+        'To copy files to iOS devices, please install ios-deploy. '
+        'You can do this using homebrew as follows:\n'
+        '\$ brew tap flutter/flutter\n'
+        '\$ brew install ios-deploy');
+  }
 
   String _installerPath;
   String get installerPath => _installerPath;
@@ -67,50 +79,25 @@ class IOSDevice extends Device {
   String _pusherPath;
   String get pusherPath => _pusherPath;
 
-  String _name;
-  String get name => _name;
-
-  factory IOSDevice({String id, String name}) {
-    IOSDevice device = Device.unique(id ?? defaultDeviceID, (String id) => new IOSDevice.fromId(id));
-    device._name = name;
-    return device;
-  }
-
-  IOSDevice.fromId(String id) : super.fromId(id) {
-    _installerPath = _checkForCommand('ideviceinstaller');
-    _listerPath = _checkForCommand('idevice_id');
-    _informerPath = _checkForCommand('ideviceinfo');
-    _debuggerPath = _checkForCommand('idevicedebug');
-    _loggerPath = _checkForCommand('idevicesyslog');
-    _pusherPath = _checkForCommand(
-        'ios-deploy',
-        'To copy files to iOS devices, please install ios-deploy. '
-        'You can do this using homebrew as follows:\n'
-        '\$ brew tap flutter/flutter\n'
-        '\$ brew install ios-deploy');
-  }
+  final String name;
 
   static List<IOSDevice> getAttachedDevices([IOSDevice mockIOS]) {
     List<IOSDevice> devices = [];
     for (String id in _getAttachedDeviceIDs(mockIOS)) {
       String name = _getDeviceName(id, mockIOS);
-      devices.add(new IOSDevice(id: id, name: name));
+      devices.add(new IOSDevice(id, name: name));
     }
     return devices;
   }
 
   static Iterable<String> _getAttachedDeviceIDs([IOSDevice mockIOS]) {
-    String listerPath =
-        (mockIOS != null) ? mockIOS.listerPath : _checkForCommand('idevice_id');
-    String output;
+    String listerPath = (mockIOS != null) ? mockIOS.listerPath : _checkForCommand('idevice_id');
     try {
-      output = runSync([listerPath, '-l']);
+      String output = runSync([listerPath, '-l']);
+      return output.trim().split('\n').where((String s) => s != null && s.isNotEmpty);
     } catch (e) {
-      return [];
+      return <String>[];
     }
-    return output.trim()
-                 .split('\n')
-                 .where((String s) => s != null && s.length > 0);
   }
 
   static String _getDeviceName(String deviceID, [IOSDevice mockIOS]) {
@@ -142,11 +129,7 @@ class IOSDevice extends Device {
   @override
   bool installApp(ApplicationPackage app) {
     try {
-      if (id == defaultDeviceID) {
-        runCheckedSync([installerPath, '-i', app.localPath]);
-      } else {
-        runCheckedSync([installerPath, '-u', id, '-i', app.localPath]);
-      }
+      runCheckedSync([installerPath, '-i', app.localPath]);
       return true;
     } catch (e) {
       return false;
@@ -155,15 +138,7 @@ class IOSDevice extends Device {
   }
 
   @override
-  bool isConnected() {
-    Iterable<String> ids = _getAttachedDeviceIDs();
-    for (String id in ids) {
-      if (id == this.id || this.id == defaultDeviceID) {
-        return true;
-      }
-    }
-    return false;
-  }
+  bool isConnected() => _getAttachedDeviceIDs().contains(id);
 
   @override
   bool isAppInstalled(ApplicationPackage app) {
@@ -238,7 +213,7 @@ class IOSDevice extends Device {
 
   Future<bool> pushFile(ApplicationPackage app, String localFile, String targetFile) async {
     if (Platform.isMacOS) {
-      runSync([
+      runSync(<String>[
         pusherPath,
         '-t',
         '1',
@@ -263,22 +238,15 @@ class IOSDevice extends Device {
 }
 
 class IOSSimulator extends Device {
-  factory IOSSimulator({String id, String name}) {
-    IOSSimulator device = Device.unique(id, (String id) => new IOSSimulator.fromId(id));
-    device._name = name;
-    return device;
-  }
+  IOSSimulator(String id, { this.name }) : super(id);
 
   static List<IOSSimulator> getAttachedDevices() {
     return SimControl.getConnectedDevices().map((SimDevice device) {
-      return new IOSSimulator(id: device.udid, name: device.name);
+      return new IOSSimulator(device.udid, name: device.name);
     }).toList();
   }
 
-  IOSSimulator.fromId(String id) : super.fromId(id);
-
-  String _name;
-  String get name => _name;
+  final String name;
 
   String get xcrunPath => path.join('/usr', 'bin', 'xcrun');
 
@@ -378,7 +346,7 @@ class IOSSimulator extends Device {
       ApplicationPackage app, String localFile, String targetFile) async {
     if (Platform.isMacOS) {
       String simulatorHomeDirectory = _getSimulatorAppHomeDirectory(app);
-      runCheckedSync(['cp', localFile, path.join(simulatorHomeDirectory, targetFile)]);
+      runCheckedSync(<String>['cp', localFile, path.join(simulatorHomeDirectory, targetFile)]);
       return true;
     }
     return false;
@@ -413,7 +381,7 @@ class _IOSDeviceLogReader extends DeviceLogReader {
       return 2;
 
     return await runCommandAndStreamOutput(
-      [device.loggerPath],
+      <String>[device.loggerPath],
       prefix: '[$name] ',
       filter: new RegExp(r'(FlutterRunner|flutter.runner.Runner)')
     );
@@ -512,7 +480,7 @@ bool _checkXcodeVersion() {
   if (!Platform.isMacOS)
     return false;
   try {
-    String version = runCheckedSync(['xcodebuild', '-version']);
+    String version = runCheckedSync(<String>['xcodebuild', '-version']);
     Match match = _xcodeVersionRegExp.firstMatch(version);
     if (int.parse(match[1]) < 7) {
       printError('Found "${match[0]}". $_xcodeRequirement');
@@ -534,12 +502,12 @@ Future<bool> _buildIOSXcodeProject(ApplicationPackage app, bool isDevice) async
   if (!_checkXcodeVersion())
     return false;
 
-  List<String> commands = [
+  List<String> commands = <String>[
     '/usr/bin/env', 'xcrun', 'xcodebuild', '-target', 'Runner', '-configuration', 'Release'
   ];
 
   if (!isDevice) {
-    commands.addAll(['-sdk', 'iphonesimulator']);
+    commands.addAll(<String>['-sdk', 'iphonesimulator']);
   }
 
   try {

packages/flutter_tools/test/android_device_test.dart

@@ -9,22 +9,10 @@ main() => defineTests();
 
 defineTests() {
   group('android_device', () {
-    test('uses the correct default ID', () {
-      AndroidDevice android = new AndroidDevice();
-      expect(android.id, equals(AndroidDevice.defaultDeviceID));
-    });
-
     test('stores the requested id', () {
       String deviceId = '1234';
-      AndroidDevice android = new AndroidDevice(id: deviceId);
-      expect(android.id, equals(deviceId));
-    });
-
-    test('correctly creates only one of each requested device id', () {
-      String deviceID = '1234';
-      AndroidDevice a1 = new AndroidDevice(id: deviceID);
-      AndroidDevice a2 = new AndroidDevice(id: deviceID);
-      expect(a1, equals(a2));
+      AndroidDevice device = new AndroidDevice(deviceId);
+      expect(device.id, equals(deviceId));
     });
   });
 }