Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not serialize optional fields with a container attribute #947

Closed
iddm opened this issue Jun 2, 2017 · 16 comments
Closed

Do not serialize optional fields with a container attribute #947

iddm opened this issue Jun 2, 2017 · 16 comments

Comments

@iddm
Copy link

iddm commented Jun 2, 2017

I know about field attribute

#[serde(skip_serializing_if = "Option::is_none")]

But it is very common, for example, for json, not to serialize fields if they are empty. I have big structures with many optional types and it is very noisy to add this attribute to each of these fields. I propose to add one container attribute instead:

#[serde(skip_optionals)]
pub struct A {
    pub field: Option<i64>,  // will not be serialized as same as if skip_serializing_if = "Option::is_none"
}
@Enet4
Copy link

Enet4 commented Jun 2, 2017

This is opportune, since there have been a few questions lately on SO around using serde with optional or null-like fields [1, 2, 3]. This includes a question of your own, it seems. 🙂

I wouldn't be against a new attribute to make this use case a bit more elegant. The more I think of it, the more I realize how hard it is to come up with a good one. Nonetheless, skip_optionals seems to work fine, with the drawback that it cannot represent mandatory fields with optional values (for instance: a field that can be either string or null in JSON, but must not be missing). It also won't work with the solution here, where a custom type was used.

With all this in mind, I'm certainly interested in hearing from the Serde developers on this subject. 🙂

@oli-obk
Copy link
Member

oli-obk commented Jun 3, 2017

The problem is that we have no clue which fields have what type. This is similar to the serde borrow attribute. We only get a textual representation of the types. But that could be anything... a newtype with the same name, or a type alias, or some random type which looks like a known type.

We could do some best effort thing where we try to do the right thing for vec, option, boxed slice, string...

@iddm
Copy link
Author

iddm commented Jun 3, 2017

@oli-obk I don't quite understand what are you gonna say. With syn crate it is very easy to enumerate fields of a struct with their types. So, we do that and mark all the options with a boolean flag which says "skip serialization if it is Option and such key was not present in the json". I will do that by myself.

@dtolnay
Copy link
Member

dtolnay commented Jun 3, 2017

The concern was things like:

type MaybeString = Option<String>;

#[derive(Serialize)]
#[serde(skip_optionals)]
struct Vityafx {
    name: MaybeString,
}

It would be unfortunate to factor out the type of a field into a type declaration (which is never a breaking change) and realize later that it broke serialization because the field no longer looks like an Option.

@dtolnay
Copy link
Member

dtolnay commented Jun 3, 2017

In any case, I would be okay with adding a container attribute that does the best-effort version of this.

@iddm
Copy link
Author

iddm commented Jun 4, 2017

@dtolnay oh, okay, now I agree that this would be difficult to implement since we don't know what the real type is (if syn crate gives only name of the type as String)... Is there any chance to extend syn in that way?

@oli-obk
Copy link
Member

oli-obk commented Jun 4, 2017

@vityafx it is not a limitation of syn, but of the procedural derive macros offered by rustc. This won't change in the near future, but there are vague plans to improve it someday. I'm not sure if things like this will ever work, but there will be some improvements.

@dtolnay
Copy link
Member

dtolnay commented Jun 4, 2017

It is not possible to solve in general, and I don't know of any vague plans that involve this. You might have a derive like:

#[derive(Wow)]
struct Vityafx {
    name: MaybeString,
}

where the output of the derive is:

type MaybeString = Option<String>;

So in general it is impossible to know the type of anything until after macro expansion is all done. There is no API you could invent that would tell you the type of MaybeString because that type is defined later on by the macro.

@oli-obk
Copy link
Member

oli-obk commented Jun 4, 2017

I meant improving the derive macros to not work on strings anymore and having some form of communication with the compiler other than "please dump this text somewhere near the type declaration you gave me". No clue whether that will ever help in cases like these.

@Diggsey
Copy link

Diggsey commented Sep 12, 2017

Wouldn't it be possible to implement this with specialization:

trait ShouldSkip { fn should_skip(&self) -> bool }
impl<T> ShouldSkip for T {
    default fn should_skip(&self) { false }
}
impl<T> ShouldSkip for Option<T> {
    fn should_skip(&self) { self.is_none() }
}

Now you can implement the attribute as just adding #[serde(skip_serializing_if = "ShouldSkip ::should_skip")] to all the fields, regardless of their type.

@salim7
Copy link

salim7 commented Oct 28, 2018

Is there any update on this issue? I think Diggsey's suggestion could be used in combination with the initial suggestion of using #[serde(skip_optionals)] on the struct to achieve our goal perfectly :-)

@mmstick
Copy link

mmstick commented Dec 20, 2018

Also adding a ping here, as we use Rust and "JSON:API" at System76. Some JSON resources in our inventory system have a dozen or so possible attributes, most of which are Option<T>, or a String / Vec that can be empty, and should not be included during serialization.

@Kampfkarren
Copy link

+1, All of my structures use Option fields and creating a custom type for it is extremely noisy.

@dtolnay
Copy link
Member

dtolnay commented Jan 22, 2019

I would prefer for this to be handled by an attribute macro in a different library. I posted a request for implementation in dtolnay/request-for-implementation#18.

@doivosevic
Copy link

+1 for this feature

@leebenson
Copy link

https://docs.rs/serde_with/latest/serde_with/attr.skip_serializing_none.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

10 participants