use diagnostic positions in exceptions (#4585)

* add a ci step for Json_Diagnostic_Positions Signed-off-by: Harinath Nampally <harinath922@gmail.com> * Update ci.cmake to address review comments Signed-off-by: Harinath Nampally <harinath922@gmail.com> * address review comment Signed-off-by: Harinath Nampally <harinath922@gmail.com> * fix typo in the comment Signed-off-by: Harinath Nampally <harinath922@gmail.com> * fix typos in ci.cmake Signed-off-by: Harinath Nampally <harinath922@gmail.com> * invoke the new ci step from ubuntu.yml Signed-off-by: Harinath Nampally <harinath922@gmail.com> * issue4561 - use diagnostic positions for exceptions Signed-off-by: Harinath Nampally <harinath922@gmail.com> * fix ci_test_documentation check Signed-off-by: Harinath Nampally <harinath922@gmail.com> * address review comments Signed-off-by: Harinath Nampally <harinath922@gmail.com> * fix ci check failures for unit-diagnostic-postions.cpp Signed-off-by: Harinath Nampally <harinath922@gmail.com> * improvements based on review comments Signed-off-by: Harinath Nampally <harinath922@gmail.com> * fix const correctness string Signed-off-by: Harinath Nampally <harinath922@gmail.com> * further refinements based on reviews Signed-off-by: Harinath Nampally <harinath922@gmail.com> * add one more test case for full coverage Signed-off-by: Harinath Nampally <harinath922@gmail.com> * ci check fix - add const Signed-off-by: Harinath Nampally <harinath922@gmail.com> * add unit tests for json_diagnostic_postions only Signed-off-by: Harinath Nampally <harinath922@gmail.com> * fix ci_test_diagnostics Signed-off-by: Harinath Nampally <harinath922@gmail.com> * fix ci_test_build_documentation check Signed-off-by: Harinath Nampally <harinath922@gmail.com> --------- Signed-off-by: Harinath Nampally <harinath922@gmail.com>
nlohmann · Jan 19, 2025 · d23291b · d23291b
1 parent 0f9e6ae
commit d23291b
Show file tree

Hide file tree

Showing 13 changed files with 261 additions and 9 deletions.
diff --git a/docs/mkdocs/docs/api/macros/json_diagnostic_positions.md b/docs/mkdocs/docs/api/macros/json_diagnostic_positions.md
@@ -81,6 +81,33 @@ When the macro is not defined, the library will define it to its default value.
 
     The output shows the start/end positions of all the objects and fields in the JSON string.
 
+??? example "Example 2: using only diagnostic positions in exceptions"
+
+    ```cpp
+    --8<-- "examples/diagnostic_positions_exception.cpp"
+    ```
+
+    Output:
+
+    ```
+    --8<-- "examples/diagnostic_positions_exception.output"
+    ```
+
+        The output shows the exception with start/end positions only.
+
+??? example "Example 3: using extended diagnostics with positions enabled in exceptions"
+
+    ```cpp
+    --8<-- "examples/diagnostics_extended_positions.cpp"
+    ```
+
+    Output:
+
+    ```
+    --8<-- "examples/diagnostics_extended_positions.output"
+    ```
+
+        The output shows the exception with diagnostic path info and start/end positions.
 ## Version history
 
 - Added in version 3.12.0.
diff --git a/docs/mkdocs/docs/api/macros/json_diagnostics.md b/docs/mkdocs/docs/api/macros/json_diagnostics.md
@@ -70,6 +70,19 @@ When the macro is not defined, the library will define it to its default value.
 
     Now the exception message contains a JSON Pointer `/address/housenumber` that indicates which value has the wrong type.
 
+??? example "Example 3: using only diagnostic positions in exceptions"
+
+    ```cpp
+    --8<-- "examples/diagnostic_positions_exception.cpp"
+    ```
+
+    Output:
+
+    ```
+    --8<-- "examples/diagnostic_positions_exception.output"
+    ```    
+        The output shows the exception with start/end positions only.
+        
 ## Version history
 
 - Added in version 3.10.0.

diff --git a/docs/mkdocs/docs/examples/diagnostic_positions_exception.cpp b/docs/mkdocs/docs/examples/diagnostic_positions_exception.cpp
@@ -0,0 +1,30 @@
+#include <iostream>
+
+#define JSON_DIAGNOSTIC_POSITIONS 1
+#include <nlohmann/json.hpp>
+
+using json = nlohmann::json;
+
+/* Demonstration of type error exception with diagnostic postions support enabled */
+int main()
+{
+    //Invalid json string - housenumber type must be int instead of string
+    const std::string json_invalid_string = R"(
+    {
+        "address": {
+            "street": "Fake Street",
+            "housenumber": "1"
+        }
+    }
+    )";
+    json j = json::parse(json_invalid_string);
+    try
+    {
+        int housenumber = j["address"]["housenumber"];
+        std::cout << housenumber;
+    }
+    catch (const json::exception& e)
+    {
+        std::cout << e.what() << '\n';
+    }
+}
diff --git a/docs/mkdocs/docs/examples/diagnostic_positions_exception.output b/docs/mkdocs/docs/examples/diagnostic_positions_exception.output
@@ -0,0 +1 @@
+[json.exception.type_error.302] (bytes 92-95) type must be number, but is string
diff --git a/docs/mkdocs/docs/examples/diagnostics_extended_positions.cpp b/docs/mkdocs/docs/examples/diagnostics_extended_positions.cpp
@@ -0,0 +1,31 @@
+#include <iostream>
+
+#define JSON_DIAGNOSTICS 1
+#define JSON_DIAGNOSTIC_POSITIONS 1
+#include <nlohmann/json.hpp>
+
+using json = nlohmann::json;
+
+/* Demonstration of type error exception with diagnostic postions support enabled */
+int main()
+{
+    //Invalid json string - housenumber type must be int instead of string
+    const std::string json_invalid_string = R"(
+    {
+        "address": {
+            "street": "Fake Street",
+            "housenumber": "1"
+        }
+    }
+    )";
+    json j = json::parse(json_invalid_string);
+    try
+    {
+        int housenumber = j["address"]["housenumber"];
+        std::cout << housenumber;
+    }
+    catch (const json::exception& e)
+    {
+        std::cout << e.what() << '\n';
+    }
+}
diff --git a/docs/mkdocs/docs/examples/diagnostics_extended_positions.output b/docs/mkdocs/docs/examples/diagnostics_extended_positions.output
@@ -0,0 +1 @@
+[json.exception.type_error.302] (/address/housenumber) (bytes 92-95) type must be number, but is string
diff --git a/include/nlohmann/detail/exceptions.hpp b/include/nlohmann/detail/exceptions.hpp
@@ -131,16 +131,34 @@ class exception : public std::exception
         {
             return concat(a, '/', detail::escape(b));
         });
