[vcpkg manifest] Add documentation! (#13488)

* [vcpkg docs] add docs for manifest files

These are just for the maintainer docs,
not user docs.

* [vcpkg] EBNF-ify platform expression parsing

this modifies nothing about what strings are accepted or rejected,
it just moves stuff around.

also adds tests.

* [vcpkg docs] add manifest mode example

* [wip] docs for augustin

also fix tabs

* [vcpkg manifest] switch to using maps for features

* Apply suggestions from code review

* un-experimentize format-manifest

* flesh out the user manifest mode docs

* CRs

* billy CRs

* final personal pass-thru

7 files changed, 213 insertions, 37 deletions

diff --git a/toolsrc/src/vcpkg-test/commands.cpp b/toolsrc/src/vcpkg-test/commands.cpp
index c19fd30ea..38346291c 100644
--- a/toolsrc/src/vcpkg-test/commands.cpp
+++ b/toolsrc/src/vcpkg-test/commands.cpp
@@ -55,11 +55,11 @@ TEST_CASE ("get_available_paths_commands works", "[commands]")
         "autocomplete",
         "hash",
         "fetch",
+        "format-manifest",
         "x-ci-clean",
         "x-history",
         "x-package-info",
         "x-vsinstances",
-        "x-format-manifest",
         });
 }
 
