Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Format instance creation expressions (constructor calls). #1307

Merged
merged 3 commits into from
Nov 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/src/ast_extensions.dart
Original file line number Diff line number Diff line change
Expand Up @@ -111,13 +111,13 @@ extension ExpressionExtensions on Expression {
/// ```
bool get isDelimited => switch (this) {
FunctionExpression() => true,
InstanceCreationExpression() => true,
ListLiteral() => true,
MethodInvocation() => true,
ParenthesizedExpression(:var expression) => expression.isDelimited,
RecordLiteral() => true,
SetOrMapLiteral() => true,
SwitchExpression() => true,
// TODO(tall): Instance creation expressions (`new` and `const`).
_ => false,
};

Expand Down
47 changes: 40 additions & 7 deletions lib/src/front_end/ast_node_visitor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import 'package:analyzer/source/line_info.dart';
import '../constants.dart';
import '../dart_formatter.dart';
import '../piece/block.dart';
import '../piece/chain.dart';
import '../piece/do_while.dart';
import '../piece/for.dart';
import '../piece/if.dart';
Expand Down Expand Up @@ -279,7 +280,7 @@ class AstNodeVisitor extends ThrowingAstVisitor<void>

@override
void visitConstructorName(ConstructorName node) {
throw UnimplementedError();
assert(false, 'This node is handled by visitInstanceCreationExpression().');
}

@override
Expand Down Expand Up @@ -618,7 +619,43 @@ class AstNodeVisitor extends ThrowingAstVisitor<void>

@override
void visitInstanceCreationExpression(InstanceCreationExpression node) {
throw UnimplementedError();
token(node.keyword, after: space);

// 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 constructor = node.constructorName;
if (constructor.type.importPrefix case var importPrefix?) {
token(importPrefix.name);
operations.add(pieces.split());
token(importPrefix.period);
}

// The name of the type being constructed.
var type = constructor.type;
token(type.name2);
visit(type.typeArguments);
token(type.question);

// If this is a named constructor call, the name.
if (constructor.name != null) {
operations.add(pieces.split());
token(constructor.period);
visit(constructor.name);
}

finishCall(node.argumentList);

// If there was a prefix or constructor name, then make a splittable piece.
if (operations.isNotEmpty) {
operations.add(pieces.take());
pieces.give(ChainPiece(operations));
}
}

@override
Expand Down Expand Up @@ -729,11 +766,7 @@ class AstNodeVisitor extends ThrowingAstVisitor<void>

visit(node.methodName);
visit(node.typeArguments);

createList(
leftBracket: node.argumentList.leftParenthesis,
node.argumentList.arguments,
rightBracket: node.argumentList.rightParenthesis);
finishCall(node.argumentList);
}

@override
Expand Down
9 changes: 9 additions & 0 deletions lib/src/front_end/piece_factory.dart
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,15 @@ mixin PieceFactory implements CommentWriter {
isValueDelimited: rightHandSide.isDelimited));
}

/// Writes the argument list part of a constructor, function, or method call
/// after the name has been written.
void finishCall(ArgumentList argumentList) {
createList(
leftBracket: argumentList.leftParenthesis,
argumentList.arguments,
rightBracket: argumentList.rightParenthesis);
}

/// Writes the condition and updaters parts of a [ForParts] after the
/// subclass's initializer clause has been written.
void finishForParts(ForParts forLoopParts, DelimitedListBuilder partsList) {
Expand Down
52 changes: 52 additions & 0 deletions lib/src/piece/chain.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// 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';

// TODO(tall): This will probably become more elaborate when full method chains
// with interesting argument lists are supported. Right now, it's just the
// basics needed for instance creation expressions which may have method-like
// `.` in them.

/// A dotted series of property access or method calls, like:
///
/// ```
/// target.getter.method().another.method();
/// ```
///
/// This piece handles splitting before the `.`.
class ChainPiece extends Piece {
/// The series of operations.
///
/// The first piece in this is the target, and the rest are operations.
final List<Piece> _operations;

ChainPiece(this._operations);

@override
List<State> get additionalStates => const [State.split];

@override
void format(CodeWriter writer, State state) {
if (state == State.unsplit) {
writer.setAllowNewlines(false);
} else {
writer.setNesting(Indent.expression);
}

for (var i = 0; i < _operations.length; i++) {
if (i > 0) writer.splitIf(state == State.split, space: false);
writer.format(_operations[i]);
}
}

@override
void forEachChild(void Function(Piece piece) callback) {
_operations.forEach(callback);
}

@override
String toString() => 'Chain';
}
50 changes: 50 additions & 0 deletions test/invocation/constructor.stmt
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
40 columns |
### Constructor invocations are handled identically to function calls, so just
### test the basics and make sure that we handle the keywords correctly.
>>> Empty argument list.
new Foo ( ) ;
<<<
new Foo();
>>> Inline arguments.
new SomeType ( argument , another ) ;
<<<
new SomeType(argument, another);
>>> Split argument list.
const SomeType ( argument , another , third ) ;
<<<
const SomeType(
argument,
another,
third,
);
>>> With type arguments.
new Map < int , String > ( 1 , 2 , 3 );
<<<
new Map<int, String>(1, 2, 3);
>>> Named constructor.
new Thing . name ( argument ) ;
<<<
new Thing.name(argument);
>>> Named constructor on class with type arguments.
const List < int > . filled ( 1 , 2 );
<<<
const List<int>.filled(1, 2);
>>> Prefixed.
new prefix . TypeName ( argument ) ;
<<<
new prefix.TypeName(argument);
>>> Prefix named constructor.
const prefix . Thing . name ( argument ) ;
<<<
const prefix.Thing.name(argument);
>>> Split at name.
new VeryLongClassName.veryLongNamedConstructor();
<<<
new VeryLongClassName
.veryLongNamedConstructor();
>>> Split at name on prefixed named constructor.
new prefix.VeryLongClassName.veryLongNamedConstructor();
<<<
new prefix
.VeryLongClassName
.veryLongNamedConstructor();
14 changes: 14 additions & 0 deletions test/variable/local.stmt
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,20 @@ var longVariableName = veryLongFunctionName(argument);
<<<
var longVariableName =
veryLongFunctionName(argument);
>>> Prefer block-like splitting for constructor calls.
var variableName = new Thing(argument, argument);
<<<
var variableName = new Thing(
argument,
argument,
);
>>> Prefer block-like splitting for const constructor calls.
var variableName = const Thing(argument, argument);
<<<
var variableName = const Thing(
argument,
argument,
);
>>> Indent block if function name doesn't fit and arguments split.
var longVariableName = veryLongFunctionName(argument, another);
<<<
Expand Down