step toward new completion API

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

Index: pkg/analysis_server/lib/src/services/completion/completion_dart.dart
diff --git a/pkg/analysis_server/lib/src/services/completion/completion_dart.dart b/pkg/analysis_server/lib/src/services/completion/completion_dart.dart
index 1cfff042d6017ca98311ae8b46b881391146b2e3..98370b4bc5d10170dbdd1dd8e02cc3098bc370d6 100644
--- a/pkg/analysis_server/lib/src/services/completion/completion_dart.dart
+++ b/pkg/analysis_server/lib/src/services/completion/completion_dart.dart
@@ -4,11 +4,21 @@
 
 library analysis_server.src.services.completion.completion_dart;
 
-import 'package:analysis_server/src/provisional/completion/completion_core.dart';
+import 'dart:async';
+
+import 'package:analysis_server/src/provisional/completion/completion_core.dart'
+    show CompletionRequest;
 import 'package:analysis_server/src/provisional/completion/completion_dart.dart';
 import 'package:analysis_server/src/provisional/completion/dart/completion_target.dart';
 import 'package:analysis_server/src/services/completion/completion_core.dart';
+import 'package:analyzer/file_system/file_system.dart';
+import 'package:analyzer/src/context/context.dart'
+    show AnalysisFutureHelper, AnalysisContextImpl;
 import 'package:analyzer/src/generated/ast.dart';
+import 'package:analyzer/src/generated/engine.dart' hide AnalysisContextImpl;
+import 'package:analyzer/src/generated/source.dart';
+import 'package:analyzer/src/task/dart.dart';
+import 'package:analyzer/task/dart.dart';
 
 /**
  * The information about a requested list of completions within a Dart file.
@@ -16,26 +26,77 @@ import 'package:analyzer/src/generated/ast.dart';
 class DartCompletionRequestImpl extends CompletionRequestImpl
     implements DartCompletionRequest {
   /**
-   * The compilation unit in which the completion was requested.
+   * The cached completion target or `null` if not computed yet.
    */
-  final CompilationUnit unit;
+  CompletionTarget _target;
 
   /**
-   * A flag indicating whether the compilation [unit] is resolved.
+   * `true` if [resolveDeclarationsInScope] has partially resolved the unit
+   * referenced by [target], else `false`.
    */
-  final bool isResolved;
+  bool _haveResolveDeclarationsInScope = false;
 
   /**
-   * The completion target.  This determines what part of the parse tree
-   * will receive the newly inserted text.
+   * Initialize a newly created completion request based on the given request.
    */
-  final CompletionTarget target;
+  factory DartCompletionRequestImpl.forRequest(CompletionRequest request) {
+    return new DartCompletionRequestImpl(request.context,
+        request.resourceProvider, request.source, request.offset);
+  }
 
   /**
    * Initialize a newly created completion request based on the given arguments.
    */
-  DartCompletionRequestImpl(
-      CompletionRequest request, this.unit, this.isResolved, this.target)
-      : super(request.context, request.resourceProvider, request.source,
-            request.offset);
+  DartCompletionRequestImpl(AnalysisContext context,
+      ResourceProvider resourceProvider, Source source, int offset)
+      : super(context, resourceProvider, source, offset);

[scheglov, 2015/11/22 05:19:02]

It may be worth to drop this constructor.
It duplicates constructor arguments of CompletionRequestImpl.
If we ever decide to change them, it would be easier to change only
CompletionRequestImpl.
Maybe always call CompletionRequestImpl.from() constructor.

Or use delegation instead of inheritance.
I'm not sure what is better, if any.

[danrubel, 2015/11/23 17:47:32]

Its easier for the tests to use this constructor, but your point is good. I'll
modify the tests to construct a CompletionRequestImpl and then use
DartCompletionRequestImpl.forRequest(CompletionRequest request).

+
+  @override
+  Future<CompilationUnit> resolveDeclarationsInScope() async {
+    CompilationUnit unit = target.unit;
+    if (_haveResolveDeclarationsInScope) {
+      return unit;
+    }
+
+    // Determine the library source
+    Source librarySource;
+    if (unit.directives.any((d) => d is PartOfDirective)) {
+      List<Source> libraries = context.getLibrariesContaining(source);
+      if (libraries.isEmpty) {
+        return null;
+      }
+      librarySource = libraries[0];
+    } else {
+      librarySource = source;
+    }
+
+    // Resolve declarations in the target unit
+    var resolvedUnit = await new AnalysisFutureHelper<CompilationUnit>(

[scheglov, 2015/11/22 05:19:02]

Please add the type.

[danrubel, 2015/11/23 17:47:32]

Done.

+        (context as AnalysisContextImpl),

[scheglov, 2015/11/22 05:19:02]

Drop parenthesis.
Actually I think we don't need even the cast.

[danrubel, 2015/11/23 17:47:32]

Done.

+        new LibrarySpecificUnit(librarySource, source),
+        RESOLVED_UNIT3).computeAsync();

[scheglov, 2015/11/22 05:19:02]

Now, when we have a single implementation of AnalysisContext it may worth to
drop AnalysisFutureHelper at all and replace it with the new
AnalysisContext.computeResultAsync() method. Not sure how to provide the
requested priority - in case of completion it is interactive, do we need any
other?  See WorkOrderPriority.

[Brian Wilkerson, 2015/11/22 18:02:46]

We effectively only have one implementation now, and we will definitely have a
single implementation within a very short period of time. Not sure we need to
wait to do as Konstantin suggested.

[danrubel, 2015/11/23 17:47:32]

Its not clear to me if I need to change anything here. Happy to address in a
subsequent CL.

+
+    // TODO(danrubel) determine if the underlying source has been modified
+    // in a way that invalidates the completion request
+    // and return null
+
+    // Gracefully degrade if unit cannot be resolved
+    if (resolvedUnit == null) {
+      return null;
+    }
+
+    // Recompute the target for the newly resolved unit
+    _target = new CompletionTarget.forOffset(resolvedUnit, offset);
+    _haveResolveDeclarationsInScope = true;
+    return resolvedUnit;
+  }
+
+  @override
+  CompletionTarget get target {
+    if (_target == null) {
+      CompilationUnit unit = context.computeResult(source, PARSED_UNIT);
+      _target = new CompletionTarget.forOffset(unit, offset);
+    }
+    return _target;
+  }
 }