Experimental analyzer front-end baby-steps.

pkg/analyzer-experimental/lib/options.dart

Index: pkg/analyzer-experimental/lib/options.dart
===================================================================
--- pkg/analyzer-experimental/lib/options.dart	(revision 0)
+++ pkg/analyzer-experimental/lib/options.dart	(revision 0)
@@ -0,0 +1,196 @@
+// 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.
+
+library options;
+
+import 'package:args/args.dart';
+
+import 'dart:io';
+
+/**
+ * Analyzer commandline configuration options.
+ */
+class CommandLineOptions {
+
+  static const _BINARY_NAME = 'analyzer';
+
+  static const _SDK_ENV = 'com.google.dart.sdk';
+
+  static final String _DEFAULT_SDK_LOCATION = Platform.environment[_SDK_ENV];

[Bob Nystrom, 2013/01/18 22:25:22]

It's OK either way, but I tend to hoist private constants out of the class and
into the top level. Maybe I just don't like writing "static const" and "static
final". :)

[pquitslund, 2013/01/22 18:31:53]

Interesting... I'm still feeling my way around this top-level namespace, 
Anyway, advice taken!

+
+  /** Batch mode (for unit testing) */
+  final bool shouldBatch;
+
+  /** Whether to use machine format for error display */
+  final bool machineFormat;
+
+  /** Whether to ignore unrecognized flags */
+  final bool ignoreUnrecognizedFlags;
+
+  /** Whether to print metrics */
+  final bool showMetrics;
+
+  /** Whether to treat warnings as fatal */
+  final bool warningsAreFatal;
+
+  /** The path to the dart SDK */
+  final String dartSdkPath;
+
+  /** The source files to analyze */
+  final List<String> sourceFiles;
+
+  /**
+   * Initialize options from the given parsed [args].
+   */
+  CommandLineOptions.fromArgs(ArgResults args)
+    : shouldBatch = args['batch'],
+      machineFormat = args['machine_format'],
+      ignoreUnrecognizedFlags = args['ignore_unrecognized_flags'],
+      showMetrics = args['metrics'],
+      warningsAreFatal = args['fatal_warnings'],
+      dartSdkPath = args['dart_sdk'],
+      sourceFiles = args.rest;
+
+  /**
+   * Parse [args] into [CommandLineOptions] describing the specified
+   * analyzer options.  In case of a format error, [null] is returned.
+   */
+  static CommandLineOptions parse(List<String> args) {

[Bob Nystrom, 2013/01/18 22:25:22]

Make this a factory constructor and make .fromArgs() private? Then you could do:

new CommandLineOptions.parse(args);

[pquitslund, 2013/01/22 18:31:53]

Nice.  Thanks.

+
+    var parser = new _CommandLineParser()
+      ..addFlag('batch', abbr: 'b', help: 'Run in batch mode',
+          defaultsTo: false, negatable: false)
+      ..addOption('dart_sdk', help: 'Specify path to the Dart sdk',
+          defaultsTo: _DEFAULT_SDK_LOCATION)
+      ..addFlag('machine_format', help: 'Specify whether errors '
+        'should be in machine format',
+          defaultsTo: false, negatable: false)
+      ..addFlag('ignore_unrecognized_flags',
+          help: 'Ignore unrecognized command line flags',
+          defaultsTo: false, negatable: false)
+      ..addFlag('fatal_warnings', help: 'Treat non-type warnings as fatal',
+          defaultsTo: false, negatable: false)
+       ..addFlag('metrics', help: 'Print metrics',
+          defaultsTo: false, negatable: false)
+      ..addFlag('help', abbr: 'h', help: 'Display this help message',
+          defaultsTo: false, negatable: false);
+
+    try {
+      var results = parser.parse(args);
+      if (results['help'] || results.rest.length == 0) {
+        _showUsage(parser);
+        return null;
+      }
+      return new CommandLineOptions.fromArgs(results);
+    } on FormatException catch (e) {
+      print(e.message);
+      _showUsage(parser);
+      return null;
+    }
+
+  }
+
+  static _showUsage(parser) {
+    print('Usage: ${_BINARY_NAME} [options...] '
+      '<libraries to analyze...>');
+    print(parser.getUsage());
+  }
+
+}
+
+/**
+ * Commandline argument parser.
+ *
+ * TODO(pquitslund): when the args package supports ignoring unrecognized
+ * options/flags, this class can be replaced with a simple [ArgParser] instance.

[Bob Nystrom, 2013/01/18 22:25:22]

Mention #5262 here.

[pquitslund, 2013/01/22 18:31:53]

On the surface, that doesn't seem to capture this specific issue.  Or?  If you
agree, I'll open another ticket and point this to it.

[Bob Nystrom, 2013/01/22 19:16:36]

I guess technically that bug is a little different, but ignoring unrecognized
options is somewhat related to it.

+ */
+class _CommandLineParser {
+
+  final List<String> _knownFlags;
+  final ArgParser _parser;

[Bob Nystrom, 2013/01/18 22:25:22]

Is there a reason you're forwarding instead of inheriting and overriding?

[pquitslund, 2013/01/22 18:31:53]

Habit.  I generally avoid subclassing where it doesn't have clear benefits. 
Blame my caution on the indoctrination of the Go4!

[Bob Nystrom, 2013/01/22 19:16:36]

Well, they said *prefer* composition, not "never ever inherit". :)

