[vcpkg] Improve Json error messages (#12981)

* [vcpkg] Fix error reporting on json parse failure

* [vcpkg] Track manifest path for use in diagnostics

* [vcpkg] Use by-value for consumer API. Improve trailing comma diagnostic.

* [vcpkg] Track errors directly inside Json::Reader

* [vcpkg] Fixup use of .u8string()

Co-authored-by: Robert Schumacher <roschuma@microsoft.com>

14 files changed, 237 insertions, 274 deletions

diff --git a/toolsrc/include/vcpkg/base/fwd/json.h b/toolsrc/include/vcpkg/base/fwd/json.h
index 7071103c1..73826127c 100644
--- a/toolsrc/include/vcpkg/base/fwd/json.h
+++ b/toolsrc/include/vcpkg/base/fwd/json.h
@@ -25,13 +25,13 @@ namespace vcpkg::Json
 
         virtual Span<const StringView> valid_fields() const;
 
-        virtual Optional<Type> visit_null(Reader&, StringView);
-        virtual Optional<Type> visit_boolean(Reader&, StringView, bool);
-        virtual Optional<Type> visit_integer(Reader& r, StringView field_name, int64_t i);
-        virtual Optional<Type> visit_number(Reader&, StringView, double);
-        virtual Optional<Type> visit_string(Reader&, StringView, StringView);
-        virtual Optional<Type> visit_array(Reader&, StringView, const Array&);
-        virtual Optional<Type> visit_object(Reader&, StringView, const Object&);
+        virtual Optional<Type> visit_null(Reader&);
+        virtual Optional<Type> visit_boolean(Reader&, bool);
+        virtual Optional<Type> visit_integer(Reader& r, int64_t i);
+        virtual Optional<Type> visit_number(Reader&, double);
+        virtual Optional<Type> visit_string(Reader&, StringView);
+        virtual Optional<Type> visit_array(Reader&, const Array&);
+        virtual Optional<Type> visit_object(Reader&, const Object&);
 
     protected:
         IDeserializer() = default;
diff --git a/toolsrc/include/vcpkg/base/json.h b/toolsrc/include/vcpkg/base/json.h
index 7cdcb0162..e4ce3210b 100644
--- a/toolsrc/include/vcpkg/base/json.h
+++ b/toolsrc/include/vcpkg/base/json.h
@@ -120,7 +120,7 @@ namespace vcpkg::Json
         static Value boolean(bool) noexcept;
         static Value integer(int64_t i) noexcept;
         static Value number(double d) noexcept;
-        static Value string(StringView) noexcept;
+        static Value string(std::string s) noexcept;
         static Value array(Array&&) noexcept;
         static Value array(const Array&) noexcept;
         static Value object(Object&&) noexcept;
@@ -297,113 +297,94 @@ namespace vcpkg::Json
     }
 
     template<class Type>
-    Optional<Type> IDeserializer<Type>::visit_null(Reader&, StringView)
+    Optional<Type> IDeserializer<Type>::visit_null(Reader&)
     {
         return nullopt;
     }
     template<class Type>
-    Optional<Type> IDeserializer<Type>::visit_boolean(Reader&, StringView, bool)
+    Optional<Type> IDeserializer<Type>::visit_boolean(Reader&, bool)
     {
         return nullopt;
     }
     template<class Type>
-    Optional<Type> IDeserializer<Type>::visit_integer(Reader& r, StringView field_name, int64_t i)
+    Optional<Type> IDeserializer<Type>::visit_integer(Reader& r, int64_t i)
     {
-        return this->visit_number(r, field_name, static_cast<double>(i));
+        return this->visit_number(r, static_cast<double>(i));
     }
     template<class Type>
-    Optional<Type> IDeserializer<Type>::visit_number(Reader&, StringView, double)
+    Optional<Type> IDeserializer<Type>::visit_number(Reader&, double)
     {
         return nullopt;
     }
     template<class Type>
-    Optional<Type> IDeserializer<Type>::visit_string(Reader&, StringView, StringView)
+    Optional<Type> IDeserializer<Type>::visit_string(Reader&, StringView)
     {
         return nullopt;
     }
     template<class Type>
-    Optional<Type> IDeserializer<Type>::visit_array(Reader&, StringView, const Array&)
+    Optional<Type> IDeserializer<Type>::visit_array(Reader&, const Array&)
     {
         return nullopt;
     }
     template<class Type>
-    Optional<Type> IDeserializer<Type>::visit_object(Reader&, StringView, const Object&)
+    Optional<Type> IDeserializer<Type>::visit_object(Reader&, const Object&)
     {
         return nullopt;
     }
 
     VCPKG_MSVC_WARNING(pop)
 
-    struct ReaderError
+    struct Reader
     {
-        virtual void add_missing_field(std::string&& type, std::string&& key) = 0;
-        virtual void add_expected_type(std::string&& key, std::string&& expected_type) = 0;
-        virtual void add_extra_fields(std::string&& type, std::vector<std::string>&& fields) = 0;
-        virtual void add_mutually_exclusive_fields(std::string&& type, std::vector<std::string>&& fields) = 0;
-
-        virtual ~ReaderError() = default;
-    };
+        const std::vector<std::string>& errors() const { return m_errors; }
+        std::vector<std::string>& errors() { return m_errors; }
 
