Validate bins on path in doctor (#113106)

Author: christopherfujino

packages/flutter_tools/lib/src/doctor.dart

@@ -512,7 +512,10 @@ class FlutterValidator extends DoctorValidator {
       versionChannel = version.channel;
       frameworkVersion = version.frameworkVersion;
 
-      messages.add(_getFlutterVersionMessage(frameworkVersion, versionChannel));
+      final String flutterRoot = _flutterRoot();
+      messages.add(_getFlutterVersionMessage(frameworkVersion, versionChannel, flutterRoot));
+
+      _validateRequiredBinaries(flutterRoot).forEach(messages.add);
       messages.add(_getFlutterUpstreamMessage(version));
       if (gitUrl != null) {
         messages.add(ValidationMessage(_userMessages.flutterGitUrl(gitUrl)));
@@ -575,8 +578,8 @@ class FlutterValidator extends DoctorValidator {
     );
   }
 
-  ValidationMessage _getFlutterVersionMessage(String frameworkVersion, String versionChannel) {
-    String flutterVersionMessage = _userMessages.flutterVersion(frameworkVersion, versionChannel, _flutterRoot());
+  ValidationMessage _getFlutterVersionMessage(String frameworkVersion, String versionChannel, String flutterRoot) {
+    String flutterVersionMessage = _userMessages.flutterVersion(frameworkVersion, versionChannel, flutterRoot);
 
     // The tool sets the channel as "unknown", if the current branch is on a
     // "detached HEAD" state or doesn't have an upstream, and sets the
@@ -594,6 +597,38 @@ class FlutterValidator extends DoctorValidator {
     return ValidationMessage.hint(flutterVersionMessage);
   }
 
+  List<ValidationMessage> _validateRequiredBinaries(String flutterRoot) {
+    final ValidationMessage? flutterWarning = _validateSdkBinary('flutter', flutterRoot);
+    final ValidationMessage? dartWarning = _validateSdkBinary('dart', flutterRoot);
+    return <ValidationMessage>[
+      if (flutterWarning != null) flutterWarning,
+      if (dartWarning != null) dartWarning,
+    ];
+  }
+
+  /// Return a warning if the provided [binary] on the user's path does not
+  /// resolve within the Flutter SDK.
+  ValidationMessage? _validateSdkBinary(String binary, String flutterRoot) {
+    final String flutterBinDir = _fileSystem.path.join(flutterRoot, 'bin');
+
+    final File? flutterBin = _operatingSystemUtils.which(binary);
+    if (flutterBin == null) {
+      return ValidationMessage.hint(
+        'The $binary binary is not on your path. Consider adding '
+        '$flutterBinDir to your path.',
+      );
+    }
+    final String resolvedFlutterPath = flutterBin.resolveSymbolicLinksSync();
+    if (!resolvedFlutterPath.contains(flutterRoot)) {
+      final String hint = 'Warning: `$binary` on your path resolves to '
+          '$resolvedFlutterPath, which is not inside your current Flutter '
+          'SDK checkout at $flutterRoot. Consider adding $flutterBinDir to '
+          'the front of your path.';
+      return ValidationMessage.hint(hint);
+    }
+    return null;
+  }
+
   ValidationMessage _getFlutterUpstreamMessage(FlutterVersion version) {
     final String? repositoryUrl = version.repositoryUrl;
     final VersionCheckError? upstreamValidationError = VersionUpstreamValidator(version: version, platform: _platform).run();

packages/flutter_tools/test/general.shard/flutter_validator_test.dart

@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+import 'package:file/file.dart';
 import 'package:file/memory.dart';
 import 'package:flutter_tools/src/artifacts.dart';
 import 'package:flutter_tools/src/base/os.dart';
@@ -384,10 +385,122 @@ void main() {
       ),
     ));
   });
+
+  testWithoutContext('detects no flutter and dart on path', () async {
+    final FlutterValidator flutterValidator = FlutterValidator(
+      platform: FakePlatform(localeName: 'en_US.UTF-8'),
+      flutterVersion: () => FakeFlutterVersion(
+        frameworkVersion: '1.0.0',
+        channel: 'beta'
+      ),
+      devToolsVersion: () => '2.8.0',
+      userMessages: UserMessages(),
+      artifacts: Artifacts.test(),
+      fileSystem: MemoryFileSystem.test(),
+      processManager: FakeProcessManager.any(),
+      operatingSystemUtils: FakeOperatingSystemUtils(
+        name: 'Linux',
+        whichLookup: const <String, File>{},
+      ),
+      flutterRoot: () => 'sdk/flutter',
+    );
+
+    expect(await flutterValidator.validate(), _matchDoctorValidation(
+      validationType: ValidationType.partial,
+      statusInfo: 'Channel beta, 1.0.0, on Linux, locale en_US.UTF-8',
+      messages: contains(const ValidationMessage.hint(
+        'The flutter binary is not on your path. Consider adding sdk/flutter/bin to your path.',
+      )),
+    ));
+  });
+
+  testWithoutContext('detects flutter and dart from outside flutter sdk', () async {
+    final FileSystem fs = MemoryFileSystem.test();
+    final FlutterValidator flutterValidator = FlutterValidator(
+      platform: FakePlatform(localeName: 'en_US.UTF-8'),
+      flutterVersion: () => FakeFlutterVersion(
+        frameworkVersion: '1.0.0',
+        channel: 'beta'
+      ),
+      devToolsVersion: () => '2.8.0',
+      userMessages: UserMessages(),
+      artifacts: Artifacts.test(),
+      fileSystem: fs,
+      processManager: FakeProcessManager.any(),
+      operatingSystemUtils: FakeOperatingSystemUtils(
+        name: 'Linux',
+        whichLookup: <String, File>{
+          'flutter': fs.file('/usr/bin/flutter')..createSync(recursive: true),
+          'dart': fs.file('/usr/bin/dart')..createSync(recursive: true),
+        },
+      ),
+      flutterRoot: () => 'sdk/flutter',
+    );
+
+    expect(await flutterValidator.validate(), _matchDoctorValidation(
+      validationType: ValidationType.partial,
+      statusInfo: 'Channel beta, 1.0.0, on Linux, locale en_US.UTF-8',
+      messages: contains(const ValidationMessage.hint(
+        'Warning: `flutter` on your path resolves to /usr/bin/flutter, which '
+        'is not inside your current Flutter SDK checkout at sdk/flutter. '
+        'Consider adding sdk/flutter/bin to the front of your path.',
+      )),
+    ));
+  });
+
+  testWithoutContext('no warnings if flutter & dart binaries are inside the Flutter SDK', () async {
+    final FileSystem fs = MemoryFileSystem.test();
+    final FlutterValidator flutterValidator = FlutterValidator(
+      platform: FakePlatform(localeName: 'en_US.UTF-8'),
+      flutterVersion: () => FakeFlutterVersion(
+        frameworkVersion: '1.0.0',
+        channel: 'beta'
+      ),
+      devToolsVersion: () => '2.8.0',
+      userMessages: UserMessages(),
+      artifacts: Artifacts.test(),
+      fileSystem: fs,
+      processManager: FakeProcessManager.any(),
+      operatingSystemUtils: FakeOperatingSystemUtils(
+        name: 'Linux',
+        whichLookup: <String, File>{
+          'flutter': fs.file('/sdk/flutter/bin/flutter')..createSync(recursive: true),
+          'dart': fs.file('/sdk/flutter/bin/dart')..createSync(recursive: true),
+        },
+      ),
+      flutterRoot: () => '/sdk/flutter',
+    );
+
+    expect(await flutterValidator.validate(), _matchDoctorValidation(
+      validationType: ValidationType.installed,
+      statusInfo: 'Channel beta, 1.0.0, on Linux, locale en_US.UTF-8',
+      messages: isNot(contains(isA<ValidationMessage>().having(
+        (ValidationMessage message) => message.message,
+        'message',
+        contains('Consider adding /sdk/flutter/bin to the front of your path'),
+      ))),
+    ));
+  });
 }
 
 class FakeOperatingSystemUtils extends Fake implements OperatingSystemUtils {
-  FakeOperatingSystemUtils({required this.name});
+  FakeOperatingSystemUtils({
+    required this.name,
+    this.whichLookup,
+    FileSystem? fs,
+  }) {
+    fs ??= MemoryFileSystem.test();
+    whichLookup ??= <String, File>{
+      'flutter': fs.file('/sdk/flutter/bin/flutter')..createSync(recursive: true),
+      'dart': fs.file('/sdk/flutter/bin/dart')..createSync(recursive: true),
+    };
+  }
+
+  /// A map of [File]s that calls to [which] will return.
+  Map<String, File>? whichLookup;
+
+  @override
+  File? which(String execName) => whichLookup![execName];
 
   @override
   final String name;