json icon indicating copy to clipboard operation
json copied to clipboard

Add `enum RawKind` and `is_*` to `RawValue`

Open bheylin opened this issue 1 year ago • 3 comments

I mentioned the proc-macro based JSON processor I'm using in PR #1221. This processor ultimately converts the JSON to a Deserializable Rust struct. While converting, it has specialized null handling and custom Errors for missing fields and such.

To accomplish this I currently use an extension trait to provide methods that report what JSON kind (null, bool, string, etc) the RawValue contains and a set of tests to assert the invariants of a RawValue. Invariants such as:

  • A RawValue has whitespace stripped during deserialization
  • An empty string can't be deserialized into a RawValue.

I think knowing the type of a RawValue can be useful to all users of serde_json.

What do you think?

bheylin avatar Dec 11 '24 20:12 bheylin

Value has similar is_... methods as well. Maybe there could be a common ValueKind (or similar) used for both, instead of adding this specific RawKind (and risking that it needs to be duplicated / renamed in the future)?

(This is just an idea from a drive-by user though; would be good to wait for the opinion of the maintainer before acting on this.)

Marcono1234 avatar Jan 01 '25 23:01 Marcono1234

Value has similar is_... methods as well. Maybe there could be a common ValueKind (or similar) used for both, instead of adding this specific RawKind (and risking that it needs to be duplicated / renamed in the future)?

(This is just an idea from a drive-by user though; would be good to wait for the opinion of the maintainer before acting on this.)

Hi @Marcono1234, thanks for your comment.

I don't see the benefit in merging the Value and RawKind. Besides the similar variants, they have a fundamentally different purpose. Value stores the value of the field while RawKind simply states what kind of value is stored in the field.

Merging the Value and RawKind enums would result in breaking changes in the much used Value enum and add complexity to the Value type that isn't justified.

(and risking that it needs to be duplicated / renamed in the future)?

Why do you think RawKind would need to be duplicated or renamed in the future?

bheylin avatar Jan 05 '25 22:01 bheylin

Ah sorry, I did not mean merging Value and RawKind, but rather have a ValueKind (or similar) which is used by Value and RawValue, e.g.:

  • Value::kind(&self) -> ValueKind
  • RawValue::kind(&self) -> ValueKind

Because Value already has methods such as Value::is_object(&self), it is conceivable that users might also want a Value::kind(&self) method in the future. So it might be useful to already to consider this now. Otherwise if this PR here adds RawKind, reusing it for a potential future Value::kind(&self) might not work or might be confusing.

Though as mentioned above, would be good to wait for the opinion of the maintainer on this.

Marcono1234 avatar Jan 09 '25 22:01 Marcono1234