Migration: Allow edges to be queried to see if they're satisfied.

stereotype441 · commit-bot@chromium.org · commit ea4eaaa145ef · 2019-06-17T21:06:21.000Z
Change-Id: I9da0233dfbac0c7aee168622c554d003c2c5e16c Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/106382 Reviewed-by: Brian Wilkerson <brianwilkerson@google.com> Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
diff --git a/pkg/nnbd_migration/lib/src/nullability_node.dart b/pkg/nnbd_migration/lib/src/nullability_node.dart
@@ -33,10 +33,27 @@ class NullabilityEdge {
 
   bool get hard => _kind != _NullabilityEdgeKind.soft;
 
+  /// Indicates whether nullability was successfully propagated through this
+  /// edge.
+  bool get isSatisfied {
+    if (!_isTriggered) return true;
+    return destinationNode.isNullable;
+  }
+
   bool get isUnion => _kind == _NullabilityEdgeKind.union;
 
   NullabilityNode get primarySource => sources.first;
 
+  /// Indicates whether all the sources of this edge are nullable (and thus
+  /// downstream nullability propagation should try to make the destination node
+  /// nullable, if possible).
+  bool get _isTriggered {
+    for (var source in sources) {
+      if (!source.isNullable) return false;
+    }
+    return true;
+  }
+
   @override
   String toString() {
     var edgeDecorations = <Object>[];
@@ -183,16 +200,10 @@ class NullabilityGraph {
     }
     var pendingSubstitutions = <NullabilityNodeForSubstitution>[];
     while (true) {
-      nextEdge:
       while (pendingEdges.isNotEmpty) {
         var edge = pendingEdges.removeLast();
+        if (!edge._isTriggered) continue;
         var node = edge.destinationNode;
-        for (var source in edge.sources) {
-          if (!source.isNullable) {
-            // Not all sources are nullable, so this edge doesn't apply yet.
-            continue nextEdge;
-          }
-        }
         if (node._state == _NullabilityState.nonNullable) {
           // The node has already been marked as non-nullable, so the edge can't
           // be satisfied.
diff --git a/pkg/nnbd_migration/test/nullability_node_test.dart b/pkg/nnbd_migration/test/nullability_node_test.dart
@@ -481,13 +481,66 @@ class NullabilityNodeTest {
     assertUnsatisfied([edge_always_1, edge_always_2, edge_always_3]);
   }
 
+  test_satisfied_edge_destination_nullable() {
+    var n1 = newNode(1);
+    var edge = connect(always, n1);
+    propagate();
+    assertUnsatisfied([]);
+    expect(edge.isSatisfied, true);
+  }
+
+  test_satisfied_edge_source_non_nullable() {
+    var n1 = newNode(1);
+    var n2 = newNode(1);
+    var edge = connect(n1, n2);
+    propagate();
+    assertUnsatisfied([]);
+    expect(edge.isSatisfied, true);
+  }
+
+  test_satisfied_edge_two_sources_first_non_nullable() {
+    var n1 = newNode(1);
+    var n2 = newNode(1);
+    connect(always, n2);
+    var edge = connect(n1, never, guards: [n2]);
+    propagate();
+    assertUnsatisfied([]);
+    expect(edge.isSatisfied, true);
+  }
+
+  test_satisfied_edge_two_sources_second_non_nullable() {
+    var n1 = newNode(1);
+    var n2 = newNode(1);
+    connect(always, n1);
+    var edge = connect(n1, never, guards: [n2]);
+    propagate();
+    assertUnsatisfied([]);
+    expect(edge.isSatisfied, true);
+  }
+
   test_unconstrainted_node_non_nullable() {
     var n1 = newNode(1);
     propagate();
     expect(n1.isNullable, false);
     assertUnsatisfied([]);
   }
 
+  test_unsatisfied_edge_multiple_sources() {
+    var n1 = newNode(1);
+    connect(always, n1);
+    var edge = connect(always, never, guards: [n1]);
+    propagate();
+    assertUnsatisfied([edge]);
+    expect(edge.isSatisfied, false);
+  }
+
+  test_unsatisfied_edge_single_source() {
+    var edge = connect(always, never);
+    propagate();
+    assertUnsatisfied([edge]);
+    expect(edge.isSatisfied, false);
+  }
+
   void union(NullabilityNode x, NullabilityNode y) {
     graph.union(x, y, _TestEdgeOrigin());
   }
