[analyzer] Exclude nested analysis roots from their parents contexts

Fixes Dart-Code/Dart-Code#5548

Change-Id: Ifb6eb52bf6589ebb5d7d60a8a3d4910d06e5be88
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/435161
Reviewed-by: Konstantin Shcheglov <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Brian Wilkerson <[email protected]>

Author: DanTup

pkg/analysis_server/test/lsp/rename_test.dart

@@ -1066,6 +1066,43 @@ void f(List<int> values) {
     );
   }
 
+  /// Local variable renames shouldn't fail to rename references to them in
+  /// workspaces with overlapped roots and `driverMap` being sorted such that
+  /// the first driver containing a file is ancestor.
+  Future<void> test_rename_variable_issueDartCode5548() async {
+    // Use hard-coded paths because the failure here may depend on the hash
+    // codes of the folder paths (as stored in `driverMap`) to reproduce.
+    var rootPath = convertPath('/dart_code_test');
+    var nestedProjectPath = join(rootPath, 'packages', 'test_package');
+    var testFilePath = join(nestedProjectPath, 'foo.dart');
+
+    // Force an additional context for the nested project.
+    newFile(join(nestedProjectPath, 'analysis_options.yaml'), '');
+
+    const content = '''
+void f() {
+  var [!^b!] = 1;
+  print(b);
+}
+''';
+    const expectedContent = '''
+void f() {
+  var a = 1;
+  print(a);
+}
+''';
+    await _test_rename_withDocumentChanges(
+      filePath: testFilePath,
+      content,
+      'a',
+      expectedContent,
+      workspaceFolders: [
+        pathContext.toUri(nestedProjectPath),
+        pathContext.toUri(rootPath),
+      ],
+    );
+  }
+
   Future<void> test_rename_withoutVersionedIdentifier() {
     // Without sending a document version, the rename should still work because
     // the server should use the version it had at the start of the rename
@@ -1200,6 +1237,7 @@ final a = new MyNewClass();
     String? expectedFilePath,
     bool sendRenameVersion = true,
     bool supportsWindowShowMessageRequest = true,
+    List<Uri>? workspaceFolders,
   }) async {
     filePath ??= mainFilePath;
     expectedFilePath ??= filePath;
@@ -1219,6 +1257,7 @@ final a = new MyNewClass();
           supportsWindowShowMessageRequest
               ? const {'supportsWindowShowMessageRequest': true}
               : null,
+      workspaceFolders: workspaceFolders,
     );
     await openFile(fileUri, code.code, version: documentVersion);
     await initialAnalysis;

pkg/analyzer/lib/src/dart/analysis/context_locator.dart

@@ -181,11 +181,26 @@ class ContextLocatorImpl {
       );
 
       ContextRootImpl? root;
+      // Check whether there are existing roots that overlap with this one.
       for (var existingRoot in roots) {
-        if (existingRoot.root.isOrContains(folder.path) &&
-            _matchRootWithLocation(existingRoot, location)) {
-          root = existingRoot;
-          break;
+        if (existingRoot.root.isOrContains(folder.path)) {
+          if (_matchRootWithLocation(existingRoot, location)) {
+            // This root is covered exactly by the existing root (with the same
+            // options/packages file) so we can simple use it.
+            root = existingRoot;
+            break;
+          } else {
+            // This root is within another (but doesn't share options/packages)
+            // so we still need a new root. However, we should exclude this
+            // from the existing root so these files aren't analyzed by both.
+            //
+            // It's possible this folder is already excluded (for example
+            // because it's also a project and had a context root created as
+            // part of the parent analysis root).
+            if (!existingRoot.excluded.contains(folder)) {
+              existingRoot.excluded.add(folder);
+            }
+          }
         }
       }
 

pkg/analyzer/test/src/dart/analysis/analysis_context_collection_test.dart

@@ -1140,14 +1140,6 @@ contexts
     packagesFile: /home/.dart_tool/package_config.json
     workspace: workspace_0
     analyzedFiles
-      /home/packages/package1/lib/package1.dart
-        uri: package:package1/package1.dart
-        analysisOptions_0
-        workspacePackage_0_0
-      /home/packages/package2/lib/package2.dart
-        uri: package:package2/package2.dart
-        analysisOptions_1
-        workspacePackage_0_1
   /home/packages/package1
     packagesFile: /home/.dart_tool/package_config.json
     workspace: workspace_1
@@ -1170,13 +1162,6 @@ analysisOptions
 workspaces
   workspace_0: PackageConfigWorkspace
     root: /home
-    pubPackages
-      workspacePackage_0_0: PubPackage
-        root: /home/packages/package1
-        sdkVersionConstraint: ^3.6.0
-      workspacePackage_0_1: PubPackage
-        root: /home/packages/package2
-        sdkVersionConstraint: ^3.6.0
   workspace_1: PackageConfigWorkspace
     root: /home
     pubPackages

pkg/analyzer/test/src/dart/analysis/context_locator_test.dart

@@ -951,6 +951,62 @@ ${getFolder(outPath).path}
     expect(outerRoot.packagesFile, outerPackagesFile);
   }
 
