Formatter sanity-checking (via token stream verification).

Formatter sanity-checking (via token stream verification).

R=brianwilkerson@google.com

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

[Brian Wilkerson, 2013-09-09 22:21:47]

LGTM

https://codereview.chromium.org/23480055/diff/1005/pkg/analyzer_experimental/...
File pkg/analyzer_experimental/lib/src/services/formatter_impl.dart (right):

https://codereview.chromium.org/23480055/diff/1005/pkg/analyzer_experimental/...
pkg/analyzer_experimental/lib/src/services/formatter_impl.dart:108:
checkTokenStreams(start, tokenize(source));
It looks like you're testing to see whether the scanner is deterministic (start
is the same as tokenize(source), so the left and right should always be the
same). I think you want to tokenize the return value before returning it.

https://codereview.chromium.org/23480055/diff/1005/pkg/analyzer_experimental/...
pkg/analyzer_experimental/lib/src/services/formatter_impl.dart:259: 
The parser uses a similar method, but passes the expected token type in as a
parameter:

  matches(Token token, TokenType type)

(there's less duplication that way).

https://codereview.chromium.org/23480055/diff/1005/pkg/analyzer_experimental/...
File pkg/analyzer_experimental/test/services/formatter_test.dart (right):

https://codereview.chromium.org/23480055/diff/1005/pkg/analyzer_experimental/...
pkg/analyzer_experimental/test/services/formatter_test.dart:757:
expectTokenizedEqual('class A{}', 'class A{ }');
In all of the equal cases the strings are identical. It would be good to test
with strings that have whitespace differences.

https://codereview.chromium.org/23480055/diff/1005/pkg/analyzer_experimental/...
pkg/analyzer_experimental/test/services/formatter_test.dart:910: Token
string(String lexeme) => new StringToken(TokenType.STRING, lexeme, 0);
There is a TokenFactory class in the engine tests that might be useful.

[pquitslund, 2013-09-10 17:11:02]

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

[pquitslund, 2013-09-10 18:24:13]

Thank you!

https://codereview.chromium.org/23480055/diff/1005/pkg/analyzer_experimental/...
File pkg/analyzer_experimental/lib/src/services/formatter_impl.dart (right):

https://codereview.chromium.org/23480055/diff/1005/pkg/analyzer_experimental/...
pkg/analyzer_experimental/lib/src/services/formatter_impl.dart:108:
checkTokenStreams(start, tokenize(source));
On 2013/09/09 22:21:47, Brian Wilkerson wrote:
> It looks like you're testing to see whether the scanner is deterministic
(start
> is the same as tokenize(source), so the left and right should always be the
> same). I think you want to tokenize the return value before returning it.

Hilarious.  Looks like I did one undo too many!  Thanks for the catch :)

https://codereview.chromium.org/23480055/diff/1005/pkg/analyzer_experimental/...
pkg/analyzer_experimental/lib/src/services/formatter_impl.dart:259: 
On 2013/09/09 22:21:47, Brian Wilkerson wrote:
> The parser uses a similar method, but passes the expected token type in as a
> parameter:
> 
>   matches(Token token, TokenType type)
> 
> (there's less duplication that way).

Done.

https://codereview.chromium.org/23480055/diff/1005/pkg/analyzer_experimental/...
File pkg/analyzer_experimental/test/services/formatter_test.dart (right):

https://codereview.chromium.org/23480055/diff/1005/pkg/analyzer_experimental/...
pkg/analyzer_experimental/test/services/formatter_test.dart:757:
expectTokenizedEqual('class A{}', 'class A{ }');
On 2013/09/09 22:21:47, Brian Wilkerson wrote:
> In all of the equal cases the strings are identical. It would be good to test
> with strings that have whitespace differences.

Added some.  Thanks!

https://codereview.chromium.org/23480055/diff/1005/pkg/analyzer_experimental/...
pkg/analyzer_experimental/test/services/formatter_test.dart:910: Token
string(String lexeme) => new StringToken(TokenType.STRING, lexeme, 0);
On 2013/09/09 22:21:47, Brian Wilkerson wrote:
> There is a TokenFactory class in the engine tests that might be useful.

I'll look.  Thanks!