Merge pull request #381 from Workiva/import_sort_v371backpatch

Add import sorting to format tool (backpatch to v3.7.1)

Author: rmconsole5-wk

.github/workflows/dart_ci.yml

@@ -15,7 +15,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        sdk: [ 2.7.2, 2.13.4, stable, dev ]
+        sdk: [ 2.7.2, 2.13.4 ]
     steps:
       - uses: actions/checkout@v2
       - uses: dart-lang/[email protected]
@@ -30,7 +30,7 @@ jobs:
         run: pub get
 
       - name: Validate dependencies
-        run: pub run dependency_validator -i pedantic,over_react_format,meta
+        run: pub run dependency_validator
         if: always() && steps.install.outcome == 'success'
 
       - name: Analyze project source
@@ -60,6 +60,9 @@ jobs:
         run: dart run dart_dev format --check
         if: always() && steps.install.outcome == 'success'
 
-      - name: Publish dry run
-        run: dart pub publish --dry-run
-        if: always() && steps.install.outcome == 'success'
+# Disabled for backpatch releases since pubspec contains dependency_validator
+# key and cannot update to dependency_validator ^3.0.0
+#
+#      - name: Publish dry run
+#        run: dart pub publish --dry-run
+#        if: always() && steps.install.outcome == 'success'

CHANGELOG.md

@@ -1,5 +1,10 @@
 # Changelog
 
+## [3.7.2](https://github.com/Workiva/dart_dev/compare/3.7.1...3.7.2)
+
+- Added an `organizeDirectives` option to the `FormatTool` (default false). When
+true, the formatter will also sort a file's imports and exports.
+
 ## [3.7.1](https://github.com/Workiva/dart_dev/compare/3.7.0...3.7.1)
 
 - Update `TestTool` to allow arguments after a separator (`--`). These arguments

doc/tools/format-tool.md

@@ -92,6 +92,21 @@ final config = {
 };
 ```
 
+### Organizing directives
+
+By default, the format tool will not sort imports/export. They can be automatically
+sorted by setting `organizeDirectives`. 
+
+```dart
+// tool/dart_dev/config.dart
+import 'package:dart_dev/dart_dev.dart';
+
+final config = {
+  'format': FormatTool()
+    ..organizeDirectives = true
+};
+```
+
 ## Command-line options
 
 ```bash

lib/src/dart_dev_runner.dart

@@ -7,8 +7,6 @@ import 'dart_dev_tool.dart';
 import 'events.dart' as events;
 import 'utils/version.dart';
 
