Allow whitespace in version constraints.

utils/pub/version.dart

 // 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.
 
 /// Handles version numbers, following the [Semantic Versioning][semver] spec.
 ///
 /// [semver]: http://semver.org/
 library version;
 
 import 'dart:math';
 
 import 'utils.dart';
 
+
+/// Regex that matches a version number at the beginning of a string.
+final _START_VERSION = new RegExp(
+    r'^'                                        // Start at beginning.
+    r'(\d+).(\d+).(\d+)'                        // Version number.
+    r'(-([0-9A-Za-z-]+(\.[0-9A-Za-z-]+)*))?'    // Pre-release.
+    r'(\+([0-9A-Za-z-]+(\.[0-9A-Za-z-]+)*))?'); // Build.
+
+/// Like [_START_VERSION] but matches the entire string.
+final _COMPLETE_VERSION = new RegExp("${_START_VERSION.pattern}\$");
+
+/// Parses a comparison operator ("<", ">", "<=", or ">=") at the beginning of
+/// a string.
+final _START_COMPARISON = new RegExp(r"[<>]=?");

[nweiz, 2013/02/21 01:31:11]

This should start with "^".

[Bob Nystrom, 2013/02/21 20:00:50]

Oops. Done.

+
 /// A parsed semantic version number.
 class Version implements Comparable<Version>, VersionConstraint {
   /// No released version: i.e. "0.0.0".
   static Version get none => new Version(0, 0, 0);
-
-  static final _PARSE_REGEX = new RegExp(
-      r'^'                                       // Start at beginning.
-      r'(\d+).(\d+).(\d+)'                       // Version number.
-      r'(-([0-9A-Za-z-]+(\.[0-9A-Za-z-]+)*))?'   // Pre-release.
-      r'(\+([0-9A-Za-z-]+(\.[0-9A-Za-z-]+)*))?'  // Build.
-      r'$');                                     // Consume entire string.
-
   /// The major version number: "1" in "1.2.3".
   final int major;
 
   /// The minor version number: "2" in "1.2.3".
   final int minor;
 
   /// The patch version number: "3" in "1.2.3".
   final int patch;
 
   /// The pre-release identifier: "foo" in "1.2.3-foo". May be `null`.
   final String preRelease;
 
   /// The build identifier: "foo" in "1.2.3+foo". May be `null`.
   final String build;
 
   /// Creates a new [Version] object.
   Version(this.major, this.minor, this.patch, {String pre, this.build})
     : preRelease = pre {
     if (major < 0) throw new ArgumentError(
         'Major version must be non-negative.');
     if (minor < 0) throw new ArgumentError(
         'Minor version must be non-negative.');
     if (patch < 0) throw new ArgumentError(
         'Patch version must be non-negative.');
   }
 
   /// Creates a new [Version] by parsing [text].
   factory Version.parse(String text) {
-    final match = _PARSE_REGEX.firstMatch(text);
+    final match = _COMPLETE_VERSION.firstMatch(text);
     if (match == null) {
       throw new FormatException('Could not parse "$text".');
     }
 
     try {
       int major = int.parse(match[1]);
       int minor = int.parse(match[2]);
       int patch = int.parse(match[3]);
 
       String preRelease = match[5];
@@ skipping 142 matching lines @@
 /// version is valid or not. For example, a ">= 2.0.0" constraint allows any
 /// version that is "2.0.0" or greater. Version objects themselves implement
 /// this to match a specific version.
 abstract class VersionConstraint {
   /// A [VersionConstraint] that allows all versions.
   static VersionConstraint any = new VersionRange();
 
   /// A [VersionConstraint] that allows no versions: i.e. the empty set.
   static VersionConstraint empty = const _EmptyVersion();
 
-  /// Parses a version constraint. This string is a space-separated series of
-  /// version parts. Each part can be one of:
+  /// Parses a version constraint. This string is a series of version parts.
+  /// Each part can be one of:
   ///
   ///   * A version string like `1.2.3`. In other words, anything that can be
   ///     parsed by [Version.parse()].
   ///   * A comparison operator (`<`, `>`, `<=`, or `>=`) followed by a version
-  ///     string. There cannot be a space between the operator and the version.
+  ///     string.
+  ///
+  /// Whitespace is ignored.
   ///
   /// Examples:
   ///
   ///     1.2.3-alpha
   ///     <=5.1.4
-  ///     >2.0.4 <=2.4.6
+  ///     >2.0.4 <= 2.4.6
+  ///     any
   factory VersionConstraint.parse(String text) {
-    if (text.trim() == '') {
+    var originalText = text;
+    var constraints = <VersionConstraint>[];
+
+    void skipWhitespace() {
+      text = text.trim();
+    }
+
+    // Try to parse and consume "any".
+    VersionRange matchAny() {
+      // TODO(rnystrom): This doesn't require whitespace or a punctuator to
+      // separate "any". So this is valid: "anyany>1.0.0any". Is that OK?

[nweiz, 2013/02/21 01:31:11]

It's weird that we allow "any" to be alongside any other version constraints,
but even given that "anyany>1.0.0any" is awful and shouldn't be allowed.

[Bob Nystrom, 2013/02/21 20:00:50]

You're exactly right. Thinking of "any" as a constraint *part* is the error
here. It should be a standalone constraint.
Agreed. Fixed. Now the rule is a constraint is either "any" or a series of
parts.

Much cleaner now.

+      if (!text.startsWith("any")) return null;
+      text = text.substring("any".length);
+      return new VersionRange();
+    }
+
+    // Try to parse and consume a version number.
+    Version matchVersion() {
+      var version = _START_VERSION.firstMatch(text);
+      if (version == null) return null;
+
+      text = text.substring(version.end);
+      return new Version.parse(version[0]);
+    }
+
+    // Try to parse and consume a comparison operator followed by a version.
+    VersionConstraint matchComparison() {
+      var comparison = _START_COMPARISON.firstMatch(text);
+      if (comparison == null) return null;
+
+      var op = comparison[0];
+      text = text.substring(comparison.end);
+      skipWhitespace();
+
+      var version = matchVersion();
+      if (version == null) {
+        throw new FormatException('Expected version number after "$op" in '
+            '"$originalText", got "$text".');

[nweiz, 2013/02/21 01:31:11]

I still wish these exceptions gave more context, although that's more of an
issue with the yaml library than this code.

[Bob Nystrom, 2013/02/21 20:00:50]

There's not much Version can do here. What we can (and probably should) do is
catch this exception in the pubspec parser and wrap it in that additional
context. But that's a separate change.

+      }
+
+      switch (op) {
+        case '<=':
+          return new VersionRange(max: version, includeMax: true);
+        case '<':
+          return new VersionRange(max: version, includeMax: false);
+        case '>=':
+          return new VersionRange(min: version, includeMin: true);
+        case '>':
+          return new VersionRange(min: version, includeMin: false);
+      }
+    }
+
+    while (true) {
+      skipWhitespace();
+      if (text.isEmpty) break;
+
+      var any = matchAny();
+      if (any != null) {
+        constraints.add(any);
+        continue;
+      }
+
+      var version = matchVersion();
+      if (version != null) {
+        constraints.add(version);
+        continue;
+      }
+
+      var comparison = matchComparison();
+      if (comparison != null) {
+        constraints.add(comparison);
+        continue;
+      }
+
+      // If we got here, we couldn't parse the remaining string.
+      throw new FormatException('Could not parse version "$originalText". '
+          'Unknown text at "$text".');
+    }
+
+    if (constraints.isEmpty) {
       throw new FormatException('Cannot parse an empty string.');
     }
 
-    // Split it into space-separated parts.
-    var constraints = <VersionConstraint>[];
-    for (var part in text.split(' ')) {
-      constraints.add(_parseSingleConstraint(part));
-    }
-
     return new VersionConstraint.intersection(constraints);
   }
 
   /// Creates a new version constraint that is the intersection of
   /// [constraints]. It will only allow versions that all of those constraints
   /// allow. If constraints is empty, then it returns a VersionConstraint that
   /// allows all versions.
   factory VersionConstraint.intersection(
       Iterable<VersionConstraint> constraints) {
     var constraint = new VersionRange();
     for (var other in constraints) {
       constraint = constraint.intersect(other);
     }
     return constraint;
   }
 
   /// Returns `true` if this constraint allows no versions.
   bool get isEmpty;
 
   /// Returns `true` if this constraint allows all versions.
   bool get isAny;
 
   /// Returns `true` if this constraint allows [version].
   bool allows(Version version);
 
   /// Creates a new [VersionConstraint] that only allows [Version]s allowed by
   /// both this and [other].
   VersionConstraint intersect(VersionConstraint other);
-
-  static VersionConstraint _parseSingleConstraint(String text) {
-    if (text == 'any') {
-      return new VersionRange();
-    }
-
-    // TODO(rnystrom): Consider other syntaxes for version constraints. This
-    // one is whitespace sensitive (you can't do "< 1.2.3") and "<" is
-    // unfortunately meaningful in YAML, requiring it to be quoted in a
-    // pubspec.
-    // See if it's a comparison operator followed by a version, like ">1.2.3".
-    var match = new RegExp(r"^([<>]=?)?(.*)$").firstMatch(text);
-    if (match != null) {
-      var comparison = match[1];
-      var version = new Version.parse(match[2]);
-      switch (match[1]) {
-        case '<=': return new VersionRange(max: version, includeMax: true);
-        case '<': return new VersionRange(max: version, includeMax: false);
-        case '>=': return new VersionRange(min: version, includeMin: true);
-        case '>': return new VersionRange(min: version, includeMin: false);
-      }
-    }
-
-    // Otherwise, it must be an explicit version.
-    return new Version.parse(text);
-  }
 }
 
 /// Constrains versions to a fall within a given range. If there is a minimum,
 /// then this only allows versions that are at that minimum or greater. If there
 /// is a maximum, then only versions less than that are allowed. In other words,
 /// this allows `>= min, < max`.
 class VersionRange implements VersionConstraint {
   final Version min;
   final Version max;
   final bool includeMin;
@@ skipping 114 matching lines @@
 
 class _EmptyVersion implements VersionConstraint {
   const _EmptyVersion();
 
   bool get isEmpty => true;
   bool get isAny => false;
   bool allows(Version other) => false;
   VersionConstraint intersect(VersionConstraint other) => this;
   String toString() => '<empty>';
 }