Adding some asset-related utilities to code_transformers

Adding some asset-related utilities to code_transformers

Including dart entry point checks and URI->asset resolution.

BUG=
R=sigmund@google.com

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

[Siggi Cherem (dart-lang), 2014-03-18 16:42:15]

we should probably remove `polymer/lib/src/build/common.dart#resolve` and update
the code in polymer too, but I'm OK doing that on a second pass later. Up to
you.

https://codereview.chromium.org/196943024/diff/1/pkg/code_transformers/lib/sr...
File pkg/code_transformers/lib/src/assets.dart (right):

https://codereview.chromium.org/196943024/diff/1/pkg/code_transformers/lib/sr...
pkg/code_transformers/lib/src/assets.dart:22: AssetId uriToAssetId(AssetId
source, String url, TransformLogger logger,
let's split this function out to a separate public library in this package, that
way the dependency on the analyzer is only there if you need it.

https://codereview.chromium.org/196943024/diff/1/pkg/code_transformers/lib/sr...
pkg/code_transformers/lib/src/assets.dart:125: bool
_isEntrypoint(CompilationUnit compilationUnit) {
nit: rename to _couldBeEntryPoint or _mayBeEntryPoint

https://codereview.chromium.org/196943024/diff/1/pkg/code_transformers/lib/sr...
pkg/code_transformers/lib/src/assets.dart:129: return node is
FunctionDeclaration && node.name.name == "main" &&
Use =>? (maybe moving the comment outside)?

// Allow two or fewer arguments so that entrypoints intended for use with
// [spawnUri] get counted.
bool hasMain = unit.declarations.any((node) =>
    node is FunctionDeclaration &&
    node.name.name == "main" &&
    node.functionExpression.parameters.parameters.length <= 2);

https://codereview.chromium.org/196943024/diff/1/pkg/code_transformers/lib/sr...
pkg/code_transformers/lib/src/assets.dart:131: }) ||
compilationUnit.directives.any((node) {
the mix of multi-line closures with || is not that easy to digest, it might be
worth adding a couple local vars to make this more readable, or split it in to
`if (...) return true` to keep the short-circuit logic.

https://codereview.chromium.org/196943024/diff/1/pkg/code_transformers/lib/sr...
pkg/code_transformers/lib/src/assets.dart:132: return node is ExportDirective ||
node is PartDirective;
use => ?, for example:

bool hasPartOrExport = unit.directives.any(
    (d) => d is ExportDirective || d is PartDirective);

https://codereview.chromium.org/196943024/diff/1/pkg/code_transformers/test/a...
File pkg/code_transformers/test/assets_test.dart (right):

https://codereview.chromium.org/196943024/diff/1/pkg/code_transformers/test/a...
pkg/code_transformers/test/assets_test.dart:110: result: new AssetId('foo',
'lib/foo.dart'));
seems that we didn't have enough unittests in polymer for it (we have some
end-to-end tests in the polymer/example/canonicalization* packages)

Here are some additional tests that might be useful to include:
 - it is valid to use package: inside lib/.../foo.dart
 - it is invalid to use 'packages/foo/bar' in lib/foo.html
 - but it is valid to use '../../packages/foo/bar' in lib/foo.html
 - error message when assetId is .html, but uri uses "package:" or starts with
"/" (and !allowAbsolute)
 - null result when allowAbsolute is true (normally this is allowed in cases
like hrefs to external resources like .css)

[blois, 2014-03-18 17:39:07]

https://codereview.chromium.org/196943024/diff/1/pkg/code_transformers/lib/sr...
File pkg/code_transformers/lib/src/assets.dart (right):

