Add support for details and implement some initial details for regions

Details provide a way for clients to provide navigation support. I'm
imagining that the description could be used as link text.

The deatils are currently more of a data dump than information. I need
to see more migration results in order to figure out when and how to
pare down the data into actionable information.

We might want to capture all of the navigation targets as we build the
info so that we can create anchors in the HTML.

Change-Id: Ie48622ce7d935d3c028ea42e51cfc8da590db149
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/118908
Commit-Queue: Brian Wilkerson <[email protected]>
Reviewed-by: Samuel Rawlins <[email protected]>

Author: bwilkerson

pkg/analysis_server/lib/src/edit/fix/non_nullable_fix.dart

@@ -214,8 +214,9 @@ analyzer:
 
   /// Generate output into the given [folder].
   void _generateOutput(OverlayResourceProvider provider, Folder folder) async {
-    List<LibraryInfo> libraryInfos = await InfoBuilder(listener.server)
-        .explainMigration(instrumentationListener.data, listener);
+    List<LibraryInfo> libraryInfos =
+        await InfoBuilder(instrumentationListener.data, listener)
+            .explainMigration();
     listener.addDetail('libraryInfos has ${libraryInfos.length} libs');
     for (LibraryInfo info in libraryInfos) {
       var pathContext = provider.pathContext;

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

@@ -8,21 +8,42 @@ import 'package:analysis_server/src/edit/nnbd_migration/instrumentation_informat
 import 'package:analysis_server/src/edit/nnbd_migration/migration_info.dart';
 import 'package:analyzer/dart/analysis/results.dart';
 import 'package:analyzer/dart/analysis/session.dart';
+import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/src/generated/source.dart';
-import 'package:analyzer_plugin/protocol/protocol_common.dart';
+import 'package:analyzer_plugin/protocol/protocol_common.dart'
+    show Location, SourceEdit, SourceFileEdit;
+import 'package:nnbd_migration/instrumentation.dart';
+import 'package:nnbd_migration/nnbd_migration.dart';
+
+class FixInfo {
+  /// The fix being described.
+  SingleNullabilityFix fix;
+
+  /// The reasons why the fix was made.
+  List<FixReasonInfo> reasons;
+
+  /// Initialize information about a fix from the given map [entry].
+  FixInfo(this.fix, this.reasons);
+}
 
 /// A builder used to build the migration information for a library.
 class InfoBuilder {
-  /// The analysis session used to get information about libraries.
-  AnalysisServer server;
+  /// The instrumentation information gathered while the migration engine was
+  /// running.
+  final InstrumentationInformation info;
+
+  /// The listener used to gather the changes to be applied.
+  final DartFixListener listener;
 
   /// Initialize a newly created builder.
-  InfoBuilder(this.server);
+  InfoBuilder(this.info, this.listener);
+
+  /// The analysis server used to get information about libraries.
+  AnalysisServer get server => listener.server;
 
   /// Return the migration information for all of the libraries that were
   /// migrated.
-  Future<List<LibraryInfo>> explainMigration(
-      InstrumentationInformation info, DartFixListener listener) async {
+  Future<List<LibraryInfo>> explainMigration() async {
     Map<Source, SourceInformation> sourceInfo = info.sourceInformation;
     List<LibraryInfo> libraries = [];
     for (Source source in sourceInfo.keys) {
@@ -38,6 +59,42 @@ class InfoBuilder {
     return libraries;
   }
 
+  /// Compute the details for the fix with the given [fixInfo].
+  List<RegionDetail> _computeDetails(FixInfo fixInfo) {
+    List<RegionDetail> details = [];
+    for (FixReasonInfo reason in fixInfo.reasons) {
+      if (reason is NullabilityNodeInfo) {
+        for (EdgeInfo edge in reason.upstreamEdges) {
+          EdgeOriginInfo origin = info.edgeOrigin[edge];
+          if (origin != null) {
+            AstNode node = origin.node;
+            if (node.parent is ArgumentList) {
+              if (node is NullLiteral) {
+                details.add(RegionDetail(
+                    'null is explicitly passed as an argument.',
+                    _targetFor(origin)));
+              } else {
+                details.add(RegionDetail(
+                    'A nullable value is explicitly passed as an argument.',
+                    _targetFor(origin)));
+              }
+            } else {
+              details.add(RegionDetail(
+                  'A nullable value is assigned.', _targetFor(origin)));
+            }
+          }
+        }
+      } else if (reason is EdgeInfo) {
+        // TODO(brianwilkerson) Implement this after finding an example whose
+        //  reason is an edge.
+      } else {
+        throw UnimplementedError(
+            'Unexpected class of reason: ${reason.runtimeType}');
+      }
+    }
+    return details;
+  }
+
   /// Return the migration information for the given library.
   LibraryInfo _explainLibrary(
       ParsedLibraryResult result,
@@ -47,13 +104,14 @@ class InfoBuilder {
     List<UnitInfo> units = [];
     for (ParsedUnitResult unit in result.units) {
       SourceFileEdit edit = listener.sourceChange.getFileEdit(unit.path);
-      units.add(_explainUnit(unit, edit));
+      units.add(_explainUnit(sourceInfo, unit, edit));
     }
     return LibraryInfo(units);
   }
 
   /// Return the migration information for the given unit.
-  UnitInfo _explainUnit(ParsedUnitResult result, SourceFileEdit fileEdit) {
+  UnitInfo _explainUnit(SourceInformation sourceInfo, ParsedUnitResult result,
+      SourceFileEdit fileEdit) {
     List<RegionInfo> regions = [];
     String content = result.content;
     // [fileEdit] is null when a file has no edits.
@@ -83,14 +141,35 @@ class InfoBuilder {
       int delta = deltas[index--];
       // Insert the replacement text without deleting the replaced text.
       content = content.replaceRange(end, end, replacement);
+      FixInfo fixInfo = _findFixInfo(sourceInfo, offset);
+      String explanation = '${fixInfo.fix.description.appliedMessage}.';
+      List<RegionDetail> details = _computeDetails(fixInfo);
       if (length > 0) {
-        // TODO(brianwilkerson) Create a sensible explanation.
-        regions.add(RegionInfo(offset + delta, length, 'removed'));
+        regions.add(RegionInfo(offset + delta, length, explanation, details));
       }
-      // TODO(brianwilkerson) Create a sensible explanation.
-      regions.add(RegionInfo(end + delta, replacement.length, 'added'));
+      regions.add(
+          RegionInfo(end + delta, replacement.length, explanation, details));
     }
     regions.sort((first, second) => first.offset.compareTo(second.offset));
     return UnitInfo(result.path, content, regions);
   }
+
+  /// Return information about the fix that was applied at the given [offset],
+  /// or `null` if the information could not be found. The information is
+  /// extracted from the [sourceInfo].
+  FixInfo _findFixInfo(SourceInformation sourceInfo, int offset) {
+    for (MapEntry<SingleNullabilityFix, List<FixReasonInfo>> entry
+        in sourceInfo.fixes.entries) {
+      Location location = entry.key.location;
+      if (location.offset == offset) {
+        return FixInfo(entry.key, entry.value);
+      }
+    }
+    return null;
+  }
+
+  NavigationTarget _targetFor(EdgeOriginInfo origin) {
+    AstNode node = origin.node;
+    return NavigationTarget(origin.source.fullName, node.offset, node.length);
+  }
 }

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

@@ -12,6 +12,34 @@ class LibraryInfo {
   LibraryInfo(this.units);
 }
 
+/// A location to which a user might want to navigate.
+class NavigationTarget {
+  /// The file containing the anchor.
+  final String filePath;
+
+  /// The offset of the anchor.
+  final int offset;
+
+  /// The length of the anchor.
+  final int length;
+
+  /// Initialize a newly created anchor.
+  const NavigationTarget(this.filePath, this.offset, this.length);
+}
+
+/// An additional detail related to a region.
+class RegionDetail {
+  /// A textual description of the detail.
+  final String description;
+
+  /// The location associated with the detail, such as the location of an
+  /// argument that's assigned to a parameter.
+  final NavigationTarget target;
+
+  /// Initialize a newly created detail.
+  RegionDetail(this.description, this.target);
+}
+
 /// A description of an explanation associated with a region of code that was
 /// modified.
 class RegionInfo {
@@ -24,8 +52,11 @@ class RegionInfo {
   /// The explanation to be displayed for the region.
   final String explanation;
 
+  /// Details that further explain why a change was made.
+  final List<RegionDetail> details;
+
   /// Initialize a newly created region.
-  RegionInfo(this.offset, this.length, this.explanation);
+  RegionInfo(this.offset, this.length, this.explanation, this.details);
 }
 
 /// The migration information associated with a single compilation unit.

pkg/analysis_server/test/edit/nnbd_migration/instrumentation_output_test.dart

@@ -20,11 +20,11 @@ class InstrumentationRendererTest extends AbstractAnalysisTest {
   test_outputContainsEachPath() async {
     LibraryInfo info = LibraryInfo([
       UnitInfo('/lib/a.dart', 'int? a = null;',
-          [RegionInfo(3, 1, 'null was assigned')]),
+          [RegionInfo(3, 1, 'null was assigned', [])]),
       UnitInfo('/lib/part1.dart', 'int? b = null;',
-          [RegionInfo(3, 1, 'null was assigned')]),
+          [RegionInfo(3, 1, 'null was assigned', [])]),
       UnitInfo('/lib/part2.dart', 'int? c = null;',
-          [RegionInfo(3, 1, 'null was assigned')]),
+          [RegionInfo(3, 1, 'null was assigned', [])]),
     ]);
     String output = InstrumentationRenderer(info).render();
     expect(output, contains('<h2>/lib/a.dart</h2>'));
@@ -35,7 +35,7 @@ class InstrumentationRendererTest extends AbstractAnalysisTest {
   test_outputContainsEscapedHtml() async {
     LibraryInfo info = LibraryInfo([
       UnitInfo('/lib/a.dart', 'List<String>? a = null;',
-          [RegionInfo(12, 1, 'null was assigned')]),
+          [RegionInfo(12, 1, 'null was assigned', [])]),
     ]);
     String output = InstrumentationRenderer(info).render();
     expect(
@@ -55,7 +55,7 @@ class InstrumentationRendererTest extends AbstractAnalysisTest {
   test_outputContainsModifiedAndUnmodifiedRegions() async {
     LibraryInfo info = LibraryInfo([
       UnitInfo('/lib/a.dart', 'int? a = null;',
-          [RegionInfo(3, 1, 'null was assigned')]),
+          [RegionInfo(3, 1, 'null was assigned', [])]),
     ]);
     String output = InstrumentationRenderer(info).render();
     expect(

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

@@ -48,9 +48,9 @@ class InfoBuilderTest extends AbstractAnalysisTest {
     migration.processInput(result);
     migration.finish();
     // Build the migration info.
-    InfoBuilder builder = InfoBuilder(server);
     InstrumentationInformation info = instrumentationListener.data;
-    infos = await builder.explainMigration(info, listener);
+    InfoBuilder builder = InfoBuilder(info, listener);
+    infos = await builder.explainMigration();
   }
 
   test_parameter_nullableFromInvocation() async {

pkg/nnbd_migration/lib/instrumentation.dart

@@ -179,6 +179,9 @@ abstract class NullabilityNodeInfo implements FixReasonInfo {
   /// After migration is complete, this getter can be used to query whether
   /// the type associated with this node was determined to be nullable.
   bool get isNullable;
+
+  /// The edges that caused this node to have the nullability that it has.
+  Iterable<EdgeInfo> get upstreamEdges;
 }
 
 /// Information exposed to the migration client about a single step in the

pkg/nnbd_migration/lib/nnbd_migration.dart

@@ -36,14 +36,17 @@ class NullabilityFixDescription {
   factory NullabilityFixDescription.addRequired(
           String className, String functionName, String paramName) =>
       NullabilityFixDescription._(
-          appliedMessage: "Add 'required' keyword to parameter $paramName in " +
-              (className == null ? functionName : '$className.$functionName'));
+          appliedMessage:
+              "Add 'required' keyword to parameter '$paramName' in " +
+                  (className == null
+                      ? functionName
+                      : "'$className.$functionName'"));
 
   /// An explicit type mentioned in the source program needs to be made
   /// nullable.
   factory NullabilityFixDescription.makeTypeNullable(String type) =>
       NullabilityFixDescription._(
-        appliedMessage: 'Changed type $type to be nullable',
+        appliedMessage: "Changed type '$type' to be nullable",
       );
 
   const NullabilityFixDescription._({@required this.appliedMessage});

pkg/nnbd_migration/lib/src/nullability_node.dart

@@ -500,6 +500,9 @@ abstract class NullabilityNode implements NullabilityNodeInfo {
   /// nullability migration needs to decide whether it is optional or required.
   bool get isPossiblyOptional => _isPossiblyOptional;
 
+  @override
+  Iterable<EdgeInfo> get upstreamEdges => _upstreamEdges;
+
   String get _debugPrefix;
 
   NullabilityState get _state;