-        return concat('(', str, ") ");
+
+        return concat('(', str, ") ", get_byte_positions(leaf_element));
 #else
-        static_cast<void>(leaf_element);
-        return "";
+        return get_byte_positions(leaf_element);
 #endif
     }
 
   private:
     /// an exception object as storage for error messages
     std::runtime_error m;
+#if JSON_DIAGNOSTIC_POSITIONS
+    template<typename BasicJsonType>
+    static std::string get_byte_positions(const BasicJsonType* leaf_element)
+    {
+        if ((leaf_element->start_pos() != std::string::npos) && (leaf_element->end_pos() != std::string::npos))
+        {
+            return concat("(bytes ", std::to_string(leaf_element->start_pos()), "-", std::to_string(leaf_element->end_pos()), ") ");
+        }
+        return "";
+    }
+#else
+    template<typename BasicJsonType>
+    static std::string get_byte_positions(const BasicJsonType* leaf_element)
+    {
+        static_cast<void>(leaf_element);
+        return "";
+    }
+#endif
 };
 
 /// @brief exception indicating a parse error

diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp
@@ -4586,16 +4586,34 @@ class exception : public std::exception
         {
             return concat(a, '/', detail::escape(b));
         });
-        return concat('(', str, ") ");
+
+        return concat('(', str, ") ", get_byte_positions(leaf_element));
 #else
