commit c5a954794b30f89a20d02e2f19a9561bcc96333e
author Sam Rawlins <srawlins@google.com> Tue Jan 29 02:01:51 2019 +0000
committer commit-bot@chromium.org <commit-bot@chromium.org> Tue Jan 29 02:01:51 2019 +0000
tree 2903cb5e443dd27e9fcd3200177dbcf9ae752f5b
parent d84851daf86b48300637cc34c38f6f9ffb5cd8a8

Fix PackageBuildWorkspacePackage's contains

This implementation did not check whether the path was actually contained
in the package:build workspace. It also made an assumption about how a
resource which is a directory would be treated, which was masked by an
unconditional-and-silenced catch.

The existing test which should have caught this had a hidden exception,
caught by the same unconditional-and-silenced catch. :(

Bug: https://github.com/dart-lang/linter/issues/1393
Change-Id: I0a6ecb584e06877463a47681830eed8f6f914030
Reviewed-on: https://dart-review.googlesource.com/c/91441
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>

pkg/analyzer/lib/src/workspace/package_build.dart

diff --git a/pkg/analyzer/lib/src/workspace/package_build.dart b/pkg/analyzer/lib/src/workspace/package_build.dart
index 99dd95c..c5c74673 100644
--- a/pkg/analyzer/lib/src/workspace/package_build.dart
+++ b/pkg/analyzer/lib/src/workspace/package_build.dart
@@ -37,6 +37,10 @@
       return null;
     }
     String filePath = fileUriToNormalizedPath(provider.pathContext, uri);
+    Resource resource = provider.getResource(filePath);
+    if (resource is! File) {
+      return null;
+    }
     File file = workspace.findFile(filePath);
     if (file != null) {
       return file.createSource(actualUri ?? uri);
@@ -213,8 +217,8 @@
       return null;
     }
     path.Context context = provider.pathContext;
-    String fullBuiltPath = context.join(
-        root, _dartToolRootName, 'build', 'generated', packageName, builtPath);
+    String fullBuiltPath = context.normalize(context.join(
+        root, _dartToolRootName, 'build', 'generated', packageName, builtPath));
     return provider.getFile(fullBuiltPath);
   }
 
@@ -333,10 +337,11 @@
   PackageBuildWorkspacePackage(this.root, this.workspace);
 
   @override
-  bool contains(String path) {
+  bool contains(String filePath) {
     // There is a 1-1 relationship between PackageBuildWorkspaces and
     // PackageBuildWorkspacePackages. If a file is in a package's workspace,
     // then it is in the package as well.
-    return workspace.findFile(path) != null;
+    return workspace.provider.pathContext.isWithin(workspace.root, filePath) &&
+        workspace.findFile(filePath) != null;
   }
 }

pkg/analyzer/test/src/workspace/package_build_test.dart

diff --git a/pkg/analyzer/test/src/workspace/package_build_test.dart b/pkg/analyzer/test/src/workspace/package_build_test.dart
index 173fc46..71a2cf0 100644
--- a/pkg/analyzer/test/src/workspace/package_build_test.dart
+++ b/pkg/analyzer/test/src/workspace/package_build_test.dart
@@ -488,10 +488,20 @@
 class PackageBuildWorkspacePackageTest with ResourceProviderMixin {
   PackageBuildWorkspace _createPackageBuildWorkspace() {
     final contextBuilder = new MockContextBuilder();
-    final packages = new MockPackages();
-    final packageMap = <String, List<Folder>>{'project': []};
-    contextBuilder.packagesMapMap[convertPath('/workspace')] = packages;
-    contextBuilder.packagesToMapMap[packages] = packageMap;
+    final packagesForWorkspace = new MockPackages();
+    final packageMapForWorkspace = <String, List<Folder>>{'project': []};
+    contextBuilder.packagesMapMap[convertPath('/workspace')] =
+        packagesForWorkspace;
+    contextBuilder.packagesToMapMap[packagesForWorkspace] = {
+      'project': <Folder>[]
+    };
+
+    final packagesForWorkspace2 = new MockPackages();
+    contextBuilder.packagesMapMap[convertPath('/workspace2')] =
+        packagesForWorkspace2;
+    contextBuilder.packagesToMapMap[packagesForWorkspace2] = {
+      'project2': <Folder>[]
+    };
 
     newFolder('/workspace/.dart_tool/build');
     newFileWithBytes('/workspace/pubspec.yaml', 'name: project'.codeUnits);
@@ -504,7 +514,7 @@
     newFile('/workspace/project/lib/file.dart');
 
     var package = workspace
-        .findPackageFor(convertPath('/workspace2/project/lib/file.dart'));
+        .findPackageFor(convertPath('/workspace2/project2/lib/file.dart'));
     expect(package, isNull);
   }
 
@@ -521,11 +531,11 @@
 
   void test_contains_differentWorkspace() {
     PackageBuildWorkspace workspace = _createPackageBuildWorkspace();
-    newFile('/workspace2/project/lib/file.dart');
+    newFile('/workspace2/project2/lib/file.dart');
 
     var package = workspace
         .findPackageFor(convertPath('/workspace/project/lib/code.dart'));
-    expect(package.contains('/workspace2/project/lib/file.dart'), isFalse);
+    expect(package.contains('/workspace2/project2/lib/file.dart'), isFalse);
   }
 
   void test_contains_sameWorkspace() {