-    struct BasicReaderError : ReaderError
-    {
-        virtual void add_missing_field(std::string&& type, std::string&& key) override
-        {
-            missing_fields.emplace_back(std::move(type), std::move(key));
-        }
-        virtual void add_expected_type(std::string&& key, std::string&& expected_type) override
+        void add_missing_field_error(StringView type, StringView key, StringView key_type)
         {
-            expected_types.emplace_back(std::move(key), std::move(expected_type));
+            m_errors.push_back(
+                Strings::concat(path(), " (", type, "): ", "missing required field '", key, "' (", key_type, ")"));
         }
-        virtual void add_extra_fields(std::string&& type, std::vector<std::string>&& fields) override
+        void add_expected_type_error(StringView expected_type)
         {
-            extra_fields.emplace_back(std::move(type), std::move(fields));
+            m_errors.push_back(Strings::concat(path(), ": mismatched type: expected ", expected_type));
         }
-        virtual void add_mutually_exclusive_fields(std::string&& type, std::vector<std::string>&& fields) override
+        void add_extra_fields_error(StringView type, std::vector<std::string>&& fields)
         {
-            mutually_exclusive_fields.emplace_back(std::move(type), std::move(fields));
+            for (auto&& field : fields)
+                m_errors.push_back(Strings::concat(path(), " (", type, "): ", "unexpected field '", field, '\''));
         }
 
-        bool has_error() const
-        {
-            return !missing_fields.empty() || !expected_types.empty() || !extra_fields.empty() ||
-                   !mutually_exclusive_fields.empty();
-        }
-
-        std::vector<std::pair<std::string, std::string>> missing_fields;
-        std::vector<std::pair<std::string, std::string>> expected_types;
-        std::vector<std::pair<std::string, std::vector<std::string>>> extra_fields;
-        std::vector<std::pair<std::string, std::vector<std::string>>> mutually_exclusive_fields;
-    };
-
-    struct Reader
-    {
-        explicit Reader(ReaderError* err) : err(err) { }
-
-        ReaderError& error() const { return *err; }
+        std::string path() const noexcept;
 
     private:
-        ReaderError* err;
+        std::vector<std::string> m_errors;
+        struct Path
+        {
+            int64_t index = -1;
+            StringView field;
+        };
+        std::vector<Path> m_path;
 
         template<class Type>
-        Optional<Type> internal_visit(const Value& value, StringView key, IDeserializer<Type>& visitor)
+        Optional<Type> internal_visit(const Value& value, IDeserializer<Type>& visitor)
         {
             switch (value.kind())
             {
-                case ValueKind::Null: return visitor.visit_null(*this, key);
-                case ValueKind::Boolean: return visitor.visit_boolean(*this, key, value.boolean());
-                case ValueKind::Integer: return visitor.visit_integer(*this, key, value.integer());
-                case ValueKind::Number: return visitor.visit_number(*this, key, value.number());
-                case ValueKind::String: return visitor.visit_string(*this, key, value.string());
-                case ValueKind::Array: return visitor.visit_array(*this, key, value.array());
+                case ValueKind::Null: return visitor.visit_null(*this);
+                case ValueKind::Boolean: return visitor.visit_boolean(*this, value.boolean());
+                case ValueKind::Integer: return visitor.visit_integer(*this, value.integer());
+                case ValueKind::Number: return visitor.visit_number(*this, value.number());
+                case ValueKind::String: return visitor.visit_string(*this, value.string());
+                case ValueKind::Array: return visitor.visit_array(*this, value.array());
                 case ValueKind::Object:
                 {
                     const auto& obj = value.object();
                     check_for_unexpected_fields(obj, visitor.valid_fields(), visitor.type_name());
-                    return visitor.visit_object(*this, key, value.object());
+                    return visitor.visit_object(*this, obj);
                 }
             }
 
-            vcpkg::Checks::exit_fail(VCPKG_LINE_INFO);
+            vcpkg::Checks::unreachable(VCPKG_LINE_INFO);
         }
 
         // returns whether the field was found, not whether it was valid
@@ -416,7 +397,8 @@ namespace vcpkg::Json
                 return false;
             }
 
-            Optional<Type> opt = internal_visit(*value, key, visitor);
+            m_path.push_back({-1, key});
+            Optional<Type> opt = internal_visit(*value, visitor);
 
             if (auto val = opt.get())
             {
@@ -424,149 +406,108 @@ namespace vcpkg::Json
             }
             else
             {
-                err->add_expected_type(key.to_string(), visitor.type_name().to_string());
+                add_expected_type_error(visitor.type_name().to_string());
             }
-
+            m_path.pop_back();
             return true;
         }
 
-        static std::vector<std::string> invalid_json_fields(const Json::Object& obj,
-                                                            Span<const StringView> known_fields) noexcept
-        {
-            const auto field_is_unknown = [known_fields](StringView sv) {
-                // allow directives
-                if (sv.size() != 0 && *sv.begin() == '$')
-                {
-                    return false;
-                }
-                return std::find(known_fields.begin(), known_fields.end(), sv) == known_fields.end();
-            };
-
-            std::vector<std::string> res;
-            for (const auto& kv : obj)
-            {
-                if (field_is_unknown(kv.first))
-                {
-                    res.push_back(kv.first.to_string());
-                }
-            }
-
-            return res;
-        }
-
         // checks that an object doesn't contain any fields which both:
         // * don't start with a `$`
         // * are not in `valid_fields`
         // if known_fields.empty(), then it's treated as if all field names are valid
-        void check_for_unexpected_fields(const Object& obj, Span<const StringView> valid_fields, StringView type_name)
-        {
-            if (valid_fields.size() == 0)
-            {
-                return;
-            }
-
-            auto extra_fields = invalid_json_fields(obj, valid_fields);
-            if (!extra_fields.empty())
-            {
-                error().add_extra_fields(type_name.to_string(), std::move(extra_fields));
-            }
-        }
+        void check_for_unexpected_fields(const Object& obj, Span<const StringView> valid_fields, StringView type_name);
 
     public:
