Allow arguments before and after block-formatted functions.
This treats functions differently from collection literal arguments.
After testing this on a large corpus, that seems to produce the best
output. With this change, the arguments preceding and following a series
of functions are treated as completely independent argument lists, each
with their own splitting choice.
Fixes #369.
R=pquitslund@google.com
Review URL: https://chromiumcodereview.appspot.com//1252323003 .
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5c264d8..602970f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,7 @@
+# 0.2.0-rc.4
+
+* Smarter indentation for function arguments (#369).
+
# 0.2.0-rc.3
* Optimize splitting complex lines (#391).
diff --git a/lib/src/argument_list_visitor.dart b/lib/src/argument_list_visitor.dart
index 61e820b..4acb3b4 100644
--- a/lib/src/argument_list_visitor.dart
+++ b/lib/src/argument_list_visitor.dart
@@ -12,58 +12,40 @@
import 'source_visitor.dart';
/// Helper class for [SourceVisitor] that handles visiting and writing an
-/// [ArgumentList], including all of the special code needed to handle block
-/// arguments.
+/// [ArgumentList], including all of the special code needed to handle function
+/// and collection arguments.
class ArgumentListVisitor {
final SourceVisitor _visitor;
final ArgumentList _node;
- /// The positional arguments, in order.
- final List<Expression> _positional;
+ /// The normal arguments preceding any block function arguments.
+ final ArgumentSublist _arguments;
- /// The named arguments, in order.
- final List<Expression> _named;
-
- /// The set of arguments that are valid block literals.
- final Set<Expression> _blockArguments;
-
- /// The number of leading block arguments.
+ /// The contiguous list of block function arguments, if any.
///
- /// If all arguments are block arguments, this counts them.
- final int _leadingBlockArguments;
+ /// Otherwise, this is `null`.
+ final List<Expression> _functions;
- /// The number of trailing block arguments.
+ /// If there are block function arguments, this is the arguments after them.
///
- /// If all arguments are block arguments, this is zero.
- final int _trailingBlockArguments;
-
- /// The rule used to split the bodies of all of the block arguments.
- Rule get _blockArgumentRule {
- // Lazy initialize.
- if (_blockRule == null && _blockArguments.isNotEmpty) {
- _blockRule = new SimpleRule(cost: Cost.splitBlocks);
- }
-
- return _blockRule;
- }
-
- Rule _blockRule;
+ /// Otherwise, this is `null`.
+ final ArgumentSublist _argumentsAfterFunctions;
/// Returns `true` if there is only a single positional argument.
- bool get _isSingle => _positional.length == 1 && _named.isEmpty;
+ bool get _isSingle =>
+ _node.arguments.length == 1 && _node.arguments.single is! NamedExpression;
- /// Whether this argument list has any block arguments that are functions.
- bool get hasFunctionBlockArguments => _blockArguments.any(_isBlockFunction);
-
- bool get hasBlockArguments => _blockArguments.isNotEmpty;
+ /// Whether this argument list has any collection or block function arguments.
+ bool get hasBlockArguments =>
+ _arguments._collections.isNotEmpty || _functions != null;
/// Whether this argument list should force the containing method chain to
/// add a level of block nesting.
bool get nestMethodArguments {
// If there are block arguments, we don't want the method to force them to
// the right.
- if (_blockArguments.isNotEmpty) return false;
+ if (hasBlockArguments) return false;
// Corner case: If there is just a single argument, don't bump the nesting.
// This lets us avoid spurious indentation in cases like:
@@ -75,61 +57,52 @@
}
factory ArgumentListVisitor(SourceVisitor visitor, ArgumentList node) {
- // Assumes named arguments follow all positional ones.
- var positional =
- node.arguments.takeWhile((arg) => arg is! NamedExpression).toList();
- var named = node.arguments.skip(positional.length).toList();
+ // Look for a single contiguous range of block function arguments.
+ var functionsStart;
+ var functionsEnd;
- var blocks = node.arguments.where(_isBlockArgument).toSet();
+ for (var i = 0; i < node.arguments.length; i++) {
+ var argument = node.arguments[i];
+ if (_isBlockFunction(argument)) {
+ if (functionsStart == null) functionsStart = i;
- // Count the leading arguments that are block literals.
- var leadingBlocks = 0;
- for (var argument in node.arguments) {
- if (!blocks.contains(argument)) break;
- leadingBlocks++;
- }
+ // The functions must be one contiguous section.
+ if (functionsEnd != null && functionsEnd != i) {
+ functionsStart = null;
+ functionsEnd = null;
+ break;
+ }
- // Count the trailing arguments that are block literals.
- var trailingBlocks = 0;
- if (leadingBlocks != node.arguments.length) {
- for (var argument in node.arguments.reversed) {
- if (!blocks.contains(argument)) break;
- trailingBlocks++;
+ functionsEnd = i + 1;
}
}
- // If only some of the named arguments are blocks, treat none of them as
- // blocks. Avoids cases like:
- //
- // function(
- // a: arg,
- // b: [
- // ...
- // ]);
- if (trailingBlocks < named.length) trailingBlocks = 0;
-
- // Blocks must all be a prefix or suffix of the argument list (and not
- // both).
- if (leadingBlocks != blocks.length) leadingBlocks = 0;
- if (trailingBlocks != blocks.length) trailingBlocks = 0;
-
- // Ignore any blocks in the middle of the argument list.
- if (leadingBlocks == 0 && trailingBlocks == 0) {
- blocks.clear();
+ if (functionsStart == null) {
+ // No functions, so there is just a single argument list.
+ return new ArgumentListVisitor._(visitor, node,
+ new ArgumentSublist(node.arguments, node.arguments), null, null);
}
- return new ArgumentListVisitor._(visitor, node, positional, named, blocks,
- leadingBlocks, trailingBlocks);
+ // Split the arguments into two independent argument lists with the
+ // functions in the middle.
+ var argumentsBefore = node.arguments.take(functionsStart).toList();
+ var functions = node.arguments.sublist(functionsStart, functionsEnd);
+ var argumentsAfter = node.arguments.skip(functionsEnd).toList();
+
+ return new ArgumentListVisitor._(
+ visitor,
+ node,
+ new ArgumentSublist(node.arguments, argumentsBefore),
+ functions,
+ new ArgumentSublist(node.arguments, argumentsAfter));
}
ArgumentListVisitor._(
this._visitor,
this._node,
- this._positional,
- this._named,
- this._blockArguments,
- this._leadingBlockArguments,
- this._trailingBlockArguments);
+ this._arguments,
+ this._functions,
+ this._argumentsAfterFunctions);
/// Builds chunks for the call chain.
void visit() {
@@ -143,138 +116,45 @@
_visitor.builder.startSpan();
_visitor.token(_node.leftParenthesis);
- var rule = _writePositional();
- _writeNamed(rule);
+ _arguments.visit(_visitor);
+
+ _visitor.builder.endSpan();
+
+ if (_functions != null) {
+ // TODO(rnystrom): It might look better to treat the parameter list of the
+ // first function as if it were an argument in the preceding argument list
+ // instead of just having this little solo split here. That would try to
+ // keep the parameter list with other arguments when possible, and, I
+ // think, generally look nicer.
+ if (_functions.first == _node.arguments.first) {
+ _visitor.soloZeroSplit();
+ } else {
+ _visitor.soloSplit();
+ }
+
+ for (var argument in _functions) {
+ if (argument != _functions.first) _visitor.space();
+
+ _visitor.visit(argument);
+
+ // Write the trailing comma.
+ if (argument != _node.arguments.last) {
+ _visitor.token(argument.endToken.next);
+ }
+ }
+
+ _visitor.builder.startSpan();
+ _argumentsAfterFunctions.visit(_visitor);
+ _visitor.builder.endSpan();
+ }
_visitor.token(_node.rightParenthesis);
- _visitor.builder.endSpan();
_visitor.builder.unnest();
if (_isSingle) _visitor.builder.endSpan();
}
- /// Writes the positional arguments, if any.
- PositionalRule _writePositional() {
- if (_positional.isEmpty) return null;
-
- // Allow splitting after "(".
- var rule;
- if (_positional.length == 1) {
- rule = new SinglePositionalRule(_blockArgumentRule);
- } else {
- // Only count the positional bodies in the positional rule.
- var leadingPositional = _leadingBlockArguments;
- if (_leadingBlockArguments == _node.arguments.length) {
- leadingPositional -= _named.length;
- }
-
- var trailingPositional = _trailingBlockArguments - _named.length;
- rule = new MultiplePositionalRule(
- _blockArgumentRule, leadingPositional, trailingPositional);
- }
-
- _visitor.builder.startRule(rule);
- rule.beforeArgument(_visitor.zeroSplit());
-
- // Try to not split the arguments.
- _visitor.builder.startSpan(Cost.positionalArguments);
-
- for (var argument in _positional) {
- _writeArgument(rule, argument);
-
- // Positional arguments split independently.
- if (argument != _positional.last) {
- rule.beforeArgument(_visitor.split());
- }
- }
-
- _visitor.builder.endSpan();
- _visitor.builder.endRule();
-
- return rule;
- }
-
- /// Writes the named arguments, if any.
- void _writeNamed(PositionalRule rule) {
- if (_named.isEmpty) return;
-
- var positionalRule = rule;
- var namedRule = new NamedRule(_blockArgumentRule);
- _visitor.builder.startRule(namedRule);
-
- // Let the positional args force the named ones to split.
- if (positionalRule != null) {
- positionalRule.setNamedArgsRule(namedRule);
- }
-
- // Split before the first named argument.
- namedRule
- .beforeArguments(_visitor.builder.split(space: _positional.isNotEmpty));
-
- for (var argument in _named) {
- _writeArgument(namedRule, argument);
-
- // Write the split.
- if (argument != _named.last) _visitor.split();
- }
-
- _visitor.builder.endRule();
- }
-
- void _writeArgument(ArgumentRule rule, Expression argument) {
- // If we're about to write a block argument, handle it specially.
- if (_blockArguments.contains(argument)) {
- if (rule != null) rule.beforeBlockArgument();
-
- // Tell it to use the rule we've already created.
- _visitor.setNextLiteralBodyRule(_blockArgumentRule);
- } else if (_node.arguments.length > 1) {
- // Corner case: If there is just a single argument, don't bump the
- // nesting. This lets us avoid spurious indentation in cases like:
- //
- // function(function(() {
- // body;
- // }));
- _visitor.builder.startBlockArgumentNesting();
- }
-
- _visitor.visit(argument);
-
- if (_blockArguments.contains(argument)) {
- if (rule != null) rule.afterBlockArgument();
- } else if (_node.arguments.length > 1) {
- _visitor.builder.endBlockArgumentNesting();
- }
-
- // Write the trailing comma.
- if (argument != _node.arguments.last) {
- _visitor.token(argument.endToken.next);
- }
- }
-
- /// Returns true if [expression] denotes a block argument.
- ///
- /// That means a collection literal or a function expression with a block
- /// body. Block arguments can get special indentation to make them look more
- /// statement-like.
- static bool _isBlockArgument(Expression expression) {
- if (expression is NamedExpression) {
- expression = (expression as NamedExpression).expression;
- }
-
- // TODO(rnystrom): Should we step into parenthesized expressions?
-
- // Collections are bodies.
- if (expression is ListLiteral) return true;
- if (expression is MapLiteral) return true;
-
- // Curly body functions are.
- if (expression is! FunctionExpression) return false;
- var function = expression as FunctionExpression;
- return function.body is BlockFunctionBody;
- }
-
/// Returns `true` if [expression] is a [FunctionExpression] with a block
/// body.
static bool _isBlockFunction(Expression expression) {
@@ -288,3 +168,230 @@
return function.body is BlockFunctionBody;
}
}
+
+/// A range of arguments from a complete argument list.
+///
+/// One of these typically covers all of the arguments in an invocation. But,
+/// when an argument list has block functions in the middle, the arguments
+/// before and after the functions are treated as separate independent lists.
+/// In that case, there will be two of these.
+class ArgumentSublist {
+ /// The full argument list from the AST.
+ final List<Expression> _allArguments;
+
+ /// The positional arguments, in order.
+ final List<Expression> _positional;
+
+ /// The named arguments, in order.
+ final List<Expression> _named;
+
+ /// The arguments that are collection literals that get special formatting.
+ final Set<Expression> _collections;
+
+ /// The number of leading collections.
+ ///
+ /// If all arguments are collections, this counts them.
+ final int _leadingCollections;
+
+ /// The number of trailing collections.
+ ///
+ /// If all arguments are collections, this is zero.
+ final int _trailingCollections;
+
+ /// The rule used to split the bodies of all of the collection arguments.
+ Rule get _collectionRule {
+ // Lazy initialize.
+ if (_collectionRuleField == null && _collections.isNotEmpty) {
+ _collectionRuleField = new SimpleRule(cost: Cost.splitCollections);
+ }
+
+ return _collectionRuleField;
+ }
+
+ Rule _collectionRuleField;
+
+ bool get _hasMultipleArguments => _positional.length + _named.length > 1;
+
+ 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 collections = arguments.where(_isCollectionArgument).toSet();
+
+ // Count the leading arguments that are collection literals.
+ var leadingCollections = 0;
+ for (var argument in arguments) {
+ if (!collections.contains(argument)) break;
+ leadingCollections++;
+ }
+
+ // Count the trailing arguments that are collection literals.
+ var trailingCollections = 0;
+ if (leadingCollections != arguments.length) {
+ for (var argument in arguments.reversed) {
+ if (!collections.contains(argument)) break;
+ trailingCollections++;
+ }
+ }
+
+ // If only some of the named arguments are collections, treat none of them
+ // specially. Avoids cases like:
+ //
+ // function(
+ // a: arg,
+ // b: [
+ // ...
+ // ]);
+ if (trailingCollections < named.length) trailingCollections = 0;
+
+ // Collections must all be a prefix or suffix of the argument list (and not
+ // both).
+ if (leadingCollections != collections.length) leadingCollections = 0;
+ if (trailingCollections != collections.length) trailingCollections = 0;
+
+ // Ignore any collections in the middle of the argument list.
+ if (leadingCollections == 0 && trailingCollections == 0) {
+ collections.clear();
+ }
+
+ return new ArgumentSublist._(allArguments, positional, named, collections,
+ leadingCollections, trailingCollections);
+ }
+
+ ArgumentSublist._(this._allArguments, this._positional, this._named,
+ this._collections, this._leadingCollections, this._trailingCollections);
+
+ void visit(SourceVisitor visitor) {
+ var rule = _visitPositional(visitor);
+ _visitNamed(visitor, rule);
+ }
+
+ /// Writes the positional arguments, if any.
+ PositionalRule _visitPositional(SourceVisitor visitor) {
+ if (_positional.isEmpty) return null;
+
+ // Allow splitting after "(".
+ var rule;
+ if (_positional.length == 1) {
+ rule = new SinglePositionalRule(_collectionRule,
+ splitsOnInnerRules: _allArguments.length > 1 &&
+ !_isCollectionArgument(_positional.first));
+ } else {
+ // Only count the positional bodies in the positional rule.
+ var leadingPositional = _leadingCollections;
+ if (_leadingCollections == _positional.length + _named.length) {
+ leadingPositional -= _named.length;
+ }
+
+ var trailingPositional = _trailingCollections - _named.length;
+ rule = new MultiplePositionalRule(
+ _collectionRule, leadingPositional, trailingPositional);
+ }
+
+ visitor.builder.startRule(rule);
+
+ var chunk;
+ if (_isFirstArgument(_positional.first)) {
+ chunk = visitor.zeroSplit();
+ } else {
+ chunk = visitor.split();
+ }
+ rule.beforeArgument(chunk);
+
+ // Try to not split the arguments.
+ visitor.builder.startSpan(Cost.positionalArguments);
+
+ for (var argument in _positional) {
+ _visitArgument(visitor, rule, argument);
+
+ // Positional arguments split independently.
+ if (argument != _positional.last) {
+ rule.beforeArgument(visitor.split());
+ }
+ }
+
+ visitor.builder.endSpan();
+ visitor.builder.endRule();
+
+ return rule;
+ }
+
+ /// Writes the named arguments, if any.
+ void _visitNamed(SourceVisitor visitor, PositionalRule rule) {
+ if (_named.isEmpty) return;
+
+ var positionalRule = rule;
+ var namedRule = new NamedRule(_collectionRule);
+ visitor.builder.startRule(namedRule);
+
+ // Let the positional args force the named ones to split.
+ if (positionalRule != null) {
+ positionalRule.setNamedArgsRule(namedRule);
+ }
+
+ // Split before the first named argument.
+ namedRule.beforeArguments(visitor.builder.split(
+ space: !_isFirstArgument(_named.first)));
+
+ for (var argument in _named) {
+ _visitArgument(visitor, namedRule, argument);
+
+ // Write the split.
+ if (argument != _named.last) visitor.split();
+ }
+
+ visitor.builder.endRule();
+ }
+
+ void _visitArgument(SourceVisitor visitor, ArgumentRule rule, Expression argument) {
+ // If we're about to write a collection argument, handle it specially.
+ if (_collections.contains(argument)) {
+ if (rule != null) rule.beforeCollection();
+
+ // Tell it to use the rule we've already created.
+ visitor.setNextLiteralBodyRule(_collectionRule);
+ } else if (_hasMultipleArguments) {
+ // Corner case: If there is just a single argument, don't bump the
+ // nesting. This lets us avoid spurious indentation in cases like:
+ //
+ // function(function(() {
+ // body;
+ // }));
+ visitor.builder.startBlockArgumentNesting();
+ }
+
+ visitor.visit(argument);
+
+ if (_collections.contains(argument)) {
+ if (rule != null) rule.afterCollection();
+ } else if (_hasMultipleArguments) {
+ visitor.builder.endBlockArgumentNesting();
+ }
+
+ // Write the trailing comma.
+ if (!_isLastArgument(argument)) {
+ visitor.token(argument.endToken.next);
+ }
+ }
+
+ bool _isFirstArgument(Expression argument) => argument == _allArguments.first;
+
+ bool _isLastArgument(Expression argument) => argument == _allArguments.last;
+
+ /// Returns true if [expression] denotes a collection literal argument.
+ ///
+ /// Similar to block functions, collection arguments can get special
+ /// indentation to make them look more statement-like.
+ static bool _isCollectionArgument(Expression expression) {
+ if (expression is NamedExpression) {
+ expression = (expression as NamedExpression).expression;
+ }
+
+ // TODO(rnystrom): Should we step into parenthesized expressions?
+
+ return expression is ListLiteral || expression is MapLiteral;
+ }
+}
\ No newline at end of file
diff --git a/lib/src/chunk.dart b/lib/src/chunk.dart
index bb2d98b..e92a85b 100644
--- a/lib/src/chunk.dart
+++ b/lib/src/chunk.dart
@@ -293,11 +293,11 @@
/// Splitting inside the brackets of a list with only one element.
static const singleElementList = 2;
- /// Splitting the internals of literal block arguments.
+ /// Splitting the internals of collection literal arguments.
///
/// Used to prefer splitting at the argument boundary over splitting the
- /// block contents.
- static const splitBlocks = 2;
+ /// collection contents.
+ static const splitCollections = 2;
/// Splitting before a type argument or type parameter.
static const typeArgument = 4;
diff --git a/lib/src/rule/argument.dart b/lib/src/rule/argument.dart
index 95768a4..8e08fba 100644
--- a/lib/src/rule/argument.dart
+++ b/lib/src/rule/argument.dart
@@ -9,36 +9,36 @@
/// Base class for a rule that handles argument or parameter lists.
abstract class ArgumentRule extends Rule {
- /// The rule used to split block arguments in the argument list, if any.
- final Rule _blockRule;
+ /// The rule used to split collections in the argument list, if any.
+ final Rule _collectionRule;
/// If true, then inner rules that are written will force this rule to split.
///
- /// Temporarily disabled while writing block arguments so that they can be
+ /// Temporarily disabled while writing collectio arguments so that they can be
/// multi-line without forcing the whole argument list to split.
bool _trackInnerRules = true;
- /// Don't split when an inner block rule splits.
+ /// Don't split when an inner collection rule splits.
bool get splitsOnInnerRules => _trackInnerRules;
/// Creates a new rule for a positional argument list.
///
- /// If [_blockRule] is given, it is the rule used to split the block
- /// arguments in the list.
- ArgumentRule(this._blockRule);
+ /// If [_collectionRule] is given, it is the rule used to split the
+ /// collections in the list.
+ ArgumentRule(this._collectionRule);
- /// Called before a block argument is written.
+ /// Called before a collection argument is written.
///
- /// Disables tracking inner rules while a block argument is being written.
- void beforeBlockArgument() {
+ /// Disables tracking inner rules while a collection argument is written.
+ void beforeCollection() {
assert(_trackInnerRules == true);
_trackInnerRules = false;
}
- /// Called after a block argument is complete.
+ /// Called after a collection argument is complete.
///
- /// Re-enables tracking inner rules after a block argument is complete.
- void afterBlockArgument() {
+ /// Re-enables tracking inner rules.
+ void afterCollection() {
assert(_trackInnerRules == false);
_trackInnerRules = true;
}
@@ -55,9 +55,9 @@
/// Creates a new rule for a positional argument list.
///
- /// If [blockRule] is given, it is the rule used to split the block arguments
- /// in the list.
- PositionalRule(Rule blockRule) : super(blockRule);
+ /// If [collectionRule] is given, it is the rule used to split the collection
+ /// arguments in the list.
+ PositionalRule(Rule collectionRule) : super(collectionRule);
/// Remembers [chunk] as containing the split that occurs right before an
/// argument in the list.
@@ -96,20 +96,26 @@
}
/// Split rule for a call with a single positional argument (which may or may
-/// not be a block argument.)
+/// not be a collection argument.)
class SinglePositionalRule extends PositionalRule {
int get numValues => 2;
- /// If there is only a single argument, allow it to split internally without
- /// forcing a split before the argument.
- bool get splitsOnInnerRules => false;
+ /// If there is only a single non-collection argument, allow it to split
+ /// internally without forcing a split before the argument.
+ final bool splitsOnInnerRules;
+
+ bool hack = false;
/// Creates a new rule for a positional argument list.
///
- /// If [blockRule] is given, it is the rule used to split the block arguments
- /// in the list. If [isSingleArgument] is `true`, then the argument list will
- /// only contain a single argument.
- SinglePositionalRule(Rule blockRule) : super(blockRule);
+ /// If [collectionRule] is given, it is the rule used to split the
+ /// collections in the list. If [splitsOnInnerRules] is `true`, then we will
+ /// split before the argument if the argument itself contains a split.
+ SinglePositionalRule(Rule collectionRule, {bool splitsOnInnerRules})
+ : super(collectionRule),
+ splitsOnInnerRules = splitsOnInnerRules
+ != null
+ ? splitsOnInnerRules : false;
bool isSplit(int value, Chunk chunk) => value == 1;
@@ -117,12 +123,12 @@
var constrained = super.constrain(value, other);
if (constrained != null) return constrained;
- if (other != _blockRule) return null;
+ if (other != _collectionRule) return null;
- // If we aren't splitting any args, we can split the block.
+ // If we aren't splitting any args, we can split the collection.
if (value == 0) return null;
- // We are splitting before a block, so don't let it split internally.
+ // We are splitting before a collection, so don't let it split internally.
return 0;
}
@@ -143,40 +149,41 @@
///
/// Then there is a value that splits before every argument.
///
-/// Finally, if there are block arguments, there is another value that splits
-/// before all of the non-block arguments, but does not split before the block
-/// ones, so that they can split internally.
+/// Finally, if there are collection arguments, there is another value that
+/// splits before all of the non-collection arguments, but does not split
+/// before the collections, so that they can split internally.
class MultiplePositionalRule extends PositionalRule {
- /// The number of leading block arguments.
+ /// The number of leading collection arguments.
///
- /// This and [_trailingBlocks] cannot both be positive. If every argument is
- /// a block, this will be [_arguments.length] and [_trailingBlocks] will be 0.
- final int _leadingBlocks;
+ /// This and [_trailingCollections] cannot both be positive. If every
+ /// argument is a collection, this will be [_arguments.length] and
+ /// [_trailingCollections] will be 0.
+ final int _leadingCollections;
- /// The number of trailing block arguments.
+ /// The number of trailing collections.
///
- /// This and [_leadingBlocks] cannot both be positive.
- final int _trailingBlocks;
+ /// This and [_leadingCollections] cannot both be positive.
+ final int _trailingCollections;
int get numValues {
// Can split before any one argument, none, or all.
var result = 2 + _arguments.length;
- // When there are block arguments, there are two ways we can split on "all"
- // arguments:
+ // When there are collection arguments, there are two ways we can split on
+ // "all" arguments:
//
- // - Split on just the non-block arguments, and force the block arguments
- // to split internally.
- // - Split on all of them including the block arguments, and do not allow
- // the block arguments to split internally.
- if (_leadingBlocks > 0 || _trailingBlocks > 0) result++;
+ // - Split on just the non-collection arguments, and force the collection
+ // arguments to split internally.
+ // - Split on all of them including the collection arguments, and do not
+ // allow the collection arguments to split internally.
+ if (_leadingCollections > 0 || _trailingCollections > 0) result++;
return result;
}
MultiplePositionalRule(
- Rule blockRule, this._leadingBlocks, this._trailingBlocks)
- : super(blockRule);
+ Rule collectionRule, this._leadingCollections, this._trailingCollections)
+ : super(collectionRule);
String toString() => "*Pos${super.toString()}";
@@ -195,24 +202,13 @@
return chunk == _arguments[argument];
}
- // Only split before the non-block arguments. Note that we consider this
- // case to correctly prefer this over the latter case because function
- // block arguments always split internally. Preferring this case ensures we
- // avoid:
- //
- // function( // <-- :(
- // () {
- // ...
- // }),
- // argument,
- // ...
- // argument;
+ // Only split before the non-collection arguments.
if (value == _arguments.length + 1) {
- for (var i = 0; i < _leadingBlocks; i++) {
+ for (var i = 0; i < _leadingCollections; i++) {
if (chunk == _arguments[i]) return false;
}
- for (var i = _arguments.length - _trailingBlocks;
+ for (var i = _arguments.length - _trailingCollections;
i < _arguments.length;
i++) {
if (chunk == _arguments[i]) return false;
@@ -221,7 +217,7 @@
return true;
}
- // Split before all of the arguments, even the block ones.
+ // Split before all of the arguments, even the collections.
return true;
}
@@ -229,37 +225,38 @@
var constrained = super.constrain(value, other);
if (constrained != null) return constrained;
- if (other != _blockRule) return null;
+ if (other != _collectionRule) return null;
- // If we aren't splitting any args, we can split the block.
+ // If we aren't splitting any args, we can split the collection.
if (value == 0) return null;
// Split only before the first argument.
if (value == 1) {
- if (_leadingBlocks > 0) {
- // We are splitting before a block, so don't let it split internally.
+ if (_leadingCollections > 0) {
+ // We are splitting before a collection, so don't let it split
+ // internally.
return 0;
} else {
- // The split is outside of the blocks so they can split or not.
+ // The split is outside of the collections so they can split or not.
return null;
}
}
- // Split before a single argument. If it's in the middle of the block
+ // Split before a single argument. If it's in the middle of the collection
// arguments, don't allow them to split.
if (value <= _arguments.length) {
var argument = _arguments.length - value + 1;
- if (argument < _leadingBlocks) return 0;
- if (argument >= _arguments.length - _trailingBlocks) return 0;
+ if (argument < _leadingCollections) return 0;
+ if (argument >= _arguments.length - _trailingCollections) return 0;
return null;
}
- // Only split before the non-block arguments. This case only comes into
- // play when we do want to split the blocks, so force that here.
+ // Only split before the non-collection arguments. This case only comes into
+ // play when we do want to split the collection, so force that here.
if (value == _arguments.length + 1) return 1;
- // Split before all of the arguments, even the block ones, so don't let
+ // Split before all of the arguments, even the collection, so don't let
// them split.
return 0;
}
@@ -276,7 +273,7 @@
int get numValues => 3;
- NamedRule(Rule blockRule) : super(blockRule);
+ NamedRule(Rule collectionRule) : super(collectionRule);
void beforeArguments(Chunk chunk) {
assert(_first == null);
@@ -300,12 +297,12 @@
var constrained = super.constrain(value, other);
if (constrained != null) return constrained;
- if (other != _blockRule) return null;
+ if (other != _collectionRule) return null;
- // If we aren't splitting any args, we can split the block.
+ // If we aren't splitting any args, we can split the collection.
if (value == 0) return null;
- // Split before all of the arguments, even the block ones, so don't let
+ // Split before all of the arguments, even the collections, so don't let
// them split.
return 0;
}
diff --git a/lib/src/source_visitor.dart b/lib/src/source_visitor.dart
index 3e7c3dc..a59ff5e 100644
--- a/lib/src/source_visitor.dart
+++ b/lib/src/source_visitor.dart
@@ -1676,6 +1676,10 @@
if (elements.isEmpty && rightBracket.precedingComments == null) {
token(leftBracket);
token(rightBracket);
+
+ // Clear this out in case this empty collection is in an argument list.
+ // We don't want this rule to bleed over to some other collection.
+ _nextLiteralBodyRule = null;
return;
}
diff --git a/pubspec.yaml b/pubspec.yaml
index ddc8b42..e05bba0 100644
--- a/pubspec.yaml
+++ b/pubspec.yaml
@@ -1,5 +1,5 @@
name: dart_style
-version: 0.2.0-rc.3
+version: 0.2.0-rc.4.dev
author: Dart Team <misc@dartlang.org>
description: Opinionated, automatic Dart source code formatter.
homepage: https://github.com/dart-lang/dart_style
diff --git a/test/regression/0000/0000.stmt b/test/regression/0000/0000.stmt
index ac9cf4d..8edbac4 100644
--- a/test/regression/0000/0000.stmt
+++ b/test/regression/0000/0000.stmt
@@ -41,8 +41,7 @@
DartType type = localsHandler.substInContext(member.type);
constructorArguments.add(potentiallyCheckOrTrustType(value, type));
}
- },
- includeSuperAndInjectedMembers: true);
+ }, includeSuperAndInjectedMembers: true);
>>> (indent 6)
HLoopBlockInformation info = new HLoopBlockInformation(
HLoopBlockInformation.loopType(loop),
diff --git a/test/regression/0000/0050.stmt b/test/regression/0000/0050.stmt
index d245e97..4bb1449 100644
--- a/test/regression/0000/0050.stmt
+++ b/test/regression/0000/0050.stmt
@@ -9,10 +9,8 @@
onDone: () { print('file is now closed'); },
onError: (e) { print(e.toString()); });
<<<
- inputStream
- .transform(UTF8.decoder)
- .transform(new LineSplitter())
- .listen((String line) {
+ inputStream.transform(UTF8.decoder).transform(new LineSplitter()).listen(
+ (String line) {
print('Got ${line.length} characters from stream');
}, onDone: () {
print('file is now closed');
diff --git a/test/regression/0000/0057.stmt b/test/regression/0000/0057.stmt
index b7598f4..282a5c7 100644
--- a/test/regression/0000/0057.stmt
+++ b/test/regression/0000/0057.stmt
@@ -4,6 +4,9 @@
zero: 'none', one: 'one', other: 'some')} plus some text.",
name: 'embeddedPlural2', desc: 'An embedded plural', args: [n]);
<<<
-embeddedPlural2(n) => Intl.message("${Intl.plural(n,
+embeddedPlural2(n) => Intl.message(
+ "${Intl.plural(n,
zero: 'none', one: 'one', other: 'some')} plus some text.",
- name: 'embeddedPlural2', desc: 'An embedded plural', args: [n]);
\ No newline at end of file
+ name: 'embeddedPlural2',
+ desc: 'An embedded plural',
+ args: [n]);
\ No newline at end of file
diff --git a/test/regression/0300/0369.stmt b/test/regression/0300/0369.stmt
new file mode 100644
index 0000000..fcb8278
--- /dev/null
+++ b/test/regression/0300/0369.stmt
@@ -0,0 +1,26 @@
+>>>
+return log.progress("Loading asset environment", () async {
+ var graph = await entrypoint.loadPackageGraph();
+ graph = _adjustPackageGraph(graph, mode, packages);
+ var barback = new Barback(new PubPackageProvider(graph));
+ barback.log.listen(_log);
+
+ var environment = new AssetEnvironment._(graph, barback, mode,
+ watcherType, hostname, basePort, environmentConstants);
+
+ await environment._load(entrypoints: entrypoints, useDart2JS: useDart2JS);
+ return environment;
+}, fine: true);
+<<<
+return log.progress("Loading asset environment", () async {
+ var graph = await entrypoint.loadPackageGraph();
+ graph = _adjustPackageGraph(graph, mode, packages);
+ var barback = new Barback(new PubPackageProvider(graph));
+ barback.log.listen(_log);
+
+ var environment = new AssetEnvironment._(graph, barback, mode, watcherType,
+ hostname, basePort, environmentConstants);
+
+ await environment._load(entrypoints: entrypoints, useDart2JS: useDart2JS);
+ return environment;
+}, fine: true);
\ No newline at end of file
diff --git a/test/regression/other/analysis_server.unit b/test/regression/other/analysis_server.unit
index decf236..9e662a7 100644
--- a/test/regression/other/analysis_server.unit
+++ b/test/regression/other/analysis_server.unit
@@ -186,4 +186,64 @@
toHtmlVisitor.write(impliedType.humanReadableName);
});
}));
+ }
+>>> (indent 2)
+ main() {
+ manager.setRoots(<String>[
+ projPath
+ ], <String>[], <String, String>{});
+ }
+<<<
+ main() {
+ manager.setRoots(<String>[projPath], <String>[], <String, String>{});
+ }
+>>> (indent 2)
+ main() {
+ server.setAnalysisRoots('0', [
+ '/foo',
+ '/bar'
+ ], [], {});
+ return pumpEventQueue(40).then((_) {
+ expect(server.statusAnalyzing, isFalse);
+ });
+ }
+<<<
+ main() {
+ server.setAnalysisRoots('0', ['/foo', '/bar'], [], {});
+ return pumpEventQueue(40).then((_) {
+ expect(server.statusAnalyzing, isFalse);
+ });
+ }
+>>> (indent 2)
+ main() {
+ return withBuilder(new DartIrBuilder(
+ DART_CONSTANT_SYSTEM,
+ element,
+ // TODO(johnniwinther): Support closure variables.
+ new Set<dart2js.Local>()), () {
+ irBuilder.buildFunctionHeader(
+ constructor.parameters.map(converter.convertElement));
+ // Visit the body directly to avoid processing the signature as
+ // expressions.
+ // Call to allow for `body == null` in case of synthesized constructors.
+ build(body);
+ return irBuilder.makeConstructorDefinition(const [], const []);
+ });
+ }
+<<<
+ main() {
+ return withBuilder(
+ new DartIrBuilder(
+ DART_CONSTANT_SYSTEM,
+ element,
+ // TODO(johnniwinther): Support closure variables.
+ new Set<dart2js.Local>()), () {
+ irBuilder.buildFunctionHeader(
+ constructor.parameters.map(converter.convertElement));
+ // Visit the body directly to avoid processing the signature as
+ // expressions.
+ // Call to allow for `body == null` in case of synthesized constructors.
+ build(body);
+ return irBuilder.makeConstructorDefinition(const [], const []);
+ });
}
\ No newline at end of file
diff --git a/test/splitting/arguments.stmt b/test/splitting/arguments.stmt
index bffa08c..6a292dc 100644
--- a/test/splitting/arguments.stmt
+++ b/test/splitting/arguments.stmt
@@ -176,7 +176,7 @@
'other key': value,
'third key': value
});
->>> non-body non-preceding argument forces all bodies to indent
+>>> non-collection non-preceding argument forces all collections to indent
function([element, element, element, element], argument,
{'key': value, 'other key': value, 'third key': value}, () {;});
<<<
@@ -192,7 +192,6 @@
'key': value,
'other key': value,
'third key': value
- },
- () {
- ;
- });
\ No newline at end of file
+ }, () {
+ ;
+});
\ No newline at end of file
diff --git a/test/splitting/function_arguments.stmt b/test/splitting/function_arguments.stmt
index 450b53a..3da9e71 100644
--- a/test/splitting/function_arguments.stmt
+++ b/test/splitting/function_arguments.stmt
@@ -1,13 +1,10 @@
40 columns |
->>> args before and after function forces nesting
+>>> args before and after function do not force nesting
method(first,() {fn;},third);
<<<
-method(
- first,
- () {
- fn;
- },
- third);
+method(first, () {
+ fn;
+}, third);
>>> nothing but function args does not nest
longFunctionName(() {;}, () {;}, () {;});
<<<
@@ -168,14 +165,21 @@
obj.inner(() {
body;
}));
->>> force named args to split on positional function
+>>> do not force named args to split on positional function
function(argument, () {;},
named: argument, another: argument);
<<<
-function(
- argument,
- () {
- ;
- },
- named: argument,
- another: argument);
\ No newline at end of file
+function(argument, () {
+ ;
+}, named: argument, another: argument);
+>>> args before and after functions split independently
+longFunction(argument, argument, argument, argument, argument,
+() {;}, () {;}, argument, argument, argument, argument, argument);
+<<<
+longFunction(argument, argument,
+ argument, argument, argument, () {
+ ;
+}, () {
+ ;
+}, argument, argument, argument,
+ argument, argument);
\ No newline at end of file
