[vcpkg] Consolidate specifier parsing

4 files changed, 124 insertions, 94 deletions

diff --git a/toolsrc/src/PackageSpec.cpp b/toolsrc/src/PackageSpec.cpp
index a7e5648cd..e243f4d89 100644
--- a/toolsrc/src/PackageSpec.cpp
+++ b/toolsrc/src/PackageSpec.cpp
@@ -13,52 +13,16 @@ namespace vcpkg
     ExpectedT<FullPackageSpec, PackageSpecParseResult> FullPackageSpec::from_string(const std::string& spec_as_string,
                                                                                     const Triplet& default_triplet)
     {
-        auto pos = spec_as_string.find(':');
-        auto pos_l_bracket = spec_as_string.find('[');
-        auto pos_r_bracket = spec_as_string.find(']');
-
-        FullPackageSpec f;
-        if (pos == std::string::npos && pos_l_bracket == std::string::npos)
-        {
-            f.package_spec =
-                PackageSpec::from_name_and_triplet(spec_as_string, default_triplet).value_or_exit(VCPKG_LINE_INFO);
-            return f;
-        }
-        else if (pos == std::string::npos)
-        {
-            if (pos_r_bracket == std::string::npos || pos_l_bracket >= pos_r_bracket)
-            {
-                return PackageSpecParseResult::INVALID_CHARACTERS;
-            }
-            const std::string name = spec_as_string.substr(0, pos_l_bracket);
-            f.package_spec = PackageSpec::from_name_and_triplet(name, default_triplet).value_or_exit(VCPKG_LINE_INFO);
-            f.features = parse_comma_list(spec_as_string.substr(pos_l_bracket + 1, pos_r_bracket - pos_l_bracket - 1));
-            return f;
-        }
-        else if (pos_l_bracket == std::string::npos && pos_r_bracket == std::string::npos)
-        {
-            const std::string name = spec_as_string.substr(0, pos);
-            const Triplet triplet = Triplet::from_canonical_name(spec_as_string.substr(pos + 1));
-            f.package_spec = PackageSpec::from_name_and_triplet(name, triplet).value_or_exit(VCPKG_LINE_INFO);
-        }
-        else
+        auto res = ParsedSpecifier::from_string(spec_as_string);
+        if (auto p = res.get())
         {
-            if (pos_r_bracket == std::string::npos || pos_l_bracket >= pos_r_bracket)
-            {
-                return PackageSpecParseResult::INVALID_CHARACTERS;
-            }
-            const std::string name = spec_as_string.substr(0, pos_l_bracket);
-            f.features = parse_comma_list(spec_as_string.substr(pos_l_bracket + 1, pos_r_bracket - pos_l_bracket - 1));
-            const Triplet triplet = Triplet::from_canonical_name(spec_as_string.substr(pos + 1));
-            f.package_spec = PackageSpec::from_name_and_triplet(name, triplet).value_or_exit(VCPKG_LINE_INFO);
+            FullPackageSpec fspec;
+            Triplet t = p->triplet.empty() ? default_triplet : Triplet::from_canonical_name(p->triplet);
+            fspec.package_spec = PackageSpec::from_name_and_triplet(p->name, t).value_or_exit(VCPKG_LINE_INFO);
+            fspec.features = std::move(p->features);
+            return fspec;
         }
-
-        auto pos2 = spec_as_string.find(':', pos + 1);
-        if (pos2 != std::string::npos)
-        {
-            return PackageSpecParseResult::TOO_MANY_COLONS;
-        }
-        return f;
+        return res.error();
     }
 
     ExpectedT<PackageSpec, PackageSpecParseResult> PackageSpec::from_name_and_triplet(const std::string& name,
@@ -93,4 +57,53 @@ namespace vcpkg
     }
 
     bool operator!=(const PackageSpec& left, const PackageSpec& right) { return !(left == right); }