+
+  /** Creates a new command line parser */
+  _CommandLineParser()
+    : _knownFlags = <String>[],
+      _parser = new ArgParser();
+
+
+  /**
+   * Defines a flag.
+   *
+   * See [ArgParser.addFlag()].
+   */
+  void addFlag(String name, {String abbr, String help, bool defaultsTo: false,
+      bool negatable: true, void callback(bool value)}) {
+    _knownFlags.add(name);
+    _parser.addFlag(name, abbr:abbr, help:help, defaultsTo:defaultsTo,

[Bob Nystrom, 2013/01/18 22:25:22]

Space after ":" here and elsewhere.

[pquitslund, 2013/01/22 18:31:53]

My kingdom for a formatter! :)

+        negatable:negatable, callback:callback);
+  }
+
+  /**
+   * Defines a value-taking option.
+   *
+   * See [ArgParser.addOption()].
+   */
+  void addOption(String name, {String abbr, String help, List<String> allowed,
+      Map<String, String> allowedHelp, String defaultsTo,
+      void callback(value), bool allowMultiple: false}) {
+    _parser.addOption(name, abbr:abbr, help:help, allowed:allowed,
+        allowedHelp:allowedHelp, defaultsTo:defaultsTo, callback:callback,
+        allowMultiple:allowMultiple);
+  }
+
+
+  /**
+   * Generates a string displaying usage information for the defined options.
+   *
+   * See [ArgParser.getUsage()].
+   */
+  String getUsage() => _parser.getUsage();
+
+  /**
+   * Parses [args], a list of command-line arguments, matches them against the
+   * flags and options defined by this parser, and returns the result.
+   *
+   * See [ArgParser].
+   */
+  ArgResults parse(List<String> args) => _parser.parse(_filterUnknowns(args));
+
+  List<String> _filterUnknowns(args) {
+
+    //Only filter if the ignore flag is specified

[Bob Nystrom, 2013/01/18 22:25:22]

Space after "//" and make a sentence.

[pquitslund, 2013/01/22 18:31:53]

Done.

+    if (!args.contains('--ignore_unrecognized_flags')) {
+      return args;
+    }
+
+    //Filter all unrecognized flags and options

[Bob Nystrom, 2013/01/18 22:25:22]

Ditto.

[pquitslund, 2013/01/22 18:31:53]

Done.

+    var filtered = <String>[];

[Bob Nystrom, 2013/01/18 22:25:22]

return args.where((arg) => !arg.startsWith('--') ||
                           _knownFlags.contains(arg.substring(2));

:)

[pquitslund, 2013/01/22 18:31:53]

Cheeky! :)  But I think this is missing the parameter eating part.  That is, I
think this should work for options as well.  Or?  IOW, seeing this

 --ignore me --but not any of me 

should yield ['--but', 'not', 'any', 'of', 'me'] assuming 'but' is an understood
option.

What do you think?

[Bob Nystrom, 2013/01/22 19:16:36]

I believe this should do the right thing here. If it's not an option, (i.e.
doesn't start with "--") it will always get included in the result since
!args.startsWith('--') will be true.

If it is an option, it will only be included in the result set if it's a known
one.

[pquitslund, 2013/01/22 23:30:01]

Well fine.  Are you ALWAYS right? ;)

Follow-up here: https://codereview.chromium.org/12049028/

+    for (var i=0; i < args.length; ++i) {
+      var arg = args[i];
+      if (arg.startsWith('--') && arg.length > 2) {
+        if (!_knownFlags.contains(arg.substring(2))) {
+          //"eat" params by advancing to the next flag/option
+          i = _getNextFlagIndex(args, i);
+        } else {
+          filtered.add(arg);
+        }
+      } else {
+        filtered.add(arg);
+      }
+    }
+
+    return filtered;
+  }
+
+  _getNextFlagIndex(args, i) {
+    for ( ; i < args.length; ++i) {
+      if (args[i].startsWith('--')) {
+        return i;
+      }
+    }
+    return i;
+  }
+
+}
+