Keep the byte range in exceptions about the root value (#5987)

With JSON_DIAGNOSTICS and JSON_DIAGNOSTIC_POSITIONS both set, exception::diagnostics()
and the json_view diagnostics() returned early for a value without a path (the
root), dropping "(bytes a-b)". Build the "(path) " prefix only when there are
tokens and always append the byte range.

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

docs/mkdocs/docs/api/macros/json_diagnostic_positions.md

@@ -35,6 +35,9 @@ Note that enabling this macro increases the size of every JSON value by two `std
 overhead to parsing, copying JSON value objects, and the generation of error messages for exceptions. It also causes
 these values to be reported in those error messages.
 
+If [`JSON_DIAGNOSTICS`](json_diagnostics.md) is also set, the message contains the path and the byte range, such as
+`(/1) (bytes 4-8)`. An exception about the root value has no path and only the range, such as `(bytes 0-4)`.
+
 ## Default definition
 
 The default value is `0` (position diagnostics are switched off).

include/nlohmann/detail/exceptions.hpp

@@ -198,18 +198,19 @@ class exception : public std::exception
             // all other parent types (primitives) contribute no token
         }
 
-        if (tokens.empty())
+        // the root value has no path, but still has a byte range
+        std::string prefix;
+        if (!tokens.empty())
         {
-            return "";
+            const auto str = std::accumulate(tokens.rbegin(), tokens.rend(), std::string{},
+                                             [](const std::string & a, const std::string & b)
+            {
+                return concat(a, '/', detail::escape(b));
+            });
+            prefix = concat('(', str, ") ");
         }
 
-        auto str = std::accumulate(tokens.rbegin(), tokens.rend(), std::string{},
-                                   [](const std::string & a, const std::string & b)
-        {
-            return concat(a, '/', detail::escape(b));
-        });
-
-        return concat('(', str, ") ", get_byte_positions(leaf_element));
+        return concat(prefix, get_byte_positions(leaf_element));
 #else
         return get_byte_positions(leaf_element);
 #endif

include/nlohmann/detail/view/errors.hpp

@@ -234,8 +234,7 @@ void value_range(const document_data& d, const node* n, std::size_t& start, std:
 
 /// the context basic_json puts in front of the message of an exception about
 /// a value: "(/path) " with JSON_DIAGNOSTICS (none for the root), then
-/// "(bytes a-b) " with JSON_DIAGNOSTIC_POSITIONS (with JSON_DIAGNOSTICS, only
-/// if there is a path)
+/// "(bytes a-b) " with JSON_DIAGNOSTIC_POSITIONS (also for the root)
 ///
 /// This and the throw helpers that take an error_site are templates, so that
 /// the path and range code is instantiated only where an exception can be
@@ -257,18 +256,17 @@ std::string diagnostics(const error_site site)
 #if JSON_DIAGNOSTICS
     std::vector<std::string> tokens;
     const bool found = edited ? value_path<true>(d, site.value(), tokens) : value_path<false>(d, site.value(), tokens);
-    if (!found || tokens.empty())
+    if (found && !tokens.empty())
     {
-        // context is still empty; returning it keeps the copy elided (-Wnrvo)
-        return context;
+        // the root value has no path, but still has a byte range
+        std::string path;
+        for (const std::string& token : tokens)
+        {
+            path.push_back('/');
+            path += detail::escape(token);
+        }
+        context = concat('(', path, ") ");
     }
-    std::string path;
-    for (const std::string& token : tokens)
-    {
-        path.push_back('/');
-        path += detail::escape(token);
-    }
-    context = concat('(', path, ") ");
 #endif
 #if JSON_DIAGNOSTIC_POSITIONS
     std::size_t start = 0;

single_include/nlohmann/json.hpp

@@ -5695,18 +5695,19 @@ class exception : public std::exception
             // all other parent types (primitives) contribute no token
         }
 
-        if (tokens.empty())
+        // the root value has no path, but still has a byte range
+        std::string prefix;
+        if (!tokens.empty())
         {
-            return "";
+            const auto str = std::accumulate(tokens.rbegin(), tokens.rend(), std::string{},
+                                             [](const std::string & a, const std::string & b)
+            {
+                return concat(a, '/', detail::escape(b));
+            });
+            prefix = concat('(', str, ") ");
         }
 
-        auto str = std::accumulate(tokens.rbegin(), tokens.rend(), std::string{},
-                                   [](const std::string & a, const std::string & b)
-        {
-            return concat(a, '/', detail::escape(b));
-        });
-
-        return concat('(', str, ") ", get_byte_positions(leaf_element));
+        return concat(prefix, get_byte_positions(leaf_element));
 #else
         return get_byte_positions(leaf_element);
 #endif

single_include/nlohmann/json_view.hpp

@@ -5076,8 +5076,7 @@ void value_range(const document_data& d, const node* n, std::size_t& start, std:
 
 /// the context basic_json puts in front of the message of an exception about
 /// a value: "(/path) " with JSON_DIAGNOSTICS (none for the root), then
-/// "(bytes a-b) " with JSON_DIAGNOSTIC_POSITIONS (with JSON_DIAGNOSTICS, only
-/// if there is a path)
+/// "(bytes a-b) " with JSON_DIAGNOSTIC_POSITIONS (also for the root)
 ///
 /// This and the throw helpers that take an error_site are templates, so that
 /// the path and range code is instantiated only where an exception can be
@@ -5099,18 +5098,17 @@ std::string diagnostics(const error_site site)
 #if JSON_DIAGNOSTICS
     std::vector<std::string> tokens;
     const bool found = edited ? value_path<true>(d, site.value(), tokens) : value_path<false>(d, site.value(), tokens);
-    if (!found || tokens.empty())
+    if (found && !tokens.empty())
     {
-        // context is still empty; returning it keeps the copy elided (-Wnrvo)
-        return context;
+        // the root value has no path, but still has a byte range
+        std::string path;
+        for (const std::string& token : tokens)
+        {
+            path.push_back('/');
+            path += detail::escape(token);
+        }
+        context = concat('(', path, ") ");
     }
-    std::string path;
-    for (const std::string& token : tokens)
-    {
-        path.push_back('/');
-        path += detail::escape(token);
-    }
-    context = concat('(', path, ") ");
 #endif
 #if JSON_DIAGNOSTIC_POSITIONS
     std::size_t start = 0;

tests/src/unit-diagnostic-positions.cpp

@@ -193,6 +193,28 @@ TEST_CASE("Better diagnostics with positions")
         check_conversion(300);
     }
 
+    SECTION("exception about the root value (#5987)")
+    {
+        // the root has no path, but still has a byte range
+        const json root = json::parse("true");
+        const json nested = json::parse("[1, true]");
+        const json constructed = true;
+#if JSON_DIAGNOSTICS
+        CHECK_THROWS_WITH_AS(root.get<std::string>(),
+                             "[json.exception.type_error.302] (bytes 0-4) type must be string, but is boolean", json::type_error);
+        CHECK_THROWS_WITH_AS(nested[1].get<std::string>(),
+                             "[json.exception.type_error.302] (/1) (bytes 4-8) type must be string, but is boolean", json::type_error);
+#else
+        CHECK_THROWS_WITH_AS(root.get<std::string>(),
+                             "[json.exception.type_error.302] (bytes 0-4) type must be string, but is boolean", json::type_error);
+        CHECK_THROWS_WITH_AS(nested[1].get<std::string>(),
+                             "[json.exception.type_error.302] (bytes 4-8) type must be string, but is boolean", json::type_error);
+#endif
+        // a value without positions gets no range and no stray whitespace
+        CHECK_THROWS_WITH_AS(constructed.get<std::string>(),
+                             "[json.exception.type_error.302] type must be string, but is boolean", json::type_error);
+    }
+
     SECTION("JSON patch add to primitive parent (#4292)")
     {
         // the JSON Patch "add" target /foo/bar/baz has a string parent

tests/src/unit-json_view_positions.cpp

@@ -300,6 +300,27 @@ TEST_CASE("json_view materialize() source positions")
             static_cast<void>(j.at("list").at(1).get<int>());
         }));
     }
+
+    SECTION("exception about the root value (#5987)")
+    {
+        const json_document root = json_document::parse("true");
+        const json_document nested = json_document::parse("[1, true]");
+        CHECK(json_view_test::exception_of_call([&]
+        {
+            static_cast<void>(root.root().get<std::string>());
+        }) == "[json.exception.type_error.302] (bytes 0-4) type must be string, but is boolean");
+#if JSON_DIAGNOSTICS
+        CHECK(json_view_test::exception_of_call([&]
+        {
+            static_cast<void>(nested.root()[1].get<std::string>());
+        }) == "[json.exception.type_error.302] (/1) (bytes 4-8) type must be string, but is boolean");
+#else
+        CHECK(json_view_test::exception_of_call([&]
+        {
+            static_cast<void>(nested.root()[1].get<std::string>());
+        }) == "[json.exception.type_error.302] (bytes 4-8) type must be string, but is boolean");
+#endif
+    }
 #endif
 
     SECTION("edited documents")
@@ -550,12 +571,8 @@ TEST_CASE("json_view exceptions name the path and the bytes of the value")
         expected_zz += "(bytes 12-35) key 'zz' not found";
         CHECK(exception_of_call([&] { static_cast<void>(d.root().at(json::json_pointer("/people/0/zz"))); }) == expected_zz);
 
-        // the root has no path, and with JSON_DIAGNOSTICS then no bytes either (as basic_json)
-        std::string expected_missing = "[json.exception.out_of_range.403] ";
-#if !JSON_DIAGNOSTICS
-        expected_missing += "(bytes 0-79) ";
-#endif
-        expected_missing += "key 'missing' not found";
+        // the root has no path, but still its bytes (#5987)
+        const std::string expected_missing = "[json.exception.out_of_range.403] (bytes 0-79) key 'missing' not found";
         CHECK(exception_of_call([&] { static_cast<void>(d.root().at("missing")); }) == expected_missing);
     }