Migration: add isUpstreamTriggered getter

It would have been very useful in helping to track down
#38341 (comment)
to have this information surfaced through the instrumentation output.

Specifically, it would be nice if the user could hover over a type
annotation that was *not* made nullable due to a propagationStep whose
`reason` was `StateChangeReason.upstream`, and see information about
all the edges for which the `sourceNode` corresponds to that type and
`isUpstreamTriggered` returns `true`.  These are, in effect, the edges
that prevented the type from being marked nullable.

(Note that there's another possible reason a type might not be marked
nullable.  It might be that there are no edges for which the
`destinationNode` corresponds to that type and `isTriggered` returns
`true`.  In this case, the node is indeterminate, so the migration
engine left it as non-nullable because there was no need to make it
nullable).

Change-Id: I9b969201b813496d41f3a373454c2659f683178d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/121941
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>

Author: stereotype441

pkg/nnbd_migration/lib/instrumentation.dart

@@ -87,6 +87,11 @@ abstract class EdgeInfo implements FixReasonInfo {
   /// The [isHard] property is always true for union edges.
   bool get isUnion;
 
+  /// Indicates whether the downstream node of this edge is non-nullable and the
+  /// edge is hard (and thus upstream nullability propagation should try to make
+  /// the source node non-nullable, if possible).
+  bool get isUpstreamTriggered;
+
   /// Information about the graph node that this edge "points away from".
   NullabilityNodeInfo get sourceNode;
 }

pkg/nnbd_migration/lib/src/nullability_node.dart

@@ -49,6 +49,13 @@ class NullabilityEdge implements EdgeInfo {
   @override
   bool get isUnion => _kind == _NullabilityEdgeKind.union;
 
+  @override
+  bool get isUpstreamTriggered {
+    if (!isHard) return false;
+    if (destinationNode._state != NullabilityState.nonNullable) return false;
+    return true;
+  }
+
   @override
   NullabilityNode get sourceNode => upstreamNodes.first;
 
@@ -277,6 +284,9 @@ class NullabilityGraph {
     _pendingEdges.addAll(never._upstreamEdges);
     while (_pendingEdges.isNotEmpty) {
       var edge = _pendingEdges.removeLast();
+      // We only propagate for nodes that are "upstream triggered".  At this
+      // point of propagation, a node is upstream triggered if it is hard.
+      assert(edge.isUpstreamTriggered == edge.isHard);
       if (!edge.isHard) continue;
       var node = edge.sourceNode;
       if (node is NullabilityNodeMutable &&

pkg/nnbd_migration/test/instrumentation_test.dart

@@ -284,6 +284,70 @@ main() {
     expect(kToL.isSatisfied, true);
   }
 
+  test_graphEdge_isUpstreamTriggered() async {
+    await analyze('''
+void f(int i, bool b) {
+  assert(i != null);
+  i.isEven; // unconditional
+  g(i);
+  h(i);
+  if (b) {
+    i.isEven; // conditional
+  }
+}
+void g(int/*?*/ j) {}
+void h(int k) {}
+''');
+    var iNode = explicitTypeNullability[findNode.typeAnnotation('int i')];
+    var jNode = explicitTypeNullability[findNode.typeAnnotation('int/*?*/ j')];
+    var kNode = explicitTypeNullability[findNode.typeAnnotation('int k')];
+    var assertNode = findNode.statement('assert');
+    var unconditionalUsageNode = findNode.simple('i.isEven; // unconditional');
+    var conditionalUsageNode = findNode.simple('i.isEven; // conditional');
+    var nonNullEdges = edgeOrigin.entries
+        .where((entry) =>
+            entry.key.sourceNode == iNode && entry.key.destinationNode == never)
+        .toList();
+    var assertEdge = nonNullEdges
+        .where((entry) => entry.value.node == assertNode)
+        .single
+        .key;
+    var unconditionalUsageEdge = edgeOrigin.entries
+        .where((entry) => entry.value.node == unconditionalUsageNode)
+        .single
+        .key;
+    var gCallEdge = edges
+        .where((e) => e.sourceNode == iNode && e.destinationNode == jNode)
+        .single;
+    var hCallEdge = edges
+        .where((e) => e.sourceNode == iNode && e.destinationNode == kNode)
+        .single;
+    var conditionalUsageEdge = edgeOrigin.entries
+        .where((entry) => entry.value.node == conditionalUsageNode)
+        .single
+        .key;
+    // Both assertEdge and unconditionalUsageEdge are upstream triggered because
+    // either of them would have been sufficient to cause i to be marked as
+    // non-nullable, even though only one of them was actually reported to have
+    // done so via propagationStep.
+    expect(propagationSteps.where((s) => s.node == iNode), hasLength(1));
+    expect(assertEdge.isUpstreamTriggered, true);
+    expect(unconditionalUsageEdge.isUpstreamTriggered, true);
+    // conditionalUsageEdge is not upstream triggered because it is a soft edge,
+    // so it would not have caused i to be marked as non-nullable.
+    expect(conditionalUsageEdge.isUpstreamTriggered, false);
+    // Even though gCallEdge is a hard edge, it is not upstream triggered
+    // because its destination node is nullable.
+    expect(gCallEdge.isHard, true);
+    expect(gCallEdge.isUpstreamTriggered, false);
+    // Even though hCallEdge is a hard edge and its destination node is
+    // non-nullable, it is not upstream triggered because k could have been made
+    // nullable without causing any problems, so the presence of this edge would
+    // not have caused i to be marked as non-nullable.
+    expect(hCallEdge.isHard, true);
+    expect(hCallEdge.isUpstreamTriggered, false);
+  }
+
   test_graphEdge_origin() async {
     await analyze('''
 int f(int x) => x;
@@ -817,7 +881,7 @@ List<Object> f(List l) => l;
         hasLength(1));
   }
 
-  test_propagationStep() async {
+  test_propagationStep_downstream() async {
     await analyze('''
 int x = null;
 ''');
@@ -829,6 +893,20 @@ int x = null;
     expect(step.edge.destinationNode, xNode);
   }
 
+  test_propagationStep_upstream() async {
+    await analyze('''
+void f(int x) {
+  assert(x != null);
+}
+''');
+    var xNode = explicitTypeNullability[findNode.typeAnnotation('int')];
+    var step = propagationSteps.where((s) => s.node == xNode).single;
+    expect(step.newState, NullabilityState.nonNullable);
+    expect(step.reason, StateChangeReason.upstream);
+    expect(step.edge.sourceNode, xNode);
+    expect(step.edge.destinationNode, never);
+  }
+
   test_substitutionNode() async {
     await analyze('''
 class C<T> {