diff --git a/pkg/dartfix/lib/src/driver.dart b/pkg/dartfix/lib/src/driver.dart index 549f41d2b445..78951b1d06cc 100644 --- a/pkg/dartfix/lib/src/driver.dart +++ b/pkg/dartfix/lib/src/driver.dart @@ -36,7 +36,7 @@ class Driver { Context testContext, Logger testLogger, }) async { - final Options options = Options.parse(args); + final Options options = Options.parse(args, testContext, testLogger); force = options.force; overwrite = options.overwrite; @@ -48,7 +48,7 @@ class Driver { // Start showing progress before we start the analysis server. Progress progress; - if (options.listFixes) { + if (options.showHelp) { progress = logger.progress('${ansi.emphasized('Listing fixes')}'); } else { progress = logger.progress('${ansi.emphasized('Calculating fixes')}'); @@ -63,26 +63,26 @@ class Driver { context.exit(1); } - if (options.listFixes) { + if (options.showHelp) { try { await showListOfFixes(progress: progress); } finally { await server.stop(); } - } else { - Future serverStopped; + context.exit(1); + } - try { - await setupFixesAnalysis(options); - result = await requestFixes(options, progress: progress); - serverStopped = server.stop(); - await applyFixes(); - await serverStopped; - } finally { - // If we didn't already try to stop the server, then stop it now. - if (serverStopped == null) { - await server.stop(); - } + Future serverStopped; + try { + await setupFixesAnalysis(options); + result = await requestFixes(options, progress: progress); + serverStopped = server.stop(); + await applyFixes(); + await serverStopped; + } finally { + // If we didn't already try to stop the server, then stop it now. + if (serverStopped == null) { + await server.stop(); } } } @@ -122,14 +122,13 @@ class Driver { unsupportedOption(includeOption); return false; } - if (options.listFixes) { - unsupportedOption(listOption); - return false; - } if (options.requiredFixes) { unsupportedOption(requiredOption); return false; } + if (options.showHelp) { + return false; + } return true; } diff --git a/pkg/dartfix/lib/src/options.dart b/pkg/dartfix/lib/src/options.dart index 3b6d6519c776..d8653e30019f 100644 --- a/pkg/dartfix/lib/src/options.dart +++ b/pkg/dartfix/lib/src/options.dart @@ -22,12 +22,12 @@ class Options { final List excludeFixes; final bool force; - final bool listFixes; + final bool showHelp; final bool overwrite; final bool useColor; final bool verbose; - static Options parse(List args, {Context context, Logger logger}) { + static Options parse(List args, Context context, Logger logger) { final parser = new ArgParser(allowTrailingOptions: true) ..addSeparator('Choosing fixes to be applied:') ..addMultiOption(includeOption, @@ -39,11 +39,6 @@ class Options { help: 'Apply required fixes.', defaultsTo: false, negatable: false) - ..addFlag(listOption, - abbr: 'l', - help: 'Display a list of fixes that can be applied.', - defaultsTo: false, - negatable: false) ..addSeparator('Modifying files:') ..addFlag(overwriteOption, abbr: 'w', @@ -99,13 +94,9 @@ class Options { } options.logger = logger; - if (results[_helpOption] as bool) { - _showUsage(parser, logger); - context.exit(1); - } - - // For '--list', we short circuit the logic to validate the sdk and project. - if (options.listFixes) { + // For '--help', we short circuit the logic to validate the sdk and project. + if (options.showHelp) { + _showUsage(parser, logger, showHelpHint: false); return options; } @@ -157,10 +148,10 @@ class Options { (results[includeOption] as List ?? []).cast().toList(), excludeFixes = (results[excludeOption] as List ?? []).cast().toList(), - listFixes = results[listOption] as bool, overwrite = results[overwriteOption] as bool, requiredFixes = results[requiredOption] as bool, sdkPath = _getSdkPath(), + showHelp = results[_helpOption] as bool || results.arguments.isEmpty, targets = results.rest, useColor = results.wasParsed(_colorOption) ? results[_colorOption] as bool @@ -181,7 +172,7 @@ class Options { } static _showUsage(ArgParser parser, Logger logger, - {bool showListHint = true}) { + {bool showHelpHint = true}) { logger.stderr('Usage: $_binaryName [options...] '); logger.stderr(''); logger.stderr(parser.usage); @@ -190,10 +181,10 @@ class Options { If neither --$includeOption nor --$requiredOption is specified, then all fixes will be applied. Any fixes specified using --$excludeOption will not be applied regardless of whether they are required or specifed using --$includeOption.'''); - if (showListHint) { + if (showHelpHint) { logger.stderr(''' -Use --list to display the fixes that can be specified using either +Use --$_helpOption to display the fixes that can be specified using either --$includeOption or --$excludeOption.'''); } } @@ -209,5 +200,4 @@ const _verboseOption = 'verbose'; // options only supported by server 1.22.2 and greater const excludeOption = 'exclude'; const includeOption = 'include'; -const listOption = 'list'; const requiredOption = 'required'; diff --git a/pkg/dartfix/test/all.dart b/pkg/dartfix/test/all.dart index 010f04719f62..704e37616b9b 100644 --- a/pkg/dartfix/test/all.dart +++ b/pkg/dartfix/test/all.dart @@ -7,7 +7,7 @@ import 'package:test/test.dart'; import 'src/client_version_test.dart' as client_version_test; import 'src/driver_exclude_test.dart' as driver_exclude_test; import 'src/driver_include_test.dart' as driver_include_test; -import 'src/driver_list_test.dart' as driver_list_test; +import 'src/driver_help_test.dart' as driver_list_test; import 'src/driver_required_test.dart' as driver_required_test; import 'src/driver_test.dart' as driver_test; import 'src/options_test.dart' as options_test; diff --git a/pkg/dartfix/test/src/driver_help_test.dart b/pkg/dartfix/test/src/driver_help_test.dart new file mode 100644 index 000000000000..20361b080b0b --- /dev/null +++ b/pkg/dartfix/test/src/driver_help_test.dart @@ -0,0 +1,55 @@ +// Copyright (c) 2019, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'package:dartfix/src/driver.dart'; +import 'package:dartfix/src/options.dart'; +import 'package:test/test.dart'; + +import 'test_context.dart'; + +main() { + test('help explicit', () async { + final driver = new Driver(); + final testContext = new TestContext(); + final testLogger = new TestLogger(); + try { + await driver.start( + ['--help'], // display help and list fixes + testContext: testContext, + testLogger: testLogger, + ); + fail('expected exception'); + } on TestExit catch (e) { + expect(e.code, 1); + } + final errText = testLogger.stderrBuffer.toString(); + final outText = testLogger.stdoutBuffer.toString(); + expect(errText, contains('--$excludeOption')); + expect(errText, isNot(contains('Use --help to display the fixes'))); + expect(outText, contains('use-mixin')); + }); + + test('help implicit', () async { + final driver = new Driver(); + final testContext = new TestContext(); + final testLogger = new TestLogger(); + try { + await driver.start( + [], // no options or arguments should display help and list fixes + testContext: testContext, + testLogger: testLogger, + ); + fail('expected exception'); + } on TestExit catch (e) { + expect(e.code, 1); + } + final errText = testLogger.stderrBuffer.toString(); + final outText = testLogger.stdoutBuffer.toString(); + print(errText); + print(outText); + expect(errText, contains('--$excludeOption')); + expect(errText, isNot(contains('Use --help to display the fixes'))); + expect(outText, contains('use-mixin')); + }); +} diff --git a/pkg/dartfix/test/src/driver_list_test.dart b/pkg/dartfix/test/src/driver_list_test.dart deleted file mode 100644 index 372c98b86bdc..000000000000 --- a/pkg/dartfix/test/src/driver_list_test.dart +++ /dev/null @@ -1,24 +0,0 @@ -// Copyright (c) 2019, the Dart project authors. Please see the AUTHORS file -// for details. All rights reserved. Use of this source code is governed by a -// BSD-style license that can be found in the LICENSE file. - -import 'package:dartfix/src/driver.dart'; -import 'package:test/test.dart'; - -import 'test_context.dart'; - -main() { - test('list fixes', () async { - final driver = new Driver(); - final testContext = new TestContext(); - final testLogger = new TestLogger(); - await driver.start(['--list'], // list fixes - testContext: testContext, - testLogger: testLogger); - final errText = testLogger.stderrBuffer.toString(); - final outText = testLogger.stdoutBuffer.toString(); - print(errText); - print(outText); - expect(outText, contains('use-mixin')); - }); -} diff --git a/pkg/dartfix/test/src/options_test.dart b/pkg/dartfix/test/src/options_test.dart index c463ed628f58..b1b2d44cfae1 100644 --- a/pkg/dartfix/test/src/options_test.dart +++ b/pkg/dartfix/test/src/options_test.dart @@ -26,7 +26,7 @@ main() { bool force = false, List includeFixes = const [], List excludeFixes = const [], - bool listFixes = false, + bool showHelp = false, String normalOut, bool requiredFixes = false, bool overwrite = false, @@ -36,7 +36,7 @@ main() { Options options; int actualExitCode; try { - options = Options.parse(args, context: context, logger: logger); + options = Options.parse(args, context, logger); } on TestExit catch (e) { actualExitCode = e.code; } @@ -53,7 +53,7 @@ main() { expect(options.force, force); expect(options.requiredFixes, requiredFixes); expect(options.overwrite, overwrite); - expect(options.listFixes, listFixes); + expect(options.showHelp, showHelp); expect(options.includeFixes, includeFixes); expect(options.excludeFixes, excludeFixes); expect(options.verbose, verbose); @@ -79,8 +79,12 @@ main() { parse(['--force', 'foo'], force: true, targetSuffixes: ['foo']); }); - test('help', () { - parse(['--help'], errorOut: 'Display this help message', exitCode: 1); + test('help explicit', () { + parse(['--help'], errorOut: 'Display this help message', showHelp: true); + }); + + test('help implicit', () { + parse([], errorOut: 'Display this help message', showHelp: true); }); test('include fix', () { @@ -95,7 +99,7 @@ main() { test('invalid option no logger', () { try { - Options.parse(['--foo'], context: context); + Options.parse(['--foo'], context, null); fail('Expected exception'); } on TestExit catch (e) { expect(e.code, 15, reason: 'exit code'); @@ -107,10 +111,6 @@ main() { errorOut: 'Expected directory, but found', exitCode: 15); }); - test('list fixes', () { - parse(['--list'], normalOut: '', listFixes: true); - }); - test('overwrite', () { parse(['--overwrite', 'foo'], overwrite: true, targetSuffixes: ['foo']); });