Migration: Add "why not nullable" traces where "!"s are inserted.

Change-Id: I0367f869d59da0397589df7df43170f1ecbcb69c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/139401
Reviewed-by: Mike Fairhurst <[email protected]>
Commit-Queue: Paul Berry <[email protected]>

Author: stereotype441

pkg/analysis_server/lib/src/edit/nnbd_migration/info_builder.dart

@@ -514,7 +514,27 @@ class InfoBuilder {
     }).toList();
   }
 
-  void _computeTraceInfo(NullabilityNodeInfo node, List<TraceInfo> traces) {
+  void _computeTraceNonNullableInfo(
+      NullabilityNodeInfo node, List<TraceInfo> traces) {
+    List<TraceEntryInfo> entries = [];
+    var step = node.whyNotNullable;
+    if (step == null) {
+      return;
+    }
+    assert(identical(step.node, node));
+    while (step != null) {
+      entries.add(_nodeToTraceEntry(step.node));
+      if (step.codeReference != null) {
+        entries.add(_stepToTraceEntry(step));
+      }
+      step = step.principalCause;
+    }
+    var description = 'Non-nullability reason';
+    traces.add(TraceInfo(description, entries));
+  }
+
+  void _computeTraceNullableInfo(
+      NullabilityNodeInfo node, List<TraceInfo> traces) {
     List<TraceEntryInfo> entries = [];
     var step = node.whyNullable;
     if (step == null) {
@@ -537,12 +557,13 @@ class InfoBuilder {
     for (var reason in fixReasons) {
       if (reason is NullabilityNodeInfo) {
         if (reason.isNullable) {
-          _computeTraceInfo(reason, traces);
+          _computeTraceNullableInfo(reason, traces);
         }
       } else if (reason is EdgeInfo) {
         assert(reason.sourceNode.isNullable);
         assert(!reason.destinationNode.isNullable);
-        _computeTraceInfo(reason.sourceNode, traces);
+        _computeTraceNullableInfo(reason.sourceNode, traces);
+        _computeTraceNonNullableInfo(reason.destinationNode, traces);
       } else {
         assert(false, 'Unrecognized reason type: ${reason.runtimeType}');
       }

pkg/analysis_server/lib/src/edit/nnbd_migration/migration_info.dart

@@ -123,7 +123,7 @@ class NavigationTarget extends NavigationRegion {
   }
 
   @override
-  String toString() => 'NavigationTarget["$filePath", $offset, $length]';
+  String toString() => 'NavigationTarget["$filePath", $line, $offset, $length]';
 }
 
 /// An additional detail related to a region.

pkg/analysis_server/test/src/edit/nnbd_migration/info_builder_test.dart

@@ -1856,6 +1856,58 @@ void h() {
     assertTraceEntry(unit, entries[1], 'f', unit.content.indexOf('int?'));
   }
 
+  Future<void> test_trace_nullCheck_notNullableReason() async {
+    UnitInfo unit = await buildInfoForSingleTestFile('''
+void f(int i) { // f
+  assert(i != null);
+}
+void g(int i) { // g
+  f(i); // call f
+}
+void h(int/*?*/ i) {
+  g(i);
+}
+''', migratedContent: '''
+void f(int i) { // f
+  assert(i != null);
+}
+void g(int i) { // g
+  f(i); // call f
+}
+void h(int?/*?*/ i) {
+  g(i!);
+}
+''');
+    var region = unit.regions
+        .where((regionInfo) => regionInfo.offset == unit.content.indexOf('!)'))
+        .single;
+    expect(region.traces, hasLength(2));
+    // Trace 0 is the nullability reason; we don't care about that right now.
+    // Trace 1 is the non-nullability reason.
+    var trace = region.traces[1];
+    expect(trace.description, 'Non-nullability reason');
+    var entries = trace.entries;
+    expect(entries, hasLength(5));
+    // Entry 0 is the nullability of g's argument
+    assertTraceEntry(
+        unit, entries[0], 'g', unit.content.indexOf('int i) { // g'));
+    // Entry 1 is the edge from g's argument to f's argument, due to g's call to
+    // f.
+    assertTraceEntry(
+        unit, entries[1], 'g', unit.content.indexOf('i); // call f'));
+    // Entry 2 is the nullability of f's argument
+    assertTraceEntry(
+        unit, entries[2], 'f', unit.content.indexOf('int i) { // f'));
+    // Entry 3 is the edge f's argument to never, due to the assert.
+    assertTraceEntry(unit, entries[3], 'f', unit.content.indexOf('assert'));
+    // Entry 4 is the "never" node.
+    // TODO(paulberry): this node provides no additional useful information and
+    // shouldn't be included in the trace.
+    expect(entries[4].description, 'never');
+    expect(entries[4].function, null);
+    expect(entries[4].target, null);
+  }
+
   Future<void> test_trace_substitutionNode() async {
     UnitInfo unit = await buildInfoForSingleTestFile('''
 class C<T extends Object/*!*/> {}
@@ -1875,10 +1927,12 @@ String/*!*/ y = x[0]!;
     var region = unit.regions
         .where((regionInfo) => regionInfo.offset == unit.content.indexOf('!;'))
         .single;
-    // The nullability node associated with adding the `!` is a substitution
+    // The "why nullable" node associated with adding the `!` is a substitution
     // node, and we don't currently generate a trace for a substitution node.
     // TODO(paulberry): fix this.
-    expect(region.traces, isEmpty);
+    // We do, however, generate a trace for "why not nullable".
+    expect(region.traces, hasLength(1));
+    expect(region.traces[0].description, 'Non-nullability reason');
   }
 
   Future<void> test_uninitializedField() async {

pkg/nnbd_migration/lib/instrumentation.dart

@@ -112,7 +112,7 @@ abstract class DecoratedTypeInfo {
   DecoratedTypeInfo typeArgument(int i);
 }
 
-/// Information about a propagation stup that occurred during downstream
+/// Information about a propagation step that occurred during downstream
 /// propagation.
 abstract class DownstreamPropagationStepInfo implements PropagationStepInfo {
   DownstreamPropagationStepInfo get principalCause;
@@ -362,6 +362,10 @@ abstract class NullabilityNodeInfo implements FixReasonInfo {
   /// The edges that caused this node to have the nullability that it has.
   Iterable<EdgeInfo> get upstreamEdges;
 
+  /// If [isNullable] is false, the propagation step that caused this node to
+  /// become non-nullable (if any).
+  UpstreamPropagationStepInfo get whyNotNullable;
+
   /// If [isNullable] is true, the propagation step that caused this node to
   /// become nullable.
   DownstreamPropagationStepInfo get whyNullable;
@@ -388,3 +392,17 @@ abstract class SubstitutionNodeInfo extends NullabilityNodeInfo {
   /// `*` in `T*`.
   NullabilityNodeInfo get outerNode;
 }
+
+/// Information about a propagation step that occurred during upstream
+/// propagation.
+abstract class UpstreamPropagationStepInfo implements PropagationStepInfo {
+  /// The node whose nullability was changed.
+  ///
+  /// Any propagation step that took effect should have a non-null value here.
+  /// Propagation steps that are pending but have not taken effect yet, or that
+  /// never had an effect (e.g. because an edge was not triggered) will have a
+  /// `null` value for this field.
+  NullabilityNodeInfo get node;
+
+  UpstreamPropagationStepInfo get principalCause;
+}

pkg/nnbd_migration/lib/src/nullability_node.dart

@@ -766,8 +766,7 @@ abstract class NullabilityNode implements NullabilityNodeInfo {
   @override
   Iterable<EdgeInfo> get upstreamEdges => _upstreamEdges;
 
-  /// If this node has non-null intent, the propagation step that caused it to
-  /// have non-null intent, otherwise `null`.
+  @override
   UpstreamPropagationStep get whyNotNullable;
 
   String get _jsonKind;
@@ -1167,7 +1166,8 @@ class SimpleExactNullablePropagationStep extends ExactNullablePropagationStep {
 
 /// Propagation step where we mark a node as having non-null intent due to it
 /// being upstream from another node with non-null intent.
-class UpstreamPropagationStep extends PropagationStep {
+class UpstreamPropagationStep extends PropagationStep
+    implements UpstreamPropagationStepInfo {
   @override
   final UpstreamPropagationStep principalCause;
 
@@ -1177,23 +1177,32 @@ class UpstreamPropagationStep extends PropagationStep {
   /// The new state of the node's non-null intent.
   final NonNullIntent newNonNullIntent;
 
+  /// The nullability edge connecting [node] to the node it is upstream from, if
+  /// any.
+  final NullabilityEdge edge;
+
   UpstreamPropagationStep(
-      this.principalCause, this.node, this.newNonNullIntent);
+      this.principalCause, this.node, this.newNonNullIntent, this.edge);
 
   UpstreamPropagationStep.fromJson(
       dynamic json, NullabilityGraphDeserializer deserializer)
       : principalCause = deserializer.stepForId(json['cause'] as int)
             as UpstreamPropagationStep,
         node = deserializer.nodeForId(json['node'] as int),
-        newNonNullIntent = NonNullIntent.fromJson(json['newState']);
+        newNonNullIntent = NonNullIntent.fromJson(json['newState']),
+        edge = deserializer.edgeForId(json['edge'] as int);
+
+  @override
+  CodeReference get codeReference => edge?.codeReference;
 
   @override
   Map<String, Object> toJson(NullabilityGraphSerializer serializer) {
     return {
       'kind': 'upstream',
       'cause': serializer.idForStep(principalCause),
       'node': serializer.idForNode(node),
-      'newState': newNonNullIntent.toJson()
+      'newState': newNonNullIntent.toJson(),
+      'edge': serializer.idForEdge(edge)
     };
   }
 
@@ -1388,7 +1397,7 @@ class _PropagationState {
   void _propagateUpstream() {
     Queue<UpstreamPropagationStep> pendingSteps = Queue();
     pendingSteps
-        .add(UpstreamPropagationStep(null, _never, NonNullIntent.direct));
+        .add(UpstreamPropagationStep(null, _never, NonNullIntent.direct, null));
     while (pendingSteps.isNotEmpty) {
       var cause = pendingSteps.removeFirst();
       var pendingNode = cause.node;
@@ -1408,7 +1417,8 @@ class _PropagationState {
           } else {
             newNonNullIntent = oldNonNullIntent.addIndirect();
           }
-          var step = UpstreamPropagationStep(cause, node, newNonNullIntent);
+          var step =
+              UpstreamPropagationStep(cause, node, newNonNullIntent, edge);
           _setNonNullIntent(step);
           if (!oldNonNullIntent.isPresent) {
             // We did not previously have non-null intent, so we need to
@@ -1426,7 +1436,7 @@ class _PropagationState {
         }
         var oldNonNullIntent = node._nonNullIntent;
         var newNonNullIntent = oldNonNullIntent.addIndirect();
-        var step = UpstreamPropagationStep(cause, node, newNonNullIntent);
+        var step = UpstreamPropagationStep(cause, node, newNonNullIntent, null);
         _setNonNullIntent(step);
         if (!oldNonNullIntent.isPresent) {
           // We did not previously have non-null intent, so we need to