https://codereview.chromium.org/196943024/diff/1/pkg/code_transformers/lib/sr...
pkg/code_transformers/lib/src/assets.dart:22: AssetId uriToAssetId(AssetId
source, String url, TransformLogger logger,
On 2014/03/18 16:42:16, Siggi Cherem (dart-lang) wrote:
> let's split this function out to a separate public library in this package,
that
> way the dependency on the analyzer is only there if you need it.

Done.

https://codereview.chromium.org/196943024/diff/1/pkg/code_transformers/lib/sr...
pkg/code_transformers/lib/src/assets.dart:125: bool
_isEntrypoint(CompilationUnit compilationUnit) {
On 2014/03/18 16:42:16, Siggi Cherem (dart-lang) wrote:
> nit: rename to _couldBeEntryPoint or _mayBeEntryPoint

Done.

https://codereview.chromium.org/196943024/diff/1/pkg/code_transformers/lib/sr...
pkg/code_transformers/lib/src/assets.dart:129: return node is
FunctionDeclaration && node.name.name == "main" &&
On 2014/03/18 16:42:16, Siggi Cherem (dart-lang) wrote:
> Use =>? (maybe moving the comment outside)?
> 
> // Allow two or fewer arguments so that entrypoints intended for use with
> // [spawnUri] get counted.
> bool hasMain = unit.declarations.any((node) =>
>     node is FunctionDeclaration &&
>     node.name.name == "main" &&
>     node.functionExpression.parameters.parameters.length <= 2);

Done.

https://codereview.chromium.org/196943024/diff/1/pkg/code_transformers/lib/sr...
pkg/code_transformers/lib/src/assets.dart:131: }) ||
compilationUnit.directives.any((node) {
On 2014/03/18 16:42:16, Siggi Cherem (dart-lang) wrote:
> the mix of multi-line closures with || is not that easy to digest, it might be
> worth adding a couple local vars to make this more readable, or split it in to
> `if (...) return true` to keep the short-circuit logic.

Done.

https://codereview.chromium.org/196943024/diff/1/pkg/code_transformers/lib/sr...
pkg/code_transformers/lib/src/assets.dart:132: return node is ExportDirective ||
node is PartDirective;
On 2014/03/18 16:42:16, Siggi Cherem (dart-lang) wrote:
> use => ?, for example:
> 
> bool hasPartOrExport = unit.directives.any(
>     (d) => d is ExportDirective || d is PartDirective);

Done.

https://codereview.chromium.org/196943024/diff/1/pkg/code_transformers/test/a...
File pkg/code_transformers/test/assets_test.dart (right):

https://codereview.chromium.org/196943024/diff/1/pkg/code_transformers/test/a...
pkg/code_transformers/test/assets_test.dart:110: result: new AssetId('foo',
'lib/foo.dart'));
On 2014/03/18 16:42:16, Siggi Cherem (dart-lang) wrote:
> seems that we didn't have enough unittests in polymer for it (we have some
> end-to-end tests in the polymer/example/canonicalization* packages)
> 
> Here are some additional tests that might be useful to include:
>  - it is valid to use package: inside lib/.../foo.dart
>  - it is invalid to use 'packages/foo/bar' in lib/foo.html
>  - but it is valid to use '../../packages/foo/bar' in lib/foo.html
>  - error message when assetId is .html, but uri uses "package:" or starts with
> "/" (and !allowAbsolute)
>  - null result when allowAbsolute is true (normally this is allowed in cases
> like hrefs to external resources like .css)

Done.

[Siggi Cherem (dart-lang), 2014-03-18 19:06:25]

https://codereview.chromium.org/196943024/diff/20001/pkg/code_transformers/li...
File pkg/code_transformers/lib/assets.dart (right):

https://codereview.chromium.org/196943024/diff/20001/pkg/code_transformers/li...
pkg/code_transformers/lib/assets.dart:8: export 'src/assets.dart';
why not just put the code here?

https://codereview.chromium.org/196943024/diff/20001/pkg/polymer/lib/src/mirr...
File pkg/polymer/lib/src/mirror_loader.dart (right):

https://codereview.chromium.org/196943024/diff/20001/pkg/polymer/lib/src/mirr...
pkg/polymer/lib/src/mirror_loader.dart:16: import
'package:code_transformers/assets.dart';
is this one used?

https://codereview.chromium.org/196943024/diff/20001/pkg/polymer/pubspec.yaml
File pkg/polymer/pubspec.yaml (right):

https://codereview.chromium.org/196943024/diff/20001/pkg/polymer/pubspec.yaml...
pkg/polymer/pubspec.yaml:14: code_transformers: 0.0.1-dev.3
 shouldn't we use ">=0.0.1-dev.3 <0.1.0"? (I was about to do a similar change
with analzyer above.)

[blois, 2014-03-18 19:41:24]

https://codereview.chromium.org/196943024/diff/20001/pkg/code_transformers/li...
File pkg/code_transformers/lib/assets.dart (right):

https://codereview.chromium.org/196943024/diff/20001/pkg/code_transformers/li...
pkg/code_transformers/lib/assets.dart:8: export 'src/assets.dart';
On 2014/03/18 19:06:25, Siggi Cherem (dart-lang) wrote:
> why not just put the code here?

Done. No specific reason- was just following a pattern of implementation in src/
then explicit exports of what should be visible. But is a bit more convoluted
than necessary.

https://codereview.chromium.org/196943024/diff/20001/pkg/polymer/lib/src/mirr...
File pkg/polymer/lib/src/mirror_loader.dart (right):

https://codereview.chromium.org/196943024/diff/20001/pkg/polymer/lib/src/mirr...
pkg/polymer/lib/src/mirror_loader.dart:16: import
'package:code_transformers/assets.dart';
On 2014/03/18 19:06:25, Siggi Cherem (dart-lang) wrote:
> is this one used?

Ack- had a resolve that was popping up in my search, but the wrong resolve.

https://codereview.chromium.org/196943024/diff/20001/pkg/polymer/pubspec.yaml
File pkg/polymer/pubspec.yaml (right):

https://codereview.chromium.org/196943024/diff/20001/pkg/polymer/pubspec.yaml...
pkg/polymer/pubspec.yaml:14: code_transformers: 0.0.1-dev.3
On 2014/03/18 19:06:25, Siggi Cherem (dart-lang) wrote:
>  shouldn't we use ">=0.0.1-dev.3 <0.1.0"? (I was about to do a similar change
> with analzyer above.)

Done.

[Siggi Cherem (dart-lang), 2014-03-18 19:43:46]

lgtm!

[blois, 2014-03-18 19:55:15]

Committed patchset #3 manually as r34055 (presubmit successful).