-// import 'package:completion/completion.dart' as completion;
-
 class DartDevRunner extends CommandRunner<int> {
   DartDevRunner(Map<String, DevTool> commands)
       : super('dart_dev', 'Dart tool runner.') {

lib/src/executable.dart

@@ -1,6 +1,5 @@
 import 'dart:async';
 import 'dart:io';
-import 'dart:math';
 
 import 'package:analyzer/dart/analysis/utilities.dart';
 import 'package:args/command_runner.dart';

lib/src/tools/format_tool.dart

@@ -13,6 +13,7 @@ import '../dart_dev_tool.dart';
 import '../utils/arg_results_utils.dart';
 import '../utils/assert_no_positional_args_nor_args_after_separator.dart';
 import '../utils/logging.dart';
+import '../utils/organize_directives/organize_directives_in_paths.dart';
 import '../utils/package_is_immediate_dependency.dart';
 import '../utils/process_declaration.dart';
 import '../utils/run_process_and_ensure_exit.dart';
@@ -68,6 +69,11 @@ class FormatTool extends DevTool {
   /// Run `dartfmt -h -v` to see all available args.
   List<String> formatterArgs;
 
+  /// If the formatter should also organize imports and exports.
+  ///
+  /// By default, this is disabled.
+  bool organizeDirectives = false;
+
   // ---------------------------------------------------------------------------
   // DevTool Overrides
   // ---------------------------------------------------------------------------
@@ -97,17 +103,34 @@ class FormatTool extends DevTool {
   String description = 'Format dart files in this package.';
 
   @override
-  FutureOr<int> run([DevToolExecutionContext context]) {
+  FutureOr<int> run([DevToolExecutionContext context]) async {
     context ??= DevToolExecutionContext();
-    final execution = buildExecution(
+    final formatExecution = buildExecution(
       context,
       configuredFormatterArgs: formatterArgs,
       defaultMode: defaultMode,
       exclude: exclude,
       formatter: formatter,
+      organizeDirectives: organizeDirectives,
+    );
+    if (formatExecution.exitCode != null) {
+      return formatExecution.exitCode;
+    }
+    var exitCode = await runProcessAndEnsureExit(
+      formatExecution.formatProcess,
+      log: _log,
     );
-    return execution.exitCode ??
-        runProcessAndEnsureExit(execution.process, log: _log);
+    if (exitCode != 0) {
+      return exitCode;
+    }
+    if (formatExecution.directiveOrganization != null) {
+      exitCode = organizeDirectivesInPaths(
+        formatExecution.directiveOrganization.inputs,
+        check: formatExecution.directiveOrganization.check,
+        verbose: context.verbose,
+      );
+    }
+    return exitCode;
   }
 
   /// Builds and returns the object that contains:
@@ -283,14 +306,17 @@ class FormatterInputs {
 /// A declarative representation of an execution of the [FormatTool].
 ///
 /// This class allows the [FormatTool] to break its execution up into two steps:
-/// 1. Validation of confg/inputs and creation of this class.
+/// 1. Validation of config/inputs and creation of this class.
 /// 2. Execution of expensive or hard-to-test logic based on step 1.
 ///
 /// As a result, nearly all of the logic in [FormatTool] can be tested via the
 /// output of step 1 (an instance of this class) with very simple unit tests.
 class FormatExecution {
-  FormatExecution.exitEarly(this.exitCode) : process = null;
-  FormatExecution.process(this.process) : exitCode = null;
+  FormatExecution.exitEarly(this.exitCode)
+      : formatProcess = null,
+        directiveOrganization = null;
+  FormatExecution.process(this.formatProcess, [this.directiveOrganization])
+      : exitCode = null;
 
   /// If non-null, the execution is already complete and the [FormatTool] should
   /// exit with this code.
@@ -300,8 +326,20 @@ class FormatExecution {
 
   /// A declarative representation of the formatter process that should be run.
   ///
-  /// This process' result should become the final result of the [FormatTool].
-  final ProcessDeclaration process;
+  /// If this process results in a non-zero exit code, [FormatTool] should return it.
+  final ProcessDeclaration formatProcess;
+
+  /// A declarative representation of the directive organization work to be done
+  /// (if enabled) after running the formatter.
+  final DirectiveOrganization directiveOrganization;
+}
+
+/// A declarative representation of the directive organization work.
+class DirectiveOrganization {
+  DirectiveOrganization(this.inputs, {this.check});
+
+  final bool check;
+  final Set<String> inputs;
 }
 
 /// Modes supported by the dart formatter.
@@ -385,6 +423,8 @@ Iterable<String> buildArgs(
 ///
 /// [include] will be populated from [FormatTool.include].
 ///
+/// [organizeDirectives] will be populated from [FormatTool.organizeDirectives].
+///
 /// If non-null, [path] will override the current working directory for any
 /// operations that require it. This is intended for use by tests.
 ///
@@ -397,6 +437,7 @@ FormatExecution buildExecution(
   FormatMode defaultMode,
   List<Glob> exclude,
   Formatter formatter,
+  bool organizeDirectives = false,
   String path,
 }) {
   FormatMode mode;
@@ -463,23 +504,37 @@ FormatExecution buildExecution(
         '${inputs.hiddenDirectories.join('\n  ')}');
   }
 
-  final dartfmt = buildProcess(formatter);
+  final dartfmt = buildFormatProcess(formatter);
   final args = buildArgs(dartfmt.args, mode,
       argResults: context.argResults,
       configuredFormatterArgs: configuredFormatterArgs);
   logCommand(dartfmt.executable, inputs.includedFiles, args,
       verbose: context.verbose);
-  return FormatExecution.process(ProcessDeclaration(
-      dartfmt.executable, [...args, ...inputs.includedFiles],
-      mode: ProcessStartMode.inheritStdio));
+
+  final formatProcess = ProcessDeclaration(
+    dartfmt.executable,
+    [...args, ...inputs.includedFiles],
+    mode: ProcessStartMode.inheritStdio,
+  );
+  DirectiveOrganization directiveOrganization;
+  if (organizeDirectives) {
+    directiveOrganization = DirectiveOrganization(
+      inputs.includedFiles,
+      check: mode == FormatMode.check,
+    );
+  }
+  return FormatExecution.process(
+    formatProcess,
+    directiveOrganization,
+  );
 }
 
 /// Returns a representation of the process that will be run by [FormatTool]
 /// based on the given [formatter].
 ///
 /// - [Formatter.dartfmt] -> `dartfmt`
 /// - [Formatter.dartStyle] -> `pub run dart_style:format`
-ProcessDeclaration buildProcess([Formatter formatter]) {
+ProcessDeclaration buildFormatProcess([Formatter formatter]) {
   switch (formatter) {
     case Formatter.dartStyle:
       return ProcessDeclaration('pub', ['run', 'dart_style:format']);

lib/src/tools/function_tool.dart

@@ -1,7 +1,6 @@
 import 'dart:async';
 
 import 'package:args/args.dart';
-import 'package:dart_dev/src/utils/assert_no_args_after_separator.dart';
 import 'package:io/io.dart';
 import 'package:logging/logging.dart';
 

lib/src/tools/test_tool.dart

@@ -9,7 +9,6 @@ import 'package:logging/logging.dart';
 
 import '../dart_dev_tool.dart';
 import '../utils/arg_results_utils.dart';
-import '../utils/assert_no_args_after_separator.dart';
 import '../utils/logging.dart';
 import '../utils/package_is_immediate_dependency.dart';
 import '../utils/process_declaration.dart';

lib/src/tools/webdev_serve_tool.dart

@@ -10,8 +10,8 @@ import 'package:pub_semver/pub_semver.dart';
 import '../dart_dev_tool.dart';
 import '../utils/arg_results_utils.dart';
 import '../utils/assert_no_positional_args_nor_args_after_separator.dart';
-import '../utils/logging.dart';
 import '../utils/global_package_is_active_and_compatible.dart';
+import '../utils/logging.dart';
 import '../utils/process_declaration.dart';
 import '../utils/run_process_and_ensure_exit.dart';
 
@@ -91,7 +91,7 @@ class WebdevServeTool extends DevTool {
 ///
 /// This class allows the [WebdevServeTool] to break its execution up into two
 /// steps:
-/// 1. Validation of confg/inputs and creation of this class.
+/// 1. Validation of config/inputs and creation of this class.
 /// 2. Execution of expensive or hard-to-test logic based on step 1.
 ///
 /// As a result, nearly all of the logic in [WebdevServeTool] can be tested via

lib/src/utils/organize_directives/namespace.dart

@@ -0,0 +1,76 @@
+import 'package:analyzer/dart/ast/ast.dart';
+import 'package:analyzer/dart/ast/token.dart';
+
+/// A representation of an namespace directive.
+///
+/// Capable of tracking comments that should be associated with an namespace
+/// during organization (which cannot be represented by the AST)
+class Namespace {
+  /// The AST node that represents an namespace in a file.
+  final NamespaceDirective directive;
+
+  /// Comments that appear before the namespace that should stay with the
+  /// namespace when organized.
+  List<Token> beforeComments = [];
+
+  /// Comments that appear after the namespace that should stay with the
+  /// namespace when organized.
+  List<Token> afterComments = [];
+
+  /// The file being imported/exported.
+  String get target {
+    return directive.uri.stringValue;
+  }
+
+  /// If the namespace is a dart namespace. Memoized for performance.
+  bool _isDart;
+  bool get isDart {
+    return _isDart ??= target.startsWith('dart:');
+  }
+
+  /// If the namespace is an external package namespace. Memoized for performance.
+  bool _isExternalPkg;
+  bool get isExternalPkg {
+    return _isExternalPkg ??= target.startsWith('package:');
+  }
+
+  /// If the namespace is a relative namespace. Memoized for performance.
+  bool _isRelative;
+  bool get isRelative {
+    return _isRelative ??= !isExternalPkg && !isDart;
+  }
+
+  Namespace(this.directive);
+
+  /// The character offset of the start of the namespace statement in source text.
+  /// Excludes comments associated with this namespace.
+  int get statementStart => directive.beginToken.charOffset;
+
+  /// The character offset of the end of the namespace statement in source text.
+  /// Excludes comments associated with this namespace.
+  int get statementEnd => directive.endToken.end;
+
+  /// The character offset of the end of this namespace in source text.
+  /// Includes comments associated with this namespace.
+  int end() {
+    var end = directive.endToken.end;
+    for (final afterComment in afterComments) {
+      if (afterComment.end > end) {
+        end = afterComment.end;
+      }
+    }
+    return end;
+  }
+
+  /// The character offset of the start of this namespace in source text.
+  /// Includes comments associated with this namespace.
+  int start() {
+    var charOffset = directive.beginToken.charOffset;
+    for (final beforeComment in beforeComments) {
+      if (beforeComment.charOffset < charOffset) {
+        charOffset = beforeComment.charOffset;
+      }
+    }
+    return charOffset;
+  }
+}