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 <mfairhurst@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
diff --git a/pkg/nnbd_migration/lib/migration_cli.dart b/pkg/nnbd_migration/lib/migration_cli.dart
index 3a0a4af..d0494cd 100644
--- a/pkg/nnbd_migration/lib/migration_cli.dart
+++ b/pkg/nnbd_migration/lib/migration_cli.dart
@@ -257,6 +257,19 @@
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 @@
_fixCodeProcessor = _FixCodeProcessor(context, this);
_dartFixListener =
DartFixListener(DriverProviderImpl(resourceProvider, context));
- nonNullableFix = NonNullableFix(
+ nonNullableFix = createNonNullableFix(
_dartFixListener, resourceProvider, _fixCodeProcessor.getLineInfo,
included: [options.directory],
preferredPort: options.previewPort,
diff --git a/pkg/nnbd_migration/lib/src/front_end/non_nullable_fix.dart b/pkg/nnbd_migration/lib/src/front_end/non_nullable_fix.dart
index 001ee1f..ed37a85 100644
--- a/pkg/nnbd_migration/lib/src/front_end/non_nullable_fix.dart
+++ b/pkg/nnbd_migration/lib/src/front_end/non_nullable_fix.dart
@@ -26,9 +26,8 @@
/// 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 @@
int get numPhases => 3;
+ InstrumentationListener createInstrumentationListener(
+ {MigrationSummary migrationSummary}) =>
+ InstrumentationListener(migrationSummary: migrationSummary);
+
Future<void> finish() async {
migration.finish();
final state = MigrationState(
@@ -188,7 +191,7 @@
}
void reset() {
- instrumentationListener = InstrumentationListener(
+ instrumentationListener = createInstrumentationListener(
migrationSummary: summaryPath == null
? null
: MigrationSummary(summaryPath, resourceProvider, includedRoot));
diff --git a/pkg/nnbd_migration/test/migration_cli_test.dart b/pkg/nnbd_migration/test/migration_cli_test.dart
index f86b9a9..019bb30 100644
--- a/pkg/nnbd_migration/test/migration_cli_test.dart
+++ b/pkg/nnbd_migration/test/migration_cli_test.dart
@@ -6,14 +6,21 @@
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 @@
});
}
+/// 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 @@
_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 @@
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 @@
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 {
