Language version all of the formatting changes since Dart 3.7. (#1707)

Language version all of the formatting changes since Dart 3.7.

The new formatter launch was pretty rocky, but one thing I think helped mitigate that was that the formatting style was gated on language version. Users could install the new SDK and their code wouldn't be spontaneously reformatted under them until they deliberately updated their SDK constraint to opt in to the new language version.

This PR applies that same logic to the changes we've made since Dart 3.7.

Any source file at language version 3.7 is formatted (almost, see below) identically to how it would be formatted with the SDK as it shipped in 3.7. Source files at language version 3.8 or higher get all of the new style changes I've landed since then.

This increases the complexity of the formatter codebase a decent amount, but it's mostly just a lot of "if 3.7 do X else do Y". Tedious but not intractable.

I expect to continue to language version changes like this going forward. So after Dart 3.8 ships, whatever formatter style is in that version will be locked down and style changes after that will have to support 3.7 and 3.8 fallback behavior. I expect there to be much less style churn going forward, so it's probably manageable.

In cases where there are *bugs* (i.e. the formatter produces invalid syntax or rearranges code), those won't be language versioned. But most other style changes will be.

It's important to make sure we don't regress and accidentally affect the formatting of older language versioned files when making changes to the formatter. To avoid that, this also expands testing to be multi-versioned. Every test is now run on multiple versions and for cases where the style differs between versions, there are different expectations for each. Fortunately, the tests run very fast, so it doesn't slow down things more than a couple of seconds.

In addition to running through the formatter's own test suite and regression tests, I also tested this on a giant corpus of pub packages. I formatted them all using the shipped Dart 3.7 formatter, then using this PR with `--language-version=3.7`. Of the 89,468 files, 7 had differences. They all relate to a single weird corner case around giant multiline strings containing multiline string interpolation expressions where the formatting is slightly different. #1706 helps that somewhat -- there were about dozen diffs before -- but doesn't totally eliminate it. I think it's close enough to be tolerable.

Author: munificent

CHANGELOG.md

@@ -1,6 +1,32 @@
 ## 3.1.0-wip
 
-* Format null-aware elements.
+### Features
+
+* 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 work in the old short style formatter
+  applied to code before language version 3.7.
+
+[reversibility]: https://github.com/dart-lang/dart_style/wiki/Reversibility-principle
+
+### Bug fixes
+
+* Don't add a trailing comma in lists that don't allow it, even when there is
+  a trailing comment (#1639).
+
+### Style changes
+
+The following style changes are language versioned and only affect code whose
+language version is 3.8 or later. Dart code at 3.7 or earlier is formatted the
+same as it was before.
 
 * Allow more code on the same line as a named argument or `=>` (#1536, #1545,
   #1668, #1679).
@@ -175,26 +201,6 @@
   );
   ```
 
-* 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

@@ -5,6 +5,7 @@
 import 'package:dart_style/dart_style.dart';
 import 'package:dart_style/src/debug.dart' as debug;
 import 'package:dart_style/src/testing/test_file.dart';
+import 'package:pub_semver/pub_semver.dart';
 
 void main(List<String> args) {
   // Enable debugging so you can see some of the formatter's internal state.
@@ -32,29 +33,29 @@ void main(List<String> args) {
 
 void _formatStmt(
   String source, {
-  bool tall = true,
+  Version? version,
   int pageWidth = 40,
   TrailingCommas trailingCommas = TrailingCommas.automate,
 }) {
   _runFormatter(
     source,
     pageWidth,
-    tall: tall,
+    version: version ?? DartFormatter.latestLanguageVersion,
     isCompilationUnit: false,
     trailingCommas: trailingCommas,
   );
 }
 
 void _formatUnit(
   String source, {
-  bool tall = true,
+  Version? version,
   int pageWidth = 40,
   TrailingCommas trailingCommas = TrailingCommas.automate,
 }) {
   _runFormatter(
     source,
     pageWidth,
-    tall: tall,
+    version: version ?? DartFormatter.latestLanguageVersion,
     isCompilationUnit: true,
     trailingCommas: trailingCommas,
   );
@@ -63,16 +64,13 @@ void _formatUnit(
 void _runFormatter(
   String source,
   int pageWidth, {
-  required bool tall,
+  required Version version,
   required bool isCompilationUnit,
   TrailingCommas trailingCommas = TrailingCommas.automate,
 }) {
   try {
     var formatter = DartFormatter(
-      languageVersion:
-          tall
-              ? DartFormatter.latestLanguageVersion
-              : DartFormatter.latestShortStyleLanguageVersion,
+      languageVersion: version,
       pageWidth: pageWidth,
       trailingCommas: trailingCommas,
     );
@@ -110,19 +108,20 @@ Future<void> _runTest(
   var formatTest = testFile.tests.firstWhere((test) => test.line == line);
   var formatter = testFile.formatterForTest(formatTest);
 
-  var actual = formatter.formatSource(formatTest.input);
+  var actual = formatter.formatSource(formatTest.input.code);
 
   // The test files always put a newline at the end of the expectation.
   // Statements from the formatter (correctly) don't have that, so add
   // one to line up with the expected result.
   var actualText = actual.textWithSelectionMarkers;
   if (!testFile.isCompilationUnit) actualText += '\n';
 
-  var expectedText = formatTest.output.textWithSelectionMarkers;
+  // TODO(rnystrom): Handle multiple outputs.
+  var expectedText = formatTest.outputs.first.code.textWithSelectionMarkers;
 
-  print('$path ${formatTest.description}');
+  print('$path ${formatTest.input.description}');
   _drawRuler('before', pageWidth);
-  print(formatTest.input.textWithSelectionMarkers);
+  print(formatTest.input.code.textWithSelectionMarkers);
   if (actualText == expectedText) {
     _drawRuler('result', pageWidth);
     print(actualText);

lib/src/ast_extensions.dart

@@ -420,22 +420,76 @@ extension AdjacentStringsExtensions on AdjacentStrings {
   /// To balance these, we omit the indentation in argument lists only if there
   /// are no other string arguments.
   bool get indentStrings {
-    bool hasOtherStringArgument(List<Expression> arguments) => arguments.any(
-      (argument) => argument != this && argument is StringLiteral,
-    );
+    return switch (parent) {
+      ArgumentList(:var arguments) => _hasOtherStringArgument(arguments),
+
+      // Treat asserts like argument lists.
+      Assertion(:var condition, :var message) => _hasOtherStringArgument([
+        condition,
+        if (message != null) message,
+      ]),
+
+      _ => true,
+    };
+  }
 
+  /// Whether subsequent strings should be indented relative to the first
+  /// string (in 3.7 style).
+  ///
+  /// We generally want to indent adjacent strings because it can be confusing
+  /// otherwise when they appear in a list of expressions, like:
+  ///
+  ///     [
+  ///       "one",
+  ///       "two"
+  ///       "three",
+  ///       "four"
+  ///     ]
+  ///
+  /// Especially when these strings are longer, it can be hard to tell that
+  /// "three" is a continuation of the previous element.
+  ///
+  /// However, the indentation is distracting in places that don't suffer from
+  /// this ambiguity:
+  ///
+  ///     var description =
+  ///         "A very long description..."
+  ///             "this extra indentation is unnecessary.");
+  ///
+  /// To balance these, we omit the indentation when an adjacent string
+  /// expression is in a context where it's unlikely to be confusing.
+  bool get indentStringsV37 {
     return switch (parent) {
-      ArgumentList(:var arguments) => hasOtherStringArgument(arguments),
+      ArgumentList(:var arguments) => _hasOtherStringArgument(arguments),
 
       // Treat asserts like argument lists.
-      Assertion(:var condition, :var message) => hasOtherStringArgument([
+      Assertion(:var condition, :var message) => _hasOtherStringArgument([
         condition,
         if (message != null) message,
       ]),
 
+      // Don't add extra indentation in a variable initializer or assignment:
+      //
+      //     var variable =
+      //         "no extra"
+      //         "indent";
+      VariableDeclaration() => false,
+      AssignmentExpression(:var rightHandSide) when rightHandSide == this =>
+        false,
+
+      // Don't indent when following `:`.
+      MapLiteralEntry(:var value) when value == this => false,
+      NamedExpression() => false,
+
+      // Don't indent when the body of a `=>` function.
+      ExpressionFunctionBody() => false,
       _ => true,
     };
   }
+
+  bool _hasOtherStringArgument(List<Expression> arguments) => arguments.any(
+    (argument) => argument != this && argument is StringLiteral,
+  );
 }
 
 extension PatternExtensions on DartPattern {
@@ -445,7 +499,7 @@ extension PatternExtensions on DartPattern {
   /// Whether this expression is a non-empty delimited container for inner
   /// expressions that allows "block-like" formatting in some contexts.
   ///
-  /// See [ExpressionExtensions.canBlockSplit].
+  /// See [ExpressionExtensions38.canBlockSplit].
   bool get canBlockSplit => switch (this) {
     ConstantPattern(:var expression) => expression.canBlockSplit,
     ListPattern(:var elements, :var rightBracket) => elements.canSplit(

lib/src/back_end/code.dart

@@ -135,7 +135,7 @@ final class GroupCode extends Code {
 
 /// A [Code] object for a newline followed by any leading indentation.
 final class _NewlineCode extends Code {
-  /// True if a blank line (two newlines) should be written.
+  /// Whether a blank line (two newlines) should be written.
   final bool _blank;
 
   /// The number of spaces of indentation after this newline.