doctor: make JDK validation message more descriptive (#157280)

Sameri11 · andrewkolos · web-flow · commit 4b46b8066173 · 2024-11-23T00:27:18.000Z
This PR attempts to improve clarity of androids section of `flutter doctor -v` output by providing explicit information about which JDK is being used and how to configure a different one if needed. ### Before ```console • Java binary at: /Users/user/Applications/Android Studio Ladybug Feature Drop 2024.2.2 Canary 2.app/Contents/jbr/Contents/Home/bin/java ``` ### After 1. When JDK is from Android Studio: ```console • Java binary at: /Users/users/Applications/Android Studio Ladybug Feature Drop 2024.2.2 Canary 2.app/Contents/jbr/Contents/Home/bin/java This is the JDK bundled with latest Android Studio installation To manually set a custom JDK path, use: `flutter config --jdk-dir="path/to/jdk"` ``` 2. When JDK is from JAVA_HOME env variable: ```console • Java binary at: /Users/user/Applications/Android Studio Ladybug Feature Drop 2024.2.2 Canary 2.app/Contents/jbr/Contents/Home/bin/java This JDK is specified by JAVA_HOME environment variable To manually set a custom JDK path, use: `flutter config --jdk-dir="path/to/jdk"` ``` 3. When path to JDK is set in flutter config: ```console • Java binary at: /Users/user/Applications/Android Studio Ladybug Feature Drop 2024.2.2 Canary 2.app/Contents/jbr/Contents/Home/bin/java This JDK was found in system PATH To change current JDK, run: `flutter config --jdk-dir="path/to/jdk"` ``` 4. When java binary is found in PATH (as fallback) ```console • Java binary at: /Users/user/Applications/Android Studio Ladybug Feature Drop 2024.2.2 Canary 2.app/Contents/jbr/Contents/Home/bin/java This JDK is specified in Flutter configuration To manually set a custom JDK path, use: `flutter config --jdk-dir="path/to/jdk"` ``` ### Motivation I think it's described in flutter/flutter#153156 (comment). TLDR; many developers struggle with Java-related issues and more verbose doctor's output will (presumably) improve DX in that part. fixes #153156 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord].  [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --------- Co-authored-by: Andrew Kolos <andrewrkolos@gmail.com>
diff --git a/packages/flutter_tools/lib/src/android/android_workflow.dart b/packages/flutter_tools/lib/src/android/android_workflow.dart
@@ -105,7 +105,12 @@ class AndroidValidator extends DoctorValidator {
         messages.add(ValidationMessage.error(_userMessages.androidMissingJdk));
         return false;
       }
-      messages.add(ValidationMessage(_userMessages.androidJdkLocation(_java!.binaryPath)));
+      messages.add(ValidationMessage(
+        _androidJdkLocationMessage(
+          _java!.binaryPath,
+          _java.javaSource,
+        ),
+      ));
       if (!_java.canRun()) {
         messages.add(ValidationMessage.error(_userMessages.androidCantRunJavaBinary(_java.binaryPath)));
         return false;
@@ -454,3 +459,26 @@ class AndroidLicenseValidator extends DoctorValidator {
     );
   }
 }
+
+String _androidJdkLocationMessage(String location, JavaSource source) {
+  final String setWithConfigBreadcrumb = switch (source) {
+    JavaSource.androidStudio || JavaSource.path || JavaSource.javaHome =>
+      'To manually set the JDK path, use: `flutter config --jdk-dir="path/to/jdk"`.',
+    JavaSource.flutterConfig =>
+      'To change the current JDK, run: `flutter config --jdk-dir="path/to/jdk"`.'
+  };
+  final String sourceMessagePart = switch (source) {
+    JavaSource.androidStudio =>
+      'This is the JDK bundled with the latest Android Studio installation on this machine.',
+    JavaSource.javaHome =>
+      'This JDK is specified by the JAVA_HOME environment variable.',
+    JavaSource.path =>
+      'This JDK was found in the system PATH.',
+    JavaSource.flutterConfig =>
+      'This JDK is specified in your Flutter configuration.',
+  };
+
+  return 'Java binary at: $location\n'
+  '$sourceMessagePart\n'
+  '$setWithConfigBreadcrumb';
+}
diff --git a/packages/flutter_tools/lib/src/android/java.dart b/packages/flutter_tools/lib/src/android/java.dart
@@ -15,11 +15,25 @@ import 'android_studio.dart';
 
 const String _javaExecutable = 'java';
 
