Remove the special casing of 'packages' files from the analyzer and analysis server.

Change-Id: I11fa2f73f7cdba8c53478c9002cefe82e58c09d6
Reviewed-on: https://dart-review.googlesource.com/49822
Commit-Queue: Devon Carew <devoncarew@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>

CHANGELOG.md

@@ -19,6 +19,16 @@
 
 [pub#1856]: https://github.com/dart-lang/pub/issues/1856
 
+#### Analyzer
+
+* The command line analyzer (dartanalyzer) and the analysis server no longer
+  treat directories named `packages` specially. Previously they had ignored
+  these directories - and their contents - from the point of view of analysis. Now
+  they'll be treated just as regular directories. This special-casing of
+  `packages` directories was to support using symlinks for package: resolution;
+  that functionality is now handled by `.packages` files.
+
+
 ## 2.0.0-dev.44.0
 
 ### Core library changes

pkg/analysis_server/lib/src/context_manager.dart

@@ -407,11 +407,6 @@ class ContextManagerImpl implements ContextManager {
    */
   static const String LIB_DIR_NAME = 'lib';
 
-  /**
-   * The name of `packages` folders.
-   */
-  static const String PACKAGES_NAME = 'packages';
-
   /**
    * File name of pubspec files.
    */
@@ -836,9 +831,6 @@ class ContextManagerImpl implements ContextManager {
         changeSet.addedSource(source);
         info.sources[path] = source;
       } else if (child is Folder) {
-        if (child.shortName == PACKAGES_NAME) {
-          continue;
-        }
         _addPreviouslyExcludedSources(info, changeSet, child, oldExcludedPaths);
       }
     }
@@ -875,10 +867,6 @@ class ContextManagerImpl implements ContextManager {
           info.sources[path] = source;
         }
       } else if (child is Folder) {
-        String shortName = child.shortName;
-        if (shortName == PACKAGES_NAME) {
-          continue;
-        }
         _addSourceFiles(changeSet, child, info);
       }
     }
