From a410d0727b9248cfa9f119d75bc6854543f84c5b Mon Sep 17 00:00:00 2001 From: Tom Lachecki Date: Wed, 6 Sep 2023 14:48:18 +0100 Subject: [PATCH] Fixed basic_json::diff and basic_json::patch not compiling with custom string type (#4134) --- include/nlohmann/json.hpp | 39 ++++++++++++++++++++----------- tests/src/unit-alt-string.cpp | 44 ++++++++++++++++++++++++++++++++--- 2 files changed, 67 insertions(+), 16 deletions(-) diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index a3c0af1e00..ada82efdb9 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -4711,7 +4711,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec // the valid JSON Patch operations enum class patch_operations {add, remove, replace, move, copy, test, invalid}; - const auto get_op = [](const std::string & op) + const auto get_op = [](const StringType & op) { if (op == "add") { @@ -4848,8 +4848,8 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec for (const auto& val : json_patch) { // wrapper to get a value for an operation - const auto get_value = [&val](const std::string & op, - const std::string & member, + const auto get_value = [&val](const StringType & op, + const StringType& member, bool string_type) -> basic_json & { // find value @@ -4883,8 +4883,8 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } // collect mandatory members - const auto op = get_value("op", "op", true).template get(); - const auto path = get_value(op, "path", true).template get(); + const auto op = get_value("op", "op", true).template get(); + const auto path = get_value(op, "path", true).template get(); json_pointer ptr(path); switch (get_op(op)) @@ -4910,7 +4910,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec case patch_operations::move: { - const auto from_path = get_value("move", "from", true).template get(); + const auto from_path = get_value("move", "from", true).template get(); json_pointer from_ptr(from_path); // the "from" location must exist - use at() @@ -4927,7 +4927,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec case patch_operations::copy: { - const auto from_path = get_value("copy", "from", true).template get(); + const auto from_path = get_value("copy", "from", true).template get(); const json_pointer from_ptr(from_path); // the "from" location must exist - use at() @@ -4987,7 +4987,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec /// @sa https://json.nlohmann.me/api/basic_json/diff/ JSON_HEDLEY_WARN_UNUSED_RESULT static basic_json diff(const basic_json& source, const basic_json& target, - const std::string& path = "") + const StringType& path = "") { // the patch basic_json result(value_t::array); @@ -5016,8 +5016,14 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec std::size_t i = 0; while (i < source.size() && i < target.size()) { + StringType array_index_str; + { + using namespace detail; + int_to_string(array_index_str, i); + } + // recursive call to compare array values at index i - auto temp_diff = diff(source[i], target[i], detail::concat(path, '/', std::to_string(i))); + auto temp_diff = diff(source[i], target[i], detail::concat(path, '/', array_index_str)); result.insert(result.end(), temp_diff.begin(), temp_diff.end()); ++i; } @@ -5031,10 +5037,17 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec { // add operations in reverse order to avoid invalid // indices + + StringType array_index_str; + { + using namespace detail; + int_to_string(array_index_str, i); + } + result.insert(result.begin() + end_index, object( { {"op", "remove"}, - {"path", detail::concat(path, '/', std::to_string(i))} + {"path", detail::concat(path, '/', array_index_str)} })); ++i; } @@ -5045,7 +5058,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec result.push_back( { {"op", "add"}, - {"path", detail::concat(path, "/-")}, + {"path", detail::concat(path, "/-")}, {"value", target[i]} }); ++i; @@ -5060,7 +5073,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec for (auto it = source.cbegin(); it != source.cend(); ++it) { // escape the key name to be used in a JSON patch - const auto path_key = detail::concat(path, '/', detail::escape(it.key())); + const auto path_key = detail::concat(path, '/', detail::escape(it.key())); if (target.find(it.key()) != target.end()) { @@ -5084,7 +5097,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec if (source.find(it.key()) == source.end()) { // found a key that is not in this -> add it - const auto path_key = detail::concat(path, '/', detail::escape(it.key())); + const auto path_key = detail::concat(path, '/', detail::escape(it.key())); result.push_back( { {"op", "add"}, {"path", path_key}, diff --git a/tests/src/unit-alt-string.cpp b/tests/src/unit-alt-string.cpp index 291c0ad5fc..a58ad1919d 100644 --- a/tests/src/unit-alt-string.cpp +++ b/tests/src/unit-alt-string.cpp @@ -36,10 +36,33 @@ class alt_string alt_string(size_t count, char chr): str_impl(count, chr) {} alt_string() = default; - template - alt_string& append(TParams&& ...params) + alt_string& append(std::size_t count, char ch) { - str_impl.append(std::forward(params)...); + str_impl.append(count, ch); + return *this; + } + + alt_string& append(const alt_string& str) + { + str_impl.append(str.str_impl); + return *this; + } + + alt_string& append(const char* s, std::size_t count) + { + str_impl.append(s, count); + return *this; + } + + alt_string& append(const char* s) + { + str_impl.append(s); + return *this; + } + + alt_string& operator+=(char ch) + { + str_impl += ch; return *this; } @@ -75,6 +98,11 @@ class alt_string return str_impl.size(); } + void reserve (std::size_t n) + { + str_impl.reserve(n); + } + void resize (std::size_t n) { str_impl.resize(n); @@ -321,4 +349,14 @@ TEST_CASE("alternative string type") CHECK(j.at(alt_json::json_pointer("/foo/0")) == j["foo"][0]); CHECK(j.at(alt_json::json_pointer("/foo/1")) == j["foo"][1]); } + + SECTION("JSON patch") + { + // for now, just ensures successful compilation (see #4134) + alt_json base, target; + alt_json patch = alt_json::array(); + + base.patch(patch); + alt_json::diff(target, base); + } }