From 193451461cae569bbc3dec3607b9b612b254db33 Mon Sep 17 00:00:00 2001
From: Robert Nystrom <rnystrom@google.com>
Date: Wed, 17 Jan 2024 14:54:31 -0800
Subject: [PATCH 1/3] Format multi-line strings and string interpolation.

In the old style, the formatter has some special code to discard line
splits that occur inside string interpolation expressions. That's
largely for historical reasons because the formatter initially didn't
support formatting of string interpolation expressions *at all* and I
didn't want too much churn when adding support for formatting them.

In the new style here, we don't do that: The contents of a string
interpolation expression are split like any other expression. In
practice, it doesn't matter much since users generally reorganize their
code to avoid long strings and splits in string interpolation. This way
leads to less special case code in the formatter.

This change is somewhat large because I also reorganized how newlines
inside lexemes are handled in general. Previously, TextPiece stored a
list of "lines" to handle things like line comments preceding or
following a token. But it was also possible for a single "line" string
in that list to internally contain newline characters because of
multi-line strings or block comments.

But those internal newlines also need to force surrounding code to
split, so there was this "_containsNewline" bit that had to be plumbed
through and tracked. Even so, there were latent bugs where the column
calculation in CodeWriter would be incorrect if a line contained
internal newlines because it just used to the length of the entire
"line" string.

With this change, the "_lines" list in TextPiece really is a list of
lines. We eagerly split any incoming lexeme into multiple lines before
writing it to the TextPiece. I think the resulting code is simpler, it
fixes the column calculation in CodeWriter, and it means the formatter
will correctly normalize line endings even when they occur inside block
comments or multiline strings.

This was a good time to test the line ending code, so I copied those
existing tests over from short_format_test.dart. I went ahead and
copied all of the unit tests from that file, even the ones not related
to line endings, since they're all working and passing now.

This PR does *not* handle adjacent strings. Those have a decent amount
of special handling not related to what's going on here, so I'll do
those separately.
---
 lib/src/ast_extensions.dart                  |  18 +--
 lib/src/back_end/code_writer.dart            |  47 ++++----
 lib/src/front_end/ast_node_visitor.dart      |  18 ++-
 lib/src/front_end/piece_writer.dart          |  49 ++++++--
 lib/src/piece/piece.dart                     |  67 +++++++----
 test/expression/interpolation.stmt           |  82 +++++++++++++
 test/expression/string.stmt                  |  88 ++++++++++++++
 test/expression/string_comment.stmt          |  72 +++++++++++
 test/invocation/block_argument_multiple.stmt |  11 +-
 test/invocation/block_argument_string.stmt   |  97 +++++++++++++++
 test/selection/selection.stmt                |   4 +
 test/tall_format_test.dart                   | 118 ++++++++++++++++++-
 12 files changed, 598 insertions(+), 73 deletions(-)
 create mode 100644 test/expression/interpolation.stmt
 create mode 100644 test/expression/string.stmt
 create mode 100644 test/expression/string_comment.stmt
 create mode 100644 test/invocation/block_argument_string.stmt

diff --git a/lib/src/ast_extensions.dart b/lib/src/ast_extensions.dart
index 7a1366e5..566cf1a4 100644
--- a/lib/src/ast_extensions.dart
+++ b/lib/src/ast_extensions.dart
@@ -156,14 +156,6 @@ extension ExpressionExtensions on Expression {
       expression = expression.expression;
     }
 
-    // TODO(tall): We should also allow multi-line strings to be formatted
-    // like block arguments, at least in some cases like:
-    //
-    //     function('''
-    //       Lots of
-    //       text
-    //     ''');
-
     // TODO(tall): Consider whether immediately-invoked function expressions
     // should be block argument candidates, like:
     //
@@ -177,6 +169,8 @@ extension ExpressionExtensions on Expression {
         parameters.parameters.canSplit(parameters.rightParenthesis) ||
             (body is BlockFunctionBody &&
                 body.block.statements.canSplit(body.block.rightBracket)),
+
+      // Non-empty collection literals can block split.
       ListLiteral(:var elements, :var rightBracket) ||
       SetOrMapLiteral(:var elements, :var rightBracket) =>
         elements.canSplit(rightBracket),
@@ -184,9 +178,17 @@ extension ExpressionExtensions on Expression {
         fields.canSplit(rightParenthesis),
       SwitchExpression(:var cases, :var rightBracket) =>
         cases.canSplit(rightBracket),
+
+      // Function calls can block split if their argument lists can.
       InstanceCreationExpression(:var argumentList) ||
       MethodInvocation(:var argumentList) =>
         argumentList.arguments.canSplit(argumentList.rightParenthesis),
+
+      // Multi-line strings can.
+      StringInterpolation(isMultiline: true) => true,
+      SimpleStringLiteral(isMultiline: true) => true,
+
+      // Parenthesized expressions can if the inner one can.
       ParenthesizedExpression(:var expression) => expression.canBlockSplit,
       _ => false,
     };
diff --git a/lib/src/back_end/code_writer.dart b/lib/src/back_end/code_writer.dart
index ecdc6bc8..97e6a0f7 100644
--- a/lib/src/back_end/code_writer.dart
+++ b/lib/src/back_end/code_writer.dart
@@ -128,22 +128,6 @@ class CodeWriter {
         isValid: !_hasInvalidNewline, overflow: _overflow, cost: _cost);
   }
 
