Package map source factory support.

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

Index: pkg/analyzer/lib/src/generated/source.dart
diff --git a/pkg/analyzer/lib/src/generated/source.dart b/pkg/analyzer/lib/src/generated/source.dart
index 4900cf687fd5f68a6be3a52acc51f9d5c249b6d0..d5f228579ad0e3a6281ef917b3240a239dfc18df 100644
--- a/pkg/analyzer/lib/src/generated/source.dart
+++ b/pkg/analyzer/lib/src/generated/source.dart
@@ -8,11 +8,14 @@
 library engine.source;
 
 import 'dart:collection';
+import 'dart:io';
 import "dart:math" as math;
 
 import 'package:analyzer/file_system/file_system.dart';
+import 'package:analyzer/file_system/physical_file_system.dart';
 import 'package:analyzer/source/package_map_resolver.dart';
 import 'package:analyzer/task/model.dart';
+import 'package:package_config/packages.dart';
 import 'package:path/path.dart' as pathos;
 
 import 'engine.dart';
@@ -575,6 +578,17 @@ class SourceFactory {
   AnalysisContext context;
 
   /**
+   * URI processor used to find mappings for `package:` URIs found in a `.packages` config
+   * file.
+   */
+  final Packages _packages;
+
+  /**
+   * Resource provider used in working with package maps.
+   */
+  final ResourceProvider _resourceProvider;
+
+  /**
    * The resolvers used to resolve absolute URI's.
    */
   final List<UriResolver> _resolvers;
@@ -585,11 +599,14 @@ class SourceFactory {
   LocalSourcePredicate _localSourcePredicate = LocalSourcePredicate.NOT_SDK;
 
   /**
-   * Initialize a newly created source factory.
-   *
-   * @param resolvers the resolvers used to resolve absolute URI's
+   * Initialize a newly created source factory with the given absolute URI [resolvers] and
+   * optional [packages] resolution helper.
    */
-  SourceFactory(this._resolvers);
+  SourceFactory(this._resolvers,
+      [this._packages = Packages.noPackages, ResourceProvider resourceProvider])
+      : _resourceProvider = resourceProvider != null
+          ? resourceProvider
+          : PhysicalResourceProvider.INSTANCE;
 
   /**
    * Return the [DartSdk] associated with this [SourceFactory], or `null` if there
@@ -620,6 +637,19 @@ class SourceFactory {
   /// A table mapping package names to paths of directories containing
   /// the package (or [null] if there is no registered package URI resolver).
   Map<String, List<Folder>> get packageMap {
+    // Start by looking in .packages.
+    if (_packages != Packages.noPackages) {
+      Map<String, List<Folder>> packageMap = <String, List<Folder>>{};
+      _packages.asMap().forEach((String name, Uri uri) {
+        if (uri.scheme == 'file' || uri.scheme == '' /* unspecified */) {
+          packageMap[name] =
+              <Folder>[_resourceProvider.getFolder(uri.toFilePath())];
+        }
+      });
+      return packageMap;
+    }
+
+    // Default to the PackageMapUriResolver.
     PackageMapUriResolver resolver = _resolvers.firstWhere(
         (r) => r is PackageMapUriResolver, orElse: () => null);
     return resolver != null ? resolver.packageMap : null;
@@ -727,13 +757,42 @@ class SourceFactory {
    * @return the absolute URI representing the given source
    */
   Uri restoreUri(Source source) {
+    // First see if a resolver can restore the URI.
     for (UriResolver resolver in _resolvers) {
       Uri uri = resolver.restoreAbsolute(source);
       if (uri != null) {
+        // Now see if there's a package mapping.
+        Uri packageMappedUri = _getPackageMapping(uri);
+        if (packageMappedUri != null) {
+          return packageMappedUri;
+        }
+        // Fall back to the resolver's computed URI.
         return uri;
       }
     }
-    return null;
+
+    // In case no resolver can restore the URI, default to a .packages look-up.
+    return _getPackageMapping(source.uri);

[Paul Berry, 2015/07/01 15:55:40]

I don't think we want this.  The idea of restoreUri is that it should look at
the source's file path only, and determine a URI that resolves to the given file
path.  Which is what the loop above does.  If the loop above fails to create a
URI, then we should return null because there is no possible URI that resolves
to the given source.

+  }
+
+  Uri _getPackageMapping(Uri uri) {

[Paul Berry, 2015/07/01 15:55:40]

I'm concerned about the algorithm for this function.  It seems to me that (a)
there's no need to restrict it to operating on just file URI's, and (b) we
shouldn't be converting the URI to a file path (because that will introduce
unintended behavioral differences between Windows and non-Windows environments).
 We should simply iterate through the key/value pairs in _packages seeing if any
of the values is a valid prefix for the passed-in URI, and then convert it as
appropriate.

+    if (uri.scheme != 'file') {
+      //TODO(pquitslund): add support for non-file URIs.
+      return null;
+    }
+
+    String sourcePath = uri.toFilePath();
+    Uri packageUri;
+    _packages.asMap().forEach((String name, Uri uri) {
+      if (packageUri == null) {
+        String uriPath = uri.toFilePath();
+        if (sourcePath.startsWith(uriPath)) {

[Paul Berry, 2015/07/01 15:55:40]

Is it guaranteed that "uriPath" will end with a trailing slash?  If not, this
won't work properly because it will falsely match a path like "/foobar/baz" to
the uriPath "/foo".

Consider instead using Uri.pathSegments on both URI's that you are comparing,
and then comparing the path segments one by one.

+          packageUri = Uri
+              .parse('package:$name/${sourcePath.substring(uriPath.length)}');
+        }
+      }
+    });
+    return packageUri;
   }
 
   /**
@@ -748,6 +807,14 @@ class SourceFactory {
    *           against the source object's URI
    */
   Source _internalResolveUri(Source containingSource, Uri containedUri) {
+    // Check .packages first.

[Paul Berry, 2015/07/01 15:55:40]

This needs to go after the "if (!containedUri.isAbsolute)" block.  Otherwise we
will resolve relative URI's incorrectly.

+    if (containedUri.scheme == 'package') {
+      Uri packageUri = _packages.resolve(containedUri);
+      //TODO(pquitslund): package_config needs to be updated to set schemes for file URIs.
+      if (packageUri != null && packageUri.scheme == '') {

[Paul Berry, 2015/07/01 15:55:40]

To reduce churn once the TODO above is addressed, let's write this in a way that
it will do what we want once package_config has been updated.  E.g.:

if (packageUri != null) {
  if (packageUri.scheme = '') {
    packageUri = packageUri.replace(scheme: 'file');
  }
  containedUri = packageUri;
}

Also, it might be good to add a TODO to remind us of the problem that Brian
mentioned in his email this morning (this algorithm will cause us to create a
Source object that contains the wrong URI).

+        containedUri = new Uri.file(packageUri.toFilePath());
+      }
+    }
     if (!containedUri.isAbsolute) {
       if (containingSource == null) {
         throw new AnalysisException(