-        static_cast<void>(leaf_element);
-        return "";
+        return get_byte_positions(leaf_element);
 #endif
     }
 
   private:
     /// an exception object as storage for error messages
     std::runtime_error m;
+#if JSON_DIAGNOSTIC_POSITIONS
+    template<typename BasicJsonType>
+    static std::string get_byte_positions(const BasicJsonType* leaf_element)
+    {
+        if ((leaf_element->start_pos() != std::string::npos) && (leaf_element->end_pos() != std::string::npos))
+        {
+            return concat("(bytes ", std::to_string(leaf_element->start_pos()), "-", std::to_string(leaf_element->end_pos()), ") ");
+        }
+        return "";
+    }
+#else
+    template<typename BasicJsonType>
+    static std::string get_byte_positions(const BasicJsonType* leaf_element)
+    {
+        static_cast<void>(leaf_element);
+        return "";
+    }
+#endif
 };
 
 /// @brief exception indicating a parse error

diff --git a/tests/src/unit-diagnostic-positions-only.cpp b/tests/src/unit-diagnostic-positions-only.cpp
@@ -0,0 +1,44 @@
+//     __ _____ _____ _____
+//  __|  |   __|     |   | |  JSON for Modern C++ (supporting code)
+// |  |  |__   |  |  | | | |  version 3.11.3
+// |_____|_____|_____|_|___|  https://github.com/nlohmann/json
+//
+// SPDX-FileCopyrightText: 2013 - 2024 Niels Lohmann <https://nlohmann.me>
+// SPDX-License-Identifier: MIT
+
+#include "doctest_compatibility.h"
+
+#ifdef JSON_DIAGNOSTICS
+    #undef JSON_DIAGNOSTICS
+#endif
+
+#define JSON_DIAGNOSTICS 0
+#define JSON_DIAGNOSTIC_POSITIONS 1
+#include <nlohmann/json.hpp>
+
+using json = nlohmann::json;
+
+TEST_CASE("Better diagnostics with positions only")
+{
+    SECTION("invalid type")
+    {
+        const std::string json_invalid_string = R"(
+        {
+            "address": {
+                "street": "Fake Street",
+                "housenumber": "1"
+            }
+        }
+        )";
+        json j = json::parse(json_invalid_string);
+        CHECK_THROWS_WITH_AS(j.at("address").at("housenumber").get<int>(),
+                             "[json.exception.type_error.302] (bytes 108-111) type must be number, but is string", json::type_error);
+    }
+
+    SECTION("invalid type without positions")
+    {
+        const json j = "foo";
+        CHECK_THROWS_WITH_AS(j.get<int>(),
+                             "[json.exception.type_error.302] type must be number, but is string", json::type_error);
+    }
+}
diff --git a/tests/src/unit-diagnostic-positions.cpp b/tests/src/unit-diagnostic-positions.cpp
@@ -0,0 +1,40 @@
+//     __ _____ _____ _____
+//  __|  |   __|     |   | |  JSON for Modern C++ (supporting code)
+// |  |  |__   |  |  | | | |  version 3.11.3
+// |_____|_____|_____|_|___|  https://github.com/nlohmann/json
+//
+// SPDX-FileCopyrightText: 2013 - 2024 Niels Lohmann <https://nlohmann.me>
+// SPDX-License-Identifier: MIT
+
+#include "doctest_compatibility.h"
+
+#define JSON_DIAGNOSTICS 1
+#define JSON_DIAGNOSTIC_POSITIONS 1
+#include <nlohmann/json.hpp>
+
+using json = nlohmann::json;
+
+TEST_CASE("Better diagnostics with positions")
+{
+    SECTION("invalid type")
+    {
+        const std::string json_invalid_string = R"(
+        {
+            "address": {
+                "street": "Fake Street",
+                "housenumber": "1"
+            }
+        }
+        )";
+        json j = json::parse(json_invalid_string);
+        CHECK_THROWS_WITH_AS(j.at("address").at("housenumber").get<int>(),
+                             "[json.exception.type_error.302] (/address/housenumber) (bytes 108-111) type must be number, but is string", json::type_error);
+    }
+
+    SECTION("invalid type without positions")
+    {
+        const json j = "foo";
+        CHECK_THROWS_WITH_AS(j.get<int>(),
+                             "[json.exception.type_error.302] type must be number, but is string", json::type_error);
+    }
+}
diff --git a/tests/src/unit-json_patch.cpp b/tests/src/unit-json_patch.cpp
@@ -60,14 +60,20 @@ TEST_CASE("JSON patch")
             json const doc2 = R"({ "q": { "bar": 2 } })"_json;
 
             // because "a" does not exist.
