More sophisticated handling of parameter metadata.

lib/src/source_visitor.dart

Index: lib/src/source_visitor.dart
diff --git a/lib/src/source_visitor.dart b/lib/src/source_visitor.dart
index 3ade12dc988321bc2b676e04ff96144981daf992..3053e1f431bfa1a3cdb0f4f753bedd36879af2a8 100644
--- a/lib/src/source_visitor.dart
+++ b/lib/src/source_visitor.dart
@@ -15,6 +15,7 @@ import 'chunk_builder.dart';
 import 'dart_formatter.dart';
 import 'rule/argument.dart';
 import 'rule/combinator.dart';
+import 'rule/metadata.dart';
 import 'rule/rule.dart';
 import 'rule/type_argument.dart';
 import 'source_code.dart';
@@ -59,6 +60,14 @@ class SourceVisitor implements AstVisitor {
   /// split.
   final List<bool> _collectionSplits = [];
 
+  /// The stack of current rules for handling parameter metadata.
+  ///
+  /// Each time a parameter (or type parameter) list is begun, a single rule
+  /// for all of the metadata annotations on parameters in that list is pushed
+  /// onto this stack. We reuse this rule for all annotations so that they split
+  /// in unison.
+  final List<MetadataRule> _metadataRules = [];
+
   /// The mapping for collection literals that are managed by the argument
   /// list that contains them.
   ///
@@ -789,6 +798,8 @@ class SourceVisitor implements AstVisitor {
     builder.nestExpression();
     token(node.leftParenthesis);
 
+    _metadataRules.add(new MetadataRule());
+
     var rule;
     if (requiredParams.isNotEmpty) {
       if (requiredParams.length > 1) {
@@ -797,6 +808,8 @@ class SourceVisitor implements AstVisitor {
         rule = new SinglePositionalRule(null);
       }
 
+      _metadataRules.last.bindPositionalRule(rule);
+
       builder.startRule(rule);
       if (_isInLambda(node)) {
         // Don't allow splitting before the first argument (i.e. right after
@@ -827,6 +840,8 @@ class SourceVisitor implements AstVisitor {
       var namedRule = new NamedRule();
       if (rule != null) rule.setNamedArgsRule(namedRule);
 
+      _metadataRules.last.bindNamedRule(namedRule);
+
       builder.startRule(namedRule);
 
       // Make sure multi-line default values are indented.
@@ -853,6 +868,8 @@ class SourceVisitor implements AstVisitor {
       token(node.rightDelimiter);
     }
 
+    _metadataRules.removeLast();
+
     token(node.rightParenthesis);
     builder.unnest();
   }
@@ -1465,7 +1482,11 @@ class SourceVisitor implements AstVisitor {
   }
 
   visitTypeParameterList(TypeParameterList node) {
+    _metadataRules.add(new MetadataRule());
+
     _visitGenericList(node.leftBracket, node.rightBracket, node.typeParameters);
+
+    _metadataRules.removeLast();
   }
 
   visitVariableDeclaration(VariableDeclaration node) {
@@ -1570,11 +1591,29 @@ class SourceVisitor implements AstVisitor {
   /// These are always on the same line as the parameter.
   void visitParameterMetadata(
       NodeList<Annotation> metadata, void visitParameter()) {
+    if (metadata == null || metadata.isEmpty) {
+      visitParameter();
+      return;
+    }
+
     // Split before all of the annotations or none.
-    builder.startRule();
-    visitNodes(metadata, between: split, after: split);
+    builder.startLazyRule(_metadataRules.last);
+
+    visitNodes(metadata, between: split, after: () {
+      // Don't nest until right before the last metadata. Ensures we only
+      // indent the parameter and not any of the metadata:
+      //
+      //     function(
+      //         @LongAnnotation
+      //         @LongAnnotation
+      //             indentedParameter) {}
+      builder.nestExpression(now: true);
+      split();
+    });
     visitParameter();
 
+    builder.unnest();
+
     // Wrap the rule around the parameter too. If it splits, we want to force
     // the annotations to split as well.
     builder.endRule();
@@ -1603,6 +1642,9 @@ class SourceVisitor implements AstVisitor {
     builder.startSpan();
     builder.nestExpression();
 
+    // TODO(rnystrom): Should the MetadataRule constrain a TypeArgumentRule
+    // as well?

[nweiz, 2015/11/30 19:17:01]

The doc comment for MetadataRule implies that it does. If it doesn't yet, that
comment should be updated.

[Bob Nystrom, 2015/11/30 21:36:14]

Done.

+
     token(leftBracket);
     rule.beforeArgument(zeroSplit());