From 01abcb45c78175fecce10c890c20a3b2b982a869 Mon Sep 17 00:00:00 2001 From: "floitsch@google.com" <floitsch@google.com> Date: Mon, 15 Apr 2013 21:24:27 +0000 Subject: [PATCH] Refactor Future constructors. BUG= Review URL: https://codereview.chromium.org//14070010 git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge@21517 260f80e4-7a28-3924-810f-c04153c831b5 --- lib/src/command_uploader.dart | 2 +- lib/src/entrypoint.dart | 16 ++++++++-------- lib/src/error_group.dart | 2 +- lib/src/git.dart | 4 ++-- lib/src/git_source.dart | 4 ++-- lib/src/hosted_source.dart | 4 ++-- lib/src/io.dart | 4 ++-- lib/src/oauth2.dart | 2 +- lib/src/path_source.dart | 4 ++-- lib/src/pub.dart | 4 ++-- lib/src/source.dart | 4 ++-- lib/src/utils.dart | 4 ++-- lib/src/validator/compiled_dartdoc.dart | 2 +- lib/src/validator/dependency.dart | 4 ++-- lib/src/validator/directory.dart | 2 +- lib/src/validator/lib.dart | 2 +- lib/src/validator/license.dart | 2 +- lib/src/validator/name.dart | 2 +- lib/src/validator/pubspec_field.dart | 2 +- lib/src/validator/utf8_readme.dart | 2 +- lib/src/version_solver.dart | 7 +++---- test/error_group_test.dart | 4 ++-- test/test_pub.dart | 6 +++--- test/validator/dependency_test.dart | 4 ++-- test/validator/size_test.dart | 2 +- test/version_solver_test.dart | 4 ++-- 26 files changed, 49 insertions(+), 50 deletions(-) diff --git a/lib/src/command_uploader.dart b/lib/src/command_uploader.dart index 029f78c6..5bb05f29 100644 --- a/lib/src/command_uploader.dart +++ b/lib/src/command_uploader.dart @@ -59,7 +59,7 @@ class UploaderCommand extends PubCommand { exit(exit_codes.USAGE); } - return new Future.of(() { + return new Future.sync(() { var package = commandOptions['package']; if (package != null) return package; return new Entrypoint(path.current, cache).root.name; diff --git a/lib/src/entrypoint.dart b/lib/src/entrypoint.dart index db6ad47c..fe058c4d 100644 --- a/lib/src/entrypoint.dart +++ b/lib/src/entrypoint.dart @@ -71,7 +71,7 @@ class Entrypoint { if (pendingOrCompleted != null) return pendingOrCompleted; var packageDir = path.join(packagesDir, id.name); - var future = new Future.of(() { + var future = new Future.sync(() { ensureDir(path.dirname(packageDir)); if (entryExists(packageDir)) { @@ -102,7 +102,7 @@ class Entrypoint { /// directory, respecting the [LockFile] if present. Returns a [Future] that /// completes when all dependencies are installed. Future installDependencies() { - return new Future.of(() { + return new Future.sync(() { return resolveVersions(cache.sources, root, loadLockFile()); }).then(_installDependencies); } @@ -119,7 +119,7 @@ class Entrypoint { /// other dependencies as specified by the [LockFile] if possible. Returns a /// [Future] that completes when all dependencies are installed. Future updateDependencies(List<String> dependencies) { - return new Future.of(() { + return new Future.sync(() { var solver = new VersionSolver(cache.sources, root, loadLockFile()); for (var dependency in dependencies) { solver.useLatestVersion(dependency); @@ -131,10 +131,10 @@ class Entrypoint { /// Removes the old packages directory, installs all dependencies listed in /// [packageVersions], and writes a [LockFile]. Future _installDependencies(List<PackageId> packageVersions) { - return new Future.of(() { + return new Future.sync(() { cleanDir(packagesDir); return Future.wait(packageVersions.map((id) { - if (id.isRoot) return new Future.immediate(id); + if (id.isRoot) return new Future.value(id); return install(id); }).toList()); }).then((ids) { @@ -148,13 +148,13 @@ class Entrypoint { /// reached packages. This should only be called after the lockfile has been /// successfully generated. Future<List<Pubspec>> walkDependencies() { - return new Future.of(() { + return new Future.sync(() { var lockFile = loadLockFile(); var group = new FutureGroup<Pubspec>(); var visited = new Set<String>(); // Include the root package in the results. - group.add(new Future.immediate(root.pubspec)); + group.add(new Future.value(root.pubspec)); visitPackage(Pubspec pubspec) { for (var ref in pubspec.dependencies) { @@ -166,7 +166,7 @@ class Entrypoint { visited.add(ref.name); var future; if (ref.name == root.name) { - future = new Future<Pubspec>.immediate(root.pubspec); + future = new Future<Pubspec>.value(root.pubspec); } else { future = cache.describe(id); } diff --git a/lib/src/error_group.dart b/lib/src/error_group.dart index 624e51cf..694638c2 100644 --- a/lib/src/error_group.dart +++ b/lib/src/error_group.dart @@ -269,7 +269,7 @@ class _ErrorGroupStream extends Stream { if (_isDone) return; _subscription.cancel(); // Call these asynchronously to work around issue 7913. - new Future.immediate(null).then((_) { + new Future.value().then((_) { _controller.addError(e); _controller.close(); }); diff --git a/lib/src/git.dart b/lib/src/git.dart index 53ecbe78..8d996229 100644 --- a/lib/src/git.dart +++ b/lib/src/git.dart @@ -13,7 +13,7 @@ import 'utils.dart'; /// Tests whether or not the git command-line app is available for use. Future<bool> get isInstalled { if (_isGitInstalledCache != null) { - return new Future.immediate(_isGitInstalledCache); + return new Future.value(_isGitInstalledCache); } return _gitCommand.then((git) => git != null); @@ -44,7 +44,7 @@ String _gitCommandCache; /// found on the user's PATH. Future<String> get _gitCommand { if (_gitCommandCache != null) { - return new Future.immediate(_gitCommandCache); + return new Future.value(_gitCommandCache); } return _tryGitCommand("git").then((success) { diff --git a/lib/src/git_source.dart b/lib/src/git_source.dart index 2802ad8a..c98616d1 100644 --- a/lib/src/git_source.dart +++ b/lib/src/git_source.dart @@ -123,7 +123,7 @@ class GitSource extends Source { /// future that completes once this is finished and throws an exception if it /// fails. Future _ensureRepoCache(PackageId id) { - return new Future.of(() { + return new Future.sync(() { var path = _repoCachePath(id); if (!entryExists(path)) return _clone(_getUrl(id), path, mirror: true); return git.run(["fetch"], workingDir: path).then((result) => null); @@ -143,7 +143,7 @@ class GitSource extends Source { /// the working tree, but instead makes the repository a local mirror of the /// remote repository. See the manpage for `git clone` for more information. Future _clone(String from, String to, {bool mirror: false}) { - return new Future.of(() { + return new Future.sync(() { // Git on Windows does not seem to automatically create the destination // directory. ensureDir(to); diff --git a/lib/src/hosted_source.dart b/lib/src/hosted_source.dart index 5f63ec3e..5a4106e2 100644 --- a/lib/src/hosted_source.dart +++ b/lib/src/hosted_source.dart @@ -63,7 +63,7 @@ class HostedSource extends Source { /// Downloads a package from the site and unpacks it. Future<bool> install(PackageId id, String destPath) { - return new Future.of(() { + return new Future.sync(() { var url = _makeVersionUrl(id, (server, package, version) => "$server/packages/$package/versions/$version.tar.gz"); log.io("Install package from $url."); @@ -98,7 +98,7 @@ class HostedSource extends Source { return '%${match[0].codeUnitAt(0)}'; }); - return new Future.immediate( + return new Future.value( path.join(systemCacheRoot, urlDir, "${parsed.first}-${id.version}")); } diff --git a/lib/src/io.dart b/lib/src/io.dart index 697e02bd..32c4574d 100644 --- a/lib/src/io.dart +++ b/lib/src/io.dart @@ -550,9 +550,9 @@ Future timeout(Future input, int milliseconds, String description) { /// Returns a future that completes to the value that the future returned from /// [fn] completes to. Future withTempDir(Future fn(String path)) { - return new Future.of(() { + return new Future.sync(() { var tempDir = createTempDir(); - return new Future.of(() => fn(tempDir)) + return new Future.sync(() => fn(tempDir)) .whenComplete(() => deleteEntry(tempDir)); }); } diff --git a/lib/src/oauth2.dart b/lib/src/oauth2.dart index 4a436811..e4b8ee07 100644 --- a/lib/src/oauth2.dart +++ b/lib/src/oauth2.dart @@ -104,7 +104,7 @@ Future withClient(SystemCache cache, Future fn(Client client)) { /// Gets a new OAuth2 client. If saved credentials are available, those are /// used; otherwise, the user is prompted to authorize the pub client. Future<Client> _getClient(SystemCache cache) { - return new Future.of(() { + return new Future.sync(() { var credentials = _loadCredentials(cache); if (credentials == null) return _authorize(); diff --git a/lib/src/path_source.dart b/lib/src/path_source.dart index e849b6f7..e4d1f02a 100644 --- a/lib/src/path_source.dart +++ b/lib/src/path_source.dart @@ -25,7 +25,7 @@ class PathSource extends Source { final shouldCache = false; Future<Pubspec> describe(PackageId id) { - return new Future.of(() { + return new Future.sync(() { _validatePath(id.name, id.description); return new Pubspec.load(id.name, id.description["path"], systemCache.sources); @@ -40,7 +40,7 @@ class PathSource extends Source { } Future<bool> install(PackageId id, String destination) { - return new Future.of(() { + return new Future.sync(() { try { _validatePath(id.name, id.description); } on FormatException catch(err) { diff --git a/lib/src/pub.dart b/lib/src/pub.dart index e5236082..9af9c27c 100644 --- a/lib/src/pub.dart +++ b/lib/src/pub.dart @@ -145,7 +145,7 @@ main() { /// Checks that pub is running on a supported platform. If it isn't, it prints /// an error message and exits. Completes when the validation is done. Future validatePlatform() { - return new Future.of(() { + return new Future.sync(() { if (Platform.operatingSystem != 'windows') return; return runProcess('ver', []).then((result) { @@ -267,7 +267,7 @@ abstract class PubCommand { exit(_chooseExitCode(error)); } - new Future.of(() { + new Future.sync(() { if (requiresEntrypoint) { // TODO(rnystrom): Will eventually need better logic to walk up // subdirectories until we hit one that looks package-like. For now, diff --git a/lib/src/source.dart b/lib/src/source.dart index adb95714..b5e5d531 100644 --- a/lib/src/source.dart +++ b/lib/src/source.dart @@ -154,7 +154,7 @@ abstract class Source { /// /// This doesn't need to be implemented if [shouldCache] is false. Future<String> systemCacheDirectory(PackageId id) { - return new Future.immediateError( + return new Future.error( "systemCacheDirectory() must be implemented if shouldCache is true."); } @@ -206,7 +206,7 @@ abstract class Source { /// [descriptionsEqual]. /// /// By default, this just returns [id]. - Future<PackageId> resolveId(PackageId id) => new Future.immediate(id); + Future<PackageId> resolveId(PackageId id) => new Future.value(id); /// Returns the [Package]s that have been installed in the system cache. List<Package> getCachedPackages() { diff --git a/lib/src/utils.dart b/lib/src/utils.dart index 6836fb43..5e80e570 100644 --- a/lib/src/utils.dart +++ b/lib/src/utils.dart @@ -249,7 +249,7 @@ Stream<String> streamToLines(Stream<String> stream) { Future<Iterable> futureWhere(Iterable iter, test(value)) { return Future.wait(iter.map((e) { var result = test(e); - if (result is! Future) result = new Future.immediate(result); + if (result is! Future) result = new Future.value(result); return result.then((result) => new Pair(e, result)); })) .then((pairs) => pairs.where((pair) => pair.last)) @@ -343,7 +343,7 @@ Future awaitObject(object) { if (object is Iterable) { return Future.wait(object.map(awaitObject).toList()); } - if (object is! Map) return new Future.immediate(object); + if (object is! Map) return new Future.value(object); var pairs = <Future<Pair>>[]; object.forEach((key, value) { diff --git a/lib/src/validator/compiled_dartdoc.dart b/lib/src/validator/compiled_dartdoc.dart index 1998c47c..45a8fbaf 100644 --- a/lib/src/validator/compiled_dartdoc.dart +++ b/lib/src/validator/compiled_dartdoc.dart @@ -20,7 +20,7 @@ class CompiledDartdocValidator extends Validator { : super(entrypoint); Future validate() { - return new Future.of(() { + return new Future.sync(() { for (var entry in listDir(entrypoint.root.dir, recursive: true)) { if (path.basename(entry) != "nav.json") continue; var dir = path.dirname(entry); diff --git a/lib/src/validator/dependency.dart b/lib/src/validator/dependency.dart index d66059de..711e82da 100644 --- a/lib/src/validator/dependency.dart +++ b/lib/src/validator/dependency.dart @@ -31,12 +31,12 @@ class DependencyValidator extends Validator { 'package.\n' 'Pub enables "package:${entrypoint.root.name}" imports ' 'implicitly.'); - return new Future.immediate(null); + return new Future.value(); } if (dependency.constraint.isAny) _warnAboutConstraint(dependency); - return new Future.immediate(null); + return new Future.value(); }); } diff --git a/lib/src/validator/directory.dart b/lib/src/validator/directory.dart index 5d6628d5..216f2bf6 100644 --- a/lib/src/validator/directory.dart +++ b/lib/src/validator/directory.dart @@ -21,7 +21,7 @@ class DirectoryValidator extends Validator { static final _PLURAL_NAMES = ["tools", "tests", "docs", "examples"]; Future validate() { - return new Future.of(() { + return new Future.sync(() { for (var dir in listDir(entrypoint.root.dir)) { if (!dirExists(dir)) continue; diff --git a/lib/src/validator/lib.dart b/lib/src/validator/lib.dart index 7ff06fca..2e72738b 100644 --- a/lib/src/validator/lib.dart +++ b/lib/src/validator/lib.dart @@ -23,7 +23,7 @@ class LibValidator extends Validator { : super(entrypoint); Future validate() { - return new Future.of(() { + return new Future.sync(() { var libDir = path.join(entrypoint.root.dir, "lib"); if (!dirExists(libDir)) { diff --git a/lib/src/validator/license.dart b/lib/src/validator/license.dart index ffcbd10c..50d1bfe2 100644 --- a/lib/src/validator/license.dart +++ b/lib/src/validator/license.dart @@ -19,7 +19,7 @@ class LicenseValidator extends Validator { : super(entrypoint); Future validate() { - return new Future.of(() { + return new Future.sync(() { var licenseLike = new RegExp( r"^([a-zA-Z0-9]+[-_])?(LICENSE|COPYING)(\..*)?$"); if (listDir(entrypoint.root.dir) diff --git a/lib/src/validator/name.dart b/lib/src/validator/name.dart index 412d6c39..542d9fdc 100644 --- a/lib/src/validator/name.dart +++ b/lib/src/validator/name.dart @@ -28,7 +28,7 @@ class NameValidator extends Validator { : super(entrypoint); Future validate() { - return new Future.of(() { + return new Future.sync(() { _checkName(entrypoint.root.name, 'Package name "${entrypoint.root.name}"', isPackage: true); diff --git a/lib/src/validator/pubspec_field.dart b/lib/src/validator/pubspec_field.dart index e87f38b2..03a1b8e0 100644 --- a/lib/src/validator/pubspec_field.dart +++ b/lib/src/validator/pubspec_field.dart @@ -56,6 +56,6 @@ class PubspecFieldValidator extends Validator { errors.add('Your pubspec.yaml is missing a "version" field.'); } - return new Future.immediate(null); + return new Future.value(); } } diff --git a/lib/src/validator/utf8_readme.dart b/lib/src/validator/utf8_readme.dart index 8bf52a9d..cbb85c47 100644 --- a/lib/src/validator/utf8_readme.dart +++ b/lib/src/validator/utf8_readme.dart @@ -20,7 +20,7 @@ class Utf8ReadmeValidator extends Validator { : super(entrypoint); Future validate() { - return new Future.of(() { + return new Future.sync(() { var readme = entrypoint.root.readmePath; if (readme == null) return; var bytes = readBinaryFile(readme); diff --git a/lib/src/version_solver.dart b/lib/src/version_solver.dart index 5b055d87..36569bab 100644 --- a/lib/src/version_solver.dart +++ b/lib/src/version_solver.dart @@ -97,7 +97,7 @@ class VersionSolver { Future processNextWorkItem(_) { while (true) { // Stop if we are done. - if (_work.isEmpty) return new Future.immediate(buildResults()); + if (_work.isEmpty) return new Future.value(buildResults()); // If we appear to be stuck in a loop, then we probably have an unstable // graph, bail. We guess this based on a rough heuristic that it should @@ -284,8 +284,7 @@ class ChangeVersion implements WorkItem { Version version) { // If there is no version, it means no package, so no dependencies. if (version == null) { - return new Future<Map<String, PackageRef>>.immediate( - <String, PackageRef>{}); + return new Future<Map<String, PackageRef>>.value(<String, PackageRef>{}); } var id = new PackageId(package, source, version, description); @@ -475,7 +474,7 @@ class PubspecCache { Future<Pubspec> load(PackageId id) { // Complete immediately if it's already cached. if (_pubspecs.containsKey(id)) { - return new Future<Pubspec>.immediate(_pubspecs[id]); + return new Future<Pubspec>.value(_pubspecs[id]); } return id.describe().then((pubspec) { diff --git a/test/error_group_test.dart b/test/error_group_test.dart index 13f24153..3460ddd9 100644 --- a/test/error_group_test.dart +++ b/test/error_group_test.dart @@ -36,7 +36,7 @@ main() { expect(errorGroup.done, throwsFormatException); errorGroup.signalError(new FormatException()); - expect(() => errorGroup.registerFuture(new Future.immediate(null)), + expect(() => errorGroup.registerFuture(new Future.value()), throwsStateError); expect(() => errorGroup.registerStream(new StreamController().stream), throwsStateError); @@ -63,7 +63,7 @@ main() { "been called", () { completer.complete('value'); - expect(() => errorGroup.registerFuture(new Future.immediate(null)), + expect(() => errorGroup.registerFuture(new Future.value()), throwsStateError); expect(() => errorGroup.registerStream(new StreamController().stream), throwsStateError); diff --git a/test/test_pub.dart b/test/test_pub.dart index 3ba46a4d..ba049989 100644 --- a/test/test_pub.dart +++ b/test/test_pub.dart @@ -119,7 +119,7 @@ void serve([List<d.Descriptor> contents]) { /// Closes [_server]. Returns a [Future] that will complete after the [_server] /// is closed. Future _closeServer() { - if (_server == null) return new Future.immediate(null); + if (_server == null) return new Future.value(); _server.close(); _server = null; _portCompleterCache = null; @@ -356,7 +356,7 @@ ScheduledProcess startPub({List args, Future<Uri> tokenEndpoint}) { '--trace']; dartArgs.addAll(args); - if (tokenEndpoint == null) tokenEndpoint = new Future.immediate(null); + if (tokenEndpoint == null) tokenEndpoint = new Future.value(); var optionsFuture = tokenEndpoint.then((tokenEndpoint) { var options = new ProcessOptions(); options.workingDirectory = pathInSandbox(appPath); @@ -588,7 +588,7 @@ Future<Pair<List<String>, List<String>>> schedulePackageValidation( return schedule(() { var cache = new SystemCache.withSources(path.join(sandboxDir, cachePath)); - return new Future.of(() { + return new Future.sync(() { var validator = fn(new Entrypoint(path.join(sandboxDir, appPath), cache)); return validator.validate().then((_) { return new Pair(validator.errors, validator.warnings); diff --git a/test/validator/dependency_test.dart b/test/validator/dependency_test.dart index de4d08e9..4001330a 100644 --- a/test/validator/dependency_test.dart +++ b/test/validator/dependency_test.dart @@ -38,9 +38,9 @@ setUpDependency(Map dep, {List<String> hostedVersions}) { expect(request.url.path, equals("/packages/foo.json")); if (hostedVersions == null) { - return new Future.immediate(new http.Response("not found", 404)); + return new Future.value(new http.Response("not found", 404)); } else { - return new Future.immediate(new http.Response(json.stringify({ + return new Future.value(new http.Response(json.stringify({ "name": "foo", "uploaders": ["nweiz@google.com"], "versions": hostedVersions diff --git a/test/validator/size_test.dart b/test/validator/size_test.dart index 47c7d1e6..3dd1e214 100644 --- a/test/validator/size_test.dart +++ b/test/validator/size_test.dart @@ -16,7 +16,7 @@ import 'utils.dart'; Function size(int size) { return (entrypoint) => - new SizeValidator(entrypoint, new Future.immediate(size)); + new SizeValidator(entrypoint, new Future.value(size)); } main() { diff --git a/test/version_solver_test.dart b/test/version_solver_test.dart index 94a7c58a..845f597a 100644 --- a/test/version_solver_test.dart +++ b/test/version_solver_test.dart @@ -481,11 +481,11 @@ class MockSource extends Source { : _packages = <String, Map<Version, Package>>{}; Future<List<Version>> getVersions(String name, String description) { - return new Future.of(() => _packages[description].keys.toList()); + return new Future.sync(() => _packages[description].keys.toList()); } Future<Pubspec> describe(PackageId id) { - return new Future.of(() => _packages[id.name][id.version].pubspec); + return new Future.sync(() => _packages[id.name][id.version].pubspec); } Future<bool> install(PackageId id, String path) { -- GitLab