Begin adding top-level function declaration support to AstBuilder.

Begin adding top-level function declaration support to AstBuilder.

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

Committed: https://github.com/dart-lang/sdk/commit/901dc916cf2d36c8075b053ade74b5e6e38b4306

[Paul Berry, 2017-02-16 23:56:28]

paulberry@google.com changed reviewers:
+ ahe@google.com, scheglov@google.com

[scheglov, 2017-02-17 00:48:05]

LGTM

[ahe, 2017-02-17 09:02:03]

lgtm

All my comments below are conversation, not action items for code review.

https://codereview.chromium.org/2701513006/diff/1/pkg/analyzer/test/generated...
File pkg/analyzer/test/generated/parser_fasta_test.dart (right):

https://codereview.chromium.org/2701513006/diff/1/pkg/analyzer/test/generated...
pkg/analyzer/test/generated/parser_fasta_test.dart:643: // as the name of a top
level function with an implicit return type.
I filed this bug: https://github.com/dart-lang/sdk/issues/28808

Perhaps you can use that instead of the TODO. Please don't hesitate to file bugs
like this against Fasta.

https://codereview.chromium.org/2701513006/diff/1/pkg/front_end/lib/src/fasta...
File pkg/front_end/lib/src/fasta/analyzer/ast_builder.dart (right):

https://codereview.chromium.org/2701513006/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/analyzer/ast_builder.dart:8: show BeginGroupToken,
Token;
FYI: I'm really happy to see that dartfmt doesn't remove the newlines between
imports :-)
I was actually worried about that.

https://codereview.chromium.org/2701513006/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/analyzer/ast_builder.dart:598: // the necessary
analyzer AST data structures.
What's missing? Position information?

https://codereview.chromium.org/2701513006/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/analyzer/ast_builder.dart:648: analyzer.Token
beginToken = null; // TODO(paulberry)
Here's a suggestion for when you get to this:

@override
void beginCompilationUnit(Token begin) {
  push(token);
}

https://codereview.chromium.org/2701513006/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/analyzer/ast_builder.dart:649: ScriptTag scriptTag =
null; // TODO(paulberry)
What are script tags?

[Paul Berry, 2017-02-17 16:14:26]

https://codereview.chromium.org/2701513006/diff/1/pkg/analyzer/test/generated...
File pkg/analyzer/test/generated/parser_fasta_test.dart (right):

https://codereview.chromium.org/2701513006/diff/1/pkg/analyzer/test/generated...
pkg/analyzer/test/generated/parser_fasta_test.dart:643: // as the name of a top
level function with an implicit return type.
On 2017/02/17 09:02:02, ahe wrote:
> I filed this bug: https://github.com/dart-lang/sdk/issues/28808
> 
> Perhaps you can use that instead of the TODO. Please don't hesitate to file
bugs
> like this against Fasta.

Thanks.  I'll look through my recent CLs to see if there are other test failures
I've characterized well enough to file bugs for them.

https://codereview.chromium.org/2701513006/diff/1/pkg/front_end/lib/src/fasta...
File pkg/front_end/lib/src/fasta/analyzer/ast_builder.dart (right):

https://codereview.chromium.org/2701513006/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/analyzer/ast_builder.dart:598: // the necessary
analyzer AST data structures.
On 2017/02/17 09:02:02, ahe wrote:
> What's missing? Position information?

Yes.  The analyzer ASTs expect to be able to point to the "async", "sync",
and/or "*" tokens.  This is important for code completion, which needs to find
the exact point in the AST matching the cursor position; the only way it can do
that is to be able to access the positions of all tokens.

https://codereview.chromium.org/2701513006/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/analyzer/ast_builder.dart:648: analyzer.Token
beginToken = null; // TODO(paulberry)
On 2017/02/17 09:02:02, ahe wrote:
> Here's a suggestion for when you get to this:
> 
> @override
> void beginCompilationUnit(Token begin) {
>   push(token);
> }

Thanks!

https://codereview.chromium.org/2701513006/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/analyzer/ast_builder.dart:649: ScriptTag scriptTag =
null; // TODO(paulberry)
On 2017/02/17 09:02:02, ahe wrote:
> What are script tags?

"script tag" is the Dart spec's name for the initial line of a file if it starts
with "#!".  See p.128 of
http://www.ecma-international.org/publications/files/ECMA-ST/ECMA-408.pdf

[Paul Berry, 2017-02-17 16:25:49]

Description was changed from

==========
Begin adding top-level function declaration support to AstBuilder.
==========

to

==========
Begin adding top-level function declaration support to AstBuilder.

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

Committed:
https://github.com/dart-lang/sdk/commit/901dc916cf2d36c8075b053ade74b5e6e38b4306
==========

[Paul Berry, 2017-02-17 16:25:50]

Committed patchset #1 (id:1) manually as
901dc916cf2d36c8075b053ade74b5e6e38b4306 (presubmit successful).

[ahe, 2017-02-17 18:05:50]

FYI

https://codereview.chromium.org/2701513006/diff/1/pkg/front_end/lib/src/fasta...
File pkg/front_end/lib/src/fasta/analyzer/ast_builder.dart (right):

https://codereview.chromium.org/2701513006/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/analyzer/ast_builder.dart:598: // the necessary
analyzer AST data structures.
On 2017/02/17 16:14:26, Paul Berry wrote:
> On 2017/02/17 09:02:02, ahe wrote:
> > What's missing? Position information?
> 
> Yes.  The analyzer ASTs expect to be able to point to the "async", "sync",
> and/or "*" tokens.  This is important for code completion, which needs to find
> the exact point in the AST matching the cursor position; the only way it can
do
> that is to be able to access the positions of all tokens.

OK. handleAsyncModifier should probably just create the object the analyzer
needs.

https://codereview.chromium.org/2701513006/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/analyzer/ast_builder.dart:649: ScriptTag scriptTag =
null; // TODO(paulberry)
On 2017/02/17 16:14:26, Paul Berry wrote:
> On 2017/02/17 09:02:02, ahe wrote:
> > What are script tags?
> 
> "script tag" is the Dart spec's name for the initial line of a file if it
starts
> with "#!".  See p.128 of
> http://www.ecma-international.org/publications/files/ECMA-ST/ECMA-408.pdf

Ah. Shebang :-)

Assuming this is just the first line of such a file, it should be easy to
support, it must just have been overlooked when we added support for comments.
AbstractScanner.tokenizeTag just needs to call appendComment (or a new method
similar to appendComment).