Skip to content
This repository has been archived by the owner on Sep 24, 2022. It is now read-only.

values must be emitted before tables / ValueAfterTable error when using a HashMap #142

Closed
shepmaster opened this issue Feb 10, 2017 · 9 comments

Comments

@shepmaster
Copy link
Contributor

Trying to parse and then generate a Cargo.toml, I'm using the master branch with the new Serde 0.9 support and I'd like to parse simple and complex dependencies. This works ok for parsing them, but not writing:

extern crate serde;
#[macro_use]
extern crate serde_derive;
extern crate toml;

use std::collections::BTreeMap;

#[derive(Debug, PartialEq, Serialize, Deserialize)]
#[serde(untagged)]
enum Dependency {
    Short(String),
    Long {
        version: String,
        #[serde(default)]
        features: Vec<String>
    },
}

fn main() {
    let mut dependencies = BTreeMap::new();

    dependencies.insert(
        "void".to_string(),
        Dependency::Short("1.0.2".to_string())
    );
    dependencies.insert(
        "serde".to_string(),
        Dependency::Long {
            version: "0.9.7".to_string(),
            features: vec!["playground".to_string()]
        }
    );

    let s = toml::to_string(&dependencies).unwrap();
    println!("{}", s);
}

Do I really need to partition my dependencies before writing them out? Is there a cleaner method?

@alexcrichton
Copy link
Collaborator

Yeah this is one aspect of the Serialize implementation that I wasn't happy with unfortunately.

TOML as a format requires that if a table has non-table keys they come first, and then all table keys are emitted. This means that at a fundamental level that ordering has to happen somewhere. For a generic BTreeMap it will serialize in order of iteration, which isn't always desired.

Some possible solutions here could entail:

  • Buffer all serialized tables in memory until a table is finished. I'd ideally prefer to avoid this as it's unidiomatic in Serde, though
  • Add the ability to configure serialization and emit inline tables here instead of full tables (which are also values)
  • Require callers to order correctly.

The last is obviously the easiest to implement, but also the least ergonomic! I'm not entirely sure what the ideal solution would be here... One thing you can do for now to work around this is to serialize to a toml::Value and then serialize that to a string. That should always work.

@shepmaster
Copy link
Contributor Author

@dtolnay hooked me up with a reimplementation of my code that uses Cargo as a library instead of via an executable, so I'm no longer personally impacted by this issue. Feel free to prioritize as appropriate. Perhaps the first step is to document this somewhere, but it's unclear where the right place would be!

It's good to know about the trick to serialize to a toml::Value though!

@dtolnay
Copy link
Contributor

dtolnay commented Feb 10, 2017

An idiomatic Serde solution would be to provide a serialize_with function that handles the ordering on demand. This avoids any buffering.

struct Manifest {
    package: Package,
    #[serde(serialize_with = "toml::ser::tables_last")]
    dependencies: Map<String, Dependency>,
}

We can do this backward-compatibly after the release of 0.3.

Something like:

enum Category {
    Primitive,
    Array,
    Table,
}

pub fn tables_last<'a, I, K, V, S>(data: &'a I, serializer: S) -> Result<S::Ok, S::Error>
    where &'a I: IntoIterator<Item = (K, V)>,
          K: Serialize,
          V: Serialize,
          S: Serializer
{
    let mut map = serializer.serialize_map(None)?;
    for (k, v) in data {
        if let Category::Primitive = v.serialize(Categorize::new())? {
            map.serialize_entry(&k, &v)?;
        }
    }
    for (k, v) in data {
        if let Category::Array = v.serialize(Categorize::new())? {
            map.serialize_entry(&k, &v)?;
        }
    }
    for (k, v) in data {
        if let Category::Table = v.serialize(Categorize::new())? {
            map.serialize_entry(&k, &v)?;
        }
    }
    map.end()
}

struct Categorize<E>(PhantomData<E>);

impl<E> Categorize<E> {
    fn new() -> Self {
        Categorize(PhantomData)
    }
}

