Fix handling of circular links in ContextRootImpl and ContextLocatorImpl.
Change-Id: Ib1de5a9f6bbf13e72024ddae6c83d3a73dcf24a5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/186980
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
diff --git a/pkg/analyzer/lib/src/dart/analysis/context_locator.dart b/pkg/analyzer/lib/src/dart/analysis/context_locator.dart
index 7198e6b..f4749c5 100644
--- a/pkg/analyzer/lib/src/dart/analysis/context_locator.dart
+++ b/pkg/analyzer/lib/src/dart/analysis/context_locator.dart
@@ -101,7 +101,7 @@
root.included.add(folder);
root.excludedGlobs = _getExcludedGlobs(root);
roots.add(root);
- _createContextRootsIn(roots, folder, excludedFolders, root,
+ _createContextRootsIn(roots, {}, folder, excludedFolders, root,
root.excludedGlobs, defaultOptionsFile, defaultPackagesFile);
}
Map<Folder, ContextRoot> rootMap = <Folder, ContextRoot>{};
@@ -140,6 +140,7 @@
/// search for nested context roots.
void _createContextRoots(
List<ContextRoot> roots,
+ Set<String> visited,
Folder folder,
List<Folder> excludedFolders,
ContextRoot containingRoot,
@@ -181,8 +182,8 @@
excludedGlobs = _getExcludedGlobs(root);
root.excludedGlobs = excludedGlobs;
}
- _createContextRootsIn(roots, folder, excludedFolders, containingRoot,
- excludedGlobs, optionsFile, packagesFile);
+ _createContextRootsIn(roots, visited, folder, excludedFolders,
+ containingRoot, excludedGlobs, optionsFile, packagesFile);
}
/// For each directory within the given [folder] that is neither in the list
@@ -193,6 +194,7 @@
/// file will be used even if there is a local version of the file.
void _createContextRootsIn(
List<ContextRoot> roots,
+ Set<String> visited,
Folder folder,
List<Folder> excludedFolders,
ContextRoot containingRoot,
@@ -212,6 +214,12 @@
return false;
}
+ // Stop infinite recursion via links.
+ var canonicalFolderPath = folder.resolveSymbolicLinksSync().path;
+ if (!visited.add(canonicalFolderPath)) {
+ return;
+ }
+
//
// Check each of the subdirectories to see whether a context root needs to
// be added for it.
@@ -222,8 +230,8 @@
if (isExcluded(child)) {
containingRoot.excluded.add(child);
} else {
- _createContextRoots(roots, child, excludedFolders, containingRoot,
- excludedGlobs, optionsFile, packagesFile);
+ _createContextRoots(roots, visited, child, excludedFolders,
+ containingRoot, excludedGlobs, optionsFile, packagesFile);
}
}
}
diff --git a/pkg/analyzer/lib/src/dart/analysis/context_root.dart b/pkg/analyzer/lib/src/dart/analysis/context_root.dart
index 583fd5b..830e013 100644
--- a/pkg/analyzer/lib/src/dart/analysis/context_root.dart
+++ b/pkg/analyzer/lib/src/dart/analysis/context_root.dart
@@ -56,13 +56,14 @@
@override
Iterable<String> analyzedFiles() sync* {
+ var visitedCanonicalPaths = <String>{};
for (String path in includedPaths) {
if (!_isExcluded(path)) {
Resource resource = resourceProvider.getResource(path);
if (resource is File) {
yield path;
} else if (resource is Folder) {
- yield* _includedFilesInFolder(resource);
+ yield* _includedFilesInFolder(visitedCanonicalPaths, resource);
} else {
Type type = resource.runtimeType;
throw StateError('Unknown resource at path "$path" ($type)');
@@ -78,14 +79,21 @@
/// Return the absolute paths of all of the files that are included in the
/// given [folder].
- Iterable<String> _includedFilesInFolder(Folder folder) sync* {
+ Iterable<String> _includedFilesInFolder(
+ Set<String> visited,
+ Folder folder,
+ ) sync* {
for (Resource resource in folder.getChildren()) {
String path = resource.path;
if (!_isExcluded(path)) {
if (resource is File) {
yield path;
} else if (resource is Folder) {
- yield* _includedFilesInFolder(resource);
+ var canonicalPath = resource.resolveSymbolicLinksSync().path;
+ if (visited.add(canonicalPath)) {
+ yield* _includedFilesInFolder(visited, resource);
+ visited.remove(canonicalPath);
+ }
} else {
Type type = resource.runtimeType;
throw StateError('Unknown resource at path "$path" ($type)');
diff --git a/pkg/analyzer/lib/src/services/available_declarations.dart b/pkg/analyzer/lib/src/services/available_declarations.dart
index 06acfc4..6fba2c3 100644
--- a/pkg/analyzer/lib/src/services/available_declarations.dart
+++ b/pkg/analyzer/lib/src/services/available_declarations.dart
@@ -338,8 +338,15 @@
void _findPackages() {
var pathContext = _tracker._resourceProvider.pathContext;
var pubPathPrefixToPathList = <String, List<String>>{};
+ var visitedFolderPaths = <String>{};
+ /// TODO(scheglov) Use analyzedFiles() ?
void visitFolder(Folder folder) {
+ var canonicalFolderPath = folder.resolveSymbolicLinksSync().path;
+ if (!visitedFolderPaths.add(canonicalFolderPath)) {
+ return;
+ }
+
var buildFile = folder.getChildAssumingFile('BUILD');
var pubspecFile = folder.getChildAssumingFile('pubspec.yaml');
if (buildFile.exists) {
diff --git a/pkg/analyzer/test/src/dart/analysis/context_locator_test.dart b/pkg/analyzer/test/src/dart/analysis/context_locator_test.dart
index e7df093..469a182 100644
--- a/pkg/analyzer/test/src/dart/analysis/context_locator_test.dart
+++ b/pkg/analyzer/test/src/dart/analysis/context_locator_test.dart
@@ -42,6 +42,136 @@
contextLocator = ContextLocatorImpl(resourceProvider: resourceProvider);
}
+ void test_locateRoots_link_file_toOutOfRoot() {
+ Folder rootFolder = newFolder('/home/test');
+ newFile('/home/test/lib/a.dart');
+ newFile('/home/b.dart');
+ resourceProvider.newLink(
+ convertPath('/home/test/lib/c.dart'),
+ convertPath('/home/b.dart'),
+ );
+
+ List<ContextRoot> roots =
+ contextLocator.locateRoots(includedPaths: [rootFolder.path]);
+ expect(roots, hasLength(1));
+
+ ContextRoot root = findRoot(roots, rootFolder);
+ expect(root.includedPaths, unorderedEquals([rootFolder.path]));
+ expect(root.excludedPaths, isEmpty);
+ expect(root.optionsFile, isNull);
+ expect(root.packagesFile, isNull);
+
+ _assertAnalyzedFiles(root, [
+ '/home/test/lib/a.dart',
+ '/home/test/lib/c.dart',
+ ]);
+ }
+
+ void test_locateRoots_link_file_toSiblingInRoot() {
+ Folder rootFolder = newFolder('/test');
+ newFile('/test/lib/a.dart');
+ resourceProvider.newLink(
+ convertPath('/test/lib/b.dart'),
+ convertPath('/test/lib/a.dart'),
+ );
+
+ List<ContextRoot> roots =
+ contextLocator.locateRoots(includedPaths: [rootFolder.path]);
+ expect(roots, hasLength(1));
+
+ ContextRoot root = findRoot(roots, rootFolder);
+ expect(root.includedPaths, unorderedEquals([rootFolder.path]));
+ expect(root.excludedPaths, isEmpty);
+ expect(root.optionsFile, isNull);
+ expect(root.packagesFile, isNull);
+
+ _assertAnalyzedFiles(root, [
+ '/test/lib/a.dart',
+ '/test/lib/b.dart',
+ ]);
+ }
+
+ void test_locateRoots_link_folder_toParentInRoot() {
+ Folder rootFolder = newFolder('/test');
+ newFile('/test/lib/a.dart');
+ resourceProvider.newLink(
+ convertPath('/test/lib/foo'),
+ convertPath('/test/lib'),
+ );
+
+ List<ContextRoot> roots =
+ contextLocator.locateRoots(includedPaths: [rootFolder.path]);
+ expect(roots, hasLength(1));
+
+ ContextRoot root = findRoot(roots, rootFolder);
+ expect(root.includedPaths, unorderedEquals([rootFolder.path]));
+ expect(root.excludedPaths, isEmpty);
+ expect(root.optionsFile, isNull);
+ expect(root.packagesFile, isNull);
+
+ _assertAnalyzedFiles(root, ['/test/lib/a.dart']);
+
+ _assertAnalyzed(root, [
+ '/test/lib/a.dart',
+ '/test/lib/foo/b.dart',
+ ]);
+ }
+
+ void test_locateRoots_link_folder_toParentOfRoot() {
+ Folder rootFolder = newFolder('/home/test');
+ newFile('/home/test/lib/a.dart');
+ newFile('/home/b.dart');
+ newFile('/home/other/c.dart');
+ resourceProvider.newLink(
+ convertPath('/home/test/lib/foo'),
+ convertPath('/home'),
+ );
+
+ List<ContextRoot> roots =
+ contextLocator.locateRoots(includedPaths: [rootFolder.path]);
+ expect(roots, hasLength(1));
+
+ ContextRoot root = findRoot(roots, rootFolder);
+ expect(root.includedPaths, unorderedEquals([rootFolder.path]));
+ expect(root.excludedPaths, isEmpty);
+ expect(root.optionsFile, isNull);
+ expect(root.packagesFile, isNull);
+
+ // The set of analyzed files includes everything in `/home`,
+ // but does not repeat `/home/test/lib/a.dart` and does not cycle.
+ _assertAnalyzedFiles(root, [
+ '/home/test/lib/a.dart',
+ '/home/test/lib/foo/b.dart',
+ '/home/test/lib/foo/other/c.dart',
+ ]);
+ }
+
+ void test_locateRoots_link_folder_toSiblingInRoot() {
+ Folder rootFolder = newFolder('/test');
+ newFile('/test/lib/a.dart');
+ newFile('/test/lib/foo/b.dart');
+ resourceProvider.newLink(
+ convertPath('/test/lib/bar'),
+ convertPath('/test/lib/foo'),
+ );
+
+ List<ContextRoot> roots =
+ contextLocator.locateRoots(includedPaths: [rootFolder.path]);
+ expect(roots, hasLength(1));
+
+ ContextRoot root = findRoot(roots, rootFolder);
+ expect(root.includedPaths, unorderedEquals([rootFolder.path]));
+ expect(root.excludedPaths, isEmpty);
+ expect(root.optionsFile, isNull);
+ expect(root.packagesFile, isNull);
+
+ _assertAnalyzedFiles(root, [
+ '/test/lib/a.dart',
+ '/test/lib/foo/b.dart',
+ '/test/lib/bar/b.dart',
+ ]);
+ }
+
void test_locateRoots_multiple_dirAndNestedDir() {
Folder outerRootFolder = newFolder('/test/outer');
File outerOptionsFile = newOptionsFile('/test/outer');
@@ -789,6 +919,12 @@
}
}
+ void _assertAnalyzedFiles(ContextRoot root, List<String> posixPathList) {
+ var analyzedFiles = root.analyzedFiles().toList();
+ var pathList = posixPathList.map(convertPath).toList();
+ expect(analyzedFiles, unorderedEquals(pathList));
+ }
+
void _assertNotAnalyzed(ContextRoot root, List<String> posixPathList) {
for (var posixPath in posixPathList) {
var path = convertPath(posixPath);
diff --git a/pkg/analyzer/test/src/services/available_declarations_test.dart b/pkg/analyzer/test/src/services/available_declarations_test.dart
index 019fe7b..6ba066f 100644
--- a/pkg/analyzer/test/src/services/available_declarations_test.dart
+++ b/pkg/analyzer/test/src/services/available_declarations_test.dart
@@ -211,10 +211,6 @@
expect(library.id, id);
}
- @SkippedTest(
- issue: 'https://github.com/dart-lang/sdk/issues/44501',
- reason: 'Actually, with fixed ResourceProvider, this test cycles',
- )
test_getLibrary_exportViaRecursiveLink() async {
resourceProvider.newLink(
convertPath('/home/test/lib/foo'),
