Add BazelPackageUriResolver with resolveAbsolute().

Add BazelPackageUriResolver with resolveAbsolute().

R=brianwilkerson@google.com, paulberry@google.com
BUG=

Committed: https://github.com/dart-lang/sdk/commit/f63b3d3e7b499c7f9aef7c62aefcc26486113a21

[Brian Wilkerson, 2016-10-06 21:55:52]

lgtm

https://codereview.chromium.org/2393963004/diff/1/pkg/analyzer/lib/src/genera...
File pkg/analyzer/lib/src/generated/bazel.dart (right):

https://codereview.chromium.org/2393963004/diff/1/pkg/analyzer/lib/src/genera...
pkg/analyzer/lib/src/generated/bazel.dart:225: class _BazelPackageUri {
This class seems like overkill because it does so little.

In addition, it isn't actually specific to Bazel. The way it splits the package
name and lib path are the same as is done for all package: URIs, so we might
want to rename it. Or get rid of it, given that we don't have a wrapper class
for the PackageMapUriResolver.

https://codereview.chromium.org/2393963004/diff/1/pkg/analyzer/test/generated...
File pkg/analyzer/test/generated/bazel_test.dart (right):

https://codereview.chromium.org/2393963004/diff/1/pkg/analyzer/test/generated...
pkg/analyzer/test/generated/bazel_test.dart:304: String _p(String path) =>
provider.convertPath(path);
Hah! I missed that before. Much better than 'ppp' :-)

[Paul Berry, 2016-10-07 15:30:57]

https://codereview.chromium.org/2393963004/diff/20001/pkg/analyzer/lib/src/ge...
File pkg/analyzer/lib/src/generated/bazel.dart (right):

https://codereview.chromium.org/2393963004/diff/20001/pkg/analyzer/lib/src/ge...
pkg/analyzer/lib/src/generated/bazel.dart:62: String path =
'${workspace.root}/third_party/dart/'
This won't work on Windows because it uses hardcoded '/'s.  Also because
bazelUri.path may contain '/'s.

https://codereview.chromium.org/2393963004/diff/20001/pkg/analyzer/lib/src/ge...
pkg/analyzer/lib/src/generated/bazel.dart:69: String path =
'${workspace.root}/$packagePath/lib/${bazelUri.path}';
Similar problem here.

https://codereview.chromium.org/2393963004/diff/20001/pkg/analyzer/lib/src/ge...
pkg/analyzer/lib/src/generated/bazel.dart:209: * [absolutePath] is not in the
workspace [root].
Since the caller always provides a path within the workspace root, the
"isWithin" check seems wasteful.

How about if instead we say that the argument to getPath is relative to the
workspace root.  Then this function can just be:

return provider.getFile(provider.pathContext.join(root, path));

And "${workspace.root}/" can be dropped from the caller.

[scheglov, 2016-10-07 16:02:43]

PTAL

https://codereview.chromium.org/2393963004/diff/1/pkg/analyzer/lib/src/genera...
File pkg/analyzer/lib/src/generated/bazel.dart (right):

https://codereview.chromium.org/2393963004/diff/1/pkg/analyzer/lib/src/genera...
pkg/analyzer/lib/src/generated/bazel.dart:225: class _BazelPackageUri {
On 2016/10/06 21:55:52, Brian Wilkerson wrote:
> This class seems like overkill because it does so little.
> 
> In addition, it isn't actually specific to Bazel. The way it splits the
package
> name and lib path are the same as is done for all package: URIs, so we might
> want to rename it. Or get rid of it, given that we don't have a wrapper class
> for the PackageMapUriResolver.

Done.

https://codereview.chromium.org/2393963004/diff/20001/pkg/analyzer/lib/src/ge...
File pkg/analyzer/lib/src/generated/bazel.dart (right):

https://codereview.chromium.org/2393963004/diff/20001/pkg/analyzer/lib/src/ge...
pkg/analyzer/lib/src/generated/bazel.dart:62: String path =
'${workspace.root}/third_party/dart/'
On 2016/10/07 15:30:57, Paul Berry wrote:
> This won't work on Windows because it uses hardcoded '/'s.  Also because
> bazelUri.path may contain '/'s.

Thanks.
I tested it now manually with Windows style MemoryResourceProvider and fixed
these problems.

https://codereview.chromium.org/2393963004/diff/20001/pkg/analyzer/lib/src/ge...
pkg/analyzer/lib/src/generated/bazel.dart:69: String path =
'${workspace.root}/$packagePath/lib/${bazelUri.path}';
On 2016/10/07 15:30:57, Paul Berry wrote:
> Similar problem here.

Acknowledged.

https://codereview.chromium.org/2393963004/diff/20001/pkg/analyzer/lib/src/ge...
pkg/analyzer/lib/src/generated/bazel.dart:209: * [absolutePath] is not in the
workspace [root].
On 2016/10/07 15:30:57, Paul Berry wrote:
> Since the caller always provides a path within the workspace root, the
> "isWithin" check seems wasteful.
> 
> How about if instead we say that the argument to getPath is relative to the
> workspace root.  Then this function can just be:
> 
> return provider.getFile(provider.pathContext.join(root, path));
> 
> And "${workspace.root}/" can be dropped from the caller.

Done.

[Paul Berry, 2016-10-07 16:05:48]

lgtm

[Brian Wilkerson, 2016-10-07 16:17:35]

lgtm

[scheglov, 2016-10-07 16:19:58]

Description was changed from

==========
Add BazelPackageUriResolver with resolveAbsolute().

R=brianwilkerson@google.com, paulberry@google.com
BUG=
==========

to

==========
Add BazelPackageUriResolver with resolveAbsolute().

R=brianwilkerson@google.com, paulberry@google.com
BUG=

Committed:
https://github.com/dart-lang/sdk/commit/f63b3d3e7b499c7f9aef7c62aefcc26486113a21
==========

[scheglov, 2016-10-07 16:19:59]

Committed patchset #3 (id:40001) manually as
f63b3d3e7b499c7f9aef7c62aefcc26486113a21 (presubmit successful).