+#if JSON_DIAGNOSTIC_POSITIONS
+            CHECK_THROWS_WITH_AS(doc2.patch(patch1), "[json.exception.out_of_range.403] (bytes 0-21) key 'a' not found", json::out_of_range&);
+#else
             CHECK_THROWS_WITH_AS(doc2.patch(patch1), "[json.exception.out_of_range.403] key 'a' not found", json::out_of_range&);
+#endif
 
             json const doc3 = R"({ "a": {} })"_json;
             json const patch2 = R"([{ "op": "add", "path": "/a/b/c", "value": 1 }])"_json;
 
             // should cause an error because "b" does not exist in doc3
 #if JSON_DIAGNOSTICS
             CHECK_THROWS_WITH_AS(doc3.patch(patch2), "[json.exception.out_of_range.403] (/a) key 'b' not found", json::out_of_range&);
+#elif JSON_DIAGNOSTIC_POSITIONS
+            CHECK_THROWS_WITH_AS(doc3.patch(patch2), "[json.exception.out_of_range.403] (bytes 7-9) key 'b' not found", json::out_of_range&);
 #else
             CHECK_THROWS_WITH_AS(doc3.patch(patch2), "[json.exception.out_of_range.403] key 'b' not found", json::out_of_range&);
 #endif
@@ -333,6 +339,8 @@ TEST_CASE("JSON patch")
             CHECK_THROWS_AS(doc.patch(patch), json::other_error&);
 #if JSON_DIAGNOSTICS
             CHECK_THROWS_WITH_STD_STR(doc.patch(patch), "[json.exception.other_error.501] (/0) unsuccessful: " + patch[0].dump());
+#elif JSON_DIAGNOSTIC_POSITIONS
+            CHECK_THROWS_WITH_STD_STR(doc.patch(patch), "[json.exception.other_error.501] (bytes 47-95) unsuccessful: " + patch[0].dump());
 #else
             CHECK_THROWS_WITH_STD_STR(doc.patch(patch), "[json.exception.other_error.501] unsuccessful: " + patch[0].dump());
 #endif
@@ -417,8 +425,11 @@ TEST_CASE("JSON patch")
             // applied), because the "add" operation's target location that
             // references neither the root of the document, nor a member of
             // an existing object, nor a member of an existing array.
-
+#if JSON_DIAGNOSTIC_POSITIONS
+            CHECK_THROWS_WITH_AS(doc.patch(patch), "[json.exception.out_of_range.403] (bytes 21-37) key 'baz' not found", json::out_of_range&);
+#else
             CHECK_THROWS_WITH_AS(doc.patch(patch), "[json.exception.out_of_range.403] key 'baz' not found", json::out_of_range&);
+#endif
         }
 
         // A.13. Invalid JSON Patch Document
@@ -476,6 +487,8 @@ TEST_CASE("JSON patch")
             CHECK_THROWS_AS(doc.patch(patch), json::other_error&);
 #if JSON_DIAGNOSTICS
             CHECK_THROWS_WITH_STD_STR(doc.patch(patch), "[json.exception.other_error.501] (/0) unsuccessful: " + patch[0].dump());
