Split Android license checks into their own subvalidator (#22446)

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

@@ -19,6 +19,7 @@ import 'android_sdk.dart';
 
 AndroidWorkflow get androidWorkflow => context[AndroidWorkflow];
 AndroidValidator get androidValidator => context[AndroidValidator];
+AndroidLicenseValidator get androidLicenseValidator => context[AndroidLicenseValidator];
 
 enum LicensesAccepted {
   none,
@@ -52,7 +53,7 @@ class AndroidValidator extends DoctorValidator {
 
   /// Returns false if we cannot determine the Java version or if the version
   /// is not compatible.
-  bool _checkJavaVersion(String javaBinary, List<ValidationMessage> messages) {
+  Future<bool> _checkJavaVersion(String javaBinary, List<ValidationMessage> messages) async {
     if (!processManager.canRun(javaBinary)) {
       messages.add(ValidationMessage.error('Cannot execute $javaBinary to determine the version'));
       return false;
@@ -60,12 +61,14 @@ class AndroidValidator extends DoctorValidator {
     String javaVersion;
     try {
       printTrace('java -version');
-      final ProcessResult result = processManager.runSync(<String>[javaBinary, '-version']);
+      final ProcessResult result = await processManager.run(<String>[javaBinary, '-version']);
       if (result.exitCode == 0) {
         final List<String> versionLines = result.stderr.split('\n');
         javaVersion = versionLines.length >= 2 ? versionLines[1] : versionLines[0];
       }
-    } catch (_) { /* ignore */ }
+    } catch (error) {
+      printTrace(error.toString());
+    }
     if (javaVersion == null) {
       // Could not determine the java version.
       messages.add(ValidationMessage.error('Could not determine java version'));
@@ -148,10 +151,31 @@ class AndroidValidator extends DoctorValidator {
     messages.add(ValidationMessage('Java binary at: $javaBinary'));
 
     // Check JDK version.
-    if (!_checkJavaVersion(javaBinary, messages)) {
+    if (! await _checkJavaVersion(javaBinary, messages)) {
       return ValidationResult(ValidationType.partial, messages, statusInfo: sdkVersionText);
     }
 
+    // Success.
+    return ValidationResult(ValidationType.installed, messages, statusInfo: sdkVersionText);
+  }
+}
+
+class AndroidLicenseValidator extends DoctorValidator {
+  AndroidLicenseValidator(): super('Android license subvalidator',);
+
+  @override
+  Future<ValidationResult> validate() async {
+    final List<ValidationMessage> messages = <ValidationMessage>[];
+
+    // Match pre-existing early termination behavior
+    if (androidSdk == null || androidSdk.latestVersion == null ||
+        androidSdk.validateSdkWellFormed().isNotEmpty ||
+        ! await _checkJavaVersionNoOutput()) {
+      return ValidationResult(ValidationType.missing, messages);
+    }
+
+    final String sdkVersionText = 'Android SDK ${androidSdk.latestVersion.buildToolsVersionName}';
+
     // Check for licenses.
     switch (await licensesAccepted) {
       case LicensesAccepted.all:
@@ -167,11 +191,34 @@ class AndroidValidator extends DoctorValidator {
         messages.add(ValidationMessage.error('Android license status unknown.'));
         return ValidationResult(ValidationType.partial, messages, statusInfo: sdkVersionText);
     }
-
-    // Success.
     return ValidationResult(ValidationType.installed, messages, statusInfo: sdkVersionText);
   }
 
+  Future<bool> _checkJavaVersionNoOutput() async {
+    final String javaBinary = AndroidSdk.findJavaBinary();
+    if (javaBinary == null) {
+      return false;
+    }
+    if (!processManager.canRun(javaBinary)) {
+      return false;
+    }
+    String javaVersion;
+    try {
+      final ProcessResult result = await processManager.run(<String>[javaBinary, '-version']);
+      if (result.exitCode == 0) {
+        final List<String> versionLines = result.stderr.split('\n');
+        javaVersion = versionLines.length >= 2 ? versionLines[1] : versionLines[0];
+      }
+    } catch (error) {
+      printTrace(error.toString());
+    }
+    if (javaVersion == null) {
+      // Could not determine the java version.
+      return false;
+    }
+    return true;
+  }
+
   Future<LicensesAccepted> get licensesAccepted async {
     LicensesAccepted status;
 

packages/flutter_tools/lib/src/context_runner.dart

@@ -48,6 +48,7 @@ Future<T> runInContext<T>(
       AndroidStudio: AndroidStudio.latestValid,
       AndroidWorkflow: () => AndroidWorkflow(),
       AndroidValidator: () => AndroidValidator(),
+      AndroidLicenseValidator: () => AndroidLicenseValidator(),
       Artifacts: () => CachedArtifacts(),
       AssetBundleFactory: () => AssetBundleFactory.defaultInstance,
       BotDetector: () => const BotDetector(),

packages/flutter_tools/lib/src/doctor.dart

@@ -48,7 +48,7 @@ class _DefaultDoctorValidatorsProvider implements DoctorValidatorsProvider {
       _validators.add(_FlutterValidator());
 
       if (androidWorkflow.appliesToHostPlatform)
-        _validators.add(androidValidator);
+        _validators.add(GroupedValidator(<DoctorValidator>[androidValidator, androidLicenseValidator]));
 
       if (iosWorkflow.appliesToHostPlatform)
         _validators.add(GroupedValidator(<DoctorValidator>[iosValidator, cocoapodsValidator]));
@@ -159,7 +159,7 @@ class Doctor {
   /// Print information about the state of installed tooling.
   Future<bool> diagnose({ bool androidLicenses = false, bool verbose = true }) async {
     if (androidLicenses)
-      return AndroidValidator.runLicenseManager();
+      return AndroidLicenseValidator.runLicenseManager();
 
     if (!verbose) {
       printStatus('Doctor summary (to see all details, run flutter doctor -v):');

packages/flutter_tools/test/android/android_workflow_test.dart

@@ -40,8 +40,8 @@ void main() {
   testUsingContext('licensesAccepted throws if cannot run sdkmanager', () async {
     processManager.succeed = false;
     when(sdk.sdkManagerPath).thenReturn('/foo/bar/sdkmanager');
-    final AndroidValidator androidValidator = AndroidValidator();
+    final AndroidLicenseValidator licenseValidator = AndroidLicenseValidator();
-    expect(androidValidator.licensesAccepted, throwsToolExit());
+    expect(licenseValidator.licensesAccepted, throwsToolExit());
   }, overrides: <Type, Generator>{
     AndroidSdk: () => sdk,
     FileSystem: () => fs,
@@ -52,8 +52,8 @@ void main() {
 
   testUsingContext('licensesAccepted handles garbage/no output', () async {
     when(sdk.sdkManagerPath).thenReturn('/foo/bar/sdkmanager');
-    final AndroidValidator androidValidator = AndroidValidator();
+    final AndroidLicenseValidator licenseValidator = AndroidLicenseValidator();
-    final LicensesAccepted result = await androidValidator.licensesAccepted;
+    final LicensesAccepted result = await licenseValidator.licensesAccepted;
     expect(result, equals(LicensesAccepted.unknown));
     expect(processManager.commands.first, equals('/foo/bar/sdkmanager'));
     expect(processManager.commands.last, equals('--licenses'));
@@ -72,8 +72,8 @@ void main() {
        'All SDK package licenses accepted.'
     ]);
 
-    final AndroidValidator androidValidator = AndroidValidator();
+    final AndroidLicenseValidator licenseValidator = AndroidLicenseValidator();
-    final LicensesAccepted result = await androidValidator.licensesAccepted;
+    final LicensesAccepted result = await licenseValidator.licensesAccepted;
     expect(result, equals(LicensesAccepted.all));
   }, overrides: <Type, Generator>{
     AndroidSdk: () => sdk,
@@ -91,8 +91,8 @@ void main() {
       'Review licenses that have not been accepted (y/N)?',
     ]);
 
-    final AndroidValidator androidValidator = AndroidValidator();
+    final AndroidLicenseValidator licenseValidator = AndroidLicenseValidator();
-    final LicensesAccepted result = await androidValidator.licensesAccepted;
+    final LicensesAccepted result = await licenseValidator.licensesAccepted;
     expect(result, equals(LicensesAccepted.some));
   }, overrides: <Type, Generator>{
     AndroidSdk: () => sdk,
@@ -110,8 +110,8 @@ void main() {
       'Review licenses that have not been accepted (y/N)?',
     ]);
 
-    final AndroidValidator androidValidator = AndroidValidator();
+    final AndroidLicenseValidator licenseValidator = AndroidLicenseValidator();
-    final LicensesAccepted result = await androidValidator.licensesAccepted;
+    final LicensesAccepted result = await licenseValidator.licensesAccepted;
     expect(result, equals(LicensesAccepted.none));
   }, overrides: <Type, Generator>{
     AndroidSdk: () => sdk,
@@ -125,7 +125,7 @@ void main() {
     when(sdk.sdkManagerPath).thenReturn('/foo/bar/sdkmanager');
     when(sdk.sdkManagerVersion).thenReturn('26.0.0');
 
-    expect(await AndroidValidator.runLicenseManager(), isTrue);
+    expect(await AndroidLicenseValidator.runLicenseManager(), isTrue);
   }, overrides: <Type, Generator>{
     AndroidSdk: () => sdk,
     FileSystem: () => fs,
@@ -138,7 +138,7 @@ void main() {
     when(sdk.sdkManagerPath).thenReturn('/foo/bar/sdkmanager');
     when(sdk.sdkManagerVersion).thenReturn('25.0.0');
 
-    expect(AndroidValidator.runLicenseManager(), throwsToolExit(message: 'To update, run'));
+    expect(AndroidLicenseValidator.runLicenseManager(), throwsToolExit(message: 'To update, run'));
   }, overrides: <Type, Generator>{
     AndroidSdk: () => sdk,
     FileSystem: () => fs,
@@ -151,7 +151,7 @@ void main() {
     when(sdk.sdkManagerPath).thenReturn('/foo/bar/sdkmanager');
     when(sdk.sdkManagerVersion).thenReturn(null);
 
-    expect(AndroidValidator.runLicenseManager(), throwsToolExit(message: 'To update, run'));
+    expect(AndroidLicenseValidator.runLicenseManager(), throwsToolExit(message: 'To update, run'));
   }, overrides: <Type, Generator>{
     AndroidSdk: () => sdk,
     FileSystem: () => fs,
@@ -164,7 +164,7 @@ void main() {
     when(sdk.sdkManagerPath).thenReturn('/foo/bar/sdkmanager');
     processManager.succeed = false;
 
-    expect(AndroidValidator.runLicenseManager(), throwsToolExit());
+    expect(AndroidLicenseValidator.runLicenseManager(), throwsToolExit());
   }, overrides: <Type, Generator>{
     AndroidSdk: () => sdk,
     FileSystem: () => fs,

packages/flutter_tools/test/src/context.dart

@@ -179,7 +179,7 @@ class MockDeviceManager implements DeviceManager {
   Future<List<String>> getDeviceDiagnostics() async => <String>[];
 }
 
-class MockAndroidWorkflowValidator extends AndroidValidator {
+class MockAndroidLicenseValidator extends AndroidLicenseValidator {
   @override
   Future<LicensesAccepted> get licensesAccepted async => LicensesAccepted.all;
 }
@@ -200,8 +200,8 @@ class MockDoctor extends Doctor {
   List<DoctorValidator> get validators {
     final List<DoctorValidator> superValidators = super.validators;
     return superValidators.map<DoctorValidator>((DoctorValidator v) {
-      if (v is AndroidValidator) {
+      if (v is AndroidLicenseValidator) {
-        return MockAndroidWorkflowValidator();
+        return MockAndroidLicenseValidator();
       }
       return v;
     }).toList();