From 83f0bb25a4fdeaeca14859cd8a59220bc6b51244 Mon Sep 17 00:00:00 2001
From: Danny Tuppeny <danny@tuppeny.com>
Date: Wed, 6 Jul 2022 17:19:40 +0000
Subject: [PATCH] [analysis_server] Allow multiple not-imported items in
 completion if name not already imported

Change-Id: Ie5bfa353d59546518180a08cd4720f793a01ba30
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/250660
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
---
 .../completion/dart/suggestion_builder.dart   |  12 ++
 .../test/client/completion_driver_test.dart   | 103 ++++++++++++++----
 2 files changed, 96 insertions(+), 19 deletions(-)

diff --git a/pkg/analysis_server/lib/src/services/completion/dart/suggestion_builder.dart b/pkg/analysis_server/lib/src/services/completion/dart/suggestion_builder.dart
index ad77b39c762..71845bb913f 100644
--- a/pkg/analysis_server/lib/src/services/completion/dart/suggestion_builder.dart
+++ b/pkg/analysis_server/lib/src/services/completion/dart/suggestion_builder.dart
@@ -1128,6 +1128,18 @@ class SuggestionBuilder {
       var key = suggestion.key;
       listener?.builtSuggestion(suggestion);
       if (laterReplacesEarlier || !_suggestionMap.containsKey(key)) {
+        // When suggesting from not-yet-imported libraries, record items
+        // with a key that includes the URI so that multiple not-yet-imported
+        // libraries can be included, but only if there is no imported library
+        // contributing that key.
+        if (isNotImportedLibrary) {
+          key += '::$libraryUriStr';
+          // If `!laterReplacesEarlier`, also ensure we don't already have this
+          // new key.
+          if (!laterReplacesEarlier && _suggestionMap.containsKey(key)) {
+            return;
+          }
+        }
         _suggestionMap[key] = suggestion;
       }
     }
diff --git a/pkg/analysis_server/test/client/completion_driver_test.dart b/pkg/analysis_server/test/client/completion_driver_test.dart
index 9d8f5e40e1a..39d20e988b0 100644
--- a/pkg/analysis_server/test/client/completion_driver_test.dart
+++ b/pkg/analysis_server/test/client/completion_driver_test.dart
@@ -71,6 +71,7 @@ abstract class AbstractCompletionDriverTest
     ElementKind? element,
     CompletionSuggestionKind? kind,
     String? file,
+    String? libraryUri,
   }) {
     expect(
         suggestionWith(
@@ -78,22 +79,7 @@ abstract class AbstractCompletionDriverTest
           element: element,
           kind: kind,
           file: file,
-        ),
-        isNotNull);
-  }
-
-  void assertSuggestions({
-    required String completion,
-    ElementKind? element,
-    CompletionSuggestionKind? kind,
-    String? file,
-  }) {
-    expect(
-        suggestionWith(
-          completion: completion,
-          element: element,
-          kind: kind,
-          file: file,
+          libraryUri: libraryUri,
         ),
         isNotNull);
   }
@@ -161,6 +147,7 @@ name: test
     ElementKind? element,
     CompletionSuggestionKind? kind,
     String? file,
+    String? libraryUri,
   }) =>
       (CompletionSuggestion s) {
         if (s.completion != completion) {
@@ -176,6 +163,9 @@ name: test
         if (file != null && s.element?.location?.file != convertPath(file)) {
           return false;
         }
+        if (libraryUri != null && s.libraryUri != libraryUri) {
+          return false;
+        }
         return true;
       };
 
@@ -184,18 +174,28 @@ name: test
     ElementKind? element,
     CompletionSuggestionKind? kind,
     String? file,
+    String? libraryUri,
   }) =>
       suggestions.where(suggestionHas(
-          completion: completion, element: element, kind: kind, file: file));
+          completion: completion,
+          element: element,
+          kind: kind,
+          file: file,
+          libraryUri: libraryUri));
 
   CompletionSuggestion suggestionWith({
     required String completion,
     ElementKind? element,
     CompletionSuggestionKind? kind,
     String? file,
+    String? libraryUri,
   }) {
     final matches = suggestionsWith(
-        completion: completion, element: element, kind: kind, file: file);
+        completion: completion,
+        element: element,
+        kind: kind,
+        file: file,
+        libraryUri: libraryUri);
     expect(matches, hasLength(1));
     return matches.first;
   }
@@ -272,6 +272,14 @@ class CompletionWithSuggestionsTest1 extends AbstractCompletionDriverTest
   Future<void> test_project_lib_multipleExports() async {
     return super.test_project_lib_multipleExports();
   }
+
+  @FailingTest(
+      reason:
+          'This test fails with available suggestions because it checks libraryUri')
+  @override
+  Future<void> test_project_lib_multipleExports_filteredByLocal() async {
+    return super.test_project_lib_multipleExports_filteredByLocal();
+  }
 }
 
 @reflectiveTest
@@ -575,11 +583,68 @@ void f() {
 }
 ''');
 
-    // Should only have one suggestion.
+    // Should be suggested from both libraries.
     assertSuggestion(
         completion: 'A',
+        libraryUri: 'package:test/a.dart',
         element: ElementKind.CONSTRUCTOR,
         kind: CompletionSuggestionKind.INVOCATION);
+    assertSuggestion(
+        completion: 'A',
+        libraryUri: 'package:test/b.dart',
+        element: ElementKind.CONSTRUCTOR,
+        kind: CompletionSuggestionKind.INVOCATION);
+  }
+
+  Future<void> test_project_lib_multipleExports_filteredByImport() async {
+    newFile('$testPackageLibPath/a.dart', r'''
+class A {}
+''');
+
+    newFile('$testPackageLibPath/b.dart', r'''
+export 'a.dart';
+''');
+
+    if (isProtocolVersion1) {
+      await waitForSetWithUri('package:test/a.dart');
+      await waitForSetWithUri('package:test/b.dart');
+    }
+
+    await addTestFile('''
+import 'b.dart';
+void f() {
+  ^
+}
+''');
+
+    // Should be only one suggestion, which comes from the import of 'b.dart'.
+    assertSuggestion(
+      completion: 'A',
+      element: ElementKind.CLASS,
+    );
+  }
+
+  Future<void> test_project_lib_multipleExports_filteredByLocal() async {
+    newFile('$testPackageLibPath/a.dart', r'''
+class A {}
+''');
+
+    if (isProtocolVersion1) {
+      await waitForSetWithUri('package:test/a.dart');
+    }
+
+    await addTestFile('''
+class A {}
+void f() {
+  ^
+}
+''');
+
+    // Should be only one suggestion, which comes from local declaration.
+    assertSuggestion(
+      completion: 'A',
+      element: ElementKind.CLASS,
+    );
   }
 
   Future<void> test_project_lib_setters_class() async {