diff --git a/toolsrc/src/vcpkg-test/manifests.cpp b/toolsrc/src/vcpkg-test/manifests.cpp
index 9a0ca94e2..0d5c9782a 100644
--- a/toolsrc/src/vcpkg-test/manifests.cpp
+++ b/toolsrc/src/vcpkg-test/manifests.cpp
@@ -250,7 +250,7 @@ TEST_CASE ("SourceParagraph manifest empty supports", "[manifests]")
 
 TEST_CASE ("Serialize all the ports", "[manifests]")
 {
-    std::vector<std::string> args_list = {"x-format-manifest"};
+    std::vector<std::string> args_list = {"format-manifest"};
     auto& fs = Files::get_real_filesystem();
     auto args = VcpkgCmdArguments::create_from_arg_sequence(args_list.data(), args_list.data() + args_list.size());
     VcpkgPaths paths{fs, args};
diff --git a/toolsrc/src/vcpkg-test/platform-expression.cpp b/toolsrc/src/vcpkg-test/platform-expression.cpp
new file mode 100644
index 000000000..aef999230
--- /dev/null
+++ b/toolsrc/src/vcpkg-test/platform-expression.cpp
@@ -0,0 +1,80 @@
+#include <catch2/catch.hpp>
+
+#include <vcpkg/platform-expression.h>
+
+using vcpkg::StringView;
+using namespace vcpkg::PlatformExpression;
+
+static vcpkg::ExpectedS<Expr> parse_expr(StringView s)
+{
+    return parse_platform_expression(s, MultipleBinaryOperators::Deny);
+}
+
+TEST_CASE ("platform-expression-identifier", "[platform-expression]")
+{
+    auto m_expr = parse_expr("windows");
+    REQUIRE(m_expr);
+    auto& expr = *m_expr.get();
+
+    CHECK(expr.evaluate({{"VCPKG_CMAKE_SYSTEM_NAME", ""}}));
+    CHECK(expr.evaluate({{"VCPKG_CMAKE_SYSTEM_NAME", "WindowsStore"}}));
+    CHECK_FALSE(expr.evaluate({{"VCPKG_CMAKE_SYSTEM_NAME", "Linux"}}));
+    CHECK_FALSE(expr.evaluate({{"VCPKG_CMAKE_SYSTEM_NAME", "Darwin"}}));
+}
+
+TEST_CASE ("platform-expression-not", "[platform-expression]")
+{
+    auto m_expr = parse_expr("!windows");
+    REQUIRE(m_expr);
+    auto& expr = *m_expr.get();
+
+    CHECK_FALSE(expr.evaluate({{"VCPKG_CMAKE_SYSTEM_NAME", ""}}));
+    CHECK_FALSE(expr.evaluate({{"VCPKG_CMAKE_SYSTEM_NAME", "WindowsStore"}}));
+    CHECK(expr.evaluate({{"VCPKG_CMAKE_SYSTEM_NAME", "Linux"}}));
+    CHECK(expr.evaluate({{"VCPKG_CMAKE_SYSTEM_NAME", "Darwin"}}));
+}
+
+TEST_CASE ("platform-expression-and", "[platform-expression]")
+{
+    auto m_expr = parse_expr("!windows & !arm");
+    REQUIRE(m_expr);
+    auto& expr = *m_expr.get();
+
+    CHECK_FALSE(expr.evaluate({{"VCPKG_CMAKE_SYSTEM_NAME", ""}}));
+    CHECK_FALSE(expr.evaluate({{"VCPKG_CMAKE_SYSTEM_NAME", "WindowsStore"}}));
+    CHECK(expr.evaluate({{"VCPKG_CMAKE_SYSTEM_NAME", "Linux"}}));
+    CHECK_FALSE(expr.evaluate({
+        {"VCPKG_CMAKE_SYSTEM_NAME", "Linux"},
+        {"VCPKG_TARGET_ARCHITECTURE", "arm"},
+    }));
+}
+
+TEST_CASE ("platform-expression-or", "[platform-expression]")
+{
+    auto m_expr = parse_expr("!windows | arm");
+    REQUIRE(m_expr);
+    auto& expr = *m_expr.get();
+
+    CHECK_FALSE(expr.evaluate({{"VCPKG_CMAKE_SYSTEM_NAME", ""}}));
+    CHECK(expr.evaluate({{"VCPKG_CMAKE_SYSTEM_NAME", ""}, {"VCPKG_TARGET_ARCHITECTURE", "arm"}}));
+    CHECK(expr.evaluate({{"VCPKG_CMAKE_SYSTEM_NAME", "Linux"}}));
+}
+
+TEST_CASE ("weird platform-expressions whitespace", "[platform-expression]")
+{
+    auto m_expr = parse_expr(" ! \t  windows \n| arm \r");
+    REQUIRE(m_expr);
+    auto& expr = *m_expr.get();
+
+    CHECK_FALSE(expr.evaluate({{"VCPKG_CMAKE_SYSTEM_NAME", ""}}));
+    CHECK(expr.evaluate({{"VCPKG_CMAKE_SYSTEM_NAME", ""}, {"VCPKG_TARGET_ARCHITECTURE", "arm"}}));
+    CHECK(expr.evaluate({{"VCPKG_CMAKE_SYSTEM_NAME", "Linux"}}));
+}
+
+TEST_CASE ("no mixing &, | in platform expressions", "[platform-expression]")
+{
+    auto m_expr = parse_expr("windows & arm | linux");
+    CHECK_FALSE(m_expr);
+    m_expr = parse_expr("windows | !arm & linux");
+    CHECK_FALSE(m_expr);
+}
diff --git a/toolsrc/src/vcpkg/commands.cpp b/toolsrc/src/vcpkg/commands.cpp
index d01e1973e..6e87c1774 100644
--- a/toolsrc/src/vcpkg/commands.cpp
+++ b/toolsrc/src/vcpkg/commands.cpp
@@ -93,7 +93,7 @@ namespace vcpkg::Commands
             {"x-package-info", &info},
             {"x-history", &porthistory},
             {"x-vsinstances", &vsinstances},
-            {"x-format-manifest", &format_manifest},
+            {"format-manifest", &format_manifest},
         };
         return t;
     }