-        template<class Type>
+        template<class Type, class Deserializer>
         void required_object_field(
-            StringView type, const Object& obj, StringView key, Type& place, IDeserializer<Type>& visitor)
+            StringView type, const Object& obj, StringView key, Type& place, Deserializer&& visitor)
         {
             if (!internal_field(obj, key, place, visitor))
             {
-                err->add_missing_field(type.to_string(), key.to_string());
+                this->add_missing_field_error(type, key, visitor.type_name());
             }
         }
-        template<class Type>
-        void required_object_field(
-            StringView type, const Object& obj, StringView key, Type& place, IDeserializer<Type>&& visitor)
-        {
-            required_object_field(type, obj, key, place, visitor);
-        }
 
         // returns whether key \in obj
-        template<class Type>
-        bool optional_object_field(const Object& obj, StringView key, Type& place, IDeserializer<Type>& visitor)
+        template<class Type, class Deserializer>
+        bool optional_object_field(const Object& obj, StringView key, Type& place, Deserializer&& visitor)
         {
             return internal_field(obj, key, place, visitor);
         }
-        template<class Type>
-        bool optional_object_field(const Object& obj, StringView key, Type& place, IDeserializer<Type>&& visitor)
-        {
-            return optional_object_field(obj, key, place, visitor);
-        }
 
         template<class Type>
-        Optional<Type> visit_value(const Value& value, StringView key, IDeserializer<Type>& visitor)
+        Optional<Type> visit_value(const Value& value, IDeserializer<Type>& visitor)
         {
-            return internal_visit(value, key, visitor);
+            return internal_visit(value, visitor);
         }
         template<class Type>
-        Optional<Type> visit_value(const Value& value, StringView key, IDeserializer<Type>&& visitor)
+        Optional<Type> visit_value(const Value& value, IDeserializer<Type>&& visitor)
         {
-            return visit_value(value, key, visitor);
+            return visit_value(value, visitor);
         }
 
         template<class Type>
-        Optional<Type> visit_value(const Array& value, StringView key, IDeserializer<Type>& visitor)
+        Optional<Type> visit_value(const Array& value, IDeserializer<Type>& visitor)
         {
-            return visitor.visit_array(*this, key, value);
+            return visitor.visit_array(*this, value);
         }
         template<class Type>
-        Optional<Type> visit_value(const Array& value, StringView key, IDeserializer<Type>&& visitor)
+        Optional<Type> visit_value(const Array& value, IDeserializer<Type>&& visitor)
         {
-            return visit_value(value, key, visitor);
+            return visit_value(value, visitor);
         }
 
         template<class Type>
-        Optional<Type> visit_value(const Object& value, StringView key, IDeserializer<Type>& visitor)
+        Optional<Type> visit_value(const Object& value, IDeserializer<Type>& visitor)
         {
             check_for_unexpected_fields(value, visitor.valid_fields(), visitor.type_name());
-            return visitor.visit_object(*this, key, value);
+            return visitor.visit_object(*this, value);
         }
         template<class Type>
-        Optional<Type> visit_value(const Object& value, StringView key, IDeserializer<Type>&& visitor)
+        Optional<Type> visit_value(const Object& value, IDeserializer<Type>&& visitor)
         {
-            return visit_value(value, key, visitor);
+            return visit_value(value, visitor);
         }
 
         template<class Type>
-        Optional<std::vector<Type>> array_elements(const Array& arr, StringView key, IDeserializer<Type>& visitor)
+        Optional<std::vector<Type>> array_elements(const Array& arr, IDeserializer<Type>& visitor)
         {
             std::vector<Type> result;
-            for (const auto& el : arr)
+            m_path.emplace_back();
+            for (size_t i = 0; i < arr.size(); ++i)
             {
-                auto opt = internal_visit(el, key, visitor);
+                m_path.back().index = static_cast<int64_t>(i);
+                auto opt = internal_visit(arr[i], visitor);
                 if (auto p = opt.get())
                 {
                     result.push_back(std::move(*p));
                 }
                 else
                 {
-                    return nullopt;
+                    this->add_expected_type_error(visitor.type_name());
+                    for (++i; i < arr.size(); ++i)
+                    {
+                        m_path.back().index = static_cast<int64_t>(i);
+                        auto opt2 = internal_visit(arr[i], visitor);
+                        if (!opt2) this->add_expected_type_error(visitor.type_name());
+                    }
                 }
             }
+            m_path.pop_back();
             return std::move(result);
         }
         template<class Type>
