Skip to content
This repository has been archived by the owner on Mar 25, 2024. It is now read-only.

Support emitting spans when deserializing structures #201

Closed
wants to merge 3 commits into from

Conversation

Michael-F-Bryan
Copy link

Similar to toml-rs/toml-rs#236, serde-rs/json#637, and serde-rs/serde#1811, this provides a mechanism for extracting the source location for a particular value. I've mostly copied the toml crate's approach.

For more complex cases like mappings and sequences the spans don't look 100% correct (e.g. it may miss a closing ] or mappings start at : value instead of key: value), but that may just be because of where yaml_rust decides to emit an event or because yaml_rust::scanner::Marker only records where an event starts and not its length.

In the long run I'd like to have a common mechanism/protocol for emitting span information in serde (outlined here), but in the meantime this should give us a good place to experiment.

@Michael-F-Bryan
Copy link
Author

I've added some more tests and believe this is feature complete. @dtolnay, would you be able to review this PR?

There's a quirk when trying to get the start location of a map. For example, given this document...

document:
  first: 1
  second: 2

... and this struct definition...

#[derive(Debug, serde_derive::Deserialize)]
struct Document {
    nested: Spanned<Nested>,
}

#[derive(Debug, serde_derive::Deserialize)]
struct Nested {
   first: u32,
   second: u32,
}

... Our Nested item will seem to start at the colon after the first key instead of at the f character as one might expect.

I think this may have something to do with how maps are deserialized and when tokens are consumed, but I don't know how to resolve the issue cleanly. I wrote a doc-test so people are aware of this behaviour and to help detect when it is fixed in the future.

@Michael-F-Bryan
Copy link
Author

@dtolnay other than making it compatible with Rust 1.31, is anything else blocking this PR from being merged?

emk added a commit to faradayio/openapi-interfaces that referenced this pull request Sep 23, 2021
serde and serde_yaml generate terrible error messages for untagged
enums. We can fix this with manual deserialization.

Or at least improve it.

serde-rs/serde#773
serde-rs/serde#741
dtolnay/serde-yaml#201
@dccsillag
Copy link

This would be great to have!
Hope it gets merged.

Copy link
Owner

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

I'd prefer not to build this into serde_yaml. For now I would recommend using a different crate if you need the spans.

Thanks anyway for the PR!

@dtolnay dtolnay closed this Feb 17, 2022
@dccsillag
Copy link

dccsillag commented Feb 17, 2022

Oh, I see. Thank you for your time :)

But is there even a YAML crate out there which supports this?

@Michael-F-Bryan
Copy link
Author

But is there even a YAML crate out there which supports this?

Not that I could find. Your best bet is to fork serde_yaml and implement it yourself.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants