API for the new AnalysisDriver.

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

Index: pkg/analyzer/lib/src/dart/analysis/driver.dart
diff --git a/pkg/analyzer/lib/src/dart/analysis/driver.dart b/pkg/analyzer/lib/src/dart/analysis/driver.dart
new file mode 100644
index 0000000000000000000000000000000000000000..3f9c89905e5732a4e0cd8a1f47829a69f5d8f4cf
--- /dev/null
+++ b/pkg/analyzer/lib/src/dart/analysis/driver.dart
@@ -0,0 +1,178 @@
+// Copyright (c) 2016, 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.
+
+import 'dart:async';
+
+import 'package:analyzer/dart/ast/ast.dart';
+import 'package:analyzer/error/error.dart';
+import 'package:analyzer/file_system/file_system.dart';
+import 'package:analyzer/src/dart/analysis/byte_store.dart';
+import 'package:analyzer/src/generated/source.dart';
+
+/**
+ * This class computes [AnalysisResult]s for Dart files.
+ */
+class AnalysisDriver {

[Brian Wilkerson, 2016/10/25 17:24:12]

It looks like AnalysisDriver is the replacement for AnalysisContext. Is that a
fair understanding?

[scheglov, 2016/10/25 18:38:45]

It seems so - it produces analysis results, clients notify it about changes.

+  /**
+   * The byte storage to get and put serialized data.
+   *
+   * It can be shared between other [AnalysisDriver]s.

[Paul Berry, 2016/10/23 10:39:19]

s/between/with/.

Also, yay!  I like how you've designed for cross-context caching from the
beginning.

[scheglov, 2016/10/23 20:54:34]

Done.

+   */
+  final ByteStore _byteStore;
+
+  /**
+   * The [SourceFactory] is used to resolve URIs to paths and restore URIs
+   * from file paths.
+   */
+  final SourceFactory _sourceFactory;
+
+  /**
+   * This [ContentCache] is consulted for a file content before reading
+   * the content from the file.
+   */
+  final ContentCache _contentCache;
+
+  AnalysisDriver(this._byteStore, this._sourceFactory, this._contentCache);
+
+  /**
+   * Set the list of files that the driver should try to analyze sooner.
+   *
+   * Every path in the list must be absolute and normalized.
+   *
+   * The driver will produce the results through the [results] stream. The
+   * exact order in which results are produced is not defined, neither
+   * between priority files, not between them and not priority files.

[Paul Berry, 2016/10/23 10:39:19]

s/not priority/non-priority/

[scheglov, 2016/10/23 20:54:34]

Done.

+   */
+  void set priorityFiles(List<String> priorityPaths) {
+    // TODO(scheglov) implement
+  }
+
+  /**
+   * Return the [Stream] that produces [AnalysisResult]s for added files.
+   *
+   * Analysis starts when the client starts listening to the stream, and stops
+   * when the client cancels the subscription.
+   *
+   * Analysis is eventual, the driver will try to produce results that are at
+   * some point more consistent with the state of the files, but does not
+   * guarantee that this will ever happen.

[Paul Berry, 2016/10/23 10:39:19]

I think we should make a stronger guarantee than this.  To do so we have to
define some additional terminology.  How about something like this:

Let the set of "explicitly analyzed files" denote the set of paths that have
been passed to [addFile] but not subsequently passed to [removeFile].  Let the
"current analysis results" denote the map from the set of explicitly analyzed
files to the most recent [AnalysisResult] delivered to [results] for each file. 
Let the "current file state" represent a map from file path to the file contents
most recently read from that file, or fetched from the content cache
(considering all possible possible file paths, regardless of whether they're in
the set of explicitly analyzed files).  Let the "analysis state" be either
"analyzing" or "idle".

(These are theoretical constructs; they may not necessarily reflect data
structures maintained explicitly by the driver).

Then we make the following guarantees:

- Whenever the analysis state is idle, the current analysis results are
consistent with the current file state.

- A call to [addFile] or [changeFile] causes the analysis state to transition to
"analyzing", and schedules the file contents to be read into the current file
state prior to the next time the analysis state transitions back to "idle".

- If at any time the client stops making calls to [addFile], [changeFile], and
[deleteFile], the analysis state will eventually transition back to "idle" after
a finite amount of processing.

As a result of these guarantees, a client may ensure that the analysis results
are "eventually consistent" with the filesystem by simply calling [changeFile]
any time the contents of a file on the file system have changed.

[scheglov, 2016/10/23 20:54:34]

Thank you, Paul.
I added this comment to the class documentation with a tiny change - when
[addFile] or [changeFile] are called, I explicitly stated that only content of
the given files will be read again.

+   *
+   * More than one result might be produced for the same file, even if the
+   * client does not change the state of the files.
+   *
+   * Results might be produced even for files that have never been added
+   * using [addFile], for example when [getResult] was called for a file.
+   */
+  Stream<AnalysisResult> get results async* {
+    // TODO(scheglov) implement
+  }
+
+  /**
+   * Add the file with the given [path] to the set of files to analyze.
+   *
+   * The [path] must be absolute and normalized.
+   *
+   * The results of analysis are eventually produced by the [results] stream.
+   */
+  void addFile(String path) {

[Brian Wilkerson, 2016/10/25 17:24:12]

I think we'll want addFiles (plural) for performance reasons.
I think we need to add URI's (or Sources), given that two different URI's can
map to the same file.

[scheglov, 2016/10/25 18:38:45]

I don't know yet if we need addFiles().
Maybe as a potential optimization in the future.
For now, we won't get any.

  void addFile(String path) {
    _explicitFiles.add(path);
    _filesToAnalyze.add(path);
    _hasWorkStreamController.add('do it!');
  }


I don't understand yet about different URIs for the same file.
You have the file system, and you tell the driver to analyzer a file.
This file is either in lib/ (so package:x/y.dart) or outside of it (so
file://path/to/the/file.dart).

[Brian Wilkerson, 2016/10/26 06:54:47]

// foo/lib/a.dart
import 'file:///c.dart'

// foo/lib/b.dart
import 'package:foo/c.dart'

Unless we change the ecosystem, these imports have different URI's that both
resolve to the same file, and we are required to treat them as importing
different (albeit equivalent) libraries.

[scheglov, 2016/10/26 16:45:39]

OK, I see.
Let's discuss this when we're all in the office.

+    // TODO(scheglov) implement
+  }
+
+  /**
+   * The file with the given [path] might have changed - updated, added or
+   * removed. Or not, we don't know. Or it might have, but then changed back.
+   *
+   * The [path] must be absolute and normalized.
+   *
+   * The driver might use this information to decide that new analysis results
+   * should be produced, but does not guarantee this, nor for the given file,
+   * nor for any other file.

[Paul Berry, 2016/10/23 10:39:19]

If you take my suggestion above, you could replace this paragraph with something
like:

Causes the analysis state to transition to "analyzing" (if it is not in that
state already).  Schedules the file contents for [path] to be read into the
current file state prior to the next time the analysis state transitions to
"idle".

[scheglov, 2016/10/23 20:54:34]

Done.

+   *
+   * Invocation of this method will not prevent a [Future] returned from
+   * [getResult] from completing with a result, and does not guarantee that the
+   * result will reflect the state of the file at the moment before, at or
+   * after the invocation of [changeFile].
+   */

[Paul Berry, 2016/10/23 10:39:19]

Is it ok to pass a path to [changeFile] that is not in the set of explicitly
analyzed files?  I hope so because it would be nice to be able to force analysis
to be updated when an implicitly analyzed file changes.

[scheglov, 2016/10/23 20:54:34]

Yes, it is OK to notify about implicitly analyzed files changes!

I updated the comment.

+  void changeFile(String path) {
+    // TODO(scheglov) implement
+  }
+
+  /**
+   * Return the [Future] that completes with a [AnalysisResult] for the file
+   * with the given [path].
+   *
+   * The [path] must be absolute and normalized.

[Paul Berry, 2016/10/23 10:39:19]

Is it required that the path is in the set of explicitly analyzed files?

[scheglov, 2016/10/23 20:54:34]

No, it isn't.
The path can be any file - explicitly or implicitly analyzed, or neither.

The users might navigate to an implicitly analyzed file, e.g.  a used library in
a package, and we need to produce the fully resolved unit for it.

But the used might then open a sibling file in the package, which she does not
use yet, but might want look on.
So, we need to analyze and produce a fully resolved unit, at least in one of the
contexts.
The context need to "contain" the file and its dependencies in its
SourceFactory.

We might need a way to prevent "poisoning" the context with these "ad hoc"
analyzed files -they might extend the set of implicitly analyzed files, even
though the original file has never been an explicitly analyzed one, and the user
closed the editor tab with it 5 seconds after opening. Actually, this is a
problem even for explicitly analyzed files - libraries that were imported and
the removed from the imports list. I will add TODO.

+   *
+   * The result is not guaranteed to be produced for the state of the file
+   * which is closest to the moment of the invocation. But if the client
+   * continues invoking this method, eventually one of the invocations will
+   * return a [Future] that completes with the result that is closer to the
+   * state of the files.

[Paul Berry, 2016/10/23 10:39:19]

Can we make a stronger guarantee when the analysis state is idle?

[scheglov, 2016/10/23 20:54:34]

Done.

+   */
+  Future<AnalysisResult> getResult(String path) {
+    // TODO(scheglov) implement
+    throw new UnimplementedError();
+  }
+
+  /**
+   * Remove the file with the given [path] from the list of files to analyze.
+   *
+   * The [path] must be absolute and normalized.
+   *
+   * The results of analysis of the file might still be produced by the
+   * [results] stream. The driver will try to stop producing these results,
+   * but does not guarantee this.
+   */
+  void removeFile(String path) {
+    // TODO(scheglov) implement
+  }
+}

[Paul Berry, 2016/10/23 10:39:19]

Additional API that we may need to add in the future:

- A stream of "analysis status" information so that the client can know when the
analysis state is idle.

- Some way to indicate when analysis state transitions between "idle" and
"analyzing".

- Some way for the client to know when a file is added to or removed from the
set of implicitly analyzed files, in case it wants to watch implicitly analyzed
files for changes.  (Note: in order to avoid a gap where the file is not being
watched, this would need to be a callback rather than a stream, so that the
client can be sure it starts watching the file before the driver reads it).

- A single method call which is equivalent to calling [changeFile] on all
reachable files (both explicit and implicit).  This would allow the client to
recover from file watching bugs.

- The ability to abstract the file system using
pkg/front_end/lib/file_system.dart

[scheglov, 2016/10/23 20:54:34]

  Thanks, I will add this to the list.
  What is the difference between the first and the second?

[Paul Berry, 2016/10/23 21:07:19]

Oops, sorry.  No difference.  I'm not sure what I was thinking.

+
+/**
+ * The result of analyzing of a single file.
+ *
+ * These results are self-consistent, i.e. [content], [contentHash], the
+ * resolved [unit] correspond to each other. All referenced elements, even
+ * external ones, are also self-consistent. But none of the results is
+ * guaranteed to be consistent with the state of the files.
+ *
+ * Every result is independent, and is not guaranteed to be consistent with
+ * any previously returned result, even inside of the same library.
+ */
+class AnalysisResult {
+  /**
+   * The path of the analysed file, absolute and normalized.
+   */
+  final String path;
+
+  /**
+   * The URI of the file that corresponded to the [path] in the used
+   * [SourceFactory] at some point. Is it not guaranteed to be still consistent
+   * to the [path], and provided as FYI.
+   */
+  final Uri uri;
+
+  /**
+   * The content of the file that was scanned, parsed and resolved.
+   */
+  final String content;
+
+  /**
+   * The MD5 hash of the [content].
+   */
+  final String contentHash;
+
+  /**
+   * The fully resolved compilation unit for the [content].
+   */
+  final CompilationUnit unit;
+
+  /**
+   * The full list of computed analysis errors, both syntactic and semantic.
+   */
+  final List<AnalysisError> errors;
+
+  AnalysisResult(this.path, this.uri, this.content, this.contentHash, this.unit,
+      this.errors);
+}