-        Optional<std::vector<Type>> array_elements(const Array& arr, StringView key, IDeserializer<Type>&& visitor)
+        Optional<std::vector<Type>> array_elements(const Array& arr, IDeserializer<Type>&& visitor)
         {
-            return array_elements(arr, key, visitor);
+            return array_elements(arr, visitor);
         }
     };
 
     struct StringDeserializer final : IDeserializer<std::string>
     {
         virtual StringView type_name() const override { return type_name_; }
-        virtual Optional<std::string> visit_string(Reader&, StringView, StringView sv) override
-        {
-            return sv.to_string();
-        }
+        virtual Optional<std::string> visit_string(Reader&, StringView sv) override { return sv.to_string(); }
 
         explicit StringDeserializer(StringView type_name_) : type_name_(type_name_) { }
 
@@ -577,14 +518,14 @@ namespace vcpkg::Json
     struct PathDeserializer final : IDeserializer<fs::path>
     {
         virtual StringView type_name() const override { return "a path"; }
-        virtual Optional<fs::path> visit_string(Reader&, StringView, StringView sv) override { return fs::u8path(sv); }
+        virtual Optional<fs::path> visit_string(Reader&, StringView sv) override { return fs::u8path(sv); }
     };
 
     struct NaturalNumberDeserializer final : IDeserializer<int>
     {
         virtual StringView type_name() const override { return "a natural number"; }
 
-        virtual Optional<int> visit_integer(Reader&, StringView, int64_t value) override
+        virtual Optional<int> visit_integer(Reader&, int64_t value) override
         {
             if (value > std::numeric_limits<int>::max() || value < 0)
             {
@@ -598,7 +539,7 @@ namespace vcpkg::Json
     {
         virtual StringView type_name() const override { return "a boolean"; }
 
-        virtual Optional<bool> visit_boolean(Reader&, StringView, bool b) override { return b; }
+        virtual Optional<bool> visit_boolean(Reader&, bool b) override { return b; }
     };
 
     enum class AllowEmpty : bool
@@ -619,13 +560,13 @@ namespace vcpkg::Json
         {
         }
 
-        virtual Optional<type> visit_array(Reader& r, StringView key, const Array& arr) override
+        virtual Optional<type> visit_array(Reader& r, const Array& arr) override
         {
             if (allow_empty_ == AllowEmpty::No && arr.size() == 0)
             {
                 return nullopt;
             }
-            return r.array_elements(arr, key, underlying_visitor_);
+            return r.array_elements(arr, underlying_visitor_);
         }
 
     private:
@@ -638,16 +579,16 @@ namespace vcpkg::Json
     {
         virtual StringView type_name() const override { return "a string or array of strings"; }
 
-        virtual Optional<std::vector<std::string>> visit_string(Reader&, StringView, StringView sv) override
+        virtual Optional<std::vector<std::string>> visit_string(Reader&, StringView sv) override
         {
             std::vector<std::string> out;
             out.push_back(sv.to_string());
             return out;
         }
 
-        virtual Optional<std::vector<std::string>> visit_array(Reader& r, StringView key, const Array& arr) override
+        virtual Optional<std::vector<std::string>> visit_array(Reader& r, const Array& arr) override
         {
-            return r.array_elements(arr, key, StringDeserializer{"a string"});
+            return r.array_elements(arr, StringDeserializer{"a string"});
         }
     };
 
@@ -658,7 +599,7 @@ namespace vcpkg::Json
         // [a-z0-9]+(-[a-z0-9]+)*, plus not any of {prn, aux, nul, con, lpt[1-9], com[1-9], core, default}
         static bool is_ident(StringView sv);
 
-        virtual Optional<std::string> visit_string(Json::Reader&, StringView, StringView sv) override
+        virtual Optional<std::string> visit_string(Json::Reader&, StringView sv) override
         {
             if (is_ident(sv))
             {
@@ -693,7 +634,7 @@ namespace vcpkg::Json
             return true;
         }
 
-        virtual Optional<std::string> visit_string(Json::Reader&, StringView, StringView sv) override
+        virtual Optional<std::string> visit_string(Json::Reader&, StringView sv) override
         {
             if (!is_package_name(sv))
             {
diff --git a/toolsrc/include/vcpkg/configuration.h b/toolsrc/include/vcpkg/configuration.h
index 44440fa04..a4ed38223 100644
--- a/toolsrc/include/vcpkg/configuration.h
+++ b/toolsrc/include/vcpkg/configuration.h
@@ -29,7 +29,7 @@ namespace vcpkg
             return t;
         }
 
-        virtual Optional<Configuration> visit_object(Json::Reader& r, StringView, const Json::Object& obj) override;
+        virtual Optional<Configuration> visit_object(Json::Reader& r, const Json::Object& obj) override;
 
         ConfigurationDeserializer(const VcpkgCmdArguments& args);
 
diff --git a/toolsrc/include/vcpkg/paragraphparser.h b/toolsrc/include/vcpkg/paragraphparser.h
index 4403b3aea..22060f32f 100644
--- a/toolsrc/include/vcpkg/paragraphparser.h
+++ b/toolsrc/include/vcpkg/paragraphparser.h
@@ -20,11 +20,13 @@ namespace vcpkg::Parse
         std::map<std::string, std::vector<std::string>> extra_fields;
         std::map<std::string, std::string> expected_types;
         std::map<std::string, std::vector<std::string>> mutually_exclusive_fields;
+        std::vector<std::string> other_errors;
         std::string error;
 
         bool has_error() const
         {
-            return !missing_fields.empty() || !extra_fields.empty() || !expected_types.empty() || !error.empty();
+            return !missing_fields.empty() || !extra_fields.empty() || !expected_types.empty() ||
+                   !other_errors.empty() || !error.empty();
         }
     };
 
diff --git a/toolsrc/include/vcpkg/registries.h b/toolsrc/include/vcpkg/registries.h
index 78c055dae..100bee170 100644
--- a/toolsrc/include/vcpkg/registries.h
+++ b/toolsrc/include/vcpkg/registries.h
@@ -50,10 +50,8 @@ namespace vcpkg
         virtual StringView type_name() const override;
         virtual Span<const StringView> valid_fields() const override;
 
-        virtual Optional<std::unique_ptr<RegistryImpl>> visit_null(Json::Reader&, StringView) override;
-        virtual Optional<std::unique_ptr<RegistryImpl>> visit_object(Json::Reader&,
-                                                                     StringView,
-                                                                     const Json::Object&) override;
+        virtual Optional<std::unique_ptr<RegistryImpl>> visit_null(Json::Reader&) override;
+        virtual Optional<std::unique_ptr<RegistryImpl>> visit_object(Json::Reader&, const Json::Object&) override;
     };
 
     struct RegistryDeserializer final : Json::IDeserializer<Registry>
@@ -63,7 +61,7 @@ namespace vcpkg
         virtual StringView type_name() const override;
         virtual Span<const StringView> valid_fields() const override;
 
-        virtual Optional<Registry> visit_object(Json::Reader&, StringView, const Json::Object&) override;
+        virtual Optional<Registry> visit_object(Json::Reader&, const Json::Object&) override;
     };
 
     // this type implements the registry fall back logic from the registries RFC:
diff --git a/toolsrc/include/vcpkg/vcpkgpaths.h b/toolsrc/include/vcpkg/vcpkgpaths.h
index ae2ee3823..8abc84129 100644
--- a/toolsrc/include/vcpkg/vcpkgpaths.h
+++ b/toolsrc/include/vcpkg/vcpkgpaths.h
@@ -108,7 +108,7 @@ namespace vcpkg
         const std::string& get_tool_version(const std::string& tool) const;
 
         Optional<const Json::Object&> get_manifest() const;
-        Optional<const Json::JsonStyle&> get_manifest_style() const;
+        Optional<const fs::path&> get_manifest_path() const;
         const Configuration& get_configuration() const;
 
         /// <summary>Retrieve a toolset matching a VS version</summary>
diff --git a/toolsrc/src/vcpkg-test/json.cpp b/toolsrc/src/vcpkg-test/json.cpp
index 0b9941861..6858a5ac8 100644
--- a/toolsrc/src/vcpkg-test/json.cpp
+++ b/toolsrc/src/vcpkg-test/json.cpp
@@ -28,8 +28,8 @@ static std::string mystringify(const Value& val) { return Json::stringify(val, J
 
 TEST_CASE ("JSON stringify weird strings", "[json]")
 {
-    vcpkg::StringView str = U8_STR("😀 😁 😂 🤣 😃 😄 😅 😆 😉");
-    REQUIRE(mystringify(Value::string(str)) == ('"' + str.to_string() + "\"\n"));
+    std::string str = U8_STR("😀 😁 😂 🤣 😃 😄 😅 😆 😉");
+    REQUIRE(mystringify(Value::string(str)) == ('"' + str + "\"\n"));
     REQUIRE(mystringify(Value::string("\xED\xA0\x80")) == "\"\\ud800\"\n"); // unpaired surrogate
 }
 
@@ -228,3 +228,14 @@ TEST_CASE ("JSON parse full file", "[json]")
     }
     REQUIRE(res);
 }
+
+TEST_CASE ("JSON track newlines", "[json]")
+{
+    auto res = Json::parse("{\n,", fs::u8path("filename"));
+    REQUIRE(!res);
+    REQUIRE(res.error()->format() ==
+            R"(Error: filename:2:1: Unexpected character; expected property name
+   on expression: ,
+                  ^
+)");
+}
diff --git a/toolsrc/src/vcpkg/base/json.cpp b/toolsrc/src/vcpkg/base/json.cpp
index 58121ac8e..495a36e32 100644
--- a/toolsrc/src/vcpkg/base/json.cpp
+++ b/toolsrc/src/vcpkg/base/json.cpp
@@ -253,15 +253,15 @@ namespace vcpkg::Json
         val.underlying_ = std::make_unique<ValueImpl>(ValueKindConstant<VK::Number>(), d);
         return val;
     }
-    Value Value::string(StringView sv) noexcept
+    Value Value::string(std::string s) noexcept
     {
-        if (!Unicode::utf8_is_valid_string(sv.begin(), sv.end()))
+        if (!Unicode::utf8_is_valid_string(s.data(), s.data() + s.size()))
         {
-            Debug::print("Invalid string: ", sv, '\n');
-            vcpkg::Checks::exit_with_message(VCPKG_LINE_INFO, "Invalid utf8 passed to Value::string(StringView)");
+            Debug::print("Invalid string: ", s, '\n');
+            vcpkg::Checks::exit_with_message(VCPKG_LINE_INFO, "Invalid utf8 passed to Value::string(std::string)");
         }
         Value val;
-        val.underlying_ = std::make_unique<ValueImpl>(ValueKindConstant<VK::String>(), sv.to_string());
+        val.underlying_ = std::make_unique<ValueImpl>(ValueKindConstant<VK::String>(), std::move(s));
         return val;
     }
     Value Value::array(Array&& arr) noexcept
@@ -489,10 +489,6 @@ namespace vcpkg::Json
             {
                 return code_point == '-' || is_digit(code_point);
             }
-            static bool is_keyword_start(char32_t code_point) noexcept
-            {
-                return code_point == 'f' || code_point == 'n' || code_point == 't';
-            }
 
             static unsigned char from_hex_digit(char32_t code_point) noexcept
             {
@@ -807,6 +803,7 @@ namespace vcpkg::Json
                     }
                     else if (current == ',')
                     {
+                        auto comma_loc = cur_loc();
                         next();
                         skip_whitespace();
                         current = cur();
@@ -817,7 +814,7 @@ namespace vcpkg::Json
                         }
                         if (current == ']')
                         {
-                            add_error("Trailing comma in array");
+                            add_error("Trailing comma in array", comma_loc);
                             return Value::array(std::move(arr));
                         }
                     }
@@ -903,6 +900,7 @@ namespace vcpkg::Json
                     }
                     else if (current == ',')
                     {
+                        auto comma_loc = cur_loc();
                         next();
                         skip_whitespace();
                         current = cur();
@@ -913,7 +911,7 @@ namespace vcpkg::Json
                         }
                         else if (current == '}')
                         {
-                            add_error("Trailing comma in an object");
+                            add_error("Trailing comma in an object", comma_loc);
                             return Value();
                         }
                     }