-  /// Notes that a newline has been written.
-  ///
-  /// If this occurs in a place where newlines are prohibited, then invalidates
-  /// the solution.
-  ///
-  /// This is called externally by [TextPiece] to let the writer know some of
-  /// the raw text contains a newline, which can happen in multi-line block
-  /// comments and multi-line string literals.
-  void handleNewline() {
-    if (!_options.allowNewlines) _hasInvalidNewline = true;
-
-    // Note that this piece contains a newline so that we can propagate that
-    // up to containing pieces too.
-    _options.hasNewline = true;
-  }
-
   /// Appends [text] to the output.
   ///
   /// If [text] contains any internal newlines, the caller is responsible for
@@ -204,16 +188,21 @@ class CodeWriter {
   ///
   /// If [indent] is given, set the indentation of the new line (and all
   /// subsequent lines) to that indentation relative to the containing piece.
-  void newline({bool blank = false, int? indent}) {
+  ///
+  /// If [flushLeft] is `true`, then the new line begins at column 1 and ignores
+  /// any surrounding indentation. This is used for multi-line block comments
+  /// and multi-line strings.
+  void newline({bool blank = false, int? indent, bool flushLeft = false}) {
     if (indent != null) setIndent(indent);
 
-    whitespace(blank ? Whitespace.blankLine : Whitespace.newline);
+    whitespace(blank ? Whitespace.blankLine : Whitespace.newline,
+        flushLeft: flushLeft);
   }
 
-  void whitespace(Whitespace whitespace) {
+  void whitespace(Whitespace whitespace, {bool flushLeft = false}) {
     if (whitespace case Whitespace.newline || Whitespace.blankLine) {
-      handleNewline();
-      _pendingIndent = _options.indent;
+      _handleNewline();
+      _pendingIndent = flushLeft ? 0 : _options.indent;
     }
 
     _pendingWhitespace = _pendingWhitespace.collapse(whitespace);
@@ -246,9 +235,7 @@ class CodeWriter {
     var childOptions = _pieceOptions.removeLast();
 
     // If the child [piece] contains a newline then this one transitively does.
-    // TODO(tall): At some point, we may want to provide an API so that pieces
-    // can block this from propagating outward.
-    if (childOptions.hasNewline) handleNewline();
+    if (childOptions.hasNewline) _handleNewline();
   }
 
   /// Format [piece] if not null.
@@ -272,6 +259,18 @@ class CodeWriter {
     _selectionEnd = _buffer.length + end;
   }
 
+  /// Notes that a newline has been written.
+  ///
+  /// If this occurs in a place where newlines are prohibited, then invalidates
+  /// the solution.
+  void _handleNewline() {
+    if (!_options.allowNewlines) _hasInvalidNewline = true;
+
+    // Note that this piece contains a newline so that we can propagate that
+    // up to containing pieces too.
+    _options.hasNewline = true;
+  }
+
   /// Write any pending whitespace.
   ///
   /// This is called before non-whitespace text is about to be written, or
diff --git a/lib/src/front_end/ast_node_visitor.dart b/lib/src/front_end/ast_node_visitor.dart
index f161711b..e08cae17 100644
--- a/lib/src/front_end/ast_node_visitor.dart
+++ b/lib/src/front_end/ast_node_visitor.dart
@@ -1070,12 +1070,17 @@ class AstNodeVisitor extends ThrowingAstVisitor<Piece> with PieceFactory {
 
   @override
   Piece visitInterpolationExpression(InterpolationExpression node) {
-    throw UnimplementedError();
+    return buildPiece((b) {
+      b.token(node.leftBracket);
+      b.visit(node.expression);
+      b.token(node.rightBracket);
+    });
   }
 
   @override
   Piece visitInterpolationString(InterpolationString node) {
-    throw UnimplementedError();
+    return pieces.stringLiteralPiece(node.contents,
+        isMultiline: (node.parent as StringInterpolation).isMultiline);
   }
 
   @override
@@ -1521,7 +1526,8 @@ class AstNodeVisitor extends ThrowingAstVisitor<Piece> with PieceFactory {
 
   @override
   Piece visitSimpleStringLiteral(SimpleStringLiteral node) {
-    return tokenPiece(node.literal);
+    return pieces.stringLiteralPiece(node.literal,
+        isMultiline: node.isMultiline);
   }
 
   @override
@@ -1534,7 +1540,11 @@ class AstNodeVisitor extends ThrowingAstVisitor<Piece> with PieceFactory {
 
   @override
   Piece visitStringInterpolation(StringInterpolation node) {
-    throw UnimplementedError();
+    return buildPiece((b) {
+      for (var element in node.elements) {
+        b.visit(element);
+      }
+    });
   }
 
   @override
diff --git a/lib/src/front_end/piece_writer.dart b/lib/src/front_end/piece_writer.dart
index d85eb24a..6f12116d 100644
--- a/lib/src/front_end/piece_writer.dart
+++ b/lib/src/front_end/piece_writer.dart
@@ -11,6 +11,9 @@ import '../piece/piece.dart';
 import '../source_code.dart';
 import 'comment_writer.dart';
 
+/// RegExp that matches any valid Dart line terminator.
+final _lineTerminatorPattern = RegExp(r'\r\n?|\n');
+
 /// Builds [TextPiece]s for [Token]s and comments.
 ///
 /// Handles updating selection markers and attaching comments to the tokens
@@ -69,6 +72,22 @@ class PieceWriter {
     return tokenPiece;
   }
 
+  /// Creates a piece for a simple or interpolated string [literal].
+  ///
+  /// Handles splitting it into multiple lines in the resulting [TextPiece] if
+  /// [isMultiline] is `true`.
+  Piece stringLiteralPiece(Token literal, {required bool isMultiline}) {
+    if (!isMultiline) return tokenPiece(literal);
+
+    if (!_writeCommentsBefore(literal)) {
+      // We want this token to be in its own TextPiece, so if the comments
+      // didn't already lead to ending the previous TextPiece than do so now.
+      _currentText = TextPiece();
+    }
+
+    return _writeMultiLine(literal.lexeme, offset: literal.offset);
+  }
+
   // TODO(tall): Much of the comment handling code in CommentWriter got moved
   // into here, so there isn't great separation of concerns anymore. Can we
   // organize this code better? Or just combine CommentWriter with this class
@@ -95,9 +114,7 @@ class PieceWriter {
   Piece writeComment(SourceComment comment) {
     _currentText = TextPiece();
 
-    _write(comment.text,
-        offset: comment.offset, containsNewline: comment.text.contains('\n'));
-    return _currentText;
+    return _writeMultiLine(comment.text, offset: comment.offset);
   }
 
   /// Writes all of the comments that appear between [token] and the previous
@@ -146,8 +163,7 @@ class PieceWriter {
         _currentText.newline();
       }
 
-      _write(comment.text,
-          offset: comment.offset, containsNewline: comment.text.contains('\n'));
+      _write(comment.text, offset: comment.offset);
     }
 
     // Output a trailing newline after the last comment if it needs one.
@@ -180,14 +196,31 @@ class PieceWriter {
       _currentText = TextPiece();
     }
 
-    _write(lexeme ?? token.lexeme, offset: token.offset);
+    lexeme ??= token.lexeme;
+
+    _write(lexeme, offset: token.offset);
+  }
+
+  /// Writes multi-line [text] to the current [TextPiece].
+  ///
+  /// Handles breaking [text] into lines and adding them to the [TextPiece].
+  Piece _writeMultiLine(String lexeme, {required int offset}) {
+    var lines = lexeme.split(_lineTerminatorPattern);
+    var currentOffset = offset;
+    for (var i = 0; i < lines.length; i++) {
+      if (i > 0) _currentText.newline(flushLeft: true);
+      _write(lines[i], offset: currentOffset);
+      currentOffset += lines[i].length;
+    }
+
+    return _currentText;
   }
 
   /// Writes [text] to the current [TextPiece].
   ///
   /// If [offset] is given and it contains any selection markers, then attaches
   /// those markers to the [TextPiece].
-  void _write(String text, {bool containsNewline = false, int? offset}) {
+  void _write(String text, {int? offset}) {
     if (offset != null) {
       // If this text contains any of the selection endpoints, note their
       // relative locations in the text piece.
@@ -200,7 +233,7 @@ class PieceWriter {
       }
     }
 
-    _currentText.append(text, containsNewline: containsNewline);
+    _currentText.append(text);
   }
 
   /// Finishes writing and returns a [SourceCode] containing the final output
diff --git a/lib/src/piece/piece.dart b/lib/src/piece/piece.dart
index 3c5884b1..dfa7550a 100644
--- a/lib/src/piece/piece.dart
+++ b/lib/src/piece/piece.dart
@@ -98,13 +98,7 @@ class TextPiece extends Piece {
   /// preceding comments that are on their own line will have multiple. These
   /// are stored as separate lines instead of a single multi-line string so that
   /// each line can be indented appropriately during formatting.
-  final List<String> _lines = [];
-
-  /// True if this text piece contains or ends with a mandatory newline.
-  ///
-  /// This can be from line comments, block comments with newlines inside,
-  /// multiline strings, etc.
-  bool _containsNewline = false;
+  final List<_Line> _lines = [];
 
   /// Whitespace at the end of this [TextPiece].
   ///
@@ -116,6 +110,14 @@ class TextPiece extends Piece {
   /// empty [_lines] list on the first write.
   Whitespace _trailingWhitespace = Whitespace.newline;
 
+  /// Whether the line after the next newline written should be fixed at column
+  /// one or indented to match the surrounding code.
+  ///
+  /// This is false for most lines, but is true for multiline strings where
+  /// subsequent lines in the string don't get any additional indentation from
+  /// formatting.
+  bool _flushLeft = false;
+
   /// The offset from the beginning of [text] where the selection starts, or
   /// `null` if the selection does not start within this chunk.
   int? _selectionStart;
@@ -125,48 +127,43 @@ class TextPiece extends Piece {
   int? _selectionEnd;
 
   /// Whether the last line of this piece's text ends with [text].
-  bool endsWith(String text) => _lines.isNotEmpty && _lines.last.endsWith(text);
+  bool endsWith(String text) =>
+      _lines.isNotEmpty && _lines.last._text.endsWith(text);
 
   /// Append [text] to the end of this piece.
   ///
   /// If [text] internally contains a newline, then [containsNewline] should
   /// be `true`.
-  void append(String text, {bool containsNewline = false}) {
+  void append(String text) {
     // Write any pending whitespace into the text.
     switch (_trailingWhitespace) {
       case Whitespace.none:
         break; // Nothing to do.
       case Whitespace.space:
-        // TODO(perf): Consider a faster way of accumulating text.
-        _lines.last += ' ';
+        _lines.last.append(' ');
       case Whitespace.newline:
-        _lines.add('');
+        _lines.add(_Line(flushLeft: _flushLeft));
       case Whitespace.blankLine:
         throw UnsupportedError('No blank lines in TextPieces.');
     }
 
     _trailingWhitespace = Whitespace.none;
+    _flushLeft = false;
 
-    // TODO(perf): Consider a faster way of accumulating text.
-    _lines.last = _lines.last + text;
-
-    if (containsNewline) _containsNewline = true;
+    _lines.last.append(text);
   }
 
   void space() {
     _trailingWhitespace = _trailingWhitespace.collapse(Whitespace.space);
   }
 
-  void newline() {
+  void newline({bool flushLeft = false}) {
     _trailingWhitespace = _trailingWhitespace.collapse(Whitespace.newline);
+    _flushLeft = flushLeft;
   }
 
   @override
   void format(CodeWriter writer, State state) {
-    // Let the writer know if there are any embedded newlines even if there is
-    // only one "line" in [_lines].
-    if (_containsNewline) writer.handleNewline();
-
     if (_selectionStart case var start?) {
       writer.startSelection(start);
     }
@@ -176,8 +173,9 @@ class TextPiece extends Piece {
     }
 
     for (var i = 0; i < _lines.length; i++) {
-      if (i > 0) writer.newline();
-      writer.write(_lines[i]);
+      var line = _lines[i];
+      if (i > 0) writer.newline(flushLeft: line._isFlushLeft);
+      writer.write(line._text);
     }
 
     writer.whitespace(_trailingWhitespace);
@@ -201,7 +199,7 @@ class TextPiece extends Piece {
   /// Adjust [offset] by the current length of this [TextPiece].
   int _adjustSelection(int offset) {
     for (var line in _lines) {
-      offset += line.length;
+      offset += line._text.length;
     }
 
     if (_trailingWhitespace == Whitespace.space) offset++;
@@ -210,7 +208,26 @@ class TextPiece extends Piece {
   }
 
   @override
-  String toString() => '`${_lines.join('¬')}`${_containsNewline ? '!' : ''}';
+  String toString() => '`${_lines.join('¬')}`';
+}
+
+/// A single line of text within a [TextPiece].
+class _Line {
+  String _text = '';
+
+  /// Whether this line should start at column one or use the surrounding
+  /// indentation.
+  final bool _isFlushLeft;
+
+  _Line({required bool flushLeft}) : _isFlushLeft = flushLeft;
+
+  void append(String text) {
+    // TODO(perf): Consider a faster way of accumulating text.
+    _text += text;
+  }
+
+  @override
+  String toString() => _text;
 }
 
 /// A piece that writes a single space.
diff --git a/test/expression/interpolation.stmt b/test/expression/interpolation.stmt
new file mode 100644
index 00000000..61df5478
--- /dev/null
+++ b/test/expression/interpolation.stmt
@@ -0,0 +1,82 @@
+40 columns                              |
+>>> Fix whitespace.
+" ${   interp+olate } and ${fn  (  1 ) } end";
+<<<
+" ${interp + olate} and ${fn(1)} end";
+>>> Allow splitting in interpolation if there is nowhere else.
+"some text that is long ${interpolate + a + thing} more";
+<<<
+"some text that is long ${interpolate +
+    a +
+    thing} more";
+>>> Prefer not splitting in interpolation.
+"first string ${has + interpolation}" + "another ${inter + polated}";
+<<<
+"first string ${has + interpolation}" +
+    "another ${inter + polated}";
+>>> Split interpolation in multi-line string.
+"""
+some text that is pretty long
+some more text that is pretty long ${    interpolate + a + thing   } more text
+""";
+<<<
+"""
+some text that is pretty long
+some more text that is pretty long ${interpolate +
+    a +
+    thing} more text
+""";
+>>> Split in nested interpolation.
+"some text that is ${pretty +  'long ${    interpolate +
+a + thing   } more'} text";
+<<<
+"some text that is ${pretty +
+    'long ${interpolate +
+        a +
+        thing} more'} text";
+>>> Mandatory newline in interpolation.
+"before ${(){statement();statement();statement();}} after";
+<<<
+"before ${() {
+  statement();
+  statement();
+  statement();
+}} after";
+>>> Deeply nested interpolation.
+'''a
+${b +
+"""c
+${d
++ '''e
+f'''
++
+g}
+h"""
++ i}
+j ${k
++
+l}''';
+<<<
+'''a
+${b +
+    """c
+${d +
+        '''e
+f''' +
+        g}
+h""" +
+    i}
+j ${k + l}''';
+>>> Line comment at beginning of interpolation.
+"before ${// comment
+a + b} after";
+<<<
+"before ${ // comment
+a + b} after";
+>>> Line comment inside interpolation.
+"before ${
+a + // comment
+b} after";
+<<<
+"before ${a + // comment
+    b} after";
\ No newline at end of file
diff --git a/test/expression/string.stmt b/test/expression/string.stmt
new file mode 100644
index 00000000..c0b40b32
--- /dev/null
+++ b/test/expression/string.stmt
@@ -0,0 +1,88 @@
+40 columns                              |
+>>> Long single-line.
+"this string is longer than forty characters";
+<<<
+"this string is longer than forty characters";
+>>> Empty multi-line.
+{""""""; '''''';}
+<<<
+{
+  """""";
+  '''''';
+}
+>>> Blank lines in multi-line.
+'''
+
+
+two before
+
+one
+
+
+two
+
+
+''';
+<<<
+'''
+
+
+two before
+
+one
+
+
+two
+
+
+''';
+>>> Short one line multi-line.
+"""not too long""";
+<<<
+"""not too long""";
+>>> Multi-line with short lines.
+"""
+not too long
+or this one
+""";
+<<<
+"""
+not too long
+or this one
+""";
+>>> Multi-line with long lines.
+"""
+this string is longer than forty characters
+this one is also is longer than forty characters
+""";
+<<<
+"""
+this string is longer than forty characters
+this one is also is longer than forty characters
+""";
+>>> Only indent the first line of multiline strings.
+{
+"""
+multiline
+""";
+}
+<<<
+{
+  """
+multiline
+""";
+}
+>>>
+main() {
+  inner() {
+    function("""
+string""");
+  }
+}
+<<<
+main() {
+  inner() {
+    function("""
+string""");
+  }
+}
\ No newline at end of file
diff --git a/test/expression/string_comment.stmt b/test/expression/string_comment.stmt
new file mode 100644
index 00000000..63586245
--- /dev/null
+++ b/test/expression/string_comment.stmt
@@ -0,0 +1,72 @@
+40 columns                              |
+### Since both comments and multi-line strings involve adding lines to the same
+### TextPiece, make sure they don't get confused.
+>>> Line comment before indented multi-line string.
+{{
+before +
+// comment 1
+// comment 2
+// comment 3
+"""multi
+line
+string
+""";
+}}
+<<<
+{
+  {
+    before +
+        // comment 1
+        // comment 2
+        // comment 3
+        """multi
+line
+string
+""";
+  }
+}
+>>> Line comment after indented multi-line string.
+{{
+"""multi
+line
+string
+""" // comment 1
+// comment 2
+// comment 3
++ after;
+}}
+<<<
+{
+  {
+    """multi
+line
+string
+""" // comment 1
+        // comment 2
+        // comment 3
+        +
+        after;
+  }
+}
+>>> Line comment after indented multi-line string.
+{{
+"""multi
+line
+string
+""" + // comment 1
+// comment 2
+// comment 3
+after;
+}}
+<<<
+{
+  {
+    """multi
+line
+string
+""" + // comment 1
+        // comment 2
+        // comment 3
+        after;
+  }
+}
\ No newline at end of file
diff --git a/test/invocation/block_argument_multiple.stmt b/test/invocation/block_argument_multiple.stmt
index 894b1ab5..643dac77 100644
--- a/test/invocation/block_argument_multiple.stmt
+++ b/test/invocation/block_argument_multiple.stmt
@@ -63,4 +63,13 @@ function(switch (a) {}, switch (b) { 1 => 2 }, switch (c) {});
 <<<
 function(switch (a) {}, switch (b) {
   1 => 2,
-}, switch (c) {});
\ No newline at end of file
+}, switch (c) {});
+>>> Collection and multi-line string prevents block formatting.
+function([element, element], '''multiple
+lines''');
+<<<
+function(
+  [element, element],
+  '''multiple
+lines''',
+);
\ No newline at end of file
diff --git a/test/invocation/block_argument_string.stmt b/test/invocation/block_argument_string.stmt
new file mode 100644
index 00000000..b3cb30e5
--- /dev/null
+++ b/test/invocation/block_argument_string.stmt
@@ -0,0 +1,97 @@
+40 columns                              |
+### Test multi-line strings as block arguments.
+>>> Allow block formatting a multi-line string.
+someMethod("""first line fits in here
+more stuff down here too that is long
+""");
+<<<
+someMethod("""first line fits in here
+more stuff down here too that is long
+""");
+>>>
+someMethod('''first line fits in here
+more stuff down here too that is long
+''');
+<<<
+someMethod('''first line fits in here
+more stuff down here too that is long
+''');
+>>> Allow block formatting a multi-line string with interpolation.
+someMethod("""first line fits in here
+more stuff $down here too that is long
+""");
+<<<
+someMethod("""first line fits in here
+more stuff $down here too that is long
+""");
+>>>
+someMethod('''first line fits in here
+more stuff ${down + here} that is long
+''');
+<<<
+someMethod('''first line fits in here
+more stuff ${down + here} that is long
+''');
+>>> Don't block format if first line doesn't fit.
+someMethod("""first line does not fit here
+""");
+<<<
+someMethod(
+  """first line does not fit here
+""",
+);
+>>> Block format multi-line string with non-block arguments before.
+someMethod("foo", "bar", """
+some
+text
+""");
+<<<
+someMethod("foo", "bar", """
+some
+text
+""");
+>>> Block format multi-line string with non-block arguments after.
+someMethod("""
+some
+text
+""", "foo", "bar");
+<<<
+someMethod("""
+some
+text
+""", "foo", "bar");
+>>> Block format multi-line string with non-block arguments before and after.
+someMethod("foo", """
+some
+text
+""",
+"bar");
+<<<
+someMethod("foo", """
+some
+text
+""", "bar");
+>>> Can't have multiple block formatted multi-line strings.
+someMethod("""
+some
+text
+""", """
+some
+more
+""", """
+even more
+""");
+<<<
+someMethod(
+  """
+some
+text
+""",
+  """
+some
+more
+""",
+  """
+even more
+""",
+);
\ No newline at end of file
diff --git a/test/selection/selection.stmt b/test/selection/selection.stmt
index 5c02d315..51a9e629 100644
--- a/test/selection/selection.stmt
+++ b/test/selection/selection.stmt
@@ -49,6 +49,10 @@ sec‹ond"""  ;›
 <<<
 """first
 sec‹ond""";›
+>>> In string interpolation.
+foo(  "$fi‹rst",  "${  sec›ond  }" );
+<<<
+foo("$fi‹rst", "${sec›ond}");
 >>> Only whitespace in zero space selected.
 foo(  ‹  ›  argument);
 <<<
diff --git a/test/tall_format_test.dart b/test/tall_format_test.dart
index 3f7b3ae9..91761a7d 100644
--- a/test/tall_format_test.dart
+++ b/test/tall_format_test.dart
@@ -5,6 +5,7 @@
 @TestOn('vm')
 library dart_style.test.tall_format_test;
 
+import 'package:dart_style/dart_style.dart';
 import 'package:test/test.dart';
 
 import 'utils.dart';
@@ -20,7 +21,118 @@ void main() async {
   await testDirectory('type', tall: true);
   await testDirectory('variable', tall: true);
 
-  // TODO(tall): The old formatter_test.dart has tests here for things like
-  // trailing newlines. Port those over to the new style once it supports all
-  // the syntax those tests rely on.
+  test('throws a FormatterException on failed parse', () {
+    var formatter = DartFormatter();
+    expect(() => formatter.format('wat?!'), throwsA(isA<FormatterException>()));
+  });
+
+  test('FormatterException.message() does not throw', () {
+    // This is a regression test for #358 where an error whose position is
+    // past the end of the source caused FormatterException to throw.
+    try {
+      DartFormatter().format('library');
+    } on FormatterException catch (err) {
+      var message = err.message();
+      expect(message, contains('Could not format'));
+    }
+  });
+
+  test('FormatterException describes parse errors', () {
+    try {
+      DartFormatter().format('''
+
+      var a = some error;
+
+      var b = another one;
+      ''', uri: 'my_file.dart');
+
+      fail('Should throw.');
+    } on FormatterException catch (err) {
+      var message = err.message();
+      expect(message, contains('my_file.dart'));
+      expect(message, contains('line 2'));
+      expect(message, contains('line 4'));
+    }
+  });
+
+  test('adds newline to unit', () {
+    expect(DartFormatter().format('var x = 1;'), equals('var x = 1;\n'));
+  });
+
+  test('adds newline to unit after trailing comment', () {
+    expect(DartFormatter().format('library foo; //zamm'),
+        equals('library foo; //zamm\n'));
+  });
+
+  test('removes extra newlines', () {
+    expect(DartFormatter().format('var x = 1;\n\n\n'), equals('var x = 1;\n'));
+  });
+
+  test('does not add newline to statement', () {
+    expect(DartFormatter().formatStatement('var x = 1;'), equals('var x = 1;'));
+  });
+
+  test('fails if anything is after the statement', () {
+    try {
+      DartFormatter().formatStatement('var x = 1;;');
+
+      fail('Should throw.');
+    } on FormatterException catch (ex) {
+      expect(ex.errors.length, equals(1));
+      expect(ex.errors.first.offset, equals(10));
+    }
+  });
+
+  test('preserves initial indent', () {
+    var formatter = DartFormatter(indent: 3);
+    expect(
+        formatter.formatStatement('if (foo) {bar;}'),
+        equals('   if (foo) {\n'
+            '     bar;\n'
+            '   }'));
+  });
+
+  group('line endings', () {
+    test('uses given line ending', () {
+      // Use zero width no-break space character as the line ending. We have
+      // to use a whitespace character for the line ending as the formatter
+      // will throw an error if it accidentally makes non-whitespace changes
+      // as will occur
+      var lineEnding = '\t';
+      expect(DartFormatter(lineEnding: lineEnding).format('var i = 1;'),
+          equals('var i = 1;\t'));
+    });
+
+    test('infers \\r\\n if the first newline uses that', () {
+      expect(DartFormatter().format('var\r\ni\n=\n1;\n'),
+          equals('var i = 1;\r\n'));
+    });
+
+    test('infers \\n if the first newline uses that', () {
+      expect(DartFormatter().format('var\ni\r\n=\r\n1;\r\n'),
+          equals('var i = 1;\n'));
+    });
+
+    test('defaults to \\n if there are no newlines', () {
+      expect(DartFormatter().format('var i =1;'), equals('var i = 1;\n'));
+    });
+
+    test('handles Windows line endings in multiline strings', () {
+      expect(
+          DartFormatter(lineEnding: '\r\n').formatStatement('  """first\r\n'
+              'second\r\n'
+              'third"""  ;'),
+          equals('"""first\r\n'
+              'second\r\n'
+              'third""";'));
+    });
+  });
+
+  test('throws an UnexpectedOutputException on non-whitespace changes', () {
+    // Use an invalid line ending character to ensure the formatter will
+    // attempt to make non-whitespace changes.
+    var formatter = DartFormatter(lineEnding: '%');
+    expect(() => formatter.format('var i = 1;'),
+        throwsA(isA<UnexpectedOutputException>()));
+  });
 }

