Format constructor declarations. (#1338)
Format constructor declarations.
Includes everything that can go in a constructor:
- Factory and redirecting constructors.
- Initializing formals ("this." parameters).
- Super parameters.
- Constructor initializers.
- Assert initializers.
Constructors are tricky because there are some constraints between how
the parameter list and initializer list are allowed to split. Up until
now, aside from setAllowNewlines(), there aren't really any ways that
pieces interact.
To support this, I added a pretty simple API where when a piece calls
CodeWriter.format() on a child piece, it can pass it what state the
piece is required to be in. If the solution doesn't have the child in
that state, it invalidates the solution.
It seems to work pretty well. There is probably a more optimal way to
implement that in the solver so that we test these constraints even
before formatting and discard the solution more eagerly. I left a TODO
comment about that and we can look into it later when we're
benchmarking.
diff --git a/lib/src/back_end/code_writer.dart b/lib/src/back_end/code_writer.dart
index 9636722..0d5c06a 100644
--- a/lib/src/back_end/code_writer.dart
+++ b/lib/src/back_end/code_writer.dart
@@ -53,10 +53,16 @@
/// The number of characters in the line currently being written.
int _column = 0;
- /// Whether this solution has encountered a newline where none is allowed.
+ /// Whether this solution has encountered a conflict that makes it not a valid
+ /// solution.
///
- /// If true, it means the solution is invalid.
- bool _containsInvalidNewline = false;
+ /// This can occur if:
+ ///
+ /// - A mandatory newline occurs like from a line comment or statement where
+ /// none is permitted.
+ /// - An outer piece requires a child piece to have a certain state and it
+ /// doesn't.
+ bool _isInvalid = false;
/// The stack of state for each [Piece] being formatted.
///
@@ -126,7 +132,7 @@
return Solution(_pieceStates, _buffer.toString(), _selectionStart,
_selectionEnd, _nextPieceToExpand,
- isValid: !_containsInvalidNewline, overflow: _overflow, cost: _cost);
+ isValid: !_isInvalid, overflow: _overflow, cost: _cost);
}
/// Notes that a newline has been written.
@@ -138,7 +144,7 @@
/// 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;
+ if (!_options.allowNewlines) _isInvalid = true;
// Note that this piece contains a newline so that we can propagate that
// up to containing pieces too.
@@ -228,11 +234,12 @@
/// 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;
-
+ ///
+ /// If [requireState] is given, then [piece] must be in that state or the
+ /// solution is considered invalid. This is used to create constraints
+ /// between pieces. For example, a constructor piece forces the initializer
+ /// list child piece to split if the parameter list splits.
+ void format(Piece piece, {State? requireState}) {
_pieceOptions.add(_PieceOptions(_options.indent, _options.allowNewlines));
var isUnsolved = !_pieceStates.isBound(piece) && piece.states.length > 1;
@@ -240,7 +247,20 @@
var state = _pieceStates.pieceState(piece);
- _cost += state.cost;
+ // If the parent piece constrains this child to a certain state, then
+ // invalidate the solution if it doesn't match.
+ if (requireState != null && state != requireState) {
+ _isInvalid = true;
+ // TODO(perf): The solver doesn't discard invalid states that are caused
+ // by newlines because sometimes an invalid state like that can lead to
+ // a valid state by selecting values for other pieces.
+ //
+ // But in this case, once a partial solution is invalidated by one piece's
+ // state conflicting with another's, any solution derived from that is
+ // also going to be invalid and we can prune that entire solution tree.
+ }
+
+ _cost += piece.stateCost(state);
// TODO(perf): Memoize this. Might want to create a nested PieceWriter
// instead of passing in `this` so we can better control what state needs
@@ -316,7 +336,7 @@
// expand it next.
if (!_foundExpandLine &&
_nextPieceToExpand != null &&
- (_column > _pageWidth || _containsInvalidNewline)) {
+ (_column > _pageWidth || _isInvalid)) {
// We found a problematic line, so remember it and the piece on it.
_foundExpandLine = true;
} else if (!_foundExpandLine) {
diff --git a/lib/src/front_end/ast_node_visitor.dart b/lib/src/front_end/ast_node_visitor.dart
index b39171c..e6d109d 100644
--- a/lib/src/front_end/ast_node_visitor.dart
+++ b/lib/src/front_end/ast_node_visitor.dart
@@ -13,6 +13,7 @@
import '../piece/assign.dart';
import '../piece/block.dart';
import '../piece/chain.dart';
+import '../piece/constructor.dart';
import '../piece/for.dart';
import '../piece/if.dart';
import '../piece/infix.dart';
@@ -130,7 +131,17 @@
@override
Piece visitAssertInitializer(AssertInitializer node) {
- throw UnimplementedError();
+ return buildPiece((b) {
+ b.token(node.assertKeyword);
+ b.add(createList(
+ leftBracket: node.leftParenthesis,
+ [
+ node.condition,
+ if (node.message case var message?) message,
+ ],
+ rightBracket: node.rightParenthesis,
+ ));
+ });
}
@override
@@ -343,18 +354,85 @@
@override
Piece visitConstructorDeclaration(ConstructorDeclaration node) {
- throw UnimplementedError();
+ var header = buildPiece((b) {
+ b.modifier(node.externalKeyword);
+ b.modifier(node.constKeyword);
+ b.modifier(node.factoryKeyword);
+ b.visit(node.returnType);
+ b.token(node.period);
+ b.token(node.name);
+ });
+
+ var parameters = nodePiece(node.parameters);
+
+ Piece? redirect;
+ Piece? initializerSeparator;
+ Piece? initializers;
+ if (node.redirectedConstructor case var constructor?) {
+ redirect = AssignPiece(
+ tokenPiece(node.separator!), nodePiece(constructor),
+ isValueDelimited: false);
+ } else if (node.initializers.isNotEmpty) {
+ initializerSeparator = tokenPiece(node.separator!);
+ initializers = createList(node.initializers,
+ style: const ListStyle(commas: Commas.nonTrailing));
+ }
+
+ var body = createFunctionBody(node.body);
+
+ return ConstructorPiece(header, parameters, body,
+ canSplitParameters: node.parameters.parameters
+ .canSplit(node.parameters.rightParenthesis),
+ hasOptionalParameter: node.parameters.rightDelimiter != null,
+ redirect: redirect,
+ initializerSeparator: initializerSeparator,
+ initializers: initializers);
}
@override
Piece visitConstructorFieldInitializer(ConstructorFieldInitializer node) {
- throw UnimplementedError();
+ return buildPiece((b) {
+ b.token(node.thisKeyword);
+ b.token(node.period);
+ b.add(createAssignment(node.fieldName, node.equals, node.expression));
+ });
}
@override
Piece visitConstructorName(ConstructorName node) {
- throw UnsupportedError(
- 'This node is handled by visitInstanceCreationExpression().');
+ // If there is an import prefix and/or constructor name, then allow
+ // splitting before the `.`. This doesn't look good, but is consistent with
+ // constructor calls that don't have `new` or `const`. We allow splitting
+ // in the latter because there is no way to distinguish syntactically
+ // between a named constructor call and any other kind of method call or
+ // property access.
+ var operations = <Piece>[];
+
+ var builder = AdjacentBuilder(this);
+ if (node.type.importPrefix case var importPrefix?) {
+ builder.token(importPrefix.name);
+ operations.add(builder.build());
+ builder.token(importPrefix.period);
+ }
+
+ // The name of the type being constructed.
+ var type = node.type;
+ builder.token(type.name2);
+ builder.visit(type.typeArguments);
+ builder.token(type.question);
+
+ // If this is a named constructor, the name.
+ if (node.name != null) {
+ operations.add(builder.build());
+ builder.token(node.period);
+ builder.visit(node.name);
+ }
+
+ // If there was a prefix or constructor name, then make a splittable piece.
+ // Otherwise, the current piece is a simple identifier for the name.
+ operations.add(builder.build());
+ if (operations.length == 1) return operations.first;
+ return ChainPiece(operations);
}
@override
@@ -550,7 +628,30 @@
@override
Piece visitFieldFormalParameter(FieldFormalParameter node) {
- throw UnimplementedError();
+ if (node.parameters case var parameters?) {
+ // A function-typed field formal like:
+ //
+ // ```
+ // C(this.fn(parameter));
+ // ```
+ return createFunctionType(
+ node.type,
+ fieldKeyword: node.thisKeyword,
+ period: node.period,
+ node.name,
+ node.typeParameters,
+ parameters,
+ node.question,
+ parameter: node);
+ } else {
+ return createFormalParameter(
+ node,
+ mutableKeyword: node.keyword,
+ fieldKeyword: node.thisKeyword,
+ period: node.period,
+ node.type,
+ node.name);
+ }
}
@override
@@ -736,7 +837,7 @@
@override
Piece visitFunctionDeclaration(FunctionDeclaration node) {
return createFunction(
- externalKeyword: node.externalKeyword,
+ modifiers: [node.externalKeyword],
returnType: node.returnType,
propertyKeyword: node.propertyKeyword,
name: node.name,
@@ -974,11 +1075,9 @@
@override
Piece visitMethodDeclaration(MethodDeclaration node) {
return createFunction(
- externalKeyword: node.externalKeyword,
- modifierKeyword: node.modifierKeyword,
+ modifiers: [node.externalKeyword, node.modifierKeyword],
returnType: node.returnType,
- operatorKeyword: node.operatorKeyword,
- propertyKeyword: node.propertyKeyword,
+ propertyKeyword: node.operatorKeyword ?? node.propertyKeyword,
name: node.name,
typeParameters: node.typeParameters,
parameters: node.parameters,
@@ -1166,7 +1265,12 @@
@override
Piece visitRedirectingConstructorInvocation(
RedirectingConstructorInvocation node) {
- throw UnimplementedError();
+ return buildPiece((b) {
+ b.token(node.thisKeyword);
+ b.token(node.period);
+ b.visit(node.constructorName);
+ b.visit(node.argumentList);
+ });
}
@override
@@ -1260,22 +1364,8 @@
@override
Piece visitSimpleFormalParameter(SimpleFormalParameter node) {
- var builder = AdjacentBuilder(this);
- startFormalParameter(node, builder);
- builder.modifier(node.keyword);
-
- if ((node.type, node.name) case (var _?, var name?)) {
- // Have both a type and name, so allow splitting after the type.
- builder.visit(node.type);
- var typePiece = builder.build();
- var namePiece = tokenPiece(name);
- return VariablePiece(typePiece, [namePiece], hasType: true);
- } else {
- // Don't have both a type and name, so just write whichever one we have.
- builder.visit(node.type);
- builder.token(node.name);
- return builder.build();
- }
+ return createFormalParameter(node, node.type, node.name,
+ mutableKeyword: node.keyword);
}
@override
@@ -1300,7 +1390,12 @@
@override
Piece visitSuperConstructorInvocation(SuperConstructorInvocation node) {
- throw UnimplementedError();
+ return buildPiece((b) {
+ b.token(node.superKeyword);
+ b.token(node.period);
+ b.visit(node.constructorName);
+ b.visit(node.argumentList);
+ });
}
@override
@@ -1310,7 +1405,30 @@
@override
Piece visitSuperFormalParameter(SuperFormalParameter node) {
- throw UnimplementedError();
+ if (node.parameters case var parameters?) {
+ // A function-typed super parameter like:
+ //
+ // ```
+ // C(super.fn(parameter));
+ // ```
+ return createFunctionType(
+ node.type,
+ fieldKeyword: node.superKeyword,
+ period: node.period,
+ node.name,
+ node.typeParameters,
+ parameters,
+ node.question,
+ parameter: node);
+ } else {
+ return createFormalParameter(
+ node,
+ mutableKeyword: node.keyword,
+ fieldKeyword: node.superKeyword,
+ period: node.period,
+ node.type,
+ node.name);
+ }
}
@override
diff --git a/lib/src/front_end/delimited_list_builder.dart b/lib/src/front_end/delimited_list_builder.dart
index 68f917d..e8b1ce6 100644
--- a/lib/src/front_end/delimited_list_builder.dart
+++ b/lib/src/front_end/delimited_list_builder.dart
@@ -55,9 +55,10 @@
var blockElement = -1;
if (_style.allowBlockElement) blockElement = _findBlockElement();
- return ListPiece(_leftBracket, _elements, _blanksAfter, _rightBracket,
- _style, blockElement,
- mustSplit: _mustSplit);
+ var piece = ListPiece(_leftBracket, _elements, _blanksAfter, _rightBracket,
+ _style, blockElement);
+ if (_mustSplit) piece.pin(State.split);
+ return piece;
}
/// Adds the opening [bracket] to the built list.
diff --git a/lib/src/front_end/piece_factory.dart b/lib/src/front_end/piece_factory.dart
index ed1b02a..6af2080 100644
--- a/lib/src/front_end/piece_factory.dart
+++ b/lib/src/front_end/piece_factory.dart
@@ -16,6 +16,7 @@
import '../piece/postfix.dart';
import '../piece/try.dart';
import '../piece/type.dart';
+import '../piece/variable.dart';
import 'adjacent_builder.dart';
import 'ast_node_visitor.dart';
import 'comment_writer.dart';
@@ -219,6 +220,38 @@
});
}
+ /// Creates a normal (not function-typed) formal parameter with a name and/or
+ /// type annotation.
+ ///
+ /// If [mutableKeyword] is given, it should be the `var` or `final` keyword.
+ /// If [fieldKeyword] and [period] are given, the former should be the `this`
+ /// or `super` keyword for an initializing formal or super parameter.
+ Piece createFormalParameter(
+ NormalFormalParameter node, TypeAnnotation? type, Token? name,
+ {Token? mutableKeyword, Token? fieldKeyword, Token? period}) {
+ var builder = AdjacentBuilder(this);
+ startFormalParameter(node, builder);
+ builder.modifier(mutableKeyword);
+ builder.visit(type);
+
+ Piece? typePiece;
+ if (type != null && name != null) {
+ typePiece = builder.build();
+ }
+
+ builder.token(fieldKeyword);
+ builder.token(period);
+ builder.token(name);
+
+ // If we have both a type and name, allow splitting between them.
+ if (typePiece != null) {
+ var namePiece = builder.build();
+ return VariablePiece(typePiece, [namePiece], hasType: true);
+ }
+
+ return builder.build();
+ }
+
/// Creates a function, method, getter, or setter declaration.
///
/// If [modifierKeyword] is given, it should be the `static` or `abstract`
@@ -227,8 +260,7 @@
/// [propertyKeyword] is given, it should be the `get` or `set` keyword on a
/// getter or setter declaration.
Piece createFunction(
- {Token? externalKeyword,
- Token? modifierKeyword,
+ {List<Token?> modifiers = const [],
AstNode? returnType,
Token? operatorKeyword,
Token? propertyKeyword,
@@ -237,8 +269,9 @@
FormalParameterList? parameters,
required FunctionBody body}) {
var builder = AdjacentBuilder(this);
- builder.modifier(externalKeyword);
- builder.modifier(modifierKeyword);
+ for (var keyword in modifiers) {
+ builder.modifier(keyword);
+ }
Piece? returnTypePiece;
if (returnType != null) {
@@ -253,23 +286,37 @@
builder.visit(parameters);
var signature = builder.build();
- var bodyPiece = nodePiece(body);
+ var bodyPiece = createFunctionBody(body);
- return FunctionPiece(returnTypePiece, signature,
- body: bodyPiece, spaceBeforeBody: body is! EmptyFunctionBody);
+ return FunctionPiece(returnTypePiece, signature, bodyPiece);
+ }
+
+ /// Creates a piece for a function, method, or constructor body.
+ Piece createFunctionBody(FunctionBody body) {
+ return buildPiece((b) {
+ // Don't put a space before `;` bodies.
+ if (body is! EmptyFunctionBody) b.space();
+ b.visit(body);
+ });
}
/// Creates a function type or function-typed formal.
///
/// If creating a piece for a function-typed formal, then [parameter] is the
/// formal parameter.
+ ///
+ /// If this is a function-typed initializing formal (`this.foo()`), then
+ /// [fieldKeyword] is `this` and [period] is the `.`. Likewise, for a
+ /// function-typed super parameter, [fieldKeyword] is `super`.
Piece createFunctionType(
TypeAnnotation? returnType,
Token functionKeywordOrName,
TypeParameterList? typeParameters,
FormalParameterList parameters,
Token? question,
- {FunctionTypedFormalParameter? parameter}) {
+ {FormalParameter? parameter,
+ Token? fieldKeyword,
+ Token? period}) {
var builder = AdjacentBuilder(this);
if (parameter != null) startFormalParameter(parameter, builder);
@@ -280,6 +327,8 @@
returnTypePiece = builder.build();
}
+ builder.token(fieldKeyword);
+ builder.token(period);
builder.token(functionKeywordOrName);
builder.visit(typeParameters);
builder.visit(parameters);
diff --git a/lib/src/piece/assign.dart b/lib/src/piece/assign.dart
index c21e5ec..66da394 100644
--- a/lib/src/piece/assign.dart
+++ b/lib/src/piece/assign.dart
@@ -54,7 +54,7 @@
/// block-like formatting.
final bool _isValueDelimited;
- AssignPiece(this.target, this.value, {required bool isValueDelimited})
+ AssignPiece(this.target, this.value, {bool isValueDelimited = false})
: _isValueDelimited = isValueDelimited;
// TODO(tall): The old formatter allows the first operand of a split
diff --git a/lib/src/piece/constructor.dart b/lib/src/piece/constructor.dart
new file mode 100644
index 0000000..3303b7a
--- /dev/null
+++ b/lib/src/piece/constructor.dart
@@ -0,0 +1,176 @@
+// 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 constructor declaration.
+///
+/// This is somewhat similar to [FunctionPiece], but constructor initializers
+/// add a lot of complexity. In particular, there are constraints between how
+/// the parameter list is allowed to split and how the initializer list is
+/// allowed to split. Only a few combinations of splits are allowed:
+///
+/// [State.unsplit] No splits at all, in the parameters or initializers.
+///
+/// ```
+/// SomeClass(param) : a = 1, b = 2;
+/// ```
+///
+/// [_splitBeforeInitializers] Split before the `:` and between the
+/// initializers but not in the parameters.
+///
+/// ```
+/// SomeClass(param)
+/// : a = 1,
+/// b = 2;
+/// ```
+///
+/// [_splitBetweenInitializers] Split between the initializers but not before
+/// the `:`. This state should only be chosen when the parameters split. If
+/// there are no parameters, this state is excluded.
+///
+/// ```
+/// SomeClass(
+/// param
+/// ) : a = 1,
+/// b = 2;
+/// ```
+///
+/// In addition, this piece deals with indenting initializers appropriately
+/// based on whether the parameter list has a `]` or `}` before the `)`. If
+/// there are optional parameters, then initializers after the first are
+/// indented one space more to line up with the first initializer:
+///
+/// ```
+/// SomeClass(
+/// mandatory,
+/// ) : firstInitializer = 1,
+/// second = 2;
+/// // ^ Four spaces of indentation.
+///
+/// SomeClass([
+/// optional,
+/// ]) : firstInitializer = 1,
+/// second = 2;
+/// // ^ Five spaces of indentation.
+/// ```
+class ConstructorPiece extends Piece {
+ static const _splitBeforeInitializers = State(1, cost: 1);
+
+ static const _splitBetweenInitializers = State(2, cost: 2);
+
+ /// True if there are parameters or comments inside the parameter list.
+ ///
+ /// If so, then we allow splitting the parameter list while leaving the `:`
+ /// on the same line as the `)`.
+ final bool _canSplitParameters;
+
+ /// Whether the parameter list contains a `]` or `}` closing delimiter before
+ /// the `)`.
+ final bool _hasOptionalParameter;
+
+ /// The leading keywords, class name, and constructor name.
+ final Piece _header;
+
+ /// The constructor parameter list.
+ final Piece _parameters;
+
+ /// If this is a redirecting constructor, the redirection clause.
+ final Piece? _redirect;
+
+ /// If there are initializers, the `:` before them.
+ final Piece? _initializerSeparator;
+
+ /// The constructor initializers, if there are any.
+ final Piece? _initializers;
+
+ /// The constructor body.
+ final Piece _body;
+
+ ConstructorPiece(this._header, this._parameters, this._body,
+ {required bool canSplitParameters,
+ required bool hasOptionalParameter,
+ Piece? redirect,
+ Piece? initializerSeparator,
+ Piece? initializers})
+ : _canSplitParameters = canSplitParameters,
+ _hasOptionalParameter = hasOptionalParameter,
+ _redirect = redirect,
+ _initializerSeparator = initializerSeparator,
+ _initializers = initializers;
+
+ @override
+ List<State> get additionalStates => [
+ if (_initializers != null) _splitBeforeInitializers,
+ if (_canSplitParameters && _initializers != null)
+ _splitBetweenInitializers
+ ];
+
+ @override
+ void format(CodeWriter writer, State state) {
+ // There are constraints between how the parameters may split and now the
+ // initializers may split.
+ var (parameterState, initializerState) = switch (state) {
+ // If there are no initializers, the parameters can do whatever.
+ State.unsplit when _initializers == null => (null, null),
+ // All parameters and initializers on one line.
+ State.unsplit => (State.unsplit, State.unsplit),
+ // If the `:` splits, then the parameters can't.
+ _splitBeforeInitializers => (State.unsplit, State.split),
+ // The `) :` on its own line.
+ _splitBetweenInitializers => (State.split, State.split),
+ _ => throw ArgumentError(),
+ };
+
+ // If there's a newline in the header or parameters (like a line comment
+ // after the `)`), then don't allow the initializers to remain unsplit.
+ if (_initializers != null && state == State.unsplit) {
+ writer.setAllowNewlines(false);
+ }
+
+ writer.format(_header);
+ writer.format(_parameters, requireState: parameterState);
+
+ if (_redirect case var redirect?) {
+ writer.space();
+ writer.format(redirect);
+ }
+
+ if (_initializers case var initializers?) {
+ writer.setAllowNewlines(state != State.unsplit);
+ writer.splitIf(state == _splitBeforeInitializers, indent: 2);
+
+ writer.format(_initializerSeparator!);
+ writer.space();
+
+ // Indent subsequent initializers past the `:`.
+ if (_hasOptionalParameter && state == _splitBetweenInitializers) {
+ // If the parameter list ends in `]) : init...` then we need to indent
+ // +5 to line up subsequent initializers.
+ writer.setIndent(5);
+ } else {
+ writer.setIndent(4);
+ }
+
+ writer.format(initializers, requireState: initializerState);
+ }
+
+ writer.setIndent(0);
+ writer.setAllowNewlines(true);
+ writer.format(_body);
+ }
+
+ @override
+ void forEachChild(void Function(Piece piece) callback) {
+ callback(_header);
+ callback(_parameters);
+ if (_redirect case var redirect?) callback(redirect);
+ if (_initializerSeparator case var separator?) callback(separator);
+ if (_initializers case var initializers?) callback(initializers);
+ callback(_body);
+ }
+
+ @override
+ String get debugName => 'Ctor';
+}
diff --git a/lib/src/piece/function.dart b/lib/src/piece/function.dart
index 88aae1b..1193c89 100644
--- a/lib/src/piece/function.dart
+++ b/lib/src/piece/function.dart
@@ -21,22 +21,7 @@
/// If this is a function declaration with a (non-empty `;`) body, the body.
final Piece? _body;
- /// Whether we should write a space between the function signature and body.
- ///
- /// This is `true` for most bodies except for empty function bodies, like:
- ///
- /// ```
- /// class C {
- /// C();
- /// // ^ No space before `;`.
- /// }
- /// ```
- final bool _spaceBeforeBody;
-
- FunctionPiece(this._returnType, this._signature,
- {Piece? body, bool spaceBeforeBody = false})
- : _body = body,
- _spaceBeforeBody = spaceBeforeBody;
+ FunctionPiece(this._returnType, this._signature, [this._body]);
@override
List<State> get additionalStates =>
@@ -57,10 +42,8 @@
}
writer.format(_signature);
- if (_body case var body?) {
- if (_spaceBeforeBody) writer.space();
- writer.format(body);
- }
+
+ if (_body case var body?) writer.format(body);
}
@override
diff --git a/lib/src/piece/list.dart b/lib/src/piece/list.dart
index 5c3f398..7b3ec1a 100644
--- a/lib/src/piece/list.dart
+++ b/lib/src/piece/list.dart
@@ -61,32 +61,21 @@
/// The details of how this particular list should be formatted.
final ListStyle _style;
- /// The state when the list is split.
- ///
- /// We use this instead of [State.split] because the cost is higher for some
- /// kinds of lists.
- ///
- /// Also, if this list does have a block element, then we also increase the
- /// cost of splitting the list itself a little more to prefer the block
- /// argument splitting when possible.
- // TODO(rnystrom): Having to use a different state for this is a little
- // cumbersome. Maybe it would be better to most costs out of [State] and
- // instead have the [Solver] ask each [Piece] for the cost of its state.
- final State _splitState;
-
/// If this list has an element that can receive block formatting, this is
/// the elements's index. Otherwise `-1`.
final int _blockElement;
ListPiece(this._before, this._elements, this._blanksAfter, this._after,
- this._style, this._blockElement,
- {required bool mustSplit})
- : _splitState = State(2, cost: _style.splitCost) {
- if (mustSplit) pin(_splitState);
- }
+ this._style, this._blockElement);
@override
- List<State> get additionalStates => [if (_elements.isNotEmpty) _splitState];
+ List<State> get additionalStates => [if (_elements.isNotEmpty) State.split];
+
+ @override
+ int stateCost(State state) {
+ if (state == State.split) return _style.splitCost;
+ return super.stateCost(state);
+ }
@override
void format(CodeWriter writer, State state) {
@@ -121,13 +110,13 @@
// Only allow newlines in the block element or in all elements if we're
// fully split.
- writer.setAllowNewlines(i == _blockElement || state == _splitState);
+ writer.setAllowNewlines(i == _blockElement || state == State.split);
var element = _elements[i];
element.format(writer,
appendComma: appendComma,
// Only allow newlines in comments if we're fully split.
- allowNewlinesInComments: state == _splitState);
+ allowNewlinesInComments: state == State.split);
// Write a space or newline between elements.
if (!isLast) {
diff --git a/lib/src/piece/piece.dart b/lib/src/piece/piece.dart
index 116763a..371bf5e 100644
--- a/lib/src/piece/piece.dart
+++ b/lib/src/piece/piece.dart
@@ -55,6 +55,14 @@
/// Invokes [callback] on each piece contained in this piece.
void forEachChild(void Function(Piece piece) callback);
+ /// The cost that this piece should apply to the solution when in [state].
+ ///
+ /// This is usually just the state's cost, but some pieces may want to tweak
+ /// the cost in certain circumstances.
+ // TODO(tall): Given that we have this API now, consider whether it makes
+ // sense to remove the cost field from State entirely.
+ int stateCost(State state) => state.cost;
+
/// Forces this piece to always use [state].
void pin(State state) {
_pinnedState = state;
diff --git a/test/declaration/constructor.unit b/test/declaration/constructor.unit
new file mode 100644
index 0000000..4e2cde2
--- /dev/null
+++ b/test/declaration/constructor.unit
@@ -0,0 +1,129 @@
+40 columns |
+>>> Semicolon body.
+class A {
+ A();
+}
+<<<
+class A {
+ A();
+}
+>>> Empty body.
+class A {
+ A() {}
+}
+<<<
+class A {
+ A() {}
+}
+>>> Non-empty body.
+class A {
+ A() {body;}
+}
+<<<
+class A {
+ A() {
+ body;
+ }
+}
+>>> Constant and factory constructors.
+class Foo {
+ const Foo . constant ();
+ factory Foo . fact () => foo;
+}
+<<<
+class Foo {
+ const Foo.constant();
+ factory Foo.fact() => foo;
+}
+>>> Named constructor.
+class Foo {
+ Foo . bar () { }
+ const Foo . baz() ;
+ factory Foo . bang( int param ) => foo;
+}
+<<<
+class Foo {
+ Foo.bar() {}
+ const Foo.baz();
+ factory Foo.bang(int param) => foo;
+}
+>>> Don't split at named constructor.
+class SomeLongClassName {
+ SomeLongClassName.someLongConstructorName();
+}
+<<<
+class SomeLongClassName {
+ SomeLongClassName.someLongConstructorName();
+}
+>>> Redirecting constructor.
+class A {
+ A ( ) : this . named ( 1 );
+ A . named ( int x ) : this ( );
+}
+<<<
+class A {
+ A() : this.named(1);
+ A.named(int x) : this();
+}
+>>> Split after `:` in redirecting constructor.
+class LongClassName {
+ LongClassName() : this(argument, another);
+}
+<<<
+class LongClassName {
+ LongClassName()
+ : this(argument, another);
+}
+>>> Split in redirecting constructor argument list.
+class LongClassName {
+ LongClassName() : this(argument, anotherArgument, thirdOne);
+}
+<<<
+class LongClassName {
+ LongClassName()
+ : this(
+ argument,
+ anotherArgument,
+ thirdOne,
+ );
+}
+>>> Redirecting factory constructor.
+class A {
+ factory A . nonConst ( ) = B < int >;
+ const factory A ( int x ) = prefix . B;
+}
+<<<
+class A {
+ factory A.nonConst() = B<int>;
+ const factory A(int x) = prefix.B;
+}
+>>> Split after `=` in redirecting factory constructor.
+class VeryLongClassName {
+ factory VeryLongClassName() = OtherLongClass;
+}
+<<<
+class VeryLongClassName {
+ factory VeryLongClassName() =
+ OtherLongClass;
+}
+>>> Prefer to split at `=` instead of in type arguments.
+class LongClass {
+ factory LongClass() = Other<LongType, Another>;
+}
+<<<
+class LongClass {
+ factory LongClass() =
+ Other<LongType, Another>;
+}
+>>> Split in redirecting factory constructor type argument list.
+class LongClass {
+ factory LongClass() = Other<SomeLongType, AnotherLongType>;
+}
+<<<
+class LongClass {
+ factory LongClass() =
+ Other<
+ SomeLongType,
+ AnotherLongType
+ >;
+}
\ No newline at end of file
diff --git a/test/declaration/constructor_assert.unit b/test/declaration/constructor_assert.unit
new file mode 100644
index 0000000..25ea2a6
--- /dev/null
+++ b/test/declaration/constructor_assert.unit
@@ -0,0 +1,82 @@
+40 columns |
+>>> Unsplit assert initializers.
+class Foo {
+ Foo() : assert(1), assert(2);
+}
+<<<
+class Foo {
+ Foo() : assert(1), assert(2);
+}
+>>> Split assert initializers.
+class Foo {
+ Foo(parameter, another) : assert(condition, 'some long message'),
+ assert(cond),
+ assert(anotherCondition, 'another message');
+}
+<<<
+class Foo {
+ Foo(parameter, another)
+ : assert(
+ condition,
+ 'some long message',
+ ),
+ assert(cond),
+ assert(
+ anotherCondition,
+ 'another message',
+ );
+}
+>>> Split in assert forces initializers to split.
+class Foo {
+ Foo() : assert(
+ veryLongConditionExpression);
+}
+<<<
+class Foo {
+ Foo()
+ : assert(
+ veryLongConditionExpression,
+ );
+}
+>>> Align split assert argument lists past the `:`.
+class Foo {
+ Foo(parameter1, parameter2, parameter3)
+ : assert(condition, 'some long assert message'),
+ assert(anotherLongCondition, 'a message');
+}
+<<<
+class Foo {
+ Foo(
+ parameter1,
+ parameter2,
+ parameter3,
+ ) : assert(
+ condition,
+ 'some long assert message',
+ ),
+ assert(
+ anotherLongCondition,
+ 'a message',
+ );
+}
+>>> Align split assert argument lists past the `:`.
+class Foo {
+ Foo(parameter1, [parameter2, parameter3])
+ : assert(condition, 'some long assert message'),
+ assert(anotherLongCondition, 'a message');
+}
+<<<
+class Foo {
+ Foo(
+ parameter1, [
+ parameter2,
+ parameter3,
+ ]) : assert(
+ condition,
+ 'some long assert message',
+ ),
+ assert(
+ anotherLongCondition,
+ 'a message',
+ );
+}
\ No newline at end of file
diff --git a/test/declaration/constructor_comment.unit b/test/declaration/constructor_comment.unit
new file mode 100644
index 0000000..68cc86f
--- /dev/null
+++ b/test/declaration/constructor_comment.unit
@@ -0,0 +1,184 @@
+40 columns |
+>>> Line comment before redirecting constructor separator.
+class A {
+ A() // comment
+ : this();
+}
+<<<
+class A {
+ A() // comment
+ : this();
+}
+>>> Line comment after redirecting constructor separator.
+class A {
+ A() : // comment
+ this();
+}
+<<<
+class A {
+ A()
+ : // comment
+ this();
+}
+>>> Line comment before redirecting factory constructor separator.
+class A {
+ factory A() // comment
+ = B;
+}
+<<<
+class A {
+ factory A() // comment
+ = B;
+}
+>>> Line comment after redirecting factory constructor separator.
+class A {
+ factory A() = // comment
+ B;
+}
+<<<
+class A {
+ factory A() = // comment
+ B;
+}
+>>> Line comment in parameter list.
+class A {
+ A(param, // comment
+ param2) : b = 1, c = 2;
+}
+<<<
+class A {
+ A(
+ param, // comment
+ param2,
+ ) : b = 1,
+ c = 2;
+}
+>>> Line comment before initializer separator.
+class A {
+ A(param) // comment
+ : b = 1, c = 2;
+}
+<<<
+class A {
+ A(param) // comment
+ : b = 1,
+ c = 2;
+}
+>>> Line comment after initializer separator.
+class A {
+ A(param) :// comment
+ b = 1, c = 2;
+}
+<<<
+class A {
+ A(param)
+ : // comment
+ b = 1,
+ c = 2;
+}
+>>> Line comment with newline after initializer separator.
+class A {
+ A(param) :
+ // comment
+ b = 1, c = 2;
+}
+<<<
+class A {
+ A(param)
+ : // comment
+ b = 1,
+ c = 2;
+}
+>>> Line comment before initializer comma.
+class A {
+ A(param) : b = 1 // comment
+ , c = 2;
+}
+<<<
+class A {
+ A(param)
+ : b = 1, // comment
+ c = 2;
+}
+>>> Line comment after initializer comma.
+class A {
+ A(param) : b = 1, // comment
+ c = 2;
+}
+<<<
+class A {
+ A(param)
+ : b = 1, // comment
+ c = 2;
+}
+>>> Line comment between initializer lines.
+class A {
+ A(param) : b = 1,
+ // comment
+ c = 2;
+}
+<<<
+class A {
+ A(param)
+ : b = 1,
+ // comment
+ c = 2;
+}
+>>> Line comment before semicolon body.
+class A {
+ A(param) : b = 1, c = 2 // comment
+ ;
+}
+<<<
+### It's weird to force the `=` to split, but a comment here is unusual.
+class A {
+ A(param)
+ : b = 1,
+ c =
+ 2 // comment
+ ;
+}
+>>> Line comment after semicolon body.
+class A {
+ A(param) : b = 1, c = 2; // comment
+}
+<<<
+class A {
+ A(param) : b = 1, c = 2; // comment
+}
+>>>
+class A {
+ A(param) : b = 1, c = 2; // longer comment
+}
+<<<
+class A {
+ A(param)
+ : b = 1,
+ c = 2; // longer comment
+}
+>>> Line comment before block body.
+class A {
+ A(param) : b = 1, c = 2 // comment
+ { body; }
+}
+<<<
+### Weird, but users rarely write this.
+class A {
+ A(param)
+ : b = 1,
+ c =
+ 2 // comment
+ {
+ body;
+ }
+}
+>>> Line comment after block body.
+class A {
+ A(param) : b = 1, c = 2 { body; } // comment
+}
+<<<
+class A {
+ A(param) : b = 1, c = 2 {
+ body;
+ } // comment
+}
\ No newline at end of file
diff --git a/test/declaration/constructor_initializer.unit b/test/declaration/constructor_initializer.unit
new file mode 100644
index 0000000..cdc6249
--- /dev/null
+++ b/test/declaration/constructor_initializer.unit
@@ -0,0 +1,244 @@
+40 columns |
+>>> Unnamed super constructor.
+class Foo {
+ Foo(a, b) : super ( a , other : b ) ;
+}
+<<<
+class Foo {
+ Foo(a, b) : super(a, other: b);
+}
+>>> Named super constructor.
+class Foo {
+ Foo(a, b) : super . name ( a , other : b ) ;
+}
+<<<
+class Foo {
+ Foo(a, b) : super.name(a, other: b);
+}
+>>> Split a single initializer.
+class MyClass {
+ MyClass(first, second) : super(first, second);
+}
+<<<
+class MyClass {
+ MyClass(first, second)
+ : super(first, second);
+}
+>>> No parameters and unsplit field initializers.
+class X {
+ var x, y;
+ X() : x = 1, y = 2;
+}
+<<<
+class X {
+ var x, y;
+ X() : x = 1, y = 2;
+}
+>>> No parameters and split field initializers.
+class MyClass {
+ MyClass() : first = "some value", second = "another",
+ third = "last";
+}
+<<<
+class MyClass {
+ MyClass()
+ : first = "some value",
+ second = "another",
+ third = "last";
+}
+>>> Unsplit parameters and initializers.
+class C {
+ C(a, b) : super(a), _b = b, _c = 1;
+
+ C.b(a, b) : super(a), _b = b, _c = 1 { body; }
+}
+<<<
+class C {
+ C(a, b) : super(a), _b = b, _c = 1;
+
+ C.b(a, b) : super(a), _b = b, _c = 1 {
+ body;
+ }
+}
+>>> If parameters split, initializers do too even if they would fit unsplit.
+class C {
+ C(String first, String second, String third) : super(a), _b = b, _c = 1;
+}
+<<<
+class C {
+ C(
+ String first,
+ String second,
+ String third,
+ ) : super(a),
+ _b = b,
+ _c = 1;
+}
+>>>
+class C {
+ C(String first, String second, String third) : super(a), _b = b, _c = 1
+ { body; }
+}
+<<<
+class C {
+ C(
+ String first,
+ String second,
+ String third,
+ ) : super(a),
+ _b = b,
+ _c = 1 {
+ body;
+ }
+}
+>>>
+class Foo {
+ Foo(int longArg1, int longArg2, int longArg3) : this._(longArg1);
+}
+<<<
+class Foo {
+ Foo(
+ int longArg1,
+ int longArg2,
+ int longArg3,
+ ) : this._(longArg1);
+}
+>>> Split at `:` and not in parameter list.
+class Foo {
+ Foo(int a, int b): super(aLongIdentifier);
+}
+<<<
+class Foo {
+ Foo(int a, int b)
+ : super(aLongIdentifier);
+}
+>>> Split each initializer but not parameters.
+class Foo {
+ Foo(int a, int b) : super(a), this.b = longExpression;
+}
+<<<
+class Foo {
+ Foo(int a, int b)
+ : super(a),
+ this.b = longExpression;
+}
+>>> Align initializers when split parameter list has no optional section.
+class Foo {
+ Foo(parameter1, parameter2, parameter3) : initializer1 = 1, initializer2 = 2;
+}
+<<<
+class Foo {
+ Foo(
+ parameter1,
+ parameter2,
+ parameter3,
+ ) : initializer1 = 1,
+ initializer2 = 2;
+}
+>>> Align initializers when split parameter list has optional section.
+class Foo {
+ Foo(parameter1, [parameter2, parameter3]) : initializer1 = 1, initializer2 = 2;
+}
+<<<
+class Foo {
+ Foo(
+ parameter1, [
+ parameter2,
+ parameter3,
+ ]) : initializer1 = 1,
+ initializer2 = 2;
+}
+>>> Align initializers when split parameter list has named section.
+class Foo {
+ Foo(parameter1, {parameter2, parameter3}) : initializer1 = 1, initializer2 = 2;
+}
+<<<
+class Foo {
+ Foo(
+ parameter1, {
+ parameter2,
+ parameter3,
+ }) : initializer1 = 1,
+ initializer2 = 2;
+}
+>>> Align initializers when unsplit parameter list has no optional section.
+class Foo {
+ Foo(parameter1) : initializer1 = 1, initializer2 = 2;
+}
+<<<
+class Foo {
+ Foo(parameter1)
+ : initializer1 = 1,
+ initializer2 = 2;
+}
+>>> Align initializers when unsplit parameter list has optional section.
+class Foo {
+ Foo([parameter1]) : initializer1 = 1, initializer2 = 2;
+}
+<<<
+class Foo {
+ Foo([parameter1])
+ : initializer1 = 1,
+ initializer2 = 2;
+}
+>>> Align initializers when unsplit parameter list has named section.
+class Foo {
+ Foo({parameter1}) : initializer1 = 1, initializer2 = 2;
+}
+<<<
+class Foo {
+ Foo({parameter1})
+ : initializer1 = 1,
+ initializer2 = 2;
+}
+>>> Wrap split initializers past the `:`.
+class Foo {
+ Foo(parameter)
+ : initializer = function(argument, argument),
+ initializer2 = function(argument, argument);
+}
+<<<
+class Foo {
+ Foo(parameter)
+ : initializer = function(
+ argument,
+ argument,
+ ),
+ initializer2 = function(
+ argument,
+ argument,
+ );
+}
+>>> Wrap split initializers with split optional parameter list past the `:`.
+class Foo {
+ Foo(parameter1, [parameter2, parameter3])
+ : initializer = function(argument, argument),
+ initializer2 = function(argument, argument);
+}
+<<<
+class Foo {
+ Foo(
+ parameter1, [
+ parameter2,
+ parameter3,
+ ]) : initializer = function(
+ argument,
+ argument,
+ ),
+ initializer2 = function(
+ argument,
+ argument,
+ );
+}
+>>> Allow block formatting the initialized value.
+class Foo {
+ Foo() : initializer =function(argument, arg);
+}
+<<<
+class Foo {
+ Foo()
+ : initializer = function(
+ argument,
+ arg,
+ );
+}
\ No newline at end of file
diff --git a/test/declaration/constructor_parameter.unit b/test/declaration/constructor_parameter.unit
new file mode 100644
index 0000000..63f9e03
--- /dev/null
+++ b/test/declaration/constructor_parameter.unit
@@ -0,0 +1,83 @@
+40 columns |
+>>> Initializing formal.
+class Foo {
+ Foo(this . a, int this . b, final this . c);
+ Foo.optional([ this . a, int this . b = 123 ]);
+ Foo.named({ required this . a, int this . b = 123 });
+}
+<<<
+class Foo {
+ Foo(this.a, int this.b, final this.c);
+ Foo.optional([
+ this.a,
+ int this.b = 123,
+ ]);
+ Foo.named({
+ required this.a,
+ int this.b = 123,
+ });
+}
+>>> Function-typed initializing formal.
+class Foo {
+ Foo.function(this . a ( int x ), this . b ( ) ? );
+ Foo.optional([ int this . c() , this . d ( ) = x ]);
+ Foo.named({ required int this . e() , this . f ( ) = x });
+}
+<<<
+class Foo {
+ Foo.function(
+ this.a(int x),
+ this.b()?,
+ );
+ Foo.optional([
+ int this.c(),
+ this.d() = x,
+ ]);
+ Foo.named({
+ required int this.e(),
+ this.f() = x,
+ });
+}
+>>> Super parameter.
+class Foo {
+ Foo(super . a, int super . b, final super . c);
+ Foo.optional([ super . a, int super . b = 123 ]);
+ Foo.named({ required super . a, int super . b = 123 });
+}
+<<<
+class Foo {
+ Foo(
+ super.a,
+ int super.b,
+ final super.c,
+ );
+ Foo.optional([
+ super.a,
+ int super.b = 123,
+ ]);
+ Foo.named({
+ required super.a,
+ int super.b = 123,
+ });
+}
+>>> Function-typed super parameter.
+class Foo {
+ Foo.function(super . a ( int x ), super . b ( ) ? );
+ Foo.optional([ int super . c() , super . d ( ) = x ]);
+ Foo.named({ required int super . e() , super . f ( ) = x });
+}
+<<<
+class Foo {
+ Foo.function(
+ super.a(int x),
+ super.b()?,
+ );
+ Foo.optional([
+ int super.c(),
+ super.d() = x,
+ ]);
+ Foo.named({
+ required int super.e(),
+ super.f() = x,
+ });
+}
\ No newline at end of file
diff --git a/test/declaration/external.unit b/test/declaration/external.unit
index 80199b5..2f8d5cc 100644
--- a/test/declaration/external.unit
+++ b/test/declaration/external.unit
@@ -71,6 +71,20 @@
int value,
);
}
+>>> Constructor.
+class A {
+ external A();
+ external const A.constant();
+ external factory A.fact();
+ external const factory A.constantFact();
+}
+<<<
+class A {
+ external A();
+ external const A.constant();
+ external factory A.fact();
+ external const factory A.constantFact();
+}
>>> Don't split after `external`.
class Foo {
external var soMuchSoVeryLongFieldNameHere;
