Incorrect namespace in C++ to/from json (nlohmann)

[hemantvallabh]

The to_json/from_json functions should be in the same namespace as the structure. I'm using Visual Studio 2017 (VC++)

Additional information: nlohmann/json#780 (comment)

QuickType Code Gen

#pragma once

#include "json.hpp"

namespace quicktype {
    using nlohmann::json;

    inline json get_untyped(const json & j, const char * property) {
        if (j.find(property) != j.end()) {
            return j.at(property).get<json>();
        }
        return json();
    }

    inline json get_untyped(const json & j, std::string property) {
        return get_untyped(j, property.data());
    }

    struct Welcome {
        int64_t response_code;
        std::string response_message;
    };
}

namespace nlohmann {
    void from_json(const json & j, quicktype::Welcome & x);
    void to_json(json & j, const quicktype::Welcome & x);

    inline void from_json(const json & j, quicktype::Welcome& x) {
        x.response_code = j.at("response_code").get<int64_t>();
        x.response_message = j.at("response_message").get<std::string>();
    }

    inline void to_json(json & j, const quicktype::Welcome & x) {
        j = json::object();
        j["response_code"] = x.response_code;
        j["response_message"] = x.response_message;
    }
}

Working Code

#pragma once
namespace quicktype {
	using nlohmann::json;

	inline json get_untyped(const json & j, const char * property) {
		if (j.find(property) != j.end()) {
			return j.at(property).get<json>();
		}
		return json();
	}

	inline json get_untyped(const json & j, std::string property) {
		return get_untyped(j, property.data());
	}

	struct Welcome {
		int64_t response_code;
		std::string response_message;
	};

	void from_json(const json & j, quicktype::Welcome & x);
	void to_json(json & j, const quicktype::Welcome & x);

	inline void from_json(const json & j, quicktype::Welcome& x) {
		x.response_code = j.at("response_code").get<int64_t>();
		x.response_message = j.at("response_message").get<std::string>();
	}

	inline void to_json(json & j, const quicktype::Welcome & x) {
		j = json::object();
		j["response_code"] = x.response_code;
		j["response_message"] = x.response_message;
	}
}

[smalhotra-spirent]

I am facing a similar issue with Visual Studio 2017. What I have done is to add an additional useTopLevelNamespace option for C++. This basically reuses the top level namespace provided for wrapping to_json and from_json functions.

--[no-]useTopLevelNamespace                       Moves to_json and from_json functions under the top level namespace (off by default)

I can create a PR for this, but haven't had time to run tests.

Here's the diff:

commit 3053f81167b8686ce4646a977bfcdb63463cde40 (HEAD -> smalhotra/devel)
Author: Sumeet Malhotra <[email protected]>
Date:   Fri Jun 28 11:23:00 2019 +0530

    Add useTopLevelNamespace option to all VS2017 compiles

diff --git a/src/quicktype-core/language/CPlusPlus.ts b/src/quicktype-core/language/CPlusPlus.ts
index 20a5c7cd..c628d868 100644
--- a/src/quicktype-core/language/CPlusPlus.ts
+++ b/src/quicktype-core/language/CPlusPlus.ts
@@ -73,6 +73,10 @@ export const cPlusPlusOptions = {
         "not-permissive",
         "secondary"
     ),
+    useTopLevelNamespace: new BooleanOption(
+        "useTopLevelNamespace",
+        "Moves to_json and from_json functions under the top level namespace",
+        false),
     westConst : new EnumOption(
       "const-style",
       "Put const to the left/west (const T) or right/east (T const)",
@@ -121,6 +125,7 @@ export class CPlusPlusTargetLanguage extends TargetLanguage {
             cPlusPlusOptions.codeFormat,
             cPlusPlusOptions.wstring,
             cPlusPlusOptions.msbuildPermissive,
+            cPlusPlusOptions.useTopLevelNamespace,
             cPlusPlusOptions.westConst,
             cPlusPlusOptions.typeSourceStyle,
             cPlusPlusOptions.includeLocation,
@@ -2050,6 +2055,9 @@ export class CPlusPlusRenderer extends ConvenienceRenderer {
             if (this._options.msbuildPermissive) {
                 namespaces = ["nlohmann", "detail"];
             }
+            if (this._options.useTopLevelNamespace) {
+                namespaces = this._namespaceNames.slice();
+            }
             this.emitNamespaces(namespaces, () => {
                 this.forEachObject("leading-and-interposing", (_: any, className: Name) =>
                     this.emitClassHeaders(className)

[llilakoblock]

Confirmed in Visual Studio 2019.

[jushar]

@schani As to the linked issue (nlohmann/json#780 (comment)) the fix described by @smalhotra-spirent correctly solves the problem and better complies with the C++ standard.

His fix only adds a new option, thus it can't even break backwards compatibility and is the only chance to get it to compile with the latest Microsoft compiler (I think after a short test phase, it should get the default though).

If it helps to speed things up, I could also create a pull request with @smalhotra-spirent's fix.

[LeStahL]

Using MSVC-2022 with c++-20 enabled resolves the problem.

[SylvainFogale]

While the solution is very simple, the issue cause recurring support in the popular Nlohmann repo
nlohmann/json#3280
Indeed the generated code need to be corrected so that the to and from are not in the lib namespace, or at least in the generated header comments, please mention the possibility that the compilation error "no matching overloaded function found when implementing from_json function" relate to the choice of namespace where "to" and "from" are implemented in the autogenerated code.
The combination of the quicktype tool and the Nlohmann is a very powerfull tool that should gain in popularity but we can't evaluate how many users could be discouraged at the first occurence of the issue !