From f09bdc11d8e85765fe417aa23b6c2702cc3f1f88 Mon Sep 17 00:00:00 2001
From: Robert Nystrom <rnystrom@google.com>
Date: Mon, 22 Jan 2024 15:46:39 -0800
Subject: [PATCH 2/3] Add test description.

---
 test/expression/string.stmt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/expression/string.stmt b/test/expression/string.stmt
index c0b40b32..054808c5 100644
--- a/test/expression/string.stmt
+++ b/test/expression/string.stmt
@@ -72,7 +72,7 @@ multiline
 multiline
 """;
 }
->>>
+>>> Only indent the first line of multiline strings.
 main() {
   inner() {
     function("""

From f914d0f1a60f8e2766cf7526ecf2b1c8deb3a5c3 Mon Sep 17 00:00:00 2001
From: Robert Nystrom <rnystrom@google.com>
Date: Mon, 22 Jan 2024 19:28:53 -0800
Subject: [PATCH 3/3] Apply review feedback.

---
 lib/src/back_end/code_writer.dart   |  9 +++++++
 lib/src/front_end/piece_writer.dart |  7 ++++--
 test/tall_format_test.dart          | 38 +++++++++++++----------------
 3 files changed, 31 insertions(+), 23 deletions(-)

diff --git a/lib/src/back_end/code_writer.dart b/lib/src/back_end/code_writer.dart
index 97e6a0f7..ae2bfdaa 100644
--- a/lib/src/back_end/code_writer.dart
+++ b/lib/src/back_end/code_writer.dart
@@ -199,6 +199,15 @@ class CodeWriter {
         flushLeft: flushLeft);
   }
 
