From e8966c1fbb39fc779ad497369a7250c478fa1982 Mon Sep 17 00:00:00 2001 From: Robert Nystrom Date: Wed, 10 Jan 2024 19:00:45 -0800 Subject: [PATCH] Format call chains! \o/ This handles unsplit call chains, fully split ones, as well as block-style calls, like: ``` target.call( argument, ); ``` This doesn't fully implement the proposal I sketched out because it only allows a single block call. There's a long TODO(tall) comment about whether we want to do that, but I figured it makes sense to do that as a separate iteration since there is so much in here. But it implements most of it, and I've migrated all of the existing tests over. It ended up being both simpler than the old formatter while also producing better output. The old formatter has a restriction where the indentation of a piece of code must be fixed regardless of which way it splits. It can't handle: ``` // Block style, so indent argument +2: target.property.call( argument ); // Can't do block style, so indent +6: target .slightlyLongerProperty .call( argument ); ``` That leads to it sometimes producing gross output like: ``` target .longProperty .anotherLongProperty .method( wtfIsGoingOnHere, ); ``` To try to avoid that, it has lots of heuristics during Chunk construction to try to not end up with output like that during line splitting. The new formatter doesn't need any of those heuristics because it can freely choose indentation during line splitting. --- lib/src/front_end/ast_node_visitor.dart | 90 +++-------- lib/src/front_end/chain_builder.dart | 206 ++++++++++++++++++++++++ lib/src/front_end/piece_factory.dart | 23 +++ lib/src/piece/chain.dart | 198 ++++++++++++++++++++--- test/expression/postfix.stmt | 31 +++- test/invocation/chain.stmt | 102 ++++++++++++ test/invocation/chain_block.stmt | 158 ++++++++++++++++++ test/invocation/chain_comment.stmt | 93 +++++++++++ test/invocation/chain_postfix.stmt | 170 +++++++++++++++++++ test/invocation/chain_property.stmt | 53 ++++++ test/invocation/chain_target.stmt | 161 ++++++++++++++++++ test/invocation/constructor.stmt | 11 +- test/invocation/null_aware.stmt | 13 ++ 13 files changed, 1218 insertions(+), 91 deletions(-) create mode 100644 lib/src/front_end/chain_builder.dart create mode 100644 test/invocation/chain.stmt create mode 100644 test/invocation/chain_block.stmt create mode 100644 test/invocation/chain_comment.stmt create mode 100644 test/invocation/chain_postfix.stmt create mode 100644 test/invocation/chain_property.stmt create mode 100644 test/invocation/chain_target.stmt create mode 100644 test/invocation/null_aware.stmt diff --git a/lib/src/front_end/ast_node_visitor.dart b/lib/src/front_end/ast_node_visitor.dart index 029fcfd8..f885f482 100644 --- a/lib/src/front_end/ast_node_visitor.dart +++ b/lib/src/front_end/ast_node_visitor.dart @@ -12,7 +12,6 @@ import '../dart_formatter.dart'; import '../piece/adjacent.dart'; import '../piece/assign.dart'; import '../piece/block.dart'; -import '../piece/chain.dart'; import '../piece/constructor.dart'; import '../piece/for.dart'; import '../piece/if.dart'; @@ -22,6 +21,7 @@ import '../piece/piece.dart'; import '../piece/variable.dart'; import '../source_code.dart'; import 'adjacent_builder.dart'; +import 'chain_builder.dart'; import 'comment_writer.dart'; import 'delimited_list_builder.dart'; import 'piece_factory.dart'; @@ -412,18 +412,9 @@ class AstNodeVisitor extends ThrowingAstVisitor with PieceFactory { @override Piece visitConstructorName(ConstructorName node) { - // If there is an import prefix and/or constructor name, then allow - // splitting before the `.`. This doesn't look good, but is consistent with - // constructor calls that don't have `new` or `const`. We allow splitting - // in the latter because there is no way to distinguish syntactically - // between a named constructor call and any other kind of method call or - // property access. - var operations = []; - var builder = AdjacentBuilder(this); if (node.type.importPrefix case var importPrefix?) { builder.token(importPrefix.name); - operations.add(builder.build()); builder.token(importPrefix.period); } @@ -435,16 +426,13 @@ class AstNodeVisitor extends ThrowingAstVisitor with PieceFactory { // If this is a named constructor, the name. if (node.name != null) { - operations.add(builder.build()); builder.token(node.period); builder.visit(node.name); } // If there was a prefix or constructor name, then make a splittable piece. // Otherwise, the current piece is a simple identifier for the name. - operations.add(builder.build()); - if (operations.length == 1) return operations.first; - return ChainPiece(operations); + return builder.build(); } @override @@ -1027,16 +1015,9 @@ class AstNodeVisitor extends ThrowingAstVisitor with PieceFactory { @override Piece visitIndexExpression(IndexExpression node) { - // TODO(tall): Allow splitting before and/or after the `[` when method - // chain formatting is fully implemented. For now, we just output the code - // so that tests of other language features that contain index expressions - // can run. - return buildPiece((b) { - b.visit(node.target); - b.token(node.leftBracket); - b.visit(node.index); - b.token(node.rightBracket); - }); + Piece? targetPiece; + if (node.target case var target?) targetPiece = nodePiece(target); + return createIndexExpression(targetPiece, node); } @override @@ -1044,18 +1025,9 @@ class AstNodeVisitor extends ThrowingAstVisitor with PieceFactory { var builder = AdjacentBuilder(this); builder.token(node.keyword, spaceAfter: true); - // If there is an import prefix and/or constructor name, then allow - // splitting before the `.`. This doesn't look good, but is consistent with - // constructor calls that don't have `new` or `const`. We allow splitting - // in the latter because there is no way to distinguish syntactically - // between a named constructor call and any other kind of method call or - // property access. - var operations = []; - var constructor = node.constructorName; if (constructor.type.importPrefix case var importPrefix?) { builder.token(importPrefix.name); - operations.add(builder.build()); builder.token(importPrefix.period); } @@ -1066,19 +1038,13 @@ class AstNodeVisitor extends ThrowingAstVisitor with PieceFactory { // If this is a named constructor call, the name. if (constructor.name case var name?) { - operations.add(builder.build()); builder.token(constructor.period); builder.visit(name); } builder.visit(node.argumentList); - operations.add(builder.build()); - if (operations.length > 1) { - return ChainPiece(operations); - } else { - return operations.first; - } + return builder.build(); } @override @@ -1195,15 +1161,23 @@ class AstNodeVisitor extends ThrowingAstVisitor with PieceFactory { @override Piece visitMethodInvocation(MethodInvocation node) { - return buildPiece((b) { - // TODO(tall): Support splitting at `.` or `?.`. Right now we just format - // it inline so that we can use method calls in other tests. - b.visit(node.target); - b.token(node.operator); - b.visit(node.methodName); - b.visit(node.typeArguments); - b.visit(node.argumentList); - }); + // If there's no target, this is a "bare" function call like "foo(1, 2)", + // or a section in a cascade. + // + // If it looks like a constructor or static call, we want to keep the + // target and method together instead of including the method in the + // subsequent method chain. + if (node.target == null || node.looksLikeStaticCall) { + return buildPiece((b) { + b.visit(node.target); + b.token(node.operator); + b.visit(node.methodName); + b.visit(node.typeArguments); + b.visit(node.argumentList); + }); + } + + return ChainBuilder(this, node).build(); } @override @@ -1354,14 +1328,7 @@ class AstNodeVisitor extends ThrowingAstVisitor with PieceFactory { @override Piece visitPrefixedIdentifier(PrefixedIdentifier node) { - // TODO(tall): Allow splitting before the `.` when method chain formatting - // is fully implemented. For now, we just output the code so that tests - // of other language features that contain prefixed identifiers can run. - return buildPiece((b) { - b.visit(node.prefix); - b.token(node.period); - b.visit(node.identifier); - }); + return ChainBuilder(this, node).build(); } @override @@ -1382,14 +1349,7 @@ class AstNodeVisitor extends ThrowingAstVisitor with PieceFactory { @override Piece visitPropertyAccess(PropertyAccess node) { - // TODO(tall): Allow splitting before the `.` when method chain formatting - // is fully implemented. For now, we just output the code so that tests - // of other language features that contain property accesses can run. - return buildPiece((b) { - b.visit(node.target); - b.token(node.operator); - b.visit(node.propertyName); - }); + return ChainBuilder(this, node).build(); } @override diff --git a/lib/src/front_end/chain_builder.dart b/lib/src/front_end/chain_builder.dart new file mode 100644 index 00000000..b98a389c --- /dev/null +++ b/lib/src/front_end/chain_builder.dart @@ -0,0 +1,206 @@ +// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/token.dart'; + +import '../ast_extensions.dart'; +import '../piece/chain.dart'; +import '../piece/piece.dart'; +import 'piece_factory.dart'; + +/// Creates [Chain] pieces from method calls and property accesses, along with +/// postfix operations (`!`, index operators, and function invocation +/// expressions) that follow them. +/// +/// In the AST for method calls, selectors are nested bottom up such that this +/// expression: +/// +/// obj.a(1)[2].c(3) +/// +/// Is structured like: +/// +/// .c() +/// / \ +/// [] 3 +/// / \ +/// .a() 2 +/// / \ +/// obj 1 +/// +/// This means visiting the AST from top down visits the selectors from right +/// to left. It's easier to format if we organize them as a linear series of +/// selectors from left to right. Further, we want to organize it into a +/// two-tier hierarchy. We have an outer list of method calls and property +/// accesses. Then each of those may have one or more postfix selectors +/// attached: indexers, null-assertions, or invocations. This mirrors how they +/// are formatted. +/// +/// This lets us create a single [ChainPiece] for the entire series of dotted +/// operations, so that we can control splitting them or not as a unit. +class ChainBuilder { + final PieceFactory _visitor; + + /// The left-most target of the chain. + late Piece _target; + + /// Whether the target expression may contain newlines when the chain is not + /// fully split. (It may always contain newlines when the chain splits.) + /// + /// This is true for most expressions but false for delimited ones to avoid + /// ugly formatting like: + /// + /// function( + /// argument, + /// ) + /// .method(); + late final bool _allowSplitInTarget; + + /// The dotted property accesses and method calls following the target. + final List _calls = []; + + ChainBuilder(this._visitor, Expression expression) { + _unwrapCall(expression); + } + + Piece build() { + // If there are no calls, there's no chain. + if (_calls.isEmpty) return _target; + + // Count the number of contiguous properties at the beginning of the chain. + var leadingProperties = 0; + while (leadingProperties < _calls.length && + _calls[leadingProperties].type == CallType.property) { + leadingProperties++; + } + + // See if there is a call that we can block format. It can either be the + // very last call, if non-empty: + // + // target.property.method().last( + // argument, + // ); + // + // Or the second-to-last if the last call can't split: + // + // target.property.method().penultimate( + // argument, + // ).toList(); + var blockCallIndex = switch (_calls) { + [..., ChainCall(canSplit: true)] => _calls.length - 1, + [..., ChainCall(canSplit: true), ChainCall(canSplit: false)] => + _calls.length - 2, + _ => -1, + }; + + return ChainPiece(_target, _calls, leadingProperties, blockCallIndex, + allowSplitInTarget: _allowSplitInTarget); + } + + /// Given [expression], which is the outermost expression for some call chain, + /// recursively traverses the selectors to fill in the list of [_calls]. + /// + /// Initializes [_target] with the innermost subexpression that isn't a part + /// of the call chain. For example, given: + /// + /// foo.bar()!.baz[0][1].bang() + /// + /// This returns `foo` and fills [_calls] with: + /// + /// .bar()! + /// .baz[0][1] + /// .bang() + void _unwrapCall(Expression expression) { + switch (expression) { + case Expression(looksLikeStaticCall: true): + // Don't include things that look like static method or constructor + // calls in the call chain because that tends to split up named + // constructors from their class. + _visitTarget(expression); + + // Selectors. + case MethodInvocation(:var target?): + _unwrapCall(target); + + var callPiece = _visitor.buildPiece((b) { + b.token(expression.operator); + b.visit(expression.methodName); + b.visit(expression.typeArguments); + b.visit(expression.argumentList); + }); + + var canSplit = expression.argumentList.arguments + .canSplit(expression.argumentList.rightParenthesis); + _calls.add(ChainCall(callPiece, + canSplit ? CallType.splittableCall : CallType.unsplittableCall)); + + case PropertyAccess(:var target?): + _unwrapCall(target); + + var piece = _visitor.buildPiece((b) { + b.token(expression.operator); + b.visit(expression.propertyName); + }); + + _calls.add(ChainCall(piece, CallType.property)); + + case PrefixedIdentifier(:var prefix): + _unwrapCall(prefix); + + var piece = _visitor.buildPiece((b) { + b.token(expression.period); + b.visit(expression.identifier); + }); + + _calls.add(ChainCall(piece, CallType.property)); + + // Postfix expressions. + case FunctionExpressionInvocation(): + _unwrapPostfix(expression.function, (target) { + return _visitor.buildPiece((b) { + b.add(target); + b.visit(expression.typeArguments); + b.visit(expression.argumentList); + }); + }); + + case IndexExpression(): + _unwrapPostfix(expression.target!, (target) { + return _visitor.createIndexExpression(target, expression); + }); + + case PostfixExpression() when expression.operator.type == TokenType.BANG: + _unwrapPostfix(expression.operand, (target) { + return _visitor.buildPiece((b) { + b.add(target); + b.token(expression.operator); + }); + }); + + default: + // Otherwise, it isn't a selector so we've reached the target. + _visitTarget(expression); + } + } + + /// Creates a Piece for [target]. + void _visitTarget(Expression target) { + _allowSplitInTarget = target.canBlockSplit; + _target = _visitor.nodePiece(target); + } + + void _unwrapPostfix( + Expression operand, Piece Function(Piece target) createPostfix) { + _unwrapCall(operand); + // If we don't have a preceding call to hang the postfix expression off of, + // wrap it around the target expression. For example: + // + // (list + another)! + if (_calls.isEmpty) { + _target = createPostfix(_target); + } else { + _calls.last.wrapPostfix(createPostfix); + } + } +} diff --git a/lib/src/front_end/piece_factory.dart b/lib/src/front_end/piece_factory.dart index 191b4345..faf6a618 100644 --- a/lib/src/front_end/piece_factory.dart +++ b/lib/src/front_end/piece_factory.dart @@ -573,6 +573,29 @@ mixin PieceFactory { return builder.build(); } + /// Creates a [Piece] for an index expression whose [target] has already been + /// converted to a piece. + /// + /// The [target] may be `null` if [index] is an index expression for a + /// cascade section. + Piece createIndexExpression(Piece? target, IndexExpression index) { + // TODO(tall): Consider whether we should allow splitting between + // successive index expressions, like: + // + // jsonData['some long key'] + // ['another long key']; + // + // The current formatter allows it, but it's very rarely used (0.021% of + // index expressions in a corpus of pub packages). + return buildPiece((b) { + if (target != null) b.add(target); + b.token(index.question); + b.token(index.leftBracket); + b.visit(index.index); + b.token(index.rightBracket); + }); + } + /// Creates a single infix operation. /// /// If [hanging] is `true` then the operator goes at the end of the first diff --git a/lib/src/piece/chain.dart b/lib/src/piece/chain.dart index b1ac7405..3eb101d3 100644 --- a/lib/src/piece/chain.dart +++ b/lib/src/piece/chain.dart @@ -5,43 +5,205 @@ import '../back_end/code_writer.dart'; import '../constants.dart'; import 'piece.dart'; -// TODO(tall): This will probably become more elaborate when full method chains -// with interesting argument lists are supported. Right now, it's just the -// basics needed for instance creation expressions which may have method-like -// `.` in them. - /// A dotted series of property access or method calls, like: /// /// target.getter.method().another.method(); /// -/// This piece handles splitting before the `.`. +/// This piece handles splitting before the `.` and controlling which argument +/// lists in the method calls are allowed to contain newlines. +/// +/// Chains can split in four ways: +/// +/// [State.unsplit] The entire chain on one line: +/// +/// target.getter.method().another.method(); +/// +/// [_blockFormatTrailingCall] Don't split before any `.`. Split the last (or +/// next-to-last if there is a hanging unsplittable call at the end) method +/// call in the chain like a block while leaving other calls unsplit, as in: +/// +/// target.property.first(1).block( +/// argument, +/// argument, +/// ); +/// +/// [_splitAfterProperties] Split the call chain at each method call, but leave +/// the leading properties on the same line as the target. We allow leading +/// properties to remain unsplit while splitting the rest of the chain since +/// property accesses often feel "closer" to the target then the methods called +/// on it, as in: +/// +/// motorcycle.wheels.front +/// .rotate(); +/// +/// [State.split] Split before every `.` and indent the chain, like: +/// +/// target +/// .getter +/// .method( +/// argument, +/// argument, +/// ).another.method( +/// argument, +/// argument, +/// ); class ChainPiece extends Piece { - /// The series of operations. + /// Allow newlines in the last (or next-to-last) call but nowhere else. + static const State _blockFormatTrailingCall = State(1, cost: 0); + + // TODO(tall): Currently, we only allow a single call in the chain to be + // block-formatted, and it must be the last or next-to-last. That covers + // the majority of common use cases (>90% of Flutter call chains), but there + // are some cases (<1%) where it might be good to support multiple block + // calls in a chain, like: + // + // future.then((_) { + // doStuff(); + // }).then((_) { + // moreStuff(); + // }).catchError((error) { + // print('Oh no!'); + // }); + // + // Decide if we want to support this and, if so, which calls are allowed to + // be block formatted. A reasonable approach would be to say that multiple + // block calls are allowed when the chain is (possibly zero) leading + // properties followed by only splittable calls and all splittable calls get + // block formatted. + + /// Split the call chain at each method call, but leave the leading properties + /// on the same line as the target. + static const State _splitAfterProperties = State(2); + + /// The target expression at the beginning of the call chain. + final Piece _target; + + /// The series of calls. /// /// The first piece in this is the target, and the rest are operations. - final List _operations; + final List _calls; + + /// The number of contiguous calls at the beginning of the chain that are + /// properties. + final int _leadingProperties; + + /// The index of the call in the chain that may be block formatted or `-1` if + /// none can. + /// + /// This will either be the index of the last call, or the index of the + /// second to last call if the last call is a property or unsplittable. + final int _blockCallIndex; + + /// Whether the target expression may contain newlines when the chain is not + /// fully split. (It may always contain newlines when the chain splits.) + /// + /// This is true for most expressions but false for delimited ones to avoid + /// this weird output: + /// + /// function( + /// argument, + /// ) + /// .method(); + final bool _allowSplitInTarget; - ChainPiece(this._operations); + /// Creates a new ChainPiece. + /// + /// Instead of calling this directly, prefer using [ChainBuilder]. + ChainPiece( + this._target, this._calls, this._leadingProperties, this._blockCallIndex, + {required bool allowSplitInTarget}) + : _allowSplitInTarget = allowSplitInTarget, + // If there are no calls, we shouldn't have created a chain. + assert(_calls.isNotEmpty); @override - List get additionalStates => const [State.split]; + List get additionalStates => [ + if (_blockCallIndex != -1) _blockFormatTrailingCall, + if (_leadingProperties > 0) _splitAfterProperties, + State.split + ]; @override void format(CodeWriter writer, State state) { - if (state == State.unsplit) { - writer.setAllowNewlines(false); - } else { - writer.setIndent(Indent.expression); + // If we split at the ".", then indent all of the calls, like: + // + // target + // .call( + // arg, + // ); + switch (state) { + case State.unsplit: + writer.setAllowNewlines(_allowSplitInTarget); + case _splitAfterProperties: + writer.setIndent(Indent.expression); + writer.setAllowNewlines(_allowSplitInTarget); + case _blockFormatTrailingCall: + writer.setAllowNewlines(_allowSplitInTarget); + case State.split: + writer.setIndent(Indent.expression); } - for (var i = 0; i < _operations.length; i++) { - if (i > 0) writer.splitIf(state == State.split, space: false); - writer.format(_operations[i]); + writer.format(_target); + + for (var i = 0; i < _calls.length; i++) { + switch (state) { + case State.unsplit: + writer.setAllowNewlines(false); + case _splitAfterProperties: + writer.setAllowNewlines(i >= _leadingProperties); + writer.splitIf(i >= _leadingProperties, space: false); + case _blockFormatTrailingCall: + writer.setAllowNewlines(i == _blockCallIndex); + case State.split: + writer.setAllowNewlines(true); + writer.newline(); + } + + var call = _calls[i]; + writer.format(call._call); } } @override void forEachChild(void Function(Piece piece) callback) { - _operations.forEach(callback); + callback(_target); + + for (var call in _calls) { + callback(call._call); + } } } + +/// A method or getter call in a call chain, along with any postfix operations +/// applies to it. +class ChainCall { + /// Piece for the call. + Piece _call; + + final CallType type; + + ChainCall(this._call, this.type); + + bool get canSplit => type == CallType.splittableCall; + + /// Applies a postfix operation to this call. + /// + /// Invokes [createPostfix] with the current piece for the call. That + /// callback should return a new piece that contains [target] followed by the + /// postfix operation. + void wrapPostfix(Piece Function(Piece target) createPostfix) { + _call = createPostfix(_call); + } +} + +/// What kind of "call" a dotted expression in a call chain is. +enum CallType { + /// A property access, like `.foo`. + property, + + /// A method call with an empty argument list that can't split. + unsplittableCall, + + /// A method call with a non-empty argument list that can split. + splittableCall +} diff --git a/test/expression/postfix.stmt b/test/expression/postfix.stmt index e1724cd1..91e3abd1 100644 --- a/test/expression/postfix.stmt +++ b/test/expression/postfix.stmt @@ -30,4 +30,33 @@ obj!.getter!.method(arg)! + 3; >>> Null-assert before index and call operators. obj ! [ index ] ! ( call ) ! + 3; <<< -obj![index]!(call)! + 3; \ No newline at end of file +obj![index]!(call)! + 3; +>>> Simple index expression. +list [ 123 ]; +<<< +list[123]; +>>> Index expressions don't split. +verylongIdentifier[someParticularlyLongArgument]; +<<< +verylongIdentifier[someParticularlyLongArgument]; +>>> Split inside index. +verylongIdentifier[someParticularly + longArgument]; +<<< +verylongIdentifier[someParticularly + + longArgument]; +>>> Nested index expressions. +verylongIdentifier[longIdentifier[someParticularlyLongArgument]]; +<<< +verylongIdentifier[longIdentifier[someParticularlyLongArgument]]; +>>> Chained index expressions do not split. +identifier[longArgument][longArgument][longArgument]; +<<< +identifier[longArgument][longArgument][longArgument]; +>>> Null-aware index expression. +list ? [ 123 ]; +<<< +list?[123]; +>>> Chained null-aware index expressions do not split. +identifier?[longArgument][longArgument]?[longArgument]; +<<< +identifier?[longArgument][longArgument]?[longArgument]; \ No newline at end of file diff --git a/test/invocation/chain.stmt b/test/invocation/chain.stmt new file mode 100644 index 00000000..1fa4c235 --- /dev/null +++ b/test/invocation/chain.stmt @@ -0,0 +1,102 @@ +40 columns | +>>> Keep chain on one line if it fits. +compiler.something().something().some(); +<<< +compiler.something().something().some(); +>>> Split all chained calls if they don't fit on one line. +compiler.something().something().something(); +<<< +compiler + .something() + .something() + .something(); +>>> Indent contents of split argument lists in calls. +target.arguments(argument1, argument2, argument3) +.list([element1, element2, element3]).function(() {body;}) +.operator(someLongOperand + anotherLongOperand); +<<< +target + .arguments( + argument1, + argument2, + argument3, + ) + .list([ + element1, + element2, + element3, + ]) + .function(() { + body; + }) + .operator( + someLongOperand + + anotherLongOperand, + ); +>>> Indent split calls past the target indentation. +someVeryLongExpression = someVeryLongExpression.someLongMethod(); +<<< +someVeryLongExpression = + someVeryLongExpression + .someLongMethod(); +>>> Don't split before an implicit receiver. +return + firstLongMethod() + .secondLongMethod(); +<<< +return firstLongMethod() + .secondLongMethod(); +>>> If call looks like named constructor, don't put in chain. +Foo.named().method().method().method().method().method(); +<<< +Foo.named() + .method() + .method() + .method() + .method() + .method(); +>>> If call looks like prefixed constructor, don't put in chain. +prefix.Foo().method().method().method().method().method(); +<<< +prefix.Foo() + .method() + .method() + .method() + .method() + .method(); +>>> If call looks like prefixed named constructor, don't put in chain. +prefix.Foo.named().method().method().method().method().method(); +<<< +prefix.Foo.named() + .method() + .method() + .method() + .method() + .method(); +>>> If call looks like private named constructor, don't put in chain. +_Foo.named().method().method().method().method().method(); +<<< +_Foo.named() + .method() + .method() + .method() + .method() + .method(); +>>> If call looks like private prefixed constructor, don't put in chain. +prefix._Foo().method().method().method().method().method(); +<<< +prefix._Foo() + .method() + .method() + .method() + .method() + .method(); +>>> If call looks like private prefixed named constructor, don't put in chain. +prefix._Foo.named().method().method().method().method().method(); +<<< +prefix._Foo.named() + .method() + .method() + .method() + .method() + .method(); \ No newline at end of file diff --git a/test/invocation/chain_block.stmt b/test/invocation/chain_block.stmt new file mode 100644 index 00000000..2fe9d884 --- /dev/null +++ b/test/invocation/chain_block.stmt @@ -0,0 +1,158 @@ +40 columns | +### Tests "block-like" formatting of method chains where we don't split at +### "." while still allowing newlines in some argument lists. +>>> Block format single call with regular arguments. +target.method(argument1, argument2, argument3); +<<< +target.method( + argument1, + argument2, + argument3, +); +>>> Block format single call with collection argument. +target.method([element1, element2, element3]); +<<< +target.method([ + element1, + element2, + element3, +]); +>>> Block format single call with function argument. +target.method(() { body; }); +<<< +target.method(() { + body; +}); +>>> Block format single call with line comment in argument list. +target.method(// comment +); +<<< +target.method( + // comment +); +>>> Block format single call with block comment in argument list. +target.method(/* a very long comment */); +<<< +target.method( + /* a very long comment */ +); +>>> Allow block format with leading properties. +target.property1.property2.method(argument1, argument2); +<<< +target.property1.property2.method( + argument1, + argument2, +); +>>> If leading properties split, then don't block format. +target.property1.property2.property3.method(argument1, argument2, argument3); +<<< +target.property1.property2.property3 + .method( + argument1, + argument2, + argument3, + ); +>>> Allow block format with leading calls. +target.zero().one(1).two(1, 2).method(argument); +<<< +target.zero().one(1).two(1, 2).method( + argument, +); +>>> Allow unsplit method chain with function at end. +compiler + .run(script) + .then((_) { + body; + }); +<<< +compiler.run(script).then((_) { + body; +}); +>>> Don't block format if the preceding chain doesn't fit on one line. +compiler + .run(longerScriptArgumentHere) + .then((_) { + body; + }); +<<< +compiler + .run(longerScriptArgumentHere) + .then((_) { + body; + }); +>>> +target.property1.property2.property3.method(argument); +<<< +target.property1.property2.property3 + .method(argument); +>>> Allow a trailing property after the block-formatted call. +target.method(argument1, argument2).property; +<<< +target.method( + argument1, + argument2, +).property; +>>> Allow a trailing empty call after the block-formatted call. +target.method(argument1, argument2).another(); +<<< +target.method( + argument1, + argument2, +).another(); +>>> Don't allow a trailing non-empty call after the block-formatted call. +target.method(argument1, argument2).another(1); +<<< +target + .method(argument1, argument2) + .another(1); +>>> Don't allow a trailing non-empty call after the block-formatted call. +target.method(argument1, argument2).another(/* c */); +<<< +target + .method(argument1, argument2) + .another(/* c */); +>>> Allow postfix before block call. +target.prop!.other[1]().method(argument1, argument2); +<<< +target.prop!.other[1]().method( + argument1, + argument2, +); +>>> Allow postfix `!` on block call. +target.method(argument1, argument2, argument3)!; +<<< +target.method( + argument1, + argument2, + argument3, +)!; +>>> Postfix index on block call. +target.method(argument1, argument2, argument3)[index]; +<<< +target.method( + argument1, + argument2, + argument3, +)[index]; +>>> Postfix call on block call. +target.method(argument1, argument2, argument3)(argument4); +<<< +target.method( + argument1, + argument2, + argument3, +)(argument4); +>>> Postfix call on block call. +target.method(argument1, argument2, argument3) +(argument4, argument5, argument6, argument7); +<<< +target.method( + argument1, + argument2, + argument3, +)( + argument4, + argument5, + argument6, + argument7, +); \ No newline at end of file diff --git a/test/invocation/chain_comment.stmt b/test/invocation/chain_comment.stmt new file mode 100644 index 00000000..5213bc0b --- /dev/null +++ b/test/invocation/chain_comment.stmt @@ -0,0 +1,93 @@ +40 columns | +>>> Line comment before `.` on property. +target // c +.property.other; +<<< +target // c + .property + .other; +>>> Line comment after `.` on property. +target. // c +property.other; +<<< +### Ugly, but not where users place comments. +target + . // c + property + .other; +>>> Line comment before `.` on method. +target // c +.method().other(); +<<< +target // c + .method() + .other(); +>>> Line comment after `.` on property. +target. // c +method().other(); +<<< +### Ugly, but not where users place comments. +target + . // c + method() + .other(); +>>> Line comments between calls. +target // c1 +.a(1) // c2 +.b // c3 +.c() // c4 +.d(2); +<<< +target // c1 + .a(1) // c2 + .b // c3 + .c() // c4 + .d(2); +>>> Line comment after method chain. +target.prop.method() // c +; +<<< +### A little weird to force the split, but users don't put comments here. +target.prop + .method() // c + ; +>>> Line comment after method chain. +target.prop.method(); // c +<<< +target.prop.method(); // c +>>> Line comment after method chain. +target.prop.method(); // very long comment +<<< +target.prop + .method(); // very long comment +>>> Line comment in target argument list. +someFunction(// c +someExtremelyLongArgumentName).clamp(); +<<< +someFunction( + // c + someExtremelyLongArgumentName, +).clamp(); +>>> Line comment in method chain argument list. +target.method(// c +); +<<< +target.method( + // c +); +>>> +target.first(// c1 +).second(// c2 +).third(// c3 +); +<<< +target + .first( + // c1 + ) + .second( + // c2 + ) + .third( + // c3 + ); \ No newline at end of file diff --git a/test/invocation/chain_postfix.stmt b/test/invocation/chain_postfix.stmt new file mode 100644 index 00000000..93128cdc --- /dev/null +++ b/test/invocation/chain_postfix.stmt @@ -0,0 +1,170 @@ +40 columns | +>>> Don't split null-asserted chained calls if not needed. +compiler!.a().b()!.c.d(); +<<< +compiler!.a().b()!.c.d(); +>>> Keep `!` with operand before method call. +verylongIdentifier!.longIdentifier().another()!.aThird()!; +<<< +verylongIdentifier! + .longIdentifier() + .another()! + .aThird()!; +>>> Keep `!` with operand before property access. +verylongIdentifier!.longIdentifier.another!.aThird!; +<<< +verylongIdentifier! + .longIdentifier + .another! + .aThird!; +>>> Keep `!` with operand before property access. +verylongIdentifier!.longIdentifier.another!.aThird!.longerPropertyChain; +<<< +verylongIdentifier! + .longIdentifier + .another! + .aThird! + .longerPropertyChain; +>>> Index in property chain. +someReceiverObject.property1.property2 + .property3[0] + .property4 + .property5 + .property6; +<<< +someReceiverObject + .property1 + .property2 + .property3[0] + .property4 + .property5 + .property6; +>>> Chained indexes. +someReceiverObject.property1.property2 + .property3[argument] + [argument][argument] + .property4 + .property5 + .property6; +<<< +### TODO(tall): Allow splitting between successive indexes. +someReceiverObject + .property1 + .property2 + .property3[argument][argument][argument] + .property4 + .property5 + .property6; +>>> Index on method call. +someReceiverObject.property1.property2 + .method3()[0] + .property4 + .property5 + .property6; +<<< +someReceiverObject.property1.property2 + .method3()[0] + .property4 + .property5 + .property6; +>>> Split inside index. +someReceiverObject.method1().method2()[veryLongIndexExpression + thatHasInternalSplit] +.method4(); +<<< +someReceiverObject + .method1() + .method2()[veryLongIndexExpression + + thatHasInternalSplit] + .method4(); +>>> Null-aware index. +receiver.property1.property2 + .property3?[0][1]?[2] + .method1()?[0][1]?[2] + .method2(); +<<< +receiver + .property1 + .property2 + .property3?[0][1]?[2] + .method1()?[0][1]?[2] + .method2(); +>>> Function invocation in chain. +someReceiverObject.method1().method2().method3()(argument) +.method4()(another).method5().method6(); +<<< +someReceiverObject + .method1() + .method2() + .method3()(argument) + .method4()(another) + .method5() + .method6(); +>>> Split argument list in invocation. +someReceiverObject.method1().method2()(argument1, argument2, argument3) +.method4(); +<<< +someReceiverObject + .method1() + .method2()( + argument1, + argument2, + argument3, + ) + .method4(); +>>> Invocation with type arguments. +target.method()(123, 'string').another(); +<<< +target + .method()( + 123, + 'string', + ) + .another(); +>>> Chained invocations. +target.method1().method2()(1)(2, 3)(4, 5, 6).method3().method4(); +<<< +target + .method1() + .method2()(1)(2, 3)(4, 5, 6) + .method3() + .method4(); +>>> Chained complex invocations. +someReceiverObject.method1().method2().method3() +(argument)(argument)(argument, argument, argument, argument, argument) +(argument).method4().method5().method6(); +<<< +someReceiverObject + .method1() + .method2() + .method3()( + argument, + )(argument)( + argument, + argument, + argument, + argument, + argument, + )(argument) + .method4() + .method5() + .method6(); +>>> Keep `!` with operand before index. +verylongIdentifier![i]![j].longIdentifier[i][j].another[i]![j].aThird!; +<<< +verylongIdentifier![i]![j] + .longIdentifier[i][j] + .another[i]![j] + .aThird!; +>>> Keep `!` with operand before invocation. +verylongIdentifier!(i)!(j).longIdentifier(i)(j).another(i)!(j).aThird!; +<<< +verylongIdentifier!(i)!(j) + .longIdentifier(i)(j) + .another(i)!(j) + .aThird!; +>>> Mixed postfix operations. +target.method()![1](2)![3](4).another()![1](2)![3](4); +<<< +target + .method()![1](2)![3](4) + .another()![1](2)![3](4); \ No newline at end of file diff --git a/test/invocation/chain_property.stmt b/test/invocation/chain_property.stmt new file mode 100644 index 00000000..c4e2848c --- /dev/null +++ b/test/invocation/chain_property.stmt @@ -0,0 +1,53 @@ +40 columns | +### Tests special (or not) handling of properties in call chains. +>>> Don't split leading properties in a chain. +compiler.property.property.method().method().method(); +<<< +compiler.property.property + .method() + .method() + .method(); +>>> Don't split leading properties even if other properties split. +compiler.property.method().property.method(); +<<< +compiler.property + .method() + .property + .method(); +>>> Split properties after a method chain. +compiler.method().method().method().property.property; +<<< +compiler + .method() + .method() + .method() + .property + .property; +>>> Split properties inside a method chain. +compiler.method().property.method().property.method(); +<<< +compiler + .method() + .property + .method() + .property + .method(); +>>> Split all properties if any split. +avian.bovine.canine.equine.feline.piscine; +<<< +avian + .bovine + .canine + .equine + .feline + .piscine; +>>> Split all leading properties if any split. +avian.bovine.canine.equine.feline.piscine.method(); +<<< +avian + .bovine + .canine + .equine + .feline + .piscine + .method(); \ No newline at end of file diff --git a/test/invocation/chain_target.stmt b/test/invocation/chain_target.stmt new file mode 100644 index 00000000..b80d4c7d --- /dev/null +++ b/test/invocation/chain_target.stmt @@ -0,0 +1,161 @@ +40 columns | +### Test how splits in call chain targets affect the chain. +>>> Split function call target with unsplit chain. +someTargetFunction(argument1, argument2).prop.method(1).method(2); +<<< +someTargetFunction( + argument1, + argument2, +).prop.method(1).method(2); +>>> Split function call target with block split chain. +someTargetFunction(argument1, argument2) +.prop.method(argument3, argument4, argument5); +<<< +someTargetFunction( + argument1, + argument2, +).prop.method( + argument3, + argument4, + argument5, +); +>>> Split function call target with fully split chain. +someTargetFunction(argument1, argument2, argument3) +.method(argument).another(argument).third(argument); +<<< +someTargetFunction( + argument1, + argument2, + argument3, + ) + .method(argument) + .another(argument) + .third(argument); +>>> Split collection target with unsplit chain. +[element1, element2, element3, element4].method().prop.another(); +<<< +[ + element1, + element2, + element3, + element4, +].method().prop.another(); +>>> Split collection target with block split chain. +[element1, element2, element3, element4].method().prop.another( +argument1, argument2); +<<< +[element1, element2, element3, element4] + .method() + .prop + .another(argument1, argument2); +>>> +[element1, element2, element3, element4, element5].method().prop.another( +argument1, argument2); +<<< +[ + element1, + element2, + element3, + element4, + element5, +].method().prop.another( + argument1, + argument2, +); +>>> +[element1, element2, element3, element4].method().prop.another( +argument1, argument2, argument3); +<<< +[ + element1, + element2, + element3, + element4, +].method().prop.another( + argument1, + argument2, + argument3, +); +>>> Allow split in function call target without splitting chain. +function(argument1, argument2, argument3).method().chain(); +<<< +function( + argument1, + argument2, + argument3, +).method().chain(); +>>> Allow split in instance creation target without splitting chain. +new Foo(argument1, argument2, argument3).method().chain(); +<<< +new Foo( + argument1, + argument2, + argument3, +).method().chain(); +>>> Allow split in list target without splitting chain. +[element1, element2, element3, element4].method().chain(); +<<< +[ + element1, + element2, + element3, + element4, +].method().chain(); +>>> Allow split in map target without splitting chain. +return {key1: value1, key2: value2, key3: value3}.method().chain(); +<<< +return { + key1: value1, + key2: value2, + key3: value3, +}.method().chain(); +>>> Allow split in set target without splitting chain. +return {element1, element2, element3, element4}.method().chain(); +<<< +return { + element1, + element2, + element3, + element4, +}.method().chain(); +>>> Allow split in record target without splitting chain. +(element1, element2, element3, element4).method().chain(); +<<< +( + element1, + element2, + element3, + element4, +).method().chain(); +>>> Allow split in function expression target without splitting chain. +(parameter) {body;}.method().chain(); +<<< +(parameter) { + body; +}.method().chain(); +>>> Allow split in switch expression target without splitting chain. +return switch (value) {1 => true, 2 => false, 3 => true}.method().chain(); +<<< +return switch (value) { + 1 => true, + 2 => false, + 3 => true, +}.method().chain(); +>>> Allow split in parenthesized target if inner expression allows it. +(([element1, element2, element3, element4])).method().chain(); +<<< +(([ + element1, + element2, + element3, + element4, +])).method().chain(); +>>> Split in other target expression forces chain to fully split. +(operand1 + operand2 + operand3 + operand4).method().chain(argument); +<<< +(operand1 + + operand2 + + operand3 + + operand4) + .method() + .chain(argument); \ No newline at end of file diff --git a/test/invocation/constructor.stmt b/test/invocation/constructor.stmt index 3c0d1264..ef2b0c32 100644 --- a/test/invocation/constructor.stmt +++ b/test/invocation/constructor.stmt @@ -37,17 +37,14 @@ new prefix.TypeName(argument); const prefix . Thing . name ( argument ) ; <<< const prefix.Thing.name(argument); ->>> Split at name. +>>> Don't split at name. new VeryLongClassName.veryLongNamedConstructor(); <<< -new VeryLongClassName - .veryLongNamedConstructor(); ->>> Split at name on prefixed named constructor. +new VeryLongClassName.veryLongNamedConstructor(); +>>> Don't split at name on prefixed named constructor. new prefix.VeryLongClassName.veryLongNamedConstructor(); <<< -new prefix - .VeryLongClassName - .veryLongNamedConstructor(); +new prefix.VeryLongClassName.veryLongNamedConstructor(); >>> Allow block-formatted argument. new Future(new Duration(1), () { print('I am a callback'); diff --git a/test/invocation/null_aware.stmt b/test/invocation/null_aware.stmt new file mode 100644 index 00000000..0c62cfd5 --- /dev/null +++ b/test/invocation/null_aware.stmt @@ -0,0 +1,13 @@ +40 columns | +>>> Unsplit. +receiver ?. method() ?. getter; +<<< +receiver?.method()?.getter; +>>> In split method chain. +object?.method().method()?.method().method(); +<<< +object + ?.method() + .method() + ?.method() + .method(); \ No newline at end of file