Migration: tolerate relative imports into lib.

Prior to this fix, if the user's package had a relative import from
outside `lib` to inside `lib` (e.g. from `test` to `lib`), the
migration tool would regard the imported file as being reached via a
`file:` URI.  This caused it to get confused and complain that the
user had a dependency on unmigrated code (this happened because the
check for dependency on unmigrated code excluded the user's files via
their canonical URIs, and files inside `lib` use `package:` for their
canonical URIs).

To fix the problem, we modify the check for dependency on unmigrated
code so that it excludes the user's files via their path rather than
their URI.

Note that relative imports into the `lib` directory are discouraged by the style guide*, but it still seems worth fixing this bug since they do crop up in the wild.

*https://dart.dev/guides/language/effective-dart/usage#dont-allow-an-import-path-to-reach-into-or-out-of-lib

Fixes #45780.

Bug: https://github.com/dart-lang/sdk/issues/45780
Change-Id: Iff41ca0059d78bbb812dd6f421be6458e7049895
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/196344
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This commit is contained in:
Paul Berry 2021-04-21 22:04:30 +00:00 committed by commit-bot@chromium.org
parent f32f6afbfd
commit 1c86b2f5f6
3 changed files with 34 additions and 1 deletions

View file

@ -121,7 +121,7 @@ class NullabilityMigrationImpl implements NullabilityMigration {
_queriedUnmigratedDependencies = true;
var unmigratedDependencies = <Source>[];
for (var entry in _libraryOptInStatus.entries) {
if (_graph.isBeingMigrated(entry.key)) continue;
if (_graph.isPathBeingMigrated(entry.key.fullName)) continue;
if (!entry.value) {
unmigratedDependencies.add(entry.key);
}

View file

@ -276,6 +276,9 @@ class NullabilityGraph {
/// Set containing all sources being migrated.
final _sourcesBeingMigrated = <Source>{};
/// Set containing paths to all sources being migrated.
final _pathsBeingMigrated = <String>{};
/// A set containing all of the nodes in the graph.
final Set<NullabilityNode> nodes = {};
@ -391,6 +394,10 @@ class NullabilityGraph {
return _sourcesBeingMigrated.contains(source);
}
bool isPathBeingMigrated(String path) {
return _pathsBeingMigrated.contains(path);
}
/// Creates a graph edge that will try to force the given [node] to be
/// non-nullable.
NullabilityEdge makeNonNullable(NullabilityNode node, EdgeOrigin origin,
@ -421,6 +428,7 @@ class NullabilityGraph {
/// Record source as code that is being migrated.
void migrating(Source source) {
_sourcesBeingMigrated.add(source);
_pathsBeingMigrated.add(source.fullName);
}
/// Determines the nullability of each node in the graph by propagating

View file

@ -832,6 +832,31 @@ int? f() => null
expect(output, isNot(contains('package:bar/bar.dart')));
}
test_lifecycle_import_lib_from_test() async {
Map<String, String> makeProject({bool migrated = false}) {
return simpleProject(migrated: migrated)
..['test/foo.dart'] = '''
import '../lib/test.dart';
''';
}
var projectContents = makeProject();
var projectDir = createProjectDir(projectContents);
var cli = _createCli();
var cliRunner =
cli.decodeCommandLineArgs(_parseArgs(['--apply-changes', projectDir]));
bool applyHookCalled = false;
cli._onApplyHook = () {
expect(applyHookCalled, false);
applyHookCalled = true;
// Changes should have been made
assertProjectContents(projectDir, makeProject(migrated: true));
};
await cliRunner.run();
assertNormalExit(cliRunner);
expect(applyHookCalled, true);
}
@FailingTest(issue: 'https://github.com/dart-lang/sdk/issues/44118')
test_lifecycle_issue_44118() async {
var projectContents = simpleProject(sourceText: '''