@@ -1260,4 +1258,57 @@ namespace vcpkg::Json
     }
     // } auto stringify()
 
+    static std::vector<std::string> invalid_json_fields(const Json::Object& obj,
+                                                        Span<const StringView> known_fields) noexcept
+    {
+        const auto field_is_unknown = [known_fields](StringView sv) {
+            // allow directives
+            if (sv.size() != 0 && *sv.begin() == '$')
+            {
+                return false;
+            }
+            return std::find(known_fields.begin(), known_fields.end(), sv) == known_fields.end();
+        };
+
+        std::vector<std::string> res;
+        for (const auto& kv : obj)
+        {
+            if (field_is_unknown(kv.first))
+            {
+                res.push_back(kv.first.to_string());
+            }
+        }
+
+        return res;
+    }
+
+    void Reader::check_for_unexpected_fields(const Object& obj,
+                                             Span<const StringView> valid_fields,
+                                             StringView type_name)
+    {
+        if (valid_fields.size() == 0)
+        {
+            return;
+        }
+
+        auto extra_fields = invalid_json_fields(obj, valid_fields);
+        if (!extra_fields.empty())
+        {
+            add_extra_fields_error(type_name.to_string(), std::move(extra_fields));
+        }
+    }
+
+    std::string Reader::path() const noexcept
+    {
+        std::string p("$");
+        for (auto&& s : m_path)
+        {
+            if (s.index < 0)
+                Strings::append(p, '.', s.field);
+            else
+                Strings::append(p, '[', s.index, ']');
+        }
+        return p;
+    }
+
 }
