fixes #610, incorrect help output

R=nweiz@google.com

Review URL: https://codereview.chromium.org/2244703003 .

pkg/dev_compiler/bin/dartdevc.dart

@@ -38,7 +38,6 @@
 import 'dart:async';
 import 'dart:io';
 import 'package:analyzer/src/generated/engine.dart' show AnalysisEngine;
-import 'package:args/command_runner.dart';
 import 'package:bazel_worker/bazel_worker.dart';
 import 'package:dev_compiler/src/compiler/command.dart';
 
@@ -49,28 +48,10 @@ Future main(List<String> args) async {
   if (args.contains('--persistent_worker')) {
     new _CompilerWorker(args..remove('--persistent_worker')).run();
   } else {
-    exitCode = await _runCommand(args);
+    exitCode = compile(args);
   }
 }
 
-/// Runs a single compile command, and returns an exit code.
-Future<int> _runCommand(List<String> args,
-    {MessageHandler messageHandler}) async {
-  try {
-    if (args.isEmpty || args.first != 'compile' && args.first != 'help') {
-      // TODO(jmesserly): we should deprecate the commands. For now they are
-      // still supported for backwards compatibility.
-      args.insert(0, 'compile');
-    }
-    var runner = new CommandRunner('dartdevc', 'Dart Development Compiler');
-    runner.addCommand(new CompileCommand(messageHandler: messageHandler));
-    await runner.run(args);
-  } catch (e, s) {
-    return _handleError(e, s, args, messageHandler: messageHandler);
-  }
-  return EXIT_CODE_OK;
-}
-
 /// Runs the compiler worker loop.
 class _CompilerWorker extends AsyncWorkerLoop {
   /// The original args supplied to the executable.
@@ -83,7 +64,7 @@ class _CompilerWorker extends AsyncWorkerLoop {
     var args = _startupArgs.toList()..addAll(request.arguments);
 
     var output = new StringBuffer();
-    var exitCode = await _runCommand(args, messageHandler: output.writeln);
+    var exitCode = compile(args, printFn: output.writeln);
     AnalysisEngine.instance.clearCaches();
     return new WorkResponse()
       ..exitCode = exitCode
@@ -91,48 +72,6 @@ class _CompilerWorker extends AsyncWorkerLoop {
   }
 }
 
-/// Handles [error] in a uniform fashion. Returns the proper exit code and calls
-/// [messageHandler] with messages.
-int _handleError(dynamic error, dynamic stackTrace, List<String> args,
-    {MessageHandler messageHandler}) {
-  messageHandler ??= print;
-
-  if (error is UsageException) {
-    // Incorrect usage, input file not found, etc.
-    messageHandler(error);
-    return 64;
-  } else if (error is CompileErrorException) {
-    // Code has error(s) and failed to compile.
-    messageHandler(error);
-    return 1;
-  } else {
-    // Anything else is likely a compiler bug.
-    //
-    // --unsafe-force-compile is a bit of a grey area, but it's nice not to
-    // crash while compiling
-    // (of course, output code may crash, if it had errors).
-    //
-    messageHandler("");
-    messageHandler("We're sorry, you've found a bug in our compiler.");
-    messageHandler("You can report this bug at:");
-    messageHandler("    https://github.com/dart-lang/dev_compiler/issues");
-    messageHandler("");
-    messageHandler(
-        "Please include the information below in your report, along with");
-    messageHandler(
-        "any other information that may help us track it down. Thanks!");
-    messageHandler("");
-    messageHandler("    dartdevc arguments: " + args.join(' '));
-    messageHandler("    dart --version: ${Platform.version}");
-    messageHandler("");
-    messageHandler("```");
-    messageHandler(error);
-    messageHandler(stackTrace);
-    messageHandler("```");
-    return 70;
-  }
-}
-
 /// Always returns a new modifiable list.
 ///
 /// If the final arg is `@file_path` then read in all the lines of that file

pkg/dev_compiler/lib/src/analyzer/context.dart

@@ -74,8 +74,8 @@ class AnalyzerOptions {
   /// Whether to resolve 'package:' uris using the multi-package resolver.
   bool get useMultiPackage => packagePaths.isNotEmpty;
 
-  static ArgParser addArguments(ArgParser parser) {
-    return parser
+  static void addArguments(ArgParser parser) {
+    parser
       ..addOption('summary',
           abbr: 's', help: 'summary file(s) to include', allowMultiple: true)
       ..addOption('dart-sdk', help: 'Dart SDK Path', defaultsTo: null)

pkg/dev_compiler/lib/src/compiler/command.dart

@@ -4,49 +4,94 @@
 
 import 'dart:convert' show JSON;
 import 'dart:io';
-import 'package:args/command_runner.dart';
 import 'package:analyzer/src/generated/source.dart' show Source;
 import 'package:analyzer/src/summary/package_bundle_reader.dart'
     show InSummarySource;
+import 'package:args/args.dart' show ArgParser, ArgResults;
+import 'package:args/command_runner.dart' show UsageException;
+import 'package:path/path.dart' as path;
+
 import 'compiler.dart'
     show BuildUnit, CompilerOptions, JSModuleFile, ModuleCompiler;
 import '../analyzer/context.dart' show AnalyzerOptions;
-import 'package:path/path.dart' as path;
 
-typedef void MessageHandler(Object message);
-
-/// The command for invoking the modular compiler.
-class CompileCommand extends Command {
-  final MessageHandler messageHandler;
-  CompilerOptions _compilerOptions;
-
-  CompileCommand({MessageHandler messageHandler})
-      : this.messageHandler = messageHandler ?? print {
-    argParser.addOption('out', abbr: 'o', help: 'Output file (required)');
-    argParser.addOption('module-root',
-        help: 'Root module directory. '
-            'Generated module paths are relative to this root.');
-    argParser.addOption('library-root',
-        help: 'Root of source files. '
-            'Generated library names are relative to this root.');
-    argParser.addOption('build-root',
-        help: 'Deprecated in favor of --library-root');
-    CompilerOptions.addArguments(argParser);
+final ArgParser _argParser = () {
+  var argParser = new ArgParser()
+    ..addFlag('help', abbr: 'h', help: 'Display this message.')
+    ..addOption('out', abbr: 'o', help: 'Output file (required).')
+    ..addOption('module-root',
+        help: 'Root module directory.\n'
+            'Generated module paths are relative to this root.')
+    ..addOption('library-root',
+        help: 'Root of source files.\n'
+            'Generated library names are relative to this root.')
+    ..addOption('build-root',
+        help: 'Deprecated in favor of --library-root', hide: true);
   AnalyzerOptions.addArguments(argParser);
+  CompilerOptions.addArguments(argParser);
+  return argParser;
+}();
+
+/// Runs a single compile for dartdevc.
+///
+/// This handles argument parsing, usage, error handling.
+/// See bin/dartdevc.dart for the actual entry point, which includes Bazel
+/// worker support.
+int compile(List<String> args, {void printFn(Object obj)}) {
+  printFn ??= print;
+  ArgResults argResults;
+  try {
+    argResults = _argParser.parse(args);
+  } on FormatException catch (error) {
+    printFn('$error\n\n$_usageMessage');
+    return 64;
+  }
+  try {
+    _compile(argResults, printFn);
+    return 0;
+  } on UsageException catch (error) {
+    // Incorrect usage, input file not found, etc.
+    printFn(error);
+    return 64;
+  } on CompileErrorException catch (error) {
+    // Code has error(s) and failed to compile.
+    printFn(error);
+    return 1;
+  } catch (error, stackTrace) {
+    // Anything else is likely a compiler bug.
+    //
+    // --unsafe-force-compile is a bit of a grey area, but it's nice not to
+    // crash while compiling
+    // (of course, output code may crash, if it had errors).
+    //
+    printFn('''
+We're sorry, you've found a bug in our compiler.
+You can report this bug at:
+    https://github.com/dart-lang/dev_compiler/issues
+Please include the information below in your report, along with
+any other information that may help us track it down. Thanks!
+    dartdevc arguments: ${args.join(' ')}
+    dart --version: ${Platform.version}
+```
+$error
+$stackTrace
+```''');
+    return 70;
+  }
 }
 
-  get name => 'compile';
-  get description => 'Compile a set of Dart files into a JavaScript module.';
-
-  @override
-  void run() {
+void _compile(ArgResults argResults, void printFn(Object obj)) {
   var compiler =
       new ModuleCompiler(new AnalyzerOptions.fromArguments(argResults));
-    _compilerOptions = new CompilerOptions.fromArguments(argResults);
+  var compilerOpts = new CompilerOptions.fromArguments(argResults);
+  if (argResults['help']) {
+    printFn(_usageMessage);
+    return;
+  }
   var outPath = argResults['out'];
 
   if (outPath == null) {
-      usageException('Please include the output file location. For example:\n'
+    _usageException('Please include the output file location. For example:\n'
         '    -o PATH/TO/OUTPUT_FILE.js');
   }
 
@@ -62,7 +107,7 @@ class CompileCommand extends Command {
   if (moduleRoot != null) {
     moduleRoot = path.absolute(moduleRoot);
     if (!path.isWithin(moduleRoot, outPath)) {
-        usageException('Output file $outPath must be within the module root '
+      _usageException('Output file $outPath must be within the module root '
           'directory $moduleRoot');
     }
     modulePath =
@@ -73,10 +118,10 @@ class CompileCommand extends Command {
   }
 
   var unit = new BuildUnit(modulePath, libraryRoot, argResults.rest,
-        (source) => _moduleForLibrary(moduleRoot, source));
+      (source) => _moduleForLibrary(moduleRoot, source, compilerOpts));
 
-    JSModuleFile module = compiler.compile(unit, _compilerOptions);
-    module.errors.forEach(messageHandler);
+  JSModuleFile module = compiler.compile(unit, compilerOpts);
+  module.errors.forEach(printFn);
 
   if (!module.isValid) throw new CompileErrorException();
 
@@ -88,32 +133,40 @@ class CompileCommand extends Command {
         .writeAsStringSync(JSON.encode(module.placeSourceMap(mapPath)));
   }
   if (module.summaryBytes != null) {
-      var summaryPath = path.withoutExtension(outPath) +
-          '.${_compilerOptions.summaryExtension}';
+    var summaryPath =
+        path.withoutExtension(outPath) + '.${compilerOpts.summaryExtension}';
     new File(summaryPath).writeAsBytesSync(module.summaryBytes);
   }
 }
 
-  String _moduleForLibrary(String moduleRoot, Source source) {
+String _moduleForLibrary(
+    String moduleRoot, Source source, CompilerOptions compilerOpts) {
   if (source is InSummarySource) {
     var summaryPath = source.summaryPath;
-      var ext = '.${_compilerOptions.summaryExtension}';
+    var ext = '.${compilerOpts.summaryExtension}';
     if (path.isWithin(moduleRoot, summaryPath) && summaryPath.endsWith(ext)) {
       var buildUnitPath =
           summaryPath.substring(0, summaryPath.length - ext.length);
       return path.relative(buildUnitPath, from: moduleRoot);
     }
 
-      throw usageException(
-          'Imported file ${source.uri} is not within the module root '
+    _usageException('Imported file ${source.uri} is not within the module root '
         'directory $moduleRoot');
   }
 
-    throw usageException(
+  _usageException(
       'Imported file "${source.uri}" was not found as a summary or source '
       'file. Please pass in either the summary or the source file '
       'for this import.');
+  return null; // unreachable
 }
+
+final _usageMessage =
+    'Dart Development Compiler compiles Dart into a JavaScript module.'
+    '\n\n${_argParser.usage}';
+
+void _usageException(String message) {
+  throw new UsageException(message, _usageMessage);
 }
 
 /// Thrown when the input source code has errors.

pkg/dev_compiler/lib/src/compiler/compiler.dart

@@ -245,15 +245,19 @@ class CompilerOptions {
         hoistTypeTests = args['hoist-type-tests'],
         useAngular2Whitelist = args['unsafe-angular2-whitelist'];
 
-  static ArgParser addArguments(ArgParser parser) => parser
+  static void addArguments(ArgParser parser) {
+    parser
       ..addFlag('summarize', help: 'emit an API summary file', defaultsTo: true)
       ..addOption('summary-extension',
-        help: 'file extension for Dart summary files', defaultsTo: 'sum')
+          help: 'file extension for Dart summary files',
+          defaultsTo: 'sum',
+          hide: true)
       ..addFlag('source-map', help: 'emit source mapping', defaultsTo: true)
       ..addFlag('source-map-comment',
           help: 'adds a sourceMappingURL comment to the end of the JS,\n'
               'disable if using X-SourceMap header',
-        defaultsTo: true)
+          defaultsTo: true,
+          hide: true)
       ..addOption('modules',
           help: 'module pattern to emit',
           allowed: ['es6', 'legacy', 'node'],
@@ -270,22 +274,27 @@ class CompilerOptions {
           help: 'emit Closure Compiler-friendly code (experimental)',
           defaultsTo: false)
       ..addFlag('destructure-named-params',
-        help: 'Destructure named parameters', defaultsTo: false)
+          help: 'Destructure named parameters', defaultsTo: false, hide: true)
       ..addFlag('unsafe-force-compile',
           help: 'Compile code even if it has errors. ಠ_ಠ\n'
               'This has undefined behavior!',
-        defaultsTo: false)
+          defaultsTo: false,
+          hide: true)
       ..addFlag('hoist-instance-creation',
           help: 'Hoist the class type from generic instance creations',
-        defaultsTo: true)
+          defaultsTo: true,
+          hide: true)
       ..addFlag('hoist-signature-types',
-        help: 'Hoist types from class signatures', defaultsTo: false)
+          help: 'Hoist types from class signatures',
+          defaultsTo: false,
+          hide: true)
       ..addFlag('name-type-tests',
-        help: 'Name types used in type tests', defaultsTo: true)
+          help: 'Name types used in type tests', defaultsTo: true, hide: true)
       ..addFlag('hoist-type-tests',
-        help: 'Hoist types used in type tests', defaultsTo: true)
+          help: 'Hoist types used in type tests', defaultsTo: true, hide: true)
       ..addFlag('unsafe-angular2-whitelist', defaultsTo: false, hide: true);
   }
+}
 
 /// A unit of Dart code that can be built into a single JavaScript module.
 class BuildUnit {

pkg/dev_compiler/lib/src/compiler/element_helpers.dart

@@ -29,10 +29,9 @@ class Tuple2<T0, T1> {
 
 /*=T*/ fillDynamicTypeArgs/*<T extends DartType>*/(/*=T*/ t) {
   if (t is ParameterizedType) {
-    var pt = t as ParameterizedType;
     var dyn = new List<DartType>.filled(
-        pt.typeArguments.length, DynamicTypeImpl.instance);
-    return pt.substitute2(dyn, pt.typeArguments) as dynamic/*=T*/;
+        t.typeArguments.length, DynamicTypeImpl.instance);
+    return t.substitute2(dyn, t.typeArguments) as dynamic/*=T*/;
   }
   return t;
 }

pkg/dev_compiler/test/codegen_test.dart

@@ -94,7 +94,8 @@ main(List<String> arguments) {
 
   // Our default compiler options. Individual tests can override these.
   var defaultOptions = ['--no-source-map', '--no-summarize'];
-  var compilerArgParser = CompilerOptions.addArguments(new ArgParser());
+  var compilerArgParser = new ArgParser();
+  CompilerOptions.addArguments(compilerArgParser);
 
   // Compile each test file to JS and put the result in gen/codegen_output.
   for (var testFile in testFiles) {

pkg/dev_compiler/test/worker/worker_test.dart

@@ -17,7 +17,7 @@ main() {
     final argsFile = new File('test/worker/hello_world.args').absolute;
     final inputDartFile = new File('test/worker/hello_world.dart').absolute;
     final outputJsFile = new File('test/worker/hello_world.js').absolute;
-    final executableArgs = ['bin/dartdevc.dart', 'compile',];
+    final executableArgs = ['bin/dartdevc.dart'];
     final compilerArgs = [
       '--no-source-map',
       '--no-summarize',
@@ -118,7 +118,6 @@ main() {
     test('can compile in basic mode', () {
       var result = Process.runSync('dart', [
         'bin/dartdevc.dart',
-        'compile',
         '--summary-extension=api.ds',
         '--no-source-map',
         '-o',
@@ -133,7 +132,6 @@ main() {
 
       result = Process.runSync('dart', [
         'bin/dartdevc.dart',
-        'compile',
         '--no-source-map',
         '--no-summarize',
         '--summary-extension=api.ds',
@@ -171,7 +169,6 @@ main() {
       badFileDart.writeAsStringSync('main() => "hello world"');
       var result = Process.runSync('dart', [
         'bin/dartdevc.dart',
-        'compile',
         '--no-source-map',
         '-o',
         badFileJs.path,
@@ -205,7 +202,6 @@ main() {
     test('works if part and library supplied', () {
       var result = Process.runSync('dart', [
         'bin/dartdevc.dart',
-        'compile',
         '--no-summarize',
         '--no-source-map',
         '-o',
@@ -222,7 +218,6 @@ main() {
     test('works if part is not supplied', () {
       var result = Process.runSync('dart', [
         'bin/dartdevc.dart',
-        'compile',
         '--no-summarize',
         '--no-source-map',
         '-o',
@@ -238,7 +233,6 @@ main() {
     test('part without library is silently ignored', () {
       var result = Process.runSync('dart', [
         'bin/dartdevc.dart',
-        'compile',
         '--no-summarize',
         '--no-source-map',
         '-o',

pkg/dev_compiler/tool/build_sdk.dart

@@ -9,12 +9,10 @@
 /// command line interface. But being able to build from a Dart library means
 /// we can call this during code coverage to get more realistic numbers.
 
-import 'package:args/command_runner.dart' show CommandRunner;
 import 'package:dev_compiler/src/compiler/command.dart';
 
 main(List<String> arguments) {
   var args = [
-    'compile',
     '--unsafe-force-compile',
     '--no-source-map',
     '--no-emit-metadata'
@@ -50,7 +48,5 @@ main(List<String> arguments) {
     'dart:web_gl',
     'dart:web_sql'
   ]);
-  var runner = new CommandRunner('dartdevc', 'Dart Development Compiler');
-  runner.addCommand(new CompileCommand());
-  return runner.run(args);
+  compile(args);
 }

pkg/dev_compiler/tool/build_test_pkgs.sh

@@ -21,12 +21,13 @@ SDK=--dart-sdk-summary=lib/runtime/dart_sdk.sum
 ./bin/dartdevc.dart $SDK -o gen/codegen_output/pkg/matcher.js \
     package:matcher/matcher.dart
 
-./bin/dartdevc.dart $SDK  -o gen/codegen_output/pkg/stack_trace.js \
-    package:stack_trace/stack_trace.dart
-
 ./bin/dartdevc.dart $SDK -o gen/codegen_output/pkg/path.js \
     package:path/path.dart
 
+./bin/dartdevc.dart $SDK -o gen/codegen_output/pkg/stack_trace.js \
+    -s gen/codegen_output/pkg/path.sum \
+    package:stack_trace/stack_trace.dart
+
 ./bin/dartdevc.dart $SDK -o gen/codegen_output/pkg/unittest.js \
     package:unittest/unittest.dart \
     package:unittest/html_config.dart \