fixes #201, add support for custom URL mapping

fixes #201, add support for custom URL mapping

R=vsm@google.com

Committed: https://github.com/dart-lang/dev_compiler/commit/660b8cc0fc11057eeede171d25cb6b8b95ba4f6a

[Jennifer Messerly, 2015-06-02 21:37:26]

jmesserly@google.com changed reviewers:
+ vsm@google.com

[Jennifer Messerly, 2015-06-02 21:37:27]

https://codereview.chromium.org/1150143006/diff/1/pubspec.yaml
File pubspec.yaml (right):

https://codereview.chromium.org/1150143006/diff/1/pubspec.yaml#newcode10
pubspec.yaml:10: args: ^0.13.0
0.13 changed how comma split works

[vsm, 2015-06-03 15:37:47]

lgtm

https://codereview.chromium.org/1150143006/diff/1/lib/src/options.dart
File lib/src/options.dart (left):

https://codereview.chromium.org/1150143006/diff/1/lib/src/options.dart#oldcode21
lib/src/options.dart:21: final bool useMultiPackage;
Seems a little weird to move these out of ResolverOptions - each is directly
related to resolution.

Perhaps we don't even need a separate ResolverOptions class at this point?

https://codereview.chromium.org/1150143006/diff/1/test/checker/checker_test.dart
File test/checker/checker_test.dart (right):

https://codereview.chromium.org/1150143006/diff/1/test/checker/checker_test.d...
test/checker/checker_test.dart:2943: customUrlMappings: {
indent seems a little weird here.

[Jennifer Messerly, 2015-06-03 16:24:11]

so,  it sounds like options is more contentious than I was expecting. I'll file
a github issue and see if I can roll back some of that file, and I guess send it
out again ...

https://codereview.chromium.org/1150143006/diff/1/lib/src/options.dart
File lib/src/options.dart (left):

https://codereview.chromium.org/1150143006/diff/1/lib/src/options.dart#oldcode21
lib/src/options.dart:21: final bool useMultiPackage;
On 2015/06/03 15:37:47, vsm wrote:
> Seems a little weird to move these out of ResolverOptions - each is directly
> related to resolution.

Hmmm. What is "resolution" in this context? Analyzer uses it for at least two
meanings: URI resolution and name resolution. I thought the etymology of this
ResolutionOptions was "options used by our TypeResolver class", which is gone
now.

After my change, it was supposed to mean: "options used by our
RestrictedResolverVisitor". (lib/src/checker/resolver.dart)

IMO, there's not really a relationship between URI resolution stuff (package
roots, etc) and type inference options... both are in this class now. They are
consumed in very different places in our code base. The URI resolution stuff
won't be needed when we use analyzer_cli, for example. (... and I just noticed
entryPointFile and useImplicitHtml should be moved out too)

I'd be happy to make a seperate "URI resolution" options class, if that helps?

I also noticed Checker consumes CompilerOptions right now. Maybe worth having a
CheckerOptions class?


Hah, and here I thought I would just fix a little TODO while I was here.
Apparently options is quite a rabbit hole :-)

> 
> Perhaps we don't even need a separate ResolverOptions class at this point?

True, we certainly don't need it. I don't recall exactly how we arrived here,
but it did seem useful to be able to create a RestrictedResolverVisitor and have
it only depend on its own options. This might help, e.g. with analyzer_cli
integration. I guess I should restore the constructor though, in that case, and
also make sure checker's options are split out from CompilerOptions.

https://codereview.chromium.org/1150143006/diff/1/test/checker/checker_test.dart
File test/checker/checker_test.dart (right):

https://codereview.chromium.org/1150143006/diff/1/test/checker/checker_test.d...
test/checker/checker_test.dart:2943: customUrlMappings: {
On 2015/06/03 15:37:47, vsm wrote:
> indent seems a little weird here.

that's what dart_style does. I think Bob is on it already, judging by the many
discussions lately.

[Jennifer Messerly, 2015-06-03 16:31:48]

options issue. https://github.com/dart-lang/dev_compiler/issues/204

still working on rolling back some of those changes (unfortunately can't roll
back the whole file)

[Jennifer Messerly, 2015-06-03 16:39:03]

rolled back ResolverOptions change, so we can continue thinking about it in the
issue

[Jennifer Messerly, 2015-06-03 16:40:55]

Committed patchset #2 (id:20001) manually as
660b8cc0fc11057eeede171d25cb6b8b95ba4f6a (presubmit successful).