Support generic methods. Fix #556.
R=kevmoo@google.com
Review URL: https://codereview.chromium.org//2526633002 .
diff --git a/CHANGELOG.md b/CHANGELOG.md
index dfaf086..3bc7c37 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,6 +3,7 @@
* Add support for assert() in constructor initializers.
* Correctly indent the right-hand side of `is` and `as` expressions.
* Avoid splitting in index operators when possible.
+* Support generic methods (#556).
# 0.2.11+1
diff --git a/lib/src/argument_list_visitor.dart b/lib/src/argument_list_visitor.dart
index 843d757..38bc6eb 100644
--- a/lib/src/argument_list_visitor.dart
+++ b/lib/src/argument_list_visitor.dart
@@ -153,9 +153,6 @@
// split before it, so try not to.
if (_isSingle) _visitor.builder.startSpan();
- // Nest around the parentheses in case there are comments before or after
- // them.
- _visitor.builder.nestExpression();
_visitor.builder.startSpan();
_visitor.token(_leftParenthesis);
@@ -193,8 +190,6 @@
_visitor.token(_rightParenthesis);
- _visitor.builder.unnest();
-
if (_isSingle) _visitor.builder.endSpan();
}
diff --git a/lib/src/call_chain_visitor.dart b/lib/src/call_chain_visitor.dart
index 29c04fb..e1fec05 100644
--- a/lib/src/call_chain_visitor.dart
+++ b/lib/src/call_chain_visitor.dart
@@ -317,7 +317,7 @@
// If the expression ends in an argument list, base the splitting on the
// last argument.
- var argumentList;
+ ArgumentList argumentList;
if (expression is MethodInvocation) {
argumentList = expression.argumentList;
} else if (expression is InstanceCreationExpression) {
@@ -331,7 +331,9 @@
if (argumentList.arguments.isEmpty) return true;
var argument = argumentList.arguments.last;
- if (argument is NamedExpression) argument = argument.expression;
+ if (argument is NamedExpression) {
+ argument = (argument as NamedExpression).expression;
+ }
// TODO(rnystrom): This logic is similar (but not identical) to
// ArgumentListVisitor.hasBlockArguments. They overlap conceptually and
@@ -399,7 +401,10 @@
_endSpan();
}
- _visitor.visit(invocation.argumentList);
+ _visitor.builder.nestExpression();
+ _visitor.visit(invocation.typeArguments);
+ _visitor.visitArgumentList(invocation.argumentList, nestExpression: false);
+ _visitor.builder.unnest();
}
void _writeBlockCall(MethodInvocation invocation) {
diff --git a/lib/src/dart_formatter.dart b/lib/src/dart_formatter.dart
index 3269dbe..6c298b1 100644
--- a/lib/src/dart_formatter.dart
+++ b/lib/src/dart_formatter.dart
@@ -102,6 +102,7 @@
// Parse it.
var parser = new Parser(stringSource, errorListener);
+ parser.parseGenericMethods = true;
AstNode node;
if (source.isCompilationUnit) {
diff --git a/lib/src/source_visitor.dart b/lib/src/source_visitor.dart
index 2bc844b..2d2c257 100644
--- a/lib/src/source_visitor.dart
+++ b/lib/src/source_visitor.dart
@@ -133,7 +133,7 @@
/// 4. Split between one or more positional arguments, trying to keep as many
/// on earlier lines as possible.
/// 5. Split the named arguments each onto their own line.
- visitArgumentList(ArgumentList node) {
+ visitArgumentList(ArgumentList node, {bool nestExpression: true}) {
// Corner case: handle empty argument lists.
if (node.arguments.isEmpty) {
token(node.leftParenthesis);
@@ -154,7 +154,9 @@
return;
}
+ if (nestExpression) builder.nestExpression();
new ArgumentListVisitor(this, node).visit();
+ if (nestExpression) builder.unnest();
}
visitAsExpression(AsExpression node) {
@@ -179,9 +181,11 @@
var arguments = <Expression>[node.condition];
if (node.message != null) arguments.add(node.message);
+ builder.nestExpression();
var visitor = new ArgumentListVisitor.forArguments(
this, node.leftParenthesis, node.rightParenthesis, arguments);
visitor.visit();
+ builder.unnest();
});
}
@@ -1220,10 +1224,18 @@
token(node.keyword);
space();
builder.startSpan(Cost.constructorName);
+
+ // Start the expression nesting for the argument list here, in case this
+ // is a generic constructor with type arguments. If it is, we need the type
+ // arguments to be nested too so they get indented past the arguments.
+ builder.nestExpression();
visit(node.constructorName);
+
builder.endSpan();
- visit(node.argumentList);
+ visitArgumentList(node.argumentList, nestExpression: false);
builder.endSpan();
+
+ builder.unnest();
}
visitIntegerLiteral(IntegerLiteral node) {
@@ -1315,8 +1327,22 @@
// This will be non-null for cascade sections.
token(node.operator);
- token(node.methodName.token);
- visit(node.argumentList);
+ visit(node.methodName);
+
+ // TODO(rnystrom): Currently, there are no constraints between a generic
+ // method's type arguments and arguments. That can lead to some funny
+ // splitting like:
+ //
+ // method<VeryLongType,
+ // AnotherTypeArgument>(argument,
+ // argument, argument, argument);
+ //
+ // The indentation is fine, but splitting in the middle of each argument
+ // list looks kind of strange. If this ends up happening in real world
+ // code, consider putting a constraint between them.
+
+ visit(node.typeArguments);
+ visitArgumentList(node.argumentList, nestExpression: false);
builder.unnest();
builder.endSpan();
diff --git a/pubspec.lock b/pubspec.lock
index 6e79c83..bd701ed 100644
--- a/pubspec.lock
+++ b/pubspec.lock
@@ -48,13 +48,13 @@
name: charcode
url: "https://pub.dartlang.org"
source: hosted
- version: "1.1.0"
+ version: "1.1.1"
collection:
description:
name: collection
url: "https://pub.dartlang.org"
source: hosted
- version: "1.11.0"
+ version: "1.12.0"
convert:
description:
name: convert
@@ -72,7 +72,7 @@
name: csslib
url: "https://pub.dartlang.org"
source: hosted
- version: "0.13.2+1"
+ version: "0.13.2+2"
glob:
description:
name: glob
@@ -192,13 +192,13 @@
name: pub_semver
url: "https://pub.dartlang.org"
source: hosted
- version: "1.3.0"
+ version: "1.3.2"
scheduled_test:
description:
name: scheduled_test
url: "https://pub.dartlang.org"
source: hosted
- version: "0.12.6"
+ version: "0.12.6+1"
shelf:
description:
name: shelf
@@ -246,7 +246,7 @@
name: stack_trace
url: "https://pub.dartlang.org"
source: hosted
- version: "1.6.8"
+ version: "1.7.0"
stream_channel:
description:
name: stream_channel
@@ -270,7 +270,7 @@
name: test
url: "https://pub.dartlang.org"
source: hosted
- version: "0.12.15+12"
+ version: "0.12.16"
typed_data:
description:
name: typed_data
diff --git a/test/splitting/generic_method_arguments.stmt b/test/splitting/generic_method_arguments.stmt
new file mode 100644
index 0000000..6abf870
--- /dev/null
+++ b/test/splitting/generic_method_arguments.stmt
@@ -0,0 +1,71 @@
+40 columns |
+>>> all fit on one line
+method<A,B,C,D>();
+<<<
+method<A, B, C, D>();
+>>> prefer to split between args even when they all fit on next line
+lengthyMethodName<First, Second, Third>();
+<<<
+lengthyMethodName<First, Second,
+ Third>();
+>>> split before first if needed
+lengthyMethodName<FirstTypeArgumentIsLong, Second>();
+<<<
+lengthyMethodName<
+ FirstTypeArgumentIsLong, Second>();
+>>> split in middle if fit in two lines
+lengthyMethodName<First, Second, Third, Fourth, Fifth, Sixth, Seventh>();
+<<<
+lengthyMethodName<First, Second, Third,
+ Fourth, Fifth, Sixth, Seventh>();
+>>> split one per line if they don't fit in two lines
+lengthyMethodName<First, Second, Third, Fourth, Fifth, Sixth, Seventh, Eighth>();
+<<<
+lengthyMethodName<
+ First,
+ Second,
+ Third,
+ Fourth,
+ Fifth,
+ Sixth,
+ Seventh,
+ Eighth>();
+>>> prefers to not split at type arguments
+lengthyMethodName<
+ TypeArgument>(valueArgument);
+<<<
+lengthyMethodName<TypeArgument>(
+ valueArgument);
+>>> split both type and value arguments
+lengthyMethodName<First, Second, Third, Fourth, Fifth, Sixth, Seventh, Eighth>(first, second, third, fourth, fifth, sixth, seventh, eighth);
+<<<
+lengthyMethodName<
+ First,
+ Second,
+ Third,
+ Fourth,
+ Fifth,
+ Sixth,
+ Seventh,
+ Eighth>(
+ first,
+ second,
+ third,
+ fourth,
+ fifth,
+ sixth,
+ seventh,
+ eighth);
+>>> in method chain
+receiver.method<First, Second, Third, Fourth, Fifth>
+ (first, second, third, fourth, fifth)
+.method<First, Second, Third, Fourth, Fifth>
+ (first, second, third, fourth, fifth);
+<<<
+receiver
+ .method<First, Second, Third,
+ Fourth, Fifth>(first,
+ second, third, fourth, fifth)
+ .method<First, Second, Third,
+ Fourth, Fifth>(first,
+ second, third, fourth, fifth);
\ No newline at end of file
diff --git a/test/splitting/type_arguments.stmt b/test/splitting/type_arguments.stmt
index f89429f..fadd0d8 100644
--- a/test/splitting/type_arguments.stmt
+++ b/test/splitting/type_arguments.stmt
@@ -35,4 +35,24 @@
TypeArgument>(valueArgument);
<<<
new SomeClass<TypeArgument>(
- valueArgument);
\ No newline at end of file
+ valueArgument);
+>>> split both type and value arguments
+new SomeClass<First, Second, Third, Fourth, Fifth, Sixth, Seventh, Eighth>(first, second, third, fourth, fifth, sixth, seventh, eighth);
+<<<
+new SomeClass<
+ First,
+ Second,
+ Third,
+ Fourth,
+ Fifth,
+ Sixth,
+ Seventh,
+ Eighth>(
+ first,
+ second,
+ third,
+ fourth,
+ fifth,
+ sixth,
+ seventh,
+ eighth);
\ No newline at end of file
diff --git a/test/whitespace/expressions.stmt b/test/whitespace/expressions.stmt
index 310e7bd..ede2fe7 100644
--- a/test/whitespace/expressions.stmt
+++ b/test/whitespace/expressions.stmt
@@ -134,4 +134,8 @@
function(
named: arg,
another: arg,
-);
\ No newline at end of file
+);
+>>> generic method call
+method <int,String , bool> ();
+<<<
+method<int, String, bool>();
\ No newline at end of file
