diff --git a/CHANGELOG.md b/CHANGELOG.md index ae28abd9..a73d1619 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 9f4c6f2b..d839600a 100644 --- a/lib/src/argument_list_visitor.dart +++ b/lib/src/argument_list_visitor.dart @@ -63,87 +63,9 @@ class ArgumentListVisitor { Token leftParenthesis, Token rightParenthesis, List arguments) { - // Look for a single contiguous range of block function arguments. - int? functionsStart; - int? functionsEnd; - - for (var i = 0; i < arguments.length; i++) { - var argument = arguments[i]; - if (_isBlockFunction(argument)) { - functionsStart ??= i; + var functionRange = _contiguousFunctions(arguments); - // 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 @@ class ArgumentListVisitor { // 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 @@ class ArgumentListVisitor { 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? _contiguousFunctions(List 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 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 @@ class ArgumentSublist { /// The full argument list from the AST. final List _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 _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 _named; /// Maps each block argument, excluding functions, to the first token for that @@ -325,10 +329,9 @@ class ArgumentSublist { factory ArgumentSublist( List allArguments, List 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 = {}; for (var argument in arguments) { @@ -358,9 +361,7 @@ class ArgumentSublist { 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 @@ class ArgumentSublist { } } + /// 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> _splitArgumentLists( + List arguments) { + var positional = []; + var named = []; + 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 f9ecb7cb..6a4fb03e 100644 --- a/test/splitting/arguments.stmt +++ b/test/splitting/arguments.stmt @@ -215,4 +215,135 @@ fn(named: argument,another:argument, ); 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 a94e99d6..cf860fa6 100644 --- a/test/splitting/function_arguments.stmt +++ b/test/splitting/function_arguments.stmt @@ -201,6 +201,54 @@ longFunction( ; }, 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 @@ function( 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 ebb0248e..759510d0 100644 --- a/test/splitting/list_arguments.stmt +++ b/test/splitting/list_arguments.stmt @@ -233,6 +233,30 @@ method( 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]);