From b1e98255bde6a0174bcde217b51db377fd418e5e Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Thu, 14 Sep 2023 14:57:24 -0700 Subject: [PATCH 1/6] Bump version to public 2.3.3. (#1270) I want to kick out a release before I start landing the new IR stuff on the main branch. --- CHANGELOG.md | 2 +- lib/src/cli/formatter_options.dart | 2 +- pubspec.yaml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e8932cd3..5d9f43b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,4 @@ -## 2.3.3-dev +## 2.3.3 * Always split enum declarations containing a line comment (#1254). * Fix regression in splitting type annotations with library prefixes (#1249). diff --git a/lib/src/cli/formatter_options.dart b/lib/src/cli/formatter_options.dart index d35e11d0..c6b80978 100644 --- a/lib/src/cli/formatter_options.dart +++ b/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 { diff --git a/pubspec.yaml b/pubspec.yaml index e281a08b..423f9c68 100644 --- a/pubspec.yaml +++ b/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.3 description: >- Opinionated, automatic Dart source code formatter. Provides an API and a CLI tool. From 473a21d6dcc76a5e55ce1afcc107e83ac00171bd Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Thu, 14 Sep 2023 15:25:07 -0700 Subject: [PATCH 2/6] Start building the new Piece internal representation and new style. (#1263) * Start building the new Piece internal representation and new style. This gets the foundation in place for the new IR, the new visitor that produces it, the new line splitter that consumes it, the new style, and new tests for it. It also adds support for formatting library, import, and export directives to make sure everything is wired up together and working. Existing formatting behavior is unchanged. You have to opt in to the new stuff by passing "tall-style" as an experiment flag. This PR doesn't support comments, but it does leave some unused code in a few places that will be used to handle comments in future PRs. It has many UnimplementedError throws. Those will get filled in as more of the language is implemented in the new style. There are also two new kinds of TODO comments: TODO(tall): ... These comments describe that work that needs to be done before the new style is fully working. TODO(perf): ... These describe potential areas for optimization. Once more of the language is supported with the new IR and I can run some larger benchmark samples through it, I can start exploring where the actual performance problems are. For now, I'm just leaving breadcrumbs. * Apply review feedback. * Apply review feedback. --- example/format.dart | 19 +- lib/src/back_end/code_writer.dart | 222 +++++ lib/src/back_end/solution.dart | 169 ++++ lib/src/back_end/solver.dart | 88 ++ lib/src/constants.dart | 12 + lib/src/dart_formatter.dart | 20 +- lib/src/debug.dart | 31 + lib/src/front_end/ast_node_visitor.dart | 847 ++++++++++++++++++ lib/src/front_end/piece_factory.dart | 152 ++++ lib/src/front_end/piece_writer.dart | 158 ++++ lib/src/piece/import.dart | 195 ++++ lib/src/piece/piece.dart | 82 ++ lib/src/piece/postfix.dart | 46 + lib/src/piece/sequence.dart | 56 ++ lib/src/testing/test_file.dart | 3 + pubspec.yaml | 1 + test/README.md | 77 ++ test/fix_test.dart | 18 +- ...atter_test.dart => short_format_test.dart} | 10 +- test/tall_format_test.dart | 18 + test/top_level/export.unit | 11 + test/top_level/import.unit | 30 + test/top_level/library.unit | 13 + test/top_level/show_hide.unit | 101 +++ test/utils.dart | 18 +- 25 files changed, 2369 insertions(+), 28 deletions(-) create mode 100644 lib/src/back_end/code_writer.dart create mode 100644 lib/src/back_end/solution.dart create mode 100644 lib/src/back_end/solver.dart create mode 100644 lib/src/front_end/ast_node_visitor.dart create mode 100644 lib/src/front_end/piece_factory.dart create mode 100644 lib/src/front_end/piece_writer.dart create mode 100644 lib/src/piece/import.dart create mode 100644 lib/src/piece/piece.dart create mode 100644 lib/src/piece/postfix.dart create mode 100644 lib/src/piece/sequence.dart create mode 100644 test/README.md rename test/{formatter_test.dart => short_format_test.dart} (94%) create mode 100644 test/tall_format_test.dart create mode 100644 test/top_level/export.unit create mode 100644 test/top_level/import.unit create mode 100644 test/top_level/library.unit create mode 100644 test/top_level/show_hide.unit diff --git a/example/format.dart b/example/format.dart index 6bc04d35..ef0defaa 100644 --- a/example/format.dart +++ b/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; @@ -17,22 +18,26 @@ void main(List args) { debug.traceLineWriter = true; debug.traceSplitter = true; debug.useAnsiColors = true; + debug.tracePieceBuilder = true; + debug.traceSolver = true; - formatStmt('a is int????;'); + formatUnit("import 'a.dart';", tall: true); } -void formatStmt(String source, [int pageWidth = 80]) { - runFormatter(source, pageWidth, isCompilationUnit: false); +void formatStmt(String source, {required bool tall, int pageWidth = 80}) { + runFormatter(source, pageWidth, tall: tall, isCompilationUnit: false); } -void formatUnit(String source, [int pageWidth = 80]) { - runFormatter(source, pageWidth, isCompilationUnit: true); +void formatUnit(String source, {required bool tall, int pageWidth = 80}) { + runFormatter(source, pageWidth, tall: tall, isCompilationUnit: true); } void runFormatter(String source, int pageWidth, - {required bool isCompilationUnit}) { + {required bool tall, required bool isCompilationUnit}) { try { - var formatter = DartFormatter(pageWidth: pageWidth); + var formatter = DartFormatter( + pageWidth: pageWidth, + experimentFlags: [if (tall) tallStyleExperimentFlag]); String result; if (isCompilationUnit) { diff --git a/lib/src/back_end/code_writer.dart b/lib/src/back_end/code_writer.dart new file mode 100644 index 00000000..99cd720c --- /dev/null +++ b/lib/src/back_end/code_writer.dart @@ -0,0 +1,222 @@ +// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file +// 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. +import '../constants.dart'; +import '../piece/piece.dart'; +import 'solution.dart'; + +/// 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; + + /// The state values for the pieces being written. + final PieceStateSet _pieceStates; + + /// Buffer for the code being written. + final StringBuffer _buffer = StringBuffer(); + + /// The cost of the currently chosen line splits. + int _cost = 0; + + /// The total number of characters of code that have overflowed the page + /// width so far. + int _overflow = 0; + + /// The number of characters in the line currently being written. + int _column = 0; + + /// Whether this solution has encountered a newline where none is allowed. + /// + /// If true, it means the solution is invalid. + bool _containsInvalidNewline = false; + + /// 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. + final List<_PieceOptions> _pieceOptions = [_PieceOptions(0, 0, true)]; + + /// The options for the current innermost piece being formatted. + _PieceOptions get _options => _pieceOptions.last; + + CodeWriter(this._pageWidth, this._pieceStates); + + /// Returns the finished code produced by formatting the tree of pieces and + /// the final score. + (String, Score) finish() { + _finishLine(); + return ( + _buffer.toString(), + Score(isValid: !_containsInvalidNewline, 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) _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], relative to the indentation of the surrounding piece. + /// + /// Replaces any previous indentation set by this piece. + void setIndent(int indent) { + _options.indent = _pieceOptions[_pieceOptions.length - 2].indent + indent; + } + + /// Increase the expression nesting of the current piece if [condition] is + /// `true`. + void nestIf(bool condition) { + if (!condition) return; + + _options.nesting += Indent.expression; + } + + /// Sets the number of spaces of expression nesting for code written by the + /// current piece to [nesting], relative to the nesting of the surrounding + /// piece. + /// + /// Replaces any previous nesting set by this piece. + void setNesting(int nesting) { + _options.nesting = + _pieceOptions[_pieceOptions.length - 2].nesting + nesting; + } + + /// Inserts a newline if [condition] is true. + /// + /// If [space] is `true` and [condition] is `false`, writes a space. + /// + /// If [indent] is given, sets the amount of block-level indentation for this + /// and all subsequent newlines to [indent]. + void splitIf(bool condition, {bool space = true, int? indent}) { + if (indent != null) setIndent(indent); + + if (condition) { + newline(); + } else if (space) { + this.space(); + } + } + + /// Writes a single space to the output. + void space() { + write(' '); + } + + /// 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(); + _buffer.writeln(); + if (blank) _buffer.writeln(); + + _column = _options.combinedIndentation; + _buffer.write(' ' * _column); + } + + /// Sets whether newlines are allowed to occur from this point on for the + /// current piece or any of its children. + void setAllowNewlines(bool allowed) { + _options.allowNewlines = allowed; + } + + /// 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. + if (_containsInvalidNewline) return; + + // TODO(tall): Sometimes, we'll want to reset the expression nesting for + // an inner piece, for when a block-like construct appears inside an + // expression. If it turns out that we don't actually need to handle indent + // and nesting separately here, then merge them into a single field. + _pieceOptions.add(_PieceOptions( + _options.indent, _options.nesting, _options.allowNewlines)); + + var state = _pieceStates.pieceState(piece); + + // TODO(tall): Support pieces with different split costs, and possibly + // different costs for each state value. + if (state != 0) _cost++; + + // TODO(perf): Memoize this. Might want to create a nested PieceWriter + // instead of passing in `this` so we can better control what state needs + // to be used as the key in the memoization table. + piece.format(this, state); + + _pieceOptions.removeLast(); + } + + /// Format [piece] if not null. + void formatOptional(Piece? piece) { + if (piece != null) format(piece); + } + + void _finishLine() { + // If the completed line is too long, track the overflow. + if (_column >= _pageWidth) { + _overflow += _column - _pageWidth; + } + } +} + +/// The mutable state local to a single piece being formatted. +class _PieceOptions { + /// 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 absolute number of spaces of indentation from wrapped expressions. + int nesting; + + /// The total number of spaces of indentation. + int get combinedIndentation => indent + nesting; + + /// Whether newlines are allowed to occur. + /// + /// If a newline is written while this is `false`, the entire solution is + /// considered invalid and gets discarded. + bool allowNewlines; + + _PieceOptions(this.indent, this.nesting, this.allowNewlines); +} diff --git a/lib/src/back_end/solution.dart b/lib/src/back_end/solution.dart new file mode 100644 index 00000000..8c256647 --- /dev/null +++ b/lib/src/back_end/solution.dart @@ -0,0 +1,169 @@ +// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file +// 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. +import '../piece/piece.dart'; +import 'code_writer.dart'; + +/// A possibly incomplete set of selected states for a set of pieces being +/// solved. +class PieceStateSet { + // TODO(perf): Looking up and expanding the set of chunk states was a + // performance bottleneck in the old line splitter. If that turns out to be + // true here, then consider a faster representation for this list and the + // subsequent map field. + /// The in-order flattened list of all pieces being solved. + /// + /// This doesn't include pieces like text that have only a single value since + /// there's nothing to solve for them. + final List _pieces; + + final Map _pieceStates; + + /// Creates a new [PieceStateSet] with no pieces set to any state (which + /// implicitly means they have state 0). + PieceStateSet(this._pieces) : _pieceStates = {}; + + PieceStateSet._(this._pieces, this._pieceStates); + + /// The state this solution selects for [piece]. + int pieceState(Piece piece) => _pieceStates[piece] ?? 0; + + /// Gets the first piece that doesn't have a state selected yet, or `null` if + /// all pieces have selected states. + Piece? firstUnsolved() { + // TODO(perf): This may be slow. Could store the index at construction time. + for (var piece in _pieces) { + if (!_pieceStates.containsKey(piece)) { + return piece; + } + } + + return null; + } + + /// Creates a clone of this state with [piece] bound to [state]. + PieceStateSet cloneWith(Piece piece, int state) { + return PieceStateSet._(_pieces, {..._pieceStates, piece: state}); + } + + @override + String toString() { + return _pieces.map((piece) { + var state = _pieceStates[piece]; + var stateLabel = state == null ? '?' : '$state'; + return '$piece:$stateLabel'; + }).join(' '); + } +} + +/// A single possible line splitting solution. +/// +/// Stores the states that each piece is set to and the resulting formatted +/// code and its cost. +class Solution implements Comparable { + /// The states the pieces have been set to in this solution. + final PieceStateSet _state; + + /// The formatted code. + final String text; + + /// The score resulting from the selected piece states. + final Score score; + + factory Solution(Piece root, int pageWidth, PieceStateSet state) { + var writer = CodeWriter(pageWidth, state); + writer.format(root); + var (text, score) = writer.finish(); + return Solution._(state, text, score); + } + + Solution._(this._state, this.text, this.score); + + /// When called on a [Solution] with some unselected piece states, chooses a + /// piece and yields further solutions for each state that piece can have. + List expand(Piece root, int pageWidth) { + var piece = _state.firstUnsolved(); + if (piece == null) return const []; + + var result = []; + for (var i = 0; i < piece.stateCount; i++) { + var solution = Solution(root, pageWidth, _state.cloneWith(piece, i)); + result.add(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. + @override + int compareTo(Solution other) { + var scoreComparison = score.compareTo(other.score); + if (scoreComparison != 0) return scoreComparison; + + // Should be solving the same set of pieces. + assert(_state._pieces.length == other._state._pieces.length); + + // If all else is equal, prefer lower states in earlier pieces. + // TODO(tall): This might not be needed once piece scoring is more + // sophisticated. + for (var i = 0; i < _state._pieces.length; i++) { + var piece = _state._pieces[i]; + var thisState = _state.pieceState(piece); + var otherState = other._state.pieceState(piece); + if (thisState != otherState) return thisState.compareTo(otherState); + } + + return 0; + } + + @override + String toString() => '$score $_state'; +} + +class Score implements Comparable { + // TODO(tall): Should this actually be part of scoring? Do we want to use + // validity to determine how we order solutions to explore? + /// Whether this score is for a valid solution or not. + /// + /// An invalid solution is one where a hard newline appears in a context + /// where splitting isn't allowed. This is considered worse than any other + /// solution. + final bool isValid; + + /// The number of characters that do not fit inside the page width. + final int overflow; + + /// The amount of penalties applied based on the chosen line splits. + final int cost; + + Score({required this.isValid, required this.overflow, required this.cost}); + + @override + int compareTo(Score other) { + // All invalid solutions are equal. + if (!isValid && !other.isValid) return 0; + + // We are looking for *lower* costs and overflows, so an invalid score is + // considered higher or after all others. + if (!isValid) return 1; + if (!other.isValid) return -1; + + // Overflow is always penalized more than line splitting cost. + if (overflow != other.overflow) return overflow.compareTo(other.overflow); + + return cost.compareTo(other.cost); + } + + @override + String toString() { + return [ + '\$$cost', + if (overflow > 0) '($overflow over)', + if (!isValid) '(invalid)' + ].join(' '); + } +} diff --git a/lib/src/back_end/solver.dart b/lib/src/back_end/solver.dart new file mode 100644 index 00000000..32873f07 --- /dev/null +++ b/lib/src/back_end/solver.dart @@ -0,0 +1,88 @@ +// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file +// 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. +import 'package:collection/collection.dart'; + +import '../debug.dart' as debug; +import '../piece/piece.dart'; +import 'solution.dart'; + +/// Selects states for each piece in a tree of pieces to find the best set of +/// line splits that minimizes overflow characters and line splitting costs. +/// +/// This problem is combinatorial for the number of pieces and each of their +/// possible states, so it isn't feasible to brute force. There are a few +/// techniques we use to avoid that: +/// +/// - All pieces default to being in state zero. Every piece is implemented +/// such that that state has no line splits and zero cost. Thus, it tries +/// solutions with a minimum number of line splits first. +/// +/// - Solutions are explored in priority order. We explore solutions with the +/// fewest overflow characters and the lowest cost (in that order) first. +/// This was, as soon as we find a solution with no overflow characters, we +/// know it will be the best solution and can stop. +// TODO(perf): At some point, we probably also want to do memoization of +// previously formatted Piece subtrees. +class Solver { + final int _pageWidth; + + final PriorityQueue _queue = PriorityQueue(); + + Solver(this._pageWidth); + + /// Finds the best set of line splits for [piece] and returns the resulting + /// formatted code. + String format(Piece piece) { + // Collect all of the pieces with states that can be selected. + var unsolvedPieces = []; + + void traverse(Piece piece) { + // We don't need to worry about selecting pieces that have only one state. + if (piece.stateCount > 1) unsolvedPieces.add(piece); + piece.forEachChild(traverse); + } + + traverse(piece); + + var solution = _solve(piece, unsolvedPieces); + return solution.text; + } + + /// Finds the best solution for the piece tree starting at [root] with + /// selectable [pieces]. + Solution _solve(Piece root, List pieces) { + var solution = Solution(root, _pageWidth, PieceStateSet(pieces)); + _queue.add(solution); + + // The lowest cost solution found so far that does overflow. + var best = solution; + + while (_queue.isNotEmpty) { + var solution = _queue.removeFirst(); + + if (debug.traceSolver) { + debug.log(debug.bold(solution)); + debug.log(solution.text); + debug.log(''); + } + + // Since we process the solutions from lowest cost up, as soon as we find + // a valid one that fits, it's the best. + if (solution.score.isValid) { + if (solution.score.overflow == 0) return solution; + + if (solution.score.overflow < best.score.overflow) best = solution; + } + + // Otherwise, try to expand the solution to explore different splitting + // options. + for (var expanded in solution.expand(root, _pageWidth)) { + _queue.add(expanded); + } + } + + // If we didn't find a solution without overflow, pick the least bad one. + return best; + } +} diff --git a/lib/src/constants.dart b/lib/src/constants.dart index f505182d..2152e211 100644 --- a/lib/src/constants.dart +++ b/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 { @@ -67,4 +76,7 @@ class Indent { /// The ":" on a wrapped constructor initialization list. static const constructorInitializer = 4; + + /// A split name in a show or hide combinator. + static const combinatorName = 8; } diff --git a/lib/src/dart_formatter.dart b/lib/src/dart_formatter.dart index 19c951b2..fc349110 100644 --- a/lib/src/dart_formatter.dart +++ b/lib/src/dart_formatter.dart @@ -15,7 +15,9 @@ 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'; import 'source_visitor.dart'; import 'string_compare.dart' as string_compare; @@ -175,8 +177,15 @@ class DartFormatter { // Format it. var lineInfo = parseResult.lineInfo; - var visitor = SourceVisitor(this, lineInfo, unitSourceCode); - var output = visitor.run(node); + + SourceCode output; + if (experimentFlags.contains(tallStyleExperimentFlag)) { + var visitor = AstNodeVisitor(this, lineInfo, unitSourceCode); + output = visitor.run(node); + } else { + var visitor = SourceVisitor(this, lineInfo, unitSourceCode); + output = visitor.run(node); + } // Sanity check that only whitespace was changed if that's all we expect. if (fixes.isEmpty && @@ -211,8 +220,13 @@ class DartFormatter { ParseStringResult _parse(String source, String? uri, {required bool patterns}) { var version = patterns ? Version(3, 0, 0) : Version(2, 19, 0); + + // Don't pass the formatter's own experiment flag to the parser. + var experiments = experimentFlags.toList(); + experiments.remove(tallStyleExperimentFlag); + var featureSet = FeatureSet.fromEnableFlags2( - sdkLanguageVersion: version, flags: experimentFlags); + sdkLanguageVersion: version, flags: experiments); return parseString( content: source, diff --git a/lib/src/debug.dart b/lib/src/debug.dart index ea151244..f345d0c3 100644 --- a/lib/src/debug.dart +++ b/lib/src/debug.dart @@ -7,6 +7,7 @@ import 'dart:math' as math; import 'chunk.dart'; import 'line_splitting/rule_set.dart'; +import 'piece/piece.dart'; /// Set this to `true` to turn on diagnostic output while building chunks. bool traceChunkBuilder = false; @@ -17,6 +18,12 @@ bool traceLineWriter = false; /// Set this to `true` to turn on diagnostic output while line splitting. bool traceSplitter = false; +/// Set this to `true` to turn on diagnostic output while building pieces. +bool tracePieceBuilder = false; + +/// Set this to `true` to turn on diagnostic output while solving pieces. +bool traceSolver = false; + bool useAnsiColors = false; const unicodeSection = '\u00a7'; @@ -256,4 +263,28 @@ void dumpLines(List chunks, SplitSet splits) { log(buffer); } +/// Build a string representation of the [piece] tree. +String pieceTree(Piece piece) { + var buffer = StringBuffer(); + + void traverse(Piece piece) { + buffer.write(piece); + + if (piece is! TextPiece) { + var children = []; + piece.forEachChild(children.add); + + buffer.write('('); + for (var child in children) { + if (child != children.first) buffer.write(' '); + traverse(child); + } + buffer.write(')'); + } + } + + traverse(piece); + return buffer.toString(); +} + String _color(String ansiEscape) => useAnsiColors ? ansiEscape : ''; diff --git a/lib/src/front_end/ast_node_visitor.dart b/lib/src/front_end/ast_node_visitor.dart new file mode 100644 index 00000000..13a98498 --- /dev/null +++ b/lib/src/front_end/ast_node_visitor.dart @@ -0,0 +1,847 @@ +// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file +// 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. +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:analyzer/source/line_info.dart'; + +import '../dart_formatter.dart'; +import '../piece/sequence.dart'; +import '../source_code.dart'; +import 'piece_factory.dart'; +import 'piece_writer.dart'; + +/// Visits every token of the AST and produces a tree of [Piece]s that +/// corresponds to it and contains every token and comment in the original +/// source. +/// +/// To avoid this class becoming a monolith, functionality is divided into a +/// couple of mixins, one for each area of functionality. This class then +/// contains only shared state and the visitor methods for the AST. +class AstNodeVisitor extends ThrowingAstVisitor with PieceFactory { + /// Cached line info for calculating blank lines. + final LineInfo lineInfo; + + @override + final PieceWriter writer; + + /// Initialize a newly created visitor to write source code representing + /// the visited nodes to the given [writer]. + AstNodeVisitor(DartFormatter formatter, this.lineInfo, SourceCode source) + : writer = PieceWriter(formatter, source); + + /// Runs the visitor on [node], formatting its contents. + /// + /// Returns a [SourceCode] containing the resulting formatted source and + /// updated selection, if any. + /// + /// This is the only method that should be called externally. Everything else + /// is effectively private. + SourceCode run(AstNode node) { + visit(node); + + // TODO(tall): Output trailing comments. + if (node.endToken.next!.precedingComments != null) { + throw UnimplementedError(); + } + + // Finish writing and return the complete result. + return writer.finish(); + } + + @override + void visitAdjacentStrings(AdjacentStrings node) { + throw UnimplementedError(); + } + + @override + void visitAnnotation(Annotation node) { + throw UnimplementedError(); + } + + @override + void visitArgumentList(ArgumentList node, {bool nestExpression = true}) { + throw UnimplementedError(); + } + + @override + void visitAsExpression(AsExpression node) { + throw UnimplementedError(); + } + + @override + void visitAssertInitializer(AssertInitializer node) { + throw UnimplementedError(); + } + + @override + void visitAssertStatement(AssertStatement node) { + throw UnimplementedError(); + } + + @override + void visitAssignedVariablePattern(AssignedVariablePattern node) { + throw UnimplementedError(); + } + + @override + void visitAssignmentExpression(AssignmentExpression node) { + throw UnimplementedError(); + } + + @override + void visitAwaitExpression(AwaitExpression node) { + throw UnimplementedError(); + } + + @override + void visitBinaryExpression(BinaryExpression node) { + throw UnimplementedError(); + } + + @override + void visitBlock(Block node) { + throw UnimplementedError(); + } + + @override + void visitBlockFunctionBody(BlockFunctionBody node) { + throw UnimplementedError(); + } + + @override + void visitBooleanLiteral(BooleanLiteral node) { + throw UnimplementedError(); + } + + @override + void visitBreakStatement(BreakStatement node) { + throw UnimplementedError(); + } + + @override + void visitCascadeExpression(CascadeExpression node) { + throw UnimplementedError(); + } + + @override + void visitCastPattern(CastPattern node) { + throw UnimplementedError(); + } + + @override + void visitCatchClause(CatchClause node) { + throw UnimplementedError(); + } + + @override + void visitCatchClauseParameter(CatchClauseParameter node) { + throw UnimplementedError(); + } + + @override + void visitClassDeclaration(ClassDeclaration node) { + throw UnimplementedError(); + } + + @override + void visitClassTypeAlias(ClassTypeAlias node) { + throw UnimplementedError(); + } + + @override + void visitComment(Comment node) { + throw UnimplementedError(); + } + + @override + void visitCommentReference(CommentReference node) { + throw UnimplementedError(); + } + + @override + void visitCompilationUnit(CompilationUnit node) { + var sequence = SequencePiece(); + + // Put a blank line between the library tag and the other directives. + Iterable directives = node.directives; + if (directives.isNotEmpty && directives.first is LibraryDirective) { + addToSequence(sequence, directives.first); + sequence.addBlank(); + directives = directives.skip(1); + } + + for (var directive in directives) { + addToSequence(sequence, directive); + } + + // TODO(tall): Handle top level declarations. + if (node.declarations.isNotEmpty) throw UnimplementedError(); + + writer.push(sequence); + } + + @override + void visitConditionalExpression(ConditionalExpression node) { + throw UnimplementedError(); + } + + @override + void visitConfiguration(Configuration node) { + throw UnimplementedError(); + } + + @override + void visitConstantPattern(ConstantPattern node) { + throw UnimplementedError(); + } + + @override + void visitConstructorDeclaration(ConstructorDeclaration node) { + throw UnimplementedError(); + } + + @override + void visitConstructorFieldInitializer(ConstructorFieldInitializer node) { + throw UnimplementedError(); + } + + @override + void visitConstructorName(ConstructorName node) { + throw UnimplementedError(); + } + + @override + void visitContinueStatement(ContinueStatement node) { + throw UnimplementedError(); + } + + @override + void visitDeclaredIdentifier(DeclaredIdentifier node) { + throw UnimplementedError(); + } + + @override + void visitDeclaredVariablePattern(DeclaredVariablePattern node) { + throw UnimplementedError(); + } + + @override + void visitDefaultFormalParameter(DefaultFormalParameter node) { + throw UnimplementedError(); + } + + @override + void visitDoStatement(DoStatement node) { + throw UnimplementedError(); + } + + @override + void visitDottedName(DottedName node) { + throw UnimplementedError(); + } + + @override + void visitDoubleLiteral(DoubleLiteral node) { + throw UnimplementedError(); + } + + @override + void visitEmptyFunctionBody(EmptyFunctionBody node) { + throw UnimplementedError(); + } + + @override + void visitEmptyStatement(EmptyStatement node) { + throw UnimplementedError(); + } + + @override + void visitEnumConstantDeclaration(EnumConstantDeclaration node) { + throw UnimplementedError(); + } + + @override + void visitEnumDeclaration(EnumDeclaration node) { + throw UnimplementedError(); + } + + @override + void visitExportDirective(ExportDirective node) { + // TODO(tall): Format configurations. + if (node.configurations.isNotEmpty) throw UnimplementedError(); + + createImport(node, node.exportKeyword); + } + + @override + void visitExpressionFunctionBody(ExpressionFunctionBody node) { + throw UnimplementedError(); + } + + @override + void visitExpressionStatement(ExpressionStatement node) { + throw UnimplementedError(); + } + + @override + void visitExtendsClause(ExtendsClause node) { + throw UnimplementedError(); + } + + @override + void visitExtensionDeclaration(ExtensionDeclaration node) { + throw UnimplementedError(); + } + + @override + void visitFieldDeclaration(FieldDeclaration node) { + throw UnimplementedError(); + } + + @override + void visitFieldFormalParameter(FieldFormalParameter node) { + throw UnimplementedError(); + } + + @override + void visitFormalParameterList(FormalParameterList node) { + throw UnimplementedError(); + } + + @override + void visitForElement(ForElement node) { + throw UnimplementedError(); + } + + @override + void visitForStatement(ForStatement node) { + throw UnimplementedError(); + } + + @override + void visitForEachPartsWithDeclaration(ForEachPartsWithDeclaration node) { + throw UnimplementedError(); + } + + @override + void visitForEachPartsWithIdentifier(ForEachPartsWithIdentifier node) { + throw UnimplementedError(); + } + + @override + void visitForEachPartsWithPattern(ForEachPartsWithPattern node) { + throw UnimplementedError(); + } + + @override + void visitForPartsWithDeclarations(ForPartsWithDeclarations node) { + throw UnimplementedError(); + } + + @override + void visitForPartsWithExpression(ForPartsWithExpression node) { + throw UnimplementedError(); + } + + @override + void visitForPartsWithPattern(ForPartsWithPattern node) { + throw UnimplementedError(); + } + + @override + void visitFunctionDeclaration(FunctionDeclaration node) { + throw UnimplementedError(); + } + + @override + void visitFunctionDeclarationStatement(FunctionDeclarationStatement node) { + throw UnimplementedError(); + } + + @override + void visitFunctionExpression(FunctionExpression node) { + throw UnimplementedError(); + } + + @override + void visitFunctionExpressionInvocation(FunctionExpressionInvocation node) { + throw UnimplementedError(); + } + + @override + void visitFunctionReference(FunctionReference node) { + throw UnimplementedError(); + } + + @override + void visitFunctionTypeAlias(FunctionTypeAlias node) { + throw UnimplementedError(); + } + + @override + void visitFunctionTypedFormalParameter(FunctionTypedFormalParameter node) { + throw UnimplementedError(); + } + + @override + void visitGenericFunctionType(GenericFunctionType node) { + throw UnimplementedError(); + } + + @override + void visitGenericTypeAlias(GenericTypeAlias node) { + throw UnimplementedError(); + } + + @override + void visitHideCombinator(HideCombinator node) { + throw UnimplementedError(); + } + + @override + void visitIfElement(IfElement node) { + throw UnimplementedError(); + } + + @override + void visitIfStatement(IfStatement node) { + throw UnimplementedError(); + } + + @override + void visitImplementsClause(ImplementsClause node) { + throw UnimplementedError(); + } + + @override + void visitImportDirective(ImportDirective node) { + // TODO(tall): Format configurations. + if (node.configurations.isNotEmpty) throw UnimplementedError(); + + createImport(node, node.importKeyword, + deferredKeyword: node.deferredKeyword, + asKeyword: node.asKeyword, + prefix: node.prefix); + } + + @override + void visitIndexExpression(IndexExpression node) { + throw UnimplementedError(); + } + + @override + void visitInstanceCreationExpression(InstanceCreationExpression node) { + throw UnimplementedError(); + } + + @override + void visitIntegerLiteral(IntegerLiteral node) { + throw UnimplementedError(); + } + + @override + void visitInterpolationExpression(InterpolationExpression node) { + throw UnimplementedError(); + } + + @override + void visitInterpolationString(InterpolationString node) { + throw UnimplementedError(); + } + + @override + void visitIsExpression(IsExpression node) { + throw UnimplementedError(); + } + + @override + void visitLabel(Label node) { + throw UnimplementedError(); + } + + @override + void visitLabeledStatement(LabeledStatement node) { + throw UnimplementedError(); + } + + @override + void visitLibraryDirective(LibraryDirective node) { + createDirectiveMetadata(node); + token(node.libraryKeyword); + visit(node.name2, before: writer.space); + token(node.semicolon); + } + + @override + void visitLibraryIdentifier(LibraryIdentifier node) { + createDotted(node.components); + } + + @override + void visitListLiteral(ListLiteral node) { + throw UnimplementedError(); + } + + @override + void visitListPattern(ListPattern node) { + throw UnimplementedError(); + } + + @override + void visitLogicalAndPattern(LogicalAndPattern node) { + throw UnimplementedError(); + } + + @override + void visitLogicalOrPattern(LogicalOrPattern node) { + throw UnimplementedError(); + } + + @override + void visitMapLiteralEntry(MapLiteralEntry node) { + throw UnimplementedError(); + } + + @override + void visitMapPattern(MapPattern node) { + throw UnimplementedError(); + } + + @override + void visitMapPatternEntry(MapPatternEntry node) { + throw UnimplementedError(); + } + + @override + void visitMethodDeclaration(MethodDeclaration node) { + throw UnimplementedError(); + } + + @override + void visitMethodInvocation(MethodInvocation node) { + throw UnimplementedError(); + } + + @override + void visitMixinDeclaration(MixinDeclaration node) { + throw UnimplementedError(); + } + + @override + void visitNamedExpression(NamedExpression node) { + throw UnimplementedError(); + } + + @override + void visitNamedType(NamedType node) { + throw UnimplementedError(); + } + + @override + void visitNativeClause(NativeClause node) { + throw UnimplementedError(); + } + + @override + void visitNativeFunctionBody(NativeFunctionBody node) { + throw UnimplementedError(); + } + + @override + void visitNullAssertPattern(NullAssertPattern node) { + throw UnimplementedError(); + } + + @override + void visitNullCheckPattern(NullCheckPattern node) { + throw UnimplementedError(); + } + + @override + void visitNullLiteral(NullLiteral node) { + throw UnimplementedError(); + } + + @override + void visitObjectPattern(ObjectPattern node) { + throw UnimplementedError(); + } + + @override + void visitOnClause(OnClause node) { + throw UnimplementedError(); + } + + @override + void visitParenthesizedExpression(ParenthesizedExpression node) { + throw UnimplementedError(); + } + + @override + void visitParenthesizedPattern(ParenthesizedPattern node) { + throw UnimplementedError(); + } + + @override + void visitPartDirective(PartDirective node) { + throw UnimplementedError(); + } + + @override + void visitPartOfDirective(PartOfDirective node) { + throw UnimplementedError(); + } + + @override + void visitPatternAssignment(PatternAssignment node) { + throw UnimplementedError(); + } + + @override + void visitPatternField(PatternField node) { + throw UnimplementedError(); + } + + @override + void visitPatternVariableDeclaration(PatternVariableDeclaration node) { + throw UnimplementedError(); + } + + @override + void visitPatternVariableDeclarationStatement( + PatternVariableDeclarationStatement node) { + throw UnimplementedError(); + } + + @override + void visitPostfixExpression(PostfixExpression node) { + throw UnimplementedError(); + } + + @override + void visitPrefixedIdentifier(PrefixedIdentifier node) { + throw UnimplementedError(); + } + + @override + void visitPrefixExpression(PrefixExpression node) { + throw UnimplementedError(); + } + + @override + void visitPropertyAccess(PropertyAccess node) { + throw UnimplementedError(); + } + + @override + void visitRedirectingConstructorInvocation( + RedirectingConstructorInvocation node) { + throw UnimplementedError(); + } + + @override + void visitRecordLiteral(RecordLiteral node) { + throw UnimplementedError(); + } + + @override + void visitRecordPattern(RecordPattern node) { + throw UnimplementedError(); + } + + @override + void visitRecordTypeAnnotation(RecordTypeAnnotation node) { + throw UnimplementedError(); + } + + @override + void visitRecordTypeAnnotationNamedField( + RecordTypeAnnotationNamedField node) { + throw UnimplementedError(); + } + + @override + void visitRecordTypeAnnotationPositionalField( + RecordTypeAnnotationPositionalField node) { + throw UnimplementedError(); + } + + @override + void visitRelationalPattern(RelationalPattern node) { + throw UnimplementedError(); + } + + @override + void visitRethrowExpression(RethrowExpression node) { + throw UnimplementedError(); + } + + @override + void visitRestPatternElement(RestPatternElement node) { + throw UnimplementedError(); + } + + @override + void visitReturnStatement(ReturnStatement node) { + throw UnimplementedError(); + } + + @override + void visitScriptTag(ScriptTag node) { + throw UnimplementedError(); + } + + @override + void visitSetOrMapLiteral(SetOrMapLiteral node) { + throw UnimplementedError(); + } + + @override + void visitShowCombinator(ShowCombinator node) { + throw UnimplementedError(); + } + + @override + void visitSimpleFormalParameter(SimpleFormalParameter node) { + throw UnimplementedError(); + } + + @override + void visitSimpleIdentifier(SimpleIdentifier node) { + token(node.token); + } + + @override + void visitSimpleStringLiteral(SimpleStringLiteral node) { + token(node.literal); + } + + @override + void visitSpreadElement(SpreadElement node) { + throw UnimplementedError(); + } + + @override + void visitStringInterpolation(StringInterpolation node) { + throw UnimplementedError(); + } + + @override + void visitSuperConstructorInvocation(SuperConstructorInvocation node) { + throw UnimplementedError(); + } + + @override + void visitSuperExpression(SuperExpression node) { + throw UnimplementedError(); + } + + @override + void visitSuperFormalParameter(SuperFormalParameter node) { + throw UnimplementedError(); + } + + @override + void visitSwitchExpression(SwitchExpression node) { + throw UnimplementedError(); + } + + @override + void visitSwitchExpressionCase(SwitchExpressionCase node) { + throw UnimplementedError(); + } + + @override + void visitSwitchStatement(SwitchStatement node) { + throw UnimplementedError(); + } + + @override + void visitSymbolLiteral(SymbolLiteral node) { + throw UnimplementedError(); + } + + @override + void visitThisExpression(ThisExpression node) { + throw UnimplementedError(); + } + + @override + void visitThrowExpression(ThrowExpression node) { + throw UnimplementedError(); + } + + @override + void visitTopLevelVariableDeclaration(TopLevelVariableDeclaration node) { + throw UnimplementedError(); + } + + @override + void visitTryStatement(TryStatement node) { + throw UnimplementedError(); + } + + @override + void visitTypeArgumentList(TypeArgumentList node) { + throw UnimplementedError(); + } + + @override + void visitTypeParameter(TypeParameter node) { + throw UnimplementedError(); + } + + @override + void visitTypeParameterList(TypeParameterList node) { + throw UnimplementedError(); + } + + @override + void visitVariableDeclaration(VariableDeclaration node) { + throw UnimplementedError(); + } + + @override + void visitVariableDeclarationList(VariableDeclarationList node) { + throw UnimplementedError(); + } + + @override + void visitVariableDeclarationStatement(VariableDeclarationStatement node) { + throw UnimplementedError(); + } + + @override + void visitWhileStatement(WhileStatement node) { + throw UnimplementedError(); + } + + @override + void visitWildcardPattern(WildcardPattern node) { + throw UnimplementedError(); + } + + @override + void visitWithClause(WithClause node) { + throw UnimplementedError(); + } + + @override + void visitYieldStatement(YieldStatement node) { + throw UnimplementedError(); + } + + /// If [node] is not `null`, then visit it. + /// + /// Invokes [before] before visiting [node], and [after] afterwards, but only + /// if [node] is present. + @override + void visit(AstNode? node, {void Function()? before, void Function()? after}) { + if (node == null) return; + + if (before != null) before(); + node.accept(this); + if (after != null) after(); + } +} diff --git a/lib/src/front_end/piece_factory.dart b/lib/src/front_end/piece_factory.dart new file mode 100644 index 00000000..95e5c0e9 --- /dev/null +++ b/lib/src/front_end/piece_factory.dart @@ -0,0 +1,152 @@ +// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file +// 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. +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/token.dart'; + +import '../piece/import.dart'; +import '../piece/piece.dart'; +import '../piece/postfix.dart'; +import '../piece/sequence.dart'; +import 'piece_writer.dart'; + +/// Utility methods for creating pieces that share formatting logic across +/// multiple parts of the language. +/// +/// 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 +/// [Piece]s given the various AST subcomponents passed in as parameters. The +/// main [AstNodeVisitor] class then calls those for all of the AST nodes that +/// should receive that similar formatting. +/// +/// These are all void functions because they generally push their result into +/// the [PieceWriter]. +/// +/// Naming these functions can be hard. For example, there isn't an obvious +/// word for "import or export directive" or "named thing with argument list". +/// To avoid that, we pick one concrete construct formatted by the function, +/// usually the most common, and name it after that, as in [createImport()]. +mixin PieceFactory { + PieceWriter get writer; + + void visit(AstNode? node, {void Function()? before, void Function()? after}); + + /// Adds [node] to [sequence], handling blank lines around it. + void addToSequence(SequencePiece sequence, AstNode node) { + visit(node); + sequence.add(writer.pop()); + writer.split(); + } + + /// Creates metadata annotations for a directive. + /// + /// Always forces the annotations to be on a previous line. + void createDirectiveMetadata(Directive directive) { + // TODO(tall): Implement. See SourceVisitor._visitDirectiveMetadata(). + if (directive.metadata.isNotEmpty) throw UnimplementedError(); + } + + /// Creates a dotted or qualified identifier. + void createDotted(NodeList components) { + for (var component in components) { + // Write the preceding ".". + if (component != components.first) { + token(component.beginToken.previous); + } + + visit(component); + } + } + + /// Creates an [ImportPiece] for an import or export directive. + void createImport(NamespaceDirective directive, Token keyword, + {Token? deferredKeyword, Token? asKeyword, SimpleIdentifier? prefix}) { + createDirectiveMetadata(directive); + token(keyword); + writer.space(); + visit(directive.uri); + var directivePiece = writer.pop(); + + Piece? asClause; + if (asKeyword != null) { + writer.split(); + token(deferredKeyword, after: writer.space); + token(asKeyword); + writer.space(); + visit(prefix); + asClause = PostfixPiece([writer.pop()]); + } + + var combinators = []; + for (var combinatorNode in directive.combinators) { + writer.split(); + token(combinatorNode.keyword); + var combinator = ImportCombinator(writer.pop()); + combinators.add(combinator); + + switch (combinatorNode) { + case HideCombinator(hiddenNames: var names): + case ShowCombinator(shownNames: var names): + for (var name in names) { + writer.split(); + token(name.token); + commaAfter(name); + combinator.names.add(writer.pop()); + } + default: + throw StateError('Unknown combinator type $combinatorNode.'); + } + } + + var combinator = switch (combinators.length) { + 0 => null, + 1 => OneCombinatorPiece(combinators[0]), + 2 => TwoCombinatorPiece(combinators), + _ => throw StateError('Directives can only have up to two combinators.'), + }; + + token(directive.semicolon); + + writer.push(ImportPiece(directivePiece, asClause, combinator)); + } + + /// Emit [token], along with any comments and formatted whitespace that comes + /// before it. + /// + /// Does nothing if [token] is `null`. If [before] is given, it will be + /// executed before the token is outout. Likewise, [after] will be called + /// after the token is output. + void token(Token? token, {void Function()? before, void Function()? after}) { + if (token == null) return; + + // TODO(tall): Write comments before the token. + + if (before != null) before(); + writeLexeme(token.lexeme); + if (after != null) after(); + } + + /// Writes the raw [lexeme] to the current text piece. + void writeLexeme(String lexeme) { + // TODO(tall): Preserve selection. + writer.write(lexeme); + } + + /// Writes a comma after [node], if there is one. + void commaAfter(AstNode node, {bool trailing = false}) { + var nextToken = node.endToken.next!; + if (nextToken.lexeme == ',') { + token(nextToken); + } else if (trailing) { + // If there isn't a comma there, it must be a place where a trailing + // comma can appear, so synthesize it. During formatting, we will decide + // whether to include it. + writeLexeme(','); + } + } +} diff --git a/lib/src/front_end/piece_writer.dart b/lib/src/front_end/piece_writer.dart new file mode 100644 index 00000000..2141384b --- /dev/null +++ b/lib/src/front_end/piece_writer.dart @@ -0,0 +1,158 @@ +// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file +// 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. + +import '../back_end/solver.dart'; +import '../dart_formatter.dart'; +import '../debug.dart' as debug; +import '../piece/piece.dart'; +import '../source_code.dart'; + +/// Incrementally builds [Piece]s while visiting AST nodes. +/// +/// The nodes in the piece tree don't always map precisely to AST nodes. For +/// example, in: +/// +/// ``` +/// a + b; +/// ``` +/// +/// The AST structure is like: +/// +/// ``` +/// ExpressionStatement +/// BinaryExpression +/// SimpleIdentifier("a") +/// Token("+") +/// SimpleIdentifier("b") +/// ``` +/// +/// But the resulting piece tree looks like: +/// +/// ``` +/// Infix +/// TextPiece("a +") +/// TextPiece("b;") +/// ``` +/// +/// Note how the infix operator is attached to the preceding piece (which +/// 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 +/// the source AST nodes. +class PieceWriter { + final DartFormatter _formatter; + + final SourceCode _source; + + /// The current [TextPiece] being written to or `null` if no text piece has + /// been started yet. + TextPiece? get currentText => _currentText; + TextPiece? _currentText; + + /// The most recently pushed piece, waiting to be taken by some surrounding + /// piece. + /// + /// Since we traverse the AST in syntax order and pop built pieces on the way + /// back up, the "stack" of completed pieces is only ever one deep at the + /// most, so we model it with just a single field. + Piece? _pushed; + + /// Whether we should write a space before the next text that is written. + bool _pendingSpace = false; + + /// Whether we should create a new [TextPiece] the next time text is written. + bool _pendingSplit = false; + + PieceWriter(this._formatter, this._source); + + /// Gives the builder a newly completed [piece], to be taken by a later call + /// to [pop] from some surrounding piece. + void push(Piece piece) { + // Should never push more than one piece. + assert(_pushed == null); + + _pushed = piece; + } + + /// Captures the most recently created complete [Piece]. + /// + /// If the most recent operation was [push], then this returns the piece given + /// by that call. Otherwise, returns the piece created by the preceding calls + /// to [write] since the last split. + Piece pop() { + if (_pushed case var piece?) { + _pushed = null; + return piece; + } + + return _currentText!; + } + + /// Ends the current text piece and (lazily) begins a new one. + /// + /// The previous text piece should already be taken. + void split() { + // Shouldn't have redundant splits. + assert(!_pendingSplit); + + _pendingSplit = true; + } + + /// Writes a space to the current [TextPiece]. + void space() { + _pendingSpace = true; + } + + /// Writes [text] raw text to the current innermost [TextPiece]. Starts a new + /// one if needed. + /// + /// If [text] internally contains a newline, then [containsNewline] should + /// be `true`. + void write(String text, + {bool containsNewline = false, bool following = false}) { + var textPiece = _currentText; + + // Create a new text piece if we don't have one or we are after a split. + // Ignore the split if the text is deliberately intended to follow the + // current text. + if (textPiece == null || _pendingSplit && !following) { + textPiece = _currentText = TextPiece(); + } else if (_pendingSpace) { + textPiece.append(' '); + } + + textPiece.append(text, containsNewline: containsNewline); + + _pendingSpace = false; + if (!following) _pendingSplit = false; + } + + /// Finishes writing and returns a [SourceCode] containing the final output + /// and updated selection, if any. + SourceCode finish() { + var formatter = Solver(_formatter.pageWidth); + + var piece = pop(); + + if (debug.tracePieceBuilder) { + print(debug.pieceTree(piece)); + } + + var result = formatter.format(piece); + + // Be a good citizen, end with a newline. + if (_source.isCompilationUnit) result += _formatter.lineEnding!; + + return SourceCode(result, + uri: _source.uri, + isCompilationUnit: _source.isCompilationUnit, + // TODO(new-ir): Update selection. + selectionStart: null, + selectionLength: null); + } +} diff --git a/lib/src/piece/import.dart b/lib/src/piece/import.dart new file mode 100644 index 00000000..077e4da1 --- /dev/null +++ b/lib/src/piece/import.dart @@ -0,0 +1,195 @@ +// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file +// 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. + +import '../back_end/code_writer.dart'; +import '../constants.dart'; +import 'piece.dart'; + +/// An import or export directive. +/// +/// Contains pieces for the keyword and URI, the optional `as` clause for +/// imports, the configurations (`if` clauses), and combinators (`show` and +/// `hide`). +class ImportPiece extends Piece { + /// The directive keyword and its URI. + final Piece directive; + + /// 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. + final Piece? combinator; + + ImportPiece(this.directive, this.asClause, this.combinator); + + @override + int get stateCount => 1; + + @override + void format(CodeWriter writer, int state) { + writer.format(directive); + writer.formatOptional(asClause); + writer.formatOptional(combinator); + } + + @override + void forEachChild(void Function(Piece piece) callback) { + callback(directive); + if (asClause case var asClause?) callback(asClause); + if (combinator case var combinator?) callback(combinator); + } + + @override + String toString() => 'Directive'; +} + +/// The combinator on a directive with only one combinator. It can be split: +/// +/// // 0: All on one line: +/// import 'animals.dart' show Ant, Bat, Cat; +/// +/// // 1: Split before the keyword: +/// import 'animals.dart' +/// show Ant, Bat, Cat; +/// +/// // 2: Split before the keyword and each name: +/// import 'animals.dart' +/// show +/// Ant, +/// Bat, +/// Cat; +class OneCombinatorPiece extends Piece { + final ImportCombinator combinator; + + OneCombinatorPiece(this.combinator); + + /// 0: No splits anywhere. + /// 1: Split before combinator keyword. + /// 2: Split before combinator keyword and before each name. + @override + int get stateCount => 3; + + @override + void format(CodeWriter writer, int state) { + combinator.format(writer, splitKeyword: state != 0, splitNames: state == 2); + } + + @override + void forEachChild(void Function(Piece piece) callback) { + combinator.forEachChild(callback); + } + + @override + String toString() => '1Comb'; +} + +/// The combinators on a directive with two combinators. It can be split: +/// +/// // 0: All on one line: +/// import 'animals.dart' show Ant, Bat hide Cat, Dog; +/// +/// // 1: Wrap before each keyword: +/// import 'animals.dart' +/// show Ant, Bat +/// hide Cat, Dog; +/// +/// // 2: Wrap before each keyword and split the first list of names: +/// import 'animals.dart' +/// show +/// Ant, +/// Bat +/// hide Cat, Dog; +/// +/// // 3: Wrap before each keyword and split the second list of names: +/// import 'animals.dart' +/// show Ant, Bat +/// hide +/// Cat, +/// Dog; +/// +/// // 4: Wrap before each keyword and split both lists of names: +/// import 'animals.dart' +/// show +/// Ant, +/// Bat +/// hide +/// Cat, +/// Dog; +/// +/// These are not allowed: +/// +/// // Wrap list but not keyword: +/// import 'animals.dart' show +/// Ant, +/// Bat +/// hide Cat, Dog; +/// +/// // Wrap one keyword but not both: +/// import 'animals.dart' +/// show Ant, Bat hide Cat, Dog; +/// +/// import 'animals.dart' show Ant, Bat +/// hide Cat, Dog; +/// +/// This ensures that when any wrapping occurs, the keywords are always at +/// the beginning of the line. +class TwoCombinatorPiece extends Piece { + final List combinators; + + TwoCombinatorPiece(this.combinators); + + @override + int get stateCount => 5; + + @override + void format(CodeWriter writer, int state) { + assert(combinators.length == 2); + + combinators[0].format(writer, + splitKeyword: state != 0, splitNames: state == 2 || state == 4); + combinators[1].format(writer, + splitKeyword: state != 0, splitNames: state == 3 || state == 4); + } + + @override + void forEachChild(void Function(Piece piece) callback) { + for (var combinator in combinators) { + combinator.forEachChild(callback); + } + } + + @override + String toString() => '2Comb'; +} + +/// A single `show` or `hide` combinator within an import or export directive. +class ImportCombinator { + /// The `show` or `hide` keyword. + final Piece keyword; + + /// The names being shown or hidden. + final List names = []; + + ImportCombinator(this.keyword); + + void format(CodeWriter writer, + {required bool splitKeyword, required bool splitNames}) { + writer.setAllowNewlines(true); + writer.splitIf(splitKeyword, indent: Indent.expression); + writer.setAllowNewlines(splitKeyword); + writer.format(keyword); + for (var name in names) { + writer.splitIf(splitNames, indent: Indent.combinatorName); + writer.setAllowNewlines(splitNames); + writer.format(name); + } + } + + void forEachChild(void Function(Piece piece) callback) { + callback(keyword); + names.forEach(callback); + } +} diff --git a/lib/src/piece/piece.dart b/lib/src/piece/piece.dart new file mode 100644 index 00000000..86549cad --- /dev/null +++ b/lib/src/piece/piece.dart @@ -0,0 +1,82 @@ +// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file +// 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. + +import '../back_end/code_writer.dart'; + +/// Base class for the formatter's internal representation used for line +/// splitting. +/// +/// We visit the source AST and convert it to a tree of [Piece]s. This tree +/// roughly follows the AST but includes comments and is optimized for +/// formatting and line splitting. The final output is then determined by +/// deciding which pieces split and how. +abstract class Piece { + /// The number of different ways this piece can be split. + /// + /// States are numbered incrementally starting at zero. State zero should + /// always be the lowest cost state with the fewest line splits. Lower states + /// should generally be preferred over higher states. + int get stateCount; + + /// Given that this piece is in [state], use [writer] to produce its formatted + /// output. + void format(CodeWriter writer, int state); + + /// Invokes [callback] on each piece contained in this piece. + void forEachChild(void Function(Piece piece) callback); +} + +/// A simple atomic piece of code. +/// +/// This may represent a series of tokens where no split can occur between them. +/// It may also contain one or more comments. +class TextPiece extends Piece { + /// The lines of text in this piece. + /// + /// Most [TextPieces] will contain only a single line, but a piece with + /// 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 _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 _hasNewline = false; + + @override + int get stateCount => 1; + + /// 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}) { + if (_lines.isEmpty) _lines.add(''); + + // TODO(perf): Consider a faster way of accumulating text. + _lines.last = _lines.last + text; + + if (containsNewline) _hasNewline = true; + } + + @override + void format(CodeWriter writer, int state) { + // Let the writer know if there are any embedded newlines even if there is + // only one "line" in [_lines]. + if (_hasNewline) writer.handleNewline(); + + for (var i = 0; i < _lines.length; i++) { + if (i > 0) writer.newline(); + writer.write(_lines[i]); + } + } + + @override + void forEachChild(void Function(Piece piece) callback) {} + + @override + String toString() => '`${_lines.join('¬')}`${_hasNewline ? '!' : ''}'; +} diff --git a/lib/src/piece/postfix.dart b/lib/src/piece/postfix.dart new file mode 100644 index 00000000..e1bccddf --- /dev/null +++ b/lib/src/piece/postfix.dart @@ -0,0 +1,46 @@ +// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file +// 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. + +import '../back_end/code_writer.dart'; +import '../constants.dart'; +import 'piece.dart'; + +/// A piece for a series of pieces that all split before or not. +/// +/// For example, an [ImportPiece] uses a [PostfixPiece] for the list of +/// configurations: +/// +/// ``` +/// import 'foo.dart' +/// if (a) 'foo_a.dart' +/// if (b) 'foo_a.dart' +/// if (c) 'foo_a.dart'; +/// ``` +/// +/// We either split before every `if` or none of them, and the [PostfixPiece] +/// contains a piece for each configuration to model that. +class PostfixPiece extends Piece { + final List pieces; + + PostfixPiece(this.pieces); + + @override + int get stateCount => 2; + + @override + void format(CodeWriter writer, int state) { + for (var piece in pieces) { + writer.splitIf(state == 1, indent: Indent.expression); + writer.format(piece); + } + } + + @override + void forEachChild(void Function(Piece piece) callback) { + pieces.forEach(callback); + } + + @override + String toString() => 'Post'; +} diff --git a/lib/src/piece/sequence.dart b/lib/src/piece/sequence.dart new file mode 100644 index 00000000..06cec593 --- /dev/null +++ b/lib/src/piece/sequence.dart @@ -0,0 +1,56 @@ +// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file +// 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. + +import '../back_end/code_writer.dart'; +import 'piece.dart'; + +/// A piece for a series of statements or members inside a block or declaration +/// body. +class SequencePiece extends Piece { + /// The series of members or statements. + final List contents = []; + + /// The pieces that should have a blank line preserved between them and the + /// next piece. + final Set _blanksAfter = {}; + + /// Appends [piece] to the sequence. + void add(Piece piece) { + contents.add(piece); + } + + /// Appends a blank line before the next piece in the sequence. + void addBlank() { + if (contents.isEmpty) return; + _blanksAfter.add(contents.last); + } + + /// Removes the blank line that has been appended over the last piece. + void removeBlank() { + if (contents.isEmpty) return; + _blanksAfter.remove(contents.last); + } + + @override + int get stateCount => 1; + + @override + void format(CodeWriter writer, int state) { + for (var i = 0; i < contents.length; i++) { + writer.format(contents[i]); + + if (i < contents.length - 1) { + writer.newline(blank: _blanksAfter.contains(contents[i])); + } + } + } + + @override + void forEachChild(void Function(Piece piece) callback) { + contents.forEach(callback); + } + + @override + String toString() => 'Sequence'; +} diff --git a/lib/src/testing/test_file.dart b/lib/src/testing/test_file.dart index 3bcd365a..21e185b6 100644 --- a/lib/src/testing/test_file.dart +++ b/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; diff --git a/pubspec.yaml b/pubspec.yaml index 423f9c68..86915378 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -11,6 +11,7 @@ environment: dependencies: analyzer: '^6.2.0' args: ">=1.0.0 <3.0.0" + collection: "^1.17.0" path: ^1.0.0 pub_semver: ">=1.4.4 <3.0.0" source_span: ^1.4.0 diff --git a/test/README.md b/test/README.md new file mode 100644 index 00000000..06591813 --- /dev/null +++ b/test/README.md @@ -0,0 +1,77 @@ +The formatter is tested similar to a compiler where most of the test +functionality is "end-to-end" tests that validate that a given input produces +an expected output. + +## Formatting file format + +The actual formatting logic live in test data files ending in ".unit" or +".stmt". The ".unit" extension is for tests whose input should be parsed as an +entire Dart compilation unit (roughly library or part file). The ".stmt" files +parse each expectation as a statement. + +These test files have a custom diff-like format: + +``` +40 columns | +>>> (indent 4) arithmetic operators +var a=1+2/(3*-b~/4); +<<< + var a = 1 + 2 / (3 * -b ~/ 4); +``` + +If the first line contains a `|`, then it indicates the page width that all +tests in this file should be formatted using. All other text on that line are +ignored. This is mainly used so that tests can test line wrapping behavior +without having to create long code to force things to wrap. + +The `>>>` line begins a test. It may have comment text afterwards describing the +test. If the line contains `(indent )` for some `n`, then formatter is told +to run with that level of indentation. This is mainly for regression tests where +the erroneous code appeared deeply nested inside some class or function and the +test wants to reproduce that same surrounding indentation. + +Lines after the `>>>` line are the input code to be formatted. + +The `<<<` ends the input and begins the expected formatted result. The end of +the file or the next `>>>` marks the end of the expected output. + +For each pair of input and expected output, the test runner creates a separate +test. It runs the input code through the formatter and validates that the +resulting code matches the expectation. + +Lines starting with `###` are treated as comments and are ignored. + +## Test directories + +These expectation files are organized in subdirectories of `test/`. The +formatter currently supports to separate formatting styles. Eventually support +for the older "short" style will be removed. + +The older short style tests are organized like: + +``` +comments/ - Test comment handling. +fixes/ - Test `--fix`. +regression/ - Regression tests. File names correspond to issues. +selections/ - Test how the formatter preserves selection information. +splitting/ - Test line splitting behavior. +whitespace/ - Test whitespace insertion and removal. +``` + +These tests are all run by `short_format_test.dart`. + +The newer tall style tests are: + +``` +expression/ - Test formatting expressions. +invocation/ - Test formatting function and member invocations. +member/ - Test formatting class/enum/extension/mixin members. +statement/ - Test formatting statements. +top_level/ - Test formatting top-level declarations and directives. +``` + +These tests are all run by `tall_format_test.dart`. + +The directory naming is a little muddled right now, but the idea is that once +the short style is no longer supported and can be removed, the remaining test +directories will make sense. diff --git a/test/fix_test.dart b/test/fix_test.dart index b1b81758..cfcad8b8 100644 --- a/test/fix_test.dart +++ b/test/fix_test.dart @@ -11,12 +11,16 @@ import 'package:test/test.dart'; import 'utils.dart'; void main() async { - await testFile( - 'fixes/named_default_separator.unit', [StyleFix.namedDefaultSeparator]); - await testFile('fixes/doc_comments.stmt', [StyleFix.docComments]); - await testFile('fixes/function_typedefs.unit', [StyleFix.functionTypedefs]); - await testFile('fixes/optional_const.unit', [StyleFix.optionalConst]); - await testFile('fixes/optional_new.stmt', [StyleFix.optionalNew]); + await testFile('fixes/named_default_separator.unit', + tall: false, fixes: [StyleFix.namedDefaultSeparator]); + await testFile('fixes/doc_comments.stmt', + tall: false, fixes: [StyleFix.docComments]); + await testFile('fixes/function_typedefs.unit', + tall: false, fixes: [StyleFix.functionTypedefs]); + await testFile('fixes/optional_const.unit', + tall: false, fixes: [StyleFix.optionalConst]); + await testFile('fixes/optional_new.stmt', + tall: false, fixes: [StyleFix.optionalNew]); await testFile('fixes/single_cascade_statements.stmt', - [StyleFix.singleCascadeStatements]); + tall: false, fixes: [StyleFix.singleCascadeStatements]); } diff --git a/test/formatter_test.dart b/test/short_format_test.dart similarity index 94% rename from test/formatter_test.dart rename to test/short_format_test.dart index 8422beaa..6576bd8f 100644 --- a/test/formatter_test.dart +++ b/test/short_format_test.dart @@ -11,11 +11,11 @@ import 'package:test/test.dart'; import 'utils.dart'; void main() async { - await testDirectory('comments'); - await testDirectory('regression'); - await testDirectory('selections'); - await testDirectory('splitting'); - await testDirectory('whitespace'); + await testDirectory('comments', tall: false); + await testDirectory('regression', tall: false); + await testDirectory('selections', tall: false); + await testDirectory('splitting', tall: false); + await testDirectory('whitespace', tall: false); test('throws a FormatterException on failed parse', () { var formatter = DartFormatter(); diff --git a/test/tall_format_test.dart b/test/tall_format_test.dart new file mode 100644 index 00000000..5a0dc926 --- /dev/null +++ b/test/tall_format_test.dart @@ -0,0 +1,18 @@ +// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file +// 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. + +@TestOn('vm') +library dart_style.test.tall_format_test; + +import 'package:test/test.dart'; + +import 'utils.dart'; + +void main() async { + await testDirectory('top_level', 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. +} diff --git a/test/top_level/export.unit b/test/top_level/export.unit new file mode 100644 index 00000000..a61e26ca --- /dev/null +++ b/test/top_level/export.unit @@ -0,0 +1,11 @@ +40 columns | +>>> Don't split after "export" even with long string. +export 'package:some/very/long/export/path.dart'; +<<< +export 'package:some/very/long/export/path.dart'; +>>> Handle "show" and "hide" combinators. +### Most tests of "show" and "hide" use import, but we just do a token test +### here to make sure that export directives handle the combinators. +export 'a.dart'show Ape,Bear hide Cat; +<<< +export 'a.dart' show Ape, Bear hide Cat; \ No newline at end of file diff --git a/test/top_level/import.unit b/test/top_level/import.unit new file mode 100644 index 00000000..47c903c7 --- /dev/null +++ b/test/top_level/import.unit @@ -0,0 +1,30 @@ +40 columns | +>>> Don't split after "import" even with long string. +import +'package:some/very/long/import/path.dart'; +<<< +import 'package:some/very/long/import/path.dart'; +>>> Keep "as" on same line. +import 'package:foo.dart' + as foo; +<<< +import 'package:foo.dart' as foo; +>>> Wrap before "as". +import 'package:some/path/foo.dart' as foo; +<<< +import 'package:some/path/foo.dart' + as foo; +>>> Keep "deferred as" on same line. +import 'foo.dart' deferred as foo; +<<< +import 'foo.dart' deferred as foo; +>>> Wrap before "deferred". +import 'package:foo/foo.dart' deferred as path; +<<< +import 'package:foo/foo.dart' + deferred as path; +>>> Don't split before "deferred" and "as". +import 'package:foo/some/path/foo.dart' deferred as very_long_identifier_path; +<<< +import 'package:foo/some/path/foo.dart' + deferred as very_long_identifier_path; \ No newline at end of file diff --git a/test/top_level/library.unit b/test/top_level/library.unit new file mode 100644 index 00000000..9d40e20e --- /dev/null +++ b/test/top_level/library.unit @@ -0,0 +1,13 @@ +40 columns | +>>> No spaces between identifiers. +library a . b . c; +<<< +library a.b.c; +>>> Don't wrap identifier. +library veryLong.alsoLong.evenMoreLong.thisToo; +<<< +library veryLong.alsoLong.evenMoreLong.thisToo; +>>> No spaces after unnamed library. +library ; +<<< +library; \ No newline at end of file diff --git a/test/top_level/show_hide.unit b/test/top_level/show_hide.unit new file mode 100644 index 00000000..c88d6fb9 --- /dev/null +++ b/test/top_level/show_hide.unit @@ -0,0 +1,101 @@ +40 columns | +>>> Keep shows on one line. +import 'foo.dart'show Ape,Bear,Cat; +<<< +import 'foo.dart' show Ape, Bear, Cat; +>>> Move all shows to next line. +import 'foo.dart' show Ape, Bear, Cat, Dog; +<<< +import 'foo.dart' + show Ape, Bear, Cat, Dog; +>>> Move all shows each to their own line. +import 'foo.dart'show Ape,Bear,Cat,Dog,Echidna,FlyingFox,Gorilla; +<<< +import 'foo.dart' + show + Ape, + Bear, + Cat, + Dog, + Echidna, + FlyingFox, + Gorilla; +>>> Keeps hide on one line. +import 'foo.dart'hide Ape,Bear,Cat; +<<< +import 'foo.dart' hide Ape, Bear, Cat; +>>> Move hides to next line. +import 'foo.dart' hide Ape, Bear, Cat, Dog; +<<< +import 'foo.dart' + hide Ape, Bear, Cat, Dog; +>>> Moves hides each to their own line. +import 'foo.dart'hide Ape,Bear,Cat,Dog,Echidna,FlyingFox,Gorilla; +<<< +import 'foo.dart' + hide + Ape, + Bear, + Cat, + Dog, + Echidna, + FlyingFox, + Gorilla; +>>> Both show and hide on directive line. +import 'foo.dart'hide Ape show Bear; +<<< +import 'foo.dart' hide Ape show Bear; +>>> Each combinator on its own line. +import 'foo.dart'hide Ape,Bear,Cat,Dog show Ape,Bear,Cat,Dog; +<<< +import 'foo.dart' + hide Ape, Bear, Cat, Dog + show Ape, Bear, Cat, Dog; +>>> Each combinator on own line, first one is split. +import 'foo.dart'hide Ape,Bear,Cat,Dog, Echidna, FlyingFox show Ape,Bear,Cat,Dog; +<<< +import 'foo.dart' + hide + Ape, + Bear, + Cat, + Dog, + Echidna, + FlyingFox + show Ape, Bear, Cat, Dog; +>>> Each combinator on own line, second one is split. +import 'foo.dart'hide Ape,Bear,Cat,Dog show Ape,Bear,Cat,Dog, Echidna, FlyingFox; +<<< +import 'foo.dart' + hide Ape, Bear, Cat, Dog + show + Ape, + Bear, + Cat, + Dog, + Echidna, + FlyingFox; +>>> Each combinator on own line, both are split. +import 'foo.dart'hide Ape,Bear,Cat,Dog, Echidna, FlyingFox show Ape,Bear,Cat,Dog, Echidna, FlyingFox; +<<< +import 'foo.dart' + hide + Ape, + Bear, + Cat, + Dog, + Echidna, + FlyingFox + show + Ape, + Bear, + Cat, + Dog, + Echidna, + FlyingFox; +>>> If combinators don't all fit on first line, always split both. +import 'foo.dart' hide Ape, Bear show Ape, Bear, Cat, Dog; +<<< +import 'foo.dart' + hide Ape, Bear + show Ape, Bear, Cat, Dog; \ No newline at end of file diff --git a/test/utils.dart b/test/utils.dart index f7fa98df..27ccc69e 100644 --- a/test/utils.dart +++ b/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'; @@ -128,24 +129,29 @@ Future runCommandOnDir([List? args]) { } /// Run tests defined in "*.unit" and "*.stmt" files inside directory [name]. -Future testDirectory(String name, [Iterable? fixes]) async { +Future testDirectory(String name, + {required bool tall, Iterable? fixes}) async { for (var test in await TestFile.listDirectory(name)) { - _testFile(test, fixes); + _testFile(test, tall, fixes); } } -Future testFile(String path, [Iterable? fixes]) async { - _testFile(await TestFile.read(path), fixes); +Future testFile(String path, + {required bool tall, Iterable? fixes}) async { + _testFile(await TestFile.read(path), tall, fixes); } -void _testFile(TestFile testFile, Iterable? baseFixes) { +void _testFile( + TestFile testFile, bool useTallStyle, Iterable? baseFixes) { group(testFile.path, () { for (var formatTest in testFile.tests) { test(formatTest.label, () { var formatter = DartFormatter( pageWidth: testFile.pageWidth, indent: formatTest.leadingIndent, - fixes: [...?baseFixes, ...formatTest.fixes]); + fixes: [...?baseFixes, ...formatTest.fixes], + experimentFlags: + useTallStyle ? const [tallStyleExperimentFlag] : null); var actual = formatter.formatSource(formatTest.input); From bcdafc080afba08069937748c246b8e0c51543b2 Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Thu, 14 Sep 2023 15:41:34 -0700 Subject: [PATCH 3/6] Format configurations in imports and exports. (#1265) Format configurations in imports and exports. This also brings in InfixPiece which will be used for other infix operators and infix-like constructs. --- lib/src/front_end/ast_node_visitor.dart | 22 ++++++---- lib/src/front_end/piece_factory.dart | 42 ++++++++++++++++++- lib/src/piece/import.dart | 10 ++++- lib/src/piece/infix.dart | 54 +++++++++++++++++++++++++ test/top_level/export.unit | 7 +++- test/top_level/import.unit | 36 ++++++++++++++++- 6 files changed, 158 insertions(+), 13 deletions(-) create mode 100644 lib/src/piece/infix.dart diff --git a/lib/src/front_end/ast_node_visitor.dart b/lib/src/front_end/ast_node_visitor.dart index 13a98498..6108cfba 100644 --- a/lib/src/front_end/ast_node_visitor.dart +++ b/lib/src/front_end/ast_node_visitor.dart @@ -188,7 +188,19 @@ class AstNodeVisitor extends ThrowingAstVisitor with PieceFactory { @override void visitConfiguration(Configuration node) { - throw UnimplementedError(); + token(node.ifKeyword); + writer.space(); + token(node.leftParenthesis); + + if (node.equalToken case var equals?) { + createInfix(node.name, equals, node.value!, hanging: true); + } else { + visit(node.name); + } + + token(node.rightParenthesis); + writer.space(); + visit(node.uri); } @override @@ -238,7 +250,7 @@ class AstNodeVisitor extends ThrowingAstVisitor with PieceFactory { @override void visitDottedName(DottedName node) { - throw UnimplementedError(); + createDotted(node.components); } @override @@ -268,9 +280,6 @@ class AstNodeVisitor extends ThrowingAstVisitor with PieceFactory { @override void visitExportDirective(ExportDirective node) { - // TODO(tall): Format configurations. - if (node.configurations.isNotEmpty) throw UnimplementedError(); - createImport(node, node.exportKeyword); } @@ -416,9 +425,6 @@ class AstNodeVisitor extends ThrowingAstVisitor with PieceFactory { @override void visitImportDirective(ImportDirective node) { - // TODO(tall): Format configurations. - if (node.configurations.isNotEmpty) throw UnimplementedError(); - createImport(node, node.importKeyword, deferredKeyword: node.deferredKeyword, asKeyword: node.asKeyword, diff --git a/lib/src/front_end/piece_factory.dart b/lib/src/front_end/piece_factory.dart index 95e5c0e9..6a0e9425 100644 --- a/lib/src/front_end/piece_factory.dart +++ b/lib/src/front_end/piece_factory.dart @@ -5,6 +5,7 @@ import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/token.dart'; import '../piece/import.dart'; +import '../piece/infix.dart'; import '../piece/piece.dart'; import '../piece/postfix.dart'; import '../piece/sequence.dart'; @@ -72,6 +73,18 @@ mixin PieceFactory { visit(directive.uri); var directivePiece = writer.pop(); + Piece? configurationsPiece; + if (directive.configurations.isNotEmpty) { + var configurations = []; + for (var configuration in directive.configurations) { + writer.split(); + visit(configuration); + configurations.add(writer.pop()); + } + + configurationsPiece = PostfixPiece(configurations); + } + Piece? asClause; if (asKeyword != null) { writer.split(); @@ -112,7 +125,34 @@ mixin PieceFactory { token(directive.semicolon); - writer.push(ImportPiece(directivePiece, asClause, combinator)); + writer.push( + ImportPiece(directivePiece, configurationsPiece, asClause, combinator)); + } + + /// Creates a single infix operation. + /// + /// If [hanging] is `true` then the operator goes at the end of the first + /// line, like `+`. Otherwise, it goes at the beginning of the second, like + /// `as`. + void createInfix(AstNode left, Token operator, AstNode right, + {bool hanging = false}) { + var operands = []; + visit(left); + operands.add(writer.pop()); + + if (hanging) { + writer.space(); + token(operator); + writer.split(); + } else { + writer.split(); + token(operator); + writer.space(); + } + + visit(right); + operands.add(writer.pop()); + writer.push(InfixPiece(operands)); } /// Emit [token], along with any comments and formatted whitespace that comes diff --git a/lib/src/piece/import.dart b/lib/src/piece/import.dart index 077e4da1..d5547107 100644 --- a/lib/src/piece/import.dart +++ b/lib/src/piece/import.dart @@ -12,9 +12,12 @@ import 'piece.dart'; /// imports, the configurations (`if` clauses), and combinators (`show` and /// `hide`). class ImportPiece extends Piece { - /// The directive keyword and its URI. + /// The main directive and its URI. final Piece directive; + /// If the directive has `if` configurations, this is them. + final Piece? configurations; + /// The `as` clause for this directive. /// /// Null if this is not an import or it has no library prefix. @@ -23,7 +26,8 @@ class ImportPiece extends Piece { /// The piece for the `show` and/or `hide` combinators. final Piece? combinator; - ImportPiece(this.directive, this.asClause, this.combinator); + ImportPiece( + this.directive, this.configurations, this.asClause, this.combinator); @override int get stateCount => 1; @@ -31,6 +35,7 @@ class ImportPiece extends Piece { @override void format(CodeWriter writer, int state) { writer.format(directive); + writer.formatOptional(configurations); writer.formatOptional(asClause); writer.formatOptional(combinator); } @@ -38,6 +43,7 @@ class ImportPiece extends Piece { @override void forEachChild(void Function(Piece piece) callback) { callback(directive); + if (configurations case var configurations?) callback(configurations); if (asClause case var asClause?) callback(asClause); if (combinator case var combinator?) callback(combinator); } diff --git a/lib/src/piece/infix.dart b/lib/src/piece/infix.dart new file mode 100644 index 00000000..abc69ee1 --- /dev/null +++ b/lib/src/piece/infix.dart @@ -0,0 +1,54 @@ +// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file +// 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. +import '../back_end/code_writer.dart'; +import '../constants.dart'; +import 'piece.dart'; + +/// A piece for a series of binary expressions at the same precedence, like: +/// +/// ``` +/// a + b + c +/// ``` +class InfixPiece extends Piece { + /// The series of operands. + /// + /// Since we don't split on both sides of the operator, the operators will be + /// embedded in the operand pieces. If the operator is a hanging one, it will + /// be in the preceding operand, so `1 + 2` becomes "Infix(`1 +`, `2`)". + /// A leading operator like `foo as int` becomes "Infix(`foo`, `as int`)". + final List operands; + + InfixPiece(this.operands); + + @override + int get stateCount => 2; + + @override + void format(CodeWriter writer, int state) { + switch (state) { + case 0: + writer.setAllowNewlines(false); + for (var i = 0; i < operands.length; i++) { + writer.format(operands[i]); + + if (i < operands.length - 1) writer.space(); + } + + case 1: + writer.setNesting(Indent.expression); + for (var i = 0; i < operands.length; i++) { + writer.format(operands[i]); + if (i < operands.length - 1) writer.newline(); + } + } + } + + @override + void forEachChild(void Function(Piece piece) callback) { + operands.forEach(callback); + } + + @override + String toString() => 'Infix'; +} diff --git a/test/top_level/export.unit b/test/top_level/export.unit index a61e26ca..7bb3e81d 100644 --- a/test/top_level/export.unit +++ b/test/top_level/export.unit @@ -8,4 +8,9 @@ export 'package:some/very/long/export/path.dart'; ### here to make sure that export directives handle the combinators. export 'a.dart'show Ape,Bear hide Cat; <<< -export 'a.dart' show Ape, Bear hide Cat; \ No newline at end of file +export 'a.dart' show Ape, Bear hide Cat; +>>> Configuration. +### More detailed configuration tests are handled under import. +export'a'if(b . c=='d' )'e'; +<<< +export 'a' if (b.c == 'd') 'e'; diff --git a/test/top_level/import.unit b/test/top_level/import.unit index 47c903c7..b3976bf0 100644 --- a/test/top_level/import.unit +++ b/test/top_level/import.unit @@ -27,4 +27,38 @@ import 'package:foo/foo.dart' import 'package:foo/some/path/foo.dart' deferred as very_long_identifier_path; <<< import 'package:foo/some/path/foo.dart' - deferred as very_long_identifier_path; \ No newline at end of file + deferred as very_long_identifier_path; +>>> Dotted identifier in configuration. +import'a'if(b . c . d)'e'; +<<< +import 'a' if (b.c.d) 'e'; +>>> Multiple configurations on one line. +import 'a' if (b) 'b' if (c) 'c'; +<<< +import 'a' if (b) 'b' if (c) 'c'; +>>> If configurations don't fit, they all split. +import 'long/import/url.dart' if (b) 'b' if (c) 'c'; +<<< +import 'long/import/url.dart' + if (b) 'b' + if (c) 'c'; +>>> Configurations don't split before URI. +import 'long/import/url.dart' if (config) 'very/long/configured/import/url.dart'; +<<< +import 'long/import/url.dart' + if (config) 'very/long/configured/import/url.dart'; +>>> Unsplit configuration with `==`. +import 'a.dart' if (b == 's') 'c'; +<<< +import 'a.dart' if (b == 's') 'c'; +>>> Split before `if` before `==`. +import 'some/uri.dart' if (debug == 'string') 'c'; +<<< +import 'some/uri.dart' + if (debug == 'string') 'c'; +>>> Split before `==` in configuration. +import 'some/uri.dart' if (config.name.debug == 'string') 'c'; +<<< +import 'some/uri.dart' + if (config.name.debug == + 'string') 'c'; \ No newline at end of file From 251e094baea9105ee6e170dcf8300f310792d25e Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Fri, 15 Sep 2023 10:26:52 -0700 Subject: [PATCH 4/6] Bump pubspec now that there are unpublished changes. (#1271) Bump pubspec now that there are unpublished changes. (I forgot to do this when I merged the first new IR PR onto main.) --- CHANGELOG.md | 5 +++++ pubspec.yaml | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d9f43b0..da5e8eb9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +## 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). diff --git a/pubspec.yaml b/pubspec.yaml index 86915378..0db4c822 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,6 +1,6 @@ name: dart_style # Note: See tool/grind.dart for how to bump the version. -version: 2.3.3 +version: 2.3.4-wip description: >- Opinionated, automatic Dart source code formatter. Provides an API and a CLI tool. From cf8aef9fb670c3904480e40a857d0ae000849164 Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Fri, 15 Sep 2023 15:06:19 -0700 Subject: [PATCH 5/6] Format binary expressions with the new back-end. (#1267) Format binary expressions with the new back-end. This includes all arithmetic, logic, comparison, relational, type test, and bitwise operators. It doesn't include assignment operators, because those are right associative and need some special handling for how the RHS is indented. --- lib/src/front_end/ast_node_visitor.dart | 33 +++++-- lib/src/front_end/piece_factory.dart | 57 +++++++++++- test/expression/binary.stmt | 118 ++++++++++++++++++++++++ test/expression/type_test.stmt | 28 ++++++ test/statement/expression.stmt | 5 + test/tall_format_test.dart | 2 + 6 files changed, 236 insertions(+), 7 deletions(-) create mode 100644 test/expression/binary.stmt create mode 100644 test/expression/type_test.stmt create mode 100644 test/statement/expression.stmt diff --git a/lib/src/front_end/ast_node_visitor.dart b/lib/src/front_end/ast_node_visitor.dart index 6108cfba..47bc4a0a 100644 --- a/lib/src/front_end/ast_node_visitor.dart +++ b/lib/src/front_end/ast_node_visitor.dart @@ -66,7 +66,7 @@ class AstNodeVisitor extends ThrowingAstVisitor with PieceFactory { @override void visitAsExpression(AsExpression node) { - throw UnimplementedError(); + createInfix(node.expression, node.asOperator, node.type); } @override @@ -96,7 +96,14 @@ class AstNodeVisitor extends ThrowingAstVisitor with PieceFactory { @override void visitBinaryExpression(BinaryExpression node) { - throw UnimplementedError(); + createInfixChain( + node, + precedence: node.operator.type.precedence, + (expression) => ( + expression.leftOperand, + expression.operator, + expression.rightOperand + )); } @override @@ -290,7 +297,8 @@ class AstNodeVisitor extends ThrowingAstVisitor with PieceFactory { @override void visitExpressionStatement(ExpressionStatement node) { - throw UnimplementedError(); + visit(node.expression); + token(node.semicolon); } @override @@ -443,7 +451,7 @@ class AstNodeVisitor extends ThrowingAstVisitor with PieceFactory { @override void visitIntegerLiteral(IntegerLiteral node) { - throw UnimplementedError(); + token(node.literal); } @override @@ -458,7 +466,11 @@ class AstNodeVisitor extends ThrowingAstVisitor with PieceFactory { @override void visitIsExpression(IsExpression node) { - throw UnimplementedError(); + createInfix( + node.expression, + node.isOperator, + operator2: node.notOperator, + node.type); } @override @@ -541,7 +553,16 @@ class AstNodeVisitor extends ThrowingAstVisitor with PieceFactory { @override void visitNamedType(NamedType node) { - throw UnimplementedError(); + // TODO(tall): Handle import prefix. + if (node.importPrefix != null) throw UnimplementedError(); + + token(node.name2); + + // TODO(tall): Handle type arguments. + if (node.typeArguments != null) throw UnimplementedError(); + + // TODO(tall): Handle nullable types. + if (node.question != null) throw UnimplementedError(); } @override diff --git a/lib/src/front_end/piece_factory.dart b/lib/src/front_end/piece_factory.dart index 6a0e9425..2d436637 100644 --- a/lib/src/front_end/piece_factory.dart +++ b/lib/src/front_end/piece_factory.dart @@ -11,6 +11,9 @@ import '../piece/postfix.dart'; import '../piece/sequence.dart'; import 'piece_writer.dart'; +/// Record type for a destructured binary operator-like syntactic construct. +typedef BinaryOperation = (AstNode left, Token operator, AstNode right); + /// Utility methods for creating pieces that share formatting logic across /// multiple parts of the language. /// @@ -134,8 +137,11 @@ mixin PieceFactory { /// If [hanging] is `true` then the operator goes at the end of the first /// line, like `+`. Otherwise, it goes at the beginning of the second, like /// `as`. + /// + /// The [operator2] parameter may be passed if the "operator" is actually two + /// separate tokens, as in `foo is! Bar`. void createInfix(AstNode left, Token operator, AstNode right, - {bool hanging = false}) { + {bool hanging = false, Token? operator2}) { var operands = []; visit(left); operands.add(writer.pop()); @@ -143,10 +149,12 @@ mixin PieceFactory { if (hanging) { writer.space(); token(operator); + token(operator2); writer.split(); } else { writer.split(); token(operator); + token(operator2); writer.space(); } @@ -155,6 +163,53 @@ mixin PieceFactory { writer.push(InfixPiece(operands)); } + /// Creates a chained infix operation: a binary operator expression, or + /// binary pattern. + /// + /// In a tree of binary AST nodes, all operators at the same precedence are + /// treated as a single chain of operators that either all split or none do. + /// Operands within those (which may themselves be chains of higher + /// precedence binary operators) are then formatted independently. + /// + /// [T] is the type of node being visited and [destructure] is a callback + /// that takes one of those and yields the operands and operator. We need + /// this since there's no interface shared by the various binary operator + /// AST nodes. + /// + /// If [precedence] is given, then this only flattens binary nodes with that + /// same precedence. + void createInfixChain( + T node, BinaryOperation Function(T node) destructure, + {int? precedence}) { + var operands = []; + + void traverse(AstNode e) { + if (e is! T) { + visit(e); + operands.add(writer.pop()); + } else { + var (left, operator, right) = destructure(e); + if (precedence != null && operator.type.precedence != precedence) { + // Binary node, but a different precedence, so don't flatten. + visit(e); + operands.add(writer.pop()); + } else { + traverse(left); + + writer.space(); + token(operator); + + writer.split(); + traverse(right); + } + } + } + + traverse(node); + + writer.push(InfixPiece(operands)); + } + /// Emit [token], along with any comments and formatted whitespace that comes /// before it. /// diff --git a/test/expression/binary.stmt b/test/expression/binary.stmt new file mode 100644 index 00000000..a19b5c20 --- /dev/null +++ b/test/expression/binary.stmt @@ -0,0 +1,118 @@ +40 columns | +>>> Multiplicative operators. +1*2/3~/4%5; +<<< +1 * 2 / 3 ~/ 4 % 5; +>>> Additive operators. +1+2-3; +<<< +1 + 2 - 3; +>>> Shift operators. +1<<2>>3>>>4; +<<< +1 << 2 >> 3 >>> 4; +>>> Bitwise operators. +1&2^3|4; +<<< +1 & 2 ^ 3 | 4; +>>> Relation operators (which are not associative in Dart). +1<2; +<<< +1 < 2; +>>> +1>2; +<<< +1 > 2; +>>> +1<=2; +<<< +1 <= 2; +>>> +1>=2; +<<< +1 >= 2; +>>> Equality operators (which are not associative in Dart). +1==2; +<<< +1 == 2; +>>> Equality operators. +1!=2; +<<< +1 != 2; +>>> Logical operators. +1&&2||3; +<<< +1 && 2 || 3; +>>> If-null operator. +foo??bar; +<<< +foo ?? bar; +>>> Unsplit operators with mixed precedence. +1+2/3-4*5%6<<7; +<<< +1 + 2 / 3 - 4 * 5 % 6 << 7; +>>> If any operator splits, they all do. +operand1 + operand2 + operand3 + operand4; +<<< +operand1 + + operand2 + + operand3 + + operand4; +>>> Mixed multiplicative operators split together. +longName * longName / longName % longName ~/ longName; +<<< +longName * + longName / + longName % + longName ~/ + longName; +>>> Mixed additive operators split together. +longName + longName - longName + longName - longName; +<<< +longName + + longName - + longName + + longName - + longName; +>>> Mixed shift operators split together. +longName >> longName << longName >> longName >>> longName; +<<< +longName >> + longName << + longName >> + longName >>> + longName; +>>> Mixed ascending precedence. +b___________________ || a______________ && a______________ == a______________ > +a______________ + a______________; +<<< +b___________________ || + a______________ && + a______________ == + a______________ > + a______________ + + a______________; +>>> Mixed descending precedence. +b___________________ + a_______________ > a______________ == a______________ && +a______________ || a______________; +<<< +b___________________ + + a_______________ > + a______________ == + a______________ && + a______________ || + a______________; +>>> Mixture of same and different precedence. +veryLongIdentifier + veryLongIdentifier / veryLongIdentifier * +veryLongIdentifier - veryLongIdentifier * veryLongIdentifier + +veryLongIdentifier / veryLongIdentifier - veryLongIdentifier; +<<< +veryLongIdentifier + + veryLongIdentifier / + veryLongIdentifier * + veryLongIdentifier - + veryLongIdentifier * + veryLongIdentifier + + veryLongIdentifier / + veryLongIdentifier - + veryLongIdentifier; \ No newline at end of file diff --git a/test/expression/type_test.stmt b/test/expression/type_test.stmt new file mode 100644 index 00000000..aa9e3497 --- /dev/null +++ b/test/expression/type_test.stmt @@ -0,0 +1,28 @@ +40 columns | +>>> +foo as Bar; +<<< +foo as Bar; +>>> +foo is Bar; +<<< +foo is Bar; +>>> +foo is ! Bar; +<<< +foo is! Bar; +>>> Split `as` before operator. +extremelyLongIdentifier as VeryLongTypeName; +<<< +extremelyLongIdentifier + as VeryLongTypeName; +>>> Split `is` before operator. +extremelyLongIdentifier is VeryLongTypeName; +<<< +extremelyLongIdentifier + is VeryLongTypeName; +>>> Split `is!` before operator. +extremelyLongIdentifier is ! VeryLongTypeName; +<<< +extremelyLongIdentifier + is! VeryLongTypeName; \ No newline at end of file diff --git a/test/statement/expression.stmt b/test/statement/expression.stmt new file mode 100644 index 00000000..ba1d80ae --- /dev/null +++ b/test/statement/expression.stmt @@ -0,0 +1,5 @@ +40 columns | +>>> No space before ";". +expression ; +<<< +expression; \ No newline at end of file diff --git a/test/tall_format_test.dart b/test/tall_format_test.dart index 5a0dc926..da6c9ea4 100644 --- a/test/tall_format_test.dart +++ b/test/tall_format_test.dart @@ -10,6 +10,8 @@ import 'package:test/test.dart'; import 'utils.dart'; void main() async { + await testDirectory('expression', tall: true); + await testDirectory('statement', tall: true); await testDirectory('top_level', tall: true); // TODO(tall): The old formatter_test.dart has tests here for things like From 6b3677110115ab09e275aa5dbad515c058c2cc7e Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Mon, 18 Sep 2023 14:53:53 -0700 Subject: [PATCH 6/6] Format blocks and labeled statements. (#1268) Format blocks and labeled statements. This doesn't support blank lines between statements in blocks. That will happen when comments are supported, which this is leading up to. --- lib/src/constants.dart | 3 ++ lib/src/front_end/ast_node_visitor.dart | 17 +++++-- lib/src/front_end/piece_factory.dart | 28 +++++++++++ lib/src/front_end/piece_writer.dart | 3 -- lib/src/piece/block.dart | 62 +++++++++++++++++++++++++ test/statement/block.stmt | 51 ++++++++++++++++++++ test/statement/expression.stmt | 11 ++++- 7 files changed, 167 insertions(+), 8 deletions(-) create mode 100644 lib/src/piece/block.dart create mode 100644 test/statement/block.stmt diff --git a/lib/src/constants.dart b/lib/src/constants.dart index 2152e211..385db06a 100644 --- a/lib/src/constants.dart +++ b/lib/src/constants.dart @@ -65,6 +65,9 @@ class Cost { /// Constants for the number of spaces for various kinds of indentation. class Indent { + /// Reset back to no indentation. + static const none = 0; + /// The number of spaces in a block or collection body. static const block = 2; diff --git a/lib/src/front_end/ast_node_visitor.dart b/lib/src/front_end/ast_node_visitor.dart index 47bc4a0a..88105568 100644 --- a/lib/src/front_end/ast_node_visitor.dart +++ b/lib/src/front_end/ast_node_visitor.dart @@ -108,7 +108,7 @@ class AstNodeVisitor extends ThrowingAstVisitor with PieceFactory { @override void visitBlock(Block node) { - throw UnimplementedError(); + createBlock(node.leftBracket, node.statements, node.rightBracket); } @override @@ -272,7 +272,7 @@ class AstNodeVisitor extends ThrowingAstVisitor with PieceFactory { @override void visitEmptyStatement(EmptyStatement node) { - throw UnimplementedError(); + token(node.semicolon); } @override @@ -475,12 +475,21 @@ class AstNodeVisitor extends ThrowingAstVisitor with PieceFactory { @override void visitLabel(Label node) { - throw UnimplementedError(); + visit(node.label); + token(node.colon); } @override void visitLabeledStatement(LabeledStatement node) { - throw UnimplementedError(); + var sequence = SequencePiece(); + + for (var label in node.labels) { + addToSequence(sequence, label); + } + + addToSequence(sequence, node.statement); + + writer.push(sequence); } @override diff --git a/lib/src/front_end/piece_factory.dart b/lib/src/front_end/piece_factory.dart index 2d436637..d0d4a714 100644 --- a/lib/src/front_end/piece_factory.dart +++ b/lib/src/front_end/piece_factory.dart @@ -4,6 +4,7 @@ import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/token.dart'; +import '../piece/block.dart'; import '../piece/import.dart'; import '../piece/infix.dart'; import '../piece/piece.dart'; @@ -55,6 +56,33 @@ mixin PieceFactory { if (directive.metadata.isNotEmpty) throw UnimplementedError(); } + /// Creates a [BlockPiece] for a given bracket-delimited block or declaration + /// body. + void createBlock(Token leftBracket, List nodes, Token rightBracket) { + // Edge case: If the block is completely empty, output it as simple + // unsplittable text. + if (nodes.isEmpty && rightBracket.precedingComments == null) { + token(leftBracket); + token(rightBracket); + return; + } + + token(leftBracket); + var leftBracketPiece = writer.pop(); + writer.split(); + + var sequence = SequencePiece(); + for (var node in nodes) { + addToSequence(sequence, node); + } + + token(rightBracket); + var rightBracketPiece = writer.pop(); + + writer.push(BlockPiece(leftBracketPiece, sequence, rightBracketPiece, + alwaysSplit: nodes.isNotEmpty)); + } + /// Creates a dotted or qualified identifier. void createDotted(NodeList components) { for (var component in components) { diff --git a/lib/src/front_end/piece_writer.dart b/lib/src/front_end/piece_writer.dart index 2141384b..85262931 100644 --- a/lib/src/front_end/piece_writer.dart +++ b/lib/src/front_end/piece_writer.dart @@ -97,9 +97,6 @@ class PieceWriter { /// /// The previous text piece should already be taken. void split() { - // Shouldn't have redundant splits. - assert(!_pendingSplit); - _pendingSplit = true; } diff --git a/lib/src/piece/block.dart b/lib/src/piece/block.dart new file mode 100644 index 00000000..8794563f --- /dev/null +++ b/lib/src/piece/block.dart @@ -0,0 +1,62 @@ +// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file +// 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. + +import '../back_end/code_writer.dart'; +import '../constants.dart'; +import 'piece.dart'; +import 'sequence.dart'; + +/// A piece for a series of statements or members inside a block or declaration +/// body. +class BlockPiece extends Piece { + /// The opening delimiter. + final Piece leftBracket; + + /// The sequence of members, statements, and sequence-level comments. + final SequencePiece contents; + + /// The closing delimiter. + final Piece rightBracket; + + /// Whether the block should always split its contents. + /// + /// True for most blocks, but false for enums and blocks containing only + /// inline block comments. + final bool _alwaysSplit; + + BlockPiece(this.leftBracket, this.contents, this.rightBracket, + {bool alwaysSplit = true}) + : _alwaysSplit = alwaysSplit; + + @override + int get stateCount => _alwaysSplit ? 1 : 2; + + @override + void format(CodeWriter writer, int state) { + writer.format(leftBracket); + + if (_alwaysSplit || state == 1) { + writer.setIndent(Indent.block); + writer.newline(); + writer.format(contents); + writer.setIndent(Indent.none); + writer.newline(); + } else { + writer.setAllowNewlines(false); + writer.format(contents); + } + + writer.format(rightBracket); + } + + @override + void forEachChild(void Function(Piece piece) callback) { + callback(leftBracket); + callback(contents); + callback(rightBracket); + } + + @override + String toString() => 'Block'; +} diff --git a/test/statement/block.stmt b/test/statement/block.stmt new file mode 100644 index 00000000..40ee492b --- /dev/null +++ b/test/statement/block.stmt @@ -0,0 +1,51 @@ +40 columns | +>>> Empty block. +{ + +} +<<< +{} +>>> At least one newline between statements. +{a;b;c;} +<<< +{ + a; + b; + c; +} +>>> Nested blocks increase indentation. +{a;{b;{c;}d;}e;} +<<< +{ + a; + { + b; + { + c; + } + d; + } + e; +} +>>> Labelled statement. +{label:statement;} +<<< +{ + label: + statement; +} +>>> Multiple labels. +a: b:c:d: + + + +e: + +statement; +<<< +a: +b: +c: +d: +e: +statement; \ No newline at end of file diff --git a/test/statement/expression.stmt b/test/statement/expression.stmt index ba1d80ae..f56a7695 100644 --- a/test/statement/expression.stmt +++ b/test/statement/expression.stmt @@ -2,4 +2,13 @@ >>> No space before ";". expression ; <<< -expression; \ No newline at end of file +expression; +>>> Empty statements. +{;;;;} +<<< +{ + ; + ; + ; + ; +} \ No newline at end of file