Formatter improvements (comments and empty bodies).

Formatter improvements (comments and empty bodies).

With these fixes we can now safely format the analyzer source itself (w/ sanity checking). Next stop the SDK! :)

R=brianwilkerson@google.com

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

[Brian Wilkerson, 2013-12-04 19:52:49]

LGTM

https://codereview.chromium.org/104993003/diff/20001/pkg/analyzer/lib/src/ser...
File pkg/analyzer/lib/src/services/formatter_impl.dart (right):

https://codereview.chromium.org/104993003/diff/20001/pkg/analyzer/lib/src/ser...
pkg/analyzer/lib/src/services/formatter_impl.dart:1439: currentToken.previous !=
null ? currentToken.previous : token.previous;
Unless you're creating your own tokens, "currentToken.previous" should never be
null (the beginning and end of the token stream is a special EOF token that
loops around to itself). Was this just defensive programming, or did you
actually see this return null at some point?

https://codereview.chromium.org/104993003/diff/20001/pkg/analyzer/test/servic...
File pkg/analyzer/test/services/formatter_test.dart (right):

https://codereview.chromium.org/104993003/diff/20001/pkg/analyzer/test/servic...
pkg/analyzer/test/services/formatter_test.dart:559: test('CU - comments (10)',
() {
If there isn't already, it might be good to add a test for a list with a
trailing separator, such as
  var l = [1, 2, ];

[pquitslund, 2013-12-04 21:09:10]

Thanks!

https://codereview.chromium.org/104993003/diff/20001/pkg/analyzer/lib/src/ser...
File pkg/analyzer/lib/src/services/formatter_impl.dart (right):

https://codereview.chromium.org/104993003/diff/20001/pkg/analyzer/lib/src/ser...
pkg/analyzer/lib/src/services/formatter_impl.dart:1439: currentToken.previous !=
null ? currentToken.previous : token.previous;
On 2013/12/04 19:52:50, Brian Wilkerson wrote:
> Unless you're creating your own tokens, "currentToken.previous" should never
be
> null (the beginning and end of the token stream is a special EOF token that
> loops around to itself). Was this just defensive programming, or did you
> actually see this return null at some point?

I believe I added this when I was creating token streams for testing.  I think I
didn't know about the looping around bit...  I'll do some investigating and see
if I can make this go away.

https://codereview.chromium.org/104993003/diff/20001/pkg/analyzer/test/servic...
File pkg/analyzer/test/services/formatter_test.dart (right):

https://codereview.chromium.org/104993003/diff/20001/pkg/analyzer/test/servic...
pkg/analyzer/test/services/formatter_test.dart:559: test('CU - comments (10)',
() {
On 2013/12/04 19:52:50, Brian Wilkerson wrote:
> If there isn't already, it might be good to add a test for a list with a
> trailing separator, such as
>   var l = [1, 2, ];

Good call.  I've got one of those in my list tests.  Thanks!

[pquitslund, 2013-12-04 21:12:56]

Committed patchset #2 manually as r30869 (presubmit successful).