Hints etc. to prep for migrating the migration tool

This change adds just enough migration hints so that I can run the
migration tool on itself and get reasonable results.  Most of the
hints are just marking a few types as nullable or non-nullable that
the migration tool isn't able to correctly infer by itself.  In a few
circumstances I've had to make a type explicit that was previously
inferred, so that I could mark it as nullable.  In a few other
circumstances I've had to add `/*!*/` after an expression to let the
migration tool know that the expression should be null checked.

Note that unlike most of the rest of the Dart SDK, the migration tool
is not expected to be a long term part of the Dart ecosystem, since
the whole point of it is to help people migrate their code to null
safety.  It's not likely to receive a lot more maintenance, and it
will likely be removed sometime around the time of Dart 3.0 once
people stop needing it.  Accordingly, I've tried to err on the side of
liberally allowing types to be nullable where possible.  Although this
means that we will wind up with more null checks in the final result,
it makes the migration process a lot easier and substantially reduces
the risk of introducing bugs during the migration process.

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

pkg/nnbd_migration/lib/migration_cli.dart

@@ -554,7 +554,7 @@ class MigrationCliRunner implements DartFixListenerClient {
       String summaryPath,
       @required String sdkPath}) {
     return NonNullableFix(listener, resourceProvider, getLineInfo, bindAddress,
-        logger, (String path) => shouldBeMigrated(path),
+        logger, (String /*?*/ path) => shouldBeMigrated(path),
         included: included,
         preferredPort: preferredPort,
         summaryPath: summaryPath,

pkg/nnbd_migration/lib/src/decorated_type.dart

@@ -314,7 +314,7 @@ class DecoratedType implements DecoratedTypeInfo {
       var type = this.type;
       undecoratedResult = Substitution.fromPairs(
         substitution.keys.toList(),
-        substitution.values.map((d) => d.type).toList(),
+        substitution.values.map((d) => d.type /*!*/).toList(),
       ).substituteType(type);
       if (undecoratedResult is FunctionType && type is FunctionType) {
         for (int i = 0; i < undecoratedResult.typeFormals.length; i++) {

pkg/nnbd_migration/lib/src/edge_builder.dart

@@ -645,7 +645,7 @@ class EdgeBuilder extends GeneralizingAstVisitor<DecoratedType>
               _variables.getLateHint(source, member.fields) == null)
             for (var field in member.fields.variables)
               if (!field.declaredElement.isStatic && field.initializer == null)
-                field.declaredElement as FieldElement
+                (field.declaredElement as FieldElement)
       };
       if (_currentClassOrExtension is ClassElement &&
           (_currentClassOrExtension as ClassElement)
@@ -1178,10 +1178,12 @@ class EdgeBuilder extends GeneralizingAstVisitor<DecoratedType>
       if (staticType is InterfaceType) {
         typeArgumentTypes = staticType.typeArguments;
         int index = 0;
-        decoratedTypeArguments = typeArgumentTypes.map((t) {
+        DecoratedType Function(DartType /*?*/) mapFunc = (DartType /*?*/ t) {
           return DecoratedType.forImplicitType(
               typeProvider, t, _graph, target.typeArgument(index++));
-        }).toList();
+        };
+        Iterable<DecoratedType> mapResult = typeArgumentTypes.map(mapFunc);
+        decoratedTypeArguments = mapResult.toList();
         instrumentation?.implicitTypeArguments(
             source, node, decoratedTypeArguments);
         parameterEdgeOrigins = List.filled(typeArgumentTypes.length,
@@ -1208,7 +1210,7 @@ class EdgeBuilder extends GeneralizingAstVisitor<DecoratedType>
     var calleeType =
         getOrComputeElementType(node, callee, targetType: createdType);
     for (var i = 0; i < decoratedTypeArguments.length; ++i) {
-      _checkAssignment(parameterEdgeOrigins?.elementAt(i),
+      _checkAssignment(parameterEdgeOrigins.elementAt(i),
           FixReasonTarget.root.typeArgument(i),
           source: decoratedTypeArguments[i],
           destination:
@@ -2899,7 +2901,7 @@ class EdgeBuilder extends GeneralizingAstVisitor<DecoratedType>
       List<DecoratedType> argumentTypes, List<EdgeOrigin> edgeOrigins) {
     for (var i = 0; i < argumentTypes.length; ++i) {
       _checkAssignment(
-          edgeOrigins?.elementAt(i), FixReasonTarget.root.typeArgument(i),
+          edgeOrigins.elementAt(i), FixReasonTarget.root.typeArgument(i),
           source: argumentTypes[i],
           destination: DecoratedTypeParameterBounds.current
               .get((type.type as FunctionType).typeFormals[i]),

pkg/nnbd_migration/lib/src/edit_plan.dart

@@ -686,7 +686,7 @@ class EditPlanner {
   ///
   /// Optional argument [info] contains information about why the change was
   /// made.
-  EditPlan tryRemoveNode(AstNode sourceNode,
+  EditPlan /*?*/ tryRemoveNode(AstNode sourceNode,
       {List<AstNode> sequenceNodes, AtomicEditInfo info}) {
     var parent = sourceNode.parent;
     sequenceNodes ??= _computeSequenceNodes(parent);
@@ -724,7 +724,7 @@ class EditPlanner {
     for (var entry in changes.entries) {
       var end = entry.key;
       for (var edit in entry.value) {
-        end += edit.length;
+        end = end + edit.length;
       }
       if (result == null || end > result) result = end;
     }
@@ -1709,9 +1709,9 @@ extension AtomicEditList on List<AtomicEdit> {
 }
 
 /// Extension containing useful operations on a map from offsets to lists of
-/// [AtomicEdit]s.  This data structure is used by [EditPlans] to accumulate
+/// [AtomicEdit]s.  This data structure is used by [EditPlan]s to accumulate
 /// source file changes.
-extension AtomicEditMap on Map<int, List<AtomicEdit>> {
+extension AtomicEditMap on Map<int, List<AtomicEdit>> /*?*/ {
   /// Applies the changes to source file text.
   ///
   /// If [includeInformative] is `true`, informative edits are included;
@@ -1728,7 +1728,7 @@ extension AtomicEditMap on Map<int, List<AtomicEdit>> {
   /// otherwise they are ignored.
   List<SourceEdit> toSourceEdits({bool includeInformative = false}) {
     return [
-      for (var offset in keys.toList()..sort((a, b) => b.compareTo(a)))
+      for (var offset in this.keys.toList()..sort((a, b) => b.compareTo(a)))
         this[offset]
             .toSourceEdit(offset, includeInformative: includeInformative)
     ];

pkg/nnbd_migration/lib/src/fix_aggregator.dart

@@ -54,7 +54,7 @@ class FixAggregator extends UnifyingAstVisitor<void> {
 
   /// Map from library to the prefix we should use when inserting code that
   /// refers to it.
-  final Map<LibraryElement, String> _importPrefixes = {};
+  final Map<LibraryElement, String /*?*/ > _importPrefixes = {};
 
   final bool _warnOnWeakCode;
 
@@ -109,7 +109,7 @@ class FixAggregator extends UnifyingAstVisitor<void> {
   EditPlan planForNode(AstNode node) {
     var change = _changes[node];
     if (change != null) {
-      return change._apply(node, this);
+      return change._apply(node /*!*/, this);
     } else {
       return planner.passThrough(node, innerPlans: innerPlansForNode(node));
     }
@@ -288,7 +288,7 @@ enum LateAdditionReason {
 
 /// Base class representing a kind of change that [FixAggregator] might make to
 /// a particular AST node.
-abstract class NodeChange<N extends AstNode> {
+abstract class NodeChange<N extends AstNode /*!*/ > {
   NodeChange._();
 
   /// Indicates whether this node exists solely to provide informative

pkg/nnbd_migration/lib/src/fix_builder.dart

@@ -811,6 +811,7 @@ class MigrationResolutionHooksImpl
       NodeList<CollectionElement> elements) {
     return elements
         .map(_transformCollectionElement)
+        .whereType<CollectionElement /*!*/ >()
         .where((e) => e != null)
         .toList();
   }

pkg/nnbd_migration/lib/src/front_end/migration_state.dart

@@ -52,7 +52,7 @@ class MigrationState {
 
   /// A function which returns whether a file at a given path should be
   /// migrated.
-  final bool Function(String) shouldBeMigratedFunction;
+  final bool Function(String /*?*/) shouldBeMigratedFunction;
 
   /// Initialize a newly created migration state with the given values.
   MigrationState(

pkg/nnbd_migration/lib/src/front_end/migration_summary.dart

@@ -40,8 +40,7 @@ class MigrationSummary {
             var description = info.description;
             if (description != null) {
               var key = _keyForKind(description.kind);
-              changeSummary[key] ??= 0;
-              changeSummary[key]++;
+              changeSummary[key] = (changeSummary[key] ?? 0) + 1;
             }
           }
         }

pkg/nnbd_migration/lib/src/front_end/non_nullable_fix.dart

@@ -77,7 +77,7 @@ class NonNullableFix {
 
   /// A function which returns whether a file at a given path should be
   /// migrated.
-  final bool Function(String) shouldBeMigratedFunction;
+  final bool Function(String /*?*/) shouldBeMigratedFunction;
 
   /// The set of files which are being considered for migration.
   Iterable<String> pathsToProcess;

pkg/nnbd_migration/lib/src/front_end/web/edit_details.dart

@@ -166,7 +166,7 @@ class TraceEntry {
         link = _decodeLink(json['link']),
         hintActions = (json['hintActions'] as List)
                 ?.map((value) =>
-                    HintAction.fromJson(value as Map<String, Object>))
+                    HintAction.fromJson(value as Map<String, Object /*?*/ >))
                 ?.toList() ??
             const [];
 

pkg/nnbd_migration/lib/src/front_end/web/file_details.dart

@@ -72,9 +72,11 @@ class FileDetails {
         navigationContent = json['navigationContent'] as String,
         sourceCode = json['sourceCode'] as String,
         edits = {
-          for (var entry in (json['edits'] as Map<String, Object>).entries)
+          for (var entry
+              in (json['edits'] as Map<String, Object /*?*/ >).entries)
             entry.key: [
-              for (var edit in entry.value) EditListItem.fromJson(edit)
+              for (var edit in entry.value as Iterable<dynamic>)
+                EditListItem.fromJson(edit)
             ]
         };
 

pkg/nnbd_migration/lib/src/front_end/web/migration.dart

@@ -49,7 +49,7 @@ void main() {
           document.body.classes
             ..remove('proposed')
             ..add('applied');
-        }).catchError((e, st) {
+        }).catchError((Object e, st) {
           handleError("Couldn't apply migration", e, st);
         });
       }
@@ -59,7 +59,7 @@ void main() {
     rerunMigrationButton.onClick.listen((event) async {
       try {
         document.body.classes..add('rerunning');
-        var response = await doPost('/rerun-migration');
+        Map<String, Object> /*?*/ response = await doPost('/rerun-migration');
         if (response['success'] as bool) {
           window.location.reload();
         } else {
@@ -207,11 +207,12 @@ Future<T> doGet<T>(String path,
       ..setRequestHeader('Content-Type', 'application/json; charset=UTF-8'));
 
 /// Perform a POST request on the path, return the JSON-decoded response.
-Future<Map<String, Object>> doPost(String path, [Object body]) => doRequest(
-    HttpRequest()
-      ..open('POST', pathWithQueryParameters(path, {}), async: true)
-      ..setRequestHeader('Content-Type', 'application/json; charset=UTF-8'),
-    body);
+Future<Map<String, Object /*?*/ >> doPost(String path, [Object body]) =>
+    doRequest(
+        HttpRequest()
+          ..open('POST', pathWithQueryParameters(path, {}), async: true)
+          ..setRequestHeader('Content-Type', 'application/json; charset=UTF-8'),
+        body);
 
 /// Execute the [HttpRequest], handle its error codes, and return or throw the
 /// response.
@@ -272,7 +273,7 @@ run.  Have you restarted the migration server recently?  If so, you'll need to
 check its output for a fresh URL, and use that URL to perform your migration.
 ''');
   }
-  final json = jsonDecode(xhr.responseText);
+  final Object json = jsonDecode(xhr.responseText);
   if (xhr.status == 200) {
     // Request OK.
     return json as T;
@@ -440,8 +441,9 @@ void highlightAllCode() {
 /// Loads the explanation for [region], into the ".panel-content" div.
 void loadAndPopulateEditDetails(String path, int offset, int line) async {
   try {
-    final responseJson = await doGet<Map<String, Object>>(path,
-        queryParameters: {'region': 'region', 'offset': '$offset'});
+    final Map<String, Object> /*?*/ responseJson =
+        await doGet<Map<String, Object /*?*/ >>(path,
+            queryParameters: {'region': 'region', 'offset': '$offset'});
     var response = EditDetails.fromJson(responseJson);
     populateEditDetails(response);
     pushState(path, offset, line);
@@ -473,8 +475,9 @@ Future<void> loadFile(
 
   try {
     // Navigating to another file; request it, then do work with the response.
-    final response = await doGet<Map<String, Object>>(path,
-        queryParameters: {'inline': 'true'});
+    final Map<String, Object> /*?*/ response =
+        await doGet<Map<String, Object /*?*/ >>(path,
+            queryParameters: {'inline': 'true'});
     writeCodeAndRegions(path, FileDetails.fromJson(response), clearEditDetails);
     maybeScrollToAndHighlight(offset, line);
     var filePathPart = _stripQuery(path);
@@ -493,7 +496,7 @@ void loadNavigationTree() async {
 
   // Request the navigation tree, then do work with the response.
   try {
-    final response = await doGet<List<Object>>(path);
+    final List<Object> /*?*/ response = await doGet<List<Object /*?*/ >>(path);
     var navTree = document.querySelector('.nav-tree');
     navTree.innerHtml = '';
     navigationTree = NavigationTreeNode.listFromJson(response);

pkg/nnbd_migration/lib/src/hint_action.dart

@@ -9,7 +9,7 @@ class HintAction {
   final int nodeId;
   HintAction(this.kind, this.nodeId);
 
-  HintAction.fromJson(Map<String, Object> json)
+  HintAction.fromJson(Map<String, Object /*?*/ > json)
       : nodeId = json['nodeId'] as int,
         kind = HintActionKind.values
             .singleWhere((action) => action.index == json['kind']);

pkg/nnbd_migration/lib/src/node_builder.dart

@@ -535,7 +535,7 @@ class NodeBuilder extends GeneralizingAstVisitor<DecoratedType>
           int index = 0;
           typeArguments = node.typeArguments.arguments
               .map((t) => _pushNullabilityNodeTarget(
-                  target.typeArgument(index++), () => t.accept(this)))
+                  target.typeArgument(index++), () => t.accept(this) /*!*/))
               .toList();
         }
       } else {

pkg/nnbd_migration/lib/src/nullability_node.dart

@@ -160,7 +160,7 @@ class NullabilityEdge implements EdgeInfo {
     edgeDecorations.addAll(guards);
     var edgeDecoration =
         edgeDecorations.isEmpty ? '' : '-(${edgeDecorations.join(', ')})';
-    return '${sourceNode.toString(idMapper: idMapper)} $edgeDecoration-> '
+    return '${sourceNode /*!*/ .toString(idMapper: idMapper)} $edgeDecoration-> '
         '${destinationNode.toString(idMapper: idMapper)}';
   }
 }
@@ -758,7 +758,7 @@ class ResolveSubstitutionPropagationStep extends ExactNullablePropagationStep {
 
   @override
   String toString({NodeToIdMapper idMapper}) =>
-      '${targetNode.toString(idMapper: idMapper)} becomes $newState due to '
+      '${targetNode /*!*/ .toString(idMapper: idMapper)} becomes $newState due to '
       '${node.toString(idMapper: idMapper)}';
 }
 
@@ -778,7 +778,7 @@ class SimpleDownstreamPropagationStep extends DownstreamPropagationStep {
 
   @override
   String toString({NodeToIdMapper idMapper}) =>
-      '${targetNode.toString(idMapper: idMapper)} becomes $newState due to '
+      '${targetNode /*!*/ .toString(idMapper: idMapper)} becomes $newState due to '
       '${edge.toString(idMapper: idMapper)}';
 }
 
@@ -795,7 +795,7 @@ class SimpleExactNullablePropagationStep extends ExactNullablePropagationStep {
 
   @override
   String toString({NodeToIdMapper idMapper}) =>
-      '${targetNode.toString(idMapper: idMapper)} becomes $newState due to '
+      '${targetNode /*!*/ .toString(idMapper: idMapper)} becomes $newState due to '
       '${edge.toString(idMapper: idMapper)}';
 }
 

pkg/nnbd_migration/lib/src/preview/http_preview_server.dart

@@ -61,12 +61,12 @@ class HttpPreviewServer {
   }
 
   /// Return the port this server is bound to.
-  Future<String> get boundHostname async {
+  Future<String /*?*/ > get boundHostname async {
     return (await _serverFuture)?.address?.host;
   }
 
   /// Return the port this server is bound to.
-  Future<int> get boundPort async {
+  Future<int /*?*/ > get boundPort async {
     return (await _serverFuture)?.port;
   }
 

pkg/nnbd_migration/lib/src/preview/preview_site.dart

@@ -599,7 +599,7 @@ class PreviewSite extends Site
           .map((entry) => entry.map((i) => i.toInt()).toList())
           .transform<String>(Utf8Decoder())
           .transform(JsonDecoder())
-          .single) as Map<String, Object>;
+          .single) as Map<String, Object /*?*/ > /*!*/;
 
   Future<void> rerunMigration() async {
     migrationState = await rerunFunction();

pkg/nnbd_migration/lib/src/utilities/json.dart

@@ -6,7 +6,7 @@
 ///
 /// If [map] has key [key], return the value paired with [key]; otherwise throw
 /// a FormatException.
-dynamic expectKey(Map<Object, Object> map, String key) {
+dynamic expectKey(Map<Object /*?*/, Object /*?*/ > map, String key) {
   if (map.containsKey(key)) {
     return map[key];
   }

pkg/nnbd_migration/lib/src/variables.dart

@@ -49,7 +49,7 @@ class Variables {
   final _decoratedElementTypes = <Element, DecoratedType>{};
 
   final _decoratedDirectSupertypes =
-      <ClassElement, Map<ClassElement, DecoratedType>>{};
+      <ClassElement, Map<ClassElement, DecoratedType /*?*/ >>{};
 
   final _decoratedTypeAnnotations = <Source, Map<int, DecoratedType>>{};
 

pkg/nnbd_migration/test/already_migrated_code_decorator_test.dart

@@ -525,7 +525,7 @@ class _CompilationUnitElementMock implements CompilationUnitElementImpl {
 
 class _LibraryElementMock implements LibraryElementImpl {
   @override
-  CompilationUnitElement definingCompilationUnit;
+  /*late*/ CompilationUnitElement definingCompilationUnit;
 
   @override
   Source source;

pkg/nnbd_migration/test/edge_builder_test.dart

@@ -1678,7 +1678,7 @@ void g(int j) {}
     var jNode = decoratedTypeAnnotation('int j').node;
     assertEdge(iNode, jNode,
         hard: false,
-        guards: TypeMatcher<Iterable<NullabilityNode>>()
+        guards: TypeMatcher<Iterable<NullabilityNode /*?*/ >>()
             .having((g) => g.single, 'single value', isIn(alwaysPlus)));
   }
 

pkg/nnbd_migration/test/front_end/nnbd_migration_test_base.dart

@@ -193,7 +193,7 @@ class NnbdMigrationTestBase extends AbstractAnalysisTest {
   /// Returns the singular [UnitInfo] which was built.
   Future<List<UnitInfo>> buildInfoForTestFiles(Map<String, String> files,
       {@required String includedRoot,
-      bool Function(String) shouldBeMigratedFunction,
+      bool Function(String /*?*/) shouldBeMigratedFunction,
       Iterable<String> pathsToProcess}) async {
     shouldBeMigratedFunction ??= (String path) => true;
     var testPaths = <String>[];
@@ -223,7 +223,7 @@ class NnbdMigrationTestBase extends AbstractAnalysisTest {
   /// should all share a common parent directory, [includedRoot].
   Future<void> _buildMigrationInfo(Iterable<String> testPaths,
       {@required String includedRoot,
-      @required bool Function(String) shouldBeMigratedFunction,
+      @required bool Function(String /*?*/) shouldBeMigratedFunction,
       @required Iterable<String> pathsToProcess,
       bool removeViaComments = true,
       bool warnOnWeakCode = false}) async {

pkg/nnbd_migration/test/migration_cli_test.dart

@@ -73,7 +73,7 @@ class _ExceptionGeneratingNonNullableFix extends NonNullableFix {
       String summaryPath,
       @required String sdkPath})
       : super(listener, resourceProvider, getLineInfo, bindAddress, logger,
-            (String path) => true,
+            (String /*?*/ path) => true,
             included: included,
             preferredPort: preferredPort,
             summaryPath: summaryPath,

pkg/nnbd_migration/test/migration_visitor_test_base.dart

@@ -362,7 +362,7 @@ class MigrationVisitorTestBase extends AbstractSingleUnitTest with EdgeTester {
   TypeSystemImpl get typeSystem =>
       testAnalysisResult.typeSystem as TypeSystemImpl;
 
-  Future<CompilationUnit> analyze(String code) async {
+  Future<CompilationUnit /*!*/ > analyze(String code) async {
     await resolveTestUnit(code);
     variables = InstrumentedVariables(graph, typeProvider, getLineInfo);
     testUnit.accept(NodeBuilder(

pkg/nnbd_migration/test/preview/preview_site_test.dart

@@ -44,7 +44,7 @@ class PreviewSiteTest with ResourceProviderMixin, PreviewSiteTestMixin {
     final migrationInfo =
         MigrationInfo({}, {}, resourceProvider.pathContext, null);
     state = MigrationState(
-        null, null, dartfixListener, null, {}, (String path) => true);
+        null, null, dartfixListener, null, {}, (String /*?*/ path) => true);
     state.pathMapper = PathMapper(resourceProvider);
     state.migrationInfo = migrationInfo;
     logger = TestLogger(false /*isVerbose*/);
@@ -276,7 +276,7 @@ class PreviewSiteWithEngineTest extends NnbdMigrationTestBase
   MigrationInfo migrationInfo;
 
   Future<void> setUpMigrationInfo(Map<String, String> files,
-      {bool Function(String) shouldBeMigratedFunction,
+      {bool Function(String /*?*/) shouldBeMigratedFunction,
       Iterable<String> pathsToProcess}) async {
     shouldBeMigratedFunction ??= (String path) => true;
     pathsToProcess ??= files.keys;

pkg/nnbd_migration/tool/summary_stats.dart

@@ -43,8 +43,8 @@ void _printTotals(Map<String, Object> byPath) {
   var summary = <String, int>{};
   for (var file in byPath.values.cast<Map<Object, Object>>()) {
     file.forEach((category, count) {
-      summary.putIfAbsent(category as String, () => 0);
-      summary[category as String] += count as int;
+      summary[category as String] =
+          (summary[category as String] ?? 0) + (count as int);
     });
   }
   var categories = summary.keys.toList()..sort();