Implement AstBuilder support for library, part, and part-of declarations.

Implement AstBuilder support for library, part, and part-of declarations.

R=scheglov@google.com

Committed: https://github.com/dart-lang/sdk/commit/60c2aae9f8cb02eb2465c254e49c57f25d8583ea

[Paul Berry, 2017-02-18 15:35:42]

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

[scheglov, 2017-02-18 17:31:32]

LGTM

[Paul Berry, 2017-02-18 18:26:25]

Description was changed from

==========
Implement AstBuilder support for library, part, and part-of declarations.
==========

to

==========
Implement AstBuilder support for library, part, and part-of declarations.

R=scheglov@google.com

Committed:
https://github.com/dart-lang/sdk/commit/60c2aae9f8cb02eb2465c254e49c57f25d8583ea
==========

[Paul Berry, 2017-02-18 18:26:26]

Committed patchset #1 (id:1) manually as
60c2aae9f8cb02eb2465c254e49c57f25d8583ea (presubmit successful).

[ahe, 2017-02-20 09:59:40]

lgtm

One minor action item below: endPartOf can also see metadata.

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

https://codereview.chromium.org/2707663002/diff/1/pkg/analyzer/test/generated...
pkg/analyzer/test/generated/parser_fasta_test.dart:800: // TODO(paulberry,ahe):
Fasta doesn't support script tags yet.
Can I assume this is low priority?

https://codereview.chromium.org/2707663002/diff/1/pkg/analyzer/test/generated...
pkg/analyzer/test/generated/parser_fasta_test.dart:817: // ability to Fasta's
parser.
I think that should be easy. Let me know how soon you need this.

https://codereview.chromium.org/2707663002/diff/1/pkg/analyzer/test/generated...
pkg/analyzer/test/generated/parser_fasta_test.dart:919: // supported by Fasta
yet.
Yeah. I think this is legacy syntax, and I generally try to keep support for old
syntax for improved error recovery.

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

https://codereview.chromium.org/2707663002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/analyzer/ast_builder.dart:59: var
accumulateIdentifierComponents = false;
Should we change the parser so it can tell you how many components there are on
the stack instead?

https://codereview.chromium.org/2707663002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/analyzer/ast_builder.dart:927: // in a reference to
the "of" keyword.
We can do so, just file a bug.

https://codereview.chromium.org/2707663002/diff/1/pkg/front_end/lib/src/fasta...
File pkg/front_end/lib/src/fasta/parser/listener.dart (right):

https://codereview.chromium.org/2707663002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/parser/listener.dart:480: /// - Library name (a
qualified identifier)
There can also be metadata here.

[Paul Berry, 2017-02-20 14:48:32]

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

https://codereview.chromium.org/2707663002/diff/1/pkg/analyzer/test/generated...
pkg/analyzer/test/generated/parser_fasta_test.dart:800: // TODO(paulberry,ahe):
Fasta doesn't support script tags yet.
On 2017/02/20 09:59:39, ahe wrote:
> Can I assume this is low priority?

Agreed, It's pretty low priority IMO.

https://codereview.chromium.org/2707663002/diff/1/pkg/analyzer/test/generated...
pkg/analyzer/test/generated/parser_fasta_test.dart:817: // ability to Fasta's
parser.
On 2017/02/20 09:59:39, ahe wrote:
> I think that should be easy. Let me know how soon you need this.

It would be nice to get this within the next couple of weeks.  I have some code
in pkg/front_end/lib/src/dependency_grapher_impl.dart that I'd like to switch
over to using Fasta, and it would likely take a performance hit without this
feature.

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

https://codereview.chromium.org/2707663002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/analyzer/ast_builder.dart:59: var
accumulateIdentifierComponents = false;
On 2017/02/20 09:59:40, ahe wrote:
> Should we change the parser so it can tell you how many components there are
on
> the stack instead?

I don't know.  In general I like the "handleQualified" approach--I believe it's
a good match for analyzer data structures in most cases.  It's just the two
cases in this CL (library and part-of directives) that are a little weird.  So
my temptation would be to leave it as is.

https://codereview.chromium.org/2707663002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/analyzer/ast_builder.dart:927: // in a reference to
the "of" keyword.
On 2017/02/20 09:59:40, ahe wrote:
> We can do so, just file a bug. 

Acknowledged.

https://codereview.chromium.org/2707663002/diff/1/pkg/front_end/lib/src/fasta...
File pkg/front_end/lib/src/fasta/parser/listener.dart (right):

https://codereview.chromium.org/2707663002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/parser/listener.dart:480: /// - Library name (a
qualified identifier)
On 2017/02/20 09:59:40, ahe wrote:
> There can also be metadata here.

Whoops, thank you.  Will fix in a follow-up CL.