@@ -1181,9 +1169,7 @@ class ContextManagerImpl implements ContextManager {
    */
   void _createContexts(ContextInfo parent, Folder folder,
       List<String> excludedPaths, bool withPackageSpecOnly) {
-    if (_isExcluded(folder.path) ||
-        folder.shortName.startsWith('.') ||
-        folder.shortName == 'packages') {
+    if (_isExcluded(folder.path) || folder.shortName.startsWith('.')) {
       return;
     }
     // Decide whether a context needs to be created for [folder] here, and if
@@ -1383,7 +1369,6 @@ class ContextManagerImpl implements ContextManager {
     // maybe excluded globally
     if (_isExcluded(path) ||
         _isContainedInDotFolder(info.folder.path, path) ||
-        _isInPackagesDir(info.folder.path, path) ||
         _isInTopLevelDocDir(info.folder.path, path)) {
       return;
     }
@@ -1526,19 +1511,6 @@ class ContextManagerImpl implements ContextManager {
     });
   }
 
-  /**
-   * Determine whether the given [path], when interpreted relative to the
-   * context root [root], contains a 'packages' folder.
-   */
-  bool _isInPackagesDir(String root, String path) {
-    String suffixPath = absolutePathContext.suffix(root, path);
-    if (suffixPath == null) {
-      return false;
-    }
-    List<String> pathParts = absolutePathContext.split(suffixPath);
-    return pathParts.contains(PACKAGES_NAME);
-  }
-
   /**
    * Determine whether the given [path] is in the direct 'doc' folder of the
    * context root [root].

pkg/analysis_server/test/context_manager_test.dart

@@ -180,24 +180,6 @@ test_pack:lib/''');
     expect(sourceFactory.forUri('dart:typed_data'), isNotNull);
   }
 
-  test_ignoreFilesInPackagesFolder() {
-    // create a context with a pubspec.yaml file
-    String pubspecPath = path.posix.join(projPath, 'pubspec.yaml');
-    resourceProvider.newFile(pubspecPath, 'pubspec');
-    // create a file in the "packages" folder
-    String filePath1 = path.posix.join(projPath, 'packages', 'file1.dart');
-    resourceProvider.newFile(filePath1, 'contents');
-    // "packages" files are ignored initially
-    manager.setRoots(<String>[projPath], <String>[], <String, String>{});
-    expect(callbacks.currentFilePaths, isEmpty);
-    // "packages" files are ignored during watch
-    String filePath2 = path.posix.join(projPath, 'packages', 'file2.dart');
-    resourceProvider.newFile(filePath2, 'contents');
-    return pumpEventQueue().then((_) {
-      expect(callbacks.currentFilePaths, isEmpty);
-    });
-  }
-
   void test_isInAnalysisRoot_excluded() {
     // prepare paths
     String project = convertPath('/project');
@@ -238,6 +220,23 @@ test_pack:lib/''');
     expect(manager.isInAnalysisRoot('/test.dart'), isFalse);
   }
 
+  test_packagesFolder_areAnalyzed() {
+    // create a context with a pubspec.yaml file
+    String pubspecPath = path.posix.join(projPath, 'pubspec.yaml');
+    resourceProvider.newFile(pubspecPath, 'pubspec');
+    // create a file in the "packages" folder
+    String filePath1 = path.posix.join(projPath, 'packages', 'file1.dart');
+    resourceProvider.newFile(filePath1, 'contents');
+    manager.setRoots(<String>[projPath], <String>[], <String, String>{});
+    expect(callbacks.currentFilePaths, unorderedEquals([filePath1]));
+    String filePath2 = path.posix.join(projPath, 'packages', 'file2.dart');
+    resourceProvider.newFile(filePath2, 'contents');
+    return pumpEventQueue().then((_) {
+      expect(
+          callbacks.currentFilePaths, unorderedEquals([filePath1, filePath2]));
+    });
+  }
+
   test_path_filter() async {
     // Setup context.
     Folder root = resourceProvider.newFolder(projPath);
@@ -943,16 +942,6 @@ test_pack:lib/''');
     expect(callbacks.currentFilePaths, hasLength(0));
   }
 
-  void test_setRoots_noContext_inPackagesFolder() {
-    String pubspecPath = path.posix.join(projPath, 'packages', 'pubspec.yaml');
-    resourceProvider.newFile(pubspecPath, 'name: test');
-    manager.setRoots(<String>[projPath], <String>[], <String, String>{});
-    // verify
-    expect(callbacks.currentContextRoots, hasLength(1));
-    expect(callbacks.currentContextRoots, contains(projPath));
-    expect(callbacks.currentFilePaths, hasLength(0));
-  }
-
   void test_setRoots_packageResolver() {
     String filePath = join(projPath, 'lib', 'foo.dart');
     newFile('$projPath/${ContextManagerImpl.PACKAGE_SPEC_NAME}',
@@ -968,6 +957,16 @@ test_pack:lib/''');
     expect(result.fullName, filePath);
   }
 
+  void test_setRoots_packagesFolder_hasContext() {
+    String pubspecPath = path.posix.join(projPath, 'packages', 'pubspec.yaml');
+    resourceProvider.newFile(pubspecPath, 'name: test');
+    manager.setRoots(<String>[projPath], <String>[], <String, String>{});
+    // verify
+    expect(callbacks.currentContextRoots, hasLength(2));
+    expect(callbacks.currentContextRoots, contains(projPath));
+    expect(callbacks.currentFilePaths, hasLength(0));
+  }
+
   void test_setRoots_pathContainsDotFile() {
     // If the path to a file (relative to the context root) contains a folder
     // whose name begins with '.', then the file is ignored.

pkg/analyzer/lib/src/dart/analysis/context_locator.dart

@@ -40,11 +40,6 @@ class ContextLocatorImpl implements ContextLocator {
    */
   static const String OLD_ANALYSIS_OPTIONS_NAME = '.analysis_options';
 
-  /**
-   * The name of the packages folder.
-   */
-  static const String PACKAGES_DIR_NAME = 'packages';
-
   /**
    * The name of the packages file.
    */
@@ -277,8 +272,7 @@ class ContextLocatorImpl implements ContextLocator {
       for (Resource child in folder.getChildren()) {
         if (child is Folder) {
           if (excludedFolders.contains(folder) ||
-              folder.shortName.startsWith('.') ||
-              folder.shortName == PACKAGES_DIR_NAME) {
+              folder.shortName.startsWith('.')) {
             containingRoot.excluded.add(folder);
           } else {
             _createContextRoots(roots, child, excludedFolders, containingRoot,

pkg/analyzer/lib/src/dart/analysis/context_root.dart

@@ -98,8 +98,7 @@ class ContextRootImpl implements ContextRoot {
   bool _isExcluded(String path) {
     Context context = resourceProvider.pathContext;
     String name = context.basename(path);
-    if (name.startsWith('.') ||
-        (name == 'packages' && resourceProvider.getResource(path) is Folder)) {
+    if (name.startsWith('.')) {
       return true;
     }
     for (String excludedPath in excludedPaths) {

pkg/analyzer/test/src/dart/analysis/context_locator_test.dart

@@ -211,24 +211,6 @@ class ContextLocatorImplTest extends Object with ResourceProviderMixin {
     expect(outerRoot.packagesFile, outerPackagesFile);
   }
 
-  void test_locateRoots_nested_excluded_packages() {
-    Folder outerRootFolder = newFolder('/test/outer');
-    File outerOptionsFile = newOptionsFile('/test/outer');
-    File outerPackagesFile = newPackagesFile('/test/outer');
-    Folder excludedFolder = newFolder('/test/outer/packages');
-    newOptionsFile('/test/outer/packages/inner');
-
-    List<ContextRoot> roots =
-        contextLocator.locateRoots(includedPaths: [outerRootFolder.path]);
-    expect(roots, hasLength(1));
-
-    ContextRoot outerRoot = findRoot(roots, outerRootFolder);
-    expect(outerRoot.includedPaths, unorderedEquals([outerRootFolder.path]));
-    expect(outerRoot.excludedPaths, unorderedEquals([excludedFolder.path]));
-    expect(outerRoot.optionsFile, outerOptionsFile);
-    expect(outerRoot.packagesFile, outerPackagesFile);
-  }
-
   void test_locateRoots_nested_multiple() {
     Folder outerRootFolder = newFolder('/test/outer');
     File outerOptionsFile = newOptionsFile('/test/outer');
@@ -450,6 +432,24 @@ class ContextLocatorImplTest extends Object with ResourceProviderMixin {
     expect(outerRoot.packagesFile, overridePackagesFile);
   }
 
+  void test_locateRoots_nested_packagesDirectory_included() {
+    Folder outerRootFolder = newFolder('/test/outer');
+    File outerOptionsFile = newOptionsFile('/test/outer');
+    File outerPackagesFile = newPackagesFile('/test/outer');
+    newOptionsFile('/test/outer/packages/inner');
+
+    List<ContextRoot> roots =
+        contextLocator.locateRoots(includedPaths: [outerRootFolder.path]);
+    expect(roots, hasLength(2));
+
+    ContextRoot outerRoot = findRoot(roots, outerRootFolder);
+    expect(outerRoot.includedPaths, unorderedEquals([outerRootFolder.path]));
+    expect(outerRoot.excludedPaths,
+        unorderedEquals(['/test/outer/packages/inner']));
+    expect(outerRoot.optionsFile, outerOptionsFile);
+    expect(outerRoot.packagesFile, outerPackagesFile);
+  }
+
   void test_locateRoots_single_dir_directOptions_directPackages() {
     Folder rootFolder = newFolder('/test/root');
     File optionsFile = newOptionsFile('/test/root');

pkg/analyzer/test/src/dart/analysis/context_root_test.dart

@@ -65,14 +65,14 @@ class ContextRootTest {
     expect(contextRoot.isAnalyzed(filePath), isFalse);
   }
 
-  test_isAnalyzed_implicitlyExcluded_packages() {
-    String folderPath = provider.convertPath('/test/root/lib/packages');
-    provider.newFolder(folderPath);
-    expect(contextRoot.isAnalyzed(folderPath), isFalse);
-  }
-
   test_isAnalyzed_included() {
     String filePath = provider.convertPath('/test/root/lib/root.dart');
     expect(contextRoot.isAnalyzed(filePath), isTrue);
   }
+
+  test_isAnalyzed_packagesDirectory_analyzed() {
+    String folderPath = provider.convertPath('/test/root/lib/packages');
+    provider.newFolder(folderPath);
+    expect(contextRoot.isAnalyzed(folderPath), isTrue);
+  }
 }