Roll Observatory deps (charted -> ^0.3.0)

packages/dart_style/lib/src/line_splitting/solve_state.dart

Index: packages/dart_style/lib/src/line_splitting/solve_state.dart
diff --git a/packages/dart_style/lib/src/line_splitting/solve_state.dart b/packages/dart_style/lib/src/line_splitting/solve_state.dart
index b4197561aa9fc9912e3018ef00391b0d09e9c78c..e1e9168c2a2570e90e5c7b149978e27c1776ab0f 100644
--- a/packages/dart_style/lib/src/line_splitting/solve_state.dart
+++ b/packages/dart_style/lib/src/line_splitting/solve_state.dart
@@ -6,6 +6,7 @@ library dart_style.src.line_splitting.solve_state;
 
 import '../debug.dart' as debug;
 import '../rule/rule.dart';
+import '../whitespace.dart';
 import 'line_splitter.dart';
 import 'rule_set.dart';
 
@@ -25,11 +26,21 @@ class SolveState {
   final LineSplitter _splitter;
   final RuleSet _ruleValues;
 
+  /// The set of [Rule]s that are bound by [_ruleValues].
+  ///
+  /// Cached by [_ensureConstraints] for use by [_ensureUnboundConstraints].
+  Set<Rule> _boundRules;
+
+  /// The set of [Rule]s that are not bound by [_ruleValues].
+  ///
+  /// Cached by [_ensureConstraints] for use by [_ensureUnboundConstraints].
+  Set<Rule> _unboundRules;
+
   /// The unbound rules in this state that can be bound to produce new more
   /// refined states.
   ///
   /// Keeping this set small is the key to make the entire line splitter
-  /// perform well. If we consider too make rules at each state, our
+  /// perform well. If we consider too many rules at each state, our
   /// exploration of the solution space is too branchy and we waste time on
   /// dead end solutions.
   ///
@@ -75,6 +86,19 @@ class SolveState {
   /// unbound rules.
   Map<Rule, int> _constraints;
 
+  /// The unbound rule values that are disallowed because they would place
+  /// invalid constraints on the currently bound values.
+  ///
+  /// For example, say rule A is bound to 1 and B is unbound. If B has value
+  /// 1, it constrains A to be 1. Likewise, if B is 2, it constrains A to be
+  /// 2 as well. Since we already know A is 1, that means we know B cannot be
+  /// bound to value 2. This means B is more limited in this state than it
+  /// might be in another state that binds A to a different value.
+  ///
+  /// It's important to track this, because we can't allow to states to overlap
+  /// if one permits more values for some unbound rule than the other does.
+  Map<Rule, Set<int>> _unboundConstraints;
+
   /// The bound rules that appear inside lines also containing unbound rules.
   ///
   /// By appearing in the same line, it means these bound rules may affect the
@@ -87,13 +111,9 @@ class SolveState {
     _calculateCost();
   }
 
-  /// Gets the value to use for [rule], either the bound value or `0` if it
-  /// isn't bound.
-  int getValue(Rule rule) {
-    if (rule is HardSplitRule) return 0;
-
-    return _ruleValues.getValue(rule);
-  }
+  /// Gets the value to use for [rule], either the bound value or
+  /// [Rule.unsplit] if it isn't bound.
+  int getValue(Rule rule) => _ruleValues.getValue(rule);
 
   /// Returns `true` if this state is a better solution to use as the final
   /// result than [other].
@@ -212,11 +232,10 @@ class SolveState {
 
   /// Returns `true` if [other] overlaps this state.
   bool _isOverlapping(SolveState other) {
-    _ensureOverlapFields();
-    other._ensureOverlapFields();
-
     // Lines that contain both bound and unbound rules must have the same
     // bound values.
+    _ensureBoundRulesInUnboundLines();
+    other._ensureBoundRulesInUnboundLines();
     if (_boundRulesInUnboundLines.length !=
         other._boundRulesInUnboundLines.length) {
       return false;
@@ -229,12 +248,30 @@ class SolveState {
       }
     }
 
+    _ensureConstraints();
+    other._ensureConstraints();
     if (_constraints.length != other._constraints.length) return false;
 
     for (var rule in _constraints.keys) {
       if (_constraints[rule] != other._constraints[rule]) return false;
     }
 
+    _ensureUnboundConstraints();
+    other._ensureUnboundConstraints();
+    if (_unboundConstraints.length != other._unboundConstraints.length) {
+      return false;
+    }
+
+    for (var rule in _unboundConstraints.keys) {
+      var disallowed = _unboundConstraints[rule];
+      var otherDisallowed = other._unboundConstraints[rule];
+
+      if (disallowed.length != otherDisallowed.length) return false;
+      for (var value in disallowed) {
+        if (!otherDisallowed.contains(value)) return false;
+      }
+    }
+
     return true;
   }
 
@@ -267,6 +304,8 @@ class SolveState {
 
           // And any expression nesting.
           indent += chunk.nesting.totalUsedIndent;
+
+          if (chunk.indentBlock(getValue)) indent += Indent.expression;
         }
 
         _splits.add(i, indent);
@@ -288,7 +327,8 @@ class SolveState {
 
     // The unbound rules in use by the current line. This will be null after
     // the first long line has completed.
-    var currentLineRules = [];
+    var foundOverflowRules = false;
+    var start = 0;
 
     endLine(int end) {
       // Track lines that went over the length. It is only rules contained in
@@ -298,16 +338,16 @@ class SolveState {
 
         // Only try rules that are in the first long line, since we know at
         // least one of them *will* be split.
-        if (currentLineRules != null && currentLineRules.isNotEmpty) {
-          _liveRules.addAll(currentLineRules);
-          currentLineRules = null;
-        }
-      } else {
-        // The line fit, so don't keep track of its rules.
-        if (currentLineRules != null) {
-          currentLineRules.clear();
+        if (!foundOverflowRules) {
+          for (var i = start; i < end; i++) {
+            if (_addLiveRules(_splitter.chunks[i].rule)) {
+              foundOverflowRules = true;
+            }
+          }
         }
       }
+
+      start = end;
     }
 
     // The set of spans that contain chunks that ended up splitting. We store
@@ -315,6 +355,9 @@ class SolveState {
     // one split occurs in it.
     var splitSpans = new Set();
 
+    // The nesting level of the chunk that ended the previous line.
+    var previousNesting;
+
     for (var i = 0; i < _splitter.chunks.length; i++) {
       var chunk = _splitter.chunks[i];
 
@@ -329,11 +372,37 @@ class SolveState {
         splitSpans.addAll(chunk.spans);
 
         // Include the cost of the nested block.
-        if (chunk.blockChunks.isNotEmpty) {
+        if (chunk.isBlock) {
           cost +=
               _splitter.writer.formatBlock(chunk, _splits.getColumn(i)).cost;
         }
 
+        // Do not allow sequential lines to have the same indentation but for
+        // different reasons. In other words, don't allow different expressions
+        // to claim the same nesting level on subsequent lines.
+        //
+        // A contrived example would be:
+        //
+        //     function(inner(
+        //         argument), second(
+        //         another);
+        //
+        // For the most part, we prevent this by the constraints on splits.
+        // For example, the above can't happen because the split before
+        // "argument", forces the split before "second".
+        //
+        // But there are a couple of squirrely cases where it's hard to prevent
+        // by construction. Instead, this outlaws it by penalizing it very
+        // heavily if it happens to get this far.
+        if (previousNesting != null &&
+            chunk.nesting.totalUsedIndent != 0 &&
+            chunk.nesting.totalUsedIndent == previousNesting.totalUsedIndent &&
+            !identical(chunk.nesting, previousNesting)) {
+          _overflowChars += 10000;
+        }
+
+        previousNesting = chunk.nesting;
+
         // Start the new line.
         length = _splits.getColumn(i);
       } else {
@@ -341,23 +410,12 @@ class SolveState {
 
         // Include the nested block inline, if any.
         length += chunk.unsplitBlockLength;
-
-        // If we might be in the first overly long line, keep track of any
-        // unbound rules we encounter. These are ones that we'll want to try to
-        // bind to shorten the long line.
-        if (currentLineRules != null &&
-            chunk.rule != null &&
-            !chunk.isHardSplit &&
-            !_ruleValues.contains(chunk.rule)) {
-          currentLineRules.add(chunk.rule);
-        }
       }
     }
 
-    // Add the costs for the rules that split.
+    // Add the costs for the rules that have any splits.
     _ruleValues.forEach(_splitter.rules, (rule, value) {
-      // A rule may be bound to zero if another rule constrains it to not split.
-      if (value != 0) cost += rule.cost;
+      if (value != Rule.unsplit) cost += rule.cost;
     });
 
     // Add the costs for the spans containing splits.
@@ -369,46 +427,32 @@ class SolveState {
     _splits.setCost(cost);
   }
 
-  /// Lazily initializes the fields needed to compare two states for overlap.
+  /// Adds [rule] and all of the rules it constrains to the set of [_liveRules].
   ///
-  /// We do this lazily because the calculation is a bit slow, and is only
-  /// needed when we have two states with the same score.
-  void _ensureOverlapFields() {
-    if (_constraints != null) return;
-
-    _calculateConstraints();
-    _calculateBoundRulesInUnboundLines();
-  }
+  /// Only does this if [rule] is a valid soft rule. Returns `true` if any new
+  /// live rules were added.
+  bool _addLiveRules(Rule rule) {
+    if (rule == null) return false;
 
-  /// Initializes [_constraints] with any constraints the bound rules place on
-  /// the unbound rules.
-  void _calculateConstraints() {
-    _constraints = {};
-
-    var unboundRules = [];
-    var boundRules = [];
+    var added = false;
+    for (var constrained in rule.allConstrainedRules) {
+      if (_ruleValues.contains(constrained)) continue;
 
-    for (var rule in _splitter.rules) {
-      if (_ruleValues.contains(rule)) {
-        boundRules.add(rule);
-      } else {
-        unboundRules.add(rule);
-      }
+      _liveRules.add(constrained);
+      added = true;
     }
 
-    for (var bound in boundRules) {
-      var value = _ruleValues.getValue(bound);
-
-      for (var unbound in unboundRules) {
-        var constraint = bound.constrain(value, unbound);
-        if (constraint != null) {
-          _constraints[unbound] = constraint;
-        }
-      }
-    }
+    return added;
   }
 
-  void _calculateBoundRulesInUnboundLines() {
+  /// Lazily initializes the [_boundInUnboundLines], which is needed to compare
+  /// two states for overlap.
+  ///
+  /// We do this lazily because the calculation is a bit slow, and is only
+  /// needed when we have two states with the same score.
+  void _ensureBoundRulesInUnboundLines() {
+    if (_boundRulesInUnboundLines != null) return;
+
     _boundRulesInUnboundLines = new Set();
 
     var boundInLine = new Set();
@@ -423,7 +467,7 @@ class SolveState {
       }
 
       var rule = _splitter.chunks[i].rule;
-      if (rule != null && rule is! HardSplitRule) {
+      if (rule != null) {
         if (_ruleValues.contains(rule)) {
           boundInLine.add(rule);
         } else {
@@ -435,6 +479,89 @@ class SolveState {
     if (hasUnbound) _boundRulesInUnboundLines.addAll(boundInLine);
   }
 
+  /// Lazily initializes the [_constraints], which is needed to compare two
+  /// states for overlap.
+  ///
+  /// We do this lazily because the calculation is a bit slow, and is only
+  /// needed when we have two states with the same score.
+  void _ensureConstraints() {
+    if (_constraints != null) return;
+
+    _unboundRules = new Set();
+    _boundRules = new Set();
+
+    for (var rule in _splitter.rules) {
+      if (_ruleValues.contains(rule)) {
+        _boundRules.add(rule);
+      } else {
+        _unboundRules.add(rule);
+      }
+    }
+
+    _constraints = {};
+
+    for (var bound in _boundRules) {
+      for (var unbound in bound.constrainedRules) {
+        if (!_unboundRules.contains(unbound)) continue;
+
+        var value = _ruleValues.getValue(bound);
+        var constraint = bound.constrain(value, unbound);
+        if (constraint != null) {
+          _constraints[unbound] = constraint;
+        }
+      }
+    }
+  }
+
+  /// Lazily initializes the [_unboundConstraints], which is needed to compare
+  /// two states for overlap.
+  ///
+  /// We do this lazily because the calculation is a bit slow, and is only
+  /// needed when we have two states with the same score.
+  void _ensureUnboundConstraints() {
+    if (_unboundConstraints != null) return;
+
+    // _ensureConstraints should be called first which initializes these.
+    assert(_boundRules != null);
+    assert(_unboundRules != null);
+
+    _unboundConstraints = {};
+
+    for (var unbound in _unboundRules) {
+      var disallowedValues;
+
+      for (var bound in unbound.constrainedRules) {
+        if (!_boundRules.contains(bound)) continue;
+
+        var boundValue = _ruleValues.getValue(bound);
+
+        for (var value = 0; value < unbound.numValues; value++) {
+          var constraint = unbound.constrain(value, bound);
+
+          // If the unbound rule doesn't place any constraint on this bound
+          // rule, we're fine.
+          if (constraint == null) continue;
+
+          // If the bound rule's value already meets the constraint it applies,
+          // we don't need to track it. This way, two states that have the
+          // same bound value, one of which has a satisfied constraint, are
+          // still allowed to overlap.
+          if (constraint == boundValue) continue;
+          if (constraint == Rule.mustSplit && boundValue != Rule.unsplit) {
+            continue;
+          }
+
+          if (disallowedValues == null) {
+            disallowedValues = new Set();
+            _unboundConstraints[unbound] = disallowedValues;
+          }
+
+          disallowedValues.add(value);
+        }
+      }
+    }
+  }
+
   String toString() {
     var buffer = new StringBuffer();