Have android validator suggest ANDROID_SDK_ROOT instead of deprecated ANDROID_HOME (#49846)

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

@@ -3,15 +3,17 @@
 // found in the LICENSE file.
 
 import 'package:meta/meta.dart';
+import 'package:platform/platform.dart';
 
 import '../base/common.dart';
 import '../base/context.dart';
 import '../base/file_system.dart';
+import '../base/os.dart';
 import '../base/process.dart';
 import '../base/version.dart';
 import '../convert.dart';
 import '../globals.dart' as globals;
-import 'android_studio.dart' as android_studio;
+import 'android_studio.dart';
 
 AndroidSdk get androidSdk => context.get<AndroidSdk>();
 
@@ -549,20 +551,25 @@ class AndroidSdk {
   }
 
   /// First try Java bundled with Android Studio, then sniff JAVA_HOME, then fallback to PATH.
-  static String findJavaBinary() {
-    if (android_studio.javaPath != null) {
-      return globals.fs.path.join(android_studio.javaPath, 'bin', 'java');
+  static String findJavaBinary({
+    @required AndroidStudio androidStudio,
+    @required FileSystem fileSystem,
+    @required OperatingSystemUtils operatingSystemUtils,
+    @required Platform platform,
+  }) {
+    if (androidStudio?.javaPath != null) {
+      return fileSystem.path.join(androidStudio.javaPath, 'bin', 'java');
     }
 
-    final String javaHomeEnv = globals.platform.environment[_javaHomeEnvironmentVariable];
+    final String javaHomeEnv = platform.environment[_javaHomeEnvironmentVariable];
     if (javaHomeEnv != null) {
       // Trust JAVA_HOME.
-      return globals.fs.path.join(javaHomeEnv, 'bin', 'java');
+      return fileSystem.path.join(javaHomeEnv, 'bin', 'java');
     }
 
     // MacOS specific logic to avoid popping up a dialog window.
     // See: http://stackoverflow.com/questions/14292698/how-do-i-check-if-the-java-jdk-is-installed-on-mac.
-    if (globals.platform.isMacOS) {
+    if (platform.isMacOS) {
       try {
         final String javaHomeOutput = processUtils.runSync(
           <String>['/usr/libexec/java_home'],
@@ -573,14 +580,14 @@ class AndroidSdk {
           final List<String> javaHomeOutputSplit = javaHomeOutput.split('\n');
           if ((javaHomeOutputSplit != null) && (javaHomeOutputSplit.isNotEmpty)) {
             final String javaHome = javaHomeOutputSplit[0].trim();
-            return globals.fs.path.join(javaHome, 'bin', 'java');
+            return fileSystem.path.join(javaHome, 'bin', 'java');
           }
         }
       } catch (_) { /* ignore */ }
     }
 
     // Fallback to PATH based lookup.
-    return globals.os.which(_javaExecutable)?.path;
+    return operatingSystemUtils.which(_javaExecutable)?.path;
   }
 
   Map<String, String> _sdkManagerEnv;
@@ -590,7 +597,12 @@ class AndroidSdk {
     if (_sdkManagerEnv == null) {
       // If we can locate Java, then add it to the path used to run the Android SDK manager.
       _sdkManagerEnv = <String, String>{};
-      final String javaBinary = findJavaBinary();
+      final String javaBinary = findJavaBinary(
+        androidStudio: globals.androidStudio,
+        fileSystem: globals.fs,
+        operatingSystemUtils: globals.os,
+        platform: globals.platform,
+      );
       if (javaBinary != null) {
         _sdkManagerEnv['PATH'] = globals.fs.path.dirname(javaBinary) +
                                  globals.os.pathVarSeparator +

packages/flutter_tools/lib/src/cache.dart

@@ -101,7 +101,7 @@ class Cache {
   }) : _rootOverride = rootOverride,
        _logger = logger ?? globals.logger,
        _fileSystem = fileSystem ?? globals.fs,
-       _platform = platform ?? globals.platform ,
+       _platform = platform ?? globals.platform,
        _osUtils = osUtils ?? globals.os {
     // TODO(zra): Move to initializer list once logger and platform parameters
     // are required.

packages/flutter_tools/lib/src/context_runner.dart

@@ -69,7 +69,15 @@ Future<T> runInContext<T>(
       AndroidLicenseValidator: () => AndroidLicenseValidator(),
       AndroidSdk: AndroidSdk.locateAndroidSdk,
       AndroidStudio: AndroidStudio.latestValid,
-      AndroidValidator: () => AndroidValidator(),
+      AndroidValidator: () => AndroidValidator(
+        androidStudio: globals.androidStudio,
+        androidSdk: globals.androidSdk,
+        fileSystem: globals.fs,
+        logger: globals.logger,
+        platform: globals.platform,
+        processManager: globals.processManager,
+        userMessages: globals.userMessages,
+      ),
       AndroidWorkflow: () => AndroidWorkflow(),
       ApplicationPackageFactory: () => ApplicationPackageFactory(),
       Artifacts: () => CachedArtifacts(

packages/flutter_tools/lib/src/globals.dart

@@ -5,6 +5,8 @@
 import 'package:platform/platform.dart';
 import 'package:process/process.dart';
 
+import 'android/android_sdk.dart';
+import 'android/android_studio.dart';
 import 'artifacts.dart';
 import 'base/config.dart';
 import 'base/context.dart';
@@ -14,6 +16,7 @@ import 'base/io.dart';
 import 'base/logger.dart';
 import 'base/os.dart';
 import 'base/terminal.dart';
+import 'base/user_messages.dart';
 import 'cache.dart';
 import 'ios/mac.dart';
 import 'macos/xcode.dart';
@@ -53,9 +56,12 @@ const Platform _kLocalPlatform = LocalPlatform();
 
 Platform get platform => context.get<Platform>() ?? _kLocalPlatform;
 
-Xcode get xcode => context.get<Xcode>();
+AndroidStudio get androidStudio => context.get<AndroidStudio>();
+AndroidSdk get androidSdk => context.get<AndroidSdk>();
 FlutterVersion get flutterVersion => context.get<FlutterVersion>();
 IMobileDevice get iMobileDevice => context.get<IMobileDevice>();
+UserMessages get userMessages => context.get<UserMessages>();
+Xcode get xcode => context.get<Xcode>();
 
 XCDevice get xcdevice => context.get<XCDevice>();
 

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

@@ -7,12 +7,14 @@ import 'dart:async';
 import 'package:file/memory.dart';
 import 'package:flutter_tools/src/base/file_system.dart';
 import 'package:flutter_tools/src/base/io.dart';
+import 'package:flutter_tools/src/base/os.dart';
 import 'package:flutter_tools/src/android/android_sdk.dart';
 import 'package:flutter_tools/src/android/android_workflow.dart';
+import 'package:flutter_tools/src/base/logger.dart';
+import 'package:flutter_tools/src/base/terminal.dart' show AnsiTerminal, OutputPreferences;
 import 'package:flutter_tools/src/base/user_messages.dart';
 import 'package:flutter_tools/src/base/version.dart';
 import 'package:flutter_tools/src/doctor.dart';
-import 'package:flutter_tools/src/globals.dart' as globals;
 
 import 'package:mockito/mockito.dart';
 import 'package:platform/platform.dart';
@@ -23,19 +25,30 @@ import '../../src/context.dart';
 import '../../src/mocks.dart' show MockAndroidSdk, MockProcess, MockProcessManager, MockStdio;
 
 class MockAndroidSdkVersion extends Mock implements AndroidSdkVersion {}
+class MockOperatingSystemUtils extends Mock implements OperatingSystemUtils {}
 
 void main() {
   AndroidSdk sdk;
+  Logger logger;
   MemoryFileSystem fs;
   MockProcessManager processManager;
   MockStdio stdio;
+  UserMessages userMessages;
 
   setUp(() {
     sdk = MockAndroidSdk();
     fs = MemoryFileSystem();
     fs.directory('/home/me').createSync(recursive: true);
+    logger = BufferLogger(
+      terminal: AnsiTerminal(
+        stdio: null,
+        platform: const LocalPlatform(),
+      ),
+      outputPreferences: OutputPreferences.test(),
+    );
     processManager = MockProcessManager();
     stdio = MockStdio();
+    userMessages = UserMessages();
   });
 
   MockProcess Function(List<String>) processMetaFactory(List<String> stdout) {
@@ -208,24 +221,26 @@ void main() {
     Stdio: () => stdio,
   }));
 
-  testUsingContext('detects license-only SDK installation', () async {
+  testWithoutContext('detects license-only SDK installation', () async {
     when(sdk.licensesAvailable).thenReturn(true);
     when(sdk.platformToolsAvailable).thenReturn(false);
-    final ValidationResult validationResult = await AndroidValidator().validate();
+    final ValidationResult validationResult = await AndroidValidator(
+      androidStudio: null,
+      androidSdk: sdk,
+      fileSystem: fs,
+      logger: logger,
+      processManager: processManager,
+      platform: FakePlatform()..environment = <String, String>{'HOME': '/home/me'},
+      userMessages: userMessages,
+    ).validate();
     expect(validationResult.type, ValidationType.partial);
     expect(
       validationResult.messages.last.message,
       userMessages.androidSdkLicenseOnly(kAndroidHome),
     );
-  }, overrides: Map<Type, Generator>.unmodifiable(<Type, Generator>{
-    AndroidSdk: () => sdk,
-    FileSystem: () => fs,
-    ProcessManager: () => processManager,
-    Platform: () => FakePlatform()..environment = <String, String>{'HOME': '/home/me'},
-    Stdio: () => stdio,
-  }));
+  });
 
-  testUsingContext('detects minium required SDK and buildtools', () async {
+  testWithoutContext('detects minimum required SDK and buildtools', () async {
     final AndroidSdkVersion mockSdkVersion = MockAndroidSdkVersion();
     when(sdk.licensesAvailable).thenReturn(true);
     when(sdk.platformToolsAvailable).thenReturn(true);
@@ -236,13 +251,24 @@ void main() {
     when(sdk.sdkManagerPath).thenReturn('/foo/bar/sdkmanager');
     when(sdk.latestVersion).thenReturn(mockSdkVersion);
     when(sdk.validateSdkWellFormed()).thenReturn(<String>[]);
+    when(processManager.runSync(<String>['which', 'java'])).thenReturn(ProcessResult(123, 1, '', ''));
     final String errorMessage = userMessages.androidSdkBuildToolsOutdated(
       sdk.sdkManagerPath,
       kAndroidSdkMinVersion,
       kAndroidSdkBuildToolsMinVersion.toString(),
     );
 
-    ValidationResult validationResult = await AndroidValidator().validate();
+    final AndroidValidator androidValidator = AndroidValidator(
+      androidStudio: null,
+      androidSdk: sdk,
+      fileSystem: fs,
+      logger: logger,
+      processManager: processManager,
+      platform: FakePlatform()..environment = <String, String>{'HOME': '/home/me'},
+      userMessages: userMessages,
+    );
+
+    ValidationResult validationResult = await androidValidator.validate();
     expect(validationResult.type, ValidationType.missing);
     expect(
       validationResult.messages.last.message,
@@ -253,7 +279,7 @@ void main() {
     when(mockSdkVersion.sdkLevel).thenReturn(28);
     when(mockSdkVersion.buildToolsVersion).thenReturn(Version(28, 0, 2));
 
-    validationResult = await AndroidValidator().validate();
+    validationResult = await androidValidator.validate();
     expect(validationResult.type, ValidationType.missing);
     expect(
       validationResult.messages.last.message,
@@ -265,21 +291,15 @@ void main() {
     when(mockSdkVersion.sdkLevel).thenReturn(kAndroidSdkMinVersion);
     when(mockSdkVersion.buildToolsVersion).thenReturn(kAndroidSdkBuildToolsMinVersion);
 
-    validationResult = await AndroidValidator().validate();
+    validationResult = await androidValidator.validate();
     expect(validationResult.type, ValidationType.partial); // No Java binary
     expect(
       validationResult.messages.any((ValidationMessage message) => message.message == errorMessage),
       isFalse,
     );
-  }, overrides: Map<Type, Generator>.unmodifiable(<Type, Generator>{
-    AndroidSdk: () => sdk,
-    FileSystem: () => fs,
-    ProcessManager: () => processManager,
-    Platform: () => FakePlatform()..environment = <String, String>{'HOME': '/home/me'},
-    Stdio: () => stdio,
-  }));
+  });
 
-  testUsingContext('detects minimum required java version', () async {
+  testWithoutContext('detects minimum required java version', () async {
     final AndroidSdkVersion mockSdkVersion = MockAndroidSdkVersion();
 
     // Mock a pass through scenario to reach _checkJavaVersion()
@@ -293,22 +313,41 @@ void main() {
 
     //Test with older version of JDK
     const String javaVersionText = 'openjdk version "1.7.0_212"';
-    when(globals.processManager.run(argThat(contains('-version')))).thenAnswer((_) =>
+    when(processManager.run(argThat(contains('-version')))).thenAnswer((_) =>
       Future<ProcessResult>.value(ProcessResult(0, 0, null, javaVersionText)));
     final String errorMessage = userMessages.androidJavaMinimumVersion(javaVersionText);
 
-    final ValidationResult validationResult = await AndroidValidator().validate();
+    final ValidationResult validationResult = await AndroidValidator(
+      androidSdk: sdk,
+      androidStudio: null,
+      fileSystem: fs,
+      logger: logger,
+      platform: FakePlatform()..environment = <String, String>{'HOME': '/home/me', 'JAVA_HOME': 'home/java'},
+      processManager: processManager,
+      userMessages: userMessages,
+    ).validate();
     expect(validationResult.type, ValidationType.partial);
     expect(
       validationResult.messages.last.message,
       errorMessage,
     );
-  }, overrides: Map<Type, Generator>.unmodifiable(<Type, Generator>{
-    AndroidSdk: () => sdk,
-    FileSystem: () => fs,
-    Platform: () => FakePlatform()..environment = <String, String>{'HOME': '/home/me', 'JAVA_HOME': 'home/java'},
-    ProcessManager: () => processManager,
-    Stdio: () => stdio,
-  }));
+  });
 
+  testWithoutContext('Mentions `kAndroidSdkRoot if user has no AndroidSdk`', () async {
+    final ValidationResult validationResult = await AndroidValidator(
+      androidSdk: null,
+      androidStudio: null,
+      fileSystem: fs,
+      logger: logger,
+      platform: FakePlatform()..environment = <String, String>{'HOME': '/home/me', 'JAVA_HOME': 'home/java'},
+      processManager: processManager,
+      userMessages: userMessages,
+    ).validate();
+    expect(
+      validationResult.messages.any(
+        (ValidationMessage message) => message.message.contains(kAndroidSdkRoot)
+      ),
+      true,
+    );
+  });
 }