Skip to content

Commit 3627356

Browse files
author
John Messerly
committed
fixes #610, incorrect help output
[email protected] Review URL: https://codereview.chromium.org/2244703003 .
1 parent 0aa5cbd commit 3627356

File tree

9 files changed

+211
-219
lines changed

9 files changed

+211
-219
lines changed

‎pkg/dev_compiler/bin/dartdevc.dart

Lines changed: 2 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@
3838
import 'dart:async';
3939
import 'dart:io';
4040
import 'package:analyzer/src/generated/engine.dart' show AnalysisEngine;
41-
import 'package:args/command_runner.dart';
4241
import 'package:bazel_worker/bazel_worker.dart';
4342
import 'package:dev_compiler/src/compiler/command.dart';
4443

@@ -49,28 +48,10 @@ Future main(List<String> args) async {
4948
if (args.contains('--persistent_worker')) {
5049
new _CompilerWorker(args..remove('--persistent_worker')).run();
5150
} else {
52-
exitCode = await _runCommand(args);
51+
exitCode = compile(args);
5352
}
5453
}
5554

56-
/// Runs a single compile command, and returns an exit code.
57-
Future<int> _runCommand(List<String> args,
58-
{MessageHandler messageHandler}) async {
59-
try {
60-
if (args.isEmpty || args.first != 'compile' && args.first != 'help') {
61-
// TODO(jmesserly): we should deprecate the commands. For now they are
62-
// still supported for backwards compatibility.
63-
args.insert(0, 'compile');
64-
}
65-
var runner = new CommandRunner('dartdevc', 'Dart Development Compiler');
66-
runner.addCommand(new CompileCommand(messageHandler: messageHandler));
67-
await runner.run(args);
68-
} catch (e, s) {
69-
return _handleError(e, s, args, messageHandler: messageHandler);
70-
}
71-
return EXIT_CODE_OK;
72-
}
73-
7455
/// Runs the compiler worker loop.
7556
class _CompilerWorker extends AsyncWorkerLoop {
7657
/// The original args supplied to the executable.
@@ -83,56 +64,14 @@ class _CompilerWorker extends AsyncWorkerLoop {
8364
var args = _startupArgs.toList()..addAll(request.arguments);
8465

8566
var output = new StringBuffer();
86-
var exitCode = await _runCommand(args, messageHandler: output.writeln);
67+
var exitCode = compile(args, printFn: output.writeln);
8768
AnalysisEngine.instance.clearCaches();
8869
return new WorkResponse()
8970
..exitCode = exitCode
9071
..output = output.toString();
9172
}
9273
}
9374

