Server .packages support.

pkg/analysis_server/lib/src/context_manager.dart

Index: pkg/analysis_server/lib/src/context_manager.dart
diff --git a/pkg/analysis_server/lib/src/context_manager.dart b/pkg/analysis_server/lib/src/context_manager.dart
index fc93e96ec3b5d302ee074d6c356ed0a982353e09..269f221718aeb5876d325ed51b556593fc9c1c86 100644
--- a/pkg/analysis_server/lib/src/context_manager.dart
+++ b/pkg/analysis_server/lib/src/context_manager.dart
@@ -6,6 +6,7 @@ library context.directory.manager;
 
 import 'dart:async';
 import 'dart:collection';
+import 'dart:convert';
 import 'dart:core' hide Resource;
 
 import 'package:analysis_server/src/analysis_server.dart';
@@ -18,6 +19,9 @@ import 'package:analyzer/src/generated/engine.dart';
 import 'package:analyzer/src/generated/java_io.dart';
 import 'package:analyzer/src/generated/source.dart';
 import 'package:analyzer/src/generated/source_io.dart';
+import 'package:package_config/packages.dart';
+import 'package:package_config/packages_file.dart' as pkgfile show parse;
+import 'package:package_config/src/packages_impl.dart' show MapPackages;
 import 'package:path/path.dart' as pathos;
 import 'package:watcher/watcher.dart';
 
