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>
This commit is contained in:
Paul Berry 2020-11-09 13:16:34 +00:00 committed by commit-bot@chromium.org
parent cb43fc498e
commit f4001df118
2 changed files with 52 additions and 3 deletions

View file

@ -217,7 +217,10 @@ class NullabilityMigrationImpl implements NullabilityMigration {
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 @@ class NullabilityMigrationImpl implements NullabilityMigration {
}
/// 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);
}
}

View file

@ -762,6 +762,50 @@ int? f() => null
});
}
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);