[nnbd_migration] Report info building exceptions to server adapter

Change-Id: I4e3eef0d3e404bd090f36f8b2516abb97cc6c311
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/137520
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Mike Fairhurst <mfairhurst@google.com>

Author: MichaelRFairhurst

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

@@ -46,6 +46,8 @@ class NonNullableFix extends FixCodeTask {
 
   InstrumentationListener instrumentationListener;
 
+  NullabilityMigrationAdapter adapter;
+
   NullabilityMigration migration;
 
   /// If this flag has a value of `false`, then something happened to prevent
@@ -57,7 +59,8 @@ class NonNullableFix extends FixCodeTask {
       : includedRoot =
             _getIncludedRoot(included, listener.server.resourceProvider) {
     instrumentationListener = InstrumentationListener();
-    migration = NullabilityMigration(NullabilityMigrationAdapter(listener),
+    adapter = NullabilityMigrationAdapter(listener);
+    migration = NullabilityMigration(adapter,
         permissive: _usePermissiveMode,
         instrumentation: instrumentationListener);
   }
@@ -76,7 +79,7 @@ class NonNullableFix extends FixCodeTask {
     migration.finish();
 
     var state = MigrationState(
-        migration, includedRoot, listener, instrumentationListener);
+        migration, includedRoot, listener, instrumentationListener, adapter);
     await state.refresh();
 
     server = HttpPreviewServer(state);

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

@@ -7,6 +7,7 @@ import 'dart:collection';
 import 'package:analysis_server/src/analysis_server.dart';
 import 'package:analysis_server/src/domains/analysis/navigation_dart.dart';
 import 'package:analysis_server/src/edit/fix/dartfix_listener.dart';
+import 'package:analysis_server/src/edit/fix/non_nullable_fix.dart';
 import 'package:analysis_server/src/edit/nnbd_migration/instrumentation_information.dart';
 import 'package:analysis_server/src/edit/nnbd_migration/migration_info.dart';
 import 'package:analysis_server/src/edit/nnbd_migration/offset_mapper.dart';
@@ -41,6 +42,12 @@ class InfoBuilder {
   /// The listener used to gather the changes to be applied.
   final DartFixListener listener;
 
+  /// The dartfix adapter, which can be used to report exceptions that occur.
+  final NullabilityMigrationAdapter adapter;
+
+  /// The [NullabilityMigration] instance for this migration.
+  final NullabilityMigration migration;
+
   /// A flag indicating whether types that were not changed (because they should
   /// be non-nullable) should be explained.
   final bool explainNonNullableTypes;
@@ -51,6 +58,7 @@ class InfoBuilder {
 
   /// Initialize a newly created builder.
   InfoBuilder(this.provider, this.includedPath, this.info, this.listener,
+      this.adapter, this.migration,
       // TODO(srawlins): Re-enable once
       //  https://github.com/dart-lang/sdk/issues/40253 is fixed.
       {this.explainNonNullableTypes = false});
@@ -613,25 +621,31 @@ class InfoBuilder {
         }
         var info = edit.info;
         String explanation = info?.description?.appliedMessage;
+        List<EditDetail> edits =
+            info != null ? _computeEdits(info, sourceOffset) : [];
+        List<RegionDetail> details;
         try {
-          List<EditDetail> edits =
-              info != null ? _computeEdits(info, sourceOffset) : [];
-          List<RegionDetail> details = _computeDetails(edit);
-          var lineNumber = lineInfo.getLocation(sourceOffset).lineNumber;
-          if (explanation != null) {
-            if (length > 0) {
-              regions.add(RegionInfo(RegionType.remove, offset, length,
-                  lineNumber, explanation, details,
-                  edits: edits));
-            } else {
-              regions.add(RegionInfo(RegionType.add, offset, replacement.length,
-                  lineNumber, explanation, details,
-                  edits: edits));
-            }
+          details = _computeDetails(edit);
+        } catch (e, st) {
+          // TODO(mfairhurst): get the correct Source, and an AstNode.
+          if (migration.isPermissive) {
+            adapter.reportException(result.libraryElement.source, null, e, st);
+            details = [];
+          } else {
+            rethrow;
+          }
+        }
+        var lineNumber = lineInfo.getLocation(sourceOffset).lineNumber;
+        if (explanation != null) {
+          if (length > 0) {
+            regions.add(RegionInfo(RegionType.remove, offset, length,
+                lineNumber, explanation, details,
+                edits: edits));
+          } else {
+            regions.add(RegionInfo(RegionType.add, offset, replacement.length,
+                lineNumber, explanation, details,
+                edits: edits));
           }
-        } catch (e) {
-          // TODO(mfairhurst): collect and recover instead of rethrowing.
-          throw '$e $sourceOffset ${result.path}';
         }
         offset += replacement.length;
       }

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

@@ -3,6 +3,7 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import 'package:analysis_server/src/edit/fix/dartfix_listener.dart';
+import 'package:analysis_server/src/edit/fix/non_nullable_fix.dart';
 import 'package:analysis_server/src/edit/nnbd_migration/info_builder.dart';
 import 'package:analysis_server/src/edit/nnbd_migration/instrumentation_listener.dart';
 import 'package:analysis_server/src/edit/nnbd_migration/migration_info.dart';
@@ -15,6 +16,9 @@ class MigrationState {
   /// The migration associated with the state.
   final NullabilityMigration migration;
 
+  /// The adapter to dartfix for this migration.
+  final NullabilityMigrationAdapter adapter;
+
   /// The root directory that contains all of the files that were migrated.
   final String includedRoot;
 
@@ -32,13 +36,13 @@ class MigrationState {
 
   /// Initialize a newly created migration state with the given values.
   MigrationState(this.migration, this.includedRoot, this.listener,
-      this.instrumentationListener);
+      this.instrumentationListener, this.adapter);
 
   /// Refresh the state of the migration after the migration has been updated.
   void refresh() async {
     OverlayResourceProvider provider = listener.server.resourceProvider;
-    InfoBuilder infoBuilder = InfoBuilder(
-        provider, includedRoot, instrumentationListener.data, listener);
+    InfoBuilder infoBuilder = InfoBuilder(provider, includedRoot,
+        instrumentationListener.data, listener, adapter, migration);
     Set<UnitInfo> unitInfos = await infoBuilder.explainMigration();
     var pathContext = provider.pathContext;
     migrationInfo = MigrationInfo(

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

@@ -853,31 +853,28 @@ class C {
         edit: edits[0], offset: 42, length: 0, replacement: '@required ');
   }
 
-  @FailingTest(issue: 'https://dartbug.com/40773')
   Future<void> test_insertedRequired_parameter() async {
     UnitInfo unit = await buildInfoForSingleTestFile('''
 class C {
-  int level;
+  int level = 0;
   bool f({int lvl}) => lvl >= level;
 }
 ''', migratedContent: '''
 class C {
-  int? level;
-  bool f({required int lvl}) => lvl >= level!;
+  int level = 0;
+  bool f({required int lvl}) => lvl >= level;
 }
 ''');
     List<RegionInfo> regions = unit.fixRegions;
-    expect(regions, hasLength(3));
-    // regions[0] is the `int? s` fix.
-    var region = regions[1];
+    expect(regions, hasLength(1));
+    var region = regions[0];
     var edits = region.edits;
-    assertRegion(region: region, offset: 34, length: 9, details: [
+    assertRegion(region: region, offset: 37, length: 9, details: [
       'This parameter is non-nullable, so cannot have an implicit default '
           "value of 'null'"
     ]);
     assertEdit(
-        edit: edits[0], offset: 33, length: 0, replacement: '@required ');
-    // regions[2] is the `level!` fix.
+        edit: edits[0], offset: 37, length: 0, replacement: '@required ');
   }
 
   Future<void> test_insertParens() async {
@@ -1750,6 +1747,26 @@ class C {
     assertDetail(detail: region.details[2], offset: 70, length: 3);
   }
 
+  @FailingTest(issue: 'https://dartbug.com/40773')
+  Future<void> test_uninitializedMember() async {
+    UnitInfo unit = await buildInfoForSingleTestFile('''
+class C {
+  int level;
+}
+''', migratedContent: '''
+class C {
+  int? level;
+}
+''');
+    List<RegionInfo> regions = unit.fixRegions;
+    expect(regions, hasLength(1));
+    expect(regions[0].details, isNotEmpty);
+    // disabled so that it won't interfere with @FailingTest annotation.
+    //assertRegion(region: regions[0], offset: 15, length: 1, details: [
+    //  'This field is not initialized and is therefore made nullable'
+    //]);
+  }
+
   Future<void> test_uninitializedVariable_notLate_uninitializedUse() async {
     UnitInfo unit = await buildInfoForSingleTestFile('''
 void f() {

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

@@ -78,8 +78,8 @@ class NnbdMigrationTestBase extends AbstractAnalysisTest {
     // Run the migration engine.
     DartFixListener listener = DartFixListener(server);
     InstrumentationListener instrumentationListener = InstrumentationListener();
-    NullabilityMigration migration = NullabilityMigration(
-        NullabilityMigrationAdapter(listener),
+    NullabilityMigrationAdapter adapter = NullabilityMigrationAdapter(listener);
+    NullabilityMigration migration = NullabilityMigration(adapter,
         permissive: false,
         instrumentation: instrumentationListener,
         removeViaComments: removeViaComments);
@@ -101,7 +101,7 @@ class NnbdMigrationTestBase extends AbstractAnalysisTest {
     // Build the migration info.
     InstrumentationInformation info = instrumentationListener.data;
     InfoBuilder builder = InfoBuilder(
-        resourceProvider, includedRoot, info, listener,
+        resourceProvider, includedRoot, info, listener, adapter, migration,
         explainNonNullableTypes: true);
     infos = await builder.explainMigration();
   }

pkg/nnbd_migration/lib/nnbd_migration.dart

@@ -143,6 +143,9 @@ abstract class NullabilityMigration {
       NullabilityMigrationInstrumentation instrumentation,
       bool removeViaComments}) = NullabilityMigrationImpl;
 
+  /// Check if this migration is being run permissively.
+  bool get isPermissive;
+
   void finalizeInput(ResolvedUnitResult result);
 
   void finish();

pkg/nnbd_migration/lib/src/nullability_migration_impl.dart

@@ -74,6 +74,9 @@ class NullabilityMigrationImpl implements NullabilityMigration {
     _postmortemFileWriter?.graph = _graph;
   }
 
+  @override
+  bool get isPermissive => _permissive;
+
   @override
   void finalizeInput(ResolvedUnitResult result) {
     if (!_propagated) {