@@ -42,6 +46,11 @@ abstract class AbstractContextManager implements ContextManager {
   static const String PUBSPEC_NAME = 'pubspec.yaml';
 
   /**
+   * File name of package spec files.
+   */
+  static const String PACKAGE_SPEC_NAME = '.packages';
+
+  /**
    * [_ContextInfo] object for each included directory in the most
    * recent successful call to [setRoots].
    */
@@ -106,7 +115,8 @@ abstract class AbstractContextManager implements ContextManager {
   /**
    * Create and return a new analysis context.
    */
-  AnalysisContext addContext(Folder folder, UriResolver packageUriResolver);
+  AnalysisContext addContext(
+      Folder folder, UriResolver packageUriResolver, Packages packages);
 
   /**
    * Called when the set of files associated with a context have changed (or
@@ -298,7 +308,7 @@ abstract class AbstractContextManager implements ContextManager {
    * Called when the package map for a context has changed.
    */
   void updateContextPackageUriResolver(
-      Folder contextFolder, UriResolver packageUriResolver);
+      Folder contextFolder, UriResolver packageUriResolver, Packages packages);
 
   /**
    * Resursively adds all Dart and HTML files to the [changeSet].
@@ -391,6 +401,20 @@ abstract class AbstractContextManager implements ContextManager {
     info.dependencySubscriptions.clear();
   }
 
+  void _checkForPackagespecUpdate(
+      String path, _ContextInfo info, Folder folder) {
+    // Check to see if this is the .packages file for this context and if so,
+    // update the context's source factory.
+    if (pathContext.basename(path) == PACKAGE_SPEC_NAME &&
+        info.isPathToPackageDescription(path)) {
+      File packagespec = resourceProvider.getFile(path);
+      if (packagespec.exists) {
+        Packages packages = _readPackagespec(packagespec);
+        updateContextPackageUriResolver(folder, null, packages);
+      }
+    }
+  }
+
   /**
    * Compute the appropriate package URI resolver for [folder], and store
    * dependency information in [info]. Return `null` if no package map can
@@ -479,8 +503,20 @@ abstract class AbstractContextManager implements ContextManager {
       _handleWatchEvent(folder, info, event);
     });
     try {
-      UriResolver packageUriResolver = _computePackageUriResolver(folder, info);
-      info.context = addContext(folder, packageUriResolver);
+      Packages packages;
+      UriResolver packageUriResolver;
+
+      // Try .packages first.
+      if (pathos.basename(pubspecFile.path) == PACKAGE_SPEC_NAME) {
+        packages = _readPackagespec(pubspecFile);
+      }
+
+      // Next resort to a package uri resolver.
+      if (packages == null) {
+        packageUriResolver = _computePackageUriResolver(folder, info);
+      }
+
+      info.context = addContext(folder, packageUriResolver, packages);
       info.context.name = folder.path;
     } catch (_) {
       info.changeSubscription.cancel();
@@ -496,13 +532,13 @@ abstract class AbstractContextManager implements ContextManager {
    * created for them and excluded from the context associated with the
    * [folder].
    *
-   * If [withPubspecOnly] is `true`, a context will be created only if there
-   * is a 'pubspec.yaml' file in the [folder].
+   * If [withPackageSpecOnly] is `true`, a context will be created only if there
+   * is a 'pubspec.yaml' or '.packages' file in the [folder].
    *
-   * Returns create pubspec-based contexts.
+   * Returns created contexts.
    */
-  List<_ContextInfo> _createContexts(Folder folder, bool withPubspecOnly) {
-    // try to find subfolders with pubspec files
+  List<_ContextInfo> _createContexts(Folder folder, bool withPackageSpecOnly) {
+    // Try to find subfolders with pubspecs or .packages files.
     List<_ContextInfo> children = <_ContextInfo>[];
     try {
       for (Resource child in folder.getChildren()) {
@@ -514,20 +550,27 @@ abstract class AbstractContextManager implements ContextManager {
       // The directory either doesn't exist or cannot be read. Either way, there
       // are no subfolders that need to be added.
     }
-    // check whether there is a pubspec in the folder
-    File pubspecFile = folder.getChild(PUBSPEC_NAME);
-    if (pubspecFile.exists) {
+
+    // Start by looking for .packages.
+    File packageSpec = folder.getChild(PACKAGE_SPEC_NAME);
+
+    // Fall back to looking for a pubspec.
+    if (!packageSpec.exists) {
+      packageSpec = folder.getChild(PUBSPEC_NAME);
+    }
+
+    if (packageSpec.exists) {
       return <_ContextInfo>[
-        _createContextWithSources(folder, pubspecFile, children)
+        _createContextWithSources(folder, packageSpec, children)
       ];
     }
-    // no pubspec, done
-    if (withPubspecOnly) {
+    // No packagespec? Done.
+    if (withPackageSpecOnly) {
       return children;
     }
-    // OK, create a context without a pubspec
+    // OK, create a context without a packagespec.
     return <_ContextInfo>[
-      _createContextWithSources(folder, pubspecFile, children)
+      _createContextWithSources(folder, packageSpec, children)
     ];
   }
 
@@ -613,11 +656,34 @@ abstract class AbstractContextManager implements ContextManager {
           return;
         }
         Resource resource = resourceProvider.getResource(path);
-        // pubspec was added in a sub-folder, extract a new context
-        if (_isPubspec(path) && info.isRoot && !info.isPubspec(path)) {
-          _extractContext(info, resource);
-          return;
+
+        String directoryPath = pathContext.dirname(path);
+
+        // Check to see if we need to create a new context.
+        if (info.isRoot) {
+
+          // Only create a new context if this is not the same directory
+          // described by our info object.
+          if (info.folder.path != directoryPath) {

[Brian Wilkerson, 2015/07/17 18:02:57]

If a .packages file is added to the context's root directory we need to update
the context to use the .packages file to resolve URI's. I think this misses that
case.

[pquitslund, 2015/07/17 19:30:02]

Actually, I think this is handled in _createContext(..) which is called by
_extractContext(..).  The fact remains that this is not well under-test...  I
added a TODO in the tests and hope that we can address it better after Paul's
rethink.

+            if (pathContext.basename(path) == PUBSPEC_NAME) {
+              // Check for a sibling .packages file.
+              if (!resourceProvider
+                  .getFile(pathos.join(directoryPath, '.packages')).exists) {

[Brian Wilkerson, 2015/07/17 18:02:58]

nit: use PACKAGE_SPEC_NAME (here and below)

[pquitslund, 2015/07/17 19:30:02]

Done.

+                _extractContext(info, resource);
+                return;
+              }
+            }
+            if (pathContext.basename(path) == PACKAGE_SPEC_NAME) {
+              // Check for a sibling pubspec.yaml file.
+              if (!resourceProvider
+                  .getFile(pathos.join(directoryPath, 'pubspec.yaml')).exists) {

[Brian Wilkerson, 2015/07/17 18:02:57]

nit: use PUBSPEC_NAME (here and below)

[pquitslund, 2015/07/17 19:30:02]

Done.

+                _extractContext(info, resource);
+                return;
+              }
+            }
+          }
         }
+
         // If the file went away and was replaced by a folder before we
         // had a chance to process the event, resource might be a Folder.  In
         // that case don't add it.
@@ -633,11 +699,34 @@ abstract class AbstractContextManager implements ContextManager {
         }
         break;
       case ChangeType.REMOVE:
-        // pubspec was removed, merge the context into its parent
-        if (info.isPubspec(path) && !info.isRoot) {
-          _mergeContext(info);
-          return;
+
+        // If package spec info is removed, check to see if we can merge contexts.
+        // Note that it's important to verify that there is NEITHER a .packages nor a
+        // lingering pubspec.yaml before merging.
+        if (!info.isRoot) {
+          String directoryPath = pathContext.dirname(path);
+
+          // Only merge if this is the same directory described by our info object.
+          if (info.folder.path == directoryPath) {
+            if (pathContext.basename(path) == PUBSPEC_NAME) {
+              // Check for a sibling .packages file.
+              if (!resourceProvider
+                  .getFile(pathos.join(directoryPath, '.packages')).exists) {
+                _mergeContext(info);
+                return;
+              }
+            }
+            if (pathContext.basename(path) == PACKAGE_SPEC_NAME) {
+              // Check for a sibling pubspec.yaml file.
+              if (!resourceProvider
+                  .getFile(pathos.join(directoryPath, 'pubspec.yaml')).exists) {
+                _mergeContext(info);
+                return;
+              }
+            }
+          }
         }
+
         List<Source> sources = info.context.getSourcesWithFullName(path);
         if (!sources.isEmpty) {
           ChangeSet changeSet = new ChangeSet();
@@ -660,6 +749,9 @@ abstract class AbstractContextManager implements ContextManager {
         break;
     }
 
+    //TODO(pquitslund): find the right place for this
+    _checkForPackagespecUpdate(path, info, folder);

[pquitslund, 2015/07/17 19:30:02]

Here's where we test for .packages changes.

+
     if (info.packageMapInfo != null &&
         info.packageMapInfo.isChangedDependency(path, resourceProvider)) {
       _recomputePackageUriResolver(info);
@@ -669,9 +761,7 @@ abstract class AbstractContextManager implements ContextManager {
   /**
    * Returns `true` if the given [path] is excluded by [excludedPaths].
    */
-  bool _isExcluded(String path) {
-    return _isExcludedBy(excludedPaths, path);
-  }
+  bool _isExcluded(String path) => _isExcludedBy(excludedPaths, path);
 
   /**
    * Returns `true` if the given [path] is excluded by [excludedPaths].
@@ -696,13 +786,6 @@ abstract class AbstractContextManager implements ContextManager {
   }
 
   /**
-   * Returns `true` if the given absolute [path] is a pubspec file.
-   */
-  bool _isPubspec(String path) {
-    return pathContext.basename(path) == PUBSPEC_NAME;
-  }
-
-  /**
    * Merges [info] context into its parent.
    */
   void _mergeContext(_ContextInfo info) {
@@ -721,6 +804,14 @@ abstract class AbstractContextManager implements ContextManager {
     }
   }
 
+  Packages _readPackagespec(File specFile) {
+    //TODO(pquitslund): error-handling?

[Brian Wilkerson, 2015/07/17 18:02:57]

Yes. If we cannot read the file, I think we should act as if the file doesn't
exist. It would be nice if we could create an error for the file indicating that
it's being ignored.

[pquitslund, 2015/07/17 19:30:02]

Cool.  Added a try/catch and a TODO to create an error down the road.

+    String contents = specFile.readAsStringSync();
+    Map<String, Uri> map =
+        pkgfile.parse(UTF8.encode(contents), new Uri.file(specFile.path));
+    return new MapPackages(map);
+  }
+
   /**
    * Recompute the package URI resolver for the context described by [info],
    * and update the client appropriately.
@@ -732,7 +823,7 @@ abstract class AbstractContextManager implements ContextManager {
     // "pub list" is in progress is just going to get thrown away anyhow.
     UriResolver packageUriResolver =
         _computePackageUriResolver(info.folder, info);
-    updateContextPackageUriResolver(info.folder, packageUriResolver);
+    updateContextPackageUriResolver(info.folder, packageUriResolver, null);
   }
 
   /**
@@ -875,9 +966,9 @@ class _ContextInfo {
   _ContextInfo parent;
 
   /**
-   * The `pubspec.yaml` file path for this context.
+   * The package description file path for this context.
    */
-  String pubspecPath;
+  String packageDescriptionPath;
 
   /**
    * Stream subscription we are using to watch the context's directory for
@@ -911,7 +1002,7 @@ class _ContextInfo {
   OptimizingPubPackageMapInfo packageMapInfo;
 
   _ContextInfo(this.folder, File pubspecFile, this.children, this.packageRoot) {
-    pubspecPath = pubspecFile.path;
+    packageDescriptionPath = pubspecFile.path;
     for (_ContextInfo child in children) {
       child.parent = this;
     }
@@ -934,14 +1025,12 @@ class _ContextInfo {
   /**
    * Returns `true` if [resource] is excluded, as it is in one of the children.
    */
-  bool excludesResource(Resource resource) {
-    return excludes(resource.path);
-  }
+  bool excludesResource(Resource resource) => excludes(resource.path);
 
   /**
-   * Returns `true` if [path] is the pubspec file of this context.
+   * Returns `true` if [path] is the package description file for this context 
+   * (pubspec.yaml or .packages).
    */
-  bool isPubspec(String path) {
-    return path == pubspecPath;
-  }
+  bool isPathToPackageDescription(String path) =>
+      path == packageDescriptionPath;
 }