Skip to content

Commit 68eab57

Browse files
munificentmunificent
authored
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.
1 parent 6f894c0 commit 68eab57

File tree

5 files changed

+392
-93
lines changed

5 files changed

+392
-93
lines changed

‎CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
# 2.2.2
2+
3+
* Format named arguments anywhere (#1072).
4+
15
# 2.2.1
26

37
* Require `package:analyzer` version `2.6.0`.

‎lib/src/argument_list_visitor.dart

Lines changed: 124 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -63,97 +63,19 @@ class ArgumentListVisitor {
6363
Token leftParenthesis,
6464
Token rightParenthesis,
6565
List<Expression> arguments) {
66-
// Look for a single contiguous range of block function arguments.
67-
int? functionsStart;
68-
int? functionsEnd;
69-
70-
for (var i = 0; i < arguments.length; i++) {
71-
var argument = arguments[i];
72-
if (_isBlockFunction(argument)) {
73-
functionsStart ??= i;
66+
var functionRange = _contiguousFunctions(arguments);
7467

75-
// The functions must be one contiguous section.
76-
if (functionsEnd != null && functionsEnd != i) {
77-
functionsStart = null;
78-
functionsEnd = null;
79-
break;
80-
}
81-
82-
functionsEnd = i + 1;
83-
}
84-
}
85-
86-
// Edge case: If all of the arguments are named, but they aren't all
87-
// functions, then don't handle the functions specially. A function with a
88-
// bunch of named arguments tends to look best when they are all lined up,
89-
// even the function ones (unless they are all functions).
90-
//
91-
// Prefers:
92-
//
93-
// function(
94-
// named: () {
95-
// something();
96-
// },
97-
// another: argument);
98-
//
99-
// Over:
100-
//
101-
// function(named: () {
102-
// something();
103-
// },
104-
// another: argument);
105-
if (functionsStart != null &&
106-
arguments[0] is NamedExpression &&
107-
(functionsStart > 0 || functionsEnd! < arguments.length)) {
108-
functionsStart = null;
109-
}
110-
111-
// Edge case: If all of the function arguments are named and there are
112-
// other named arguments that are "=>" functions, then don't treat the
113-
// block-bodied functions specially. In a mixture of the two function
114-
// styles, it looks cleaner to treat them all like normal expressions so
115-
// that the named arguments line up.
116-
if (functionsStart != null &&
117-
arguments[functionsStart] is NamedExpression) {
118-
bool isArrow(NamedExpression named) {
119-
var expression = named.expression;
120-
121-
if (expression is FunctionExpression) {
122-
return expression.body is ExpressionFunctionBody;
123-
}
124-
125-
return false;
126-
}
127-
128-
for (var i = 0; i < functionsStart!; i++) {
129-
var argument = arguments[i];
130-
if (argument is! NamedExpression) continue;
131-
132-
if (isArrow(argument)) {
133-
functionsStart = null;
134-
break;
135-
}
136-
}
137-
138-
for (var i = functionsEnd!; i < arguments.length; i++) {
139-
if (isArrow(arguments[i] as NamedExpression)) {
140-
functionsStart = null;
141-
break;
142-
}
143-
}
144-
}
145-
146-
if (functionsStart == null) {
68+
if (functionRange == null) {
14769
// No functions, so there is just a single argument list.
14870
return ArgumentListVisitor._(visitor, leftParenthesis, rightParenthesis,
14971
arguments, ArgumentSublist(arguments, arguments), null, null);
15072
}
15173

15274
// Split the arguments into two independent argument lists with the
15375
// functions in the middle.
154-
var argumentsBefore = arguments.take(functionsStart).toList();
155-
var functions = arguments.sublist(functionsStart, functionsEnd);
156-
var argumentsAfter = arguments.skip(functionsEnd!).toList();
76+
var argumentsBefore = arguments.take(functionRange[0]).toList();
77+
var functions = arguments.sublist(functionRange[0], functionRange[1]);
78+
var argumentsAfter = arguments.skip(functionRange[1]).toList();
15779

15880
return ArgumentListVisitor._(
15981
visitor,
@@ -224,6 +146,83 @@ class ArgumentListVisitor {
224146
if (_isSingle) _visitor.builder.endSpan();
225147
}
226148

149+
/// Look for a single contiguous range of block function [arguments] that
150+
/// should receive special formatting.
151+
///
152+
/// Returns a list of (start, end] indexes if found, otherwise returns `null`.
153+
static List<int>? _contiguousFunctions(List<Expression> arguments) {
154+
int? functionsStart;
155+
var functionsEnd = -1;
156+
157+
// Find the range of block function arguments, if any.
158+
for (var i = 0; i < arguments.length; i++) {
159+
var argument = arguments[i];
160+
if (_isBlockFunction(argument)) {
161+
functionsStart ??= i;
162+
163+
// The functions must be one contiguous section.
164+
if (functionsEnd != -1 && functionsEnd != i) return null;
165+
166+
functionsEnd = i + 1;
167+
}
168+
}
169+
170+
if (functionsStart == null) return null;
171+
172+
// Edge case: If all of the arguments are named, but they aren't all
173+
// functions, then don't handle the functions specially. A function with a
174+
// bunch of named arguments tends to look best when they are all lined up,
175+
// even the function ones (unless they are all functions).
176+
//
177+
// Prefers:
178+
//
179+
// function(
180+
// named: () {
181+
// something();
182+
// },
183+
// another: argument);
184+
//
185+
// Over:
186+
//
187+
// function(named: () {
188+
// something();
189+
// },
190+
// another: argument);
191+
if (_isAllNamed(arguments) &&
192+
(functionsStart > 0 || functionsEnd < arguments.length)) {
193+
return null;
194+
}
195+
196+
// Edge case: If all of the function arguments are named and there are
197+
// other named arguments that are "=>" functions, then don't treat the
198+
// block-bodied functions specially. In a mixture of the two function
199+
// styles, it looks cleaner to treat them all like normal expressions so
200+
// that the named arguments line up.
201+
if (_isAllNamed(arguments.sublist(functionsStart, functionsEnd))) {
202+
bool isNamedArrow(Expression expression) {
203+
if (expression is! NamedExpression) return false;
204+
expression = expression.expression;
205+
206+
return expression is FunctionExpression &&
207+
expression.body is ExpressionFunctionBody;
208+
}
209+
210+
for (var i = 0; i < functionsStart; i++) {
211+
if (isNamedArrow(arguments[i])) return null;
212+
}
213+
214+
for (var i = functionsEnd; i < arguments.length; i++) {
215+
if (isNamedArrow(arguments[i])) return null;
216+
}
217+
}
218+
219+
return [functionsStart, functionsEnd];
220+
}
221+
222+
/// Returns `true` if every expression in [arguments] is named.
223+
static bool _isAllNamed(List<Expression> arguments) =>
224+
arguments.every((argument) => argument is NamedExpression);
225+
227226
/// Returns `true` if [expression] is a [FunctionExpression] with a non-empty
228227
/// block body.
229228
static bool _isBlockFunction(Expression expression) {
@@ -295,10 +294,15 @@ class ArgumentSublist {
295294
/// The full argument list from the AST.
296295
final List<Expression> _allArguments;
297296

298-
/// The positional arguments, in order.
297+
/// If all positional arguments occur before all named arguments, then this
298+
/// contains the positional arguments, in order. Otherwise (there are no
299+
/// positional arguments or they are interleaved with named ones), this is
300+
/// empty.
299301
final List<Expression> _positional;
300302

301-
/// The named arguments, in order.
303+
/// The named arguments, in order. If there are any named arguments that occur
304+
/// before positional arguments, then all arguments are treated as named and
305+
/// end up in this list.
302306
final List<Expression> _named;
303307

304308
/// Maps each block argument, excluding functions, to the first token for that
@@ -325,10 +329,9 @@ class ArgumentSublist {
325329

326330
factory ArgumentSublist(
327331
List<Expression> allArguments, List<Expression> arguments) {
328-
// Assumes named arguments follow all positional ones.
329-
var positional =
330-
arguments.takeWhile((arg) => arg is! NamedExpression).toList();
331-
var named = arguments.skip(positional.length).toList();
332+
var argumentLists = _splitArgumentLists(arguments);
333+
var positional = argumentLists[0];
334+
var named = argumentLists[1];
332335

333336
var blocks = <Expression, Token>{};
334337
for (var argument in arguments) {
@@ -358,9 +361,7 @@ class ArgumentSublist {
358361
if (trailingBlocks != blocks.length) trailingBlocks = 0;
359362

360363
// Ignore any blocks in the middle of the argument list.
361-
if (leadingBlocks == 0 && trailingBlocks == 0) {
362-
blocks.clear();
363-
}
364+
if (leadingBlocks == 0 && trailingBlocks == 0) blocks.clear();
364365

365366
return ArgumentSublist._(
366367
allArguments, positional, named, blocks, leadingBlocks, trailingBlocks);
@@ -484,6 +485,37 @@ class ArgumentSublist {
484485
}
485486
}
486487

488+
/// Splits [arguments] into two lists: the list of leading positional
489+
/// arguments and the list of trailing named arguments.
490+
///
491+
/// If positional arguments are interleaved with the named arguments then
492+
/// all arguments are treat as named since that provides simpler, consistent
493+
/// output.
494+
///
495+
/// Returns a list of two lists: the positional arguments then the named ones.
496+
static List<List<Expression>> _splitArgumentLists(
497+
List<Expression> arguments) {
498+
var positional = <Expression>[];
499+
var named = <Expression>[];
500+
var inNamed = false;
501+
for (var argument in arguments) {
502+
if (argument is NamedExpression) {
503+
inNamed = true;
504+
} else if (inNamed) {
505+
// Got a positional argument after a named one.
506+
return [[], arguments];
507+
}
508+
509+
if (inNamed) {
510+
named.add(argument);
511+
} else {
512+
positional.add(argument);
513+
}
514+
}
515+
516+
return [positional, named];
517+
}
518+
487519
/// If [expression] can be formatted as a block, returns the token that opens
488520
/// the block, such as a collection's bracket.
489521
///

0 commit comments

Comments
 (0)