94-
/// Handles [error] in a uniform fashion. Returns the proper exit code and calls
95-
/// [messageHandler] with messages.
96-
int _handleError(dynamic error, dynamic stackTrace, List<String> args,
97-
{MessageHandler messageHandler}) {
98-
messageHandler ??= print;
99-
100-
if (error is UsageException) {
101-
// Incorrect usage, input file not found, etc.
102-
messageHandler(error);
103-
return 64;
104-
} else if (error is CompileErrorException) {
105-
// Code has error(s) and failed to compile.
106-
messageHandler(error);
107-
return 1;
108-
} else {
109-
// Anything else is likely a compiler bug.
110-
//
111-
// --unsafe-force-compile is a bit of a grey area, but it's nice not to
112-
// crash while compiling
113-
// (of course, output code may crash, if it had errors).
114-
//
115-
messageHandler("");
116-
messageHandler("We're sorry, you've found a bug in our compiler.");
117-
messageHandler("You can report this bug at:");
118-
messageHandler(" https://github.com/dart-lang/dev_compiler/issues");
119-
messageHandler("");
120-
messageHandler(
121-
"Please include the information below in your report, along with");
122-
messageHandler(
123-
"any other information that may help us track it down. Thanks!");
124-
messageHandler("");
125-
messageHandler(" dartdevc arguments: " + args.join(' '));
126-
messageHandler(" dart --version: ${Platform.version}");
127-
messageHandler("");
128-
messageHandler("```");
129-
messageHandler(error);
130-
messageHandler(stackTrace);
131-
messageHandler("```");
132-
return 70;
133-
}
134-
}
135-
13675
/// Always returns a new modifiable list.
13776
///
13877
/// If the final arg is `@file_path` then read in all the lines of that file

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,8 @@ class AnalyzerOptions {
7474
/// Whether to resolve 'package:' uris using the multi-package resolver.
7575
bool get useMultiPackage => packagePaths.isNotEmpty;
7676

77-
static ArgParser addArguments(ArgParser parser) {
78-
return parser
77+
static void addArguments(ArgParser parser) {
78+
parser
7979
..addOption('summary',
8080
abbr: 's', help: 'summary file(s) to include', allowMultiple: true)
8181
..addOption('dart-sdk', help: 'Dart SDK Path', defaultsTo: null)
@@ -86,7 +86,7 @@ class AnalyzerOptions {
8686
help: 'Package root to resolve "package:" imports',
8787
defaultsTo: 'packages/')
8888
..addOption('url-mapping',
89-
help: '--url-mapping=libraryUri,/path/to/library.dart uses \n'
89+
help: '--url-mapping=libraryUri,/path/to/library.dart uses\n'
9090
'library.dart as the source for an import of of "libraryUri".',
9191
allowMultiple: true,
9292
splitCommas: false)

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

Lines changed: 142 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -4,116 +4,169 @@
44

55
import 'dart:convert' show JSON;
66
import 'dart:io';
7-
import 'package:args/command_runner.dart';
87
import 'package:analyzer/src/generated/source.dart' show Source;
98
import 'package:analyzer/src/summary/package_bundle_reader.dart'
109
show InSummarySource;
10+
import 'package:args/args.dart' show ArgParser, ArgResults;
11+
import 'package:args/command_runner.dart' show UsageException;
12+
import 'package:path/path.dart' as path;
13+
1114
import 'compiler.dart'
1215
show BuildUnit, CompilerOptions, JSModuleFile, ModuleCompiler;
1316
import '../analyzer/context.dart' show AnalyzerOptions;
14-
import 'package:path/path.dart' as path;
1517

16-
typedef void MessageHandler(Object message);
17-
18-
/// The command for invoking the modular compiler.
19-
class CompileCommand extends Command {
20-
final MessageHandler messageHandler;
21-
CompilerOptions _compilerOptions;
22-
23-
CompileCommand({MessageHandler messageHandler})
24-
: this.messageHandler = messageHandler ?? print {
25-
argParser.addOption('out', abbr: 'o', help: 'Output file (required)');
26-
argParser.addOption('module-root',
27-
help: 'Root module directory. '
28-
'Generated module paths are relative to this root.');
29-
argParser.addOption('library-root',
30-
help: 'Root of source files. '
31-
'Generated library names are relative to this root.');
32-
argParser.addOption('build-root',
33-
help: 'Deprecated in favor of --library-root');
34-
CompilerOptions.addArguments(argParser);
35-
AnalyzerOptions.addArguments(argParser);
18+
final ArgParser _argParser = () {
19+
var argParser = new ArgParser()
20+
..addFlag('help', abbr: 'h', help: 'Display this message.')
21+
..addOption('out', abbr: 'o', help: 'Output file (required).')
22+
..addOption('module-root',
23+
help: 'Root module directory.\n'
24+
'Generated module paths are relative to this root.')
25+
..addOption('library-root',
26+
help: 'Root of source files.\n'
27+
'Generated library names are relative to this root.')
28+
..addOption('build-root',
29+
help: 'Deprecated in favor of --library-root', hide: true);
30+
AnalyzerOptions.addArguments(argParser);
31+
CompilerOptions.addArguments(argParser);
32+
return argParser;
33+
}();
34+
35+
/// Runs a single compile for dartdevc.
36+
///
37+
/// This handles argument parsing, usage, error handling.
38+
/// See bin/dartdevc.dart for the actual entry point, which includes Bazel
39+
/// worker support.
40+
int compile(List<String> args, {void printFn(Object obj)}) {
41+
printFn ??= print;
42+
ArgResults argResults;
43+
try {
44+
argResults = _argParser.parse(args);
45+
} on FormatException catch (error) {
46+
printFn('$error\n\n$_usageMessage');
47+
return 64;
3648
}
49+
try {
50+
_compile(argResults, printFn);
51+
return 0;
52+
} on UsageException catch (error) {
53+
// Incorrect usage, input file not found, etc.
54+
printFn(error);
55+
return 64;
56+
} on CompileErrorException catch (error) {
57+
// Code has error(s) and failed to compile.
58+
printFn(error);
59+
return 1;
60+
} catch (error, stackTrace) {
61+
// Anything else is likely a compiler bug.
62+
//
63+
// --unsafe-force-compile is a bit of a grey area, but it's nice not to
64+
// crash while compiling
65+
// (of course, output code may crash, if it had errors).
66+
//
67+
printFn('''
68+
We're sorry, you've found a bug in our compiler.
69+
You can report this bug at:
70+
https://github.com/dart-lang/dev_compiler/issues
71+
Please include the information below in your report, along with
72+
any other information that may help us track it down. Thanks!
73+
dartdevc arguments: ${args.join(' ')}
74+
dart --version: ${Platform.version}
75+
```
76+
$error
77+
$stackTrace
78+
```''');
79+
return 70;
80+
}
81+
}
3782

38-
get name => 'compile';
39-
get description => 'Compile a set of Dart files into a JavaScript module.';
40-
41-
@override
42-
void run() {
43-
var compiler =
44-
new ModuleCompiler(new AnalyzerOptions.fromArguments(argResults));
45-
_compilerOptions = new CompilerOptions.fromArguments(argResults);
46-
var outPath = argResults['out'];
83+
void _compile(ArgResults argResults, void printFn(Object obj)) {
84+
var compiler =
85+
new ModuleCompiler(new AnalyzerOptions.fromArguments(argResults));
86+
var compilerOpts = new CompilerOptions.fromArguments(argResults);
87+
if (argResults['help']) {
88+
printFn(_usageMessage);
89+
return;
90+
}
91+
var outPath = argResults['out'];
4792

48-
if (outPath == null) {
49-
usageException('Please include the output file location. For example:\n'
50-
' -o PATH/TO/OUTPUT_FILE.js');
51-
}
93+
if (outPath == null) {
94+
_usageException('Please include the output file location. For example:\n'
95+
' -o PATH/TO/OUTPUT_FILE.js');
96+
}
5297

53-
var libraryRoot = argResults['library-root'] as String;
54-
libraryRoot ??= argResults['build-root'] as String;
55-
if (libraryRoot != null) {
56-
libraryRoot = path.absolute(libraryRoot);
57-
} else {
58-
libraryRoot = Directory.current.path;
59-
}
60-
var moduleRoot = argResults['module-root'] as String;
61-
String modulePath;
62-
if (moduleRoot != null) {
63-
moduleRoot = path.absolute(moduleRoot);
64-
if (!path.isWithin(moduleRoot, outPath)) {
65-
usageException('Output file $outPath must be within the module root '
66-
'directory $moduleRoot');
67-
}
68-
modulePath =
69-
path.withoutExtension(path.relative(outPath, from: moduleRoot));
70-
} else {
71-
moduleRoot = path.dirname(outPath);
72-
modulePath = path.basenameWithoutExtension(outPath);
98+
var libraryRoot = argResults['library-root'] as String;
99+
libraryRoot ??= argResults['build-root'] as String;
100+
if (libraryRoot != null) {
101+
libraryRoot = path.absolute(libraryRoot);
102+
} else {
103+
libraryRoot = Directory.current.path;
104+
}
105+
var moduleRoot = argResults['module-root'] as String;
106+
String modulePath;
107+
if (moduleRoot != null) {
108+
moduleRoot = path.absolute(moduleRoot);
109+
if (!path.isWithin(moduleRoot, outPath)) {
110+
_usageException('Output file $outPath must be within the module root '
111+
'directory $moduleRoot');
73112
}
113+
modulePath =
114+
path.withoutExtension(path.relative(outPath, from: moduleRoot));
115+
} else {
116+
moduleRoot = path.dirname(outPath);
117+
modulePath = path.basenameWithoutExtension(outPath);
118+
}
74119

75-
var unit = new BuildUnit(modulePath, libraryRoot, argResults.rest,
76-
(source) => _moduleForLibrary(moduleRoot, source));
120+
var unit = new BuildUnit(modulePath, libraryRoot, argResults.rest,
121+
(source) => _moduleForLibrary(moduleRoot, source, compilerOpts));
77122

78-
JSModuleFile module = compiler.compile(unit, _compilerOptions);
79-
module.errors.forEach(messageHandler);
123+
JSModuleFile module = compiler.compile(unit, compilerOpts);
124+
module.errors.forEach(printFn);
80125

81-
if (!module.isValid) throw new CompileErrorException();
126+
if (!module.isValid) throw new CompileErrorException();
82127

83-
// Write JS file, as well as source map and summary (if requested).
84-
new File(outPath).writeAsStringSync(module.code);
85-
if (module.sourceMap != null) {
86-
var mapPath = outPath + '.map';
87-
new File(mapPath)
88-
.writeAsStringSync(JSON.encode(module.placeSourceMap(mapPath)));
89-
}
90-
if (module.summaryBytes != null) {
91-
var summaryPath = path.withoutExtension(outPath) +
92-
'.${_compilerOptions.summaryExtension}';
93-
new File(summaryPath).writeAsBytesSync(module.summaryBytes);
94-
}
128+
// Write JS file, as well as source map and summary (if requested).
129+
new File(outPath).writeAsStringSync(module.code);
130+
if (module.sourceMap != null) {
131+
var mapPath = outPath + '.map';
132+
new File(mapPath)
133+
.writeAsStringSync(JSON.encode(module.placeSourceMap(mapPath)));
134+
}
135+
if (module.summaryBytes != null) {
136+
var summaryPath =
137+
path.withoutExtension(outPath) + '.${compilerOpts.summaryExtension}';
138+
new File(summaryPath).writeAsBytesSync(module.summaryBytes);
95139
}
140+
}
96141

97-
String _moduleForLibrary(String moduleRoot, Source source) {
98-
if (source is InSummarySource) {
99-
var summaryPath = source.summaryPath;
100-
var ext = '.${_compilerOptions.summaryExtension}';
101-
if (path.isWithin(moduleRoot, summaryPath) && summaryPath.endsWith(ext)) {
102-
var buildUnitPath =
103-
summaryPath.substring(0, summaryPath.length - ext.length);
104-
return path.relative(buildUnitPath, from: moduleRoot);
105-
}
106-
107-
throw usageException(
108-
'Imported file ${source.uri} is not within the module root '
109-
'directory $moduleRoot');
142+
String _moduleForLibrary(
143+
String moduleRoot, Source source, CompilerOptions compilerOpts) {
144+
if (source is InSummarySource) {
145+
var summaryPath = source.summaryPath;
146+
var ext = '.${compilerOpts.summaryExtension}';
147+
if (path.isWithin(moduleRoot, summaryPath) && summaryPath.endsWith(ext)) {
148+
var buildUnitPath =
149+
summaryPath.substring(0, summaryPath.length - ext.length);
150+
return path.relative(buildUnitPath, from: moduleRoot);
110151
}
111152

112-
throw usageException(
113-
'Imported file "${source.uri}" was not found as a summary or source '
114-
'file. Please pass in either the summary or the source file '
115-
'for this import.');
153+
_usageException('Imported file ${source.uri} is not within the module root '
154+
'directory $moduleRoot');
116155
}
156+
157+
_usageException(
158+
'Imported file "${source.uri}" was not found as a summary or source '
159+
'file. Please pass in either the summary or the source file '
160+
'for this import.');
161+
return null; // unreachable
162+
}
163+
164+
final _usageMessage =
165+
'Dart Development Compiler compiles Dart into a JavaScript module.'
166+
'\n\n${_argParser.usage}';
167+
168+
void _usageException(String message) {
169+
throw new UsageException(message, _usageMessage);
117170
}
118171

119172
/// Thrown when the input source code has errors.

0 commit comments

Comments
 (0)