Convert the command line dart analyzer to be async.

Convert the command line dart analyzer to be async.

R=brianwilkerson@google.com, scheglov@google.com

Committed: https://code.google.com/p/dart/source/detail?r=33558

[Bob Nystrom, 2014-03-11 18:24:46]

https://codereview.chromium.org/195483004/diff/40001/pkg/analyzer/lib/src/ana...
File pkg/analyzer/lib/src/analyzer_impl.dart (right):

https://codereview.chromium.org/195483004/diff/40001/pkg/analyzer/lib/src/ana...
pkg/analyzer/lib/src/analyzer_impl.dart:68: void _analyze() {
Drive-by code review!

It's considered good style for any asynchronous code to return a Future.
Otherwise, the caller has no way of waiting until the operation has completed,
or catching an error.

There are exceptions to this (for example, if you are intentionally doing work
in the background and piping the results to a stream), but these are rare.

Even though this function is private, it's still helpful to return the future
because it makes it clear to the reader that this is doing asynchronous work.

https://codereview.chromium.org/195483004/diff/40001/pkg/analyzer/lib/src/ana...
pkg/analyzer/lib/src/analyzer_impl.dart:70: List<ChangeNotice> notices =
result.changeNotices;
Style nit: this function body should be indented +2 relative to "new Future".

https://codereview.chromium.org/195483004/diff/40001/pkg/analyzer/lib/src/ana...
pkg/analyzer/lib/src/analyzer_impl.dart:73: if(notices != null) {
Space after "if".

https://codereview.chromium.org/195483004/diff/40001/pkg/analyzer/lib/src/ana...
pkg/analyzer/lib/src/analyzer_impl.dart:80: } else {
Friendly suggestion: Since the if case always returns, the else is unneeded
here. That would let you get rid of a level of indentation.

https://codereview.chromium.org/195483004/diff/40001/pkg/analyzer/lib/src/ana...
pkg/analyzer/lib/src/analyzer_impl.dart:123:
AnalysisEngine.instance.logger.logError(exception);
This only needs to be indented +2.

[Brian Wilkerson, 2014-03-11 18:36:53]

LGTM

https://codereview.chromium.org/195483004/diff/40001/editor/tools/plugins/com...
File
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/context/AnalysisResult.java
(right):

https://codereview.chromium.org/195483004/diff/40001/editor/tools/plugins/com...
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/context/AnalysisResult.java:100:
public boolean isMoreWork() {
Perhaps "hasMoreWork"?

Also, the comment states the opposite of what this method does.

https://codereview.chromium.org/195483004/diff/40001/pkg/analyzer/bin/analyze...
File pkg/analyzer/bin/analyzer.dart (right):

https://codereview.chromium.org/195483004/diff/40001/pkg/analyzer/bin/analyze...
pkg/analyzer/bin/analyzer.dart:32: void _runAnalyzer(var options) {
Please provide a type annotation for the parameter.

https://codereview.chromium.org/195483004/diff/40001/pkg/analyzer/lib/src/ana...
File pkg/analyzer/lib/src/analyzer_impl.dart (right):

https://codereview.chromium.org/195483004/diff/40001/pkg/analyzer/lib/src/ana...
pkg/analyzer/lib/src/analyzer_impl.dart:42: AnalyzerImpl(String this.sourcePath,
CommandLineOptions this.options, int this.startTime) {
Don't include type annotations for field initializer formal parameters.

https://codereview.chromium.org/195483004/diff/40001/pkg/analyzer/lib/src/ana...
pkg/analyzer/lib/src/analyzer_impl.dart:73: if(notices != null) {
You're not making use of the utility method you defined. I assume that's because
it hasn't been translated yet.

https://codereview.chromium.org/195483004/diff/40001/pkg/analyzer/lib/src/ana...
pkg/analyzer/lib/src/analyzer_impl.dart:76: for(ChangeNotice notice in notices)
{
nit: space after "for"

[scheglov, 2014-03-11 18:44:24]

LGTM

[jwren, 2014-03-11 21:53:04]

https://codereview.chromium.org/195483004/diff/40001/editor/tools/plugins/com...
File
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/context/AnalysisResult.java
(right):

https://codereview.chromium.org/195483004/diff/40001/editor/tools/plugins/com...
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/context/AnalysisResult.java:100:
public boolean isMoreWork() {
On 2014/03/11 18:36:53, Brian Wilkerson wrote:
> Perhaps "hasMoreWork"?
> 
> Also, the comment states the opposite of what this method does.

Done.

https://codereview.chromium.org/195483004/diff/40001/pkg/analyzer/bin/analyze...
File pkg/analyzer/bin/analyzer.dart (right):

https://codereview.chromium.org/195483004/diff/40001/pkg/analyzer/bin/analyze...
pkg/analyzer/bin/analyzer.dart:32: void _runAnalyzer(var options) {
On 2014/03/11 18:36:53, Brian Wilkerson wrote:
> Please provide a type annotation for the parameter.

Done.

https://codereview.chromium.org/195483004/diff/40001/pkg/analyzer/lib/src/ana...
File pkg/analyzer/lib/src/analyzer_impl.dart (right):

https://codereview.chromium.org/195483004/diff/40001/pkg/analyzer/lib/src/ana...
pkg/analyzer/lib/src/analyzer_impl.dart:42: AnalyzerImpl(String this.sourcePath,
CommandLineOptions this.options, int this.startTime) {
On 2014/03/11 18:36:53, Brian Wilkerson wrote:
> Don't include type annotations for field initializer formal parameters.

Done.

https://codereview.chromium.org/195483004/diff/40001/pkg/analyzer/lib/src/ana...
pkg/analyzer/lib/src/analyzer_impl.dart:68: void _analyze() {
Bob- Thanks for the feedback.  For now we are going to leaving it as-is (since
it is private as you pointed out) and will revisit in the future.

https://codereview.chromium.org/195483004/diff/40001/pkg/analyzer/lib/src/ana...
pkg/analyzer/lib/src/analyzer_impl.dart:70: List<ChangeNotice> notices =
result.changeNotices;
On 2014/03/11 18:24:46, Bob Nystrom wrote:
> Style nit: this function body should be indented +2 relative to "new Future".

Done.

https://codereview.chromium.org/195483004/diff/40001/pkg/analyzer/lib/src/ana...
pkg/analyzer/lib/src/analyzer_impl.dart:73: if(notices != null) {
On 2014/03/11 18:36:53, Brian Wilkerson wrote:
> You're not making use of the utility method you defined. I assume that's
because
> it hasn't been translated yet.

Done.
Correct.

https://codereview.chromium.org/195483004/diff/40001/pkg/analyzer/lib/src/ana...
pkg/analyzer/lib/src/analyzer_impl.dart:76: for(ChangeNotice notice in notices)
{
On 2014/03/11 18:36:53, Brian Wilkerson wrote:
> nit: space after "for"

Done.

https://codereview.chromium.org/195483004/diff/40001/pkg/analyzer/lib/src/ana...
pkg/analyzer/lib/src/analyzer_impl.dart:80: } else {
On 2014/03/11 18:24:46, Bob Nystrom wrote:
> Friendly suggestion: Since the if case always returns, the else is unneeded
> here. That would let you get rid of a level of indentation.

Done.

https://codereview.chromium.org/195483004/diff/40001/pkg/analyzer/lib/src/ana...
pkg/analyzer/lib/src/analyzer_impl.dart:123:
AnalysisEngine.instance.logger.logError(exception);
On 2014/03/11 18:24:46, Bob Nystrom wrote:
> This only needs to be indented +2.

Done.

[jwren, 2014-03-11 22:05:42]

Committed patchset #6 manually as r33558 (presubmit successful).