nlohmann/jsonPublic

NotificationsYou must be signed in to change notification settings
Fork7.3k
Star48.9k

Comments

add strict enum de/serialization macro#4612

Open

hnampally wants to merge 13 commits intonlohmann:developfrom

hnampally:issue-3992

Open

add strict enum de/serialization macro#4612
hnampally wants to merge 13 commits intonlohmann:developfrom
hnampally:issue-3992

Conversation

Copy link

Contributor

hnampally commentedJan 20, 2025•
edited
Loading

#3992

The changes are described in detail, both the what and why.
If applicable, an#3992 is referenced.
TheCode coverage remained at 100%. A test case for every new line of code.
If applicable, thedocumentation is updated.
The source code is amalgamated by runningmake amalgamate.

Read theContribution Guidelines for detailed information.

hnampally requested a review fromnlohmann as acode owner

January 20, 2025 04:53

github-actionsbot added documentation L tests labels

Jan 20, 2025

add strict enum de/serialization macro

06b667c

Signed-off-by: Harinath Nampally <harinath922@gmail.com>

hnampally force-pushed theissue-3992 branch from0f7ec10 to06b667cCompare

January 20, 2025 04:56

Copy link

github-actionsbot commentedJan 20, 2025

🔴 Amalgamation check failed! 🔴

The source code has not been amalgamated.@hnampally
Please read and follow theContribution Guidelines.

Copy link

coveralls commentedJan 20, 2025•
edited
Loading

coverage: 99.186%. remained the same
when pulling6747555 on hnampally:issue-3992
into606b634 on nlohmann:develop.

hnampally added3 commits

January 20, 2025 00:40

fix ci checks

4ebebad

Signed-off-by: Harinath Nampally <harinath922@gmail.com>

fix few more ci checks

0083937

Signed-off-by: Harinath Nampally <harinath922@gmail.com>

fix typos

c6d9ea0

Signed-off-by: Harinath Nampally <harinath922@gmail.com>

nlohmann reviewed

Jan 20, 2025

View reviewed changes

docs/mkdocs/docs/api/macros/nlohmann_json_serialize_enum _strict.md OutdatedShow resolvedHide resolved

docs/mkdocs/docs/examples/nlohmann_json_deserialize_enum_strict.cpp OutdatedShow resolvedHide resolved

nlohmann requested changes

Jan 21, 2025

View reviewed changes

docs/mkdocs/docs/examples/nlohmann_json_deserialize_enum_strict.cpp OutdatedShow resolvedHide resolved

hnampally added3 commits

January 25, 2025 09:31

address review comment

4444985

Signed-off-by: Harinath Nampally <harinath922@gmail.com>

fix ununsed variable static check

e7c021f

Signed-off-by: Harinath Nampally <harinath922@gmail.com>

ci static check fix

32cbaee

Signed-off-by: Harinath Nampally <harinath922@gmail.com>

gregmarr reviewed

Jan 26, 2025

View reviewed changes

include/nlohmann/detail/macro_scope.hpp OutdatedShow resolvedHide resolved

hnampally force-pushed theissue-3992 branch from9dd05aa toe17bf40Compare

January 26, 2025 22:03

Copy link

github-actionsbot commentedJan 26, 2025

🔴 Amalgamation check failed! 🔴

The source code has not been amalgamated.@hnampally
Please read and follow theContribution Guidelines.

hnampally force-pushed theissue-3992 branch frome17bf40 toc2d0b8bCompare

January 26, 2025 22:17

address review comment

2898659

Signed-off-by: Harinath Nampally <harinath922@gmail.com>

hnampally force-pushed theissue-3992 branch fromc2d0b8b to2898659Compare

January 26, 2025 22:21

gregmarr reviewed

Jan 26, 2025

View reviewed changes

docs/mkdocs/docs/examples/nlohmann_json_serialize_enum_strict.cpp OutdatedShow resolvedHide resolved

gregmarr reviewed

Jan 26, 2025

View reviewed changes

docs/mkdocs/docs/features/enum_conversion.md OutdatedShow resolvedHide resolved

gregmarr reviewed

Jan 26, 2025

View reviewed changes

include/nlohmann/detail/macro_scope.hpp OutdatedShow resolvedHide resolved

address review comments

11165ef

Signed-off-by: Harinath Nampally <harinath922@gmail.com>

hnampally force-pushed theissue-3992 branch from4002093 toba344e0Compare

January 26, 2025 23:38

fix ci check

0cda972

Signed-off-by: Harinath Nampally <harinath922@gmail.com>

hnampally force-pushed theissue-3992 branch fromba344e0 to0cda972Compare

January 27, 2025 02:24

nlohmann requested changes

Jan 27, 2025

View reviewed changes

