Add string-parsing constructors to dependency, requirement_alternatives, and test_dependency classes

3 files changed, 195 insertions, 122 deletions

diff --git a/libbpkg/manifest.cxx b/libbpkg/manifest.cxx
index b412015..fdc2ee7 100644
--- a/libbpkg/manifest.cxx
+++ b/libbpkg/manifest.cxx
@@ -1013,6 +1013,51 @@ namespace bpkg
     return r;
   }
 
+  // dependency
+  //
+  dependency::
+  dependency (std::string d)
+  {
+    using std::string;
+    using iterator = string::const_iterator;
+
+    iterator b (d.begin ());
+    iterator i (b);
+    iterator ne (b); // End of name.
+    iterator e (d.end ());
+
+    // Find end of name (ne).
+    //
+    // Grep for '=<>([~^' in the bpkg source code and update, if changed.
+    //
+    const string cb ("=<>([~^");
+    for (char c; i != e && cb.find (c = *i) == string::npos; ++i)
+    {
+      if (!space (c))
+        ne = i + 1;
+    }
+
+    try
+    {
+      name = package_name (i == e ? move (d) : string (b, ne));
+    }
+    catch (const invalid_argument& e)
+    {
+      throw invalid_argument (string ("invalid package name: ") + e.what ());
+    }
+
+    if (i != e)
+    try
+    {
+      constraint = version_constraint (string (i, e));
+    }
+    catch (const invalid_argument& e)
+    {
+      throw invalid_argument (string ("invalid package constraint: ") +
+                              e.what ());
+    }
+  }
+
   std::string dependency::
   string () const
   {
@@ -1051,6 +1096,53 @@ namespace bpkg
     return o;
   }
 
+  // requirement_alternatives
+  //
+  requirement_alternatives::
+  requirement_alternatives (const std::string& v)
+  {
+    using std::string;
+
+    // Allow specifying ?* in any order.
+    //
+    size_t n (v.size ());
+    size_t cond ((n > 0 && v[0] == '?') || (n > 1 && v[1] == '?') ? 1 : 0);
+    size_t btim ((n > 0 && v[0] == '*') || (n > 1 && v[1] == '*') ? 1 : 0);
+
+    auto vc (parser::split_comment (v));
+
+    conditional = (cond != 0);
+    buildtime   = (btim != 0);
+    comment     = move (vc.second);
+
+    const string& vl (vc.first);
+
+    string::const_iterator b (vl.begin ());
+    string::const_iterator e (vl.end ());
+
+    if (conditional || buildtime)
+    {
+      string::size_type p (vl.find_first_not_of (spaces, cond + btim));
+      b = p == string::npos ? e : b + p;
+    }
+
+    list_parser lp (b, e, '|');
+    for (string lv (lp.next ()); !lv.empty (); lv = lp.next ())
+      push_back (lv);
+
+    if (empty () && comment.empty ())
+      throw invalid_argument ("empty package requirement specification");
+  }
+
+  std::string requirement_alternatives::
+  string () const
+  {
+    return (conditional
+            ? (buildtime ? "?* " : "? ")
+            : (buildtime ? "* " : "")) +
+           serializer::merge_comment (concatenate (*this, " | "), comment);
+  }
+
   // build_class_term
   //
   build_class_term::
@@ -1537,6 +1629,25 @@ namespace bpkg
     else throw invalid_argument ("invalid test dependency type '" + t + "'");
   }
 
+
+  // test_dependency
+  //
+  test_dependency::
+  test_dependency (std::string v, test_dependency_type t)
+      : type (t)
+  {
+    using std::string;
+
+    buildtime = (v[0] == '*');
+    size_t p (v.find_first_not_of (spaces, buildtime ? 1 : 0));
+
+    if (p == string::npos)
+      throw invalid_argument ("no package name specified");
+
+    static_cast<dependency&> (*this) =
+      dependency (p == 0 ? move (v) : string (v, p));
+  }
+
   // pkg_package_manifest
   //
   static build_class_expr
