[dartfix] refactor the dartfix cli UI a bit
Change-Id: I9935ace979b0302d4f68c8a84ed9886a0ccf0c4a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/101761
Commit-Queue: Devon Carew <devoncarew@google.com>
Reviewed-by: Dan Rubel <danrubel@google.com>
diff --git a/pkg/analysis_server/lib/src/edit/fix/dartfix_info.dart b/pkg/analysis_server/lib/src/edit/fix/dartfix_info.dart
index 728bb580..c6a3ef7 100644
--- a/pkg/analysis_server/lib/src/edit/fix/dartfix_info.dart
+++ b/pkg/analysis_server/lib/src/edit/fix/dartfix_info.dart
@@ -35,23 +35,10 @@
//
const DartFixInfo(
'double-to-int',
- 'Find double literals ending in .0 and remove the .0\n'
+ 'Find double literals ending in .0 and remove the .0 '
'wherever double context can be inferred.',
PreferIntLiteralsFix.task,
),
- //
- // Experimental fixes
- //
- const DartFixInfo(
- 'non-nullable',
- // TODO(danrubel) update description and make default/required
- // when NNBD fix is ready
- 'Experimental: Update sources to be non-nullable by default.\n'
- 'Requires the experimental non-nullable flag to be enabled.\n'
- 'This is not applied unless explicitly included.',
- NonNullableFix.task,
- isDefault: false,
- ),
const DartFixInfo(
'use-spread-collections',
'Convert to using collection spread operators.',
@@ -70,6 +57,18 @@
PreferForElementsToMapFromIterableFix.task,
isDefault: false,
),
+ //
+ // Experimental fixes
+ //
+ const DartFixInfo(
+ 'non-nullable',
+ // TODO(danrubel) update description and make default/required
+ // when NNBD fix is ready
+ 'Experimental: Update sources to be non-nullable by default.\n'
+ 'This requires the experimental non-nullable flag to be enabled.',
+ NonNullableFix.task,
+ isDefault: false,
+ ),
];
/// [DartFixInfo] represents a fix that can be applied by [EditDartFix].
diff --git a/pkg/dartfix/lib/listener/recording_listener.dart b/pkg/dartfix/lib/listener/recording_listener.dart
index 7ac4a12..db0dcf1 100644
--- a/pkg/dartfix/lib/listener/recording_listener.dart
+++ b/pkg/dartfix/lib/listener/recording_listener.dart
@@ -2,8 +2,8 @@
// 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:analysis_server_client/server.dart';
import 'package:analysis_server_client/listener/server_listener.dart';
+import 'package:analysis_server_client/server.dart';
import 'package:dartfix/listener/bad_message_listener.dart';
import 'package:dartfix/listener/timed_listener.dart';
diff --git a/pkg/dartfix/lib/src/driver.dart b/pkg/dartfix/lib/src/driver.dart
index 42bec0e..549f41d 100644
--- a/pkg/dartfix/lib/src/driver.dart
+++ b/pkg/dartfix/lib/src/driver.dart
@@ -31,8 +31,11 @@
Ansi get ansi => logger.ansi;
- Future start(List<String> args,
- {Context testContext, Logger testLogger}) async {
+ Future start(
+ List<String> args, {
+ Context testContext,
+ Logger testLogger,
+ }) async {
final Options options = Options.parse(args);
force = options.force;
@@ -43,23 +46,44 @@
server = new Server(listener: new _Listener(logger));
handler = new _Handler(this);
+ // Start showing progress before we start the analysis server.
+ Progress progress;
+ if (options.listFixes) {
+ progress = logger.progress('${ansi.emphasized('Listing fixes')}');
+ } else {
+ progress = logger.progress('${ansi.emphasized('Calculating fixes')}');
+ }
+
if (!await startServer(options)) {
context.exit(15);
}
- try {
- if (checkSupported(options)) {
- if (options.listFixes) {
- await showListOfFixes();
- } else {
- final progress = await setupAnalysis(options);
- result = await requestFixes(options, progress);
+
+ if (!checkSupported(options)) {
+ await server.stop();
+ context.exit(1);
+ }
+
+ if (options.listFixes) {
+ try {
+ await showListOfFixes(progress: progress);
+ } finally {
+ await server.stop();
+ }
+ } else {
+ 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();
}
}
- } finally {
- await server.stop();
- }
- if (result != null) {
- applyFixes();
}
}
@@ -80,7 +104,7 @@
serverPath: serverPath,
);
server.listenToOutput(notificationProcessor: handler.handleEvent);
- return handler.serverConnected(timeLimit: const Duration(seconds: 15));
+ return handler.serverConnected(timeLimit: const Duration(seconds: 30));
}
/// Check if the specified options is supported by the version of analysis
@@ -116,8 +140,10 @@
Please upgrade to a newer version of the Dart SDK to use this option.''');
}
- Future<Progress> setupAnalysis(Options options) async {
- final progress = logger.progress('${ansi.emphasized('Calculating fixes')}');
+ Future<Progress> setupFixesAnalysis(
+ Options options, {
+ Progress progress,
+ }) async {
logger.trace('');
logger.trace('Setup analysis');
await server.send(SERVER_REQUEST_SET_SUBSCRIPTIONS,
@@ -132,7 +158,9 @@
}
Future<EditDartfixResult> requestFixes(
- Options options, Progress progress) async {
+ Options options, {
+ Progress progress,
+ }) async {
logger.trace('Requesting fixes');
Future isAnalysisComplete = handler.analysisComplete();
@@ -232,14 +260,10 @@
return filePath;
}
- showListOfFixes() async {
- final progress =
- logger.progress('${ansi.emphasized('Getting list of fixes')}');
- logger.trace('');
+ Future<EditGetDartfixInfoResult> showListOfFixes({Progress progress}) async {
Map<String, dynamic> json = await server.send(
EDIT_REQUEST_GET_DARTFIX_INFO, new EditGetDartfixInfoParams().toJson());
-
- progress.finish(showTiming: true);
+ progress?.finish(showTiming: true);
ResponseDecoder decoder = new ResponseDecoder(null);
final result = EditGetDartfixInfoResult.fromJson(decoder, 'result', json);
@@ -259,6 +283,7 @@
}
}
}
+
return result;
}
diff --git a/pkg/dartfix/lib/src/options.dart b/pkg/dartfix/lib/src/options.dart
index a9af28e..3b6d651 100644
--- a/pkg/dartfix/lib/src/options.dart
+++ b/pkg/dartfix/lib/src/options.dart
@@ -4,9 +4,9 @@
import 'dart:io';
-import 'package:dartfix/src/context.dart';
import 'package:args/args.dart';
import 'package:cli_util/cli_logging.dart';
+import 'package:dartfix/src/context.dart';
import 'package:path/path.dart' as path;
/// Command line options for `dartfix`.
@@ -55,10 +55,7 @@
help: 'Overwrite files even if there are errors.',
defaultsTo: false,
negatable: false)
- ..addSeparator('Miscelaneous:')
- ..addFlag(_colorOption,
- help: 'Use ansi colors when printing messages.',
- defaultsTo: Ansi.terminalSupportsAnsi)
+ ..addSeparator('Miscellaneous:')
..addFlag(_helpOption,
abbr: 'h',
help: 'Display this help message.',
@@ -68,7 +65,10 @@
abbr: 'v',
defaultsTo: false,
help: 'Verbose output.',
- negatable: false);
+ negatable: false)
+ ..addFlag(_colorOption,
+ help: 'Use ansi colors when printing messages.',
+ defaultsTo: Ansi.terminalSupportsAnsi);
context ??= new Context();
@@ -78,6 +78,7 @@
} on FormatException catch (e) {
logger ??= new Logger.standard(ansi: new Ansi(Ansi.terminalSupportsAnsi));
logger.stderr(e.message);
+ logger.stderr('\n');
_showUsage(parser, logger);
context.exit(15);
}
@@ -103,8 +104,8 @@
context.exit(1);
}
+ // For '--list', we short circuit the logic to validate the sdk and project.
if (options.listFixes) {
- _showUsage(parser, logger, showListHint: false);
return options;
}
@@ -112,19 +113,17 @@
String sdkPath = options.sdkPath;
if (sdkPath == null) {
logger.stderr('No Dart SDK found.');
- _showUsage(parser, logger);
context.exit(15);
}
+
if (!context.exists(sdkPath)) {
logger.stderr('Invalid Dart SDK path: $sdkPath');
- _showUsage(parser, logger);
context.exit(15);
}
// Check for files and/or directories to analyze.
if (options.targets == null || options.targets.isEmpty) {
logger.stderr('Expected at least one file or directory to analyze.');
- _showUsage(parser, logger);
context.exit(15);
}
@@ -138,7 +137,6 @@
} else {
logger.stderr('Expected directory, but found: $target');
}
- _showUsage(parser, logger);
context.exit(15);
}
}
@@ -189,14 +187,14 @@
logger.stderr(parser.usage);
logger.stderr('''
-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 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) {
logger.stderr('''
-Use --list to display the fixes that can be specified
-using either --$includeOption or --$excludeOption.''');
+Use --list to display the fixes that can be specified using either
+--$includeOption or --$excludeOption.''');
}
}
}
diff --git a/pkg/dartfix/test/src/options_test.dart b/pkg/dartfix/test/src/options_test.dart
index 971517a..c463ed6 100644
--- a/pkg/dartfix/test/src/options_test.dart
+++ b/pkg/dartfix/test/src/options_test.dart
@@ -108,7 +108,7 @@
});
test('list fixes', () {
- parse(['--list'], errorOut: 'Display this help message', listFixes: true);
+ parse(['--list'], normalOut: '', listFixes: true);
});
test('overwrite', () {
