Make creating LibraryContext async, so all its transitive clients.

Make creating LibraryContext async, so all its transitive clients.

This prepare us for resynthesizing element model from Kernel, because
KernelDriver is async, because everything in front-end is async.

R=brianwilkerson@google.com, paulberry@google.com
BUG=

Committed: https://github.com/dart-lang/sdk/commit/1d5de7ffd52b3362f6093c6d0de5ec49f01d064f

[Brian Wilkerson, 2017-08-25 19:44:12]

lgtm

https://codereview.chromium.org/3010463002/diff/1/pkg/analyzer/lib/src/dart/a...
File pkg/analyzer/lib/src/dart/analysis/driver.dart (right):

https://codereview.chromium.org/3010463002/diff/1/pkg/analyzer/lib/src/dart/a...
pkg/analyzer/lib/src/dart/analysis/driver.dart:574:
_runTestAsyncWorkDuringAnalysis(path);
Missing await?

Also, could we document when we need to invoke `_runTestAsyncWorkDuringAnalysis`
after invoking `_computeAnalysisResult` and when we don't?

https://codereview.chromium.org/3010463002/diff/1/pkg/analyzer/lib/src/dart/a...
pkg/analyzer/lib/src/dart/analysis/driver.dart:577: }
And when do we need to check for changed files, and when not?

[scheglov, 2017-08-25 19:51:56]

https://codereview.chromium.org/3010463002/diff/1/pkg/analyzer/lib/src/dart/a...
File pkg/analyzer/lib/src/dart/analysis/driver.dart (right):

https://codereview.chromium.org/3010463002/diff/1/pkg/analyzer/lib/src/dart/a...
pkg/analyzer/lib/src/dart/analysis/driver.dart:577: }
On 2017/08/25 19:44:12, Brian Wilkerson wrote:
> And when do we need to check for changed files, and when not?

I added comments to `_runTestAsyncWorkDuringAnalysis`.
It is used only for tests, so we don't have to call it everywhere, as long as we
are sure that there is no problem that we want to test.

[scheglov, 2017-08-25 19:52:17]

Description was changed from

==========
Make creating LibraryContext async, so all its transitive clients.

This prepare us for resynthesizing element model from Kernel, because
KernelDriver is async, because everything in front-end is async.

R=brianwilkerson@google.com, paulberry@google.com
BUG=
==========

to

==========
Make creating LibraryContext async, so all its transitive clients.

This prepare us for resynthesizing element model from Kernel, because
KernelDriver is async, because everything in front-end is async.

R=brianwilkerson@google.com, paulberry@google.com
BUG=

Committed:
https://github.com/dart-lang/sdk/commit/1d5de7ffd52b3362f6093c6d0de5ec49f01d064f
==========

[scheglov, 2017-08-25 19:52:19]

Committed patchset #2 (id:20001) manually as
1d5de7ffd52b3362f6093c6d0de5ec49f01d064f (presubmit successful).

[Paul Berry, 2017-08-25 20:01:56]

lgtm assuming comments below are addressed

https://codereview.chromium.org/3010463002/diff/1/pkg/analyzer/lib/src/dart/a...
File pkg/analyzer/lib/src/dart/analysis/driver.dart (right):

https://codereview.chromium.org/3010463002/diff/1/pkg/analyzer/lib/src/dart/a...
pkg/analyzer/lib/src/dart/analysis/driver.dart:1321: return work != null ?
work(path) : null;
I get scared when I see a method with a return type of Future that returns null.
 How about this instead?

return work != null ? work(path) : new Future.value();

https://codereview.chromium.org/3010463002/diff/1/pkg/analyzer/lib/src/dart/a...
pkg/analyzer/lib/src/dart/analysis/driver.dart:1673: Future<Null>
Function(String path) workToWaitAfterComputingResult;
Would you mind adding a doc comment for this?  It's not obvious what its
behavior is from the name.

E.g. something like:

/// If non-null, a function that should be executed asynchronously after
/// the next result is computed.
///
/// This can be used by a test to simulate file changes during analysis.

https://codereview.chromium.org/3010463002/diff/20001/pkg/analyzer/lib/src/da...
File pkg/analyzer/lib/src/dart/analysis/driver.dart (right):

https://codereview.chromium.org/3010463002/diff/20001/pkg/analyzer/lib/src/da...
pkg/analyzer/lib/src/dart/analysis/driver.dart:1321: * also include the changes.
This comment doesn't actually help me understand what this method does. 
Consider rephrasing this as a third person verb phrase (see
https://www.dartlang.org/guides/language/effective-dart/documentation#prefer-...)

E.g. maybe the comment should look something like:

/**
 * Runs any asynchronous work that was injected as part of
 * a test using
 * [AnalysisDriverTestView.workToWaitAfterComputingResult].
 *
 * If the test view indicates that there is work to do,
 * performs the work and returns a Future that will be
 * signaled when the work completes.
 *
 * This gives tests a reliable way to simulate file changes
 * during analysis.
 * ...
 */

(I'm guessing at some of these details; feel free to edit for correctness)

[scheglov, 2017-08-25 20:30:46]

I'm fixing these new comments in https://codereview.chromium.org/3003923002 now.

https://codereview.chromium.org/3010463002/diff/1/pkg/analyzer/lib/src/dart/a...
File pkg/analyzer/lib/src/dart/analysis/driver.dart (right):

https://codereview.chromium.org/3010463002/diff/1/pkg/analyzer/lib/src/dart/a...
pkg/analyzer/lib/src/dart/analysis/driver.dart:1321: return work != null ?
work(path) : null;
On 2017/08/25 20:01:55, Paul Berry wrote:
> I get scared when I see a method with a return type of Future that returns
null.
>  How about this instead?
> 
> return work != null ? work(path) : new Future.value();

Done.

https://codereview.chromium.org/3010463002/diff/1/pkg/analyzer/lib/src/dart/a...
pkg/analyzer/lib/src/dart/analysis/driver.dart:1673: Future<Null>
Function(String path) workToWaitAfterComputingResult;
On 2017/08/25 20:01:55, Paul Berry wrote:
> Would you mind adding a doc comment for this?  It's not obvious what its
> behavior is from the name.
> 
> E.g. something like:
> 
> /// If non-null, a function that should be executed asynchronously after
> /// the next result is computed.
> ///
> /// This can be used by a test to simulate file changes during analysis.

Done.

https://codereview.chromium.org/3010463002/diff/20001/pkg/analyzer/lib/src/da...
File pkg/analyzer/lib/src/dart/analysis/driver.dart (right):

https://codereview.chromium.org/3010463002/diff/20001/pkg/analyzer/lib/src/da...
pkg/analyzer/lib/src/dart/analysis/driver.dart:1321: * also include the changes.
On 2017/08/25 20:01:55, Paul Berry wrote:
> This comment doesn't actually help me understand what this method does. 
> Consider rephrasing this as a third person verb phrase (see
>
https://www.dartlang.org/guides/language/effective-dart/documentation#prefer-...)
> 
> E.g. maybe the comment should look something like:
> 
> /**
>  * Runs any asynchronous work that was injected as part of
>  * a test using
>  * [AnalysisDriverTestView.workToWaitAfterComputingResult].
>  *
>  * If the test view indicates that there is work to do,
>  * performs the work and returns a Future that will be
>  * signaled when the work completes.
>  *
>  * This gives tests a reliable way to simulate file changes
>  * during analysis.
>  * ...
>  */
> 
> (I'm guessing at some of these details; feel free to edit for correctness)

Done.