Formatting support for named arguments anywhere. (#1094)
* Formatting support for named arguments anywhere.
I opted to add support for this while being minimally invasive to the
existing style and formatting. All existing code which does not have any
positional arguments after named ones retains its previous formatting.
For new argument lists that use positional arguments after named ones,
I try to mostly follow the existing style rules even though they can be
fairly complex. In particular, it will be pretty aggressive about
applying block-style formatting to function literals inside argument
lists even with named args anywhere. I think that's important to support
the main use case I know of for the feature which is trailing positional
closures like:
function(named: 1, another: 2, () {
block...
});
In argument lists using named args anywhere that don't have block
functions, it treats all arguments like named ones. That provides nice
simple formatting like:
function(
argument1,
named: argument2,
argument3,
another: argument4);
I think that does a good job of highlighting which arguments are named,
which is what we want.
Fix #1072.
* Rewrite doc comments.diff --git a/CHANGELOG.md b/CHANGELOG.md
index ae28abd..a73d161 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,7 @@
+# 2.2.2
+
+* Format named arguments anywhere (#1072).
+
# 2.2.1
* Require `package:analyzer` version `2.6.0`.
diff --git a/lib/src/argument_list_visitor.dart b/lib/src/argument_list_visitor.dart
index 9f4c6f2..d839600 100644
--- a/lib/src/argument_list_visitor.dart
+++ b/lib/src/argument_list_visitor.dart
@@ -63,87 +63,9 @@
Token leftParenthesis,
Token rightParenthesis,
List<Expression> arguments) {
- // Look for a single contiguous range of block function arguments.
- int? functionsStart;
- int? functionsEnd;
+ var functionRange = _contiguousFunctions(arguments);
- for (var i = 0; i < arguments.length; i++) {
- var argument = arguments[i];
- if (_isBlockFunction(argument)) {
- functionsStart ??= i;
-
- // The functions must be one contiguous section.
- if (functionsEnd != null && functionsEnd != i) {
- functionsStart = null;
- functionsEnd = null;
- break;
- }
-
- functionsEnd = i + 1;
- }
- }
-
- // Edge case: If all of the arguments are named, but they aren't all
- // functions, then don't handle the functions specially. A function with a
- // bunch of named arguments tends to look best when they are all lined up,
- // even the function ones (unless they are all functions).
- //
- // Prefers:
- //
- // function(
- // named: () {
- // something();
- // },
- // another: argument);
- //
- // Over:
- //
- // function(named: () {
- // something();
- // },
- // another: argument);
- if (functionsStart != null &&
- arguments[0] is NamedExpression &&
- (functionsStart > 0 || functionsEnd! < arguments.length)) {
- functionsStart = null;
- }
-
- // Edge case: If all of the function arguments are named and there are
- // other named arguments that are "=>" functions, then don't treat the
- // block-bodied functions specially. In a mixture of the two function
- // styles, it looks cleaner to treat them all like normal expressions so
- // that the named arguments line up.
- if (functionsStart != null &&
- arguments[functionsStart] is NamedExpression) {
- bool isArrow(NamedExpression named) {
- var expression = named.expression;
-
- if (expression is FunctionExpression) {
- return expression.body is ExpressionFunctionBody;
- }
-
- return false;
- }
-
- for (var i = 0; i < functionsStart!; i++) {
- var argument = arguments[i];
- if (argument is! NamedExpression) continue;
-
- if (isArrow(argument)) {
- functionsStart = null;
- break;
- }
- }
-
- for (var i = functionsEnd!; i < arguments.length; i++) {
- if (isArrow(arguments[i] as NamedExpression)) {
- functionsStart = null;
- break;
- }
- }
- }
-
- if (functionsStart == null) {
+ if (functionRange == null) {
// No functions, so there is just a single argument list.
return ArgumentListVisitor._(visitor, leftParenthesis, rightParenthesis,
arguments, ArgumentSublist(arguments, arguments), null, null);
@@ -151,9 +73,9 @@
// Split the arguments into two independent argument lists with the
// functions in the middle.
- var argumentsBefore = arguments.take(functionsStart).toList();
- var functions = arguments.sublist(functionsStart, functionsEnd);
- var argumentsAfter = arguments.skip(functionsEnd!).toList();
+ var argumentsBefore = arguments.take(functionRange[0]).toList();
+ var functions = arguments.sublist(functionRange[0], functionRange[1]);
+ var argumentsAfter = arguments.skip(functionRange[1]).toList();
return ArgumentListVisitor._(
visitor,
@@ -224,6 +146,83 @@
if (_isSingle) _visitor.builder.endSpan();
}
+ /// Look for a single contiguous range of block function [arguments] that
+ /// should receive special formatting.
+ ///
+ /// Returns a list of (start, end] indexes if found, otherwise returns `null`.
+ static List<int>? _contiguousFunctions(List<Expression> arguments) {
+ int? functionsStart;
+ var functionsEnd = -1;
+
+ // Find the range of block function arguments, if any.
+ for (var i = 0; i < arguments.length; i++) {
+ var argument = arguments[i];
+ if (_isBlockFunction(argument)) {
+ functionsStart ??= i;
+
+ // The functions must be one contiguous section.
+ if (functionsEnd != -1 && functionsEnd != i) return null;
+
+ functionsEnd = i + 1;
+ }
+ }
+
+ if (functionsStart == null) return null;
+
+ // Edge case: If all of the arguments are named, but they aren't all
+ // functions, then don't handle the functions specially. A function with a
+ // bunch of named arguments tends to look best when they are all lined up,
+ // even the function ones (unless they are all functions).
+ //
+ // Prefers:
+ //
+ // function(
+ // named: () {
+ // something();
+ // },
+ // another: argument);
+ //
+ // Over:
+ //
+ // function(named: () {
+ // something();
+ // },
+ // another: argument);
+ if (_isAllNamed(arguments) &&
+ (functionsStart > 0 || functionsEnd < arguments.length)) {
+ return null;
+ }
+
+ // Edge case: If all of the function arguments are named and there are
+ // other named arguments that are "=>" functions, then don't treat the
+ // block-bodied functions specially. In a mixture of the two function
+ // styles, it looks cleaner to treat them all like normal expressions so
+ // that the named arguments line up.
+ if (_isAllNamed(arguments.sublist(functionsStart, functionsEnd))) {
+ bool isNamedArrow(Expression expression) {
+ if (expression is! NamedExpression) return false;
+ expression = expression.expression;
+
+ return expression is FunctionExpression &&
+ expression.body is ExpressionFunctionBody;
+ }
+
+ for (var i = 0; i < functionsStart; i++) {
+ if (isNamedArrow(arguments[i])) return null;
+ }
+
+ for (var i = functionsEnd; i < arguments.length; i++) {
+ if (isNamedArrow(arguments[i])) return null;
+ }
+ }
+
+ return [functionsStart, functionsEnd];
+ }
+
+ /// Returns `true` if every expression in [arguments] is named.
+ static bool _isAllNamed(List<Expression> arguments) =>
+ arguments.every((argument) => argument is NamedExpression);
+
/// Returns `true` if [expression] is a [FunctionExpression] with a non-empty
/// block body.
static bool _isBlockFunction(Expression expression) {
@@ -295,10 +294,15 @@
/// The full argument list from the AST.
final List<Expression> _allArguments;
- /// The positional arguments, in order.
+ /// If all positional arguments occur before all named arguments, then this
+ /// contains the positional arguments, in order. Otherwise (there are no
+ /// positional arguments or they are interleaved with named ones), this is
+ /// empty.
final List<Expression> _positional;
- /// The named arguments, in order.
+ /// The named arguments, in order. If there are any named arguments that occur
+ /// before positional arguments, then all arguments are treated as named and
+ /// end up in this list.
final List<Expression> _named;
/// Maps each block argument, excluding functions, to the first token for that
@@ -325,10 +329,9 @@
factory ArgumentSublist(
List<Expression> allArguments, List<Expression> arguments) {
- // Assumes named arguments follow all positional ones.
- var positional =
- arguments.takeWhile((arg) => arg is! NamedExpression).toList();
- var named = arguments.skip(positional.length).toList();
+ var argumentLists = _splitArgumentLists(arguments);
+ var positional = argumentLists[0];
+ var named = argumentLists[1];
var blocks = <Expression, Token>{};
for (var argument in arguments) {
@@ -358,9 +361,7 @@
if (trailingBlocks != blocks.length) trailingBlocks = 0;
// Ignore any blocks in the middle of the argument list.
- if (leadingBlocks == 0 && trailingBlocks == 0) {
- blocks.clear();
- }
+ if (leadingBlocks == 0 && trailingBlocks == 0) blocks.clear();
return ArgumentSublist._(
allArguments, positional, named, blocks, leadingBlocks, trailingBlocks);
@@ -484,6 +485,37 @@
}
}
+ /// Splits [arguments] into two lists: the list of leading positional
+ /// arguments and the list of trailing named arguments.
+ ///
+ /// If positional arguments are interleaved with the named arguments then
+ /// all arguments are treat as named since that provides simpler, consistent
+ /// output.
+ ///
+ /// Returns a list of two lists: the positional arguments then the named ones.
+ static List<List<Expression>> _splitArgumentLists(
+ List<Expression> arguments) {
+ var positional = <Expression>[];
+ var named = <Expression>[];
+ var inNamed = false;
+ for (var argument in arguments) {
+ if (argument is NamedExpression) {
+ inNamed = true;
+ } else if (inNamed) {
+ // Got a positional argument after a named one.
+ return [[], arguments];
+ }
+
+ if (inNamed) {
+ named.add(argument);
+ } else {
+ positional.add(argument);
+ }
+ }
+
+ return [positional, named];
+ }
+
/// If [expression] can be formatted as a block, returns the token that opens
/// the block, such as a collection's bracket.
///
diff --git a/test/splitting/arguments.stmt b/test/splitting/arguments.stmt
index f9ecb7c..6a4fb03 100644
--- a/test/splitting/arguments.stmt
+++ b/test/splitting/arguments.stmt
@@ -215,4 +215,135 @@
fn(
named: argument,
another: argument,
+);
+>>> many arguments mixed named
+method(first, name1: second, third, name2: fourth, name3: fifth, sixth,
+seventh, name4: eighth, ninth, tenth);
+<<<
+method(
+ first,
+ name1: second,
+ third,
+ name2: fourth,
+ name3: fifth,
+ sixth,
+ seventh,
+ name4: eighth,
+ ninth,
+ tenth);
+>>> wrap before first argument, named first
+longFunctionIsLoooooooooooooong(name: argument, argument);
+<<<
+longFunctionIsLoooooooooooooong(
+ name: argument, argument);
+>>> named first
+function(name: firstArg * second, third * fourthAndLongest);
+<<<
+function(
+ name: firstArg * second,
+ third * fourthAndLongest);
+>>> arguments, nested
+someFunctionOne(someArgument,
+someFunctionTwo(name1: argument, argument, name2: argument),
+someFunctionTwo(argument, name: argument, argument),
+someArgument, someArgument);
+<<<
+someFunctionOne(
+ someArgument,
+ someFunctionTwo(
+ name1: argument,
+ argument,
+ name2: argument),
+ someFunctionTwo(
+ argument,
+ name: argument,
+ argument),
+ someArgument,
+ someArgument);
+>>> keep mixed positional and named on first line
+foo(arg, foo: arg, 1, bar: 2);
+<<<
+foo(arg, foo: arg, 1, bar: 2);
+>>> treat mixed named and positional as all named
+reallyLongMethod(argument, foo: first, second);
+<<<
+reallyLongMethod(
+ argument, foo: first, second);
+>>>
+reallyLongMethod(argument, argument, foo: first, bar: second, third);
+<<<
+reallyLongMethod(
+ argument,
+ argument,
+ foo: first,
+ bar: second,
+ third);
+>>>
+reallyLongMethod(leadingNamed: first, second, baz: third);
+<<<
+reallyLongMethod(
+ leadingNamed: first,
+ second,
+ baz: third);
+>>> trailing collections are indented in mixed named/positional arguments
+function(argument, a: argument, argument, b: argument,
+[element, element, element, element],
+c: {'key': value, 'other key': value, 'third key': value});
+<<<
+function(
+ argument,
+ a: argument,
+ argument,
+ b: argument,
+ [
+ element,
+ element,
+ element,
+ element
+ ],
+ c: {
+ 'key': value,
+ 'other key': value,
+ 'third key': value
+ });
+>>> all trailing collections
+function([element, element], name: [element, element], {'key': value, 'other key': value, 'third key': value});
+<<<
+function([
+ element,
+ element
+], name: [
+ element,
+ element
+], {
+ 'key': value,
+ 'other key': value,
+ 'third key': value
+});
+>>> non-collection non-preceding argument forces all collections to indent
+function([element, element, element, element], name: argument,
+{'key': value, 'other key': value, 'third key': value}, () {;});
+<<<
+function(
+ [
+ element,
+ element,
+ element,
+ element
+ ],
+ name: argument,
+ {
+ 'key': value,
+ 'other key': value,
+ 'third key': value
+ }, () {
+ ;
+});
+>>> trailing comma with mixed named
+fn(argument,name: argument ,argument , );
+<<<
+fn(
+ argument,
+ name: argument,
+ argument,
);
\ No newline at end of file
diff --git a/test/splitting/function_arguments.stmt b/test/splitting/function_arguments.stmt
index a94e99d..cf860fa 100644
--- a/test/splitting/function_arguments.stmt
+++ b/test/splitting/function_arguments.stmt
@@ -201,6 +201,54 @@
;
},
c: argument);
+>>> named args anywhere with leading non-function
+{
+ longFunction(argument, b: () {;}, c: () {;});
+ longFunction(a: argument, () {;}, c: () {;});
+ longFunction(a: argument, b: () {;}, () {;});
+}
+<<<
+{
+ longFunction(argument, b: () {
+ ;
+ }, c: () {
+ ;
+ });
+ longFunction(a: argument, () {
+ ;
+ }, c: () {
+ ;
+ });
+ longFunction(a: argument, b: () {
+ ;
+ }, () {
+ ;
+ });
+}
+>>> named args anywhere with trailing non-function
+{
+ longFunction(() {;}, b: () {;}, c: argument);
+ longFunction(a: () {;}, () {;}, c: argument);
+ longFunction(a: () {;}, b: () {;}, argument);
+}
+<<<
+{
+ longFunction(() {
+ ;
+ }, b: () {
+ ;
+ }, c: argument);
+ longFunction(a: () {
+ ;
+ }, () {
+ ;
+ }, c: argument);
+ longFunction(a: () {
+ ;
+ }, b: () {
+ ;
+ }, argument);
+}
>>> no extra indent for expression function argument with trailing comma
function(() => P(p,),a: () => [a,],);
<<<
@@ -228,4 +276,64 @@
P(
p,
),
-);
\ No newline at end of file
+);
+>>> unsplit named arguments with trailing positional closure
+function(a: argument, b: argument, () {;});
+<<<
+function(a: argument, b: argument, () {
+ ;
+});
+>>> split named arguments with trailing positional closure
+function(a: argument, b: argument, c: argument, () {;});
+<<<
+function(
+ a: argument,
+ b: argument,
+ c: argument, () {
+ ;
+});
+>>> unsingle split leading positional, named arguments with trailing positional closure
+function(argument, b: argument, () {;});
+<<<
+function(argument, b: argument, () {
+ ;
+});
+>>> single split leading positional, named arguments with trailing positional closure
+function(argument, b: argument, c: argument, () {;});
+<<<
+function(argument,
+ b: argument, c: argument, () {
+ ;
+});
+>>> split leading positional, named arguments with trailing positional closure
+function(argument, argument, argument, argument, argument, b: argument, c: argument, d: argument, () {;});
+<<<
+function(argument, argument, argument,
+ argument, argument,
+ b: argument,
+ c: argument,
+ d: argument, () {
+ ;
+});
+>>> multiple trailing positional closures
+function(argument, b: argument, c: argument, () {;}, () {;});
+<<<
+function(argument,
+ b: argument, c: argument, () {
+ ;
+}, () {
+ ;
+});
+>>> mixed named and positional trailing closures
+function(argument, b: argument, c: argument, () {;}, d: () {;}, () {;}, e: () {;});
+<<<
+function(argument,
+ b: argument, c: argument, () {
+ ;
+}, d: () {
+ ;
+}, () {
+ ;
+}, e: () {
+ ;
+});
\ No newline at end of file
diff --git a/test/splitting/list_arguments.stmt b/test/splitting/list_arguments.stmt
index ebb0248..759510d 100644
--- a/test/splitting/list_arguments.stmt
+++ b/test/splitting/list_arguments.stmt
@@ -233,6 +233,30 @@
element
],
c: third);
+>>> mixed named and positional forces nesting
+method(a:first,[element, element, element, element],c:third);
+<<<
+method(
+ a: first,
+ [
+ element,
+ element,
+ element,
+ element
+ ],
+ c: third);
+>>> mixed named and positional forces nesting
+method(a:first,b:[element, element, element, element],third);
+<<<
+method(
+ a: first,
+ b: [
+ element,
+ element,
+ element,
+ element
+ ],
+ third);
>>> nothing but named list args does not nest
longFunctionName(a: [element, element, element, element],
b: [element, element, element, element], c: [element, element, element, element]);
