Actual URI support for package URI resolution.

pkg/analyzer/lib/src/generated/source_io.dart

Index: pkg/analyzer/lib/src/generated/source_io.dart
diff --git a/pkg/analyzer/lib/src/generated/source_io.dart b/pkg/analyzer/lib/src/generated/source_io.dart
index 9c40d7461b4666ffb553a8b69545968bc2b0ee28..1dae5a04cbac737995df6f84751df4b5f5bf9918 100644
--- a/pkg/analyzer/lib/src/generated/source_io.dart
+++ b/pkg/analyzer/lib/src/generated/source_io.dart
@@ -276,11 +276,11 @@ class FileUriResolver extends UriResolver {
   static String FILE_SCHEME = "file";
 
   @override
-  Source resolveAbsolute(Uri uri) {
+  Source resolveAbsolute(Uri uri, [Uri actualUri]) {
     if (!isFileUri(uri)) {
       return null;
     }
-    return new FileBasedSource(new JavaFile.fromUri(uri), uri);
+    return new FileBasedSource(new JavaFile.fromUri(uri), actualUri);

[Paul Berry, 2015/07/21 19:39:27]

There's a subtle change in behavior in the case where actualUri is null that I'm
not sure is on purpose.  Previously, the new Source object would receive its URI
directly from the "uri" parameter.  Now, when actualUri is null, the Source
object derives its URI from JavaFile.toURI().  I suspect this is ok (since all
JavaFile.toURI() does is absolutize and normalize the path, and I *think* that
the URI's should have been absolutized and normalized before they get here). 
But I would feel more comfortable if either:

(a) we changed the code to match the old behavior when actualUri == null, or
(b) we changed actualUri to a non-optional parameter and ensured that a non-null
value was always passed in (along the lines of Brian's suggestion in his comment
on pkg/analyzer/lib/src/generated/source.dart)

[pquitslund, 2015/07/21 20:55:10]

Good catch!
I convinced myself that this would be safe but I share your uneasiness.
I picked (a) since it anticipates the same check we need to make below.

   }
 
   @override
@@ -425,7 +425,7 @@ class PackageUriResolver extends UriResolver {
   }
 
   @override
-  Source resolveAbsolute(Uri uri) {
+  Source resolveAbsolute(Uri uri, [Uri actualUri]){
     if (!isPackageUri(uri)) {
       return null;
     }
@@ -463,7 +463,7 @@ class PackageUriResolver extends UriResolver {
       }
     }
     return new FileBasedSource(
-        getCanonicalFile(_packagesDirectories[0], pkgName, relPath), uri);
+        getCanonicalFile(_packagesDirectories[0], pkgName, relPath), actualUri);

[Paul Berry, 2015/07/21 19:39:27]

This has the same problem as above, but it's definitely wrong in this case
because we're resolving a package: URI, so passing null (and letting the URI be
chosen by FileBasedSource from the JavaFile will get us the wrong kind of URI). 
We need logic so that if actualUri is null, we pass in uri.

I'm kind of surprised this didn't lead to any test failures, so I'm wondering if
we have a gap in our test coverage.

[pquitslund, 2015/07/21 20:55:10]

Absolutely.  Spot on.  Thanks!

   }
 
   @override
@@ -537,7 +537,7 @@ class RelativeFileUriResolver extends UriResolver {
       : super();
 
   @override
-  Source resolveAbsolute(Uri uri) {
+  Source resolveAbsolute(Uri uri, [Uri actualUri]) {
     String rootPath = _rootDirectory.toURI().path;
     String uriPath = uri.path;
     if (uriPath != null && uriPath.startsWith(rootPath)) {
@@ -545,7 +545,7 @@ class RelativeFileUriResolver extends UriResolver {
       for (JavaFile dir in _relativeDirectories) {
         JavaFile file = new JavaFile.relative(dir, filePath);
         if (file.exists()) {
-          return new FileBasedSource(file, uri);
+          return new FileBasedSource(file, actualUri);

[Paul Berry, 2015/07/21 19:39:27]

Similar problem here, although I can't assess the impact because I can't find
any code that uses this class (RelativeFileResolver)--not even tests.

[pquitslund, 2015/07/21 20:55:10]

Done.

         }
       }
     }