+  /// Queues [whitespace] to be written to the output.
+  ///
+  /// If any non-whitespace is written after this call, then this whitespace
+  /// will be written first. Also handles merging multiple kinds of whitespace
+  /// intelligently together.
+  ///
+  /// If [flushLeft] is `true`, then the new line begins at column 1 and ignores
+  /// any surrounding indentation. This is used for multi-line block comments
+  /// and multi-line strings.
   void whitespace(Whitespace whitespace, {bool flushLeft = false}) {
     if (whitespace case Whitespace.newline || Whitespace.blankLine) {
       _handleNewline();
diff --git a/lib/src/front_end/piece_writer.dart b/lib/src/front_end/piece_writer.dart
index 6f12116d..c5c7e5a6 100644
--- a/lib/src/front_end/piece_writer.dart
+++ b/lib/src/front_end/piece_writer.dart
@@ -204,8 +204,11 @@ class PieceWriter {
   /// Writes multi-line [text] to the current [TextPiece].
   ///
   /// Handles breaking [text] into lines and adding them to the [TextPiece].
-  Piece _writeMultiLine(String lexeme, {required int offset}) {
-    var lines = lexeme.split(_lineTerminatorPattern);
+  ///
+  /// The [offset] parameter is the offset in the original source code of the
+  /// beginning of multi-line lexeme.
+  Piece _writeMultiLine(String text, {required int offset}) {
+    var lines = text.split(_lineTerminatorPattern);
     var currentOffset = offset;
     for (var i = 0; i < lines.length; i++) {
       if (i > 0) _currentText.newline(flushLeft: true);
diff --git a/test/tall_format_test.dart b/test/tall_format_test.dart
index 91761a7d..094af2cc 100644
--- a/test/tall_format_test.dart
+++ b/test/tall_format_test.dart
@@ -29,16 +29,14 @@ void main() async {
   test('FormatterException.message() does not throw', () {
     // This is a regression test for #358 where an error whose position is
     // past the end of the source caused FormatterException to throw.
-    try {
-      DartFormatter().format('library');
-    } on FormatterException catch (err) {
-      var message = err.message();
-      expect(message, contains('Could not format'));
-    }
+    expect(
+        () => DartFormatter().format('library'),
+        throwsA(isA<FormatterException>().having(
+            (e) => e.message(), 'message', contains('Could not format'))));
   });
 
   test('FormatterException describes parse errors', () {
-    try {
+    expect(() {
       DartFormatter().format('''
 
       var a = some error;
@@ -47,12 +45,12 @@ void main() async {
       ''', uri: 'my_file.dart');
 
       fail('Should throw.');
-    } on FormatterException catch (err) {
-      var message = err.message();
-      expect(message, contains('my_file.dart'));
-      expect(message, contains('line 2'));
-      expect(message, contains('line 4'));
-    }
+    },
+        throwsA(isA<FormatterException>().having(
+            (e) => e.message(),
+            'message',
+            allOf(contains('Could not format'), contains('line 2'),
+                contains('line 4')))));
   });
 
   test('adds newline to unit', () {
@@ -73,14 +71,12 @@ void main() async {
   });
 
   test('fails if anything is after the statement', () {
-    try {
-      DartFormatter().formatStatement('var x = 1;;');
-
-      fail('Should throw.');
-    } on FormatterException catch (ex) {
-      expect(ex.errors.length, equals(1));
-      expect(ex.errors.first.offset, equals(10));
-    }
+    expect(
+        () => DartFormatter().formatStatement('var x = 1;;'),
+        throwsA(isA<FormatterException>()
+            .having((e) => e.errors.length, 'errors.length', equals(1))
+            .having((e) => e.errors.first.offset, 'errors.length.first.offset',
+                equals(10))));
   });
 
   test('preserves initial indent', () {