@@ -2034,34 +2145,14 @@ namespace bpkg
       }
       else if (n == "requires")
       {
-        // Allow specifying ?* in any order.
-        //
-        size_t n (v.size ());
-        size_t cond ((n > 0 && v[0] == '?') || (n > 1 && v[1] == '?') ? 1 : 0);
-        size_t btim ((n > 0 && v[0] == '*') || (n > 1 && v[1] == '*') ? 1 : 0);
-
-        auto vc (parser::split_comment (v));
-
-        const string& vl (vc.first);
-        requirement_alternatives ra (cond != 0, btim != 0, move (vc.second));
-
-        string::const_iterator b (vl.begin ());
-        string::const_iterator e (vl.end ());
-
-        if (ra.conditional || ra.buildtime)
+        try
         {
-          string::size_type p (vl.find_first_not_of (spaces, cond + btim));
-          b = p == string::npos ? e : b + p;
+          m.requirements.push_back (requirement_alternatives (v));
+        }
+        catch (const invalid_argument& e)
+        {
+          bad_value (e.what ());
         }
-
-        list_parser lp (b, e, '|');
-        for (string lv (lp.next ()); !lv.empty (); lv = lp.next ())
-          ra.push_back (lv);
-
-        if (ra.empty () && ra.comment.empty ())
-          bad_value ("empty package requirement specification");
-
-        m.requirements.push_back (move (ra));
       }
       else if (n == "builds")
       {
@@ -2237,68 +2328,29 @@ namespace bpkg
     // Now, when the version manifest value is parsed, we can parse the
     // dependencies and complete their constraints, if requested.
     //
-    auto parse_dependency = [&m, cd, &flag, &bad_value] (string&& d,
-                                                         const char* what)
+    auto complete_constraint = [&m, cd, &flag] (auto&& dep)
     {
-      using iterator = string::const_iterator;
-
-      iterator b (d.begin ());
-      iterator i (b);
-      iterator ne (b); // End of name.
-      iterator e (d.end ());
-
-      // Find end of name (ne).
-      //
-      // Grep for '=<>([~^' in the bpkg source code and update, if changed.
-      //
-      const string cb ("=<>([~^");
-      for (char c; i != e && cb.find (c = *i) == string::npos; ++i)
+      if (dep.constraint)
+      try
       {
-        if (!space (c))
-          ne = i + 1;
-      }
+        version_constraint& vc (*dep.constraint);
 
-      package_name nm;
+        if (!vc.complete () &&
+            flag (package_manifest_flags::forbid_incomplete_dependencies))
+          throw invalid_argument ("$ not allowed");
 
-      try
-      {
-        nm = package_name (i == e ? move (d) : string (b, ne));
+        // Complete the constraint.
+        //
+        if (cd)
+          vc = vc.effective (m.version);
       }
       catch (const invalid_argument& e)
       {
-        bad_value (string ("invalid ") + what + " package name: " +
-                   e.what ());
+        throw invalid_argument (string ("invalid package constraint: ") +
+                                e.what ());
       }
 
-      dependency r;
-
-      if (i == e)
-        r = dependency {move (nm), nullopt};
-      else
-      {
-        try
-        {
-          version_constraint vc (string (i, e));
-
-          if (!vc.complete () &&
-              flag (package_manifest_flags::forbid_incomplete_dependencies))
-            bad_value ("$ not allowed");
-
-          // Complete the constraint.
-          //
-          if (cd)
-            vc = vc.effective (m.version);
-
-          r = dependency {move (nm), move (vc)};
-        }
-        catch (const invalid_argument& e)
-        {
-          bad_value (string ("invalid ") + what + " package constraint: " +
-                     e.what ());
-        }
-      }
-
-      return r;
+      return move (dep);
     };
 
     // Parse the regular dependencies.
@@ -2329,9 +2381,16 @@ namespace bpkg
         b = p == string::npos ? e : b + p;
       }
 
