Report one-character non-numeric array indices like longer ones

A JSON pointer reference token that is not a number but has only one
character (e.g. "/a/x") was reported as out_of_range.404 ("unresolved
reference token"), because the "is not a number" check only ran for
tokens longer than one character; "/a/xy" got parse_error.109. Both now
throw parse_error.109. "-" and the empty token are still reported as
out_of_range.404. As a consequence, value(json_pointer, default) on an
array now throws for "/x" as it already did for "/xy".

Also document why ordered_map::erase's destroy/placement-new loop on
pair<const Key, T> is kept despite [basic.life]/8 before C++20.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

docs/mkdocs/docs/api/basic_json/unflatten.md

@@ -36,8 +36,9 @@ The function can throw the following exceptions:
 - Throws [`parse_error.109`](../../home/exceptions.md#jsonexceptionparse_error109) if an array index in a key is not a
   number; example: `"array index 'one' is not a number"`
 - Throws [`out_of_range.404`](../../home/exceptions.md#jsonexceptionout_of_range404) if a level becomes an array
-  (because one of its keys is `0`) and another key at that level cannot be an array index; example:
-  `"unresolved reference token 'x'"`
+  (because one of its keys is `0`) and another key at that level begins with a digit but is not a valid array index
+  (such as `1a`), or is `-`; example:
+  `"unresolved reference token '-'"`
 
 ## Complexity
 

include/nlohmann/detail/json_pointer.hpp

@@ -255,7 +255,7 @@ class json_pointer
     {
         ok,                ///< @a s is a valid, representable array index
         leading_zero,      ///< @a s begins with '0' but has more than one character
-        not_a_number,      ///< @a s does not begin with a digit
+        not_a_number,      ///< @a s is neither empty nor "-" and does not begin with a digit
         unresolved,        ///< @a s could not be converted to an integer
         exceeds_size_type  ///< @a s converts to an integer that exceeds size_type
     };
@@ -282,8 +282,10 @@ class json_pointer
             return array_index_status::leading_zero;
         }
 
-        // error condition (cf. RFC 6901, Sect. 4)
-        if (JSON_HEDLEY_UNLIKELY(s.size() > 1 && !(s[0] >= '1' && s[0] <= '9')))
+        // error condition (cf. RFC 6901, Sect. 4); this also covers single-
+        // character tokens, so "/x" and "/xy" fail alike; "-" and the empty
+        // token are left to the conversion below and are reported as unresolved
+        if (JSON_HEDLEY_UNLIKELY(!s.empty() && s != "-" && !(s[0] >= '0' && s[0] <= '9')))
         {
             return array_index_status::not_a_number;
         }

include/nlohmann/ordered_map.hpp

@@ -247,6 +247,14 @@ public:
         //               ^        ^
         //             first    last
 
+        // Note on conformance: before C++20, [basic.life]/8 did not allow an
+        // object of a type with a const member (like value_type's const Key)
+        // to transparently replace the destroyed one, so strictly, accessing
+        // it through the vector's existing pointers would have required
+        // std::launder (which does not exist before C++17). C++20 dropped that
+        // condition (P1971R0, NB comment US 041). Compilers have always treated
+        // this pattern as intended, so it is kept deliberately.
+
         // Since we cannot move const Keys, we re-construct them in place.
         // We start at first and re-construct (viz. copy) the elements from
         // the back of the vector. Example for the first iteration:

single_include/nlohmann/json.hpp

@@ -20173,7 +20173,7 @@ class json_pointer
     {
         ok,                ///< @a s is a valid, representable array index
         leading_zero,      ///< @a s begins with '0' but has more than one character
-        not_a_number,      ///< @a s does not begin with a digit
+        not_a_number,      ///< @a s is neither empty nor "-" and does not begin with a digit
         unresolved,        ///< @a s could not be converted to an integer
         exceeds_size_type  ///< @a s converts to an integer that exceeds size_type
     };
@@ -20200,8 +20200,10 @@ class json_pointer
             return array_index_status::leading_zero;
         }
 
-        // error condition (cf. RFC 6901, Sect. 4)
-        if (JSON_HEDLEY_UNLIKELY(s.size() > 1 && !(s[0] >= '1' && s[0] <= '9')))
+        // error condition (cf. RFC 6901, Sect. 4); this also covers single-
+        // character tokens, so "/x" and "/xy" fail alike; "-" and the empty
+        // token are left to the conversion below and are reported as unresolved
+        if (JSON_HEDLEY_UNLIKELY(!s.empty() && s != "-" && !(s[0] >= '0' && s[0] <= '9')))
         {
             return array_index_status::not_a_number;
         }
@@ -27780,6 +27782,14 @@ public:
         //               ^        ^
         //             first    last
 
+        // Note on conformance: before C++20, [basic.life]/8 did not allow an
+        // object of a type with a const member (like value_type's const Key)
+        // to transparently replace the destroyed one, so strictly, accessing
+        // it through the vector's existing pointers would have required
+        // std::launder (which does not exist before C++17). C++20 dropped that
+        // condition (P1971R0, NB comment US 041). Compilers have always treated
+        // this pattern as intended, so it is kept deliberately.
+
         // Since we cannot move const Keys, we re-construct them in place.
         // We start at first and re-construct (viz. copy) the elements from
         // the back of the vector. Example for the first iteration:

tests/src/unit-element_access2.cpp

@@ -535,6 +535,8 @@ TEST_CASE_TEMPLATE("element access 2", Json, nlohmann::json, nlohmann::ordered_j
                     // Test malformed index (non-numeric) throws parse_error
                     CHECK_THROWS_WITH_AS(j_array.value("/foo"_json_pointer, 1), "[json.exception.parse_error.109] parse error: array index 'foo' is not a number", typename Json::parse_error&);
                     CHECK_THROWS_WITH_AS(j_array_const.value("/foo"_json_pointer, 1), "[json.exception.parse_error.109] parse error: array index 'foo' is not a number", typename Json::parse_error&);
+                    CHECK_THROWS_WITH_AS(j_array.value("/x"_json_pointer, 1), "[json.exception.parse_error.109] parse error: array index 'x' is not a number", typename Json::parse_error&);
+                    CHECK_THROWS_WITH_AS(j_array_const.value("/x"_json_pointer, 1), "[json.exception.parse_error.109] parse error: array index 'x' is not a number", typename Json::parse_error&);
 
                     // Test leading-zero index throws parse_error
                     CHECK_THROWS_WITH_AS(j_array.value("/01"_json_pointer, 1), "[json.exception.parse_error.106] parse error: array index '01' must not begin with '0'", typename Json::parse_error&);

tests/src/unit-json_patch.cpp

@@ -1713,6 +1713,21 @@ TEST_CASE("JSON patch - move where 'from' is a proper prefix of 'path' (regressi
         CHECK(doc.patch(patch) == R"({"b": 1})"_json);
     }
 
+    SECTION("array index tokens that are not a number")
+    {
+        json const doc = R"({"a": [1, 2]})"_json;
+
+        // "-" cannot be removed and is reported as unresolved
+        json const patch_dash = {{{"op", "remove"}, {"path", "/a/-"}}};
+        CHECK_THROWS_WITH_AS(doc.patch(patch_dash), "[json.exception.out_of_range.404] unresolved reference token '-'", json::out_of_range&);
+
+        // a single character is reported like a longer token
+        json const patch_x = {{{"op", "remove"}, {"path", "/a/x"}}};
+        CHECK_THROWS_WITH_AS(doc.patch(patch_x), "[json.exception.parse_error.109] parse error: array index 'x' is not a number", json::parse_error&);
+        json const patch_xy = {{{"op", "remove"}, {"path", "/a/xy"}}};
+        CHECK_THROWS_WITH_AS(doc.patch(patch_xy), "[json.exception.parse_error.109] parse error: array index 'xy' is not a number", json::parse_error&);
+    }
+
     SECTION("the array-append token '-' is an ordinary child token")
     {
         // "-" (append-to-array) addresses a location *inside* the array,

tests/src/unit-json_pointer.cpp

@@ -421,6 +421,27 @@ TEST_CASE("JSON pointers")
             CHECK_THROWS_WITH_AS(json({{"/list/0", 1}, {"/list/1", 2}, {"/list/three", 3}}).unflatten(),
             "[json.exception.parse_error.109] parse error: array index 'three' is not a number", json::parse_error&);
 
+            // a single-character token that is not a digit is reported like a
+            // longer one (parse_error.109), not as unresolved (out_of_range.404)
+            CHECK_THROWS_WITH_AS(j["/x"_json_pointer] = 1,
+                                 "[json.exception.parse_error.109] parse error: array index 'x' is not a number", json::parse_error&);
+            CHECK_THROWS_WITH_AS(j_const["/x"_json_pointer] == 1,
+                                 "[json.exception.parse_error.109] parse error: array index 'x' is not a number", json::parse_error&);
+            CHECK_THROWS_WITH_AS(j.at("/x"_json_pointer) = 1,
+                                 "[json.exception.parse_error.109] parse error: array index 'x' is not a number", json::parse_error&);
+            CHECK_THROWS_WITH_AS(j_const.at("/x"_json_pointer) == 1,
+                                 "[json.exception.parse_error.109] parse error: array index 'x' is not a number", json::parse_error&);
+            CHECK_THROWS_WITH_AS(j.at("/+"_json_pointer),
+                                 "[json.exception.parse_error.109] parse error: array index '+' is not a number", json::parse_error&);
+            CHECK(!j.contains("/x"_json_pointer));
+            CHECK(!j_const.contains("/x"_json_pointer));
+            CHECK_THROWS_WITH_AS(json({{"/list/0", 1}, {"/list/x", 2}}).unflatten(),
+            "[json.exception.parse_error.109] parse error: array index 'x' is not a number", json::parse_error&);
+
+            // "-" is a valid reference token, so it is still reported as unresolved
+            CHECK_THROWS_WITH_AS(json({{"/list/0", 1}, {"/list/-", 2}}).unflatten(),
+            "[json.exception.out_of_range.404] unresolved reference token '-'", json::out_of_range&);
+
             // assign to "-"
             j["/-"_json_pointer] = 99;
             CHECK(j == json({1, 13, 3, 33, nullptr, 55, 99}));