Rename xFunctionBody to xBlockFunctionBody. Document related listener methods.

Rename xFunctionBody to xBlockFunctionBody. Document related listener methods.

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

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

[Paul Berry, 2017-03-13 19:58:38]

lgtm but we probably should wait for Peter's review.

[ahe, 2017-03-14 15:15:18]

lgtm

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

https://codereview.chromium.org/2747853002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/parser/listener.dart:248: void
beginBlockFunctionBody(Token token) {}
If you're willing to do it now, it may make sense to clean up these various
names. Personally, I think it would be nice that if the events were:

{begin,end}FunctionBodyBlock
endFunctionBodyEmpty
endFunctionBodyExpression

This way, when you search for, for example, beginFunctionBody, you'd find all
these.

Another thing is that since endFunctionBodyEmpty and endFunctionBodyExpression
doesn't have a begin event, they should really be called handleSomething instead
of endSomething.

https://codereview.chromium.org/2747853002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/parser/listener.dart:260: void
handleNoFunctionBody(Token token) {
Should this be the same as endEmptyFunctionBody?

[scheglov, 2017-03-14 17:45:56]

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

https://codereview.chromium.org/2747853002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/parser/listener.dart:248: void
beginBlockFunctionBody(Token token) {}
On 2017/03/14 15:15:17, ahe wrote:
> If you're willing to do it now, it may make sense to clean up these various
> names. Personally, I think it would be nice that if the events were:
> 
> {begin,end}FunctionBodyBlock
> endFunctionBodyEmpty
> endFunctionBodyExpression
> 
> This way, when you search for, for example, beginFunctionBody, you'd find all
> these.
> 
> Another thing is that since endFunctionBodyEmpty and endFunctionBodyExpression
> doesn't have a begin event, they should really be called handleSomething
instead
> of endSomething.

Ah, right.
These should be handleX.
Done.

Even better would be to name them functionBodyBlockBegin, functionBodyBlockEnd,
functionBodyExpressionHandle, etc.

And of course sort all the files :-)

https://codereview.chromium.org/2747853002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/parser/listener.dart:260: void
handleNoFunctionBody(Token token) {
On 2017/03/14 15:15:17, ahe wrote:
> Should this be the same as endEmptyFunctionBody?

How can I know?
You wrote this code.
I see that it is used only in skipFunctionBody(), but it also does not state its
intentions.

[scheglov, 2017-03-14 17:47:49]

Description was changed from

==========
Rename xFunctionBody to xBlockFunctionBody. Document related listener methods.

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

to

==========
Rename xFunctionBody to xBlockFunctionBody. Document related listener methods.

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

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

[scheglov, 2017-03-14 17:47:53]

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

[ahe, 2017-03-15 08:28:35]

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

https://codereview.chromium.org/2747853002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/parser/listener.dart:248: void
beginBlockFunctionBody(Token token) {}
On 2017/03/14 17:45:56, scheglov wrote:
> Even better would be to name them functionBodyBlockBegin,
functionBodyBlockEnd,
> functionBodyExpressionHandle, etc.
> 
> And of course sort all the files :-)

Yeah. I've tried to keep the names sorted by what follows the {begin,end,handle}
prefix.

It would make more sense to come up with a naming scheme that allows us to keep
them sorted using tools. In that case, I'd tweak the names slightly, and use
"start", "end", and "done" suffixes.

Historically, the parser has been more stable, so I think would make sense to
delay such a major rename a couple of months anticipating less churn in this
file.

https://codereview.chromium.org/2747853002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/parser/listener.dart:260: void
handleNoFunctionBody(Token token) {
On 2017/03/14 17:45:56, scheglov wrote:
> On 2017/03/14 15:15:17, ahe wrote:
> > Should this be the same as endEmptyFunctionBody?
> 
> How can I know?
> You wrote this code.

I'm asking for your opinion, as you added endEmptyFunctionBody, and it only
occurred to me now that it may make sense to combine these two.

> I see that it is used only in skipFunctionBody(), but it also does not state
its
> intentions.

Based on how it's currently used, I'd like your opinion.