Actual URI support for package URI resolution.

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

 // Copyright (c) 2014, 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.
 
 // This code was auto-generated, is not intended to be edited, and is subject to
 // significant change. Please see the README file for more information.
 
 library engine.source.io;
 
 import 'dart:collection';
@@ skipping 258 matching lines @@
 /**
  * Instances of the class `FileUriResolver` resolve `file` URI's.
  */
 class FileUriResolver extends UriResolver {
   /**
    * The name of the `file` scheme.
    */
   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
   Uri restoreAbsolute(Source source) {
     if (source is FileBasedSource) {
       return new Uri.file(source.fullName);
     }
     return null;
   }
   
@@ skipping 124 matching lines @@
         _CanLogRequiredKeyIoException = false;
         AnalysisEngine.instance.logger.logError("Canonical failed: $pkgDir",
             new CaughtException(exception, stackTrace));
       }
     }
     return new JavaFile.relative(pkgDir, relPath.replaceAll(
         '/', new String.fromCharCode(JavaFile.separatorChar)));
   }
 
   @override
-  Source resolveAbsolute(Uri uri) {
+  Source resolveAbsolute(Uri uri, [Uri actualUri]){
     if (!isPackageUri(uri)) {
       return null;
     }
     String path = uri.path;
     if (path == null) {
       path = uri.path;
       if (path == null) {
         return null;
       }
     }
@@ skipping 17 matching lines @@
       if (resolvedFile.exists()) {
         JavaFile canonicalFile =
             getCanonicalFile(packagesDirectory, pkgName, relPath);
         if (_isSelfReference(packagesDirectory, canonicalFile)) {
           uri = canonicalFile.toURI();
         }
         return new FileBasedSource(canonicalFile, uri);
       }
     }
     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
   Uri restoreAbsolute(Source source) {
     String sourcePath = source.fullName;
     for (JavaFile packagesDirectory in _packagesDirectories) {
       List<JavaFile> pkgFolders = packagesDirectory.listFiles();
       if (pkgFolders != null) {
         for (JavaFile pkgFolder in pkgFolders) {
           try {
@@ skipping 53 matching lines @@
   final JavaFile _rootDirectory;
 
   /**
    * Initialize a newly created resolver to resolve `file` URI's relative to the given root
    * directory.
    */
   RelativeFileUriResolver(this._rootDirectory, this._relativeDirectories)
       : 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)) {
       String filePath = uri.path.substring(rootPath.length);
       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.

         }
       }
     }
     return null;
   }
 
   /**
    * Return `true` if the given URI is a `file` URI.
    *
    * @param uri the URI being tested
    * @return `true` if the given URI is a `file` URI
    */
   static bool isFileUri(Uri uri) => uri.scheme == FILE_SCHEME;
 }