Skip to content

Commit

Permalink
Format constructor declarations.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
munificent committed Dec 7, 2023
1 parent 6249194 commit 8cf67ef
Show file tree
Hide file tree
Showing 15 changed files with 1,176 additions and 95 deletions.
44 changes: 32 additions & 12 deletions lib/src/back_end/code_writer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,16 @@ class CodeWriter {
/// 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.
///
/// This can occur if:
///
/// If true, it means the solution is invalid.
bool _containsInvalidNewline = false;
/// - 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.
///
Expand Down Expand Up @@ -126,7 +132,7 @@ class CodeWriter {

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.
Expand All @@ -138,7 +144,7 @@ class CodeWriter {
/// 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.
Expand Down Expand Up @@ -228,19 +234,33 @@ class CodeWriter {

/// 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;
if (isUnsolved) _currentUnsolvedPieces.add(piece);

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
Expand Down Expand Up @@ -316,7 +336,7 @@ class CodeWriter {
// 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) {
Expand Down
179 changes: 149 additions & 30 deletions lib/src/front_end/ast_node_visitor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ import '../piece/adjacent.dart';
import '../piece/assign.dart';
import '../piece/block.dart';
import '../piece/chain.dart';
import '../piece/constructor.dart';
import '../piece/for.dart';
import '../piece/function.dart';
import '../piece/if.dart';
import '../piece/infix.dart';
import '../piece/list.dart';
Expand Down Expand Up @@ -130,7 +132,17 @@ class AstNodeVisitor extends ThrowingAstVisitor<Piece> with PieceFactory {

@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
Expand Down Expand Up @@ -343,18 +355,85 @@ class AstNodeVisitor extends ThrowingAstVisitor<Piece> with PieceFactory {

@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
Expand Down Expand Up @@ -550,7 +629,30 @@ class AstNodeVisitor extends ThrowingAstVisitor<Piece> with PieceFactory {

@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
Expand Down Expand Up @@ -736,7 +838,7 @@ class AstNodeVisitor extends ThrowingAstVisitor<Piece> with PieceFactory {
@override
Piece visitFunctionDeclaration(FunctionDeclaration node) {
return createFunction(
externalKeyword: node.externalKeyword,
modifiers: [node.externalKeyword],
returnType: node.returnType,
propertyKeyword: node.propertyKeyword,
name: node.name,
Expand Down Expand Up @@ -974,11 +1076,9 @@ class AstNodeVisitor extends ThrowingAstVisitor<Piece> with PieceFactory {
@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,
Expand Down Expand Up @@ -1166,7 +1266,12 @@ class AstNodeVisitor extends ThrowingAstVisitor<Piece> with PieceFactory {
@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
Expand Down Expand Up @@ -1260,22 +1365,8 @@ class AstNodeVisitor extends ThrowingAstVisitor<Piece> with PieceFactory {

@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
Expand All @@ -1300,7 +1391,12 @@ class AstNodeVisitor extends ThrowingAstVisitor<Piece> with PieceFactory {

@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
Expand All @@ -1310,7 +1406,30 @@ class AstNodeVisitor extends ThrowingAstVisitor<Piece> with PieceFactory {

@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
Expand Down
7 changes: 4 additions & 3 deletions lib/src/front_end/delimited_list_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,10 @@ class DelimitedListBuilder {
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.
Expand Down
Loading

0 comments on commit 8cf67ef

Please sign in to comment.