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

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_writer.dart

@@ -154,31 +154,71 @@ final class CodeWriter {
   /// Increases the indentation by [indent] relative to the current amount of
   /// indentation.
   void pushIndent(Indent indent) {
-    var parent = _indentStack.last;
-
-    // Combine the new indentation with the surrounding one.
-    var offset = switch ((parent.type, indent)) {
-      // On the right-hand side of `=`, `:`, or `=>`, don't indent subsequent
-      // infix operands so that they all align:
-      //
-      //     variable =
-      //         operand +
-      //         another;
-      (Indent.assignment, Indent.infix) => 0,
-
-      // We have already indented the control flow header, so collapse the
-      // duplicate indentation.
-      (Indent.controlFlowClause, Indent.expression) => 0,
-      (Indent.controlFlowClause, Indent.infix) => 0,
-
-      // If we get here, the parent context has no effect, so just apply the
-      // indentation directly.
-      (_, _) => indent.spaces,
-    };
+    if (_cache.isVersion37) {
+      _pushIndentV37(indent);
+    } else {
+      var parent = _indentStack.last;
+
+      // Combine the new indentation with the surrounding one.
+      var offset = switch ((parent.type, indent)) {
+        // On the right-hand side of `=`, `:`, or `=>`, don't indent subsequent
+        // infix operands so that they all align:
+        //
+        //     variable =
+        //         operand +
+        //         another;
+        (Indent.assignment, Indent.infix) => 0,
+
+        // We have already indented the control flow header, so collapse the
+        // duplicate indentation.
+        (Indent.controlFlowClause, Indent.expression) => 0,
+        (Indent.controlFlowClause, Indent.infix) => 0,
+
+        // If we get here, the parent context has no effect, so just apply the
+        // indentation directly.
+        (_, _) => indent.spaces,
+      };
+
+      _indentStack.add(_IndentLevel(indent, parent.spaces + offset));
+      if (debug.traceIndent) {
+        debug.log('pushIndent: ${_indentStack.join(' ')}');
+      }
+    }
+  }
 
-    _indentStack.add(_IndentLevel(indent, parent.spaces + offset));
-    if (debug.traceIndent) {
-      debug.log('pushIndent: ${_indentStack.join(' ')}');
+  /// Increases the indentation in a control flow clause in a "collapsible" way.
+  ///
+  /// This is only used in a couple of corners of if-case and for-in headers
+  /// where the indentation is unusual.
+  void pushCollapsibleIndent() {
+    if (_cache.isVersion37) {
+      _pushIndentV37(Indent.expression, canCollapse: true);
+    } else {
+      pushIndent(Indent.controlFlowClause);
+    }
+  }
+
+  /// The 3.7 style of indentation and collapsible indentation tracking.
+  void _pushIndentV37(Indent indent, {bool canCollapse = false}) {
+    var parentIndent = _indentStack.last.spaces;
+    var parentCollapse = _indentStack.last.collapsible;
+
+    if (parentCollapse == indent.spaces) {
+      // We're indenting by the same existing collapsible amount, so collapse
+      // this new indentation with that existing one.
+      _indentStack.add(_IndentLevel.v37(parentIndent, 0));
+    } else if (canCollapse) {
+      // We should never get multiple levels of nested collapsible indentation.
+      assert(parentCollapse == 0);
+
+      // Increase the indentation and note that it can be collapsed with
+      // further indentation.
+      _indentStack.add(
+        _IndentLevel.v37(parentIndent + indent.spaces, indent.spaces),
+      );
+    } else {
+      // Regular indentation, so just increase the indent.
+      _indentStack.add(_IndentLevel.v37(parentIndent + indent.spaces, 0));
     }
   }
 
@@ -230,7 +270,7 @@ final class CodeWriter {
   /// and multi-line strings.
   void whitespace(Whitespace whitespace, {bool flushLeft = false}) {
     if (whitespace case Whitespace.newline || Whitespace.blankLine) {
-      _applyNewlineToShape();
+      _applyNewlineToShape(_pieceFormats.last);
       _pendingIndent = flushLeft ? 0 : _indentStack.last.spaces;
     }
 
@@ -261,11 +301,9 @@ final class CodeWriter {
   void format(Piece piece, {bool separate = false}) {
     if (separate) {
       Profile.count('CodeWriter.format() piece separate');
-
       _formatSeparate(piece);
     } else {
       Profile.count('CodeWriter.format() piece inline');
-
       _formatInline(piece);
     }
   }
@@ -328,23 +366,30 @@ final class CodeWriter {
     if (_pieceFormats.lastOrNull case var parent?) {
       var allowedShapes = parent.piece.allowedChildShapes(
         _solution.pieceState(parent.piece),
-        piece,
+        child.piece,
       );
 
-      if (!allowedShapes.contains(child.shape)) {
-        _solution.invalidate(parent.piece);
-        if (debug.traceSolver) {
-          debug.log(
-            'invalidate ${parent.piece} '
-            '(${_solution.pieceState(parent.piece)}) '
-            'because child $piece (${_solution.pieceState(piece)}) '
-            'has ${child.shape} and allowed are $allowedShapes',
-          );
-        }
+      bool invalid;
+      if (_cache.isVersion37) {
+        // If the child must be inline, then invalidate because we know it
+        // contains some kind of newline.
+        // TODO(rnystrom): It would be better if this logic wasn't different for
+        // 3.7. The only place where the distinction between this code and the
+        // logic in the else clause comes into play is with CaseExpressionPiece.
+        invalid =
+            child.shape != Shape.inline &&
+            allowedShapes.length == 1 &&
+            allowedShapes.contains(Shape.inline);
+      } else {
+        invalid = !allowedShapes.contains(child.shape);
       }
 
+      if (invalid) _solution.invalidate(parent.piece);
+
       // If the child had newlines, propagate that to the parent's shape.
-      if (child.shape != Shape.inline) _applyNewlineToShape(child.shape);
+      if (child.shape != Shape.inline) {
+        _applyNewlineToShape(parent, child.shape);
+      }
     }
   }
 
@@ -413,18 +458,17 @@ final class CodeWriter {
   }
 
   /// Determine how a newline affects the current piece's shape.
-  void _applyNewlineToShape([Shape shape = Shape.other]) {
-    var format = _pieceFormats.last;
-    format.shape = switch (format.mode) {
-      ShapeMode.merge => format.shape.merge(shape),
+  void _applyNewlineToShape(_FormatState state, [Shape shape = Shape.other]) {
+    state.shape = switch (state.mode) {
+      ShapeMode.merge => state.shape.merge(shape),
       ShapeMode.block => Shape.block,
       ShapeMode.beforeHeadline => Shape.other,
       // If there were no newlines inside the headline, now that there is one,
       // we have a headline shape.
-      ShapeMode.afterHeadline when format.shape == Shape.inline =>
+      ShapeMode.afterHeadline when state.shape == Shape.inline =>
         Shape.headline,
       // If there was already a newline in the headline, preserve that shape.
-      ShapeMode.afterHeadline => format.shape,
+      ShapeMode.afterHeadline => state.shape,
       ShapeMode.other => Shape.other,
     };
   }
@@ -555,12 +599,21 @@ enum ShapeMode {
 /// A level of indentation in the indentation stack.
 final class _IndentLevel {
   /// The reason this indentation was added.
+  ///
+  /// Not used for 3.7 style.
   final Indent type;
 
   /// The total number of spaces of indentation.
   final int spaces;
 
-  _IndentLevel(this.type, this.spaces);
+  /// How many spaces of [spaces] can be collapsed with further indentation.
+  ///
+  /// Only used for 3.7 style.
+  final int collapsible;
+
+  _IndentLevel.v37(this.spaces, this.collapsible) : type = Indent.none;
+
+  _IndentLevel(this.type, this.spaces) : collapsible = 0;
 
   @override
   String toString() => '${type.name}:$spaces';