pkg/analyzer: fix formatter bin crash due to empty selection flag

pkg/analyzer: fix formatter bin crash due to empty selection flag

R=pquitslund@google.com

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

[kevmoo, 2014-02-09 21:46:33]

We might want at test that actually runs the binary to make sure args are parsed
and we don't hit these kinds of crashes

[pquitslund, 2014-02-10 04:01:50]

Whoops!  Nice catch.  This was a hasty fix I added on Friday to address a
warning Brian caught.  Obviously too hasty.  And good point about testing the
binary!

Now it should come as no surprise that I take issue with some of your other
proposed changes.  I'm still feeling my way around optional types I admit but in
these cases I don't really understand the benefit of declaring them.

Popping up, I guess I would prefer a separate CL with orthogonal changes (e.g.,
type style).  That bit deserves much more back and forth and I think we should
do it.  That said, we shouldn't hold up landing the fix in the meantime.

https://codereview.chromium.org/152863006/diff/40001/pkg/analyzer/bin/formatt...
File pkg/analyzer/bin/formatter.dart (right):

https://codereview.chromium.org/152863006/diff/40001/pkg/analyzer/bin/formatt...
pkg/analyzer/bin/formatter.dart:42: main(List<String> args) {
Is this ever NOT a List of Strings?  What's the benefit of declaring it?

https://codereview.chromium.org/152863006/diff/40001/pkg/analyzer/bin/formatt...
pkg/analyzer/bin/formatter.dart:59: _readOptions(ArgResults options) {
Really?  It's like you're provoking me... ;)  Surely you don't really think all
private methods should be typed?  I thought the contract was only with public
APIs?

https://codereview.chromium.org/152863006/diff/40001/pkg/analyzer/bin/formatt...
pkg/analyzer/bin/formatter.dart:69: CodeKind _parseKind(String kindOption) {
OK, and especially here.  Isn't it obvious from the switch that this is a
String?  What's the benefit of the explicit type?

[kevmoo, 2014-02-10 04:24:46]

Oh I forgot.

Honestly, I'm just following our internal code conventions.

All dart code in the SDK annotates every argument and return type except
dynamic.

...honestly, the omission of dynamic bugs me, too, but I breath by it. :-)

I'll up date the CL to just do the fix.

[kevmoo, 2014-02-10 06:01:23]

PTAL

[pquitslund, 2014-02-10 18:31:41]

LGTM!

Thanks Kevin.  I look forward to more lively debate on type annotations. :)

[kevmoo, 2014-02-10 18:34:01]

Committed patchset #5 manually as r32509 (presubmit successful).