From 3b2a695d71057f3a538f79f48a96f6bdca2474bb Mon Sep 17 00:00:00 2001 From: Ivan Posva Date: Tue, 22 Mar 2016 09:02:04 -0700 Subject: [PATCH] - Properly return null if an unknown package is resolved. - Allow short form package:foo imports. R=asiva@google.com Review URL: https://codereview.chromium.org/1822863002 . --- runtime/bin/builtin.dart | 25 +++++++++- .../scenarios/bad_resolve_package/.packages | 1 + .../bad_resolve_package_test.dart | 47 +++++++++++++++++++ .../isolate/scenarios/short_package/.packages | 1 + .../short_package/flu_package/flu.dart | 3 ++ .../short_package/flu_package/flu.text | 1 + .../short_package/short_package_test.dart | 37 +++++++++++++++ 7 files changed, 113 insertions(+), 2 deletions(-) create mode 100644 tests/isolate/scenarios/bad_resolve_package/.packages create mode 100644 tests/isolate/scenarios/bad_resolve_package/bad_resolve_package_test.dart create mode 100644 tests/isolate/scenarios/short_package/.packages create mode 100644 tests/isolate/scenarios/short_package/flu_package/flu.dart create mode 100644 tests/isolate/scenarios/short_package/flu_package/flu.text create mode 100644 tests/isolate/scenarios/short_package/short_package_test.dart diff --git a/runtime/bin/builtin.dart b/runtime/bin/builtin.dart index 9a34b51cb0b..2f64b77be68 100644 --- a/runtime/bin/builtin.dart +++ b/runtime/bin/builtin.dart @@ -295,7 +295,18 @@ Uri _resolvePackageUri(Uri uri) { if (mapping == null) { throw "No mapping for '$packageName' package when resolving '$uri'."; } - var path = uri.path.substring(packageName.length + 1); + var path; + if (uri.path.length > packageName.length) { + path = uri.path.substring(packageName.length + 1); + } else { + // Handle naked package resolution to the default package name: + // package:foo is equivalent to package:foo/foo.dart + assert(uri.path.length == packageName.length); + path = "$packageName.dart"; + } + if (_traceLoading) { + _log("Path to be resolved in package: $path"); + } resolvedUri = mapping.resolve(path); } if (_traceLoading) { @@ -754,7 +765,17 @@ Future _resolvePackageUriFuture(Uri packageUri) async { } assert(_packagesReady); - var result = _resolvePackageUri(packageUri); + var result; + try { + result = _resolvePackageUri(packageUri); + } catch (e, s) { + // Any error during resolution will resolve this package as not mapped, + // which is indicated by a null return. + if (_traceLoading) { + _log("Exception ($e) when resolving package URI: $packageUri"); + } + result = null; + } if (_traceLoading) { _log("Resolved '$packageUri' to '$result'"); } diff --git a/tests/isolate/scenarios/bad_resolve_package/.packages b/tests/isolate/scenarios/bad_resolve_package/.packages new file mode 100644 index 00000000000..5acc702fe2c --- /dev/null +++ b/tests/isolate/scenarios/bad_resolve_package/.packages @@ -0,0 +1 @@ +# Intentionally left blank to ensure no packages are resolved. diff --git a/tests/isolate/scenarios/bad_resolve_package/bad_resolve_package_test.dart b/tests/isolate/scenarios/bad_resolve_package/bad_resolve_package_test.dart new file mode 100644 index 00000000000..c8017e1e7f7 --- /dev/null +++ b/tests/isolate/scenarios/bad_resolve_package/bad_resolve_package_test.dart @@ -0,0 +1,47 @@ +// Copyright (c) 2016, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +// PackageRoot=none + +import 'dart:io'; +import 'dart:isolate'; + +main([args, port]) async { + if (port != null) { + testBadResolvePackage(port); + return; + } + var p = new RawReceivePort(); + Isolate.spawnUri(Platform.script, [], p.sendPort); + p.handler = (msg) { + p.close(); + if (msg is! List) { + print(msg.runtimeType); + throw "Failure return from spawned isolate:\n\n$msg"; + } + // Expecting a null resolution for inexistent package mapping. + if (msg[0] != null) { + throw "Bad package config in child isolate: ${msg[0]}\n" + "Expected: 'Foo'"; + } + print("SUCCESS"); + }; +} + +testBadResolvePackage(port) async { + try { + var packageRootStr = Platform.packageRoot; + var packageConfigStr = Platform.packageConfig; + var packageConfig = await Isolate.packageConfig; + var badPackageUri = Uri.parse("package:asdf/qwerty.dart"); + var resolvedPkg = await Isolate.resolvePackageUri(badPackageUri); + print("Spawned isolate's package root flag: $packageRootStr"); + print("Spawned isolate's package config flag: $packageConfigStr"); + print("Spawned isolate's loaded package config: $packageConfig"); + print("Spawned isolate's resolved package path: $resolvedPkg"); + port.send([resolvedPkg]); + } catch (e, s) { + port.send("$e\n$s\n"); + } +} diff --git a/tests/isolate/scenarios/short_package/.packages b/tests/isolate/scenarios/short_package/.packages new file mode 100644 index 00000000000..324934cf41d --- /dev/null +++ b/tests/isolate/scenarios/short_package/.packages @@ -0,0 +1 @@ +flu:flu_package/ diff --git a/tests/isolate/scenarios/short_package/flu_package/flu.dart b/tests/isolate/scenarios/short_package/flu_package/flu.dart new file mode 100644 index 00000000000..5e33f7c5b9d --- /dev/null +++ b/tests/isolate/scenarios/short_package/flu_package/flu.dart @@ -0,0 +1,3 @@ +class Flu { + static var value = "Flu"; +} diff --git a/tests/isolate/scenarios/short_package/flu_package/flu.text b/tests/isolate/scenarios/short_package/flu_package/flu.text new file mode 100644 index 00000000000..d9d3a9a899e --- /dev/null +++ b/tests/isolate/scenarios/short_package/flu_package/flu.text @@ -0,0 +1 @@ +Bar \ No newline at end of file diff --git a/tests/isolate/scenarios/short_package/short_package_test.dart b/tests/isolate/scenarios/short_package/short_package_test.dart new file mode 100644 index 00000000000..bfbe61ca86f --- /dev/null +++ b/tests/isolate/scenarios/short_package/short_package_test.dart @@ -0,0 +1,37 @@ +// Copyright (c) 2016, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +// PackageRoot=none + +import 'dart:io'; +import 'dart:isolate'; + +import "package:flu"; + +var PACKAGE_FLU = "package:flu"; +var FLU_TEXT = "flu.text"; + +testShortResolution(package_uri) async { + var fluPackage = await Isolate.resolvePackageUri(Uri.parse(package_uri)); + print("Resolved $package_uri to $fluPackage"); + var fluText = fluPackage.resolve(FLU_TEXT); + print("Resolved $FLU_TEXT from $package_uri to $fluText"); + var fluFile = new File.fromUri(fluText); + var fluString = await fluFile.readAsString(); + if (fluString != "Bar") { + throw "Contents of $FLU_TEXT not matching.\n" + "Got: $fluString\n" + "Expected: Bar"; + } +} + +main([args, port]) async { + if (Flu.value != "Flu") { + throw "Import of wrong Flu package."; + } + await testShortResolution(PACKAGE_FLU); + await testShortResolution(PACKAGE_FLU + "/"); + await testShortResolution(PACKAGE_FLU + "/abc.def"); + print("SUCCESS"); +}