+
+    ExpectedT<ParsedSpecifier, PackageSpecParseResult> ParsedSpecifier::from_string(const std::string& input)
+    {
+        auto pos = input.find(':');
+        auto pos_l_bracket = input.find('[');
+        auto pos_r_bracket = input.find(']');
+
+        ParsedSpecifier f;
+        if (pos == std::string::npos && pos_l_bracket == std::string::npos)
+        {
+            f.name = input;
+            return f;
+        }
+        else if (pos == std::string::npos)
+        {
+            if (pos_r_bracket == std::string::npos || pos_l_bracket >= pos_r_bracket)
+            {
+                return PackageSpecParseResult::INVALID_CHARACTERS;
+            }
+            const std::string name = input.substr(0, pos_l_bracket);
+            f.name = name;
+            f.features = parse_comma_list(input.substr(pos_l_bracket + 1, pos_r_bracket - pos_l_bracket - 1));
+            return f;
+        }
+        else if (pos_l_bracket == std::string::npos && pos_r_bracket == std::string::npos)
+        {
+            const std::string name = input.substr(0, pos);
+            f.triplet = input.substr(pos + 1);
+            f.name = name;
+        }
+        else
+        {
+            if (pos_r_bracket == std::string::npos || pos_l_bracket >= pos_r_bracket)
+            {
+                return PackageSpecParseResult::INVALID_CHARACTERS;
+            }
+            const std::string name = input.substr(0, pos_l_bracket);
+            f.features = parse_comma_list(input.substr(pos_l_bracket + 1, pos_r_bracket - pos_l_bracket - 1));
+            f.triplet = input.substr(pos + 1);
+            f.name = name;
+        }
+
+        auto pos2 = input.find(':', pos + 1);
+        if (pos2 != std::string::npos)
+        {
+            return PackageSpecParseResult::TOO_MANY_COLONS;
+        }
+        return f;
+    }
 }
diff --git a/toolsrc/src/SourceParagraph.cpp b/toolsrc/src/SourceParagraph.cpp
index 2aab7c572..f9ae6854a 100644
--- a/toolsrc/src/SourceParagraph.cpp
+++ b/toolsrc/src/SourceParagraph.cpp
@@ -1,5 +1,6 @@
 #include "pch.h"
 
+#include "PackageSpec.h"
 #include "SourceParagraph.h"
 #include "Triplet.h"
 #include "vcpkg_Checks.h"
@@ -28,7 +29,11 @@ namespace vcpkg
     static span<const std::string> get_list_of_valid_fields()
     {
         static const std::string valid_fields[] = {
-            Fields::SOURCE, Fields::VERSION, Fields::DESCRIPTION, Fields::MAINTAINER, Fields::BUILD_DEPENDS,
+            Fields::SOURCE,
+            Fields::VERSION,
+            Fields::DESCRIPTION,
+            Fields::MAINTAINER,
+            Fields::BUILD_DEPENDS,
         };
 
         return valid_fields;
@@ -154,21 +159,20 @@ namespace vcpkg
 
     Features parse_feature_list(const std::string& name)
     {
-        Features f;
-        int end = (int)name.find(']');
-        if (end != std::string::npos)
+        auto maybe_spec = ParsedSpecifier::from_string(name);
+        if (auto spec = maybe_spec.get())
         {
-            int start = (int)name.find('[');
+            Checks::check_exit(
+                VCPKG_LINE_INFO, spec->triplet.empty(), "error: triplet not allowed in specifier: %s", name);
 
-            auto feature_name_list = name.substr(start + 1, end - start - 1);
-            f.name = name.substr(0, start);
-            f.features = parse_comma_list(feature_name_list);
+            Features f;
+            f.name = spec->name;
+            f.features = spec->features;
+            return f;
         }
-        else
-        {
-            f.name = name;
-        }
-        return f;
+
+        Checks::exit_with_message(
+            VCPKG_LINE_INFO, "error while parsing feature list: %s: %s", to_string(maybe_spec.error()), name);
     }
 
     Dependency Dependency::parse_dependency(std::string name, std::string qualifier)
diff --git a/toolsrc/src/tests_paragraph.cpp b/toolsrc/src/tests_paragraph.cpp
index 3f2760c22..1fd950e19 100644
--- a/toolsrc/src/tests_paragraph.cpp
+++ b/toolsrc/src/tests_paragraph.cpp
@@ -385,42 +385,53 @@ namespace UnitTest1
             Assert::AreEqual("a, b, c", pghs[0]["Depends"].c_str());
         }
 
