route device issue diagnostics to flutter doctor (#13346)

* route device issue diagnostics to flutter doctor

* review comments

* review comments

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

@@ -5,6 +5,8 @@
 import 'dart:async';
 import 'dart:convert';
 
+import 'package:meta/meta.dart';
+
 import '../android/android_sdk.dart';
 import '../android/android_workflow.dart';
 import '../android/apk.dart';
@@ -49,6 +51,9 @@ class AndroidDevices extends PollingDeviceDiscovery {
 
   @override
   Future<List<Device>> pollingGetDevices() async => getAdbDevices();
+
+  @override
+  Future<List<String>> getDiagnostics() async => getAdbDeviceDiagnostics();
 }
 
 class AndroidDevice extends Device {
@@ -530,30 +535,49 @@ Map<String, String> parseAdbDeviceProperties(String str) {
   return properties;
 }
 
-// 015d172c98400a03       device usb:340787200X product:nakasi model:Nexus_7 device:grouper
-final RegExp _kDeviceRegex = new RegExp(r'^(\S+)\s+(\S+)(.*)');
-
 /// Return the list of connected ADB devices.
-///
-/// [mockAdbOutput] is public for testing.
-List<AndroidDevice> getAdbDevices({ String mockAdbOutput }) {
+List<AndroidDevice> getAdbDevices() {
+  final String adbPath = getAdbPath(androidSdk);
+  if (adbPath == null)
+    return <AndroidDevice>[];
+  final String text = runSync(<String>[adbPath, 'devices', '-l']);
   final List<AndroidDevice> devices = <AndroidDevice>[];
-  String text;
-
-  if (mockAdbOutput == null) {
-    final String adbPath = getAdbPath(androidSdk);
-    printTrace('Listing devices using $adbPath');
-    if (adbPath == null)
-      return <AndroidDevice>[];
-    text = runSync(<String>[adbPath, 'devices', '-l']);
+  parseADBDeviceOutput(text, devices: devices);
+  return devices;
+}
+
+/// Get diagnostics about issues with any connected devices.
+Future<List<String>> getAdbDeviceDiagnostics() async {
+  final String adbPath = getAdbPath(androidSdk);
+  if (adbPath == null)
+    return <String>[];
+
+  final RunResult result = await runAsync(<String>[adbPath, 'devices', '-l']);
+  if (result.exitCode != 0) {
+    return <String>[];
   } else {
-    text = mockAdbOutput;
+    final String text = result.stdout;
+    final List<String> diagnostics = <String>[];
+    parseADBDeviceOutput(text, diagnostics: diagnostics);
+    return diagnostics;
   }
+}
+
+// 015d172c98400a03       device usb:340787200X product:nakasi model:Nexus_7 device:grouper
+final RegExp _kDeviceRegex = new RegExp(r'^(\S+)\s+(\S+)(.*)');
 
+/// Parse the given `adb devices` output in [text], and fill out the given list
+/// of devices and possible device issue diagnostics. Either argument can be null,
+/// in which case information for that parameter won't be populated.
+@visibleForTesting
+void parseADBDeviceOutput(String text, {
+  List<AndroidDevice> devices,
+  List<String> diagnostics
+}) {
   // Check for error messages from adb
   if (!text.contains('List of devices')) {
-    printError(text);
-    return <AndroidDevice>[];
+    diagnostics?.add(text);
+    return;
   }
 
   for (String line in text.trim().split('\n')) {
@@ -563,7 +587,7 @@ List<AndroidDevice> getAdbDevices({ String mockAdbOutput }) {
 
     // Skip lines about adb server and client version not matching
     if (line.startsWith(new RegExp(r'adb server (version|is out of date)'))) {
-      printStatus(line);
+      diagnostics?.add(line);
       continue;
     }
 
@@ -592,14 +616,14 @@ List<AndroidDevice> getAdbDevices({ String mockAdbOutput }) {
         info['model'] = cleanAdbDeviceName(info['model']);
 
       if (deviceState == 'unauthorized') {
-        printError(
+        diagnostics?.add(
           'Device $deviceID is not authorized.\n'
           'You might need to check your device for an authorization dialog.'
         );
       } else if (deviceState == 'offline') {
-        printError('Device $deviceID is offline.');
+        diagnostics?.add('Device $deviceID is offline.');
       } else {
-        devices.add(new AndroidDevice(
+        devices?.add(new AndroidDevice(
           deviceID,
           productID: info['product'],
           modelID: info['model'] ?? deviceID,
@@ -607,14 +631,12 @@ List<AndroidDevice> getAdbDevices({ String mockAdbOutput }) {
         ));
       }
     } else {
-      printError(
+      diagnostics?.add(
         'Unexpected failure parsing device information from adb output:\n'
         '$line\n'
         'Please report a bug at https://github.com/flutter/flutter/issues/new');
     }
   }
-
-  return devices;
 }
 
 /// A log reader that logs from `adb logcat`.

packages/flutter_tools/lib/src/base/logger.dart

@@ -252,7 +252,6 @@ enum _LogType {
   trace
 }
 
-
 class _AnsiStatus extends Status {
   _AnsiStatus(this.message, this.expectSlowOperation, this.onFinish) {
     stopwatch = new Stopwatch()..start();

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

@@ -34,6 +34,13 @@ class DevicesCommand extends FlutterCommand {
         'No devices detected.\n\n'
         'If you expected your device to be detected, please run "flutter doctor" to diagnose\n'
         'potential issues, or visit https://flutter.io/setup/ for troubleshooting tips.');
+      final List<String> diagnostics = await deviceManager.getDeviceDiagnostics();
+      if (diagnostics.isNotEmpty) {
+        printStatus('');
+        for (String diagnostic in diagnostics) {
+          printStatus('• ${diagnostic.replaceAll('\n', '\n  ')}');
+        }        
+      }
     } else {
       printStatus('${devices.length} connected ${pluralize('device', devices.length)}:\n');
       await Device.printDevices(devices);

packages/flutter_tools/lib/src/device.dart

@@ -93,6 +93,20 @@ class DeviceManager {
       }
     }
   }
+
+  /// Whether we're capable of listing any devices given the current environment configuration.
+  bool get canListAnything {
+    return _platformDiscoverers.any((DeviceDiscovery discoverer) => discoverer.canListAnything);
+  }
+
+  /// Get diagnostics about issues with any connected devices.
+  Future<List<String>> getDeviceDiagnostics() async {
+    final List<String> diagnostics = <String>[];
+    for (DeviceDiscovery discoverer in _platformDiscoverers) {
+      diagnostics.addAll(await discoverer.getDiagnostics());
+    }
+    return diagnostics;
+  }
 }
 
 /// An abstract class to discover and enumerate a specific type of devices.
@@ -104,6 +118,10 @@ abstract class DeviceDiscovery {
   bool get canListAnything;
 
   Future<List<Device>> get devices;
+
+  /// Gets a list of diagnostic messages pertaining to issues with any connected
+  /// devices (will be an empty list if there are no issues).
+  Future<List<String>> getDiagnostics() => new Future<List<String>>.value(<String>[]);
 }
 
 /// A [DeviceDiscovery] implementation that uses polling to discover device adds

packages/flutter_tools/lib/src/doctor.dart

@@ -48,7 +48,8 @@ class Doctor {
       else
         _validators.add(new NoIdeValidator());
 
-      _validators.add(new DeviceValidator());
+      if (deviceManager.canListAnything)
+        _validators.add(new DeviceValidator());
     }
     return _validators;
   }
@@ -501,12 +502,17 @@ class DeviceValidator extends DoctorValidator {
     final List<Device> devices = await deviceManager.getAllConnectedDevices().toList();
     List<ValidationMessage> messages;
     if (devices.isEmpty) {
-      messages = <ValidationMessage>[new ValidationMessage('None')];
+      final List<String> diagnostics = await deviceManager.getDeviceDiagnostics();
+      if (diagnostics.isNotEmpty) {
+        messages = diagnostics.map((String message) => new ValidationMessage(message)).toList();
+      } else {
+        messages = <ValidationMessage>[new ValidationMessage('None')];
+      }
     } else {
       messages = await Device.descriptions(devices)
           .map((String msg) => new ValidationMessage(msg)).toList();
     }
-    return new ValidationResult(ValidationType.installed, messages);
+    return new ValidationResult(devices.isEmpty ? ValidationType.partial : ValidationType.installed, messages);
   }
 }
 

packages/flutter_tools/test/android/android_device_test.dart

@@ -23,44 +23,50 @@ void main() {
 
   group('getAdbDevices', () {
     testUsingContext('physical devices', () {
-      final List<AndroidDevice> devices = getAdbDevices(mockAdbOutput: '''
+      final List<AndroidDevice> devices = <AndroidDevice>[];
+      parseADBDeviceOutput('''
 List of devices attached
 05a02bac               device usb:336592896X product:razor model:Nexus_7 device:flo
 
-''');
+''', devices: devices);
       expect(devices, hasLength(1));
       expect(devices.first.name, 'Nexus 7');
     });
 
     testUsingContext('emulators and short listings', () {
-      final List<AndroidDevice> devices = getAdbDevices(mockAdbOutput: '''
+      final List<AndroidDevice> devices = <AndroidDevice>[];
+      parseADBDeviceOutput('''
 List of devices attached
 localhost:36790        device
 0149947A0D01500C       device usb:340787200X
 emulator-5612          host features:shell_2
 
-''');
+''', devices: devices);
       expect(devices, hasLength(3));
       expect(devices.first.name, 'localhost:36790');
     });
 
     testUsingContext('android n', () {
-      final List<AndroidDevice> devices = getAdbDevices(mockAdbOutput: '''
+      final List<AndroidDevice> devices = <AndroidDevice>[];
+      parseADBDeviceOutput('''
 List of devices attached
 ZX1G22JJWR             device usb:3-3 product:shamu model:Nexus_6 device:shamu features:cmd,shell_v2
-''');
+''', devices: devices);
       expect(devices, hasLength(1));
       expect(devices.first.name, 'Nexus 6');
     });
 
     testUsingContext('adb error message', () {
-      final List<AndroidDevice> devices = getAdbDevices(mockAdbOutput: '''
+      final List<AndroidDevice> devices = <AndroidDevice>[];
+      final List<String> diagnostics = <String>[];
+      parseADBDeviceOutput('''
 It appears you do not have 'Android SDK Platform-tools' installed.
 Use the 'android' tool to install them:
     android update sdk --no-ui --filter 'platform-tools'
-''');
+''', devices: devices, diagnostics: diagnostics);
       expect(devices, hasLength(0));
-      expect(testLogger.errorText, contains('you do not have'));
+      expect(diagnostics, hasLength(1));
+      expect(diagnostics.first, contains('you do not have'));
     });
   });
 

packages/flutter_tools/test/src/context.dart

@@ -189,6 +189,12 @@ class MockDeviceManager implements DeviceManager {
   }
 
   void addDevice(Device device) => devices.add(device);
+
+  @override
+  bool get canListAnything => true;
+
+  @override
+  Future<List<String>> getDeviceDiagnostics() async => <String>[];
 }
 
 class MockDoctor extends Doctor {