diff --git a/toolsrc/src/vcpkg/commands.format-manifest.cpp b/toolsrc/src/vcpkg/commands.format-manifest.cpp
index 24beae0d8..1be6ecbeb 100644
--- a/toolsrc/src/vcpkg/commands.format-manifest.cpp
+++ b/toolsrc/src/vcpkg/commands.format-manifest.cpp
@@ -185,7 +185,7 @@ namespace vcpkg::Commands::FormatManifest
     };
 
     const CommandStructure COMMAND_STRUCTURE = {
-        create_example_string(R"###(x-format-manifest --all)###"),
+        create_example_string(R"###(format-manifest --all)###"),
         0,
         SIZE_MAX,
         {FORMAT_SWITCHES, {}, {}},
@@ -204,7 +204,7 @@ namespace vcpkg::Commands::FormatManifest
 
         if (!format_all && convert_control)
         {
-            System::print2(System::Color::warning, R"(x-format-manifest was passed '--convert-control' without '--all'.
+            System::print2(System::Color::warning, R"(format-manifest was passed '--convert-control' without '--all'.
     This doesn't do anything:
     we will automatically convert all control files passed explicitly.)");
         }
diff --git a/toolsrc/src/vcpkg/platform-expression.cpp b/toolsrc/src/vcpkg/platform-expression.cpp
index 3490dbbe2..1d38dbc05 100644
--- a/toolsrc/src/vcpkg/platform-expression.cpp
+++ b/toolsrc/src/vcpkg/platform-expression.cpp
@@ -106,10 +106,10 @@ namespace vcpkg::PlatformExpression
                 return multiple_binary_operators == MultipleBinaryOperators::Allow;
             }
 
+            // top-level-platform-expression = optional-whitespace, platform-expression
             PlatformExpression::Expr parse()
             {
                 skip_whitespace();
-
                 auto res = expr();
 
                 if (!at_eof())
@@ -121,64 +121,71 @@ namespace vcpkg::PlatformExpression
             }
 
         private:
-            // <platform-expression.and>
-            //     <platform-expression.not>
-            //     <platform-expression.and> & <platform-expression.not>
-            // <platform-expression.or>
-            //     <platform-expression.not>
-            //     <platform-expression.or> | <platform-expression.not>
-
+            // identifier-character =
+            // | lowercase-alpha
+            // | digit ;
             static bool is_identifier_char(char32_t ch) { return is_lower_alpha(ch) || is_ascii_digit(ch); }
 
-            // <platform-expression>:
-            //     <platform-expression.not>
-            //     <platform-expression.and>
-            //     <platform-expression.or>
+            // platform-expression =
+            // | platform-expression-not
+            // | platform-expression-and
+            // | platform-expression-or ;
             std::unique_ptr<ExprImpl> expr()
             {
+                // this is the common prefix of all the variants
+                // platform-expression-not,
                 auto result = expr_not();
 
                 switch (cur())
                 {
                     case '|':
                     {
+                        // { "|", optional-whitespace, platform-expression-not }
                         return expr_binary<'|', '&'>(std::make_unique<ExprImpl>(ExprKind::op_or, std::move(result)));
                     }
                     case '&':
                     {
+                        // { "&", optional-whitespace, platform-expression-not }
                         return expr_binary<'&', '|'>(std::make_unique<ExprImpl>(ExprKind::op_and, std::move(result)));
                     }
                     default: return result;
                 }
             }
 
-            // <platform-expression.simple>:
-            //     ( <platform-expression> )
-            //     <platform-expression.identifier>
+            // platform-expression-simple =
+            // | platform-expression-identifier
+            // | "(", optional-whitespace, platform-expression, ")", optional-whitespace ;
             std::unique_ptr<ExprImpl> expr_simple()
             {
                 if (cur() == '(')
                 {
+                    // "(",
                     next();
+                    // optional-whitespace,
                     skip_whitespace();
+                    // platform-expression,
                     auto result = expr();
                     if (cur() != ')')
                     {
                         add_error("missing closing )");
                         return result;
                     }
+                    // ")",
                     next();
+                    // optional-whitespace
                     skip_whitespace();
                     return result;
                 }
 
+                // platform-expression-identifier
                 return expr_identifier();
             }
 
-            // <platform-expression.identifier>:
-            //     A lowercase alpha-numeric string
+            // platform-expression-identifier =
+            // | identifier-character, { identifier-character }, optional-whitespace ;
             std::unique_ptr<ExprImpl> expr_identifier()
             {
+                // identifier-character, { identifier-character },
                 std::string name = match_zero_or_more(is_identifier_char).to_string();
 
                 if (name.empty())
@@ -186,25 +193,39 @@ namespace vcpkg::PlatformExpression
                     add_error("unexpected character in logic expression");
                 }
 
+                // optional-whitespace
                 skip_whitespace();
+
                 return std::make_unique<ExprImpl>(ExprKind::identifier, std::move(name));
             }
 
-            // <platform-expression.not>:
-            //     <platform-expression.simple>
-            //     ! <platform-expression.simple>
+            // platform-expression-not =
+            // | platform-expression-simple
+            // | "!", optional-whitespace, platform-expression-simple ;
             std::unique_ptr<ExprImpl> expr_not()
             {
                 if (cur() == '!')
                 {
+                    // "!",
                     next();
+                    // optional-whitespace,
                     skip_whitespace();
+                    // platform-expression-simple
                     return std::make_unique<ExprImpl>(ExprKind::op_not, expr_simple());
                 }
 
+                // platform-expression-simple
                 return expr_simple();
             }
 
