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

pkg/analyzer/bin/formatter.dart

 #!/usr/bin/env dart
 
 // Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 import 'dart:convert';
 import 'dart:io';
 
 import 'package:args/args.dart';
 import 'package:path/path.dart' as path;
 
 import 'package:analyzer/src/services/formatter_impl.dart';
 
 
 const BINARY_NAME = 'dartfmt';
 final dartFileRegExp = new RegExp(r'^[^.].*\.dart$', caseSensitive: false);
 final argParser = _initArgParser();
 final defaultSelection = new Selection(-1, -1);
 
-var formatterSettings;
+FormatterOptions formatterSettings;
 
 CodeKind kind;
 bool machineFormat;
 bool overwriteFileContents;
 Selection selection;
 final List<String> paths = [];
 
 
 const HELP_FLAG = 'help';
 const KIND_FLAG = 'kind';
 const MACHINE_FLAG = 'machine';
 const WRITE_FLAG = 'write';
 const SELECTION_FLAG = 'selection';
 const TRANSFORM_FLAG = 'transform';
 const MAX_LINE_FLAG = 'max_line_length';
 
 
 const FOLLOW_LINKS = false;
 
 
-main(args) {
+main(List<String> args) {

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

Is this ever NOT a List of Strings?  What's the benefit of declaring it?

   var options = argParser.parse(args);
   if (options['help']) {
     _printUsage();
     return;
   }
 
   _readOptions(options);
 
   if (options.rest.isEmpty) {
     _formatStdin(kind);
   } else {
     paths.addAll(options.rest);
     _formatPaths(paths);
   }
 }
 
-_readOptions(options) {
+_readOptions(ArgResults options) {

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

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?

   kind = _parseKind(options[KIND_FLAG]);
   machineFormat = options[MACHINE_FLAG];
   overwriteFileContents = options[WRITE_FLAG];
   selection = _parseSelection(options[SELECTION_FLAG]);
   formatterSettings =
       new FormatterOptions(codeTransforms: options[TRANSFORM_FLAG],
           pageWidth: _parseLineLength(options[MAX_LINE_FLAG]));
 }
 
-CodeKind _parseKind(kindOption) {
+CodeKind _parseKind(String kindOption) {

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

OK, and especially here.  Isn't it obvious from the switch that this is a
String?  What's the benefit of the explicit type?

   switch(kindOption) {
     case 'stmt' :
       return CodeKind.STATEMENT;
     default:
       return CodeKind.COMPILATION_UNIT;
   }
 }
 
 int _parseLineLength(String lengthOption) {
   var length = _toInt(lengthOption);
   if (length == null) {
     var val = lengthOption.toUpperCase();
     if (val == 'INF' || val == 'INFINITY') {
       length = -1;
     } else {
       throw new FormatterException('Line length is specified as an Integer or '
           'the value "Inf".');
     }
   }
   return length;
 }
 
 
-Selection _parseSelection(selectionOption) {
-  if (selectionOption != null) {
-    var units = selectionOption.split(',');
-    if (units.length == 2) {
-      var offset = _toInt(units[0]);
-      var length = _toInt(units[1]);
-      if (offset != null && length != null) {
-        return new Selection(offset, length);
-      }
+Selection _parseSelection(String selectionOption) {
+  if(selectionOption == null) return null;
+
+  var units = selectionOption.split(',');
+  if (units.length == 2) {
+    var offset = _toInt(units[0]);
+    var length = _toInt(units[1]);
+    if (offset != null && length != null) {
+      return new Selection(offset, length);
     }
   }
   throw new FormatterException('Selections are specified as integer pairs '
                                '(e.g., "(offset, length)".');
 }
 
 int _toInt(str) => int.parse(str, onError: (_) => null);
 
 _formatPaths(paths) {
   paths.forEach((path) {
@@ skipping 33 matching lines @@
         }
       } else {
         print(formatted);
       }
     } catch (e) {
       _log('Unable to format "${file.path}": $e');
     }
   }
 }
 
-_isPatchFile(file) => file.path.endsWith('_patch.dart');
+bool _isPatchFile(file) => file.path.endsWith('_patch.dart');
 
-_isDartFile(file) => dartFileRegExp.hasMatch(path.basename(file.path));
+bool _isDartFile(file) => dartFileRegExp.hasMatch(path.basename(file.path));
 
 _formatStdin(kind) {
   var input = new StringBuffer();
   stdin.transform(new Utf8Decoder())
       .listen((data) => input.write(data),
         onError: (error) => _log('Error reading from stdin'),
         onDone: () => print(_format(input.toString(), kind)));
 }
 
 /// Initialize the arg parser instance.
@@ skipping 46 matching lines @@
       kind, src, selection: selection);
   if (machineFormat) {
     if (formatResult.selection == null) {
       formatResult.selection = defaultSelection;
     }
     return _toJson(formatResult);
   }
   return formatResult.source;
 }
 
-_toJson(formatResult) =>
+String _toJson(formatResult) =>
     // Actual JSON format TBD
     JSON.encode({'source': formatResult.source,
                  'selection': {
                      'offset': formatResult.selection.offset,
                      'length': formatResult.selection.length
                   }
     });
 
 /// Log the given [msg].
 _log(String msg) {
   //TODO(pquitslund): add proper log support
   print(msg);
 }