diff --git a/toolsrc/src/vcpkg/base/parse.cpp b/toolsrc/src/vcpkg/base/parse.cpp
index 669321c05..c6a7de83d 100644
--- a/toolsrc/src/vcpkg/base/parse.cpp
+++ b/toolsrc/src/vcpkg/base/parse.cpp
@@ -66,10 +66,15 @@ namespace vcpkg::Parse
         {
             return Unicode::end_of_file;
         }
+        auto ch = *m_it;
         // See https://www.gnu.org/prep/standards/standards.html#Errors
-        advance_rowcol(*m_it, m_row, m_column);
+        advance_rowcol(ch, m_row, m_column);
 
         ++m_it;
+        if (ch == '\n')
+        {
+            m_start_of_line = m_it;
+        }
         if (m_it != m_it.end() && Unicode::utf16_is_surrogate_code_point(*m_it))
         {
             m_it = m_it.end();
diff --git a/toolsrc/src/vcpkg/configuration.cpp b/toolsrc/src/vcpkg/configuration.cpp
index 1bce07acf..9363abe80 100644
--- a/toolsrc/src/vcpkg/configuration.cpp
+++ b/toolsrc/src/vcpkg/configuration.cpp
@@ -5,9 +5,7 @@
 
 namespace vcpkg
 {
-    Optional<Configuration> ConfigurationDeserializer::visit_object(Json::Reader& r,
-                                                                    StringView,
-                                                                    const Json::Object& obj)
+    Optional<Configuration> ConfigurationDeserializer::visit_object(Json::Reader& r, const Json::Object& obj)
     {
         RegistrySet registries;
 
diff --git a/toolsrc/src/vcpkg/install.cpp b/toolsrc/src/vcpkg/install.cpp
index 06ae14ed9..0a16294a1 100644
--- a/toolsrc/src/vcpkg/install.cpp
+++ b/toolsrc/src/vcpkg/install.cpp
@@ -788,13 +788,15 @@ namespace vcpkg::Install
             {
                 pkgsconfig = fs::u8path(it_pkgsconfig->second);
             }
-
-            auto maybe_manifest_scf = SourceControlFile::parse_manifest_file("manifest", *manifest);
+            auto manifest_path = paths.get_manifest_path().value_or_exit(VCPKG_LINE_INFO);
+            auto maybe_manifest_scf = SourceControlFile::parse_manifest_file(manifest_path, *manifest);
             if (!maybe_manifest_scf)
             {
                 print_error_message(maybe_manifest_scf.error());
-                Checks::exit_with_message(
-                    VCPKG_LINE_INFO, "Failed to parse manifest %s/vcpkg.json.", fs::u8string(paths.manifest_root_dir));
+                System::print2(
+                    "See https://github.com/Microsoft/vcpkg/tree/master/docs/specifications/manifests.md for "
+                    "more information.\n");
+                Checks::exit_fail(VCPKG_LINE_INFO);
             }
             auto& manifest_scf = *maybe_manifest_scf.value_or_exit(VCPKG_LINE_INFO);
 
diff --git a/toolsrc/src/vcpkg/registries.cpp b/toolsrc/src/vcpkg/registries.cpp
index eda20f1b9..aa9a3a336 100644
--- a/toolsrc/src/vcpkg/registries.cpp
+++ b/toolsrc/src/vcpkg/registries.cpp
@@ -45,13 +45,9 @@ namespace vcpkg
         return t;
     }
 
-    Optional<std::unique_ptr<RegistryImpl>> RegistryImplDeserializer::visit_null(Json::Reader&, StringView)
-    {
-        return nullptr;
-    }
+    Optional<std::unique_ptr<RegistryImpl>> RegistryImplDeserializer::visit_null(Json::Reader&) { return nullptr; }
 
     Optional<std::unique_ptr<RegistryImpl>> RegistryImplDeserializer::visit_object(Json::Reader& r,
-                                                                                   StringView,
                                                                                    const Json::Object& obj)
     {
         std::string kind;
@@ -62,14 +58,14 @@ namespace vcpkg
         {
             if (obj.contains(PATH))
             {
-                r.error().add_extra_fields(type_name().to_string(), {PATH});
+                r.add_extra_fields_error("a builtin registry", {PATH});
             }
             return static_cast<std::unique_ptr<RegistryImpl>>(std::make_unique<BuiltinRegistry>());
         }
         else if (kind == KIND_DIRECTORY)
         {
             fs::path path;
-            r.required_object_field(type_name(), obj, PATH, path, Json::PathDeserializer{});
+            r.required_object_field("a directory registry", obj, PATH, path, Json::PathDeserializer{});
 
             return static_cast<std::unique_ptr<RegistryImpl>>(std::make_unique<DirectoryRegistry>(std::move(path)));
         }
@@ -93,9 +89,9 @@ namespace vcpkg
         return t;
     }
 
-    Optional<Registry> RegistryDeserializer::visit_object(Json::Reader& r, StringView key, const Json::Object& obj)
+    Optional<Registry> RegistryDeserializer::visit_object(Json::Reader& r, const Json::Object& obj)
     {
-        auto impl = RegistryImplDeserializer{}.visit_object(r, key, obj);
+        auto impl = RegistryImplDeserializer{}.visit_object(r, obj);
 
         if (!impl.has_value())
         {
diff --git a/toolsrc/src/vcpkg/sourceparagraph.cpp b/toolsrc/src/vcpkg/sourceparagraph.cpp
index 94a5dbe41..27861f873 100644
--- a/toolsrc/src/vcpkg/sourceparagraph.cpp
+++ b/toolsrc/src/vcpkg/sourceparagraph.cpp
@@ -361,7 +361,7 @@ namespace vcpkg
     {
         virtual StringView type_name() const override { return "a platform expression"; }
 
-        virtual Optional<PlatformExpression::Expr> visit_string(Json::Reader&, StringView, StringView sv) override
+        virtual Optional<PlatformExpression::Expr> visit_string(Json::Reader&, StringView sv) override
         {
             auto opt =
                 PlatformExpression::parse_platform_expression(sv, PlatformExpression::MultipleBinaryOperators::Deny);
@@ -398,7 +398,7 @@ namespace vcpkg
             return t;
         }
 
-        virtual Optional<Dependency> visit_string(Json::Reader&, StringView, StringView sv) override
+        virtual Optional<Dependency> visit_string(Json::Reader&, StringView sv) override
         {
             if (!Json::PackageNameDeserializer::is_package_name(sv))
             {
@@ -410,7 +410,7 @@ namespace vcpkg
             return dep;
         }
 
-        virtual Optional<Dependency> visit_object(Json::Reader& r, StringView, const Json::Object& obj) override
+        virtual Optional<Dependency> visit_object(Json::Reader& r, const Json::Object& obj) override
         {
             Dependency dep;
 
@@ -462,7 +462,6 @@ namespace vcpkg
         }
 
         virtual Optional<std::unique_ptr<FeatureParagraph>> visit_object(Json::Reader& r,
-                                                                         StringView,
                                                                          const Json::Object& obj) override
         {
             auto feature = std::make_unique<FeatureParagraph>();
@@ -516,7 +515,7 @@ namespace vcpkg
 #include "spdx-licenses.inc"
             ;
 
-        virtual Optional<std::string> visit_string(Json::Reader&, StringView, StringView sv) override
+        virtual Optional<std::string> visit_string(Json::Reader&, StringView sv) override
         {
             Mode mode = Mode::ExpectExpression;
             size_t open_parens = 0;
@@ -677,7 +676,6 @@ namespace vcpkg
         }
 
         virtual Optional<std::unique_ptr<SourceControlFile>> visit_object(Json::Reader& r,
-                                                                          StringView,
                                                                           const Json::Object& obj) override
         {
             auto control_file = std::make_unique<SourceControlFile>();
@@ -752,44 +750,16 @@ namespace vcpkg
     Parse::ParseExpected<SourceControlFile> SourceControlFile::parse_manifest_file(const fs::path& path_to_manifest,
                                                                                    const Json::Object& manifest)
     {
-        struct JsonErr final : Json::ReaderError
-        {
-            ParseControlErrorInfo pcei;
+        Json::Reader reader;
 
-            void add_missing_field(std::string&& type, std::string&& key) override
-            {
-                pcei.missing_fields[std::move(type)].push_back(std::move(key));
-            }
-            void add_expected_type(std::string&& key, std::string&& expected_type) override
-            {
-                pcei.expected_types.emplace(std::move(key), std::move(expected_type));
-            }
-            void add_extra_fields(std::string&& type, std::vector<std::string>&& fields) override
-            {
-                if (!fields.empty())
-                {
-                    auto& fields_for_type = pcei.extra_fields[std::move(type)];
-                    fields_for_type.insert(fields_for_type.end(), fields.begin(), fields.end());
-                }
-            }
-            void add_mutually_exclusive_fields(std::string&& type, std::vector<std::string>&& fields) override
-            {
-                if (!fields.empty())
-                {
-                    auto& fields_for_type = pcei.mutually_exclusive_fields[std::move(type)];
-                    fields_for_type.insert(fields_for_type.end(), fields.begin(), fields.end());
-                }
-            }
-        } err = {};
-        auto visit = Json::Reader{&err};
-
-        err.pcei.name = fs::u8string(path_to_manifest);
-
-        auto res = visit.visit_value(manifest, "$", ManifestDeserializer{});
+        auto res = reader.visit_value(manifest, ManifestDeserializer{});
 
-        if (err.pcei.has_error())
+        if (!reader.errors().empty())
         {
-            return std::make_unique<ParseControlErrorInfo>(std::move(err.pcei));
+            auto err = std::make_unique<ParseControlErrorInfo>();
+            err->name = fs::u8string(path_to_manifest);
+            err->other_errors = std::move(reader.errors());
+            return std::move(err);
         }
         else if (auto p = res.get())
         {
@@ -813,6 +783,13 @@ namespace vcpkg
                 System::print2(
                     System::Color::error, "Error: while loading ", error_info->name, ":\n", error_info->error, '\n');
             }
+
+            if (!error_info->other_errors.empty())
+            {
+                System::print2(System::Color::error, "Errors occurred while parsing ", error_info->name, "\n");
+                for (auto&& msg : error_info->other_errors)
+                    System::print2("    ", msg, '\n');
+            }
         }
 
         bool have_remaining_fields = false;
@@ -839,9 +816,6 @@ namespace vcpkg
             System::print2("This is the list of valid fields for CONTROL files (case-sensitive): \n\n    ",
                            Strings::join("\n    ", get_list_of_valid_fields()),
                            "\n\n");
-            System::print2("And this is the list of valid fields for manifest files: \n\n    ",
-                           Strings::join("\n    ", ManifestDeserializer{}.valid_fields()),
-                           "\n\n");
 #if defined(_WIN32)
             auto bootstrap = ".\\bootstrap-vcpkg.bat";
 #else
diff --git a/toolsrc/src/vcpkg/vcpkgpaths.cpp b/toolsrc/src/vcpkg/vcpkgpaths.cpp
index 677ac1a09..9f4ffce6b 100644
--- a/toolsrc/src/vcpkg/vcpkgpaths.cpp
+++ b/toolsrc/src/vcpkg/vcpkgpaths.cpp
@@ -84,50 +84,22 @@ namespace
 
 namespace vcpkg
 {
-    static Configuration deserialize_configuration(const Json::Object& obj, const VcpkgCmdArguments& args)
+    static Configuration deserialize_configuration(const Json::Object& obj,
+                                                   const VcpkgCmdArguments& args,
+                                                   const fs::path& filepath)
     {
-        Json::BasicReaderError err;
-        Json::Reader reader{&err};
+        Json::Reader reader;
         auto deserializer = ConfigurationDeserializer(args);
 
-        auto parsed_config_opt = reader.visit_value(obj, "$", deserializer);
-        if (err.has_error())
+        auto parsed_config_opt = reader.visit_value(obj, deserializer);
+        if (!reader.errors().empty())
         {
-            if (!err.missing_fields.empty())
-            {
-                System::print2(System::Color::error, "Error: missing fields in configuration:\n");
-                for (const auto& missing : err.missing_fields)
-                {
-                    System::printf(
-                        System::Color::error, "    %s was expected to have: %s\n", missing.first, missing.second);
-                }
-            }
-            if (!err.expected_types.empty())
-            {
-                System::print2(System::Color::error, "Error: Invalid types in configuration:\n");
-                for (const auto& expected : err.expected_types)
-                {
-                    System::printf(
-                        System::Color::error, "    %s was expected to be %s\n", expected.first, expected.second);
-                }
-            }
-            if (!err.extra_fields.empty())
-            {
-                System::print2(System::Color::error, "Error: Invalid fields in configuration:\n");
-                for (const auto& extra : err.extra_fields)
-                {
-                    System::printf(System::Color::error,
-                                   "    %s had invalid fields: %s\n",
-                                   extra.first,
-                                   Strings::join(", ", extra.second));
-                }
-            }
-            if (!err.mutually_exclusive_fields.empty())
-            {
-                // this should never happen
-                Checks::unreachable(VCPKG_LINE_INFO);
-            }
+            System::print2(System::Color::error, "Errors occurred while parsing ", fs::u8string(filepath), "\n");
+            for (auto&& msg : reader.errors())
+                System::print2("    ", msg, '\n');
 
+            System::print2("See https://github.com/Microsoft/vcpkg/tree/master/docs/specifications/registries.md for "
+                           "more information.\n");
             Checks::exit_fail(VCPKG_LINE_INFO);
         }
 
@@ -157,7 +129,7 @@ namespace vcpkg
         if (!manifest_opt.has_value())
         {
             Checks::exit_with_message(VCPKG_LINE_INFO,
-                                      "Failed to parse manifest at %s: %s",
+                                      "Failed to parse manifest at %s:\n%s",
                                       fs::u8string(manifest_path),
                                       manifest_opt.error()->format());
         }
@@ -216,7 +188,7 @@ namespace vcpkg
         }
         auto config_obj = std::move(parsed_config.first.object());
 
-        return {std::move(config_dir), deserialize_configuration(config_obj, args)};
+        return {std::move(config_dir), deserialize_configuration(config_obj, args, path_to_config)};
     }
 
     namespace details
@@ -244,6 +216,7 @@ namespace vcpkg
             fs::SystemHandle file_lock_handle;
 
             Optional<std::pair<Json::Object, Json::JsonStyle>> m_manifest_doc;
+            fs::path m_manifest_path;
             Configuration m_config;
         };
     }
@@ -305,6 +278,7 @@ namespace vcpkg
             }
 
             m_pimpl->m_manifest_doc = load_manifest(filesystem, manifest_root_dir);
+            m_pimpl->m_manifest_path = manifest_root_dir / fs::u8path("vcpkg.json");
         }
         else
         {
@@ -495,6 +469,17 @@ If you wish to silence this error and use classic mode, you can:
             return nullopt;
         }
     }
+    Optional<const fs::path&> VcpkgPaths::get_manifest_path() const
+    {
+        if (m_pimpl->m_manifest_doc)
+        {
+            return m_pimpl->m_manifest_path;
+        }
+        else
+        {
+            return nullopt;
+        }
+    }
 
     const Configuration& VcpkgPaths::get_configuration() const { return m_pimpl->m_config; }