Index: packages/dart_style/lib/src/argument_list_visitor.dart |
diff --git a/packages/dart_style/lib/src/argument_list_visitor.dart b/packages/dart_style/lib/src/argument_list_visitor.dart |
index c45b73724827255098d99f0f86c7a0c6a4045d40..b07198722b2ffd86b87e2ab8c10496de29ccee75 100644 |
--- a/packages/dart_style/lib/src/argument_list_visitor.dart |
+++ b/packages/dart_style/lib/src/argument_list_visitor.dart |
@@ -4,7 +4,10 @@ |
library dart_style.src.argument_list_visitor; |
+import 'dart:math' as math; |
+ |
import 'package:analyzer/analyzer.dart'; |
+import 'package:analyzer/src/generated/scanner.dart'; |
import 'chunk.dart'; |
import 'rule/argument.dart'; |
@@ -37,25 +40,13 @@ class ArgumentListVisitor { |
_node.arguments.length == 1 && _node.arguments.single is! NamedExpression; |
/// Whether this argument list has any collection or block function arguments. |
+ // TODO(rnystrom): Returning true based on collections is non-optimal. It |
+ // forces a method chain to break into two but the result collection may not |
+ // actually split which can lead to a method chain that's allowed to break |
+ // where it shouldn't. |
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 (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: |
- // |
- // object.method(function(() { |
- // body; |
- // })); |
- return _node.arguments.length > 1; |
- } |
- |
factory ArgumentListVisitor(SourceVisitor visitor, ArgumentList node) { |
// Look for a single contiguous range of block function arguments. |
var functionsStart; |
@@ -151,8 +142,8 @@ class ArgumentListVisitor { |
if (_isSingle) _visitor.builder.endSpan(); |
} |
- /// Returns `true` if [expression] is a [FunctionExpression] with a block |
- /// body. |
+ /// Returns `true` if [expression] is a [FunctionExpression] with a non-empty |
+ /// block body. |
static bool _isBlockFunction(Expression expression) { |
if (expression is NamedExpression) { |
expression = (expression as NamedExpression).expression; |
@@ -166,10 +157,23 @@ class ArgumentListVisitor { |
return _isBlockFunction(expression.argumentList.arguments.single); |
} |
- // Curly body functions are. |
+ if (expression is InstanceCreationExpression) { |
+ if (expression.argumentList.arguments.length != 1) return false; |
+ |
+ return _isBlockFunction(expression.argumentList.arguments.single); |
+ } |
+ |
+ // Must be a function. |
if (expression is! FunctionExpression) return false; |
+ |
+ // With a curly body. |
var function = expression as FunctionExpression; |
- return function.body is BlockFunctionBody; |
+ if (function.body is! BlockFunctionBody) return false; |
+ |
+ // That isn't empty. |
+ var body = function.body as BlockFunctionBody; |
+ return body.block.statements.isNotEmpty || |
+ body.block.rightBracket.precedingComments != null; |
} |
/// Returns `true` if [expression] is a valid method invocation target for |
@@ -206,8 +210,9 @@ class ArgumentSublist { |
/// The named arguments, in order. |
final List<Expression> _named; |
- /// The arguments that are collection literals that get special formatting. |
- final Set<Expression> _collections; |
+ /// Maps each argument that is a collection literal that get special |
+ /// formatting to the token for the collection's open bracket. |
+ final Map<Expression, Token> _collections; |
/// The number of leading collections. |
/// |
@@ -220,16 +225,12 @@ class ArgumentSublist { |
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); |
- } |
+ Rule get collectionRule => _collectionRule; |
+ Rule _collectionRule; |
- return _collectionRuleField; |
- } |
- |
- Rule _collectionRuleField; |
+ /// The most recent chunk that split before an argument. |
+ Chunk get previousSplit => _previousSplit; |
+ Chunk _previousSplit; |
bool get _hasMultipleArguments => _positional.length + _named.length > 1; |
@@ -240,12 +241,16 @@ class ArgumentSublist { |
arguments.takeWhile((arg) => arg is! NamedExpression).toList(); |
var named = arguments.skip(positional.length).toList(); |
- var collections = arguments.where(_isCollectionArgument).toSet(); |
+ var collections = {}; |
+ for (var argument in arguments) { |
+ var bracket = _getCollectionBracket(argument); |
+ if (bracket != null) collections[argument] = bracket; |
+ } |
// Count the leading arguments that are collection literals. |
var leadingCollections = 0; |
for (var argument in arguments) { |
- if (!collections.contains(argument)) break; |
+ if (!collections.containsKey(argument)) break; |
leadingCollections++; |
} |
@@ -253,21 +258,11 @@ class ArgumentSublist { |
var trailingCollections = 0; |
if (leadingCollections != arguments.length) { |
for (var argument in arguments.reversed) { |
- if (!collections.contains(argument)) break; |
+ if (!collections.containsKey(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; |
@@ -286,6 +281,10 @@ class ArgumentSublist { |
this._collections, this._leadingCollections, this._trailingCollections); |
void visit(SourceVisitor visitor) { |
+ if (_collections.isNotEmpty) { |
+ _collectionRule = new Rule(Cost.splitCollections); |
+ } |
+ |
var rule = _visitPositional(visitor); |
_visitNamed(visitor, rule); |
} |
@@ -299,85 +298,80 @@ class ArgumentSublist { |
if (_positional.length == 1) { |
rule = new SinglePositionalRule(_collectionRule, |
splitsOnInnerRules: _allArguments.length > 1 && |
- !_isCollectionArgument(_positional.first)); |
+ !_collections.containsKey(_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; |
+ // Only count the collections in the positional rule. |
+ var leadingCollections = |
+ math.min(_leadingCollections, _positional.length); |
+ var trailingCollections = |
+ math.max(_trailingCollections - _named.length, 0); |
rule = new MultiplePositionalRule( |
- _collectionRule, leadingPositional, trailingPositional); |
- } |
- |
- visitor.builder.startRule(rule); |
- |
- var chunk; |
- if (_isFirstArgument(_positional.first)) { |
- chunk = visitor.zeroSplit(); |
- } else { |
- chunk = visitor.split(); |
+ _collectionRule, leadingCollections, trailingCollections); |
} |
- 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(); |
+ _visitArguments(visitor, _positional, rule); |
return rule; |
} |
/// Writes the named arguments, if any. |
- void _visitNamed(SourceVisitor visitor, PositionalRule rule) { |
+ void _visitNamed(SourceVisitor visitor, PositionalRule positionalRule) { |
if (_named.isEmpty) return; |
- var positionalRule = rule; |
- var namedRule = new NamedRule(_collectionRule); |
- visitor.builder.startRule(namedRule); |
+ // Only count the collections in the named rule. |
+ var leadingCollections = |
+ math.max(_leadingCollections - _positional.length, 0); |
+ var trailingCollections = math.min(_trailingCollections, _named.length); |
+ var namedRule = |
+ new NamedRule(_collectionRule, leadingCollections, trailingCollections); |
// 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))); |
+ _visitArguments(visitor, _named, namedRule); |
+ } |
+ |
+ void _visitArguments( |
+ SourceVisitor visitor, List<Expression> arguments, ArgumentRule rule) { |
+ visitor.builder.startRule(rule); |
- for (var argument in _named) { |
- _visitArgument(visitor, namedRule, argument); |
+ // Split before the first argument. |
+ _previousSplit = |
+ visitor.builder.split(space: !_isFirstArgument(arguments.first)); |
+ rule.beforeArgument(_previousSplit); |
+ |
+ // Try to not split the positional arguments. |
+ if (arguments == _positional) { |
+ visitor.builder.startSpan(Cost.positionalArguments); |
+ } |
+ |
+ for (var argument in arguments) { |
+ _visitArgument(visitor, rule, argument); |
// Write the split. |
- if (argument != _named.last) visitor.split(); |
+ if (argument != arguments.last) { |
+ _previousSplit = visitor.split(); |
+ rule.beforeArgument(_previousSplit); |
+ } |
} |
+ if (arguments == _positional) visitor.builder.endSpan(); |
+ |
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 (_collections.containsKey(argument)) { |
if (rule != null) rule.beforeCollection(); |
// Tell it to use the rule we've already created. |
- visitor.setNextLiteralBodyRule(_collectionRule); |
+ visitor.beforeCollection(_collections[argument], this); |
} 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: |
+ // Edge 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; |
@@ -387,7 +381,7 @@ class ArgumentSublist { |
visitor.visit(argument); |
- if (_collections.contains(argument)) { |
+ if (_collections.containsKey(argument)) { |
if (rule != null) rule.afterCollection(); |
} else if (_hasMultipleArguments) { |
visitor.builder.endBlockArgumentNesting(); |
@@ -403,17 +397,22 @@ class ArgumentSublist { |
bool _isLastArgument(Expression argument) => argument == _allArguments.last; |
- /// Returns true if [expression] denotes a collection literal argument. |
+ /// Returns the token for the left bracket 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) { |
+ static Token _getCollectionBracket(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; |
+ if (expression is ListLiteral) return expression.leftBracket; |
+ if (expression is MapLiteral) return expression.leftBracket; |
+ |
+ // Not a collection literal. |
+ return null; |
} |
} |