diff --git a/pkg/dartdev/lib/dartdev.dart b/pkg/dartdev/lib/dartdev.dart index 5e8f2a9283b..287c5dd15a2 100644 --- a/pkg/dartdev/lib/dartdev.dart +++ b/pkg/dartdev/lib/dartdev.dart @@ -32,34 +32,129 @@ import 'src/vm_interop_handler.dart'; /// analytics logic, it has been moved here. Future runDartdev(List args, SendPort port) async { VmInteropHandler.initialize(port); - if (args.contains('run')) { - // These flags have a format that can't be handled by package:args, so while - // they are valid flags we'll assume the VM has verified them by this point. - args = args - .where( - (element) => !(element.contains('--observe') || - element.contains('--enable-vm-service')), - ) - .toList(); + + int result; + + // The exit code for the dartdev process; null indicates that it has not been + // set yet. The value is set in the catch and finally blocks below. + int exitCode; + + // Any caught non-UsageExceptions when running the sub command. + Object exception; + StackTrace stackTrace; + + // The Analytics instance used to report information back to Google Analytics; + // see lib/src/analytics.dart. + final analytics = createAnalyticsInstance( + args.contains('--disable-dartdev-analytics'), + ); + + // If we have not printed the analyticsNoticeOnFirstRunMessage to stdout, + // the user is on a terminal, and the machine is not a bot, then print the + // disclosure and set analytics.disclosureShownOnTerminal to true. + if (analytics is DartdevAnalytics && + !analytics.disclosureShownOnTerminal && + io.stdout.hasTerminal && + !isBot()) { + print(analyticsNoticeOnFirstRunMessage); + analytics.disclosureShownOnTerminal = true; } - // Finally, call the runner to execute the command; see DartdevRunner. + // When `--disable-analytics` or `--enable-analytics` are called we perform + // the respective intention and print any notices to standard out and exit. + if (args.contains('--disable-analytics')) { + // This block also potentially catches the case of (disableAnalytics && + // enableAnalytics), in which we favor the disabling of analytics. + analytics.enabled = false; + + // Alert the user that analytics has been disabled. + print(analyticsDisabledNoticeMessage); + VmInteropHandler.exit(0); + return; + } else if (args.contains('--enable-analytics')) { + analytics.enabled = true; + + // Alert the user again that anonymous data will be collected. + print(analyticsNoticeOnFirstRunMessage); + VmInteropHandler.exit(0); + return; + } - final runner = DartdevRunner(args); - var exitCode = 1; try { - exitCode = await runner.run(args); - } on UsageException catch (e) { - // TODO(sigurdm): It is unclear when a UsageException gets to here, and - // when it is in DartdevRunner.runCommand. - io.stderr.writeln('$e'); - exitCode = 64; + final runner = DartdevRunner(args, analytics); + + // Run can't be called with the '--disable-dartdev-analytics' flag; remove + // it if it is contained in args. + if (args.contains('--disable-dartdev-analytics')) { + args = List.from(args)..remove('--disable-dartdev-analytics'); + } + + if (args.contains('run')) { + // These flags have a format that can't be handled by package:args, so while + // they are valid flags we'll assume the VM has verified them by this point. + args = args + .where( + (element) => !(element.contains('--observe') || + element.contains('--enable-vm-service')), + ) + .toList(); + } + + // If ... help pub ... is in the args list, remove 'help', and add '--help' + // to the end of the list. This will make it possible to use the help + // command to access subcommands of pub such as `dart help pub publish`; see + // https://github.com/dart-lang/sdk/issues/42965. + if (PubUtils.shouldModifyArgs(args, runner.commands.keys.toList())) { + args = PubUtils.modifyArgs(args); + } + + // Finally, call the runner to execute the command; see DartdevRunner. + result = await runner.run(args); + } catch (e, st) { + if (e is UsageException) { + io.stderr.writeln('$e'); + exitCode = 64; + } else { + // Set the exception and stack trace only for non-UsageException cases: + exception = e; + stackTrace = st; + io.stderr.writeln('$e'); + io.stderr.writeln('$st'); + exitCode = 1; + } } finally { + // Set the exitCode, if it wasn't set in the catch block above. + exitCode ??= result ?? 0; + + // Send analytics before exiting + if (analytics.enabled) { + // And now send the exceptions and events to Google Analytics: + if (exception != null) { + unawaited( + analytics.sendException( + '${exception.runtimeType}\n${sanitizeStacktrace(stackTrace)}', + fatal: true), + ); + } + + await analytics.waitForLastPing( + timeout: const Duration(milliseconds: 200)); + } + + // Set the enabled flag in the analytics object to true. Note: this will not + // enable the analytics unless the disclosure was shown (terminal detected), + // and the machine is not detected to be a bot. + if (analytics.firstRun) { + analytics.enabled = true; + } + analytics.close(); VmInteropHandler.exit(exitCode); } } class DartdevRunner extends CommandRunner { + final Analytics analytics; + @override final ArgParser argParser = ArgParser( usageLineLength: dartdevUsageLineLength, @@ -69,7 +164,8 @@ class DartdevRunner extends CommandRunner { static const String dartdevDescription = 'A command-line utility for Dart development'; - DartdevRunner(List args) : super('dart', '$dartdevDescription.') { + DartdevRunner(List args, this.analytics) + : super('dart', '$dartdevDescription.') { final bool verbose = args.contains('-v') || args.contains('--verbose'); argParser.addFlag('verbose', @@ -84,9 +180,12 @@ class DartdevRunner extends CommandRunner { argParser.addFlag('diagnostics', negatable: false, help: 'Show tool diagnostic output.', hide: !verbose); + // A hidden flag to disable analytics on this run, this constructor can be + // called with this flag, but should be removed before run() is called as + // the flag has not been added to all sub-commands. argParser.addFlag( - 'analytics', - negatable: true, + 'disable-dartdev-analytics', + negatable: false, help: 'Disable anonymous analytics for this `dart *` run', hide: true, ); @@ -113,38 +212,7 @@ class DartdevRunner extends CommandRunner { @override Future runCommand(ArgResults topLevelResults) async { final stopwatch = Stopwatch()..start(); - // The Analytics instance used to report information back to Google Analytics; - // see lib/src/analytics.dart. - final analytics = createAnalyticsInstance(!topLevelResults['analytics']); - - // If we have not printed the analyticsNoticeOnFirstRunMessage to stdout, - // the user is on a terminal, and the machine is not a bot, then print the - // disclosure and set analytics.disclosureShownOnTerminal to true. - if (analytics is DartdevAnalytics && - !analytics.disclosureShownOnTerminal && - io.stdout.hasTerminal && - !isBot()) { - print(analyticsNoticeOnFirstRunMessage); - analytics.disclosureShownOnTerminal = true; - } - - // When `--disable-analytics` or `--enable-analytics` are called we perform - // the respective intention and print any notices to standard out and exit. - if (topLevelResults['disable-analytics']) { - // This block also potentially catches the case of (disableAnalytics && - // enableAnalytics), in which we favor the disabling of analytics. - analytics.enabled = false; - - // Alert the user that analytics has been disabled. - print(analyticsDisabledNoticeMessage); - return 0; - } else if (topLevelResults['enable-analytics']) { - analytics.enabled = true; - - // Alert the user again that anonymous data will be collected. - print(analyticsNoticeOnFirstRunMessage); - return 0; - } + assert(!topLevelResults.arguments.contains('--disable-dartdev-analytics')); if (topLevelResults.command == null && topLevelResults.arguments.isNotEmpty) { @@ -154,12 +222,14 @@ class DartdevRunner extends CommandRunner { io.stderr.writeln( "Error when reading '$firstArg': No such file or directory."); // This is the exit code used by the frontend. - return 254; + VmInteropHandler.exit(254); } } + isDiagnostics = topLevelResults['diagnostics']; + final Ansi ansi = Ansi(Ansi.terminalSupportsAnsi); - log = topLevelResults['diagnostics'] + log = isDiagnostics ? Logger.verbose(ansi: ansi) : Logger.standard(ansi: ansi); @@ -177,15 +247,8 @@ class DartdevRunner extends CommandRunner { analytics.sendScreenView(path), ); - // The exit code for the dartdev process; null indicates that it has not been - // set yet. The value is set in the catch and finally blocks below. - int exitCode; - - // Any caught non-UsageExceptions when running the sub command. - Object exception; - StackTrace stackTrace; try { - exitCode = await super.runCommand(topLevelResults); + final exitCode = await super.runCommand(topLevelResults); if (path != null && analytics.enabled) { // Send the event to analytics @@ -205,16 +268,8 @@ class DartdevRunner extends CommandRunner { ), ); } - } on UsageException catch (e) { - io.stderr.writeln('$e'); - exitCode = 64; - } catch (e, st) { - // Set the exception and stack trace only for non-UsageException cases: - exception = e; - stackTrace = st; - io.stderr.writeln('$e'); - io.stderr.writeln('$st'); - exitCode = 1; + + return exitCode; } finally { stopwatch.stop(); if (analytics.enabled) { @@ -226,32 +281,6 @@ class DartdevRunner extends CommandRunner { ), ); } - // Set the exitCode, if it wasn't set in the catch block above. - exitCode ??= 0; - - // Send analytics before exiting - if (analytics.enabled) { - // And now send the exceptions and events to Google Analytics: - if (exception != null) { - unawaited( - analytics.sendException( - '${exception.runtimeType}\n${sanitizeStacktrace(stackTrace)}', - fatal: true), - ); - } - - await analytics.waitForLastPing( - timeout: const Duration(milliseconds: 200)); - } - - // Set the enabled flag in the analytics object to true. Note: this will not - // enable the analytics unless the disclosure was shown (terminal detected), - // and the machine is not detected to be a bot. - if (analytics.firstRun) { - analytics.enabled = true; - } - analytics.close(); - return exitCode; } } } diff --git a/pkg/dartdev/lib/src/analytics.dart b/pkg/dartdev/lib/src/analytics.dart index c9b1edf47dc..14b9fb2c02c 100644 --- a/pkg/dartdev/lib/src/analytics.dart +++ b/pkg/dartdev/lib/src/analytics.dart @@ -57,7 +57,7 @@ Analytics createAnalyticsInstance(bool disableAnalytics) { } if (disableAnalytics) { - // Dartdev tests pass a hidden 'no-analytics' flag which is + // Dartdev tests pass a hidden 'disable-dartdev-analytics' flag which is // handled here. // // Also, stdout.hasTerminal is checked; if there is no terminal we infer diff --git a/pkg/dartdev/lib/src/utils.dart b/pkg/dartdev/lib/src/utils.dart index 32a9c8ec47e..88b80ed0a05 100644 --- a/pkg/dartdev/lib/src/utils.dart +++ b/pkg/dartdev/lib/src/utils.dart @@ -38,6 +38,31 @@ String trimEnd(String s, String suffix) { return s; } +/// Static util methods used in dartdev to potentially modify the order of the +/// arguments passed into dartdev. +class PubUtils { + /// If [doModifyArgs] returns true, then this method returns a modified copy + /// of the argument list, 'help' is removed from the interior of the list, and + /// '--help' is added to the end of the list of arguments. This method returns + /// a modified copy of the list, the list itself is not modified. + static List modifyArgs(List args) => List.from(args) + ..remove('help') + ..add('--help'); + + /// If ... help pub ..., and no other verb (such as 'analyze') appears before + /// the ... help pub ... in the argument list, then return true. + static bool shouldModifyArgs(List args, List allCmds) => + args != null && + allCmds != null && + args.isNotEmpty && + allCmds.isNotEmpty && + args.firstWhere((arg) => allCmds.contains(arg), orElse: () => '') == + 'help' && + args.contains('help') && + args.contains('pub') && + args.indexOf('help') + 1 == args.indexOf('pub'); +} + extension FileSystemEntityExtension on FileSystemEntity { String get name => p.basename(path); diff --git a/pkg/dartdev/test/analytics_test.dart b/pkg/dartdev/test/analytics_test.dart index 8c74972c513..26d9822a181 100644 --- a/pkg/dartdev/test/analytics_test.dart +++ b/pkg/dartdev/test/analytics_test.dart @@ -23,7 +23,7 @@ void main() { group('Sending analytics', () { test('help', () { final p = project(logAnalytics: true); - final result = p.runSync(['help']); + final result = p.runSync('help', []); expect(extractAnalytics(result), [ { 'hitType': 'screenView', @@ -51,7 +51,7 @@ void main() { }); test('create', () { final p = project(logAnalytics: true); - final result = p.runSync(['create', '-tpackage-simple', 'name']); + final result = p.runSync('create', ['-tpackage-simple', 'name']); expect(extractAnalytics(result), [ { 'hitType': 'screenView', @@ -82,7 +82,7 @@ void main() { test('pub get', () { final p = project(logAnalytics: true); - final result = p.runSync(['pub', 'get', '--dry-run']); + final result = p.runSync('pub', ['get', '--dry-run']); expect(extractAnalytics(result), [ { 'hitType': 'screenView', @@ -113,7 +113,7 @@ void main() { test('format', () { final p = project(logAnalytics: true); - final result = p.runSync(['format', '-l80']); + final result = p.runSync('format', ['-l80']); expect(extractAnalytics(result), [ { 'hitType': 'screenView', @@ -146,8 +146,7 @@ void main() { final p = project( mainSrc: 'void main(List args) => print(args)', logAnalytics: true); - final result = p.runSync([ - 'run', + final result = p.runSync('run', [ '--no-pause-isolates-on-exit', '--enable-asserts', 'lib/main.dart', @@ -185,8 +184,7 @@ void main() { final p = project( mainSrc: 'void main(List args) => print(args);', logAnalytics: true); - final result = p.runSync([ - 'run', + final result = p.runSync('run', [ '--enable-experiment=non-nullable', 'lib/main.dart', ]); @@ -223,7 +221,7 @@ void main() { mainSrc: 'void main(List args) => print(args);', logAnalytics: true); final result = p - .runSync(['compile', 'kernel', 'lib/main.dart', '-o', 'main.kernel']); + .runSync('compile', ['kernel', 'lib/main.dart', '-o', 'main.kernel']); expect(extractAnalytics(result), [ { 'hitType': 'screenView', diff --git a/pkg/dartdev/test/commands/analyze_test.dart b/pkg/dartdev/test/commands/analyze_test.dart index f2d9895629e..12e1776ce9b 100644 --- a/pkg/dartdev/test/commands/analyze_test.dart +++ b/pkg/dartdev/test/commands/analyze_test.dart @@ -64,7 +64,7 @@ void defineAnalyze() { test('--help', () { p = project(); - var result = p.runSync(['analyze', '--help']); + var result = p.runSync('analyze', ['--help']); expect(result.exitCode, 0); expect(result.stderr, isEmpty); @@ -74,7 +74,7 @@ void defineAnalyze() { test('multiple directories', () { p = project(); - var result = p.runSync(['analyze', '/no/such/dir1/', '/no/such/dir2/']); + var result = p.runSync('analyze', ['/no/such/dir1/', '/no/such/dir2/']); expect(result.exitCode, 64); expect(result.stdout, isEmpty); @@ -84,7 +84,7 @@ void defineAnalyze() { test('no such directory', () { p = project(); - var result = p.runSync(['analyze', '/no/such/dir1/']); + var result = p.runSync('analyze', ['/no/such/dir1/']); expect(result.exitCode, 64); expect(result.stdout, isEmpty); @@ -95,7 +95,7 @@ void defineAnalyze() { test('current working directory', () { p = project(mainSrc: 'int get foo => 1;\n'); - var result = p.runSync(['analyze'], workingDir: p.dirPath); + var result = p.runSync('analyze', [], workingDir: p.dirPath); expect(result.exitCode, 0); expect(result.stderr, isEmpty); @@ -104,7 +104,7 @@ void defineAnalyze() { test('no errors', () { p = project(mainSrc: 'int get foo => 1;\n'); - var result = p.runSync(['analyze', p.dirPath]); + var result = p.runSync('analyze', [p.dirPath]); expect(result.exitCode, 0); expect(result.stderr, isEmpty); @@ -113,7 +113,7 @@ void defineAnalyze() { test('one error', () { p = project(mainSrc: "int get foo => 'str';\n"); - var result = p.runSync(['analyze', p.dirPath]); + var result = p.runSync('analyze', [p.dirPath]); expect(result.exitCode, 3); expect(result.stderr, isEmpty); @@ -125,7 +125,7 @@ void defineAnalyze() { test('two errors', () { p = project(mainSrc: "int get foo => 'str';\nint get bar => 'str';\n"); - var result = p.runSync(['analyze', p.dirPath]); + var result = p.runSync('analyze', [p.dirPath]); expect(result.exitCode, 3); expect(result.stderr, isEmpty); @@ -136,7 +136,7 @@ void defineAnalyze() { p = project( mainSrc: _unusedImportCodeSnippet, analysisOptions: _unusedImportAnalysisOptions); - var result = p.runSync(['analyze', '--fatal-warnings', p.dirPath]); + var result = p.runSync('analyze', ['--fatal-warnings', p.dirPath]); expect(result.exitCode, equals(2)); expect(result.stderr, isEmpty); @@ -147,7 +147,7 @@ void defineAnalyze() { p = project( mainSrc: _unusedImportCodeSnippet, analysisOptions: _unusedImportAnalysisOptions); - var result = p.runSync(['analyze', p.dirPath]); + var result = p.runSync('analyze', [p.dirPath]); expect(result.exitCode, equals(2)); expect(result.stderr, isEmpty); @@ -158,7 +158,7 @@ void defineAnalyze() { p = project( mainSrc: _unusedImportCodeSnippet, analysisOptions: _unusedImportAnalysisOptions); - var result = p.runSync(['analyze', '--no-fatal-warnings', p.dirPath]); + var result = p.runSync('analyze', ['--no-fatal-warnings', p.dirPath]); expect(result.exitCode, 0); expect(result.stderr, isEmpty); @@ -167,7 +167,7 @@ void defineAnalyze() { test('info implicit no --fatal-infos', () { p = project(mainSrc: dartVersionFilePrefix2_9 + 'String foo() {}'); - var result = p.runSync(['analyze', p.dirPath]); + var result = p.runSync('analyze', [p.dirPath]); expect(result.exitCode, 0); expect(result.stderr, isEmpty); @@ -176,7 +176,7 @@ void defineAnalyze() { test('info --fatal-infos', () { p = project(mainSrc: dartVersionFilePrefix2_9 + 'String foo() {}'); - var result = p.runSync(['analyze', '--fatal-infos', p.dirPath]); + var result = p.runSync('analyze', ['--fatal-infos', p.dirPath]); expect(result.exitCode, 1); expect(result.stderr, isEmpty); @@ -190,7 +190,7 @@ int f() { var one = 1; return result; }'''); - var result = p.runSync(['analyze', '--verbose', p.dirPath]); + var result = p.runSync('analyze', ['--verbose', p.dirPath]); expect(result.exitCode, 3); expect(result.stderr, isEmpty); diff --git a/pkg/dartdev/test/commands/compile_test.dart b/pkg/dartdev/test/commands/compile_test.dart index f2f5f31de5f..57e669a7ae4 100644 --- a/pkg/dartdev/test/commands/compile_test.dart +++ b/pkg/dartdev/test/commands/compile_test.dart @@ -27,9 +27,8 @@ void defineCompileTests() { test('Implicit --help', () { final p = project(); var result = p.runSync( - [ - 'compile', - ], + 'compile', + [], ); expect(result.stderr, contains('Compile Dart')); expect(result.exitCode, compileErrorExitCode); @@ -38,7 +37,8 @@ void defineCompileTests() { test('--help', () { final p = project(); final result = p.runSync( - ['compile', '--help'], + 'compile', + ['--help'], ); expect(result.stdout, contains('Compile Dart')); expect(result.exitCode, 0); @@ -48,8 +48,8 @@ void defineCompileTests() { final p = project(mainSrc: 'void main() { print("I love jit"); }'); final outFile = path.join(p.dirPath, 'main.jit'); var result = p.runSync( + 'compile', [ - 'compile', 'jit-snapshot', '-o', outFile, @@ -61,7 +61,7 @@ void defineCompileTests() { expect(File(outFile).existsSync(), true, reason: 'File not found: $outFile'); - result = p.runSync(['run', 'main.jit']); + result = p.runSync('run', ['main.jit']); expect(result.stdout, contains('I love jit')); expect(result.stderr, isEmpty); expect(result.exitCode, 0); @@ -73,8 +73,8 @@ void defineCompileTests() { final outFile = path.canonicalize(path.join(p.dirPath, 'lib', 'main.exe')); var result = p.runSync( + 'compile', [ - 'compile', 'exe', inFile, ], @@ -102,8 +102,8 @@ void defineCompileTests() { final outFile = path.canonicalize(path.join(p.dirPath, 'myexe')); var result = p.runSync( + 'compile', [ - 'compile', 'exe', '--define', 'life=42', @@ -134,8 +134,8 @@ void defineCompileTests() { final outFile = path.canonicalize(path.join(p.dirPath, 'main.aot')); var result = p.runSync( + 'compile', [ - 'compile', 'aot-snapshot', '-o', 'main.aot', @@ -163,8 +163,8 @@ void defineCompileTests() { final p = project(mainSrc: 'void main() { print("I love kernel"); }'); final outFile = path.join(p.dirPath, 'main.dill'); var result = p.runSync( + 'compile', [ - 'compile', 'kernel', '-o', outFile, @@ -176,7 +176,7 @@ void defineCompileTests() { expect(result.stderr, isEmpty); expect(result.exitCode, 0); - result = p.runSync(['run', 'main.dill']); + result = p.runSync('run', ['main.dill']); expect(result.stdout, contains('I love kernel')); expect(result.stderr, isEmpty); expect(result.exitCode, 0); @@ -187,8 +187,7 @@ void defineCompileTests() { final inFile = path.canonicalize(path.join(p.dirPath, p.relativeFilePath)); final outFile = path.canonicalize(path.join(p.dirPath, 'main.js')); - final result = p.runSync([ - 'compile', + final result = p.runSync('compile', [ 'js', '-m', '-o', diff --git a/pkg/dartdev/test/commands/create_test.dart b/pkg/dartdev/test/commands/create_test.dart index 7f5c84683aa..38a8ebd0492 100644 --- a/pkg/dartdev/test/commands/create_test.dart +++ b/pkg/dartdev/test/commands/create_test.dart @@ -36,7 +36,7 @@ void defineCreateTests() { test('list templates', () { p = project(); - ProcessResult result = p.runSync(['create', '--list-templates']); + ProcessResult result = p.runSync('create', ['--list-templates']); expect(result.exitCode, 0); String output = result.stdout.toString(); @@ -50,9 +50,7 @@ void defineCreateTests() { test('no directory given', () { p = project(); - ProcessResult result = p.runSync([ - 'create', - ]); + ProcessResult result = p.runSync('create', []); expect(result.exitCode, 1); }); @@ -60,7 +58,7 @@ void defineCreateTests() { p = project(); ProcessResult result = p.runSync( - ['create', '--template', CreateCommand.defaultTemplateId, p.dir.path]); + 'create', ['--template', CreateCommand.defaultTemplateId, p.dir.path]); expect(result.exitCode, 73); }); @@ -68,7 +66,7 @@ void defineCreateTests() { p = project(); ProcessResult result = - p.runSync(['create', '--no-pub', '--template', 'foo-bar', p.dir.path]); + p.runSync('create', ['--no-pub', '--template', 'foo-bar', p.dir.path]); expect(result.exitCode, isNot(0)); }); @@ -78,7 +76,7 @@ void defineCreateTests() { p = project(); ProcessResult result = p - .runSync(['create', '--force', '--template', templateId, p.dir.path]); + .runSync('create', ['--force', '--template', templateId, p.dir.path]); expect(result.exitCode, 0); String projectName = path.basename(p.dir.path); diff --git a/pkg/dartdev/test/commands/fix_test.dart b/pkg/dartdev/test/commands/fix_test.dart index 8a92d7bca8d..40c7860d7d7 100644 --- a/pkg/dartdev/test/commands/fix_test.dart +++ b/pkg/dartdev/test/commands/fix_test.dart @@ -29,9 +29,9 @@ void defineFix() { ProcessResult runFix(List args, {String workingDir}) { if (runFromSource) { var binary = path.join(Directory.current.path, 'bin', 'dartdev.dart'); - return p.runSync([binary, 'fix', ...?args], workingDir: workingDir); + return p.runSync(binary, ['fix', ...?args], workingDir: workingDir); } - return p.runSync(['fix', ...args], workingDir: workingDir); + return p.runSync('fix', args, workingDir: workingDir); } test('none', () { @@ -65,7 +65,6 @@ linter: - prefer_single_quotes ''', ); - var result = runFix(['--apply'], workingDir: p.dirPath); expect(result.exitCode, 0); expect(result.stderr, isEmpty); diff --git a/pkg/dartdev/test/commands/flag_test.dart b/pkg/dartdev/test/commands/flag_test.dart index 0475ee3c9ee..6416ffcd832 100644 --- a/pkg/dartdev/test/commands/flag_test.dart +++ b/pkg/dartdev/test/commands/flag_test.dart @@ -6,6 +6,7 @@ import 'dart:io'; import 'package:args/command_runner.dart'; import 'package:dartdev/dartdev.dart'; +import 'package:dartdev/src/analytics.dart' show disabledAnalytics; import 'package:test/test.dart'; import '../utils.dart'; @@ -19,7 +20,7 @@ void command() { // For each command description, assert that the values are not empty, don't // have trailing white space and end with a period. test('description formatting', () { - DartdevRunner(['--no-analytics']) + DartdevRunner(['--disable-dartdev-analytics'], disabledAnalytics) .commands .forEach((String commandKey, Command command) { expect(commandKey, isNotEmpty); @@ -31,7 +32,7 @@ void command() { // Assert that all found usageLineLengths are the same and null test('argParser usageLineLength', () { - DartdevRunner(['--no-analytics']) + DartdevRunner(['--disable-dartdev-analytics'], disabledAnalytics) .commands .forEach((String commandKey, Command command) { if (command.argParser != null) { @@ -61,7 +62,7 @@ void help() { test('--help', () { p = project(); - var result = p.runSync(['--help']); + var result = p.runSync('--help', []); expect(result.exitCode, 0); expect(result.stderr, isEmpty); @@ -79,7 +80,7 @@ void help() { test('--help --verbose', () { p = project(); - var result = p.runSync(['--help', '--verbose']); + var result = p.runSync('--help', ['--verbose']); expect(result.exitCode, 0); expect(result.stdout, isEmpty); @@ -89,7 +90,7 @@ void help() { test('--help -v', () { p = project(); - var result = p.runSync(['--help', '-v']); + var result = p.runSync('--help', ['-v']); expect(result.exitCode, 0); expect(result.stdout, isEmpty); @@ -99,7 +100,7 @@ void help() { test('help', () { p = project(); - var result = p.runSync(['help']); + var result = p.runSync('help', []); expect(result.exitCode, 0); expect(result.stderr, isEmpty); @@ -117,7 +118,7 @@ void help() { test('help --verbose', () { p = project(); - var result = p.runSync(['help', '--verbose']); + var result = p.runSync('help', ['--verbose']); expect(result.exitCode, 0); expect(result.stdout, contains('migrate ')); @@ -125,7 +126,7 @@ void help() { test('help -v', () { p = project(); - var result = p.runSync(['help', '-v']); + var result = p.runSync('help', ['-v']); expect(result.exitCode, 0); expect(result.stdout, contains('migrate ')); diff --git a/pkg/dartdev/test/commands/format_test.dart b/pkg/dartdev/test/commands/format_test.dart index 6f367955c92..cded95bdbb1 100644 --- a/pkg/dartdev/test/commands/format_test.dart +++ b/pkg/dartdev/test/commands/format_test.dart @@ -19,7 +19,7 @@ void format() { test('--help', () { p = project(); - var result = p.runSync(['format', '--help']); + var result = p.runSync('format', ['--help']); expect(result.exitCode, 0); expect(result.stderr, isEmpty); expect(result.stdout, contains('Idiomatically format Dart source code.')); @@ -32,7 +32,7 @@ void format() { test('--help --verbose', () { p = project(); - var result = p.runSync(['format', '--help', '--verbose']); + var result = p.runSync('format', ['--help', '--verbose']); expect(result.exitCode, 0); expect(result.stderr, isEmpty); expect(result.stdout, contains('Idiomatically format Dart source code.')); @@ -45,7 +45,7 @@ void format() { test('unchanged', () { p = project(mainSrc: 'int get foo => 1;\n'); - ProcessResult result = p.runSync(['format', p.relativeFilePath]); + ProcessResult result = p.runSync('format', [p.relativeFilePath]); expect(result.exitCode, 0); expect(result.stderr, isEmpty); expect(result.stdout, startsWith('Formatted 1 file (0 changed) in ')); @@ -53,7 +53,7 @@ void format() { test('formatted', () { p = project(mainSrc: 'int get foo => 1;\n'); - ProcessResult result = p.runSync(['format', p.relativeFilePath]); + ProcessResult result = p.runSync('format', [p.relativeFilePath]); expect(result.exitCode, 0); expect(result.stderr, isEmpty); expect( @@ -65,7 +65,7 @@ void format() { test('unknown file', () { p = project(mainSrc: 'int get foo => 1;\n'); var unknownFilePath = '${p.relativeFilePath}-unknown-file.dart'; - ProcessResult result = p.runSync(['format', unknownFilePath]); + ProcessResult result = p.runSync('format', [unknownFilePath]); expect(result.exitCode, 0); expect(result.stderr, startsWith('No file or directory found at "$unknownFilePath".')); diff --git a/pkg/dartdev/test/commands/help_test.dart b/pkg/dartdev/test/commands/help_test.dart index afbaf0955e8..c272e439e64 100644 --- a/pkg/dartdev/test/commands/help_test.dart +++ b/pkg/dartdev/test/commands/help_test.dart @@ -4,6 +4,7 @@ import 'package:args/command_runner.dart'; import 'package:dartdev/dartdev.dart'; +import 'package:dartdev/src/analytics.dart' show disabledAnalytics; import 'package:test/test.dart'; import '../utils.dart'; @@ -21,14 +22,14 @@ void help() { List _commandsNotTested = [ 'help', // `dart help help` is redundant ]; - DartdevRunner(['--no-analytics']) + DartdevRunner(['--disable-dartdev-analytics'], disabledAnalytics) .commands .forEach((String commandKey, Command command) { if (!_commandsNotTested.contains(commandKey)) { test('(help $commandKey == $commandKey --help)', () { p = project(); - var result = p.runSync(['help', commandKey]); - var verbHelpResult = p.runSync([commandKey, '--help']); + var result = p.runSync('help', [commandKey]); + var verbHelpResult = p.runSync(commandKey, ['--help']); expect(result.stdout, contains(verbHelpResult.stdout)); expect(result.stderr, contains(verbHelpResult.stderr)); @@ -38,15 +39,15 @@ void help() { test('(help pub == pub --help)', () { p = project(); - var result = p.runSync(['help', 'pub']); - var pubHelpResult = p.runSync(['pub', '--help']); + var result = p.runSync('help', ['pub']); + var pubHelpResult = p.runSync('pub', ['--help']); expect(result.stdout, contains(pubHelpResult.stdout)); expect(result.stderr, contains(pubHelpResult.stderr)); }); test('(--help flags also have -h abbr)', () { - DartdevRunner(['--no-analytics']) + DartdevRunner(['--disable-dartdev-analytics'], disabledAnalytics) .commands .forEach((String commandKey, Command command) { var helpOption = command.argParser.options['help']; diff --git a/pkg/dartdev/test/commands/migrate_test.dart b/pkg/dartdev/test/commands/migrate_test.dart index 681d550667d..81016403974 100644 --- a/pkg/dartdev/test/commands/migrate_test.dart +++ b/pkg/dartdev/test/commands/migrate_test.dart @@ -21,7 +21,7 @@ void defineMigrateTests() { test('--help', () { p = project(); - var result = p.runSync(['migrate', '--help']); + var result = p.runSync('migrate', ['--help']); expect(result.exitCode, 0); expect(result.stderr, isEmpty); @@ -34,7 +34,7 @@ void defineMigrateTests() { test('directory implicit', () { p = project(mainSrc: dartVersionFilePrefix2_9 + 'int get foo => 1;\n'); var result = - p.runSync(['migrate', '--no-web-preview'], workingDir: p.dirPath); + p.runSync('migrate', ['--no-web-preview'], workingDir: p.dirPath); expect(result.exitCode, 0); expect(result.stderr, isEmpty); expect(result.stdout, contains('Generating migration suggestions')); @@ -42,7 +42,7 @@ void defineMigrateTests() { test('directory explicit', () { p = project(mainSrc: dartVersionFilePrefix2_9 + 'int get foo => 1;\n'); - var result = p.runSync(['migrate', '--no-web-preview', p.dirPath]); + var result = p.runSync('migrate', ['--no-web-preview', p.dirPath]); expect(result.exitCode, 0); expect(result.stderr, isEmpty); expect(result.stdout, contains('Generating migration suggestions')); @@ -50,7 +50,7 @@ void defineMigrateTests() { test('bad directory', () { p = project(mainSrc: 'int get foo => 1;\n'); - var result = p.runSync(['migrate', 'foo_bar_dir']); + var result = p.runSync('migrate', ['foo_bar_dir']); expect(result.exitCode, 1); expect(result.stderr, contains('foo_bar_dir does not exist')); expect(result.stdout, isEmpty); @@ -58,7 +58,7 @@ void defineMigrateTests() { test('pub get needs running', () { p = project(mainSrc: 'import "package:foo/foo.dart";\n'); - var result = p.runSync(['migrate', p.dirPath]); + var result = p.runSync('migrate', [p.dirPath]); expect(result.exitCode, 1); expect(result.stderr, isEmpty); expect(result.stdout, runPubGet); @@ -67,7 +67,7 @@ void defineMigrateTests() { test('non-pub-related error', () { p = project(mainSrc: 'var missing = "semicolon"\n'); - var result = p.runSync(['migrate', p.dirPath]); + var result = p.runSync('migrate', [p.dirPath]); expect(result.exitCode, 1); expect(result.stderr, isEmpty); expect(result.stdout, runPubGet); diff --git a/pkg/dartdev/test/commands/pub_test.dart b/pkg/dartdev/test/commands/pub_test.dart index e1648585f34..82f00d03e62 100644 --- a/pkg/dartdev/test/commands/pub_test.dart +++ b/pkg/dartdev/test/commands/pub_test.dart @@ -26,7 +26,7 @@ void pub() { } test('implicit --help', () { - final result = project().runSync(['pub']); + final result = project().runSync('pub', []); expect(result, isNotNull); expect(result.exitCode, 64); expect(result.stderr, contains('Missing subcommand for "dart pub".')); @@ -35,17 +35,17 @@ void pub() { }); test('--help', () { - _assertPubHelpInvoked(project().runSync(['pub', '--help'])); + _assertPubHelpInvoked(project().runSync('pub', ['--help'])); }); test('-h', () { - _assertPubHelpInvoked(project().runSync(['pub', '-h'])); + _assertPubHelpInvoked(project().runSync('pub', ['-h'])); }); test('help cache', () { p = project(); - var result = p.runSync(['help', 'pub', 'cache']); - var result2 = p.runSync(['pub', 'cache', '--help']); + var result = p.runSync('help', ['pub', 'cache']); + var result2 = p.runSync('pub', ['cache', '--help']); expect(result.exitCode, 0); @@ -58,8 +58,8 @@ void pub() { test('help publish', () { p = project(); - var result = p.runSync(['help', 'pub', 'publish']); - var result2 = p.runSync(['pub', 'publish', '--help']); + var result = p.runSync('help', ['pub', 'publish']); + var result2 = p.runSync('pub', ['publish', '--help']); expect(result.exitCode, 0); @@ -77,10 +77,10 @@ void pub() { "void main() { int? a; a = null; print('a is \$a.'); }"); // run 'pub get' - p.runSync(['pub', 'get']); + p.runSync('pub', ['get']); var result = p.runSync( - ['pub', 'run', '--enable-experiment=no-non-nullable', 'main.dart']); + 'pub', ['run', '--enable-experiment=no-non-nullable', 'main.dart']); expect(result.exitCode, 254); expect(result.stdout, isEmpty); @@ -93,7 +93,7 @@ void pub() { test('failure', () { p = project(mainSrc: 'int get foo => 1;\n'); - var result = p.runSync(['pub', 'deps']); + var result = p.runSync('pub', ['deps']); expect(result.exitCode, 65); expect(result.stdout, isEmpty); expect(result.stderr, contains('No pubspec.lock file found')); @@ -101,7 +101,7 @@ void pub() { test('failure unknown option', () { p = project(mainSrc: 'int get foo => 1;\n'); - var result = p.runSync(['pub', 'deps', '--foo']); + var result = p.runSync('pub', ['deps', '--foo']); expect(result.exitCode, 64); expect(result.stdout, isEmpty); expect(result.stderr, startsWith('Could not find an option named "foo".')); diff --git a/pkg/dartdev/test/commands/run_test.dart b/pkg/dartdev/test/commands/run_test.dart index 59bffbac318..9d2125c0642 100644 --- a/pkg/dartdev/test/commands/run_test.dart +++ b/pkg/dartdev/test/commands/run_test.dart @@ -20,7 +20,7 @@ void run() { test('--help', () { p = project(); - var result = p.runSync(['run', '--help']); + var result = p.runSync('run', ['--help']); expect(result.stdout, contains('Run a Dart program.')); expect(result.stdout, contains('Debugging options:')); @@ -30,7 +30,7 @@ void run() { test("'Hello World'", () { p = project(mainSrc: "void main() { print('Hello World'); }"); - ProcessResult result = p.runSync(['run', p.relativeFilePath]); + ProcessResult result = p.runSync('run', [p.relativeFilePath]); expect(result.stdout, contains('Hello World')); expect(result.stderr, isEmpty); @@ -40,7 +40,7 @@ void run() { test('no such file', () { p = project(mainSrc: "void main() { print('Hello World'); }"); ProcessResult result = - p.runSync(['run', 'no/such/file/${p.relativeFilePath}']); + p.runSync('run', ['no/such/file/${p.relativeFilePath}']); expect(result.stderr, isNotEmpty); expect(result.exitCode, isNot(0)); @@ -51,7 +51,7 @@ void run() { // name (package name) will be the name of the temporary directory on disk p = project(mainSrc: "void main() { print('Hello World'); }"); p.file('bin/main.dart', "void main() { print('Hello main.dart'); }"); - ProcessResult result = p.runSync(['run']); + ProcessResult result = p.runSync('run', []); expect(result.stdout, contains('Hello main.dart')); expect(result.stderr, isEmpty); @@ -62,7 +62,7 @@ void run() { test('missing implicit packageName.dart', () { p = project(mainSrc: "void main() { print('Hello World'); }"); p.file('bin/foo.dart', "void main() { print('Hello main.dart'); }"); - ProcessResult result = p.runSync(['run']); + ProcessResult result = p.runSync('run', []); expect(result.stdout, isEmpty); expect( @@ -75,8 +75,7 @@ void run() { test('arguments are properly passed', () { p = project(); p.file('main.dart', 'void main(args) { print(args); }'); - ProcessResult result = p.runSync([ - 'run', + ProcessResult result = p.runSync('run', [ '--enable-experiment=triple-shift', 'main.dart', 'argument1', @@ -112,7 +111,7 @@ import 'package:foo/foo.dart'; void main(List args) => print("$b $args"); '''); - ProcessResult result = p.runSync(['run', 'bar:main', '--arg1', 'arg2']); + ProcessResult result = p.runSync('run', ['bar:main', '--arg1', 'arg2']); expect(result.stderr, isEmpty); expect(result.stdout, contains('FOO BAR [--arg1, arg2]')); @@ -127,8 +126,7 @@ void main(List args) => print("$b $args"); p.file('main.dart', 'void main(args) { print(args); }'); // Test with absolute path final name = path.join(p.dirPath, 'main.dart'); - final result = p.runSync([ - 'run', + final result = p.runSync('run', [ '--enable-experiment=triple-shift', name, '--argument1', @@ -146,8 +144,7 @@ void main(List args) => print("$b $args"); p.file('main.dart', 'void main(args) { print(args); }'); // Test with File uri final name = path.join(p.dirPath, 'main.dart'); - final result = p.runSync([ - 'run', + final result = p.runSync('run', [ Uri.file(name).toString(), '--argument1', 'argument2', @@ -170,8 +167,7 @@ void main(List args) => print("$b $args"); // // This test ensures that allowed arguments for dart run which are valid VM // arguments are properly handled by the VM. - ProcessResult result = p.runSync([ - 'run', + ProcessResult result = p.runSync('run', [ '--observe', '--pause-isolates-on-start', // This should negate the above flag. @@ -190,8 +186,7 @@ void main(List args) => print("$b $args"); expect(result.exitCode, 0); // Again, with --disable-service-auth-codes. - result = p.runSync([ - 'run', + result = p.runSync('run', [ '--observe', '--pause-isolates-on-start', // This should negate the above flag. @@ -216,8 +211,7 @@ void main(List args) => print("$b $args"); // Any VM flags not listed under 'dart run help --verbose' should be passed // before a dartdev command. - ProcessResult result = p.runSync([ - 'run', + ProcessResult result = p.runSync('run', [ '--vm-name=foo', p.relativeFilePath, ]); @@ -235,8 +229,7 @@ void main(List args) => print("$b $args"); // Any VM flags not listed under 'dart run help --verbose' should be passed // before a dartdev command. - ProcessResult result = p.runSync([ - 'run', + ProcessResult result = p.runSync('run', [ '--verbose_gc', p.relativeFilePath, ]); @@ -254,8 +247,7 @@ void main(List args) => print("$b $args"); // Ensure --enable-asserts doesn't cause the dartdev isolate to fail to // load. Regression test for: https://github.com/dart-lang/sdk/issues/42831 - ProcessResult result = p.runSync([ - 'run', + ProcessResult result = p.runSync('run', [ '--enable-asserts', p.relativeFilePath, ]); @@ -269,8 +261,7 @@ void main(List args) => print("$b $args"); p = project(mainSrc: 'void main() { assert(false); }'); // Any VM flags passed after the script shouldn't be interpreted by the VM. - ProcessResult result = p.runSync([ - 'run', + ProcessResult result = p.runSync('run', [ p.relativeFilePath, '--enable-asserts', ]); diff --git a/pkg/dartdev/test/commands/test_test.dart b/pkg/dartdev/test/commands/test_test.dart index 00be2bc4674..ff859825ded 100644 --- a/pkg/dartdev/test/commands/test_test.dart +++ b/pkg/dartdev/test/commands/test_test.dart @@ -21,7 +21,7 @@ void defineTest() { test('--help', () { p = project(); - final result = p.runSync(['test', '--help']); + final result = p.runSync('test', ['--help']); expect(result.exitCode, 0); expect(result.stdout, contains(' tests in this package')); @@ -31,7 +31,7 @@ void defineTest() { test('dart help test', () { p = project(); - final result = p.runSync(['help', 'test']); + final result = p.runSync('help', ['test']); expect(result.exitCode, 0); expect(result.stdout, contains(' tests in this package')); @@ -43,7 +43,7 @@ void defineTest() { var pubspec = File(path.join(p.dirPath, 'pubspec.yaml')); pubspec.deleteSync(); - var result = p.runSync(['test']); + var result = p.runSync('test', []); expect(result.stderr, isEmpty); expect(result.stdout, contains('No pubspec.yaml file found')); @@ -63,7 +63,7 @@ void main() { '''); // An implicit `pub get` will happen. - final result = p.runSync(['test', '--no-color', '--reporter', 'expanded']); + final result = p.runSync('test', ['--no-color', '--reporter', 'expanded']); expect(result.stderr, isEmpty); expect(result.stdout, contains('All tests passed!')); expect(result.exitCode, 0); @@ -86,8 +86,7 @@ void main() { } '''); - final result = p.runSync(['test']); - expect(result.exitCode, 65); + final result = p.runSync('test', []); expect( result.stdout, contains('You need to add a dependency on package:test'), @@ -95,10 +94,10 @@ void main() { expect(result.stderr, isEmpty); expect(result.exitCode, 65); - final resultPubAdd = p.runSync(['pub', 'add', 'test']); + final resultPubAdd = p.runSync('pub', ['add', 'test']); expect(resultPubAdd.exitCode, 0); - final result2 = p.runSync(['test', '--no-color', '--reporter', 'expanded']); + final result2 = p.runSync('test', ['--no-color', '--reporter', 'expanded']); expect(result2.stderr, isEmpty); expect(result2.stdout, contains('All tests passed!')); expect(result2.exitCode, 0); @@ -118,7 +117,7 @@ void main() { } '''); - final result = p.runSync(['test', '--no-color', '--reporter', 'expanded']); + final result = p.runSync('test', ['--no-color', '--reporter', 'expanded']); expect(result.exitCode, 0); expect(result.stdout, contains('All tests passed!')); expect(result.stderr, isEmpty); @@ -139,13 +138,8 @@ void main() { '''); final result = p.runSync( - [ - '--enable-experiment=non-nullable', - 'test', - '--no-color', - '--reporter', - 'expanded', - ], + '--enable-experiment=non-nullable', + ['test', '--no-color', '--reporter', 'expanded'], ); expect(result.exitCode, 1); }); diff --git a/pkg/dartdev/test/no_such_file_test.dart b/pkg/dartdev/test/no_such_file_test.dart index b5a3452a66b..1d197d7e895 100644 --- a/pkg/dartdev/test/no_such_file_test.dart +++ b/pkg/dartdev/test/no_such_file_test.dart @@ -14,12 +14,12 @@ void main() { test('Ensure parsing fails after encountering invalid file', () { // Regression test for https://github.com/dart-lang/sdk/issues/43991 p = project(); - final noArgsResult = p.runSync(['foo.dart']); + final noArgsResult = p.runSync('foo.dart', []); expect(noArgsResult.stderr, isNotEmpty); expect(noArgsResult.stdout, isEmpty); expect(noArgsResult.exitCode, 64); - final argsResult = p.runSync(['foo.dart', '--bar']); + final argsResult = p.runSync('foo.dart', ['--bar']); expect(argsResult.stderr, noArgsResult.stderr); expect(argsResult.stdout, isEmpty); expect(argsResult.exitCode, 64); @@ -29,7 +29,7 @@ void main() { () { // Regression test for https://github.com/dart-lang/sdk/issues/43785 p = project(); - final result = p.runSync(['--snapshot=abc', 'foo.dart']); + final result = p.runSync('--snapshot=abc', ['foo.dart']); expect(result.stderr, isNotEmpty); expect(result.stderr, contains("Error when reading 'foo.dart':")); expect(result.stdout, isEmpty); diff --git a/pkg/dartdev/test/utils.dart b/pkg/dartdev/test/utils.dart index 48dcfd7acd9..a8de01618b1 100644 --- a/pkg/dartdev/test/utils.dart +++ b/pkg/dartdev/test/utils.dart @@ -75,15 +75,17 @@ dev_dependencies: } ProcessResult runSync( - List arguments, { + String command, + List args, { String workingDir, }) { - return Process.runSync( - Platform.resolvedExecutable, - [ - '--no-analytics', - ...arguments, - ], + var arguments = [ + command, + ...?args, + ]; + + arguments.add('--disable-dartdev-analytics'); + return Process.runSync(Platform.resolvedExecutable, arguments, workingDirectory: workingDir ?? dir.path, environment: {if (logAnalytics) '_DARTDEV_LOG_ANALYTICS': 'true'}); } diff --git a/pkg/dartdev/test/utils_test.dart b/pkg/dartdev/test/utils_test.dart index 199d461696b..9740dd1ca50 100644 --- a/pkg/dartdev/test/utils_test.dart +++ b/pkg/dartdev/test/utils_test.dart @@ -100,6 +100,39 @@ void main() { expect(File('bar.bart').name, 'bar.bart'); }); }); + + group('PubUtils', () { + test('doModifyArgs', () { + const allCmds = ['analyze', 'help', 'pub', 'migrate']; + expect(PubUtils.shouldModifyArgs(null, null), isFalse); + expect(PubUtils.shouldModifyArgs([], null), isFalse); + expect(PubUtils.shouldModifyArgs(null, []), isFalse); + expect(PubUtils.shouldModifyArgs([], []), isFalse); + expect(PubUtils.shouldModifyArgs(['-h'], allCmds), isFalse); + expect(PubUtils.shouldModifyArgs(['--help'], allCmds), isFalse); + expect(PubUtils.shouldModifyArgs(['help'], allCmds), isFalse); + expect(PubUtils.shouldModifyArgs(['pub'], allCmds), isFalse); + expect(PubUtils.shouldModifyArgs(['analyze', 'help', 'pub'], allCmds), + isFalse); + + expect(PubUtils.shouldModifyArgs(['--some-flag', 'help', 'pub'], allCmds), + isTrue); + expect(PubUtils.shouldModifyArgs(['help', 'pub'], allCmds), isTrue); + expect(PubUtils.shouldModifyArgs(['help', 'pub', 'publish'], allCmds), + isTrue); + expect(PubUtils.shouldModifyArgs(['help', 'pub', 'analyze'], allCmds), + isTrue); + }); + + test('modifyArgs', () { + expect(PubUtils.modifyArgs(['--some-flag', 'help', 'pub']), + orderedEquals(['--some-flag', 'pub', '--help'])); + expect(PubUtils.modifyArgs(['help', 'pub']), + orderedEquals(['pub', '--help'])); + expect(PubUtils.modifyArgs(['help', 'pub', 'publish']), + orderedEquals(['pub', 'publish', '--help'])); + }); + }); } const String _packageData = '''{ diff --git a/runtime/bin/gen_snapshot.cc b/runtime/bin/gen_snapshot.cc index 3815b9a8e4c..72e10c866b9 100644 --- a/runtime/bin/gen_snapshot.cc +++ b/runtime/bin/gen_snapshot.cc @@ -209,11 +209,15 @@ static int ParseArguments(int argc, char** argv, CommandLineOptions* vm_options, CommandLineOptions* inputs) { + const char* kPrefix = "-"; + const intptr_t kPrefixLen = strlen(kPrefix); + // Skip the binary name. int i = 1; // Parse out the vm options. - while ((i < argc) && OptionProcessor::IsValidFlag(argv[i])) { + while ((i < argc) && + OptionProcessor::IsValidFlag(argv[i], kPrefix, kPrefixLen)) { if (OptionProcessor::TryProcess(argv[i], vm_options)) { i += 1; continue; diff --git a/runtime/bin/main.cc b/runtime/bin/main.cc index 7299ade79ce..76d3b51e581 100644 --- a/runtime/bin/main.cc +++ b/runtime/bin/main.cc @@ -1140,10 +1140,10 @@ void main(int argc, char** argv) { // Parse command line arguments. if (app_snapshot == nullptr) { - bool success = Options::ParseArguments( + int result = Options::ParseArguments( argc, argv, vm_run_app_snapshot, &vm_options, &script_name, &dart_options, &print_flags_seen, &verbose_debug_seen); - if (!success) { + if (result < 0) { if (Options::help_option()) { Options::PrintUsage(); Platform::Exit(0); diff --git a/runtime/bin/main_options.cc b/runtime/bin/main_options.cc index d019ce4ec76..2e962c100d0 100644 --- a/runtime/bin/main_options.cc +++ b/runtime/bin/main_options.cc @@ -282,26 +282,6 @@ bool Options::ExtractPortAndAddress(const char* option_value, return true; } -// Returns true if arg starts with the characters "--" followed by option, but -// all '_' in the option name are treated as '-'. -static bool IsOption(const char* arg, const char* option) { - if (arg[0] != '-' || arg[1] != '-') { - // Special case first two characters to avoid recognizing __flag. - return false; - } - for (int i = 0; option[i] != '\0'; i++) { - auto c = arg[i + 2]; - if (c == '\0') { - // Not long enough. - return false; - } - if ((c == '_' ? '-' : c) != option[i]) { - return false; - } - } - return true; -} - const char* Options::vm_service_server_ip_ = DEFAULT_VM_SERVICE_SERVER_IP; int Options::vm_service_server_port_ = INVALID_VM_SERVICE_SERVER_PORT; bool Options::ProcessEnableVmServiceOption(const char* arg, @@ -386,14 +366,17 @@ bool Options::ProcessVMDebuggingOptions(const char* arg, return false; } -bool Options::ParseArguments(int argc, - char** argv, - bool vm_run_app_snapshot, - CommandLineOptions* vm_options, - char** script_name, - CommandLineOptions* dart_options, - bool* print_flags_seen, - bool* verbose_debug_seen) { +int Options::ParseArguments(int argc, + char** argv, + bool vm_run_app_snapshot, + CommandLineOptions* vm_options, + char** script_name, + CommandLineOptions* dart_options, + bool* print_flags_seen, + bool* verbose_debug_seen) { + const char* kPrefix = "--"; + const intptr_t kPrefixLen = strlen(kPrefix); + // Store the executable name. Platform::SetExecutableName(argv[0]); @@ -412,26 +395,43 @@ bool Options::ParseArguments(int argc, i++; } else { // Check if this flag is a potentially valid VM flag. - if (!OptionProcessor::IsValidFlag(argv[i])) { + if (!OptionProcessor::IsValidFlag(argv[i], kPrefix, kPrefixLen)) { break; } - // The following flags are processed as DartDev flags and are not to + // The following two flags are processed by both the embedder and + // the VM. + const char* kPrintFlags1 = "--print-flags"; + const char* kPrintFlags2 = "--print_flags"; + const char* kVerboseDebug1 = "--verbose_debug"; + const char* kVerboseDebug2 = "--verbose-debug"; + + // The following two flags are processed as DartDev flags and are not to // be treated as if they are VM flags. - if (IsOption(argv[i], "print-flags")) { + const char* kEnableDartDevAnalytics1 = "--enable-analytics"; + const char* kEnableDartDevAnalytics2 = "--enable_analytics"; + const char* kDisableDartDevAnalytics1 = "--disable-analytics"; + const char* kDisableDartDevAnalytics2 = "--disable_analytics"; + + if ((strncmp(argv[i], kPrintFlags1, strlen(kPrintFlags1)) == 0) || + (strncmp(argv[i], kPrintFlags2, strlen(kPrintFlags2)) == 0)) { *print_flags_seen = true; - } else if (IsOption(argv[i], "verbose-debug")) { + } else if ((strncmp(argv[i], kVerboseDebug1, strlen(kVerboseDebug1)) == + 0) || + (strncmp(argv[i], kVerboseDebug2, strlen(kVerboseDebug2)) == + 0)) { *verbose_debug_seen = true; - } else if (IsOption(argv[i], "enable-analytics")) { + } else if ((strncmp(argv[i], kEnableDartDevAnalytics1, + strlen(kEnableDartDevAnalytics1)) == 0) || + (strncmp(argv[i], kEnableDartDevAnalytics2, + strlen(kEnableDartDevAnalytics2)) == 0)) { enable_dartdev_analytics = true; skipVmOption = true; - } else if (IsOption(argv[i], "disable-analytics")) { + } else if ((strncmp(argv[i], kDisableDartDevAnalytics1, + strlen(kDisableDartDevAnalytics1)) == 0) || + (strncmp(argv[i], kDisableDartDevAnalytics2, + strlen(kDisableDartDevAnalytics2)) == 0)) { disable_dartdev_analytics = true; skipVmOption = true; - } else if (IsOption(argv[i], "no-analytics")) { - // Just add this option even if we don't go to dartdev. - // It is irelevant for the vm. - dart_options->AddArgument("--no-analytics"); - skipVmOption = true; } if (!skipVmOption) { temp_vm_options.AddArgument(argv[i]); @@ -465,6 +465,7 @@ bool Options::ParseArguments(int argc, bool implicitly_use_dart_dev = false; bool run_script = false; + // Get the script name. if (i < argc) { #if !defined(DART_PRECOMPILED_RUNTIME) @@ -505,19 +506,25 @@ bool Options::ParseArguments(int argc, DartDevIsolate::set_should_run_dart_dev(true); // Let DartDev handle the default help message. dart_options->AddArgument("help"); - return true; + return 0; } else if (!Options::disable_dart_dev() && (enable_dartdev_analytics || disable_dartdev_analytics)) { // The analytics flags are a special case as we don't have a target script // or DartDev command but we still want to launch DartDev. DartDevIsolate::set_should_run_dart_dev(true); - return true; + if (enable_dartdev_analytics) { + dart_options->AddArgument("--enable-analytics"); + } + if (disable_dartdev_analytics) { + dart_options->AddArgument("--disable-analytics"); + } + return 0; } #endif // !defined(DART_PRECOMPILED_RUNTIME) else { // NOLINT - return false; + return -1; } USE(enable_dartdev_analytics); USE(disable_dartdev_analytics); @@ -540,7 +547,7 @@ bool Options::ParseArguments(int argc, while (tmp_i < argc) { // Check if this flag is a potentially valid VM flag. If not, we've likely // hit a script name and are done parsing VM flags. - if (!OptionProcessor::IsValidFlag(argv[tmp_i])) { + if (!OptionProcessor::IsValidFlag(argv[tmp_i], kPrefix, kPrefixLen)) { break; } OptionProcessor::TryProcess(argv[tmp_i], vm_options); @@ -589,12 +596,14 @@ bool Options::ParseArguments(int argc, first_option = false; } } + + // Verify consistency of arguments. // snapshot_depfile is an alias for depfile. Passing them both is an error. if ((snapshot_deps_filename_ != NULL) && (depfile_ != NULL)) { Syslog::PrintErr("Specify only one of --depfile and --snapshot_depfile\n"); - return false; + return -1; } if (snapshot_deps_filename_ != NULL) { depfile_ = snapshot_deps_filename_; @@ -603,25 +612,25 @@ bool Options::ParseArguments(int argc, if ((packages_file_ != NULL) && (strlen(packages_file_) == 0)) { Syslog::PrintErr("Empty package file name specified.\n"); - return false; + return -1; } if ((gen_snapshot_kind_ != kNone) && (snapshot_filename_ == NULL)) { Syslog::PrintErr( "Generating a snapshot requires a filename (--snapshot).\n"); - return false; + return -1; } if ((gen_snapshot_kind_ == kNone) && (depfile_ != NULL) && (snapshot_filename_ == NULL) && (depfile_output_filename_ == NULL)) { Syslog::PrintErr( "Generating a depfile requires an output filename" " (--depfile-output-filename or --snapshot).\n"); - return false; + return -1; } if ((gen_snapshot_kind_ != kNone) && vm_run_app_snapshot) { Syslog::PrintErr( "Specifying an option to generate a snapshot and" " run using a snapshot is invalid.\n"); - return false; + return -1; } // If --snapshot is given without --snapshot-kind, default to script snapshot. @@ -629,7 +638,7 @@ bool Options::ParseArguments(int argc, gen_snapshot_kind_ = kKernel; } - return true; + return 0; } } // namespace bin diff --git a/runtime/bin/main_options.h b/runtime/bin/main_options.h index 2ab8598029f..f900a3c89a8 100644 --- a/runtime/bin/main_options.h +++ b/runtime/bin/main_options.h @@ -85,15 +85,14 @@ static constexpr int INVALID_VM_SERVICE_SERVER_PORT = -1; class Options { public: - // Returns true if argument parsing succeeded. False otherwise. - static bool ParseArguments(int argc, - char** argv, - bool vm_run_app_shapshot, - CommandLineOptions* vm_options, - char** script_name, - CommandLineOptions* dart_options, - bool* print_flags_seen, - bool* verbose_debug_seen); + static int ParseArguments(int argc, + char** argv, + bool vm_run_app_shapshot, + CommandLineOptions* vm_options, + char** script_name, + CommandLineOptions* dart_options, + bool* print_flags_seen, + bool* verbose_debug_seen); #define STRING_OPTION_GETTER(flag, variable) \ static const char* variable() { return variable##_; } diff --git a/runtime/bin/options.cc b/runtime/bin/options.cc index 6d4e3556fba..1eed113d060 100644 --- a/runtime/bin/options.cc +++ b/runtime/bin/options.cc @@ -9,8 +9,12 @@ namespace bin { OptionProcessor* OptionProcessor::first_ = NULL; -bool OptionProcessor::IsValidFlag(const char* name) { - return name[0] == '-' && name[1] == '-' && name[2] != '\0'; +bool OptionProcessor::IsValidFlag(const char* name, + const char* prefix, + intptr_t prefix_length) { + const intptr_t name_length = strlen(name); + return ((name_length > prefix_length) && + (strncmp(name, prefix, prefix_length) == 0)); } const char* OptionProcessor::ProcessOption(const char* option, diff --git a/runtime/bin/options.h b/runtime/bin/options.h index ef925790347..4bcd7debccd 100644 --- a/runtime/bin/options.h +++ b/runtime/bin/options.h @@ -22,8 +22,9 @@ class OptionProcessor { virtual ~OptionProcessor() {} - // Returns true if name starts with "--". - static bool IsValidFlag(const char* name); + static bool IsValidFlag(const char* name, + const char* prefix, + intptr_t prefix_length); virtual bool Process(const char* option, CommandLineOptions* options) = 0;