Add an optional configuration to preserve trailing commas.

CHANGELOG.md

@@ -1,4 +1,4 @@
-## 3.0.2-wip
+## 3.1.0-wip
 
 * Format null-aware elements.
 
@@ -63,10 +63,26 @@
   );
   ```
 
+* Allow preserving trailing commas and forcing the surrounding construct to
+  split even when it would otherwise fit on one line. This is off by default
+  (because it breaks [reversibility][] among other reasons) but can be enabled
+  by adding this to a surrounding `analysis_options.yaml` file:
+
+  ```yaml
+  formatter:
+    trailing_commas: preserve
+  ```
+
+  This is similar to how trailing commas worked in the old short style
+  formatter.
+
 * Don't add a trailing comma in lists that don't allow it, even when there is
   a trailing comment (#1639).
+
 * Add tests for digit separators.
 
+[reversibility]: https://github.com/dart-lang/dart_style/wiki/Reversibility-principle
+
 ## 3.0.1
 
 * Handle trailing commas in for-loop updaters (#1354).

example/format.dart

@@ -30,19 +30,42 @@ void main(List<String> args) {
   _runTest('other/selection.stmt', 2);
 }
 
-void _formatStmt(String source, {bool tall = true, int pageWidth = 40}) {
-  _runFormatter(source, pageWidth, tall: tall, isCompilationUnit: false);
+void _formatStmt(
+  String source, {
+  bool tall = true,
+  int pageWidth = 40,
+  TrailingCommas trailingCommas = TrailingCommas.automate,
+}) {
+  _runFormatter(
+    source,
+    pageWidth,
+    tall: tall,
+    isCompilationUnit: false,
+    trailingCommas: trailingCommas,
+  );
 }
 
-void _formatUnit(String source, {bool tall = true, int pageWidth = 40}) {
-  _runFormatter(source, pageWidth, tall: tall, isCompilationUnit: true);
+void _formatUnit(
+  String source, {
+  bool tall = true,
+  int pageWidth = 40,
+  TrailingCommas trailingCommas = TrailingCommas.automate,
+}) {
+  _runFormatter(
+    source,
+    pageWidth,
+    tall: tall,
+    isCompilationUnit: true,
+    trailingCommas: trailingCommas,
+  );
 }
 
 void _runFormatter(
   String source,
   int pageWidth, {
   required bool tall,
   required bool isCompilationUnit,
+  TrailingCommas trailingCommas = TrailingCommas.automate,
 }) {
   try {
     var formatter = DartFormatter(
@@ -51,6 +74,7 @@ void _runFormatter(
               ? DartFormatter.latestLanguageVersion
               : DartFormatter.latestShortStyleLanguageVersion,
       pageWidth: pageWidth,
+      trailingCommas: trailingCommas,
     );
 
     String result;
@@ -84,13 +108,7 @@ Future<void> _runTest(
 }) async {
   var testFile = await TestFile.read('${tall ? 'tall' : 'short'}/$path');
   var formatTest = testFile.tests.firstWhere((test) => test.line == line);
-
-  var formatter = DartFormatter(
-    languageVersion: formatTest.languageVersion,
-    pageWidth: testFile.pageWidth,
-    indent: formatTest.leadingIndent,
-    experimentFlags: formatTest.experimentFlags,
-  );
+  var formatter = testFile.formatterForTest(formatTest);
 
   var actual = formatter.formatSource(formatTest.input);
 

lib/src/analysis_options/analysis_options_file.dart

@@ -17,14 +17,14 @@ typedef AnalysisOptions = Map<Object?, Object?>;
 /// passed to [FileSystem.join()].
 typedef ResolvePackageUri = Future<String?> Function(Uri packageUri);
 
-/// Reads an `analysis_options.yaml` file in [directory] or in the nearest
+/// Reads an "analysis_options.yaml" file in [directory] or in the nearest
 /// surrounding folder that contains that file using [fileSystem].
 ///
 /// Stops walking parent directories as soon as it finds one that contains an
-/// `analysis_options.yaml` file. If it reaches the root directory without
+/// "analysis_options.yaml" file. If it reaches the root directory without
 /// finding one, returns an empty [YamlMap].
 ///
-/// If an `analysis_options.yaml` file is found, reads it and parses it to a
+/// If an "analysis_options.yaml" file is found, reads it and parses it to a
 /// [YamlMap]. If the map contains an `include` key whose value is a list, then
 /// reads any of the other referenced YAML files and merges them into this one.
 /// Returns the resulting map with the `include` key removed.

lib/src/ast_extensions.dart

@@ -454,6 +454,9 @@ extension PatternExtensions on DartPattern {
 }
 
 extension TokenExtensions on Token {
+  /// Whether the token before this one is a comma.
+  bool get hasCommaBefore => previous?.type == TokenType.COMMA;
+
   /// Whether this token has a preceding comment that is a line comment.
   bool get hasLineCommentBefore {
     for (

lib/src/cli/format_command.dart

@@ -106,6 +106,22 @@ final class FormatCommand extends Command<int> {
       hide: true,
     );
 
+    argParser.addOption(
+      'trailing-commas',
+      help: 'How trailing commas in input affect formatting.',
+      defaultsTo: 'automate',
+      allowedHelp: {
+        'automate':
+            'The formatter adds and removes trailing commas based on\n'
+            'its decision to split the surrounding construct.',
+        'preserve':
+            'A trailing comma forces the surrounding construct to split.\n'
+            'The formatter will add a trailing comma when it splits a\n'
+            'construct but will not remove one.',
+      },
+      hide: !verbose,
+    );
+
     argParser.addOption(
       'indent',
       abbr: 'i',
@@ -255,6 +271,19 @@ final class FormatCommand extends Command<int> {
       }
     }
 
+    TrailingCommas? trailingCommas;
+    if (argResults.wasParsed('trailing-commas')) {
+      // We check the values explicitly here instead of using `allowedValues`
+      // from [ArgParser] because this provides a better error message.
+      trailingCommas = switch (argResults['trailing-commas']) {
+        'automate' => TrailingCommas.automate,
+        'preserve' => TrailingCommas.preserve,
+        var mode => usageException(
+          '--trailing-commas must be "automate" or "preserve", was "$mode".',
+        ),
+      };
+    }
+
     var indent =
         int.tryParse(argResults['indent'] as String) ??
         usageException(
@@ -300,6 +329,7 @@ final class FormatCommand extends Command<int> {
       languageVersion: languageVersion,
       indent: indent,
       pageWidth: pageWidth,
+      trailingCommas: trailingCommas,
       followLinks: followLinks,
       show: show,
       output: output,

lib/src/cli/formatter_options.dart

@@ -6,15 +6,17 @@ import 'dart:io';
 
 import 'package:pub_semver/pub_semver.dart';
 
+import '../dart_formatter.dart';
 import '../source_code.dart';
 import 'output.dart';
 import 'show.dart';
 import 'summary.dart';
 
 // Note: The following line of code is modified by tool/grind.dart.
-const dartStyleVersion = '3.0.2-wip';
+const dartStyleVersion = '3.1.0-wip';
 
-/// Global options that affect how the formatter produces and uses its outputs.
+/// Global options parsed from the command line that affect how the formatter
+/// produces and uses its outputs.
 final class FormatterOptions {
   /// The language version formatted code should be parsed at or `null` if not
   /// specified.
@@ -30,6 +32,9 @@ final class FormatterOptions {
   /// [DartFormatter.defaultPageWidth].
   final int? pageWidth;
 
+  /// How trailing commas in the input source code affect formatting.
+  final TrailingCommas? trailingCommas;
+
   /// Whether symlinks should be traversed when formatting a directory.
   final bool followLinks;
 
@@ -53,6 +58,7 @@ final class FormatterOptions {
     this.languageVersion,
     this.indent = 0,
     this.pageWidth,
+    this.trailingCommas,
     this.followLinks = false,
     this.show = Show.changed,
     this.output = Output.write,

lib/src/config_cache.dart

@@ -9,13 +9,14 @@ import 'package:pub_semver/pub_semver.dart';
 
 import 'analysis_options/analysis_options_file.dart';
 import 'analysis_options/io_file_system.dart';
+import 'dart_formatter.dart';
 import 'profile.dart';
 
 /// Caches the nearest surrounding package config file for files in directories.
 ///
 /// The formatter reads `.dart_tool/package_config.json` files in order to
 /// determine the default language version of files in that package and to
-/// resolve "package:" URIs in `analysis_options.yaml` files.
+/// resolve "package:" URIs in "analysis_options.yaml" files.
 ///
 /// Walking the file system to find the package config and then reading it off
 /// disk is very slow. We know that every formatted file in the same directory
@@ -40,13 +41,9 @@ final class ConfigCache {
   /// discovered that there is no surrounding package.
   final Map<String, Version?> _directoryVersions = {};
 
-  /// The previously cached page width for all files immediately within a given
-  /// directory.
-  ///
-  /// The width may be `null` if we formatted a file in that directory and
-  /// discovered that there is no surrounding analysis options that sets a
-  /// page width.
-  final Map<String, int?> _directoryPageWidths = {};
+  /// The previously cached configured options for all files immediately within
+  /// a given directory.
+  final Map<String, _FormatterOptions> _directoryOptions = {};
 
   final IOFileSystem _fileSystem = IOFileSystem();
 
@@ -89,35 +86,77 @@ final class ConfigCache {
   ///     formatter:
   ///       page_width: 123
   Future<int?> findPageWidth(File file) async {
-    // Use the cached version (which may be `null`) if present.
+    return (await _findFormatterOptions(file)).pageWidth;
+  }
+
+  /// Looks for an "analysis_options.yaml" file surrounding [file] and, if
+  /// found and valid, returns the trailing comma handling specified by that
+  /// config file.
+  ///
+  /// Otherwise returns `null`.
+  ///
+  /// The schema looks like:
+  ///
+  ///     formatter:
+  ///       trailing_commas: preserve # Or "automate".
+  Future<TrailingCommas?> findTrailingCommas(File file) async {
+    return (await _findFormatterOptions(file)).trailingCommas;
+  }
+
+  /// Looks for an "analysis_options.yaml" file surrounding [file] and, if
+  /// found and valid, returns the configured options.
+  ///
+  /// If no options file could be found or it doesn't contain a "formatter" key
+  /// whose value is a map, returns a default set of options where all settings
+  /// are `null`.
+  Future<_FormatterOptions> _findFormatterOptions(File file) async {
+    // Use the cached version if present.
     var directory = file.parent.path;
-    if (_directoryPageWidths.containsKey(directory)) {
-      return _directoryPageWidths[directory];
-    }
+    if (_directoryOptions[directory] case var options?) return options;
+
+    int? pageWidth;
+    TrailingCommas? trailingCommas;
 
     try {
-      // Look for a surrounding analysis_options.yaml file.
-      var options = await findAnalysisOptions(
+      // Look for a surrounding "analysis_options.yaml" file.
+      var optionsFile = await findAnalysisOptions(
         _fileSystem,
         await _fileSystem.makePath(file.path),
         resolvePackageUri: (uri) => _resolvePackageUri(file, uri),
       );
 
-      if (options['formatter'] case Map<Object?, Object?> formatter) {
-        if (formatter['page_width'] case int pageWidth) {
-          return _directoryPageWidths[directory] = pageWidth;
+      if (optionsFile['formatter'] case Map<Object?, Object?> formatter) {
+        if (formatter case {'page_width': int width}) {
+          pageWidth = width;
+        }
+
+        if (formatter case {'trailing_commas': var commas}) {
+          switch (commas) {
+            case 'automate':
+              trailingCommas = TrailingCommas.automate;
+            case 'preserve':
+              trailingCommas = TrailingCommas.preserve;
+            default:
+              stderr.writeln(
+                'Warning: "trailing_commas" option should be "automate" or '
+                '"preserve", but was "$commas".',
+              );
+          }
         }
       }
-    } on PackageResolutionException {
-      // Silently ignore any errors coming from the processing the analyis
-      // options. If there are any issues, we just use the default page width
-      // and keep going.
+    } on PackageResolutionException catch (exception) {
+      // Report the error, but use the default settings and keep going.
+      stderr.writeln(
+        'Warning: Package resolution error when reading '
+        '"analysis_options.yaml" file:\n$exception',
+      );
     }
 
-    // If we get here, the options file either doesn't specify the page width,
-    // or specifies it in some invalid way. When that happens, silently ignore
-    // the config file and use the default width.
-    return _directoryPageWidths[directory] = null;
+    // Cache whichever options we found (or `null` if we didn't find them).
+    return _directoryOptions[directory] = _FormatterOptions(
+      pageWidth,
+      trailingCommas,
+    );
   }
 
   /// Look for and cache the nearest package surrounding [file].
@@ -176,3 +215,17 @@ final class ConfigCache {
     return config.resolve(packageUri)?.toFilePath();
   }
 }
+
+/// The formatter options that can be configured in the "analysis_options.yaml"
+/// file.
+final class _FormatterOptions {
+  /// The configured page width, or `null` if there is no options file or the
+  /// options file doesn't specify it.
+  final int? pageWidth;
+
+  /// The configured comma handling, or `null` if there is no options file or
+  /// the options file doesn't specify it.
+  final TrailingCommas? trailingCommas;
+
+  _FormatterOptions(this.pageWidth, this.trailingCommas);
+}