Add a validator for dependency version constraints.

utils/pub/validator/dependency.dart

Index: utils/pub/validator/dependency.dart
diff --git a/utils/pub/validator/dependency.dart b/utils/pub/validator/dependency.dart
new file mode 100644
index 0000000000000000000000000000000000000000..930009c92b6d740aaaa5150a66ec59f2ac43c06c
--- /dev/null
+++ b/utils/pub/validator/dependency.dart
@@ -0,0 +1,90 @@
+// Copyright (c) 2012, 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.
+
+library dependency_validator;
+
+import '../entrypoint.dart';
+import '../hosted_source.dart';
+import '../http.dart';
+import '../package.dart';
+import '../utils.dart';
+import '../validator.dart';
+import '../version.dart';
+
+/// A validator that validates a package's dependencies.
+class DependencyValidator extends Validator {
+  DependencyValidator(Entrypoint entrypoint)
+    : super(entrypoint);
+
+  Future validate() {
+    return Futures.forEach(entrypoint.root.pubspec.dependencies, (dependency) {
+      if (dependency.source is! HostedSource) {
+        return _warnAboutSource(dependency);
+      }
+
+      if (dependency.constraint.isAny &&
+          // TODO(nweiz): once we have development dependencies, we should warn
+          // about unittest. Until then, it's reasonable not to put a constraint
+          // on it.

[Bob Nystrom, 2012/12/21 00:11:46]

Mention bug #5358 here.

[nweiz, 2012/12/21 01:01:50]

Done.

+          dependency.name != 'unittest') {
+        return _warnAboutConstraint(dependency);
+      }
+
+      return new Future.immediate(null);
+    });
+  }
+
+  /// Warn that dependencies should use the hosted source.
+  Future _warnAboutSource(PackageRef ref) {
+    return entrypoint.cache.sources['hosted']
+        .getVersions(ref.name, ref.name)
+        .transformException((e) => <Version>[])
+        .transform((versions) {
+      var constraint;
+      var primary = Version.primary(versions);
+      if (primary != null) {
+        constraint = _constraintForVersion(primary);
+      } else {
+        constraint = ref.constraint.toString();
+        if (!ref.constraint.isAny && ref.constraint is! Version) {
+          constraint = '"$constraint"';
+        }
+      }
+
+      warnings.add('Don\'t depend on "${ref.name}" from the ${ref.source.name} '

[Bob Nystrom, 2012/12/21 00:11:46]

I believe we do have package authors uploading packages with git dependencies.
We've never told them not to do that. If pub is going to start suggesting
otherwise, we should have a public discussion about it too.

[nweiz, 2012/12/21 01:01:50]

I think it's very reasonable to discourage Git dependencies, especially now that
those packages can be easily uploaded to pub.dartlang.org. Not only does Git not
play well with version constraints, but a package having a Git dependency means
that no one can use it (even transitively) without having Git installed.

Since this is just a warning anyway, I think it's fine to put in. I'm fine with
mentioning it on the mailing list as well if you want.

Empirically, only two packages on pub.dartlang.org have ever had Git
dependencies, and of those only one (buckshot) still does. I suspect this is
accidental, as the Git packages it depends on have their most recent versions on
pub.dartlang.org, and previous versions of buckshot used hosted dependencies.

[Bob Nystrom, 2012/12/21 01:48:06]

That sounds reasonable to me. We should probably document this somewhere, too.

[nweiz, 2012/12/21 01:54:57]

Filed 7553 to track that.

+              'source. Use the hosted source instead. For example:\n'
+          '\n'
+          'dependencies:\n'
+          '  ${ref.name}: $constraint\n'
+          '\n'
+          'Using the hosted source ensures that everyone can download your '
+              'package\'s dependencies along with your package.');
+    });
+  }
+
+  /// Warn that dependencies should have version constraints.
+  Future _warnAboutConstraint(PackageRef ref) {
+    return entrypoint.loadLockFile().transform((lockFile) {
+      var message = 'Your dependency on "${ref.name}" should have a version '
+          'constraint.';
+      var locked = lockFile.packages[ref.name];
+      if (locked != null) {
+        message = '$message For example:\n'
+          '\n'
+          'dependencies:\n'
+          '  ${ref.name}: ${_constraintForVersion(locked.version)}\n';
+      }
+      warnings.add("$message\n"
+          "Without a constraint, you're promising to support all future "
+          "versions of ${ref.name}.");
+    });
+  }
+
+  /// Returns the suggested version constraint for a dependency that was tested
+  /// against [version].

[Bob Nystrom, 2012/12/21 00:11:46]

Move this into version.dart? Seems generally useful.

[nweiz, 2012/12/21 01:01:50]

Most of what this method is actually doing is formatting the version to put into
the YAML snippet, which I don't think is generally useful.

+  String _constraintForVersion(Version version) {
+    if (version.major == 0) return version.toString();

[Bob Nystrom, 2012/12/21 00:11:46]

For 0.x.x packages, web_ui has been using (I think) a constraint that *does*
allow patch updates. I think something like ">=0.x.y <0.x.(y+1)".

[nweiz, 2012/12/21 01:01:50]

Done.

+    return '">=$version <${version.major + 1}.0.0"';
+  }
+}