Permalink
Browse files

Always split collections and argument lists with trailing commas.

This preserves the users intention that a trailing comma means, "make
this easy to edit each element on its own line". It also means argument
lists with trailing commas are formatted similar to collections:

fn(
  arg,
  arg,
);

R=nweiz@google.com

Review URL: https://codereview.chromium.org//2181743002 .
  • Loading branch information...
munificent committed Jul 26, 2016
1 parent 6ecb3e4 commit ad69e8a8a31776a33bbcb0cd9bb9c0afeeede09e
View
@@ -10,6 +10,7 @@
* Remove newlines after script tags (#513).
* Indent initializers in multiple variable declarations.
* Avoid passing an invalid Windows file URI to analyzer.
* Always split comma-separated sequences that contain a trailing comma.
# 0.2.6
@@ -145,6 +145,15 @@ class SourceVisitor implements AstVisitor {
return;
}
// If the argument list has a trailing comma, format it like a collection
// literal where each argument goes on its own line, they are indented +2,
// and the ")" ends up on its own line.
if (node.arguments.last.endToken.next.type == TokenType.COMMA) {
_visitCollectionLiteral(
null, node.leftParenthesis, node.arguments, node.rightParenthesis);
return;
}
new ArgumentListVisitor(this, node).visit();
}
@@ -1888,11 +1897,16 @@ class SourceVisitor implements AstVisitor {
/// Visits the collection literal [node] whose body starts with [leftBracket],
/// ends with [rightBracket] and contains [elements].
///
/// This is also used for argument lists with a trailing comma which are
/// considered "collection-like". In that case, [node] is `null`.
void _visitCollectionLiteral(TypedLiteral node, Token leftBracket,
Iterable<AstNode> elements, Token rightBracket,
[int cost]) {
modifier(node.constKeyword);
visit(node.typeArguments);
if (node != null) {
modifier(node.constKeyword);
visit(node.typeArguments);
}
// Don't allow splitting in an empty collection.
if (elements.isEmpty && rightBracket.precedingComments == null) {
@@ -1957,7 +1971,9 @@ class SourceVisitor implements AstVisitor {
visit(element);
// The comma after the element.
if (element.endToken.next.lexeme == ",") token(element.endToken.next);
if (element.endToken.next.type == TokenType.COMMA) {
token(element.endToken.next);
}
builder.unnest();
}
@@ -1967,6 +1983,12 @@ class SourceVisitor implements AstVisitor {
// If there is a collection inside this one, it forces this one to split.
var force = _collectionSplits.removeLast();
// If the collection has a trailing comma, the user must want it to split.
if (elements.isNotEmpty &&
elements.last.endToken.next.type == TokenType.COMMA) {
force = true;
}
_endLiteralBody(rightBracket, ignoredRule: rule, forceSplit: force);
}
View
@@ -25,7 +25,9 @@ void main() {
new FormatterOptions(OutputReporter.overwrite, followLinks: true);
test('handles directory ending in ".dart"', () {
d.dir('code.dart', [d.file('a.dart', unformattedSource),]).create();
d.dir('code.dart', [
d.file('a.dart', unformattedSource),
]).create();
schedule(() {
var dir = new Directory(d.defaultRoot);
@@ -97,9 +99,13 @@ void main() {
});
test("doesn't follow directory symlinks by default", () {
d.dir('code', [d.file('a.dart', unformattedSource),]).create();
d.dir('code', [
d.file('a.dart', unformattedSource),
]).create();
d.dir('target_dir', [d.file('b.dart', unformattedSource),]).create();
d.dir('target_dir', [
d.file('b.dart', unformattedSource),
]).create();
schedule(() {
// Create a link to the target directory in the code directory.
@@ -114,14 +120,20 @@ void main() {
d.dir('code', [
d.file('a.dart', formattedSource),
d.dir('linked_dir', [d.file('b.dart', unformattedSource),])
d.dir('linked_dir', [
d.file('b.dart', unformattedSource),
])
]).validate();
});
test("follows directory symlinks when 'followLinks' is true", () {
d.dir('code', [d.file('a.dart', unformattedSource),]).create();
d.dir('code', [
d.file('a.dart', unformattedSource),
]).create();
d.dir('target_dir', [d.file('b.dart', unformattedSource),]).create();
d.dir('target_dir', [
d.file('b.dart', unformattedSource),
]).create();
schedule(() {
// Create a link to the target directory in the code directory.
@@ -136,7 +148,9 @@ void main() {
d.dir('code', [
d.file('a.dart', formattedSource),
d.dir('linked_dir', [d.file('b.dart', formattedSource),])
d.dir('linked_dir', [
d.file('b.dart', formattedSource),
])
]).validate();
});
@@ -156,8 +170,9 @@ void main() {
processDirectory(overwriteOptions, dir);
}, 'Run formatter.');
d.dir(
'code', [d.file('linked_file.dart', unformattedSource),]).validate();
d.dir('code', [
d.file('linked_file.dart', unformattedSource),
]).validate();
});
test("follows file symlinks when 'followLinks' is true", () {
@@ -175,7 +190,9 @@ void main() {
processDirectory(followOptions, dir);
}, 'running formatter');
d.dir('code', [d.file('linked_file.dart', formattedSource),]).validate();
d.dir('code', [
d.file('linked_file.dart', formattedSource),
]).validate();
});
}
}
@@ -14,7 +14,9 @@ Foo barBaz() => new Quux({
}, {
'toil': {
'trouble': {
'fireBurn': {'cauldronBubble': EYE_OF_NEWT,}
'fireBurn': {
'cauldronBubble': EYE_OF_NEWT,
}
}
}
})
@@ -59,7 +59,9 @@
d.dir('packages', [
d.dir('foo', [
d.file('foo.txt', 'foo'),
d.dir('sub', [d.file('bar.txt', 'bar'),]),
d.dir('sub', [
d.file('bar.txt', 'bar'),
]),
])
])
]),
@@ -68,7 +70,9 @@
d.dir('packages', [
d.dir('foo', [
d.file('foo.txt', 'foo'),
d.dir('sub', [d.file('bar.txt', 'bar'),]),
d.dir('sub', [
d.file('bar.txt', 'bar'),
]),
])
]),
d.dir("sub", [
@@ -201,13 +201,17 @@ function(
;
});
>>> trailing comma
functionn(argument,argument ,argument , );
fn(argument,argument ,argument , );
<<<
functionn(
argument, argument, argument,);
fn(
argument,
argument,
argument,
);
>>> trailing comma in named argument list
function(named: argument,another:argument, );
fn(named: argument,another:argument, );
<<<
function(
named: argument,
another: argument,);
fn(
named: argument,
another: argument,
);
@@ -73,11 +73,13 @@
[1,2,3,4];
<<<
[1, 2, 3, 4];
>>> dangling comma
>>> trailing comma forces split
[1 , ];
<<<
[1,];
>>> dangling comma multiline
[
1,
];
>>> trailing comma multiline
[first, second, third, fourth, fifth, sixth , ];
<<<
[
View
@@ -74,11 +74,13 @@ var map = const {
"foo": "bar",
"fuz": null
};
>>> dangling comma
>>> trailing comma forces split
var map = {"foo": "bar" , };
<<<
var map = {"foo": "bar",};
>>> dangling comma multiline
var map = {
"foo": "bar",
};
>>> trailing comma multiline
var map = {"foo": "bar", "fuzzy": null , };
<<<
var map = {
@@ -118,12 +118,20 @@ variableName ??= argument;
>>> trailing comma in single argument list
function(argument , );
<<<
function(argument,);
function(
argument,
);
>>> trailing comma in argument list
function(argument,argument , );
<<<
function(argument, argument,);
function(
argument,
argument,
);
>>> trailing comma in named argument list
function(named: arg,another:arg, );
<<<
function(named: arg, another: arg,);
function(
named: arg,
another: arg,
);
@@ -25,17 +25,15 @@ void main() {
var exception;
try {
return new FormatResult(
code: new DartFormatter().format(source));
return new FormatResult(code: new DartFormatter().format(source));
} on FormatterException catch (err) {
// Couldn't parse it as a compilation unit.
exception = err;
}
// Maybe it's a statement.
try {
return new FormatResult(
code: formatter.formatStatement(source));
return new FormatResult(code: formatter.formatStatement(source));
} on FormatterException catch (err) {
// There is an error when parsing it both as a compilation unit and a
// statement, so we aren't sure which one the user intended. As a

0 comments on commit ad69e8a

Please sign in to comment.