Merge branch 'main' into format-comments

# Conflicts:
#	example/format.dart
#	lib/src/back_end/code_writer.dart
#	lib/src/back_end/solution.dart
#	lib/src/back_end/solver.dart
#	lib/src/dart_formatter.dart
#	lib/src/front_end/ast_node_visitor.dart
#	lib/src/front_end/piece_factory.dart
#	lib/src/front_end/piece_writer.dart
#	lib/src/piece/import.dart
#	lib/src/piece/piece.dart
#	test/statement/block.stmt
#	test/utils.dart

CHANGELOG.md

@@ -1,4 +1,9 @@
-## 2.3.3-dev
+## 2.3.4-wip
+
+* Add `tall-style` experiment flag to enable the in-progress unstable new
+  formatting style (#1253).
+
+## 2.3.3
 
 * Always split enum declarations containing a line comment (#1254).
 * Fix regression in splitting type annotations with library prefixes (#1249).

example/format.dart

@@ -7,6 +7,7 @@ import 'dart:io';
 import 'dart:mirrors';
 
 import 'package:dart_style/dart_style.dart';
+import 'package:dart_style/src/constants.dart';
 import 'package:dart_style/src/debug.dart' as debug;
 import 'package:path/path.dart' as p;
 
@@ -35,7 +36,8 @@ void runFormatter(String source, int pageWidth,
     {required bool tall, required bool isCompilationUnit}) {
   try {
     var formatter = DartFormatter(
-        pageWidth: pageWidth, experimentFlags: [if (tall) 'tall-style']);
+        pageWidth: pageWidth,
+        experimentFlags: [if (tall) tallStyleExperimentFlag]);
 
     String result;
     if (isCompilationUnit) {

lib/src/back_end/code_writer.dart

@@ -5,8 +5,21 @@ import '../constants.dart';
 import '../piece/piece.dart';
 import 'solution.dart';
 
-/// The interface that pieces use when to produce output formatting themselves
-/// during state solving.
+/// The interface used by [Piece]s to output formatted code.
+///
+/// The back-end lowers the tree of pieces to the final formatted code by
+/// allowing each piece to produce the output for the code it represents.
+/// This way, each piece has full flexibility for how to apply its own
+/// formatting logic.
+///
+/// To build the resulting output code, when a piece is formatted, it is passed
+/// an instance of this class. It has methods that the piece can call to add
+/// output text to the resulting code, recursively format child pieces, insert
+/// whitespace, etc.
+///
+/// This class also accumulates the score (the relative desireability of a set
+/// of formatting choices) that the resulting code has by tracking things like
+/// how many characters of code overflow the page width.
 class CodeWriter {
   final int _pageWidth;
 
@@ -19,19 +32,25 @@ class CodeWriter {
   /// The cost of the currently chosen line splits.
   int _cost = 0;
 
-  /// The number of characters of code that have overflowed the page width so
-  /// far.
+  /// The total number of characters of code that have overflowed the page
+  /// width so far.
   int _overflow = 0;
 
-  /// How long the line currently being written is.
+  /// The number of characters in the line currently being written.
   int _column = 0;
 
   /// Whether this solution has encountered a newline where none is allowed.
-  /// This means the solution is invalid.
+  ///
+  /// If true, it means the solution is invalid.
   bool _containsInvalidNewline = false;
 
-  /// For each piece being formatted from a call to [format], this tracks the
-  /// indentation of any new lines it begins.
+  /// The stack of state for each [Piece] being formatted.
+  ///
+  /// For each piece being formatted from a call to [format()], we keep track of
+  /// things like indentation and nesting levels. Pieces recursively format
+  /// their children. When they do, we push new values onto this stack. When a
+  /// piece is done (a call to [format()] returns), we pop the corresponding
+  /// state off the stack.
   ///
   /// This is used to increase the cumulative nesting as we recurse into pieces
   /// and then unwind that as child pieces are completed.
@@ -64,17 +83,20 @@ class CodeWriter {
     if (!_options.allowNewlines) _containsInvalidNewline = true;
   }
 
+  /// Appends [text] to the output.
+  ///
+  /// If [text] contains any internal newlines, the caller is responsible for
+  /// also calling [handleNewline()].
   void write(String text) {
     _buffer.write(text);
     _column += text.length;
   }
 
   /// Sets the number of spaces of indentation for code written by the current
-  /// piece to [indent].
+  /// piece to [indent], relative to the indentation of the surrounding piece.
   ///
   /// Replaces any previous indentation set by this piece.
   void setIndent(int indent) {
-    // Include indentation from surrounding pieces.
     _options.indent = _pieceOptions[_pieceOptions.length - 2].indent + indent;
   }
 
@@ -87,11 +109,11 @@ class CodeWriter {
   }
 
   /// Sets the number of spaces of expression nesting for code written by the
-  /// current piece to [nesting].
+  /// current piece to [nesting], relative to the nesting of the surrounding
+  /// piece.
   ///
   /// Replaces any previous nesting set by this piece.
   void setNesting(int nesting) {
-    // Include nesting from surrounding pieces.
     _options.nesting =
         _pieceOptions[_pieceOptions.length - 2].nesting + nesting;
   }
@@ -117,8 +139,9 @@ class CodeWriter {
     write(' ');
   }
 
-  /// Inserts a line split in the output. If [blank] is true, writes an extra
-  /// newline to produce a blank line.
+  /// Inserts a line split in the output.
+  ///
+  /// If [blank] is true, writes an extra newline to produce a blank line.
   void newline({bool blank = false}) {
     handleNewline();
     _finishLine();
@@ -130,12 +153,13 @@ class CodeWriter {
   }
 
   /// Sets whether newlines are allowed to occur from this point on for the
-  /// current piece of any of its children.
+  /// current piece or any of its children.
   void setAllowNewlines(bool allowed) {
     _options.allowNewlines = allowed;
   }
 
-  /// Format [piece] and insert the result into the code.
+  /// Format [piece] and insert the result into the code being written and
+  /// returned by [finish()].
   void format(Piece piece) {
     // Don't bother recursing into the piece tree if we know the solution will
     // be discarded.
@@ -175,14 +199,14 @@ class CodeWriter {
   }
 }
 
-/// Tracks the mutable state local to a single piece currently being formatted.
+/// The mutable state local to a single piece being formatted.
 class _PieceOptions {
-  /// The number of spaces of leading indentation coming from block-like
-  /// structure or explicit extra indentation (aligning constructor
+  /// The absolute number of spaces of leading indentation coming from
+  /// block-like structure or explicit extra indentation (aligning constructor
   /// initializers, `show` clauses, etc.).
   int indent;
 
-  /// The number of spaces of indentation from wrapped expressions.
+  /// The absolute number of spaces of indentation from wrapped expressions.
   int nesting;
 
   /// The total number of spaces of indentation.

lib/src/back_end/solution.dart

@@ -94,6 +94,8 @@ class Solution implements Comparable<Solution> {
     return result;
   }
 
+  /// Compares two solutions where a more desirable solution comes first.
+  ///
   /// For performance, we want to stop checking solutions as soon as we find
   /// the best one. Best means the fewest overflow characters and the lowest
   /// code.

lib/src/back_end/solver.dart

@@ -3,7 +3,6 @@
 // BSD-style license that can be found in the LICENSE file.
 import 'package:collection/collection.dart';
 
-import '../dart_formatter.dart';
 import '../debug.dart' as debug;
 import '../piece/piece.dart';
 import 'solution.dart';
@@ -26,11 +25,11 @@ import 'solution.dart';
 // TODO(perf): At some point, we probably also want to do memoization of
 // previously formatted Piece subtrees.
 class Solver {
-  final DartFormatter _formatter;
+  final int _pageWidth;
 
   final PriorityQueue<Solution> _queue = PriorityQueue();
 
-  Solver(this._formatter);
+  Solver(this._pageWidth);
 
   /// Finds the best set of line splits for [piece] and returns the resulting
   /// formatted code.
@@ -53,7 +52,7 @@ class Solver {
   /// Finds the best solution for the piece tree starting at [root] with
   /// selectable [pieces].
   Solution _solve(Piece root, List<Piece> pieces) {
-    var solution = Solution(root, _formatter.pageWidth, PieceStateSet(pieces));
+    var solution = Solution(root, _pageWidth, PieceStateSet(pieces));
     _queue.add(solution);
 
     // The lowest cost solution found so far that does overflow.
@@ -78,7 +77,7 @@ class Solver {
 
       // Otherwise, try to expand the solution to explore different splitting
       // options.
-      for (var expanded in solution.expand(root, _formatter.pageWidth)) {
+      for (var expanded in solution.expand(root, _pageWidth)) {
         _queue.add(expanded);
       }
     }

lib/src/cli/formatter_options.dart

@@ -11,7 +11,7 @@ import 'show.dart';
 import 'summary.dart';
 
 // Note: The following line of code is modified by tool/grind.dart.
-const dartStyleVersion = '2.3.2';
+const dartStyleVersion = '2.3.3';
 
 /// Global options that affect how the formatter produces and uses its outputs.
 class FormatterOptions {

lib/src/constants.dart

@@ -2,6 +2,15 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
+/// The in-progress "tall" formatting style is enabled by passing an experiment
+/// flag with this name.
+///
+/// Note that this isn't a real Dart SDK experiment: Only the formatter supports
+/// it. We use the [experimentFlags] API to pass this in so that support for it
+/// can be removed in a later version without it being a breaking change to the
+/// dart_style library API.
+const tallStyleExperimentFlag = 'tall-style';
+
 /// Constants for the cost heuristics used to determine which set of splits is
 /// most desirable.
 class Cost {

lib/src/dart_formatter.dart

@@ -15,6 +15,7 @@ import 'package:analyzer/src/dart/scanner/scanner.dart';
 import 'package:analyzer/src/string_source.dart';
 import 'package:pub_semver/pub_semver.dart';
 
+import 'constants.dart';
 import 'exceptions.dart';
 import 'front_end/ast_node_visitor.dart';
 import 'source_code.dart';
@@ -187,7 +188,7 @@ class DartFormatter {
     var lineInfo = parseResult.lineInfo;
 
     SourceCode output;
-    if (experimentFlags.contains(_tallStyleExperimentFlag)) {
+    if (experimentFlags.contains(tallStyleExperimentFlag)) {
       var visitor = AstNodeVisitor(this, lineInfo, unitSourceCode);
       output = visitor.run(node);
     } else {
@@ -231,7 +232,7 @@ class DartFormatter {
 
     // Don't pass the formatter's own experiment flag to the parser.
     var experiments = experimentFlags.toList();
-    experiments.remove(_tallStyleExperimentFlag);
+    experiments.remove(tallStyleExperimentFlag);
 
     var featureSet = FeatureSet.fromEnableFlags2(
         sdkLanguageVersion: version, flags: experiments);

lib/src/front_end/piece_factory.dart

@@ -15,11 +15,13 @@ import 'piece_writer.dart';
 /// Record type for a destructured binary operator-like syntactic construct.
 typedef BinaryOperation = (AstNode left, Token operator, AstNode right);
 
-/// Many AST nodes are structurally similar and receive similar formatting.
+/// Utility methods for creating pieces that share formatting logic across
+/// multiple parts of the language.
 ///
-/// For example, imports and exports are mostly the same, with exports a subset
-/// of imports. Likewise, assert statements are formatted like function calls
-/// and argument lists.
+/// Many AST nodes are structurally similar and receive similar formatting. For
+/// example, imports and exports are mostly the same, with exports a subset of
+/// imports. Likewise, assert statements are formatted like function calls and
+/// argument lists.
 ///
 /// This mixin defines functions that represent a general construct that is
 /// formatted a certain way. The function builds up an appropriate set of

lib/src/front_end/piece_writer.dart

@@ -23,6 +23,7 @@ import '../source_code.dart';
 /// ExpressionStatement
 ///   BinaryExpression
 ///     SimpleIdentifier("a")
+///     Token("+")
 ///     SimpleIdentifier("b")
 /// ```
 ///
@@ -35,10 +36,10 @@ import '../source_code.dart';
 /// ```
 ///
 /// Note how the infix operator is attached to the preceding piece (which
-/// happens to just be text but could be more complex. Notice all that there
-/// is no piece for the expression statement and instead, the `;` is just
-/// appended to the last piece which is conceptually deeply nested inside the
-/// binary expression.
+/// happens to just be text but could be a more complex piece if the left
+/// operand was a nested expression). Notice also that there is no piece for
+/// the expression statement and instead, the `;` is just appended to the last
+/// piece which is conceptually deeply nested inside the binary expression.
 ///
 /// This class implements the "slippage" between these two representations. It
 /// has mutable state to allow incrementally building up pieces while traversing
@@ -143,7 +144,7 @@ class PieceWriter {
   /// Finishes writing and returns a [SourceCode] containing the final output
   /// and updated selection, if any.
   SourceCode finish() {
-    var formatter = Solver(_formatter);
+    var formatter = Solver(_formatter.pageWidth);
 
     var piece = pop();
 

lib/src/piece/import.dart

@@ -18,7 +18,9 @@ class ImportPiece extends Piece {
   /// If the directive has `if` configurations, this is them.
   final Piece? configurations;
 
-  /// If this directive is an import with an `as` clause, this is that clause.
+  /// The `as` clause for this directive.
+  ///
+  /// Null if this is not an import or it has no library prefix.
   final Piece? asClause;
 
   /// The piece for the `show` and/or `hide` combinators.

lib/src/piece/piece.dart

@@ -40,9 +40,10 @@ class TextPiece extends Piece {
   /// 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.
+  /// 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;
 
   @override

lib/src/testing/test_file.dart

@@ -48,6 +48,9 @@ class TestFile {
   factory TestFile._load(File file, String relativePath) {
     var lines = file.readAsLinesSync();
 
+    // Ignore comment lines.
+    lines.removeWhere((line) => line.startsWith('###'));
+
     // The first line may have a "|" to indicate the page width.
     var i = 0;
     int? pageWidth;

pubspec.yaml

@@ -1,6 +1,6 @@
 name: dart_style
 # Note: See tool/grind.dart for how to bump the version.
-version: 2.3.3-dev
+version: 2.3.4-wip
 description: >-
   Opinionated, automatic Dart source code formatter.
   Provides an API and a CLI tool.

test/utils.dart

@@ -5,6 +5,7 @@
 import 'dart:io';
 
 import 'package:dart_style/dart_style.dart';
+import 'package:dart_style/src/constants.dart';
 import 'package:dart_style/src/testing/test_file.dart';
 import 'package:path/path.dart' as p;
 import 'package:test/test.dart';
@@ -149,7 +150,8 @@ void _testFile(
             pageWidth: testFile.pageWidth,
             indent: formatTest.leadingIndent,
             fixes: [...?baseFixes, ...formatTest.fixes],
-            experimentFlags: useTallStyle ? const ['tall-style'] : null);
+            experimentFlags:
+                useTallStyle ? const [tallStyleExperimentFlag] : null);
 
         var actual = formatter.formatSource(formatTest.input);