-        TEST_METHOD(package_spec_parse)
+        TEST_METHOD(parsed_specifier_from_string)
         {
-            vcpkg::ExpectedT<vcpkg::FullPackageSpec, vcpkg::PackageSpecParseResult> spec =
-                vcpkg::FullPackageSpec::from_string("zlib", vcpkg::Triplet::X86_WINDOWS);
-            Assert::AreEqual(vcpkg::PackageSpecParseResult::SUCCESS, spec.error());
-            Assert::AreEqual("zlib", spec.get()->package_spec.name().c_str());
-            Assert::AreEqual(vcpkg::Triplet::X86_WINDOWS.canonical_name(),
-                             spec.get()->package_spec.triplet().canonical_name());
+            auto maybe_spec = vcpkg::ParsedSpecifier::from_string("zlib");
+            Assert::AreEqual(vcpkg::PackageSpecParseResult::SUCCESS, maybe_spec.error());
+            auto spec = maybe_spec.get();
+            Assert::AreEqual("zlib", spec->name.c_str());
+            Assert::AreEqual(size_t(0), spec->features.size());
+            Assert::AreEqual("", spec->triplet.c_str());
         }
 
-        TEST_METHOD(package_spec_parse_with_arch)
+        TEST_METHOD(parsed_specifier_from_string_with_triplet)
         {
-            vcpkg::ExpectedT<vcpkg::FullPackageSpec, vcpkg::PackageSpecParseResult> spec =
-                vcpkg::FullPackageSpec::from_string("zlib:x64-uwp", vcpkg::Triplet::X86_WINDOWS);
-            Assert::AreEqual(vcpkg::PackageSpecParseResult::SUCCESS, spec.error());
-            Assert::AreEqual("zlib", spec.get()->package_spec.name().c_str());
-            Assert::AreEqual(vcpkg::Triplet::X64_UWP.canonical_name(),
-                             spec.get()->package_spec.triplet().canonical_name());
+            auto maybe_spec = vcpkg::ParsedSpecifier::from_string("zlib:x64-uwp");
+            Assert::AreEqual(vcpkg::PackageSpecParseResult::SUCCESS, maybe_spec.error());
+            auto spec = maybe_spec.get();
+            Assert::AreEqual("zlib", spec->name.c_str());
+            Assert::AreEqual("x64-uwp", spec->triplet.c_str());
         }
 
-        TEST_METHOD(package_spec_parse_with_multiple_colon)
+        TEST_METHOD(parsed_specifier_from_string_with_colons)
         {
-            auto ec = vcpkg::FullPackageSpec::from_string("zlib:x86-uwp:", vcpkg::Triplet::X86_WINDOWS).error();
+            auto ec = vcpkg::ParsedSpecifier::from_string("zlib:x86-uwp:").error();
             Assert::AreEqual(vcpkg::PackageSpecParseResult::TOO_MANY_COLONS, ec);
         }
 