impl<E: ser::Error> Serializer for Categorize<E> {
    type Ok = Category;
    type Error = E;
    type SerializeSeq = Self;
    type SerializeTuple = Impossible<Category, E>;
    type SerializeTupleStruct = Impossible<Category, E>;
    type SerializeTupleVariant = Impossible<Category, E>;
    type SerializeMap = Self;
    type SerializeStruct = Self;
    type SerializeStructVariant = Impossible<Category, E>;
    fn serialize_bool(self, _: bool) -> Result<Self::Ok, Self::Error> { Ok(Category::Primitive) }
    fn serialize_i8(self, _: i8) -> Result<Self::Ok, Self::Error> { Ok(Category::Primitive) }
    fn serialize_i16(self, _: i16) -> Result<Self::Ok, Self::Error> { Ok(Category::Primitive) }
    fn serialize_i32(self, _: i32) -> Result<Self::Ok, Self::Error> { Ok(Category::Primitive) }
    fn serialize_i64(self, _: i64) -> Result<Self::Ok, Self::Error> { Ok(Category::Primitive) }
    fn serialize_u8(self, _: u8) -> Result<Self::Ok, Self::Error> { Ok(Category::Primitive) }
    fn serialize_u16(self, _: u16) -> Result<Self::Ok, Self::Error> { Ok(Category::Primitive) }
    fn serialize_u32(self, _: u32) -> Result<Self::Ok, Self::Error> { Ok(Category::Primitive) }
    fn serialize_u64(self, _: u64) -> Result<Self::Ok, Self::Error> { Ok(Category::Primitive) }
    fn serialize_f32(self, _: f32) -> Result<Self::Ok, Self::Error> { Ok(Category::Primitive) }
    fn serialize_f64(self, _: f64) -> Result<Self::Ok, Self::Error> { Ok(Category::Primitive) }
    fn serialize_char(self, _: char) -> Result<Self::Ok, Self::Error> { Ok(Category::Primitive) }
    fn serialize_str(self, _: &str) -> Result<Self::Ok, Self::Error> { Ok(Category::Primitive) }
    fn serialize_bytes(self, _: &[u8]) -> Result<Self::Ok, Self::Error> { Err(ser::Error::custom("unsupported")) }
    fn serialize_none(self) -> Result<Self::Ok, Self::Error> { Err(ser::Error::custom("unsupported")) }
    fn serialize_some<T: ?Sized + Serialize>(self, v: &T) -> Result<Self::Ok, Self::Error> { v.serialize(self) }
    fn serialize_unit(self) -> Result<Self::Ok, Self::Error> { Err(ser::Error::custom("unsupported")) }
    fn serialize_unit_struct(self, _: &'static str) -> Result<Self::Ok, Self::Error> { Err(ser::Error::custom("unsupported")) }
    fn serialize_unit_variant(self, _: &'static str, _: usize, _: &'static str) -> Result<Self::Ok, Self::Error> { Err(ser::Error::custom("unsupported")) }
    fn serialize_newtype_struct<T: ?Sized + Serialize>(self, _: &'static str, v: &T) -> Result<Self::Ok, Self::Error> { v.serialize(self) }
    fn serialize_newtype_variant<T: ?Sized + Serialize>(self, _: &'static str, _: usize, _: &'static str, _: &T) -> Result<Self::Ok, Self::Error> { Err(ser::Error::custom("unsupported")) }
    fn serialize_seq(self, _: Option<usize>) -> Result<Self, Self::Error> { Ok(self) }
    fn serialize_seq_fixed_size(self, _: usize) -> Result<Self, Self::Error> { Ok(self) }
    fn serialize_tuple(self, _: usize) -> Result<Self::SerializeTuple, Self::Error> { Err(ser::Error::custom("unsupported")) }
    fn serialize_tuple_struct(self, _: &'static str, _: usize) -> Result<Self::SerializeTupleStruct, Self::Error> { Err(ser::Error::custom("unsupported")) }
    fn serialize_tuple_variant(self, _: &'static str, _: usize, _: &'static str, _: usize) -> Result<Self::SerializeTupleVariant, Self::Error> { Err(ser::Error::custom("unsupported")) }
    fn serialize_map(self, _: Option<usize>) -> Result<Self, Self::Error> { Ok(self) }
    fn serialize_struct(self, _: &'static str, _: usize) -> Result<Self, Self::Error> { Ok(self) }
    fn serialize_struct_variant(self, _: &'static str, _: usize, _: &'static str, _: usize) -> Result<Self::SerializeStructVariant, Self::Error> { Err(ser::Error::custom("unsupported")) }
}

impl<E: ser::Error> SerializeSeq for Categorize<E> {
    type Ok = Category;
    type Error = E;
    fn serialize_element<T: ?Sized + Serialize>(&mut self, _: &T) -> Result<(), Self::Error> { Ok(()) }
    fn end(self) -> Result<Self::Ok, Self::Error> { Ok(Category::Array) }
}

impl<E: ser::Error> SerializeMap for Categorize<E> {
    type Ok = Category;
    type Error = E;
    fn serialize_key<T: ?Sized + Serialize>(&mut self, _: &T) -> Result<(), Self::Error> { Ok(()) }
    fn serialize_value<T: ?Sized + Serialize>(&mut self, _: &T) -> Result<(), Self::Error> { Ok(()) }
    fn end(self) -> Result<Self::Ok, Self::Error> { Ok(Category::Table) }
}

impl<E: ser::Error> SerializeStruct for Categorize<E> {
    type Ok = Category;
    type Error = E;
    fn serialize_field<T: ?Sized + Serialize>(&mut self, _: &'static str, _: &T) -> Result<(), Self::Error> { Ok(()) }
    fn end(self) -> Result<Self::Ok, Self::Error> { Ok(Category::Table) }
}

@alexcrichton
Copy link
Collaborator

@dtolnay one of these days I'm going to have a problem that you can't immediately solve, and I fear for it greatly

@alexcrichton
Copy link
Collaborator

But more seriously sounds like a great solution to me, thanks!

@joliss
Copy link

joliss commented Feb 27, 2017

I think I'm running into this issue. Can we get a release for this please?

@alexcrichton
Copy link
Collaborator

Sure!

@Byron
Copy link

Byron commented Apr 8, 2021

Interestingly, with resolver = "2" I saw the same error messages when trying to publish git-hash. Setting the resolver back to 1 fixed the issue.

@tyranron
Copy link

tyranron commented Apr 8, 2021

@Byron same issue: rust-lang/cargo#9337

Flouse pushed a commit to Flouse/godwoken that referenced this issue Nov 24, 2021
- The key of all TOML maps must be strings.
- fix ValueAfterTable error when using a HashMap
> toml-rs/toml-rs#142 (comment)
Flouse pushed a commit to Flouse/godwoken that referenced this issue Nov 24, 2021
- The key of all TOML maps must be strings.
- fix ValueAfterTable error when using a HashMap
> toml-rs/toml-rs#142 (comment)
jjyr pushed a commit to godwokenrises/godwoken that referenced this issue Nov 28, 2021
- The key of all TOML maps must be strings.
- fix ValueAfterTable error when using a HashMap
> toml-rs/toml-rs#142 (comment)
gcochard added a commit to gcochard/lpc55-host that referenced this issue Apr 29, 2022
nickray pushed a commit to lpc55/lpc55-host that referenced this issue Sep 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants