[flutter_tools] reland: avoid creating Android Devices if AndroidSDK cannot be found (#64665)

Avoid creating AndroidDevice discovery if the SDK cannot be located. Previously the tool would use which/where adb, however this required us to handle the AndroidSdk class being potentially null - which required an additional layer of indirection around all access. Sometimes these were forgotten leading to NPEs.

In general, not much can be done with an Android Device if the actual SDK is not installed.

Reland with fixed code + tests for null SDK + adb in AndroidDeviceDiscovery

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

@@ -255,7 +255,7 @@ class AndroidDevice extends Device {
   AndroidDevicePortForwarder _portForwarder;
 
   List<String> adbCommandForDevice(List<String> args) {
-    return <String>[getAdbPath(_androidSdk), '-s', id, ...args];
+    return <String>[_androidSdk.adbPath, '-s', id, ...args];
   }
 
   String runAdbCheckedSync(
@@ -318,13 +318,13 @@ class AndroidDevice extends Device {
 
     try {
       final RunResult adbVersion = await _processUtils.run(
-        <String>[getAdbPath(_androidSdk), 'version'],
+        <String>[_androidSdk.adbPath, 'version'],
         throwOnError: true,
       );
       if (_isValidAdbVersion(adbVersion.stdout)) {
         return true;
       }
-      _logger.printError('The ADB at "${getAdbPath(_androidSdk)}" is too old; please install version 1.0.39 or later.');
+      _logger.printError('The ADB at "${_androidSdk.adbPath}" is too old; please install version 1.0.39 or later.');
     } on Exception catch (error, trace) {
       _logger.printError('Error running ADB: $error', stackTrace: trace);
     }
@@ -339,7 +339,7 @@ class AndroidDevice extends Device {
       //   adb server is out of date.  killing..
       //   * daemon started successfully *
       await _processUtils.run(
-        <String>[getAdbPath(_androidSdk), 'start-server'],
+        <String>[_androidSdk.adbPath, 'start-server'],
         throwOnError: true,
       );
 

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

@@ -55,14 +55,13 @@ class AndroidDevices extends PollingDeviceDiscovery {
 
   @override
   Future<List<Device>> pollingGetDevices({ Duration timeout }) async {
-    final String adbPath = getAdbPath(_androidSdk);
-    if (adbPath == null) {
+    if (_androidSdk == null || _androidSdk.adbPath == null) {
       return <AndroidDevice>[];
     }
     String text;
     try {
       text = (await _processUtils.run(
-        <String>[adbPath, 'devices', '-l'],
+        <String>[_androidSdk.adbPath, 'devices', '-l'],
         throwOnError: true,
       )).stdout.trim();
     } on ArgumentError catch (exception) {
@@ -88,12 +87,11 @@ class AndroidDevices extends PollingDeviceDiscovery {
 
   @override
   Future<List<String>> getDiagnostics() async {
-    final String adbPath = getAdbPath(_androidSdk);
-    if (adbPath == null) {
+    if (_androidSdk == null || _androidSdk.adbPath == null) {
       return <String>[];
     }
 
-    final RunResult result = await _processUtils.run(<String>[adbPath, 'devices', '-l']);
+    final RunResult result = await _processUtils.run(<String>[_androidSdk.adbPath, 'devices', '-l']);
     if (result.exitCode != 0) {
       return <String>[];
     } else {

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

@@ -22,20 +22,6 @@ const String kAndroidSdkRoot = 'ANDROID_SDK_ROOT';
 final RegExp _numberedAndroidPlatformRe = RegExp(r'^android-([0-9]+)$');
 final RegExp _sdkVersionRe = RegExp(r'^ro.build.version.sdk=([0-9]+)$');
 
-/// Locate ADB. Prefer to use one from an Android SDK, if we can locate that.
-/// This should be used over accessing androidSdk.adbPath directly because it
-/// will work for those users who have Android Platform Tools installed but
-/// not the full SDK.
-String getAdbPath(AndroidSdk existingSdk) {
-  if (existingSdk?.adbPath != null) {
-    return existingSdk.adbPath;
-  }
-  if (existingSdk?.latestVersion == null) {
-    return globals.os.which('adb')?.path;
-  }
-  return existingSdk?.adbPath;
-}
-
 // Android SDK layout:
 
 // $ANDROID_SDK_ROOT/platform-tools/adb

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

@@ -58,13 +58,13 @@ class AndroidWorkflow implements Workflow {
   bool get appliesToHostPlatform => _featureFlags.isAndroidEnabled;
 
   @override
-  bool get canListDevices => getAdbPath(_androidSdk) != null;
+  bool get canListDevices => _androidSdk != null && _androidSdk.adbPath != null;
 
   @override
   bool get canLaunchDevices => _androidSdk != null && _androidSdk.validateSdkWellFormed().isEmpty;
 
   @override
-  bool get canListEmulators => _androidSdk.emulatorPath != null;
+  bool get canListEmulators => _androidSdk != null && _androidSdk.emulatorPath != null;
 }
 
 class AndroidValidator extends DoctorValidator {

packages/flutter_tools/test/general.shard/android/android_device_discovery_test.dart

@@ -17,7 +17,7 @@ import '../../src/fake_process_manager.dart';
 import '../../src/testbed.dart';
 
 void main() {
-  testWithoutContext('AndroidDevices returns empty device list on null adb', () async {
+  testWithoutContext('AndroidDevices returns empty device list and diagnostics on null adb', () async {
     final AndroidDevices androidDevices = AndroidDevices(
       androidSdk: MockAndroidSdk(null),
       logger: BufferLogger.test(),
@@ -31,7 +31,25 @@ void main() {
     );
 
     expect(await androidDevices.pollingGetDevices(), isEmpty);
-  }, skip: true); // a null adb unconditionally calls a static method in AndroidSDK that hits the context.
+    expect(await androidDevices.getDiagnostics(), isEmpty);
+  });
+
+  testWithoutContext('AndroidDevices returns empty device list and diagnostics on null Android SDK', () async {
+    final AndroidDevices androidDevices = AndroidDevices(
+      androidSdk: null,
+      logger: BufferLogger.test(),
+      androidWorkflow: AndroidWorkflow(
+        androidSdk: MockAndroidSdk(null),
+        featureFlags: TestFeatureFlags(),
+      ),
+      processManager: FakeProcessManager.list(<FakeCommand>[]),
+      fileSystem: MemoryFileSystem.test(),
+      platform: FakePlatform(),
+    );
+
+    expect(await androidDevices.pollingGetDevices(), isEmpty);
+    expect(await androidDevices.getDiagnostics(), isEmpty);
+  });
 
   testWithoutContext('AndroidDevices throwsToolExit on missing adb path', () {
     final ProcessManager processManager = FakeProcessManager.list(<FakeCommand>[

packages/flutter_tools/test/general.shard/android/android_workflow_test.dart

@@ -22,6 +22,7 @@ import 'package:process/process.dart';
 import '../../src/common.dart';
 import '../../src/context.dart';
 import '../../src/mocks.dart' show MockAndroidSdk, MockProcess, MockProcessManager, MockStdio;
+import '../../src/testbed.dart';
 
 class MockAndroidSdkVersion extends Mock implements AndroidSdkVersion {}
 class MockOperatingSystemUtils extends Mock implements OperatingSystemUtils {}
@@ -56,6 +57,17 @@ void main() {
     return (List<String> command) => MockProcess(stdout: stdoutStream);
   }
 
+  testWithoutContext('AndroidWorkflow handles a null AndroidSDK', () {
+    final AndroidWorkflow androidWorkflow = AndroidWorkflow(
+      featureFlags: TestFeatureFlags(),
+      androidSdk: null,
+    );
+
+    expect(androidWorkflow.canLaunchDevices, false);
+    expect(androidWorkflow.canListDevices, false);
+    expect(androidWorkflow.canListEmulators, false);
+  });
+
   testUsingContext('licensesAccepted returns LicensesAccepted.unknown if cannot find sdkmanager', () async {
     processManager.canRunSucceeds = false;
     when(sdk.sdkManagerPath).thenReturn('/foo/bar/sdkmanager');