From 9f3b2ef66c4a8b2cd79bea8f0c67b6bd8b335259 Mon Sep 17 00:00:00 2001 From: Martin Duffy Date: Thu, 2 Mar 2023 10:02:17 -0500 Subject: [PATCH 1/2] CharReader: Add Structured Error Add getStructuredError to CharReader --- include/json/reader.h | 27 ++++++++++++++++- src/lib_json/json_reader.cpp | 56 ++++++++++++++++++++++++------------ src/test_lib_json/main.cpp | 29 +++++++++++++++++++ 3 files changed, 92 insertions(+), 20 deletions(-) diff --git a/include/json/reader.h b/include/json/reader.h index 46975d86f..3b4560ee7 100644 --- a/include/json/reader.h +++ b/include/json/reader.h @@ -244,6 +244,12 @@ class JSON_API Reader { */ class JSON_API CharReader { public: + struct JSON_API StructuredError { + ptrdiff_t offset_start; + ptrdiff_t offset_limit; + String message; + }; + virtual ~CharReader() = default; /** \brief Read a Value from a JSON * document. The document must be a UTF-8 encoded string containing the @@ -262,7 +268,12 @@ class JSON_API CharReader { * error occurred. */ virtual bool parse(char const* beginDoc, char const* endDoc, Value* root, - String* errs) = 0; + String* errs); + + /** \brief Returns a vector of structured errors encountered while parsing. + * Each parse call resets the stored list of errors. + */ + std::vector getStructuredErrors() const; class JSON_API Factory { public: @@ -272,6 +283,20 @@ class JSON_API CharReader { */ virtual CharReader* newCharReader() const = 0; }; // Factory + +protected: + class Impl { + public: + virtual ~Impl() = default; + virtual bool parse(char const* beginDoc, char const* endDoc, Value* root, + String* errs) = 0; + virtual std::vector getStructuredErrors() const = 0; + }; + + explicit CharReader(std::unique_ptr impl) : _impl(std::move(impl)) { } + +private: + std::unique_ptr _impl; }; // CharReader /** \brief Build a CharReader implementation. diff --git a/src/lib_json/json_reader.cpp b/src/lib_json/json_reader.cpp index 1ac5e81ab..e202d4bb2 100644 --- a/src/lib_json/json_reader.cpp +++ b/src/lib_json/json_reader.cpp @@ -890,17 +890,12 @@ class OurReader { public: using Char = char; using Location = const Char*; - struct StructuredError { - ptrdiff_t offset_start; - ptrdiff_t offset_limit; - String message; - }; explicit OurReader(OurFeatures const& features); bool parse(const char* beginDoc, const char* endDoc, Value& root, bool collectComments = true); String getFormattedErrorMessages() const; - std::vector getStructuredErrors() const; + std::vector getStructuredErrors() const; private: OurReader(OurReader const&); // no impl @@ -1860,10 +1855,10 @@ String OurReader::getFormattedErrorMessages() const { return formattedMessage; } -std::vector OurReader::getStructuredErrors() const { - std::vector allErrors; +std::vector OurReader::getStructuredErrors() const { + std::vector allErrors; for (const auto& error : errors_) { - OurReader::StructuredError structured; + CharReader::StructuredError structured; structured.offset_start = error.token_.start_ - begin_; structured.offset_limit = error.token_.end_ - begin_; structured.message = error.message_; @@ -1873,20 +1868,34 @@ std::vector OurReader::getStructuredErrors() const { } class OurCharReader : public CharReader { - bool const collectComments_; - OurReader reader_; public: OurCharReader(bool collectComments, OurFeatures const& features) - : collectComments_(collectComments), reader_(features) {} - bool parse(char const* beginDoc, char const* endDoc, Value* root, - String* errs) override { - bool ok = reader_.parse(beginDoc, endDoc, *root, collectComments_); - if (errs) { - *errs = reader_.getFormattedErrorMessages(); + : CharReader(std::unique_ptr(new OurImpl(collectComments, features))) {} + +protected: + class OurImpl : public Impl { + public: + OurImpl(bool collectComments, OurFeatures const& features) + : collectComments_(collectComments), reader_(features) {} + + bool parse(char const* beginDoc, char const* endDoc, Value* root, + String* errs) override { + bool ok = reader_.parse(beginDoc, endDoc, *root, collectComments_); + if (errs) { + *errs = reader_.getFormattedErrorMessages(); + } + return ok; } - return ok; - } + + std::vector getStructuredErrors() const override { + return reader_.getStructuredErrors(); + } + + private: + bool const collectComments_; + OurReader reader_; + }; }; CharReaderBuilder::CharReaderBuilder() { setDefaults(&settings_); } @@ -1976,6 +1985,15 @@ void CharReaderBuilder::setDefaults(Json::Value* settings) { //! [CharReaderBuilderDefaults] } +std::vector CharReader::getStructuredErrors() const { + return _impl->getStructuredErrors(); +} + +bool CharReader::parse(char const* beginDoc, char const* endDoc, Value* root, + String* errs) { + return _impl->parse(beginDoc, endDoc, root, errs); +} + ////////////////////////////////// // global functions diff --git a/src/test_lib_json/main.cpp b/src/test_lib_json/main.cpp index d0f5364ac..872a3e9b9 100644 --- a/src/test_lib_json/main.cpp +++ b/src/test_lib_json/main.cpp @@ -3903,6 +3903,35 @@ JSONTEST_FIXTURE_LOCAL(FuzzTest, fuzzDoesntCrash) { example.size())); } +struct ParseWithStructuredErrorsTest : JsonTest::TestCase { + void testErrors(const std::string& doc, bool success, + const std::vector& expectedErrors) { + Json::CharReaderBuilder b; + CharReaderPtr reader(b.newCharReader()); + Json::Value root; + JSONTEST_ASSERT_EQUAL( + reader->parse(doc.data(), doc.data() + doc.length(), &root, nullptr), + success); + auto actualErrors = reader->getStructuredErrors(); + JSONTEST_ASSERT_EQUAL(expectedErrors.size(), actualErrors.size()); + for (std::size_t i = 0; i < actualErrors.size(); i++) { + const auto& a = actualErrors[i]; + const auto& e = expectedErrors[i]; + JSONTEST_ASSERT_EQUAL(a.offset_start, e.offset_start); + JSONTEST_ASSERT_EQUAL(a.offset_limit, e.offset_limit); + JSONTEST_ASSERT_STRING_EQUAL(a.message, e.message); + } + } +}; + +JSONTEST_FIXTURE_LOCAL(ParseWithStructuredErrorsTest, success) { + testErrors("{}", true, {}); +} + +JSONTEST_FIXTURE_LOCAL(ParseWithStructuredErrorsTest, singleError) { + testErrors("{ 1 : 2 }", false, {{2, 3, "Missing '}' or object member name"}}); +} + int main(int argc, const char* argv[]) { JsonTest::Runner runner; From 2ff6cbcb97e32e596ad7510d87b6ccb451a9cdca Mon Sep 17 00:00:00 2001 From: Jordan Bayles Date: Mon, 9 Sep 2024 18:47:01 -0700 Subject: [PATCH 2/2] run clang format --- include/json/reader.h | 14 +++++++------- src/lib_json/json_reader.cpp | 16 ++++++++++------ src/test_lib_json/main.cpp | 5 +++-- 3 files changed, 20 insertions(+), 15 deletions(-) diff --git a/include/json/reader.h b/include/json/reader.h index b4d68d4e3..38b9360cf 100644 --- a/include/json/reader.h +++ b/include/json/reader.h @@ -272,7 +272,7 @@ class JSON_API CharReader { /** \brief Returns a vector of structured errors encountered while parsing. * Each parse call resets the stored list of errors. - */ + */ std::vector getStructuredErrors() const; class JSON_API Factory { @@ -287,17 +287,17 @@ class JSON_API CharReader { protected: class Impl { public: - virtual ~Impl() = default; - virtual bool parse(char const* beginDoc, char const* endDoc, Value* root, - String* errs) = 0; - virtual std::vector getStructuredErrors() const = 0; + virtual ~Impl() = default; + virtual bool parse(char const* beginDoc, char const* endDoc, Value* root, + String* errs) = 0; + virtual std::vector getStructuredErrors() const = 0; }; - explicit CharReader(std::unique_ptr impl) : _impl(std::move(impl)) { } + explicit CharReader(std::unique_ptr impl) : _impl(std::move(impl)) {} private: std::unique_ptr _impl; -}; // CharReader +}; // CharReader /** \brief Build a CharReader implementation. * diff --git a/src/lib_json/json_reader.cpp b/src/lib_json/json_reader.cpp index 42a241326..4ab4dffd3 100644 --- a/src/lib_json/json_reader.cpp +++ b/src/lib_json/json_reader.cpp @@ -1831,7 +1831,8 @@ String OurReader::getFormattedErrorMessages() const { return formattedMessage; } -std::vector OurReader::getStructuredErrors() const { +std::vector +OurReader::getStructuredErrors() const { std::vector allErrors; for (const auto& error : errors_) { CharReader::StructuredError structured; @@ -1847,7 +1848,8 @@ class OurCharReader : public CharReader { public: OurCharReader(bool collectComments, OurFeatures const& features) - : CharReader(std::unique_ptr(new OurImpl(collectComments, features))) {} + : CharReader( + std::unique_ptr(new OurImpl(collectComments, features))) {} protected: class OurImpl : public Impl { @@ -1864,7 +1866,8 @@ class OurCharReader : public CharReader { return ok; } - std::vector getStructuredErrors() const override { + std::vector + getStructuredErrors() const override { return reader_.getStructuredErrors(); } @@ -1961,13 +1964,14 @@ void CharReaderBuilder::setDefaults(Json::Value* settings) { //! [CharReaderBuilderDefaults] } -std::vector CharReader::getStructuredErrors() const { - return _impl->getStructuredErrors(); +std::vector +CharReader::getStructuredErrors() const { + return _impl->getStructuredErrors(); } bool CharReader::parse(char const* beginDoc, char const* endDoc, Value* root, String* errs) { - return _impl->parse(beginDoc, endDoc, root, errs); + return _impl->parse(beginDoc, endDoc, root, errs); } ////////////////////////////////// diff --git a/src/test_lib_json/main.cpp b/src/test_lib_json/main.cpp index 21d05dfe4..fa41d19ed 100644 --- a/src/test_lib_json/main.cpp +++ b/src/test_lib_json/main.cpp @@ -3918,8 +3918,9 @@ JSONTEST_FIXTURE_LOCAL(FuzzTest, fuzzDoesntCrash) { } struct ParseWithStructuredErrorsTest : JsonTest::TestCase { - void testErrors(const std::string& doc, bool success, - const std::vector& expectedErrors) { + void testErrors( + const std::string& doc, bool success, + const std::vector& expectedErrors) { Json::CharReaderBuilder b; CharReaderPtr reader(b.newCharReader()); Json::Value root;