+            // platform-expression-and =
+            // | platform-expression-not, { "&", optional-whitespace, platform-expression-not } ;
+            //
+            // platform-expression-or =
+            // | platform-expression-not, { "|", optional-whitespace, platform-expression-not } ;
+            //
+            // already taken care of by the caller: platform-expression-not
+            // so we start at either "&" or "|"
             template<char oper, char other>
             std::unique_ptr<ExprImpl> expr_binary(std::unique_ptr<ExprImpl>&& seed)
             {
@@ -213,10 +234,13 @@ namespace vcpkg::PlatformExpression
                     // Support chains of the operator to avoid breaking backwards compatibility
                     do
                     {
+                        // "&" or "|",
                         next();
                     } while (allow_multiple_binary_operators() && cur() == oper);
 
+                    // optional-whitespace,
                     skip_whitespace();
+                    // platform-expression-not, (go back to start of repetition)
                     seed->exprs.push_back(expr_not());
                 } while (cur() == oper);
 
@@ -225,7 +249,6 @@ namespace vcpkg::PlatformExpression
                     add_error("mixing & and | is not allowed; use () to specify order of operations");
                 }
 
-                skip_whitespace();
                 return std::move(seed);
             }
         };
diff --git a/toolsrc/src/vcpkg/sourceparagraph.cpp b/toolsrc/src/vcpkg/sourceparagraph.cpp
index e13fcfdb7..9300c8e6c 100644
--- a/toolsrc/src/vcpkg/sourceparagraph.cpp
+++ b/toolsrc/src/vcpkg/sourceparagraph.cpp
@@ -447,6 +447,11 @@ namespace vcpkg
     constexpr StringLiteral DependencyDeserializer::DEFAULT_FEATURES;
     constexpr StringLiteral DependencyDeserializer::PLATFORM;
 
+    // reasoning for these two distinct types -- FeatureDeserializer and ArrayFeatureDeserializer:
+    // `"features"` may be defined in one of two ways:
+    // - An array of feature objects, which contains the `"name"` field
+    // - An object mapping feature names to feature objects, which do not contain the `"name"` field
+    // `ArrayFeatureDeserializer` is used for the former, `FeatureDeserializer` is used for the latter.
     struct FeatureDeserializer : Json::IDeserializer<std::unique_ptr<FeatureParagraph>>
     {
         virtual StringView type_name() const override { return "a feature"; }
@@ -457,7 +462,7 @@ namespace vcpkg
 
         virtual Span<const StringView> valid_fields() const override
         {
-            static const StringView t[] = {NAME, DESCRIPTION, DEPENDENCIES};
+            static const StringView t[] = {DESCRIPTION, DEPENDENCIES};
             return t;
         }
 
@@ -465,6 +470,7 @@ namespace vcpkg
                                                                          const Json::Object& obj) override
         {
             auto feature = std::make_unique<FeatureParagraph>();
+            feature->name = std::move(name);
 
             for (const auto& el : obj)
             {
@@ -474,8 +480,7 @@ namespace vcpkg
                 }
             }
 
-            r.required_object_field(type_name(), obj, NAME, feature->name, Json::IdentifierDeserializer{});
-            r.required_object_field(type_name(), obj, DESCRIPTION, feature->description, Json::ParagraphDeserializer{});
+            r.required_object_field("a feature", obj, DESCRIPTION, feature->description, Json::ParagraphDeserializer{});
             r.optional_object_field(
                 obj,
                 DEPENDENCIES,
@@ -484,12 +489,85 @@ namespace vcpkg
 
             return std::move(feature);
         }
