[core] show line and col instead of offset when JSON parsing failsupstream/json-parse-line-col

10 files changed, 20 insertions, 12 deletions

diff --git a/src/mbgl/sprite/sprite_parser.cpp b/src/mbgl/sprite/sprite_parser.cpp
index 99e2b0c8ca..d8d643214e 100644
--- a/src/mbgl/sprite/sprite_parser.cpp
+++ b/src/mbgl/sprite/sprite_parser.cpp
@@ -90,7 +90,7 @@ std::vector<std::unique_ptr<style::Image>> parseSprite(const std::string& encode
     JSDocument doc;
     doc.Parse<0>(json.c_str());
     if (doc.HasParseError()) {
-        throw std::runtime_error("Failed to parse JSON: " + formatJSONParseError(doc));
+        throw std::runtime_error("Failed to parse JSON " + formatJSONParseError(json, doc));
     } else if (!doc.IsObject()) {
         throw std::runtime_error("Sprite JSON root must be an object");
     } else {
diff --git a/src/mbgl/style/conversion/json.hpp b/src/mbgl/style/conversion/json.hpp
index 503f60a382..4e7ad9479a 100644
--- a/src/mbgl/style/conversion/json.hpp
+++ b/src/mbgl/style/conversion/json.hpp
@@ -15,7 +15,7 @@ optional<T> convertJSON(const std::string& json, Error& error, Args&&...args) {
     document.Parse<0>(json.c_str());
 
     if (document.HasParseError()) {
-        error = { formatJSONParseError(document) };
+        error = { formatJSONParseError(json, document) };
         return {};
     }
 
diff --git a/src/mbgl/style/parser.cpp b/src/mbgl/style/parser.cpp
index ae298bd915..2d3d251bc3 100644
--- a/src/mbgl/style/parser.cpp
+++ b/src/mbgl/style/parser.cpp
@@ -29,7 +29,7 @@ StyleParseResult Parser::parse(const std::string& json) {
     document.Parse<0>(json.c_str());
 
     if (document.HasParseError()) {
-        return std::make_exception_ptr(std::runtime_error(formatJSONParseError(document)));
+        return std::make_exception_ptr(std::runtime_error(formatJSONParseError(json, document)));
     }
 
     if (!document.IsObject()) {
diff --git a/src/mbgl/style/style_impl.cpp b/src/mbgl/style/style_impl.cpp
index fde5aa632d..8aa5f0c44b 100644
--- a/src/mbgl/style/style_impl.cpp
+++ b/src/mbgl/style/style_impl.cpp
@@ -77,7 +77,7 @@ void Style::Impl::parse(const std::string& json_) {
     Parser parser;
 
     if (auto error = parser.parse(json_)) {
-        std::string message = "Failed to parse style: " + util::toString(error);
+        std::string message = "Failed to parse style " + util::toString(error);
         Log::Error(Event::ParseStyle, message.c_str());
         observer->onStyleError(std::make_exception_ptr(util::StyleParseException(message)));
         observer->onResourceError(error);
diff --git a/src/mbgl/util/rapidjson.cpp b/src/mbgl/util/rapidjson.cpp
index 32a3c0b929..0504f8c002 100644
--- a/src/mbgl/util/rapidjson.cpp
+++ b/src/mbgl/util/rapidjson.cpp
@@ -1,11 +1,17 @@
 #include <mbgl/util/rapidjson.hpp>
 #include <mbgl/util/string.hpp>
 
+#include <algorithm>
+
 namespace mbgl {
 
-std::string formatJSONParseError(const JSDocument& doc) {
-    return std::string{ rapidjson::GetParseError_En(doc.GetParseError()) } + " at offset " +
-           util::toString(doc.GetErrorOffset());
+std::string formatJSONParseError(const std::string& json, const JSDocument& doc) {
+    const size_t offset = std::min(doc.GetErrorOffset(), json.length());
+    const size_t lastBreak = json.find_last_of('\n', offset);
+    const size_t col = offset - (lastBreak == std::string::npos ? 0 : lastBreak);
+    const size_t line = lastBreak == std::string::npos ? 1 : std::count(json.data(), json.data() + lastBreak, '\n') + 2;
+    return "on line " + util::toString(line) + " col " + util::toString(col) + ": " +
+           rapidjson::GetParseError_En(doc.GetParseError());
 }
 
 } // namespace mbgl
diff --git a/src/mbgl/util/rapidjson.hpp b/src/mbgl/util/rapidjson.hpp
index 2fb2a07c9f..8c8d576cfa 100644
--- a/src/mbgl/util/rapidjson.hpp
+++ b/src/mbgl/util/rapidjson.hpp
@@ -3,11 +3,13 @@
 #include <rapidjson/document.h>
 #include <rapidjson/error/en.h>
 
+#include <string>
+
 namespace mbgl {
 
 using JSDocument = rapidjson::GenericDocument<rapidjson::UTF8<>, rapidjson::CrtAllocator>;
 using JSValue = rapidjson::GenericValue<rapidjson::UTF8<>, rapidjson::CrtAllocator>;
 
-std::string formatJSONParseError(const JSDocument&);
+std::string formatJSONParseError(const std::string&, const JSDocument&);
 
 } // namespace mbgl
diff --git a/test/map/map.test.cpp b/test/map/map.test.cpp
index 3af2acdb8b..b5b2c77428 100644
--- a/test/map/map.test.cpp
+++ b/test/map/map.test.cpp
@@ -373,7 +373,7 @@ TEST(Map, SetStyleInvalidJSON) {
     auto observer = Log::removeObserver();
     auto flo = static_cast<FixtureLogObserver*>(observer.get());
     EXPECT_EQ(1u, flo->count({ EventSeverity::Error, Event::ParseStyle, -1,
-        "Failed to parse style: Invalid value. at offset 0" }));
+        "Failed to parse style on line 1 col 0: Invalid value." }));
     auto unchecked = flo->unchecked();
     EXPECT_TRUE(unchecked.empty()) << unchecked;
 }
diff --git a/test/sprite/sprite_loader.test.cpp b/test/sprite/sprite_loader.test.cpp
index 3691572265..a5d4a56e6d 100644
--- a/test/sprite/sprite_loader.test.cpp
+++ b/test/sprite/sprite_loader.test.cpp
@@ -140,7 +140,7 @@ TEST(SpriteLoader, JSONLoadingCorrupted) {
 
     test.observer.spriteError = [&] (std::exception_ptr error) {
         EXPECT_TRUE(error != nullptr);
-        EXPECT_EQ("Failed to parse JSON: Invalid value. at offset 0", util::toString(error));
+        EXPECT_EQ("Failed to parse JSON on line 1 col 0: Invalid value.", util::toString(error));
         test.end();
     };
 
diff --git a/test/sprite/sprite_parser.test.cpp b/test/sprite/sprite_parser.test.cpp
index 529e4c75e8..3e97ab9437 100644
--- a/test/sprite/sprite_parser.test.cpp
+++ b/test/sprite/sprite_parser.test.cpp
@@ -289,7 +289,7 @@ TEST(Sprite, SpriteParsingInvalidJSON) {
         FAIL() << "Expected exception";
     } catch (std::runtime_error& err) {
         EXPECT_STREQ(
-            "Failed to parse JSON: Missing a closing quotation mark in string. at offset 14",
+            "Failed to parse JSON on line 1 col 14: Missing a closing quotation mark in string.",
             err.what());
     }
 }
diff --git a/test/style/source.test.cpp b/test/style/source.test.cpp
index f0ff1f81b4..9caba10a3f 100644
--- a/test/style/source.test.cpp
+++ b/test/style/source.test.cpp
@@ -130,7 +130,7 @@ TEST(Source, LoadingCorrupt) {
 
     test.styleObserver.sourceError = [&] (Source& source, std::exception_ptr error) {
         EXPECT_EQ("source", source.getID());
-        EXPECT_EQ("Invalid value. at offset 0", util::toString(error));
+        EXPECT_EQ("on line 1 col 0: Invalid value.", util::toString(error));
         test.end();
     };