Migration: Fix reporting of exceptions.

Permissive mode in the analyzer currently is broken, leading to a
useless stacktrace if an exception occurs.  As a quick fix, turn
permissive mode off, so the exception just bubbles up normally and
shows on the console.  In follow-up CLs I'll actually fix permissive
mode and add a user option to enable it.

Fixes #42138.

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

Author: stereotype441

pkg/nnbd_migration/lib/migration_cli.dart

@@ -257,6 +257,19 @@ class MigrationCli {
     return stream.first;
   }
 
+  NonNullableFix createNonNullableFix(DartFixListener listener,
+      ResourceProvider resourceProvider, LineInfo getLineInfo(String path),
+      {List<String> included = const <String>[],
+      int preferredPort,
+      bool enablePreview = true,
+      String summaryPath}) {
+    return NonNullableFix(listener, resourceProvider, getLineInfo,
+        included: included,
+        preferredPort: preferredPort,
+        enablePreview: enablePreview,
+        summaryPath: summaryPath);
+  }
+
   /// Parses and validates command-line arguments, and stores the results in
   /// [options].
   ///
@@ -365,7 +378,7 @@ class MigrationCli {
       _fixCodeProcessor = _FixCodeProcessor(context, this);
       _dartFixListener =
           DartFixListener(DriverProviderImpl(resourceProvider, context));
-      nonNullableFix = NonNullableFix(
+      nonNullableFix = createNonNullableFix(
           _dartFixListener, resourceProvider, _fixCodeProcessor.getLineInfo,
           included: [options.directory],
           preferredPort: options.previewPort,

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

@@ -26,9 +26,8 @@ import 'package:yaml/yaml.dart';
 /// and determines whether the associated variable or parameter can be null
 /// then adds or removes a '?' trailing the named type as appropriate.
 class NonNullableFix {
-  /// TODO(paulberry): stop using permissive mode once the migration logic is
-  /// mature enough.
-  static const bool _usePermissiveMode = true;
+  /// TODO(paulberry): allow this to be controlled by a command-line parameter.
+  static const bool _usePermissiveMode = false;
 
   // TODO(srawlins): Refactor to use
   //  `Feature.non_nullable.firstSupportedVersion` when this becomes non-null.
@@ -101,6 +100,10 @@ class NonNullableFix {
 
   int get numPhases => 3;
 
+  InstrumentationListener createInstrumentationListener(
+          {MigrationSummary migrationSummary}) =>
+      InstrumentationListener(migrationSummary: migrationSummary);
+
   Future<void> finish() async {
     migration.finish();
     final state = MigrationState(
@@ -188,7 +191,7 @@ class NonNullableFix {
   }
 
   void reset() {
-    instrumentationListener = InstrumentationListener(
+    instrumentationListener = createInstrumentationListener(
         migrationSummary: summaryPath == null
             ? null
             : MigrationSummary(summaryPath, resourceProvider, includedRoot));

pkg/nnbd_migration/test/migration_cli_test.dart

@@ -6,14 +6,21 @@ import 'dart:async';
 import 'dart:convert';
 import 'dart:io';
 
+import 'package:analyzer/dart/element/element.dart';
+import 'package:analyzer/file_system/file_system.dart' show ResourceProvider;
 import 'package:analyzer/file_system/memory_file_system.dart';
 import 'package:analyzer/file_system/physical_file_system.dart';
+import 'package:analyzer/source/line_info.dart';
 import 'package:analyzer/src/test_utilities/mock_sdk.dart' as mock_sdk;
 import 'package:args/args.dart';
 import 'package:cli_util/cli_logging.dart';
 import 'package:http/http.dart' as http;
 import 'package:meta/meta.dart';
+import 'package:nnbd_migration/instrumentation.dart';
 import 'package:nnbd_migration/migration_cli.dart';
+import 'package:nnbd_migration/src/front_end/dartfix_listener.dart';
+import 'package:nnbd_migration/src/front_end/instrumentation_listener.dart';
+import 'package:nnbd_migration/src/front_end/migration_summary.dart';
 import 'package:nnbd_migration/src/front_end/non_nullable_fix.dart';
 import 'package:nnbd_migration/src/front_end/web/edit_details.dart';
 import 'package:nnbd_migration/src/front_end/web/file_details.dart';
@@ -29,10 +36,54 @@ main() {
   });
 }
 
+/// Specialization of [InstrumentationListener] that generates artificial
+/// exceptions, so that we can test they are properly propagated to top level.
+class _ExceptionGeneratingInstrumentationListener
+    extends InstrumentationListener {
+  _ExceptionGeneratingInstrumentationListener(
+      {MigrationSummary migrationSummary})
+      : super(migrationSummary: migrationSummary);
+
+  @override
+  void externalDecoratedType(Element element, DecoratedTypeInfo decoratedType) {
+    if (element.name == 'print') {
+      throw StateError('Artificial exception triggered');
+    }
+    super.externalDecoratedType(element, decoratedType);
+  }
+}
+
+/// Specialization of [NonNullableFix] that generates artificial exceptions, so
+/// that we can test they are properly propagated to top level.
+class _ExceptionGeneratingNonNullableFix extends NonNullableFix {
+  _ExceptionGeneratingNonNullableFix(DartFixListener listener,
+      ResourceProvider resourceProvider, LineInfo Function(String) getLineInfo,
+      {List<String> included = const <String>[],
+      int preferredPort,
+      bool enablePreview = true,
+      String summaryPath})
+      : super(listener, resourceProvider, getLineInfo,
+            included: included,
+            preferredPort: preferredPort,
+            enablePreview: enablePreview,
+            summaryPath: summaryPath);
+
+  @override
+  InstrumentationListener createInstrumentationListener(
+          {MigrationSummary migrationSummary}) =>
+      _ExceptionGeneratingInstrumentationListener(
+          migrationSummary: migrationSummary);
+}
+
 class _MigrationCli extends MigrationCli {
+  /// If `true`, then an artifical exception should be generated when migration
+  /// encounters a reference to the `print` function.
+  final bool injectArtificialException;
+
   Future<void> Function() _runWhilePreviewServerActive;
 
-  _MigrationCli(_MigrationCliTestBase test)
+  _MigrationCli(_MigrationCliTestBase test,
+      {this.injectArtificialException = false})
       : super(
             binaryName: 'nnbd_migration',
             loggerFactory: (isVerbose) => test.logger = _TestLogger(isVerbose),
@@ -50,6 +101,29 @@ class _MigrationCli extends MigrationCli {
     _runWhilePreviewServerActive = null;
   }
 
+  @override
+  NonNullableFix createNonNullableFix(DartFixListener listener,
+      ResourceProvider resourceProvider, LineInfo getLineInfo(String path),
+      {List<String> included = const <String>[],
+      int preferredPort,
+      bool enablePreview = true,
+      String summaryPath}) {
+    if (injectArtificialException) {
+      return _ExceptionGeneratingNonNullableFix(
+          listener, resourceProvider, getLineInfo,
+          included: included,
+          preferredPort: preferredPort,
+          enablePreview: enablePreview,
+          summaryPath: summaryPath);
+    } else {
+      return super.createNonNullableFix(listener, resourceProvider, getLineInfo,
+          included: included,
+          preferredPort: preferredPort,
+          enablePreview: enablePreview,
+          summaryPath: summaryPath);
+    }
+  }
+
   Future<void> runWithPreviewServer(
       ArgResults argResults, Future<void> callback()) async {
     _runWhilePreviewServerActive = callback;
@@ -362,6 +436,16 @@ int${migrated ? '?' : ''} f() => null;
     assertProjectContents(projectDir, simpleProject(migrated: true));
   }
 
+  test_lifecycle_exception_handling() async {
+    var projectContents = simpleProject(sourceText: 'main() { print(0); }');
+    var projectDir = await createProjectDir(projectContents);
+    var cli = _createCli(injectArtificialException: true);
+    expect(
+        () async => runWithPreviewServer(cli, [projectDir], (url) async {}),
+        throwsA(TypeMatcher<Error>().having((e) => e.toString(), 'toString',
+            contains('Artificial exception triggered'))));
+  }
+
   test_lifecycle_ignore_errors_disable() async {
     var projectContents = simpleProject(sourceText: '''
 int f() => null
@@ -1334,9 +1418,10 @@ name: test
         headers: {'Content-Type': 'application/json; charset=UTF-8'});
   }
 
-  _MigrationCli _createCli() {
+  _MigrationCli _createCli({bool injectArtificialException = false}) {
     mock_sdk.MockSdk(resourceProvider: resourceProvider);
-    return _MigrationCli(this);
+    return _MigrationCli(this,
+        injectArtificialException: injectArtificialException);
   }
 
   Future<String> _getHelpText({@required bool verbose}) async {