+#elif JSON_DIAGNOSTIC_POSITIONS
+            CHECK_THROWS_WITH_STD_STR(doc.patch(patch), "[json.exception.other_error.501] (bytes 47-92) unsuccessful: " + patch[0].dump());
 #else
             CHECK_THROWS_WITH_STD_STR(doc.patch(patch), "[json.exception.other_error.501] unsuccessful: " + patch[0].dump());
 #endif
@@ -1205,6 +1218,8 @@ TEST_CASE("JSON patch")
                 CHECK_THROWS_AS(doc.patch(patch), json::other_error&);
 #if JSON_DIAGNOSTICS
                 CHECK_THROWS_WITH_STD_STR(doc.patch(patch), "[json.exception.other_error.501] (/0) unsuccessful: " + patch[0].dump());
+#elif JSON_DIAGNOSTIC_POSITIONS
+                CHECK_THROWS_WITH_STD_STR(doc.patch(patch), "[json.exception.other_error.501] (bytes 47-117) unsuccessful: " + patch[0].dump());
 #else
                 CHECK_THROWS_WITH_STD_STR(doc.patch(patch), "[json.exception.other_error.501] unsuccessful: " + patch[0].dump());
 #endif

diff --git a/tests/src/unit-json_pointer.cpp b/tests/src/unit-json_pointer.cpp
@@ -203,11 +203,15 @@ TEST_CASE("JSON pointers")
             // escaped access
             CHECK(j[json::json_pointer("/a~1b")] == j["a/b"]);
             CHECK(j[json::json_pointer("/m~0n")] == j["m~n"]);
-
+#if JSON_DIAGNOSTIC_POSITIONS
+            // unescaped access
+            CHECK_THROWS_WITH_AS(j.at(json::json_pointer("/a/b")),
+                                 "[json.exception.out_of_range.403] (bytes 13-297) key 'a' not found", json::out_of_range&);
+#else
             // unescaped access
             CHECK_THROWS_WITH_AS(j.at(json::json_pointer("/a/b")),
                                  "[json.exception.out_of_range.403] key 'a' not found", json::out_of_range&);
-
+#endif
             // unresolved access
             const json j_primitive = 1;
             CHECK_THROWS_WITH_AS(j_primitive["/foo"_json_pointer],

diff --git a/tests/src/unit-regression1.cpp b/tests/src/unit-regression1.cpp
@@ -1400,14 +1400,24 @@ TEST_CASE("regression tests 1")
         auto p1 = R"([{"op": "move",
                        "from": "/one/two/three",
                        "path": "/a/b/c"}])"_json;
+#if JSON_DIAGNOSTIC_POSITIONS
+        CHECK_THROWS_WITH_AS(model.patch(p1),
+                             "[json.exception.out_of_range.403] (bytes 0-158) key 'a' not found", json::out_of_range&);
+#else
         CHECK_THROWS_WITH_AS(model.patch(p1),
                              "[json.exception.out_of_range.403] key 'a' not found", json::out_of_range&);
+#endif
 
         auto p2 = R"([{"op": "copy",
                        "from": "/one/two/three",
                        "path": "/a/b/c"}])"_json;
+#if JSON_DIAGNOSTIC_POSITIONS
+        CHECK_THROWS_WITH_AS(model.patch(p2),
+                             "[json.exception.out_of_range.403] (bytes 0-158) key 'a' not found", json::out_of_range&);
+#else
         CHECK_THROWS_WITH_AS(model.patch(p2),
                              "[json.exception.out_of_range.403] key 'a' not found", json::out_of_range&);
+#endif
     }
 
     SECTION("issue #961 - incorrect parsing of indefinite length CBOR strings")
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		[json.exception.type_error.302] (bytes 92-95) type must be number, but is string
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		[json.exception.type_error.302] (/address/housenumber) (bytes 92-95) type must be number, but is string