use CompletionTarget for replacement offset/length

pkg/analysis_server/lib/src/services/completion/dart_completion_manager.dart

 // Copyright (c) 2014, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 library services.completion.dart;
 
 import 'dart:async';
 
 import 'package:analysis_server/src/protocol.dart';
 import 'package:analysis_server/src/services/completion/arglist_computer.dart';
 import 'package:analysis_server/src/services/completion/combinator_computer.dart';
 import 'package:analysis_server/src/services/completion/completion_manager.dart';
 import 'package:analysis_server/src/services/completion/completion_target.dart';
 import 'package:analysis_server/src/services/completion/dart_completion_cache.dart';
 import 'package:analysis_server/src/services/completion/imported_computer.dart';
 import 'package:analysis_server/src/services/completion/invocation_computer.dart';
 import 'package:analysis_server/src/services/completion/keyword_computer.dart';
 import 'package:analysis_server/src/services/completion/local_computer.dart';
 import 'package:analysis_server/src/services/completion/optype.dart';
 import 'package:analysis_server/src/services/search/search_engine.dart';
 import 'package:analyzer/src/generated/ast.dart';
 import 'package:analyzer/src/generated/engine.dart';
 import 'package:analyzer/src/generated/source.dart';
+import 'package:analyzer/src/generated/scanner.dart';
 
 // TODO (danrubel) these are temporary constants as we transition completion
 // relevance from CompletionRelevance.LOW/DEFAULT/HIGH to int.
 // These should be removed in a subsequent CL
 const int COMPLETION_RELEVANCE_LOW = 500;
 const int COMPLETION_RELEVANCE_DEFAULT = 1000;
 const int COMPLETION_RELEVANCE_HIGH = 2000;
 
 /**
  * The base class for computing code completion suggestions.
@@ skipping 20 matching lines @@
 
 /**
  * Manages code completion for a given Dart file completion request.
  */
 class DartCompletionManager extends CompletionManager {
   final SearchEngine searchEngine;
   final DartCompletionCache cache;
   List<DartCompletionComputer> computers;
 
   DartCompletionManager(AnalysisContext context, this.searchEngine,
-      Source source, this.cache)
-      : super(context, source),
-        computers = [
+      Source source, this.cache, [this.computers])
+      : super(context, source) {
+    if (computers == null) {
+      computers = [
           new KeywordComputer(),
           new LocalComputer(),
           new ArgListComputer(),
           new CombinatorComputer(),
           new ImportedComputer(),
           new InvocationComputer()];
+    }
+  }
 
   /**
    * Create a new initialized Dart source completion manager
    */
   factory DartCompletionManager.create(AnalysisContext context,
       SearchEngine searchEngine, Source source) {
     return new DartCompletionManager(
         context,
         searchEngine,
         source,
@@ skipping 14 matching lines @@
   /**
    * Compute suggestions based upon cached information only
    * then send an initial response to the client.
    * Return a list of computers for which [computeFull] should be called
    */
   List<DartCompletionComputer> computeFast(DartCompletionRequest request) {
     return request.performance.logElapseTime('computeFast', () {
       CompilationUnit unit = context.parseCompilationUnit(source);
       request.unit = unit;
       request.node = new NodeLocator.con1(request.offset).searchWithin(unit);
-      request.node.accept(new _ReplacementOffsetBuilder(request));
       request.target = new CompletionTarget.forOffset(unit, request.offset);
+
+      request.replacementOffset = request.offset;

[Paul Berry, 2015/01/28 16:32:21]

Some issues with this code:
- It doesn't handle the case where entity is a keyword token but not an AstNode.
 (This would happen when the cursor is on an "is" or "as" keyword).
- The "if" and "else if" clauses below are redundant, since SimpleIdentifier is
a subclass of AstNode (and the logic is equivalent).
- The test "token.offset <= request.offset" is redundant, since the
CompletionTarget logic will never return a completion target that is after the
requested offset position.

Suggested alternative:

request.replacementOffset = request.offset;
request.replacementLength = 0;
var entity = request.target.entity;
Token targetToken = entity is AstNode ? entity.beginToken : entity;
if (targetToken != null &&
    (targetToken.type == TokenType.KEYWORD ||
     targetToken.type == TokenType.IDENTIFIER)) {
  request.replacementOffset = targetToken.offset;
  request.replacementLength = targetToken.length;
}

[danrubel, 2015/01/28 20:10:43]

Good eyes. I like that code better. Done with a slight tweak.

+      request.replacementLength = 0;
+      var entity = request.target.entity;
+      if (entity is SimpleIdentifier) {
+        if (entity.offset <= request.offset) {
+          request.replacementOffset = entity.offset;
+          request.replacementLength = entity.length;
+        }
+      } else if (entity is AstNode) {
+        Token token = entity.beginToken;
+        if (token.offset <= request.offset &&
+            (token.type == TokenType.KEYWORD || token.type == TokenType.IDENTIFIER)) {
+          request.replacementOffset = token.offset;
+          request.replacementLength = token.length;
+        }
+      }
+
       List<DartCompletionComputer> todo = new List.from(computers);
       todo.removeWhere((DartCompletionComputer c) {
         return request.performance.logElapseTime(
             'computeFast ${c.runtimeType}',
             () {
           return c.computeFast(request);
         });
       });
       sendResults(request, todo.isEmpty);
       return todo;
     });
   }
 
   /**
    * If there is remaining work to be done, then wait for the unit to be
    * resolved and request that each remaining computer finish their work.
+   * Return a [Future] that completes when the last notification has been sent.
    */
-  void computeFull(DartCompletionRequest request,
+  Future computeFull(DartCompletionRequest request,
       List<DartCompletionComputer> todo) {
     request.performance.logStartTime('waitForAnalysis');
-    waitForAnalysis().then((CompilationUnit unit) {
-      if (controller.isClosed) {

[Paul Berry, 2015/01/28 16:32:21]

Nit: can we keep this?  Granted, it's unnecessary because of your change to
sendResults() below, but it shouldn't cause any harm, and it will save us from
doing unnecessary work in the case where the completion results aren't needed.

[danrubel, 2015/01/28 20:10:43]

Makes sense. I'll put this back in. I've made the test code use this class's
computeFast method rather than rolling it into the test infrastructure. I want
to do the same for this computeFull method which is why I removed the
controller.isClosed() statement, but based on your suggestion I'll find a
different modification that will satisfy testing and keeping this statement for
efficiency.

-        return;
-      }
+    return waitForAnalysis().then((CompilationUnit unit) {
       request.performance.logElapseTime('waitForAnalysis');
       if (unit == null) {
         sendResults(request, true);
         return;
       }
       request.performance.logElapseTime('computeFull', () {
         request.unit = unit;
         request.node = new NodeLocator.con1(request.offset).searchWithin(unit);
         // TODO(paulberry): Do we need to invoke _ReplacementOffsetBuilder
         // again?
@@ skipping 31 matching lines @@
       if (!todo.isEmpty) {
         computeFull(request, todo);
       }
     });
   }
 
   /**
    * Send the current list of suggestions to the client.
    */
   void sendResults(DartCompletionRequest request, bool last) {
-    if (controller.isClosed) {
+    if (controller == null || controller.isClosed) {
       return;
     }
     controller.add(
         new CompletionResult(
             request.replacementOffset,
             request.replacementLength,
             request.suggestions,
             last));
     if (last) {
       controller.close();
@@ skipping 113 matching lines @@
    * Information about the types of suggestions that should be included.
    * The [target] must be set first.
    */
   OpType get optype {
     if (_optype == null) {
       _optype = new OpType.forCompletion(target, offset);
     }
     return _optype;
   }
 }
-
-/**
- * Visitor used to determine the replacement offset and length
- * based upon the cursor location.
- */
-class _ReplacementOffsetBuilder extends SimpleAstVisitor {
-  final DartCompletionRequest request;
-
-  _ReplacementOffsetBuilder(this.request) {
-    request.replacementOffset = request.offset;
-    request.replacementLength = 0;
-  }
-
-  visitSimpleIdentifier(SimpleIdentifier node) {
-    request.replacementOffset = node.offset;
-    request.replacementLength = node.length;
-  }
-}