Migration: include exports when checking that imports are migrated.
When I first implemented this I thought that checking exports wasn't
needed, since it's not allowed for a migrated library to re-export
symbols defined in an unmigtated library. But it turns out we still
do need to include exports in the check, since there might be migrated
libraries that are only reachable in the transitive dependency graph
via an export, and we need to check that *those* libraries don't
import unmigrated code.
Bug: https://github.com/dart-lang/sdk/issues/44061
Change-Id: I280b4117373a3320fc1efe65abe24e344199754c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/170840
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
diff --git a/pkg/nnbd_migration/lib/src/nullability_migration_impl.dart b/pkg/nnbd_migration/lib/src/nullability_migration_impl.dart
index f80f4c4..81a3079 100644
--- a/pkg/nnbd_migration/lib/src/nullability_migration_impl.dart
+++ b/pkg/nnbd_migration/lib/src/nullability_migration_impl.dart
@@ -217,7 +217,10 @@
return;
}
ExperimentStatusException.sanityCheck(result);
- _recordTransitiveImportOptInStatus(result.libraryElement.importedLibraries);
+ _recordTransitiveImportExportOptInStatus(
+ result.libraryElement.importedLibraries);
+ _recordTransitiveImportExportOptInStatus(
+ result.libraryElement.exportedLibraries);
if (_variables == null) {
_variables = Variables(_graph, result.typeProvider, _getLineInfo,
instrumentation: _instrumentation,
@@ -270,14 +273,16 @@
}
/// Records the opt in/out status of all libraries in [libraries], and any
- /// libraries they transitively import, in [_libraryOptInStatus].
- void _recordTransitiveImportOptInStatus(Iterable<LibraryElement> libraries) {
+ /// libraries they transitively import or export, in [_libraryOptInStatus].
+ void _recordTransitiveImportExportOptInStatus(
+ Iterable<LibraryElement> libraries) {
var librariesToCheck = libraries.toList();
while (librariesToCheck.isNotEmpty) {
var library = librariesToCheck.removeLast();
if (_libraryOptInStatus.containsKey(library.source)) continue;
_libraryOptInStatus[library.source] = library.isNonNullableByDefault;
librariesToCheck.addAll(library.importedLibraries);
+ librariesToCheck.addAll(library.exportedLibraries);
}
}
diff --git a/pkg/nnbd_migration/test/migration_cli_test.dart b/pkg/nnbd_migration/test/migration_cli_test.dart
index c581b6b..ce0ec97 100644
--- a/pkg/nnbd_migration/test/migration_cli_test.dart
+++ b/pkg/nnbd_migration/test/migration_cli_test.dart
@@ -762,6 +762,50 @@
});
}
+ test_lifecycle_import_check_via_export() async {
+ // If the user's code exports a library that imports a non-migrated library,
+ // that's a problem too.
+ var projectContents = simpleProject(
+ sourceText: "export 'package:foo/foo.dart';",
+ packageConfigText: _getPackageConfigText(
+ migrated: false, packagesMigrated: {'foo': true, 'bar': false}));
+ var projectDir = createProjectDir(projectContents);
+ resourceProvider.newFile(
+ packagePath('foo/lib/foo.dart'), "import 'package:bar/bar.dart';");
+ resourceProvider.newFile(packagePath('bar/lib/bar.dart'), '');
+ await assertRunFailure([projectDir], expectedExitCode: 1);
+ var output = logger.stdoutBuffer.toString();
+ expect(output, contains('Error: package has unmigrated dependencies'));
+ // Output should mention bar.dart, since it's unmigrated
+ expect(output, contains('package:bar/bar.dart'));
+ // But it should not mention foo.dart, which is migrated
+ expect(output, isNot(contains('package:foo/foo.dart')));
+ }
+
+ test_lifecycle_import_check_via_indirect_export() async {
+ // If the user's code imports a library that exports a library that imports
+ // a non-migrated library, that's a problem too.
+ var projectContents = simpleProject(
+ sourceText: "import 'package:foo/foo.dart';",
+ packageConfigText: _getPackageConfigText(
+ migrated: false,
+ packagesMigrated: {'foo': true, 'bar': true, 'baz': false}));
+ var projectDir = createProjectDir(projectContents);
+ resourceProvider.newFile(
+ packagePath('foo/lib/foo.dart'), "export 'package:bar/bar.dart';");
+ resourceProvider.newFile(
+ packagePath('bar/lib/bar.dart'), "import 'package:baz/baz.dart';");
+ resourceProvider.newFile(packagePath('baz/lib/baz.dart'), '');
+ await assertRunFailure([projectDir], expectedExitCode: 1);
+ var output = logger.stdoutBuffer.toString();
+ expect(output, contains('Error: package has unmigrated dependencies'));
+ // Output should mention baz.dart, since it's unmigrated
+ expect(output, contains('package:baz/baz.dart'));
+ // But it should not mention foo.dart or bar.dart, which are migrated
+ expect(output, isNot(contains('package:foo/foo.dart')));
+ expect(output, isNot(contains('package:bar/bar.dart')));
+ }
+
test_lifecycle_no_preview() async {
var projectContents = simpleProject();
var projectDir = createProjectDir(projectContents);
