From c5a954794b30f89a20d02e2f19a9561bcc96333e Mon Sep 17 00:00:00 2001
From: Sam Rawlins <srawlins@google.com>
Date: Tue, 29 Jan 2019 02:01:51 +0000
Subject: [PATCH] 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>
---
 .../lib/src/workspace/package_build.dart      | 13 ++++++----
 .../src/workspace/package_build_test.dart     | 24 +++++++++++++------
 2 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/pkg/analyzer/lib/src/workspace/package_build.dart b/pkg/analyzer/lib/src/workspace/package_build.dart
index 99dd95c5f9d..c5c74673ecd 100644
--- a/pkg/analyzer/lib/src/workspace/package_build.dart
+++ b/pkg/analyzer/lib/src/workspace/package_build.dart
@@ -37,6 +37,10 @@ class PackageBuildFileUriResolver extends ResourceUriResolver {
       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 @@ class PackageBuildWorkspace extends Workspace {
       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 @@ class PackageBuildWorkspacePackage extends WorkspacePackage {
   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;
   }
 }
diff --git a/pkg/analyzer/test/src/workspace/package_build_test.dart b/pkg/analyzer/test/src/workspace/package_build_test.dart
index 173fc46277c..71a2cf0cede 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 PackageBuildWorkspaceTest with ResourceProviderMixin {
 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 @@ class PackageBuildWorkspacePackageTest with ResourceProviderMixin {
     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 @@ class PackageBuildWorkspacePackageTest with ResourceProviderMixin {
 
   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() {