NLOHMANN_JSON_SERIALIZE_ENUM defaulting to the first enum value is surprising behavior

[bear24rw]

Description

I had a json document with an invalid enum string. I would like an exception to be raised if the string is not a known conversion back to the enum. I don't want to have to add and check an Unknown or Invalid enum value which would only be used for json serialization.

This is well documented: "undefined JSON values will default to the first specified conversion." but is IMO is surprising behavior since we throw json.exception.type_error's for other type conversions if they fail (like converting an integer in the json document to a std::string)

This is maybe only a good idea for strongly typed enum class's.

Reproduction steps

namespace ns
{
enum class Color
{
    red, green, blue // I don't want to define an unknown item just used for json deserialization
};

NLOHMANN_JSON_SERIALIZE_ENUM(Color,
{
    { Color::red, "red" },
    { Color::green, "green" },
    { Color::blue, "blue" }
})
}

json some_color_json = "pink";
try {
    auto color = some_color_json.get<ns::Color>();
} catch (json::type_error& e) {
    printf("Invalid color!\n");
}

Expected vs. actual results

Expected: It prints 'invalid color'
Actual: color gets set to Color::red

I believe the behavior I am after can be achieved by with this patch

diff --git a/include/nlohmann/detail/macro_scope.hpp b/include/nlohmann/detail/macro_scope.hpp
index 6248bea..9916aac 100644
--- a/include/nlohmann/detail/macro_scope.hpp
+++ b/include/nlohmann/detail/macro_scope.hpp
@@ -215,7 +215,9 @@
         {                                                                                       \
             return ej_pair.first == e;                                                          \
         });                                                                                     \
-        j = ((it != std::end(m)) ? it : std::begin(m))->second;                                 \
+        if (it == std::end(m))
+            throw ...
+        j = it;
     }                                                                                           \
     template<typename BasicJsonType>                                                            \
     inline void from_json(const BasicJsonType& j, ENUM_TYPE& e)                                 \
@@ -227,7 +229,9 @@
         {                                                                                       \
             return ej_pair.second == j;                                                         \
         });                                                                                     \
-        e = ((it != std::end(m)) ? it : std::begin(m))->first;                                  \
+        if (it == std::end(m))
+            throw ...
+        e = it;
     }
 
 // Ugly macros to avoid uglier copy-paste when specializing basic_json. They

Minimal code example

No response

Error messages

No response

Compiler and operating system

clang 15.0.7

Library version

v3.11.2

Validation

• The bug also occurs if the latest version from the develop branch is used.
• I can successfully compile and run the unit tests.

[nlohmann]

I removed the bug label as this is documented behavior. We cannot change this behavior as it would be a breaking change. I could think of a different macro to achieve this behavior though.

[bear24rw]

We cannot change this behavior as it would be a breaking change.

I agree. I think there is definitely a legitimate usecase for the current behavior for weakly typed enums which might be commonly used with implicit (un-named) enum values, for example:

enum State {
    INIT = 0
    SETUP = 1
    STEP = 2
    // bunch of implicit steps
    DONE = 10
};

if (step == State::STEP + 4) ...

It's less likely someone would be using an enum class in the same way but it's valid.

I could think of a different macro to achieve this behavior though.

It's almost like the current NLOHMANN_JSON_SERIALIZE_ENUM macro works like NLOHMANN_DEFINE_TYPE_*_WITH_DEFAULT. It's too bad we can't rename the exist macro to NLOHMANN_JSON_SERIALIZE_ENUM_WITH_DEFAULT.

Either way I'd be very eager to use a new more strict macro!

[bear24rw]

I could think of a different macro to achieve this behavior though.

Some potential naming options for a new enum courtesy of chatGPT

NLOHMANN_JSON_SERIALIZE_ENUM_STRICT
NLOHMANN_JSON_SERIALIZE_ENUM_CHECKED
NLOHMANN_JSON_SERIALIZE_ENUM_SAFE
NLOHMANN_JSON_SERIALIZE_ENUM_ENSURE
NLOHMANN_JSON_SERIALIZE_ENUM_VALIDATE
NLOHMANN_JSON_SERIALIZE_ENUM_STRICT_SERIALIZATION
NLOHMANN_JSON_SERIALIZE_ENUM_SAFE_DESERIALIZATION
NLOHMANN_JSON_SERIALIZE_ENUM_CHECKED_CONVERSION
NLOHMANN_JSON_SERIALIZE_ENUM_ENSURE_MAPPING
NLOHMANN_JSON_SERIALIZE_ENUM_VALIDATE_VALUES

[gregmarr]

I like NLOHMANN_JSON_SERIALIZE_ENUM_STRICT.

[pabloariasal]

just opened a MR for NLOHMANN_JSON_SERIALIZE_ENUM_STRICT

[bear24rw]

@pabloariasal why was #3996 closed? I just ran into this issue again and am looking forward to getting your PR merged.

[nlohmann]

Same here.

[zagoli]

Same

[nlohmann]

Any update on this? @pabloariasal

[bitFiedler]

When could I expect this feature, would be great to have it.