+enum JavaSource {
+  /// JDK bundled with latest Android Studio installation.
+  androidStudio,
+  /// JDK specified by the system's JAVA_HOME environment variable.
+  javaHome,
+  /// JDK available through the system's PATH environment variable.
+  path,
+  /// JDK specified in Flutter's configuration.
+  flutterConfig,
+}
+
+typedef _JavaHomePathWithSource = ({String path, JavaSource source});
+
 /// Represents an installation of Java.
 class Java {
   Java({
     required this.javaHome,
     required this.binaryPath,
+    required this.javaSource,
     required Logger logger,
     required FileSystem fileSystem,
     required OperatingSystemUtils os,
@@ -65,15 +79,15 @@ class Java {
       platform: platform,
       processManager: processManager
     );
-    final String? home = _findJavaHome(
+    final _JavaHomePathWithSource? home = _findJavaHome(
       config: config,
       logger: logger,
       androidStudio: androidStudio,
       platform: platform
     );
     final String? binary = _findJavaBinary(
       logger: logger,
-      javaHome: home,
+      javaHome: home?.path,
       fileSystem: fileSystem,
       operatingSystemUtils: os,
       platform: platform
@@ -83,9 +97,14 @@ class Java {
       return null;
     }
 
+    // If javaHome == null and binary is not null, it means that
+    // binary obtained from PATH as fallback.
+    final JavaSource javaSource = home?.source ?? JavaSource.path;
+
     return Java(
-      javaHome: home,
+      javaHome: home?.path,
       binaryPath: binary,
+      javaSource: javaSource,
       logger: logger,
       fileSystem: fileSystem,
       os: os,
@@ -110,6 +129,12 @@ class Java {
   /// to this class instead.
   final String binaryPath;
 
+  /// Indicates the source from where the Java runtime was located.
+  ///
+  /// This information is useful for debugging and logging purposes to track
+  /// which source was used to locate the Java runtime environment.
+  final JavaSource javaSource;
+
   final Logger _logger;
   final FileSystem _fileSystem;
   final OperatingSystemUtils _os;
@@ -192,25 +217,25 @@ class Java {
   }
 }
 
-String? _findJavaHome({
+_JavaHomePathWithSource? _findJavaHome({
   required Config config,
   required Logger logger,
   required AndroidStudio? androidStudio,
   required Platform platform,
 }) {
   final Object? configured = config.getValue('jdk-dir');
   if (configured != null) {
-    return configured as String;
+    return (path: configured as String, source: JavaSource.flutterConfig);
   }
 
   final String? androidStudioJavaPath = androidStudio?.javaPath;
   if (androidStudioJavaPath != null) {
-    return androidStudioJavaPath;
+    return (path: androidStudioJavaPath, source: JavaSource.androidStudio);
   }
 
   final String? javaHomeEnv = platform.environment[Java.javaHomeEnvironmentVariable];
   if (javaHomeEnv != null) {
-    return javaHomeEnv;
+    return (path: javaHomeEnv, source: JavaSource.javaHome);
   }
   return null;
 }
diff --git a/packages/flutter_tools/lib/src/base/user_messages.dart b/packages/flutter_tools/lib/src/base/user_messages.dart
@@ -115,7 +115,6 @@ class UserMessages {
       'No Java Development Kit (JDK) found; You must have the environment '
       'variable JAVA_HOME set and the java binary in your PATH. '
       'You can download the JDK from https://www.oracle.com/technetwork/java/javase/downloads/.';
-  String androidJdkLocation(String location) => 'Java binary at: $location';
   String get androidLicensesAll => 'All Android licenses accepted.';
   String get androidLicensesSome => 'Some Android licenses not accepted. To resolve this, run: flutter doctor --android-licenses';
   String get androidLicensesNone => 'Android licenses not accepted. To resolve this, run: flutter doctor --android-licenses';
diff --git a/packages/flutter_tools/test/general.shard/android/android_workflow_test.dart b/packages/flutter_tools/test/general.shard/android/android_workflow_test.dart
@@ -637,6 +637,146 @@ Android sdkmanager tool was found, but failed to run
     expect(processManager, hasNoRemainingExpectations);
     expect(stdio.stderr.getAndClear(), contains('UnsupportedClassVersionError'));
   });
+
+  testWithoutContext('Mentions that JDK is provided by latest Android Studio Installation', () async {
+    // Mock a pass through scenario to reach _checkJavaVersion()
+    sdk
+      ..licensesAvailable = true
+      ..platformToolsAvailable = true
+      ..cmdlineToolsAvailable = true
+      ..directory = fileSystem.directory('/foo/bar')
+      ..sdkManagerPath = '/foo/bar/sdkmanager';
+
+    final ValidationResult validationResult = await AndroidValidator(
+      java: FakeJava(),
+      androidSdk: sdk,
+      logger: logger,
+      platform: FakePlatform(),
+      userMessages: UserMessages()
+    ).validate();
+
+    expect(
+      validationResult.messages.any(
+        (ValidationMessage message) => message.message.contains(
+          'This is the JDK bundled with the latest Android Studio installation on this machine.'
+        )
+      ),
+      true,
+    );
+    expect(
+      validationResult.messages.any(
+        (ValidationMessage message) => message.message.contains(
+          'To manually set the JDK path, use: `flutter config --jdk-dir="path/to/jdk"`.'
+        )
+      ),
+      true,
+    );
+  });
+
+  testWithoutContext("Mentions that JDK is provided by user's JAVA_HOME environment variable", () async {
+    // Mock a pass through scenario to reach _checkJavaVersion()
+    sdk
+      ..licensesAvailable = true
+      ..platformToolsAvailable = true
+      ..cmdlineToolsAvailable = true
+      ..directory = fileSystem.directory('/foo/bar')
+      ..sdkManagerPath = '/foo/bar/sdkmanager';
+
+    final ValidationResult validationResult = await AndroidValidator(
+      java: FakeJava(javaSource: JavaSource.javaHome),
+      androidSdk: sdk,
+      logger: logger,
+      platform: FakePlatform(),
+      userMessages: UserMessages()
+    ).validate();
+
+    expect(
+      validationResult.messages.any(
+        (ValidationMessage message) => message.message.contains(
+          'This JDK is specified by the JAVA_HOME environment variable.'
+        )
+      ),
+      true,
+    );
+    expect(
+      validationResult.messages.any(
+        (ValidationMessage message) => message.message.contains(
+          'To manually set the JDK path, use: `flutter config --jdk-dir="path/to/jdk"`'
+        )
+      ),
+      true,
+    );
+  });
+
+  testWithoutContext('Mentions that path to Java binary is obtained from PATH', () async {
+    // Mock a pass through scenario to reach _checkJavaVersion()
+    sdk
+      ..licensesAvailable = true
+      ..platformToolsAvailable = true
+      ..cmdlineToolsAvailable = true
+      ..directory = fileSystem.directory('/foo/bar')
+      ..sdkManagerPath = '/foo/bar/sdkmanager';
+
+    final ValidationResult validationResult = await AndroidValidator(
+      java: FakeJava(javaSource: JavaSource.path),
+      androidSdk: sdk,
+      logger: logger,
+      platform: FakePlatform(),
+      userMessages: UserMessages()
+    ).validate();
+
+    expect(
+      validationResult.messages.any(
+        (ValidationMessage message) => message.message.contains(
+          'This JDK was found in the system PATH.'
+        )
+      ),
+      true,
+    );
+    expect(
+      validationResult.messages.any(
+        (ValidationMessage message) => message.message.contains(
+          'To manually set the JDK path, use: `flutter config --jdk-dir="path/to/jdk"`.'
+        )
+      ),
+      true,
+    );
+  });
+
+  testWithoutContext('Mentions that JDK is provided by Flutter config', () async {
+    // Mock a pass through scenario to reach _checkJavaVersion()
+    sdk
+      ..licensesAvailable = true
+      ..platformToolsAvailable = true
+      ..cmdlineToolsAvailable = true
+      ..directory = fileSystem.directory('/foo/bar')
+      ..sdkManagerPath = '/foo/bar/sdkmanager';
+
+    final ValidationResult validationResult = await AndroidValidator(
+      java: FakeJava(javaSource: JavaSource.flutterConfig),
+      androidSdk: sdk,
+      logger: logger,
+      platform: FakePlatform(),
+      userMessages: UserMessages()
+    ).validate();
+
+    expect(
+      validationResult.messages.any(
+        (ValidationMessage message) => message.message.contains(
+          'This JDK is specified in your Flutter configuration.'
+        )
+      ),
+      true,
+    );
+    expect(
+      validationResult.messages.any(
+        (ValidationMessage message) => message.message.contains(
+          'To change the current JDK, run: `flutter config --jdk-dir="path/to/jdk"`.'
+        )
+      ),
+      true,
+    );
+  });
 }
 
 class FakeAndroidSdk extends Fake implements AndroidSdk {
diff --git a/packages/flutter_tools/test/general.shard/android/java_test.dart b/packages/flutter_tools/test/general.shard/android/java_test.dart
@@ -44,6 +44,7 @@ void main() {
         final AndroidStudio androidStudio = _FakeAndroidStudioWithJdk();
         final String androidStudioBundledJdkHome = androidStudio.javaPath!;
         final String expectedJavaBinaryPath = fs.path.join(androidStudioBundledJdkHome, 'bin', 'java');
+        const JavaSource expectedJavaHomeSource = JavaSource.androidStudio;
 
         processManager.addCommand(FakeCommand(
           command: <String>[
@@ -70,12 +71,14 @@ OpenJDK 64-Bit Server VM Zulu19.32+15-CA (build 19.0.2+7, mixed mode, sharing)
 
         expect(java.version!.toString(), 'OpenJDK Runtime Environment Zulu19.32+15-CA (build 19.0.2+7)');
         expect(java.version, equals(Version(19, 0, 2)));
+        expect(java.javaSource, expectedJavaHomeSource);
       });
 
       testWithoutContext('finds JAVA_HOME if it is set and the JDK bundled with Android Studio could not be found', () {
         final AndroidStudio androidStudio = _FakeAndroidStudioWithoutJdk();
         const String javaHome = '/java/home';
         final String expectedJavaBinaryPath = fs.path.join(javaHome, 'bin', 'java');
+        const JavaSource expectedJavaHomeSource = JavaSource.javaHome;
 
         final Java java = Java.find(
           config: config,
@@ -90,11 +93,14 @@ OpenJDK 64-Bit Server VM Zulu19.32+15-CA (build 19.0.2+7, mixed mode, sharing)
 
         expect(java.javaHome, javaHome);
         expect(java.binaryPath, expectedJavaBinaryPath);
+        expect(java.javaSource, expectedJavaHomeSource);
       });
 
       testWithoutContext('returns the java binary found on PATH if no other can be found', () {
         final AndroidStudio androidStudio = _FakeAndroidStudioWithoutJdk();
         final OperatingSystemUtils os = _FakeOperatingSystemUtilsWithJava(fileSystem);
+        const JavaSource expectedJavaHomeSource = JavaSource.path;
+
 
         processManager.addCommand(
           const FakeCommand(
@@ -114,6 +120,7 @@ OpenJDK 64-Bit Server VM Zulu19.32+15-CA (build 19.0.2+7, mixed mode, sharing)
 
         expect(java.javaHome, isNull);
         expect(java.binaryPath, os.which('java')!.path);
+        expect(java.javaSource, expectedJavaHomeSource);
       });
 
       testWithoutContext('returns null if no java could be found', () {
@@ -138,6 +145,7 @@ OpenJDK 64-Bit Server VM Zulu19.32+15-CA (build 19.0.2+7, mixed mode, sharing)
       testWithoutContext('finds and prefers JDK found at config item "jdk-dir" if it is set', () {
         const String configuredJdkPath = '/jdk';
         config.setValue('jdk-dir', configuredJdkPath);
+        JavaSource expectedJavaHomeSource = JavaSource.flutterConfig;
 
         processManager.addCommand(
           const FakeCommand(
@@ -164,9 +172,12 @@ OpenJDK 64-Bit Server VM Zulu19.32+15-CA (build 19.0.2+7, mixed mode, sharing)
         expect(java, isNotNull);
         expect(java!.javaHome, configuredJdkPath);
         expect(java.binaryPath, fs.path.join(configuredJdkPath, 'bin', 'java'));
+        expect(java.javaSource, expectedJavaHomeSource);
 
         config.removeValue('jdk-dir');
 
+        expectedJavaHomeSource = JavaSource.androidStudio;
+
         java = Java.find(
           config: config,
           androidStudio: androidStudio,
@@ -180,6 +191,7 @@ OpenJDK 64-Bit Server VM Zulu19.32+15-CA (build 19.0.2+7, mixed mode, sharing)
         assert(androidStudio.javaPath != configuredJdkPath);
         expect(java!.javaHome, androidStudio.javaPath);
         expect(java.binaryPath, fs.path.join(androidStudio.javaPath!, 'bin', 'java'));
+        expect(java.javaSource, expectedJavaHomeSource);
       });
     });
 
@@ -196,6 +208,7 @@ OpenJDK 64-Bit Server VM Zulu19.32+15-CA (build 19.0.2+7, mixed mode, sharing)
           processManager: processManager,
           binaryPath: 'javaHome/bin/java',
           javaHome: 'javaHome',
+          javaSource: JavaSource.javaHome,
         );
       });
 
diff --git a/packages/flutter_tools/test/src/fakes.dart b/packages/flutter_tools/test/src/fakes.dart
@@ -670,6 +670,7 @@ class FakeAndroidStudio extends Fake implements AndroidStudio {
 class FakeJava extends Fake implements Java {
   FakeJava({
     this.javaHome = '/android-studio/jbr',
+    this.javaSource = JavaSource.androidStudio,
     String binary = '/android-studio/jbr/bin/java',
     Version? version,
     bool canRun = true,
@@ -687,6 +688,9 @@ class FakeJava extends Fake implements Java {
   @override
   String binaryPath;
 
+  @override
+  JavaSource javaSource;
+
   final Map<String, String> _environment;
   final bool _canRun;
 