-      list_parser lp (b, e, '|');
-      for (string lv (lp.next ()); !lv.empty (); lv = lp.next ())
-        da.push_back (parse_dependency (move (lv), "prerequisite"));
+      try
+      {
+        list_parser lp (b, e, '|');
+        for (string lv (lp.next ()); !lv.empty (); lv = lp.next ())
+          da.push_back (complete_constraint (dependency (move (lv))));
+      }
+      catch (const invalid_argument& e)
+      {
+        bad_value (e.what ());
+      }
 
       if (da.empty ())
         bad_value ("empty package dependency specification");
@@ -2345,31 +2404,16 @@ namespace bpkg
     {
       nv = move (t); // Restore as bad_value() uses its line/column.
 
-      string& v (nv.value);
-
-      bool b (v[0] == '*');
-      size_t p (v.find_first_not_of (spaces, b ? 1 : 0));
-
-      if (p == string::npos)
-        bad_value ("no " + nv.name + " package name specified");
-
-      dependency d (parse_dependency (p == 0 ? move (v) : string (v, p),
-                                      nv.name.c_str ()));
-
       try
       {
-        m.tests.emplace_back (
-          move (d.name),
-          to_test_dependency_type (nv.name),
-          b,
-          move (d.constraint));
+        m.tests.push_back (
+          complete_constraint (
+            test_dependency (move (nv.value),
+                             to_test_dependency_type (nv.name))));
       }
-      catch (const invalid_argument&)
+      catch (const invalid_argument& e)
       {
-        // to_test_dependency_type() can't throw since the type string is
-        // already validated.
-        //
-        assert (false);
+        bad_value (e.what ());
       }
     }
 
@@ -2746,14 +2790,10 @@ namespace bpkg
                 serializer::merge_comment (concatenate (d, " | "), d.comment));
 
       for (const requirement_alternatives& r: m.requirements)
-        s.next ("requires",
-                (r.conditional
-                 ? (r.buildtime ? "?* " : "? ")
-                 : (r.buildtime ? "* " : "")) +
-                serializer::merge_comment (concatenate (r, " | "), r.comment));
-
-      for (const test_dependency& p: m.tests)
-        s.next (to_string (p.type), p.string ());
+        s.next ("requires", r.string ());
+
+      for (const test_dependency& t: m.tests)
+        s.next (to_string (t.type), t.string ());
 
       for (const build_class_expr& e: m.builds)
         s.next ("builds", serializer::merge_comment (e.string (), e.comment));
diff --git a/libbpkg/manifest.hxx b/libbpkg/manifest.hxx
index 1b62512..b6b9217 100644
--- a/libbpkg/manifest.hxx
+++ b/libbpkg/manifest.hxx
@@ -395,6 +395,17 @@ namespace bpkg
     package_name name;
     butl::optional<version_constraint> constraint;
 
+    dependency () = default;
+    dependency (package_name n, butl::optional<version_constraint> c)
+        : name (std::move (n)), constraint (std::move (c)) {}
+
+    // Parse the dependency string representation in the
+    // `<name> [<version-constraint>]` form. Throw std::invalid_argument if
+    // the value is invalid.
+    //
+    explicit
+    dependency (std::string);
+
     std::string
     string () const;
   };
@@ -434,8 +445,24 @@ namespace bpkg
     requirement_alternatives () = default;
     requirement_alternatives (bool d, bool b, std::string c)
         : conditional (d), buildtime (b), comment (std::move (c)) {}
+
+    // Parse the requirement alternatives string representation in the
+    // `[?] [<requirement> [ '|' <requirement>]*] [; <comment>]` form. Throw
+    // std::invalid_argument if the value is invalid.
+    //
+    explicit LIBBPKG_EXPORT
+    requirement_alternatives (const std::string&);
+
+    LIBBPKG_EXPORT std::string
+    string () const;
   };
 
+  inline std::ostream&
+  operator<< (std::ostream& os, const requirement_alternatives& ra)
+  {
+    return os << ra.string ();
+  }
+
   class build_constraint
   {
   public:
@@ -691,7 +718,7 @@ namespace bpkg
     return os << to_string (t);
   }
 
