languages/dart-sdk
languages/dart-sdk
1
0
Fork 0
mirror of https://github.com/dart-lang/sdk synced 2024-09-05 16:41:07 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix PackageBuildWorkspacePackage's contains

Browse source 
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>
This commit is contained in:
Sam Rawlins 2019-01-29 02:01:51 +00:00 committed by commit-bot@chromium.org
parent d84851daf8
commit c5a954794b
2 changed files with 26 additions and 11 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

13
pkg/analyzer/lib/src/workspace/package_build.dart
View file

@ -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;
}
}

24
pkg/analyzer/test/src/workspace/package_build_test.dart
View file

@ -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() {