Cleanup getExecutablePath() to better respect the platform (#24)

.gitignore

@@ -1,6 +1,7 @@
 ### Dart template
 # Don’t commit the following directories created by pub.
 .buildlog
+.dart_tool
 .pub/
 build/
 packages

.travis.yml

@@ -1,7 +1,6 @@
 language: dart
 sudo: false
 dart:
-  - stable
   - dev
 install:
   - gem install coveralls-lcov

CHANGELOG.md

@@ -1,3 +1,7 @@
+#### 3.0.0
+
+* Cleanup getExecutablePath() to better respect the platform
+
 #### 2.0.9
 
 * Bumped `package:file` dependency

lib/src/interface/common.dart

@@ -2,58 +2,89 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
-import 'dart:io' show File, Directory;
-
-import 'package:path/path.dart' as p;
+import 'package:file/file.dart';
+import 'package:file/local.dart';
+import 'package:path/path.dart' show Context, Style;
 import 'package:platform/platform.dart';
 
-/// Searches the `PATH` for the actual executable that [commandName] is supposed
-/// to launch.
+const Map<String, String> _osToPathStyle = const <String, String>{
+  'linux': 'posix',
+  'macos': 'posix',
+  'android': 'posix',
+  'ios': 'posix',
+  'fuchsia': 'posix',
+  'windows': 'windows',
+};
+
+/// Searches the `PATH` for the executable that [command] is supposed to launch.
+///
+/// This first builds a list of candidate paths where the executable may reside.
+/// If [command] is already an absolute path, then the `PATH` environment
+/// variable will not be consulted, and the specified absolute path will be the
+/// only candidate that is considered.
+///
+/// Once the list of candidate paths has been constructed, this will pick the
+/// first such path that represents an existent file.
 ///
-/// Return `null` if the executable cannot be found.
-String getExecutablePath(String commandName, String workingDirectory,
-    {Platform platform}) {
-  platform ??= new LocalPlatform();
-  workingDirectory ??= Directory.current.path;
+/// Return `null` if there were no viable candidates, meaning the executable
+/// could not be found.
+///
+/// If [platform] is not specified, it will default to the current platform.
+String getExecutablePath(
+  String command,
+  String workingDirectory, {
+  Platform platform: const LocalPlatform(),
+  FileSystem fs: const LocalFileSystem(),
+}) {
+  assert(_osToPathStyle[platform.operatingSystem] == fs.path.style.name);
+
+  workingDirectory ??= fs.currentDirectory.path;
+  Context context =
+      new Context(style: fs.path.style, current: workingDirectory);
+
   // TODO(goderbauer): refactor when github.com/google/platform.dart/issues/2
   //     is available.
   String pathSeparator = platform.isWindows ? ';' : ':';
 
   List<String> extensions = <String>[];
-  if (platform.isWindows && p.extension(commandName).isEmpty) {
+  if (platform.isWindows && context.extension(command).isEmpty) {
     extensions = platform.environment['PATHEXT'].split(pathSeparator);
   }
 
   List<String> candidates = <String>[];
-  if (commandName.contains(p.separator)) {
-    candidates =
-        _getCandidatePaths(commandName, <String>[workingDirectory], extensions);
+  if (command.contains(context.separator)) {
+    candidates = _getCandidatePaths(
+        command, <String>[workingDirectory], extensions, context);
   } else {
     List<String> searchPath = platform.environment['PATH'].split(pathSeparator);
-    candidates = _getCandidatePaths(commandName, searchPath, extensions);
+    candidates = _getCandidatePaths(command, searchPath, extensions, context);
   }
-  return candidates.firstWhere((String path) => new File(path).existsSync(),
+  return candidates.firstWhere((String path) => fs.file(path).existsSync(),
       orElse: () => null);
 }
 
-/// Returns all possible combinations of `$searchPath\$commandName.$ext` for
+/// Returns all possible combinations of `$searchPath\$command.$ext` for
 /// `searchPath` in [searchPaths] and `ext` in [extensions].
 ///
 /// If [extensions] is empty, it will just enumerate all
-/// `$searchPath\$commandName`.
-/// If [commandName] is an absolute path, it will just enumerate
-/// `$commandName.$ext`.
+/// `$searchPath\$command`.
+/// If [command] is an absolute path, it will just enumerate
+/// `$command.$ext`.
 Iterable<String> _getCandidatePaths(
-    String commandName, List<String> searchPaths, List<String> extensions) {
+  String command,
+  List<String> searchPaths,
+  List<String> extensions,
+  Context context,
+) {
   List<String> withExtensions = extensions.isNotEmpty
-      ? extensions.map((String ext) => '$commandName$ext').toList()
-      : <String>[commandName];
-  if (p.isAbsolute(commandName)) {
+      ? extensions.map((String ext) => '$command$ext').toList()
+      : <String>[command];
+  if (context.isAbsolute(command)) {
     return withExtensions;
   }
   return searchPaths
       .map((String path) =>
-          withExtensions.map((String command) => p.join(path, command)))
+          withExtensions.map((String command) => context.join(path, command)))
       .expand((Iterable<String> e) => e)
       .toList();
 }

pubspec.yaml

@@ -1,20 +1,20 @@
 name: process
-version: 2.0.9
+version: 3.0.0
 authors:
 - Todd Volkert <[email protected]>
 - Michael Goderbauer <[email protected]>
 description: A pluggable, mockable process invocation abstraction for Dart.
 homepage: https://github.com/google/process.dart
 
 dependencies:
-  file: '>=2.0.1 <4.0.0'
+  file: '>=2.0.1'
   intl: '>=0.14.0 <0.16.0'
-  meta: ^1.0.4
-  path: ^1.4.0
-  platform: '>=1.0.1 <3.0.0'
+  meta: ^1.1.2
+  path: ^1.5.1
+  platform: '>=1.0.1'
 
 dev_dependencies:
-  test: ^0.12.10
+  test: ^0.12.33
 
 environment:
-  sdk: '>=1.21.0 <2.0.0'
+  sdk: '>=2.0.0-dev.28.0 <2.0.0'