Adding more complete tool options.

gspencergoog · gspencergoog · commit 88283d18f1b2 · 2018-09-19T20:36:08.000-07:00
diff --git a/lib/src/dartdoc_options.dart b/lib/src/dartdoc_options.dart
@@ -110,6 +110,103 @@ class CategoryConfiguration {
   }
 }
 
+class ToolDefinition {
+  final List<String> command;
+  final String description;
+
+  ToolDefinition(this.command, this.description)
+      : assert(command != null),
+        assert(command.isNotEmpty),
+        assert(description != null);
+
+  @override
+  String toString() => '$runtimeType: "${command.join(' ')}" ($description)';
+}
+
+class ToolConfiguration {
+  final Map<String, ToolDefinition> tools;
+
+  ToolConfiguration._(this.tools);
+
+  static ToolConfiguration get empty {
+    return new ToolConfiguration._({});
+  }
+
+  static ToolConfiguration fromYamlMap(
+      YamlMap yamlMap, pathLib.Context pathContext) {
+    Map<String, ToolDefinition> newToolDefinitions = {};
+    for (MapEntry entry in yamlMap.entries) {
+      String name = entry.key.toString();
+      var toolMap = entry.value;
+      String description;
+      List<String> command;
+      if (toolMap is Map) {
+        description = toolMap['description']?.toString();
+        // If the command key is given, then it applies to all platforms.
+        String commandFrom = toolMap.containsKey('command')
+            ? 'command'
+            : Platform.operatingSystem;
+        if (toolMap.containsKey(commandFrom)) {
+          if (toolMap[commandFrom].value is String) {
+            command = [toolMap[commandFrom].toString()];
+            if (command[0].isEmpty) {
+              throw new DartdocOptionError(
+                  'Tool commands must not be empty. Tool $name command entry '
+                  '"$commandFrom" must contain at least one path.');
+            }
+          } else if (toolMap[commandFrom] is YamlList) {
+            command = (toolMap[commandFrom] as YamlList)
+                .map<String>((node) => node.toString())
+                .toList();
+            if (command.isEmpty) {
+              throw new DartdocOptionError(
+                  'Tool commands must not be empty. Tool $name command entry '
+                  '"$commandFrom" must contain at least one path.');
+            }
+          } else {
+            throw new DartdocOptionError(
+                'Tool commands must be a path to an executable, or a list of '
+                'strings that starts with a path to an executable. '
+                'The tool $name has a $commandFrom entry that is a '
+                '${toolMap[commandFrom].runtimeType}');
+          }
+        }
+      } else {
+        throw new DartdocOptionError(
+            'Tools must be defined as a map of tool names to definitions. Tool '
+            '$name is not a map.');
+      }
+      if (command == null) {
+        throw new DartdocOptionError(
+            'At least one of "command" or "${Platform.operatingSystem}" must '
+            'be defined for the tool $name.');
+      }
+      String executable = command.removeAt(0);
+      executable = pathContext.canonicalize(executable);
+      File executableFile = new File(executable);
+      FileStat exeStat = executableFile.statSync();
+      if (exeStat.type == FileSystemEntityType.notFound) {
+        throw new DartdocOptionError('Command executables must exist. '
+            'The file "$executable" does not exist for tool $name.');
+      }
+      // Dart scripts don't need to be executable, because they'll be
+      // executed with the Dart binary.
+      bool isExecutable(int mode) {
+        return (0x1 & ((mode >> 6) | (mode >> 3) | mode)) != 0;
+      }
+
+      if (!executable.endsWith('.dart') && !isExecutable(exeStat.mode)) {
+        throw new DartdocOptionError('Non-Dart commands must be '
+            'executable. The file "$executable" for tool $name does not have '
+            'executable permission.');
+      }
+      newToolDefinitions[name] =
+          new ToolDefinition([executable] + command, description);
+    }
+    return new ToolConfiguration._(newToolDefinitions);
+  }
+}
+
 /// A container class to keep track of where our yaml data came from.
 class _YamlFileData {
   /// The map from the yaml file.
@@ -212,7 +309,10 @@ abstract class DartdocOption<T> {
     assert(!(isDir && isExecutable));
     assert(!(isFile && isExecutable));
     if (isDir || isFile || isExecutable)
-      assert(_isString || _isListString || _isMapString);
+      assert(_isString ||
+          _isListString ||
+          _isMapString ||
+          _convertYamlToType != null);
     if (mustExist) assert(isDir || isFile || isExecutable);
   }
 
@@ -271,6 +371,9 @@ abstract class DartdocOption<T> {
       resolvedPaths = valueWithContext.resolvedValue.toList();
     } else if (valueWithContext.value is Map<String, String>) {
       resolvedPaths = valueWithContext.resolvedValue.values.toList();
+    } else {
+      assert(false, "Trying to ensure existence of wrong type "
+          "${valueWithContext.value.runtimeType}");
     }
     for (String path in resolvedPaths) {
       FileSystemEntity f = isDir ? new Directory(path) : new File(path);
@@ -1073,7 +1176,7 @@ class DartdocOptionContext {
   List<String> get includeExternal =>
       optionSet['includeExternal'].valueAt(context);
   bool get includeSource => optionSet['includeSource'].valueAt(context);
-  Map<String, String> get tools => optionSet['tools'].valueAt(context);
+  ToolConfiguration get tools => optionSet['tools'].valueAt(context);
 
   /// _input is only used to construct synthetic options.
   // ignore: unused_element
@@ -1299,10 +1402,12 @@ Future<List<DartdocOption>> createDartdocOptions() async {
     new DartdocOptionArgOnly<bool>('verboseWarnings', true,
         help: 'Display extra debugging information and help with warnings.',
         negatable: true),
-    new DartdocOptionArgFile<Map<String, String>>('tools', <String, String>{},
-        isExecutable: true,
-        mustExist: true,
+    new DartdocOptionFileOnly<ToolConfiguration>(
+        'tools', ToolConfiguration.empty,
+        convertYamlToType: ToolConfiguration.fromYamlMap,
         help: 'A map of tool names to executable paths. Each executable must '
-            'exist.'),
+            'exist. Executables for different platforms are specified by '
+            'giving the platform name as a key, and a list of strings as the '
+            'command.'),
   ];
 }
diff --git a/lib/src/tool_runner.dart b/lib/src/tool_runner.dart
@@ -7,6 +7,7 @@ library dartdoc.tool_runner;
 import 'dart:io';
 
 import 'package:path/path.dart' as pathLib;
+import 'dartdoc_options.dart';
 
 typedef ToolErrorCallback = void Function(String message);
 typedef FakeResultCallback = String Function(String tool,
@@ -16,14 +17,14 @@ typedef FakeResultCallback = String Function(String tool,
 class ToolRunner {
   /// Creates a new ToolRunner.
   ///
-  /// Takes a [toolMap] that describes all of the available tools.
+  /// Takes a [toolConfiguration] that describes all of the available tools.
   /// An optional `errorCallback` will be called for each error message
   /// generated by the tool.
-  ToolRunner(this.toolMap, [this._errorCallback])
+  ToolRunner(this.toolConfiguration, [this._errorCallback])
       : temporaryDirectory =
             Directory.systemTemp.createTempSync('dartdoc_tools_');
 
-  final Map<String, String> toolMap;
+  final ToolConfiguration toolConfiguration;
   final Directory temporaryDirectory;
   final ToolErrorCallback _errorCallback;
   int _temporaryFileCount = 0;
@@ -61,17 +62,16 @@ class ToolRunner {
     assert(args.isNotEmpty);
     content ??= '';
     String tool = args.removeAt(0);
-    if (!toolMap.containsKey(tool)) {
+    if (!toolConfiguration.tools.containsKey(tool)) {
       _error('Unable to find definition for tool "$tool" in tool map. '
           'Did you add it to dartdoc_options.yaml?');
       return '';
     }
-    String toolPath = toolMap[tool];
-    List<String> toolArgs = <String>[];
-    if (pathLib.extension(toolPath) == '.dart') {
+    ToolDefinition toolDefinition = toolConfiguration.tools[tool];
+    List<String> toolArgs = toolDefinition.command;
+    if (pathLib.extension(toolDefinition.command.first) == '.dart') {
       // For dart tools, we want to invoke them with Dart.
-      toolArgs = [toolPath];
-      toolPath = Platform.resolvedExecutable;
+      toolArgs.insert(0, Platform.resolvedExecutable);
     }
 
     // Ideally, we would just be able to send the input text into stdin,
@@ -94,9 +94,10 @@ class ToolRunner {
     }
 
     argsWithInput = toolArgs + argsWithInput;
-    String commandString() => ([toolPath] + argsWithInput).join(' ');
+    final String commandPath = argsWithInput.removeAt(0);
+    String commandString() => ([commandPath] + argsWithInput).join(' ');
     try {
-      ProcessResult result = Process.runSync(toolPath, argsWithInput);
+      ProcessResult result = Process.runSync(commandPath, argsWithInput);
       if (result.exitCode != 0) {
         _error('Tool "$tool" returned non-zero exit code '
             '(${result.exitCode}) when run as '
diff --git a/test/tool_runner_test.dart b/test/tool_runner_test.dart
@@ -6,22 +6,27 @@ library dartdoc.model_test;
 
 import 'dart:io';
 
+import 'package:dartdoc/src/dartdoc_options.dart';
 import 'package:dartdoc/src/tool_runner.dart';
 import 'package:path/path.dart' as pathLib;
 import 'package:test/test.dart';
 
 import 'src/utils.dart' as utils;
 
 void main() {
-  Map<String, String> toolMap = {
-    'missing': '/a/missing/executable',
-    'drill':
-        pathLib.join(utils.testPackageDir.absolute.path, 'bin', 'drill.dart'),
+  ToolConfiguration toolMap = ToolConfiguration.empty;
+
+  toolMap.tools.addAll({
+    'missing': new ToolDefinition(['/a/missing/executable'], "missing"),
+    'drill': new ToolDefinition(
+        [pathLib.join(utils.testPackageDir.absolute.path, 'bin', 'drill.dart')],
+        'Makes holes'),
     // We use the Dart executable for our "non-dart" tool
     // test, because it's the only executable that we know the
     // exact location of that works on all platforms.
-    'non_dart': Platform.resolvedExecutable,
-  };
+    'non_dart':
+        new ToolDefinition([Platform.resolvedExecutable], 'non-dart tool'),
+  });
   ToolRunner runner;
   final List<String> errors = <String>[];
 
@@ -42,10 +47,7 @@ void main() {
         'TEST INPUT',
       );
       expect(errors, isEmpty);
-      expect(
-          result,
-          contains(new RegExp(
-              r'Args: \[--file=<INPUT_FILE>]')));
+      expect(result, contains(new RegExp(r'Args: \[--file=<INPUT_FILE>]')));
       expect(result, contains('## `TEST INPUT`'));
     });
     test('can invoke a non-Dart tool', () {
diff --git a/testing/test_package/dartdoc_options.yaml b/testing/test_package/dartdoc_options.yaml
@@ -8,4 +8,11 @@ dartdoc:
       markdown: "Unreal.md"
     Real Libraries:
   tools:
-    drill: "bin/drill.dart"
+    drill:
+      command: ["bin/drill.dart"]
+      description: "Puts holes in things."
+    echo:
+      macos: ['/bin/sh', '-c', 'echo']
+      linux: ['/bin/sh', '-c', 'echo']
+      windows: ['C:\\Windows\\System32\\cmd.exe', '/c', 'echo']
+      description: 'Works on everything'