-  struct test_dependency: dependency
+  struct LIBBPKG_EXPORT test_dependency: dependency
   {
     test_dependency_type type;
     bool buildtime;
@@ -703,6 +730,12 @@ namespace bpkg
                      butl::optional<version_constraint> c)
         : dependency {std::move (n), std::move (c)}, type (t), buildtime (b) {}
 
+    // Parse the test dependency string representation in the
+    // `[*] <name> [<version-constraint>]` form. Throw std::invalid_argument
+    // if the value is invalid.
+    //
+    test_dependency (std::string, test_dependency_type);
+
     inline std::string
     string () const
     {
diff --git a/tests/manifest/testscript b/tests/manifest/testscript
index 4770e96..25e0ae3 100644
--- a/tests/manifest/testscript
+++ b/tests/manifest/testscript
@@ -546,7 +546,7 @@
   {
     : short-name
     :
-    $* <<EOI 2>'stdin:6:10: error: invalid prerequisite package name: length is less than two characters' != 0
+    $* <<EOI 2>'stdin:6:10: error: invalid package name: length is less than two characters' != 0
       : 1
       name: foo
       version: 2.0.0
@@ -557,7 +557,7 @@
 
     : invalid-version-range
     :
-    $* -c <<EOI 2>'stdin:6:10: error: invalid prerequisite package constraint: min version is greater than max version' != 0
+    $* -c <<EOI 2>'stdin:6:10: error: invalid package constraint: min version is greater than max version' != 0
       : 1
       name: foo
       version: 2.0.0
@@ -628,7 +628,7 @@
         license: LGPLv2
         depends: bar ~$
         EOI
-        stdin:6:10: error: invalid prerequisite package constraint: dependent version is not standard
+        stdin:6:10: error: invalid package constraint: dependent version is not standard
         EOE
 
       : latest-snapshot
@@ -668,7 +668,7 @@
   {
     : short-name
     :
-    $* <<EOI 2>'stdin:6:8: error: invalid tests package name: length is less than two characters' != 0
+    $* <<EOI 2>'stdin:6:8: error: invalid package name: length is less than two characters' != 0
       : 1
       name: foo
       version: 2.0.0
@@ -679,7 +679,7 @@
 
     : invalid-version-range-incomplete
     :
-    $* -c <<EOI 2>'stdin:6:8: error: invalid tests package constraint: min version is greater than max version' != 0
+    $* -c <<EOI 2>'stdin:6:8: error: invalid package constraint: min version is greater than max version' != 0
       : 1
       name: foo
       version: 2.0.0
@@ -690,7 +690,7 @@
 
     : invalid-version-range
     :
-    $* -c <<EOI 2>'stdin:6:8: error: invalid tests package constraint: min version is greater than max version' != 0
+    $* -c <<EOI 2>'stdin:6:8: error: invalid package constraint: min version is greater than max version' != 0
       : 1
       name: foo
       version: 2.0.0
@@ -701,7 +701,7 @@
 
     : invalid-version
     :
-    $* -c <<EOI 2>'stdin:6:8: error: invalid tests package constraint: invalid version: equal version endpoints are earliest' != 0
+    $* -c <<EOI 2>'stdin:6:8: error: invalid package constraint: invalid version: equal version endpoints are earliest' != 0
       : 1
       name: foo
       version: 2.0.0
@@ -712,7 +712,7 @@
 
     : no-name
     :
-    $* <<EOI 2>'stdin:6:8: error: no tests package name specified' != 0
+    $* <<EOI 2>'stdin:6:8: error: no package name specified' != 0
       : 1
       name: foo
       version: 2.0.0
@@ -765,7 +765,7 @@
         license: LGPLv2
         tests: bar ~$
         EOI
-        stdin:6:8: error: invalid tests package constraint: dependent version is not standard
+        stdin:6:8: error: invalid package constraint: dependent version is not standard
         EOE
     }
 
@@ -871,7 +871,7 @@
     {
       : incomplete
       :
-      $* <<EOI 2>'stdin:8:10: error: $ not allowed' != 0
+      $* <<EOI 2>'stdin:8:10: error: invalid package constraint: $ not allowed' != 0
         : 1
         sha256sum: a2b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855
         :