tests for keyword/identifier replacementOffset - asserts fixes #23906 and fixes #23947

pkg/analysis_server/test/domain_completion_test.dart

Index: pkg/analysis_server/test/domain_completion_test.dart
diff --git a/pkg/analysis_server/test/domain_completion_test.dart b/pkg/analysis_server/test/domain_completion_test.dart
index a54a5955dc42afe29cc27e088e57bee422509cf7..13a18f5db469d547f20d42c0596c118486798cf7 100644
--- a/pkg/analysis_server/test/domain_completion_test.dart
+++ b/pkg/analysis_server/test/domain_completion_test.dart
@@ -24,6 +24,7 @@ import 'package:analysis_server/src/services/search/search_engine.dart';
 import 'package:analyzer/file_system/file_system.dart';
 import 'package:analyzer/instrumentation/instrumentation.dart';
 import 'package:analyzer/source/pub_package_map_provider.dart';
+import 'package:analyzer/src/generated/ast.dart';
 import 'package:analyzer/src/generated/engine.dart';
 import 'package:analyzer/src/generated/sdk.dart';
 import 'package:analyzer/src/generated/source.dart';
@@ -55,14 +56,9 @@ class CompletionManagerTest extends AbstractAnalysisTest {
     ExtensionManager manager = new ExtensionManager();
     ServerPlugin serverPlugin = new ServerPlugin();
     manager.processPlugins([serverPlugin]);
-    return new Test_AnalysisServer(
-        super.serverChannel,
-        super.resourceProvider,
-        super.packageMapProvider,
-        index,
-        serverPlugin,
-        new AnalysisServerOptions(),
-        new MockSdk(),
+    return new Test_AnalysisServer(super.serverChannel, super.resourceProvider,
+        super.packageMapProvider, index, serverPlugin,
+        new AnalysisServerOptions(), new MockSdk(),
         InstrumentationService.NULL_SERVICE);
   }
 
@@ -268,7 +264,7 @@ class CompletionTest extends AbstractAnalysisTest {
   List<CompletionSuggestion> suggestions = [];
   bool suggestionsDone = false;
 
-  String addTestFile(String content, {offset}) {
+  String addTestFile(String content, {int offset}) {
     completionOffset = content.indexOf('^');
     if (offset != null) {
       expect(completionOffset, -1, reason: 'cannot supply offset and ^');
@@ -283,9 +279,10 @@ class CompletionTest extends AbstractAnalysisTest {
   }
 
   void assertHasResult(CompletionSuggestionKind kind, String completion,
-      [int relevance = DART_RELEVANCE_DEFAULT,
-      bool isDeprecated = false,
-      bool isPotential = false]) {
+      {int relevance: DART_RELEVANCE_DEFAULT,
+      bool isDeprecated: false,
+      bool isPotential: false,
+      int selectionOffset}) {
     var cs;
     suggestions.forEach((s) {
       if (s.completion == completion) {
@@ -302,7 +299,7 @@ class CompletionTest extends AbstractAnalysisTest {
     }
     expect(cs.kind, equals(kind));
     expect(cs.relevance, equals(relevance));
-    expect(cs.selectionOffset, equals(completion.length));
+    expect(cs.selectionOffset, selectionOffset ?? completion.length);
     expect(cs.selectionLength, equals(0));
     expect(cs.isDeprecated, equals(isDeprecated));
     expect(cs.isPotential, equals(isPotential));
@@ -386,6 +383,55 @@ class CompletionTest extends AbstractAnalysisTest {
     });
   }
 
+  test_imports_incremental() async {
+    addTestFile('''library foo;
+      e^
+      import "dart:async";
+      import "package:foo/foo.dart";
+      class foo { }''');
+    await waitForTasksFinished();
+    server.updateContent('uc1', {testFile: new AddContentOverlay(testCode)});
+    server.updateContent('uc2', {
+      testFile:
+          new ChangeContentOverlay([new SourceEdit(completionOffset, 0, 'xp')])
+    });
+    completionOffset += 2;
+    await getSuggestions();
+    expect(replacementOffset, completionOffset - 3);
+    expect(replacementLength, 3);
+    String completion = 'export';

[Brian Wilkerson, 2015/08/12 13:59:25]

(I'd actually find it easier to read without the local variable...)

[danrubel, 2015/08/15 19:17:16]

Done.

+    assertHasResult(CompletionSuggestionKind.KEYWORD, completion,
+        relevance: DART_RELEVANCE_HIGH);

[Brian Wilkerson, 2015/08/12 13:59:25]

Should we also test for the absence of invalid suggestions?

[danrubel, 2015/08/15 19:17:16]

Good point. Added.

+  }
+
+  test_imports_partial() async {
+    addTestFile('''^
+      import "package:foo/foo.dart";
+      import "package:bar/bar.dart";
+      class Baz { }''');
+
+    // Wait for analysis then edit the content
+    await waitForTasksFinished();
+    String revisedContent = testCode.substring(0, completionOffset) +
+        'i' +
+        testCode.substring(completionOffset);
+    ++completionOffset;
+    server.handleRequest(new AnalysisUpdateContentParams(
+        {testFile: new AddContentOverlay(revisedContent)}).toRequest('add1'));
+
+    // Request code completion immediately after edit
+    Response response = handleSuccessfulRequest(new CompletionGetSuggestionsParams(
+        testFile, completionOffset).toRequest('0'));
+    completionId = response.id;
+    assertValidId(completionId);
+    await waitForTasksFinished();
+    expect(replacementOffset, completionOffset - 1);
+    expect(replacementLength, 1);
+    String completion = 'import';
+    assertHasResult(CompletionSuggestionKind.KEYWORD, completion,
+        relevance: DART_RELEVANCE_HIGH);

[Brian Wilkerson, 2015/08/12 13:59:25]

We should also test for 'export', 'library' and 'part'.

[danrubel, 2015/08/15 19:17:16]

Done.

+  }
+
   test_imports_prefixed() {
     addTestFile('''
       import 'dart:html' as foo;
@@ -424,10 +470,10 @@ class CompletionTest extends AbstractAnalysisTest {
     return getSuggestions().then((_) {
       expect(replacementOffset, equals(completionOffset - 2));
       expect(replacementLength, equals(2));
-      assertHasResult(
-          CompletionSuggestionKind.KEYWORD, 'export', DART_RELEVANCE_HIGH);
-      assertHasResult(
-          CompletionSuggestionKind.KEYWORD, 'class', DART_RELEVANCE_HIGH);
+      assertHasResult(CompletionSuggestionKind.KEYWORD, 'export',
+          relevance: DART_RELEVANCE_HIGH);

[Brian Wilkerson, 2015/08/12 13:59:25]

I think we're suggesting 'export' at this point so that if the user backspaces
twice the list will be complete. (If that's not accurate, please let me know.)
If 'export' makes sense, so do 'import', 'part', and 'typedef' (and possibly
'void').

[danrubel, 2015/08/15 19:17:16]

That is correct.

+      assertHasResult(CompletionSuggestionKind.KEYWORD, 'class',
+          relevance: DART_RELEVANCE_HIGH);
     });
   }
 
@@ -447,12 +493,12 @@ class CompletionTest extends AbstractAnalysisTest {
       expect(replacementOffset, equals(completionOffset));
       expect(replacementLength, equals(0));
       assertHasResult(CompletionSuggestionKind.INVOCATION, 'A');
-      assertHasResult(
-          CompletionSuggestionKind.INVOCATION, 'a', DART_RELEVANCE_LOCAL_FIELD);
+      assertHasResult(CompletionSuggestionKind.INVOCATION, 'a',
+          relevance: DART_RELEVANCE_LOCAL_FIELD);
       assertHasResult(CompletionSuggestionKind.INVOCATION, 'b',
-          DART_RELEVANCE_LOCAL_VARIABLE);
+          relevance: DART_RELEVANCE_LOCAL_VARIABLE);
       assertHasResult(CompletionSuggestionKind.INVOCATION, 'x',
-          DART_RELEVANCE_LOCAL_METHOD);
+          relevance: DART_RELEVANCE_LOCAL_METHOD);
       assertHasResult(CompletionSuggestionKind.INVOCATION, 'DateTime');
     });
   }
@@ -477,14 +523,12 @@ class B extends A {m() {^}}
       expect(replacementOffset, equals(completionOffset));
       expect(replacementLength, equals(0));
       assertHasResult(CompletionSuggestionKind.INVOCATION, 'm',
-          DART_RELEVANCE_LOCAL_METHOD);
+          relevance: DART_RELEVANCE_LOCAL_METHOD);
     });
   }
 
   test_partFile() {
-    addFile(
-        '/project/bin/testA.dart',
-        '''
+    addFile('/project/bin/testA.dart', '''
       library libA;
       part "$testFile";
       import 'dart:html';
@@ -504,9 +548,7 @@ class B extends A {m() {^}}
   }
 
   test_partFile2() {
-    addFile(
-        '/testA.dart',
-        '''
+    addFile('/testA.dart', '''
       part of libA;
       class A { }''');
     addTestFile('''
@@ -552,7 +594,7 @@ class B extends A {m() {^}}
       // Suggestions based upon imported elements are partially filtered
       //assertHasResult(CompletionSuggestionKind.INVOCATION, 'Object');
       assertHasResult(CompletionSuggestionKind.INVOCATION, 'test',
-          DART_RELEVANCE_LOCAL_TOP_LEVEL_VARIABLE);
+          relevance: DART_RELEVANCE_LOCAL_TOP_LEVEL_VARIABLE);
       assertNoResult('HtmlElement');
     });
   }
@@ -666,24 +708,14 @@ class MockSubscription<E> implements StreamSubscription<E> {
 class Test_AnalysisServer extends AnalysisServer {
   final MockContext mockContext = new MockContext();
 
-  Test_AnalysisServer(
-      ServerCommunicationChannel channel,
+  Test_AnalysisServer(ServerCommunicationChannel channel,
       ResourceProvider resourceProvider,
-      PubPackageMapProvider packageMapProvider,
-      Index index,
-      ServerPlugin serverPlugin,
-      AnalysisServerOptions analysisServerOptions,
-      DartSdk defaultSdk,
-      InstrumentationService instrumentationService)
-      : super(
-            channel,
-            resourceProvider,
-            packageMapProvider,
-            index,
-            serverPlugin,
-            analysisServerOptions,
-            defaultSdk,
-            instrumentationService);
+      PubPackageMapProvider packageMapProvider, Index index,
+      ServerPlugin serverPlugin, AnalysisServerOptions analysisServerOptions,
+      DartSdk defaultSdk, InstrumentationService instrumentationService)
+      : super(channel, resourceProvider, packageMapProvider, index,
+          serverPlugin, analysisServerOptions, defaultSdk,
+          instrumentationService);
 
   @override
   AnalysisContext getAnalysisContext(String path) {