include/nlohmann/detail/macro_scope.hpp Outdated

		template<typename T>
		[[noreturn]] inline void json_throw_from_serialize_macro(T&& exception)
		{
		#if defined(__cpp_exceptions) \|\| defined(__EXCEPTIONS) \|\| defined(_CPPUNWIND) \|\| defined(EXCEPTIONS)

Copy link

Owner

nlohmannJan 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

When definingJSON_THROW in the same file, we use the following code to detect exceptions. Please use the same here to avoid issues.

#if (defined(__cpp_exceptions) || defined(__EXCEPTIONS) || defined(_CPPUNWIND)) && !defined(JSON_NOEXCEPTION)

Copy link

ContributorAuthor

hnampallyJan 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

oh okay, thanks!

include/nlohmann/detail/macro_scope.hpp Outdated

		}); \
		if (it == std::end(m)) { \
		auto value = static_cast<typename std::underlying_type<ENUM_TYPE>::type>(e); \
		nlohmann::detail::json_throw_from_serialize_macro(nlohmann::detail::type_error::create(302, nlohmann::detail::concat("can't serialize - enum value ", std::to_string(value), " out of range"), &j)); \

Copy link

Owner

nlohmannJan 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I don't like the exception message. What about

serialization failed: enum value ... is out of range

Copy link

ContributorAuthor

hnampallyJan 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Yes that sounds better, thanks!

include/nlohmann/detail/macro_scope.hpp Outdated

		return ej_pair.second == j; \
		}); \
		if (it == std::end(m)) \
		nlohmann::detail::json_throw_from_serialize_macro(nlohmann::detail::type_error::create(302, nlohmann::detail::concat("can't deserialize - invalid json value : ", j.dump()), &j)); \

Copy link

Owner

nlohmannJan 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Same as above:

deserialization failed: invalid JSON value '...'

address review comments

e25fc04

Signed-off-by: Harinath Nampally <harinath922@gmail.com>

hnampally force-pushed theissue-3992 branch from3a55919 toe25fc04Compare

January 28, 2025 02:49

gregmarr reviewed

Jan 28, 2025

View reviewed changes

docs/mkdocs/docs/api/macros/nlohmann_json_serialize_enum _strict.md OutdatedShow resolvedHide resolved

docs/mkdocs/docs/api/macros/nlohmann_json_serialize_enum _strict.md Outdated


		- If an enum value appears more than once in the mapping, only the first occurrence will be used for serialization,
		subsequent mappings for the same enum value will be ignored.
		- If a JSON value appears more than once in the mapping, only the first occurrence will be used for deserialization,

Copy link

Contributor

gregmarrJan 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

"If a string value" since it's not an arbitrary json value but a string....

Oh, OH, it doesn't require that it be a string, does it?

😮

There's nothing in the code that requires that this be a string.

NLOHMANN_JSON_SERIALIZE_ENUM_STRICT(Color, {  { Red, "red" },  { Green, nlohmann::json(1) },  { Blue, nlohmann::json({{{"purple", "people eater"}}})}})int main(){  nlohmann::json j;  j = { Red, Green, Blue };  std::cout << j.dump() << "\n";  auto j2 = nlohmann::json::parse(j.dump());  std::vector<Color> vec = j2;  std::cout << vec[0] << vec[1] << vec[2];}

["red",1,[{"purple":"people eater"}]]012

@hnampally Is this intended behavior, or did this just accidentally fall out of the second parameter beingBasicJsonType instead ofstd::string?

If this is intended behavior, it should definitely be documented before someone trips upon it accidentally, or someone relies on it and then someone else "fixes" it.

If it's not intended behavior, then it should be fixed.

further documentation improvements

5f7d41c

Signed-off-by: Harinath Nampally <harinath922@gmail.com>

hnampally force-pushed theissue-3992 branch from00990b1 to5f7d41cCompare

January 29, 2025 03:04

hnampally requested a review fromnlohmann

January 31, 2025 03:13

nlohmann requested changes

Jan 31, 2025

View reviewed changes

docs/mkdocs/docs/api/macros/nlohmann_json_serialize_enum _strict.md

		Expected output:

		```
		[json.exception.type_error.302] serialization failed: enum value 3 is out of range

Copy link

Owner

nlohmannJan 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Also use an include here:

--8<-- "examples/nlohmann_json_serialize_enum_strict.output"

docs/mkdocs/docs/api/macros/nlohmann_json_serialize_enum _strict.md

		Expected output:

		```
		[json.exception.type_error.302] deserialization failed: invalid JSON value "yellow"

Copy link

Owner

nlohmannJan 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Also use an include here:

--8<-- "examples/nlohmann_json_deserialize_enum_strict.output"

docs/mkdocs/docs/examples/nlohmann_json_deserialize_enum_strict.cpp Outdated


		int main()
		{

Copy link

Owner

nlohmannJan 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Remove the empty line.

Also: move line

json j_yellow ="yellow";

above the comment

// deserialization

Copy link

Contributor

gregmarr commentedJan 31, 2025

It looks like you have a space in your filename here:
docs/mkdocs/docs/api/macros/nlohmann_json_serialize_enum _strict.md

Also, the discussion about strings vs arbitrary json for the value has been marked outdated because there was a change in the file.@nlohmann did you see that discussion?

address review comments

6747555

Signed-off-by: Harinath Nampally <harinath922@gmail.com>

hnampally requested a review fromnlohmann

February 4, 2025 04:55

Copy link

jmonticelli commentedApr 1, 2025•
edited
Loading

This looks particularly useful, I was surprised by the behavior of the default enum serialization macro.
Thanks for putting this PR up btw@hnampally

Labels

documentation L tests

Movatterモバイル変換

Uh oh!

Comments

Conversation

hnampally commentedJan 20, 2025• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

github-actionsbot commentedJan 20, 2025

🔴 Amalgamation check failed! 🔴

Uh oh!

coveralls commentedJan 20, 2025• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actionsbot commentedJan 26, 2025

🔴 Amalgamation check failed! 🔴

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gregmarr commentedJan 31, 2025

Uh oh!

jmonticelli commentedApr 1, 2025• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

hnampally commentedJan 20, 2025•
edited
Loading

coveralls commentedJan 20, 2025•
edited
Loading

jmonticelli commentedApr 1, 2025•
edited
Loading