+  /// Verify that overlapped roots do not result in nested files being analyzed
+  /// in multiple contexts (when the nested folder is passed first).
+  ///
+  /// See https://github.com/Dart-Code/Dart-Code/issues/5548')
+  void test_locateRoots_nested_issueDartCode5548_nestedThenRoot() {
+    var rootPath = convertPath('/root');
+    var rootFolder = newFolder(rootPath);
+    var rootOptionsFile = newAnalysisOptionsYamlFile(rootPath, '');
+
+    var nestedPath = convertPath('/root/packages/foo');
+    var nestedFolder = newFolder(nestedPath);
+    var nestedOptionsFile = newAnalysisOptionsYamlFile(nestedPath, '');
+    var nestedDartFile = newFile(join(nestedPath, 'main.dart'), '');
+
+    var roots = contextLocator.locateRoots(
+      includedPaths: [nestedPath, rootPath],
+    );
+    expect(roots, hasLength(2));
+
+    // The root context should not analyze nestedDartFile.
+    var root = findRoot(roots, rootFolder);
+    _assertAnalyzedFiles2(root, [rootOptionsFile]);
+
+    // The nested context should analyze nestedDartFile.
+    var nested = findRoot(roots, nestedFolder);
+    _assertAnalyzedFiles2(nested, [nestedOptionsFile, nestedDartFile]);
+  }
+
+  /// Verify that overlapped roots do not result in nested files being analyzed
+  /// in multiple contexts (when the nested folder is passed last).
+  ///
+  /// See https://github.com/Dart-Code/Dart-Code/issues/5548')
+  void test_locateRoots_nested_issueDartCode5548_rootThenNested() {
+    var rootPath = convertPath('/root');
+    var rootFolder = newFolder(rootPath);
+    var rootOptionsFile = newAnalysisOptionsYamlFile(rootPath, '');
+
+    var nestedPath = convertPath('/root/packages/foo');
+    var nestedFolder = newFolder(nestedPath);
+    var nestedOptionsFile = newAnalysisOptionsYamlFile(nestedPath, '');
+    var nestedDartFile = newFile(join(nestedPath, 'main.dart'), '');
+
+    var roots = contextLocator.locateRoots(
+      includedPaths: [rootPath, nestedPath],
+    );
+    expect(roots, hasLength(2));
+
+    // The root context should not analyze nestedDartFile.
+    var root = findRoot(roots, rootFolder);
+    _assertAnalyzedFiles2(root, [rootOptionsFile]);
+
+    // The nested context should analyze nestedDartFile.
+    var nested = findRoot(roots, nestedFolder);
+    _assertAnalyzedFiles2(nested, [nestedOptionsFile, nestedDartFile]);
+  }
+
   void test_locateRoots_nested_multiple() {
     Folder outerRootFolder = newFolder('/test/outer');
     File outerOptionsFile = newAnalysisOptionsYamlFile('/test/outer', '');