+
+        FeatureDeserializer() = default;
+        FeatureDeserializer(std::string&& s) : name(std::move(s)) { }
+
+        std::string name;
+    };
+
+    struct ArrayFeatureDeserializer : Json::IDeserializer<std::unique_ptr<FeatureParagraph>>
+    {
+        virtual StringView type_name() const override { return "a feature"; }
+
+        virtual Span<const StringView> valid_fields() const override
+        {
+            static const StringView t[] = {
+                FeatureDeserializer::NAME,
+                FeatureDeserializer::DESCRIPTION,
+                FeatureDeserializer::DEPENDENCIES,
+            };
+            return t;
+        }
+
+        virtual Optional<std::unique_ptr<FeatureParagraph>> visit_object(Json::Reader& r,
+                                                                         const Json::Object& obj) override
+        {
+            std::string name;
+            r.required_object_field(type_name(), obj, FeatureDeserializer::NAME, name, Json::IdentifierDeserializer{});
+            return FeatureDeserializer{std::move(name)}.visit_object(r, obj);
+        }
     };
 
     constexpr StringLiteral FeatureDeserializer::NAME;
     constexpr StringLiteral FeatureDeserializer::DESCRIPTION;
     constexpr StringLiteral FeatureDeserializer::DEPENDENCIES;
 
+    struct FeaturesFieldDeserializer : Json::IDeserializer<std::vector<std::unique_ptr<FeatureParagraph>>>
+    {
+        virtual StringView type_name() const override { return "a features field"; }
+
+        virtual Span<const StringView> valid_fields() const override { return {}; }
+
+        virtual Optional<std::vector<std::unique_ptr<FeatureParagraph>>> visit_array(Json::Reader& r,
+                                                                                     const Json::Array& arr) override
+        {
+            return r.visit_value(arr,
+                                 Json::ArrayDeserializer<ArrayFeatureDeserializer>{"an array of feature objects",
+                                                                                   Json::AllowEmpty::Yes});
+        }
+
+        virtual Optional<std::vector<std::unique_ptr<FeatureParagraph>>> visit_object(Json::Reader& r,
+                                                                                      const Json::Object& obj) override
+        {
+            std::vector<std::unique_ptr<FeatureParagraph>> res;
+            std::vector<std::string> extra_fields;
+
+            FeatureDeserializer deserializer;
+            for (const auto& pr : obj)
+            {
+                if (!Json::IdentifierDeserializer::is_ident(pr.first))
+                {
+                    extra_fields.push_back(pr.first.to_string());
+                    continue;
+                }
+                deserializer.name.assign(pr.first.begin(), pr.first.end());
+                auto field = r.visit_map_field(pr.first, pr.second, deserializer);
+                if (auto p = field.get())
+                {
+                    res.push_back(std::move(*p));
+                }
+            }
+
+            if (!extra_fields.empty())
+            {
+                r.add_extra_fields_error(type_name(), std::move(extra_fields));
+            }
+
+            return std::move(res);
+        }
+    };
+
     static constexpr StringView EXPRESSION_WORDS[] = {
         "WITH",
         "AND",
@@ -717,11 +795,7 @@ namespace vcpkg
                                     Json::ArrayDeserializer<Json::IdentifierDeserializer>{"an array of identifiers",
                                                                                           Json::AllowEmpty::Yes});
 
-            r.optional_object_field(
-                obj,
-                FEATURES,
-                control_file->feature_paragraphs,
-                Json::ArrayDeserializer<FeatureDeserializer>{"an array of feature definitions", Json::AllowEmpty::Yes});
+            r.optional_object_field(obj, FEATURES, control_file->feature_paragraphs, FeaturesFieldDeserializer{});
 
             canonicalize(*control_file);
             return std::move(control_file);
@@ -1001,16 +1075,15 @@ namespace vcpkg
 
         if (!scf.feature_paragraphs.empty() || debug)
         {
-            auto& arr = obj.insert(ManifestDeserializer::FEATURES, Json::Array());
+            auto& map = obj.insert(ManifestDeserializer::FEATURES, Json::Object());
             for (const auto& feature : scf.feature_paragraphs)
             {
-                auto& feature_obj = arr.push_back(Json::Object());
+                auto& feature_obj = map.insert(feature->name, Json::Object());
                 for (const auto& el : feature->extra_info)
                 {
                     feature_obj.insert(el.first.to_string(), el.second);
                 }
 
-                feature_obj.insert(FeatureDeserializer::NAME, Json::Value::string(feature->name));
                 serialize_paragraph(feature_obj, FeatureDeserializer::DESCRIPTION, feature->description, true);
 
                 if (!feature->dependencies.empty() || debug)