-        TEST_METHOD(package_spec_feature_parse_with_arch)
+        TEST_METHOD(parsed_specifier_from_string_with_feature)
         {
-            vcpkg::ExpectedT<vcpkg::FullPackageSpec, vcpkg::PackageSpecParseResult> spec =
-                vcpkg::FullPackageSpec::from_string("zlib[feature]:x64-uwp", vcpkg::Triplet::X86_WINDOWS);
-            Assert::AreEqual(vcpkg::PackageSpecParseResult::SUCCESS, spec.error());
-            Assert::AreEqual("zlib", spec.get()->package_spec.name().c_str());
-            Assert::IsTrue(spec.get()->features.size() == 1);
-            Assert::AreEqual("feature", spec.get()->features.front().c_str());
-            Assert::AreEqual(vcpkg::Triplet::X64_UWP.canonical_name(),
-                             spec.get()->package_spec.triplet().canonical_name());
+            auto maybe_spec = vcpkg::ParsedSpecifier::from_string("zlib[feature]:x64-uwp");
+            Assert::AreEqual(vcpkg::PackageSpecParseResult::SUCCESS, maybe_spec.error());
+            auto spec = maybe_spec.get();
+            Assert::AreEqual("zlib", spec->name.c_str());
+            Assert::IsTrue(spec->features.size() == 1);
+            Assert::AreEqual("feature", spec->features.front().c_str());
+            Assert::AreEqual("x64-uwp", spec->triplet.c_str());
+        }
+
+        TEST_METHOD(parsed_specifier_from_string_with_many_features)
+        {
+            auto maybe_spec = vcpkg::ParsedSpecifier::from_string("zlib[0, 1,2]");
+            Assert::AreEqual(vcpkg::PackageSpecParseResult::SUCCESS, maybe_spec.error());
+            auto spec = maybe_spec.get();
+            Assert::AreEqual("zlib", spec->name.c_str());
+            Assert::IsTrue(spec->features.size() == 3);
+            Assert::AreEqual("0", spec->features[0].c_str());
+            Assert::AreEqual("1", spec->features[1].c_str());
+            Assert::AreEqual("2", spec->features[2].c_str());
+            Assert::AreEqual("", spec->triplet.c_str());
         }
 
         TEST_METHOD(utf8_to_utf16)
diff --git a/toolsrc/src/vcpkg_Dependencies.cpp b/toolsrc/src/vcpkg_Dependencies.cpp
index c84ca73f3..799bca439 100644
--- a/toolsrc/src/vcpkg_Dependencies.cpp
+++ b/toolsrc/src/vcpkg_Dependencies.cpp
@@ -363,23 +363,25 @@ namespace vcpkg::Dependencies
         std::vector<FeatureSpec> f_specs;
         for (auto&& depend : depends)
         {
-            int end = (int)depend.find(']');
-            if (end != std::string::npos)
+            auto maybe_spec = ParsedSpecifier::from_string(depend);
+            if (auto spec = maybe_spec.get())
             {
-                int start = (int)depend.find('[');
+                Checks::check_exit(VCPKG_LINE_INFO,
+                                   spec->triplet.empty(),
+                                   "error: triplets cannot currently be specified in this context: %s",
+                                   depend);
+                PackageSpec pspec =
+                    PackageSpec::from_name_and_triplet(spec->name, triplet).value_or_exit(VCPKG_LINE_INFO);
 
-                auto feature_name = depend.substr(start + 1, end - start - 1);
-                auto package_name = depend.substr(0, start);
-                auto p_spec = PackageSpec::from_name_and_triplet(package_name, triplet).value_or_exit(VCPKG_LINE_INFO);
-                auto feature_spec = FeatureSpec{p_spec, feature_name};
-                f_specs.emplace_back(std::move(feature_spec));
+                for (auto&& feature : spec->features)
+                    f_specs.push_back(FeatureSpec{pspec, feature});
+
+                if (spec->features.empty()) f_specs.push_back(FeatureSpec{pspec, ""});
             }
             else
             {
-                auto p_spec = PackageSpec::from_name_and_triplet(depend, triplet).value_or_exit(VCPKG_LINE_INFO);
-
-                auto feature_spec = FeatureSpec{p_spec, ""};
-                f_specs.emplace_back(std::move(feature_spec));
+                Checks::exit_with_message(
+                    VCPKG_LINE_INFO, "error while parsing feature list: %s: %s", to_string(maybe_spec.error()), depend);
             }
         }
         return f_specs;